From b4aa0f76083fc5ba1e764c6004b0cb230bf3603a Mon Sep 17 00:00:00 2001 From: Chuck Driesler Date: Tue, 3 Dec 2024 18:05:04 +0000 Subject: [PATCH] fix(sso): disable slug edit if sso enabled (#3621) * fix(sso): disable slug edit if sso enabled * fix(sso): guard on backend * chore(sso): more test fixes --- .../settings/workspaces/General.vue | 21 +++++++--- .../workspaces/graph/resolvers/workspaces.ts | 8 ++++ .../modules/workspaces/services/management.ts | 16 +++++++- .../workspaces/tests/helpers/creation.ts | 1 + .../tests/integration/subs.graph.spec.ts | 2 + .../tests/unit/services/management.spec.ts | 38 +++++++++++++++++++ 6 files changed, 78 insertions(+), 8 deletions(-) diff --git a/packages/frontend-2/components/settings/workspaces/General.vue b/packages/frontend-2/components/settings/workspaces/General.vue index d17117e892..e96da6afb3 100644 --- a/packages/frontend-2/components/settings/workspaces/General.vue +++ b/packages/frontend-2/components/settings/workspaces/General.vue @@ -26,14 +26,14 @@ label="Short ID" name="shortId" :help="slugHelp" - :disabled="!isAdmin || needsSsoLogin" + :disabled="disableSlugInput" show-label label-position="left" - :tooltip-text="disabledTooltipText" + :tooltip-text="disabledSlugTooltipText" read-only - :right-icon="isAdmin || needsSsoLogin ? IconEdit : undefined" - right-icon-title="Edit short ID" - @right-icon-click="openSlugEditDialog" + :right-icon="disableSlugInput ? undefined : IconEdit" + :right-icon-title="disableSlugInput ? undefined : 'Edit short ID'" + @right-icon-click="disableSlugInput ? undefined : openSlugEditDialog" />
workspaceResult.value?.workspace?.slug || '') }) @@ -319,7 +319,16 @@ const disabledTooltipText = computed(() => { return undefined }) +const disableSlugInput = computed(() => !isAdmin.value || hasSsoEnabled.value) + +const disabledSlugTooltipText = computed(() => { + return hasSsoEnabled.value + ? 'Short ID cannot be changed while SSO is enabled.' + : disabledTooltipText.value +}) + const openSlugEditDialog = () => { + if (hasSsoEnabled.value) return showEditSlugDialog.value = true } diff --git a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts index eab34e7b29..6055834117 100644 --- a/packages/server/modules/workspaces/graph/resolvers/workspaces.ts +++ b/packages/server/modules/workspaces/graph/resolvers/workspaces.ts @@ -517,6 +517,10 @@ export = FF_WORKSPACES_MODULE_ENABLED getWorkspaceBySlug: getWorkspaceBySlugFactory({ db }) }), getWorkspace: getWorkspaceWithDomainsFactory({ db }), + getWorkspaceSsoProviderRecord: getWorkspaceSsoProviderFactory({ + db, + decrypt: getDecryptor() + }), upsertWorkspace: upsertWorkspaceFactory({ db }), emitWorkspaceEvent: getEventBus().emit }) @@ -619,6 +623,10 @@ export = FF_WORKSPACES_MODULE_ENABLED getWorkspaceBySlug: getWorkspaceBySlugFactory({ db }) }), getWorkspace: getWorkspaceWithDomainsFactory({ db }), + getWorkspaceSsoProviderRecord: getWorkspaceSsoProviderFactory({ + db, + decrypt: getDecryptor() + }), upsertWorkspace: upsertWorkspaceFactory({ db }), emitWorkspaceEvent: getEventBus().emit }) diff --git a/packages/server/modules/workspaces/services/management.ts b/packages/server/modules/workspaces/services/management.ts index 3b1dbe3244..6150c70343 100644 --- a/packages/server/modules/workspaces/services/management.ts +++ b/packages/server/modules/workspaces/services/management.ts @@ -64,7 +64,10 @@ import { userEmailsCompliantWithWorkspaceDomains } from '@/modules/workspaces/do import { workspaceRoles as workspaceRoleDefinitions } from '@/modules/workspaces/roles' import { blockedDomains } from '@speckle/shared' import { DeleteStreamRecord } from '@/modules/core/domain/streams/operations' -import { DeleteSsoProvider } from '@/modules/workspaces/domain/sso/operations' +import { + DeleteSsoProvider, + GetWorkspaceSsoProviderRecord +} from '@/modules/workspaces/domain/sso/operations' type WorkspaceCreateArgs = { userId: string @@ -230,11 +233,13 @@ const sanitizeInput = (input: Partial) => { export const updateWorkspaceFactory = ({ getWorkspace, + getWorkspaceSsoProviderRecord, validateSlug, upsertWorkspace, emitWorkspaceEvent }: { getWorkspace: GetWorkspaceWithDomains + getWorkspaceSsoProviderRecord: GetWorkspaceSsoProviderRecord validateSlug: ValidateWorkspaceSlug upsertWorkspace: UpsertWorkspace emitWorkspaceEvent: EventBus['emit'] @@ -253,7 +258,14 @@ export const updateWorkspaceFactory = throw new WorkspaceInvalidUpdateError() } - if (workspaceInput.slug) await validateSlug({ slug: workspaceInput.slug }) + if (workspaceInput.slug) { + const ssoProvider = await getWorkspaceSsoProviderRecord({ workspaceId }) + if (ssoProvider) + throw new WorkspaceInvalidUpdateError( + 'Cannot update workspace slug if SSO is configured.' + ) + await validateSlug({ slug: workspaceInput.slug }) + } const workspace = { ...omit(currentWorkspace, 'domains'), diff --git a/packages/server/modules/workspaces/tests/helpers/creation.ts b/packages/server/modules/workspaces/tests/helpers/creation.ts index 195b4e42f0..5fdee90a57 100644 --- a/packages/server/modules/workspaces/tests/helpers/creation.ts +++ b/packages/server/modules/workspaces/tests/helpers/creation.ts @@ -195,6 +195,7 @@ export const createTestWorkspace = async ( getWorkspaceBySlug: getWorkspaceBySlugFactory({ db }) }), getWorkspace: getWorkspaceWithDomainsFactory({ db }), + getWorkspaceSsoProviderRecord: getWorkspaceSsoProviderRecordFactory({ db }), upsertWorkspace: upsertWorkspaceFactory({ db }), emitWorkspaceEvent: (...args) => getEventBus().emit(...args) }) diff --git a/packages/server/modules/workspaces/tests/integration/subs.graph.spec.ts b/packages/server/modules/workspaces/tests/integration/subs.graph.spec.ts index af501e7afc..36eeaaceea 100644 --- a/packages/server/modules/workspaces/tests/integration/subs.graph.spec.ts +++ b/packages/server/modules/workspaces/tests/integration/subs.graph.spec.ts @@ -2,6 +2,7 @@ import { db } from '@/db/knex' import { ServerInvites } from '@/modules/core/dbSchema' import { getEventBus } from '@/modules/shared/services/eventBus' import { getDefaultRegionFactory } from '@/modules/workspaces/repositories/regions' +import { getWorkspaceSsoProviderRecordFactory } from '@/modules/workspaces/repositories/sso' import { getWorkspaceBySlugFactory, getWorkspaceWithDomainsFactory, @@ -59,6 +60,7 @@ const updateWorkspace = updateWorkspaceFactory({ getWorkspaceBySlug: getWorkspaceBySlugFactory({ db }) }), getWorkspace: getWorkspaceWithDomainsFactory({ db }), + getWorkspaceSsoProviderRecord: getWorkspaceSsoProviderRecordFactory({ db }), upsertWorkspace: upsertWorkspaceFactory({ db }), emitWorkspaceEvent: getEventBus().emit }) diff --git a/packages/server/modules/workspaces/tests/unit/services/management.spec.ts b/packages/server/modules/workspaces/tests/unit/services/management.spec.ts index c00b7931b7..8cb9e81206 100644 --- a/packages/server/modules/workspaces/tests/unit/services/management.spec.ts +++ b/packages/server/modules/workspaces/tests/unit/services/management.spec.ts @@ -270,6 +270,7 @@ describe('Workspace services', () => { const err = await expectToThrow(async () => { await updateWorkspaceFactory({ getWorkspace: async () => null, + getWorkspaceSsoProviderRecord: async () => null, validateSlug: async () => { expect.fail() }, @@ -291,6 +292,7 @@ describe('Workspace services', () => { const err = await expectToThrow(async () => { await updateWorkspaceFactory({ getWorkspace: async () => workspace, + getWorkspaceSsoProviderRecord: async () => null, emitWorkspaceEvent: async () => { expect.fail() }, @@ -314,6 +316,7 @@ describe('Workspace services', () => { const err = await expectToThrow(async () => { await updateWorkspaceFactory({ getWorkspace: async () => workspace, + getWorkspaceSsoProviderRecord: async () => null, emitWorkspaceEvent: async () => { expect.fail() }, @@ -337,6 +340,7 @@ describe('Workspace services', () => { const err = await expectToThrow(async () => { await updateWorkspaceFactory({ getWorkspace: async () => workspace, + getWorkspaceSsoProviderRecord: async () => null, emitWorkspaceEvent: async () => { expect.fail() }, @@ -360,6 +364,7 @@ describe('Workspace services', () => { const err = await expectToThrow(async () => { await updateWorkspaceFactory({ getWorkspace: async () => workspace, + getWorkspaceSsoProviderRecord: async () => null, emitWorkspaceEvent: async () => { expect.fail() }, @@ -382,6 +387,7 @@ describe('Workspace services', () => { const err = await expectToThrow(async () => { await updateWorkspaceFactory({ getWorkspace: async () => workspace, + getWorkspaceSsoProviderRecord: async () => null, emitWorkspaceEvent: async () => { expect.fail() }, @@ -405,6 +411,7 @@ describe('Workspace services', () => { let newWorkspaceName await updateWorkspaceFactory({ getWorkspace: async () => workspace, + getWorkspaceSsoProviderRecord: async () => null, emitWorkspaceEvent: async () => {}, validateSlug: async () => {}, @@ -417,6 +424,36 @@ describe('Workspace services', () => { }) expect(newWorkspaceName).to.be.equal(workspace.name) }) + + it('does not allow updating the workspace slug if SSO is enabled', async () => { + const workspace = createTestWorkspaceWithDomainsData() + + const err = await expectToThrow(async () => { + await updateWorkspaceFactory({ + getWorkspace: async () => workspace, + getWorkspaceSsoProviderRecord: async () => ({ + workspaceId: 'foo', + providerId: 'bar' + }), + emitWorkspaceEvent: async () => { + expect.fail() + }, + validateSlug: async () => {}, + upsertWorkspace: async () => { + expect.fail() + } + })({ + workspaceId: workspace.id, + workspaceInput: { + slug: 'new-slug' + } + }) + }) + expect(err.message).to.be.equal( + 'Cannot update workspace slug if SSO is configured.' + ) + }) + it('updates the workspace and emits the correct event payload', async () => { const workspaceId = cryptoRandomString({ length: 10 }) const workspace = createTestWorkspaceWithDomainsData({ @@ -443,6 +480,7 @@ describe('Workspace services', () => { await updateWorkspaceFactory({ getWorkspace: async () => workspace, + getWorkspaceSsoProviderRecord: async () => null, emitWorkspaceEvent: async () => {}, validateSlug: async () => {}, upsertWorkspace: async ({ workspace }) => {