Skip to content

Commit

Permalink
fix(sso): disable slug edit if sso enabled (#3621)
Browse files Browse the repository at this point in the history
* fix(sso): disable slug edit if sso enabled

* fix(sso): guard on backend

* chore(sso): more test fixes
  • Loading branch information
cdriesler authored Dec 3, 2024
1 parent a0d2831 commit b4aa0f7
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 8 deletions.
21 changes: 15 additions & 6 deletions packages/frontend-2/components/settings/workspaces/General.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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"
/>
<hr class="my-4 border-outline-3" />
<FormTextInput
Expand Down Expand Up @@ -210,7 +210,7 @@ const { result: workspaceResult, onResult } = useQuery(
})
)
const config = useRuntimeConfig()
const { needsSsoLogin } = useWorkspaceSsoStatus({
const { hasSsoEnabled, needsSsoLogin } = useWorkspaceSsoStatus({
workspaceSlug: computed(() => workspaceResult.value?.workspace?.slug || '')
})
Expand Down Expand Up @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand Down Expand Up @@ -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
})
Expand Down
16 changes: 14 additions & 2 deletions packages/server/modules/workspaces/services/management.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -230,11 +233,13 @@ const sanitizeInput = (input: Partial<Workspace>) => {
export const updateWorkspaceFactory =
({
getWorkspace,
getWorkspaceSsoProviderRecord,
validateSlug,
upsertWorkspace,
emitWorkspaceEvent
}: {
getWorkspace: GetWorkspaceWithDomains
getWorkspaceSsoProviderRecord: GetWorkspaceSsoProviderRecord
validateSlug: ValidateWorkspaceSlug
upsertWorkspace: UpsertWorkspace
emitWorkspaceEvent: EventBus['emit']
Expand All @@ -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'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -59,6 +60,7 @@ const updateWorkspace = updateWorkspaceFactory({
getWorkspaceBySlug: getWorkspaceBySlugFactory({ db })
}),
getWorkspace: getWorkspaceWithDomainsFactory({ db }),
getWorkspaceSsoProviderRecord: getWorkspaceSsoProviderRecordFactory({ db }),
upsertWorkspace: upsertWorkspaceFactory({ db }),
emitWorkspaceEvent: getEventBus().emit
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ describe('Workspace services', () => {
const err = await expectToThrow(async () => {
await updateWorkspaceFactory({
getWorkspace: async () => null,
getWorkspaceSsoProviderRecord: async () => null,
validateSlug: async () => {
expect.fail()
},
Expand All @@ -291,6 +292,7 @@ describe('Workspace services', () => {
const err = await expectToThrow(async () => {
await updateWorkspaceFactory({
getWorkspace: async () => workspace,
getWorkspaceSsoProviderRecord: async () => null,
emitWorkspaceEvent: async () => {
expect.fail()
},
Expand All @@ -314,6 +316,7 @@ describe('Workspace services', () => {
const err = await expectToThrow(async () => {
await updateWorkspaceFactory({
getWorkspace: async () => workspace,
getWorkspaceSsoProviderRecord: async () => null,
emitWorkspaceEvent: async () => {
expect.fail()
},
Expand All @@ -337,6 +340,7 @@ describe('Workspace services', () => {
const err = await expectToThrow(async () => {
await updateWorkspaceFactory({
getWorkspace: async () => workspace,
getWorkspaceSsoProviderRecord: async () => null,
emitWorkspaceEvent: async () => {
expect.fail()
},
Expand All @@ -360,6 +364,7 @@ describe('Workspace services', () => {
const err = await expectToThrow(async () => {
await updateWorkspaceFactory({
getWorkspace: async () => workspace,
getWorkspaceSsoProviderRecord: async () => null,
emitWorkspaceEvent: async () => {
expect.fail()
},
Expand All @@ -382,6 +387,7 @@ describe('Workspace services', () => {
const err = await expectToThrow(async () => {
await updateWorkspaceFactory({
getWorkspace: async () => workspace,
getWorkspaceSsoProviderRecord: async () => null,
emitWorkspaceEvent: async () => {
expect.fail()
},
Expand All @@ -405,6 +411,7 @@ describe('Workspace services', () => {
let newWorkspaceName
await updateWorkspaceFactory({
getWorkspace: async () => workspace,
getWorkspaceSsoProviderRecord: async () => null,
emitWorkspaceEvent: async () => {},
validateSlug: async () => {},

Expand All @@ -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({
Expand All @@ -443,6 +480,7 @@ describe('Workspace services', () => {

await updateWorkspaceFactory({
getWorkspace: async () => workspace,
getWorkspaceSsoProviderRecord: async () => null,
emitWorkspaceEvent: async () => {},
validateSlug: async () => {},
upsertWorkspace: async ({ workspace }) => {
Expand Down

0 comments on commit b4aa0f7

Please sign in to comment.