From ff04755a5c0f3ccaa078d88b9682126ba9532da5 Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Mon, 16 Dec 2024 11:00:54 +0100 Subject: [PATCH 1/2] feat(gatekeeper): readOnly validation for version and commit creation --- .../modules/core/graph/resolvers/commits.ts | 5 + .../modules/core/graph/resolvers/versions.ts | 5 + packages/server/modules/core/hooks.ts | 9 +- packages/server/modules/core/index.ts | 3 +- .../tests/integration/commits.graph.spec.ts | 126 ++++++++++++++++++ .../tests/integration/versions.graph.spec.ts | 122 +++++++++++++++++ packages/server/modules/gatekeeper/index.ts | 23 ++-- 7 files changed, 280 insertions(+), 13 deletions(-) create mode 100644 packages/server/modules/core/tests/integration/commits.graph.spec.ts create mode 100644 packages/server/modules/core/tests/integration/versions.graph.spec.ts diff --git a/packages/server/modules/core/graph/resolvers/commits.ts b/packages/server/modules/core/graph/resolvers/commits.ts index 2d627822a..b2f1c35a4 100644 --- a/packages/server/modules/core/graph/resolvers/commits.ts +++ b/packages/server/modules/core/graph/resolvers/commits.ts @@ -82,6 +82,7 @@ import { getRegisteredDbClients } from '@/modules/multiregion/utils/dbSelector' import { LegacyUserCommit } from '@/modules/core/domain/commits/types' +import coreModule from '@/modules/core' const getStreams = getStreamsFactory({ db }) @@ -323,6 +324,10 @@ export = { }, Mutation: { async commitCreate(_parent, args, context) { + await coreModule.executeHooks('onCreateVersionRequest', { + projectId: args.commit.streamId + }) + const projectDb = await getProjectDbClient({ projectId: args.commit.streamId }) await authorizeResolver( context.userId, diff --git a/packages/server/modules/core/graph/resolvers/versions.ts b/packages/server/modules/core/graph/resolvers/versions.ts index dc7fdcf2c..a4cc7bec7 100644 --- a/packages/server/modules/core/graph/resolvers/versions.ts +++ b/packages/server/modules/core/graph/resolvers/versions.ts @@ -57,6 +57,7 @@ import { import { getObjectFactory } from '@/modules/core/repositories/objects' import { saveActivityFactory } from '@/modules/activitystream/repositories' import { getProjectDbClient } from '@/modules/multiregion/utils/dbSelector' +import coreModule from '@/modules/core' export = { Project: { @@ -177,6 +178,10 @@ export = { ctx.resourceAccessRules ) + await coreModule.executeHooks('onCreateVersionRequest', { + projectId: args.input.projectId + }) + const rateLimitResult = await getRateLimitResult('COMMIT_CREATE', ctx.userId!) if (isRateLimitBreached(rateLimitResult)) { throw new RateLimitError(rateLimitResult) diff --git a/packages/server/modules/core/hooks.ts b/packages/server/modules/core/hooks.ts index 894404cac..26ab2a1ac 100644 --- a/packages/server/modules/core/hooks.ts +++ b/packages/server/modules/core/hooks.ts @@ -4,11 +4,18 @@ type OnCreateObjectRequest = ({ projectId: string }) => Promise | void +type OnCreateVersionRequest = ({ + projectId +}: { + projectId: string +}) => Promise | void + export type HooksConfig = { onCreateObjectRequest: OnCreateObjectRequest[] + onCreateVersionRequest: OnCreateVersionRequest[] } -export type Hook = OnCreateObjectRequest +export type Hook = OnCreateObjectRequest | OnCreateVersionRequest export type ExecuteHooks = ( key: keyof HooksConfig, diff --git a/packages/server/modules/core/index.ts b/packages/server/modules/core/index.ts index 758f47588..e45cc5569 100644 --- a/packages/server/modules/core/index.ts +++ b/packages/server/modules/core/index.ts @@ -28,7 +28,8 @@ const coreModule: SpeckleModule<{ executeHooks: ExecuteHooks }> = { hooks: { - onCreateObjectRequest: [] + onCreateObjectRequest: [], + onCreateVersionRequest: [] }, addHook(key: keyof HooksConfig, callback: Hook) { this.hooks[key].push(callback) diff --git a/packages/server/modules/core/tests/integration/commits.graph.spec.ts b/packages/server/modules/core/tests/integration/commits.graph.spec.ts new file mode 100644 index 000000000..0d7eb0239 --- /dev/null +++ b/packages/server/modules/core/tests/integration/commits.graph.spec.ts @@ -0,0 +1,126 @@ +import { beforeEachContext } from '@/test/hooks' +import { expect } from 'chai' +import { describe, it } from 'mocha' +import { + createRandomEmail, + createRandomPassword +} from '@/modules/core/helpers/testHelpers' +import { + createUserEmailFactory, + ensureNoPrimaryEmailForUserFactory, + findEmailFactory +} from '@/modules/core/repositories/userEmails' +import { db } from '@/db/knex' +import { testApolloServer } from '@/test/graphqlHelper' +import { + CreateWorkspaceDocument, + CreateWorkspaceProjectDocument +} from '@/test/graphql/generated/graphql' +import { validateAndCreateUserEmailFactory } from '@/modules/core/services/userEmails' +import { finalizeInvitedServerRegistrationFactory } from '@/modules/serverinvites/services/processing' +import { + deleteServerOnlyInvitesFactory, + updateAllInviteTargetsFactory +} from '@/modules/serverinvites/repositories/serverInvites' +import { requestNewEmailVerificationFactory } from '@/modules/emails/services/verification/request' +import { deleteOldAndInsertNewVerificationFactory } from '@/modules/emails/repositories' +import { renderEmail } from '@/modules/emails/services/emailRendering' +import { sendEmail } from '@/modules/emails/services/sending' +import { + countAdminUsersFactory, + legacyGetUserFactory, + storeUserAclFactory, + storeUserFactory +} from '@/modules/core/repositories/users' +import { createUserFactory } from '@/modules/core/services/users/management' +import { UsersEmitter } from '@/modules/core/events/usersEmitter' +import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { WorkspaceReadOnlyError } from '@/modules/gatekeeper/errors/billing' +import gql from 'graphql-tag' + +const getServerInfo = getServerInfoFactory({ db }) +const getUser = legacyGetUserFactory({ db }) +const requestNewEmailVerification = requestNewEmailVerificationFactory({ + findEmail: findEmailFactory({ db }), + getUser, + getServerInfo, + deleteOldAndInsertNewVerification: deleteOldAndInsertNewVerificationFactory({ db }), + renderEmail, + sendEmail +}) + +const createUserEmail = validateAndCreateUserEmailFactory({ + createUserEmail: createUserEmailFactory({ db }), + ensureNoPrimaryEmailForUser: ensureNoPrimaryEmailForUserFactory({ db }), + findEmail: findEmailFactory({ db }), + updateEmailInvites: finalizeInvitedServerRegistrationFactory({ + deleteServerOnlyInvites: deleteServerOnlyInvitesFactory({ db }), + updateAllInviteTargets: updateAllInviteTargetsFactory({ db }) + }), + requestNewEmailVerification +}) + +const findEmail = findEmailFactory({ db }) +const createUser = createUserFactory({ + getServerInfo, + findEmail, + storeUser: storeUserFactory({ db }), + countAdminUsers: countAdminUsersFactory({ db }), + storeUserAcl: storeUserAclFactory({ db }), + validateAndCreateUserEmail: createUserEmail, + usersEventsEmitter: UsersEmitter.emit +}) + +const createCommitMutation = gql` + mutation CreateCommit($commit: CommitCreateInput!) { + commitCreate(commit: $commit) + } +` +describe('Commits graphql @core', () => { + before(async () => { + await beforeEachContext() + }) + + describe('Create commit mutation', () => { + it('should return error if project is read-only', async () => { + const userId = await createUser({ + name: 'emails user', + email: createRandomEmail(), + password: createRandomPassword() + }) + + const apollo = await testApolloServer({ authUserId: userId }) + + const workspaceCreateRes = await apollo.execute(CreateWorkspaceDocument, { + input: { name: 'test ws' } + }) + expect(workspaceCreateRes).to.not.haveGraphQLErrors() + + const workspace = workspaceCreateRes.data?.workspaceMutations.create + + const projectCreateRes = await apollo.execute(CreateWorkspaceProjectDocument, { + input: { workspaceId: workspace!.id, name: 'test project' } + }) + expect(projectCreateRes).to.not.haveGraphQLErrors() + const project = projectCreateRes.data?.workspaceMutations.projects.create + + // Make the project read-only + await db('workspace_plans') + .update({ status: 'canceled' }) + .where({ workspaceId: workspace!.id }) + + const versionCreateRes = await apollo.execute(createCommitMutation, { + commit: { + streamId: project!.id, + branchName: 'branch', + objectId: 'objectid' + } + }) + expect(versionCreateRes).to.haveGraphQLErrors() + expect(versionCreateRes.errors).to.have.length(1) + expect(versionCreateRes.errors![0].message).to.eq( + new WorkspaceReadOnlyError().message + ) + }) + }) +}) diff --git a/packages/server/modules/core/tests/integration/versions.graph.spec.ts b/packages/server/modules/core/tests/integration/versions.graph.spec.ts new file mode 100644 index 000000000..1624526ad --- /dev/null +++ b/packages/server/modules/core/tests/integration/versions.graph.spec.ts @@ -0,0 +1,122 @@ +import { beforeEachContext } from '@/test/hooks' +import { expect } from 'chai' +import { describe, it } from 'mocha' +import { + createRandomEmail, + createRandomPassword +} from '@/modules/core/helpers/testHelpers' +import { + createUserEmailFactory, + ensureNoPrimaryEmailForUserFactory, + findEmailFactory +} from '@/modules/core/repositories/userEmails' +import { db } from '@/db/knex' +import { testApolloServer } from '@/test/graphqlHelper' +import { + CreateProjectVersionDocument, + CreateWorkspaceDocument, + CreateWorkspaceProjectDocument +} from '@/test/graphql/generated/graphql' +import { validateAndCreateUserEmailFactory } from '@/modules/core/services/userEmails' +import { finalizeInvitedServerRegistrationFactory } from '@/modules/serverinvites/services/processing' +import { + deleteServerOnlyInvitesFactory, + updateAllInviteTargetsFactory +} from '@/modules/serverinvites/repositories/serverInvites' +import { requestNewEmailVerificationFactory } from '@/modules/emails/services/verification/request' +import { deleteOldAndInsertNewVerificationFactory } from '@/modules/emails/repositories' +import { renderEmail } from '@/modules/emails/services/emailRendering' +import { sendEmail } from '@/modules/emails/services/sending' +import { + countAdminUsersFactory, + legacyGetUserFactory, + storeUserAclFactory, + storeUserFactory +} from '@/modules/core/repositories/users' +import { createUserFactory } from '@/modules/core/services/users/management' +import { UsersEmitter } from '@/modules/core/events/usersEmitter' +import { getServerInfoFactory } from '@/modules/core/repositories/server' +import { WorkspaceReadOnlyError } from '@/modules/gatekeeper/errors/billing' +import { CreateVersionInput } from '@/modules/core/graph/generated/graphql' + +const getServerInfo = getServerInfoFactory({ db }) +const getUser = legacyGetUserFactory({ db }) +const requestNewEmailVerification = requestNewEmailVerificationFactory({ + findEmail: findEmailFactory({ db }), + getUser, + getServerInfo, + deleteOldAndInsertNewVerification: deleteOldAndInsertNewVerificationFactory({ db }), + renderEmail, + sendEmail +}) + +const createUserEmail = validateAndCreateUserEmailFactory({ + createUserEmail: createUserEmailFactory({ db }), + ensureNoPrimaryEmailForUser: ensureNoPrimaryEmailForUserFactory({ db }), + findEmail: findEmailFactory({ db }), + updateEmailInvites: finalizeInvitedServerRegistrationFactory({ + deleteServerOnlyInvites: deleteServerOnlyInvitesFactory({ db }), + updateAllInviteTargets: updateAllInviteTargetsFactory({ db }) + }), + requestNewEmailVerification +}) + +const findEmail = findEmailFactory({ db }) +const createUser = createUserFactory({ + getServerInfo, + findEmail, + storeUser: storeUserFactory({ db }), + countAdminUsers: countAdminUsersFactory({ db }), + storeUserAcl: storeUserAclFactory({ db }), + validateAndCreateUserEmail: createUserEmail, + usersEventsEmitter: UsersEmitter.emit +}) + +describe('Versions graphql @core', () => { + before(async () => { + await beforeEachContext() + }) + + describe('Create version mutation', () => { + it('should return error if project is read-only', async () => { + const userId = await createUser({ + name: 'emails user', + email: createRandomEmail(), + password: createRandomPassword() + }) + + const apollo = await testApolloServer({ authUserId: userId }) + + const workspaceCreateRes = await apollo.execute(CreateWorkspaceDocument, { + input: { name: 'test ws' } + }) + expect(workspaceCreateRes).to.not.haveGraphQLErrors() + + const workspace = workspaceCreateRes.data?.workspaceMutations.create + + const projectCreateRes = await apollo.execute(CreateWorkspaceProjectDocument, { + input: { workspaceId: workspace!.id, name: 'test project' } + }) + expect(projectCreateRes).to.not.haveGraphQLErrors() + const project = projectCreateRes.data?.workspaceMutations.projects.create + + // Make the project read-only + await db('workspace_plans') + .update({ status: 'canceled' }) + .where({ workspaceId: workspace!.id }) + + const versionCreateRes = await apollo.execute(CreateProjectVersionDocument, { + input: { + projectId: project!.id, + modelId: 'modelid', + objectId: 'objectid' + } as unknown as CreateVersionInput + }) + expect(versionCreateRes).to.haveGraphQLErrors() + expect(versionCreateRes.errors).to.have.length(1) + expect(versionCreateRes.errors![0].message).to.eq( + new WorkspaceReadOnlyError().message + ) + }) + }) +}) diff --git a/packages/server/modules/gatekeeper/index.ts b/packages/server/modules/gatekeeper/index.ts index 101531ea6..71185bb39 100644 --- a/packages/server/modules/gatekeeper/index.ts +++ b/packages/server/modules/gatekeeper/index.ts @@ -172,17 +172,18 @@ const gatekeeperModule: SpeckleModule = { }) }, async finalize() { - coreModule.addHook( - 'onCreateObjectRequest', - async function isProjectReadOnly({ projectId }) { - const readOnly = await isProjectReadOnlyFactory({ - getWorkspacePlanByProjectId: getWorkspacePlanByProjectIdFactory({ - db - }) - })({ projectId }) - if (readOnly) throw new WorkspaceReadOnlyError() - } - ) + coreModule.addHook('onCreateObjectRequest', isProjectReadOnly) + coreModule.addHook('onCreateVersionRequest', isProjectReadOnly) } } + +async function isProjectReadOnly({ projectId }: { projectId: string }) { + const readOnly = await isProjectReadOnlyFactory({ + getWorkspacePlanByProjectId: getWorkspacePlanByProjectIdFactory({ + db + }) + })({ projectId }) + if (readOnly) throw new WorkspaceReadOnlyError() +} + export = gatekeeperModule From 47eb26f88e479044774512e875010c6e23190f67 Mon Sep 17 00:00:00 2001 From: Alessandro Magionami Date: Fri, 20 Dec 2024 10:12:05 +0100 Subject: [PATCH 2/2] chote(core): fix tests --- .../tests/integration/commits.graph.spec.ts | 74 ++++++++++--------- .../tests/integration/versions.graph.spec.ts | 74 ++++++++++--------- 2 files changed, 80 insertions(+), 68 deletions(-) diff --git a/packages/server/modules/core/tests/integration/commits.graph.spec.ts b/packages/server/modules/core/tests/integration/commits.graph.spec.ts index 0d7eb0239..1231a0a79 100644 --- a/packages/server/modules/core/tests/integration/commits.graph.spec.ts +++ b/packages/server/modules/core/tests/integration/commits.graph.spec.ts @@ -37,6 +37,7 @@ import { UsersEmitter } from '@/modules/core/events/usersEmitter' import { getServerInfoFactory } from '@/modules/core/repositories/server' import { WorkspaceReadOnlyError } from '@/modules/gatekeeper/errors/billing' import gql from 'graphql-tag' +import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' const getServerInfo = getServerInfoFactory({ db }) const getUser = legacyGetUserFactory({ db }) @@ -71,6 +72,8 @@ const createUser = createUserFactory({ usersEventsEmitter: UsersEmitter.emit }) +const { FF_BILLING_INTEGRATION_ENABLED } = getFeatureFlags() + const createCommitMutation = gql` mutation CreateCommit($commit: CommitCreateInput!) { commitCreate(commit: $commit) @@ -82,45 +85,48 @@ describe('Commits graphql @core', () => { }) describe('Create commit mutation', () => { - it('should return error if project is read-only', async () => { - const userId = await createUser({ - name: 'emails user', - email: createRandomEmail(), - password: createRandomPassword() - }) + ;(FF_BILLING_INTEGRATION_ENABLED ? it : it.skip)( + 'should return error if project is read-only', + async () => { + const userId = await createUser({ + name: 'emails user', + email: createRandomEmail(), + password: createRandomPassword() + }) - const apollo = await testApolloServer({ authUserId: userId }) + const apollo = await testApolloServer({ authUserId: userId }) - const workspaceCreateRes = await apollo.execute(CreateWorkspaceDocument, { - input: { name: 'test ws' } - }) - expect(workspaceCreateRes).to.not.haveGraphQLErrors() + const workspaceCreateRes = await apollo.execute(CreateWorkspaceDocument, { + input: { name: 'test ws' } + }) + expect(workspaceCreateRes).to.not.haveGraphQLErrors() - const workspace = workspaceCreateRes.data?.workspaceMutations.create + const workspace = workspaceCreateRes.data?.workspaceMutations.create - const projectCreateRes = await apollo.execute(CreateWorkspaceProjectDocument, { - input: { workspaceId: workspace!.id, name: 'test project' } - }) - expect(projectCreateRes).to.not.haveGraphQLErrors() - const project = projectCreateRes.data?.workspaceMutations.projects.create + const projectCreateRes = await apollo.execute(CreateWorkspaceProjectDocument, { + input: { workspaceId: workspace!.id, name: 'test project' } + }) + expect(projectCreateRes).to.not.haveGraphQLErrors() + const project = projectCreateRes.data?.workspaceMutations.projects.create - // Make the project read-only - await db('workspace_plans') - .update({ status: 'canceled' }) - .where({ workspaceId: workspace!.id }) + // Make the project read-only + await db('workspace_plans') + .update({ status: 'canceled' }) + .where({ workspaceId: workspace!.id }) - const versionCreateRes = await apollo.execute(createCommitMutation, { - commit: { - streamId: project!.id, - branchName: 'branch', - objectId: 'objectid' - } - }) - expect(versionCreateRes).to.haveGraphQLErrors() - expect(versionCreateRes.errors).to.have.length(1) - expect(versionCreateRes.errors![0].message).to.eq( - new WorkspaceReadOnlyError().message - ) - }) + const versionCreateRes = await apollo.execute(createCommitMutation, { + commit: { + streamId: project!.id, + branchName: 'branch', + objectId: 'objectid' + } + }) + expect(versionCreateRes).to.haveGraphQLErrors() + expect(versionCreateRes.errors).to.have.length(1) + expect(versionCreateRes.errors![0].message).to.eq( + new WorkspaceReadOnlyError().message + ) + } + ) }) }) diff --git a/packages/server/modules/core/tests/integration/versions.graph.spec.ts b/packages/server/modules/core/tests/integration/versions.graph.spec.ts index 1624526ad..727422523 100644 --- a/packages/server/modules/core/tests/integration/versions.graph.spec.ts +++ b/packages/server/modules/core/tests/integration/versions.graph.spec.ts @@ -38,6 +38,7 @@ import { UsersEmitter } from '@/modules/core/events/usersEmitter' import { getServerInfoFactory } from '@/modules/core/repositories/server' import { WorkspaceReadOnlyError } from '@/modules/gatekeeper/errors/billing' import { CreateVersionInput } from '@/modules/core/graph/generated/graphql' +import { getFeatureFlags } from '@/modules/shared/helpers/envHelper' const getServerInfo = getServerInfoFactory({ db }) const getUser = legacyGetUserFactory({ db }) @@ -72,51 +73,56 @@ const createUser = createUserFactory({ usersEventsEmitter: UsersEmitter.emit }) +const { FF_BILLING_INTEGRATION_ENABLED } = getFeatureFlags() + describe('Versions graphql @core', () => { before(async () => { await beforeEachContext() }) describe('Create version mutation', () => { - it('should return error if project is read-only', async () => { - const userId = await createUser({ - name: 'emails user', - email: createRandomEmail(), - password: createRandomPassword() - }) + ;(FF_BILLING_INTEGRATION_ENABLED ? it : it.skip)( + 'should return error if project is read-only', + async () => { + const userId = await createUser({ + name: 'emails user', + email: createRandomEmail(), + password: createRandomPassword() + }) - const apollo = await testApolloServer({ authUserId: userId }) + const apollo = await testApolloServer({ authUserId: userId }) - const workspaceCreateRes = await apollo.execute(CreateWorkspaceDocument, { - input: { name: 'test ws' } - }) - expect(workspaceCreateRes).to.not.haveGraphQLErrors() + const workspaceCreateRes = await apollo.execute(CreateWorkspaceDocument, { + input: { name: 'test ws' } + }) + expect(workspaceCreateRes).to.not.haveGraphQLErrors() - const workspace = workspaceCreateRes.data?.workspaceMutations.create + const workspace = workspaceCreateRes.data?.workspaceMutations.create - const projectCreateRes = await apollo.execute(CreateWorkspaceProjectDocument, { - input: { workspaceId: workspace!.id, name: 'test project' } - }) - expect(projectCreateRes).to.not.haveGraphQLErrors() - const project = projectCreateRes.data?.workspaceMutations.projects.create + const projectCreateRes = await apollo.execute(CreateWorkspaceProjectDocument, { + input: { workspaceId: workspace!.id, name: 'test project' } + }) + expect(projectCreateRes).to.not.haveGraphQLErrors() + const project = projectCreateRes.data?.workspaceMutations.projects.create - // Make the project read-only - await db('workspace_plans') - .update({ status: 'canceled' }) - .where({ workspaceId: workspace!.id }) + // Make the project read-only + await db('workspace_plans') + .update({ status: 'canceled' }) + .where({ workspaceId: workspace!.id }) - const versionCreateRes = await apollo.execute(CreateProjectVersionDocument, { - input: { - projectId: project!.id, - modelId: 'modelid', - objectId: 'objectid' - } as unknown as CreateVersionInput - }) - expect(versionCreateRes).to.haveGraphQLErrors() - expect(versionCreateRes.errors).to.have.length(1) - expect(versionCreateRes.errors![0].message).to.eq( - new WorkspaceReadOnlyError().message - ) - }) + const versionCreateRes = await apollo.execute(CreateProjectVersionDocument, { + input: { + projectId: project!.id, + modelId: 'modelid', + objectId: 'objectid' + } as unknown as CreateVersionInput + }) + expect(versionCreateRes).to.haveGraphQLErrors() + expect(versionCreateRes.errors).to.have.length(1) + expect(versionCreateRes.errors![0].message).to.eq( + new WorkspaceReadOnlyError().message + ) + } + ) }) })