Skip to content

Commit 61a1cad

Browse files
waleedlatif1claude
andcommitted
fix(scim): address cubic's second review round
- One entitlement gate: the deployment flag is the outer gate everywhere (authentication, admin API, reconcile job, discovery) - Group PATCH: path-less add contributes members; add without a value is refused; filter booleans only for active; formatted name applied after its parts - Projection: mappings to workspaces that left the organization are ignored and their provenance dropped without touching the other tenant - Update refuses a stale row for someone who left; deprovision serializes on the user locks and scopes account-wide effects to a real removal - Auto-map drops the stale automatic mapping on rename; mapping upserts are conflict-safe and refuse the default group; connection settings merge under the row lock - Relinking keeps email verification when the address is unchanged; addresses are syntax-checked; failed provisioning cleans up through the account-deletion path; inactive provisioning invalidates caches - Reconcile watermark advances only on a completed pass - Wrapped orchestration errors keep their SCIM status; revoke audit names the connection; group members show their display name - UI: error state on the settings read, activity polls; SCIM reachable for admins who did not configure SSO; docs wording on keys and cookies Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent aa695d1 commit 61a1cad

28 files changed

Lines changed: 394 additions & 194 deletions

File tree

apps/docs/content/docs/platform/enterprise/scim.mdx

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ It pairs with [SSO](/platform/enterprise/sso). SSO proves who someone is when th
2222
| --- | --- |
2323
| Assigns a person to the Sim app | Creates their account and adds them to your organization as a Member |
2424
| Updates their name or email | Updates the Sim account, and ends their sessions if the address changed |
25-
| Deactivates them | Blocks sign-in and stops their API keys. Everything they own, and every grant they hold, is left untouched |
25+
| Deactivates them | Blocks sign-in and stops their personal API keys. Everything they own, and every grant they hold, is left untouched; shared workspace keys keep working |
2626
| Reactivates them | Restores access exactly as it was |
2727
| Removes them from the app | Removes their organization membership and reassigns what they owned |
2828
| Adds them to a group | Grants whatever that group maps to |
@@ -156,7 +156,7 @@ Sim also re-applies every group mapping on a schedule, so drift cannot persist.
156156
<FAQ items={[
157157
{
158158
question: "What happens to someone's workflows when they are deactivated?",
159-
answer: "Nothing. A deactivation blocks sign-in and stops their API keys, and leaves every workspace, workflow, and credential they own exactly as it was. Reactivating them restores access. Only a removal — which your provider sends explicitly — reassigns what they owned. A deactivated member also keeps their seat until they are removed."
159+
answer: "Nothing. A deactivation blocks sign-in and stops their personal API keys, and leaves every workspace, workflow, and credential they own exactly as it was. Reactivating them restores access. Only a removal — which your provider sends explicitly — reassigns what they owned. A deactivated member also keeps their seat until they are removed."
160160
},
161161
{
162162
question: "Can the directory provision someone outside our verified domains?",
@@ -176,7 +176,7 @@ Sim also re-applies every group mapping on a schedule, so drift cannot persist.
176176
},
177177
{
178178
question: "Does a deactivation sign someone out immediately?",
179-
answer: "Their sessions are deleted at once and their API keys stop authenticating immediately. A browser tab holding a cached session cookie can survive up to five minutes; enabling a session policy shortens that window."
179+
answer: "Their sessions are deleted at once and their personal API keys stop authenticating immediately. The organization's security version changes at the same time, so a browser still holding a cached session cookie is re-checked on its next request and refused."
180180
},
181181
{
182182
question: "Can we still invite people manually?",

apps/sim/app/api/workspaces/invitations/batch/route.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,6 @@ function batchErrorResponse(error: unknown) {
4040
return NextResponse.json({ error: error.message }, { status: 403 })
4141
}
4242

43-
if (error instanceof ForbiddenOperationError) {
44-
return NextResponse.json(
45-
{ error: error.message, details: { code: error.detailCode } },
46-
{ status: 403 }
47-
)
48-
}
49-
5043
logger.error('Error creating workspace invitation batch:', error)
5144
return NextResponse.json({ error: 'Failed to create invitation batch' }, { status: 500 })
5245
}
@@ -107,6 +100,11 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
107100
failed.push({ email: error.email ?? normalizedEmail, error: error.message })
108101
continue
109102
}
103+
/** A directory-managed address is refused with its reason, like any other per-email refusal. */
104+
if (error instanceof ForbiddenOperationError) {
105+
failed.push({ email: normalizedEmail, error: error.message })
106+
continue
107+
}
110108

111109
/**
112110
* One bad address must not discard the invitations that already

apps/sim/ee/scim/components/scim-section.tsx

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ import type {
2626
} from '@/lib/api/contracts/organization-scim'
2727
import { useDeploymentShape } from '@/lib/core/config/deployment-shape'
2828
import { RowActionsMenu } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu'
29-
import { SettingsEmptyState } from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state'
29+
import {
30+
SettingsEmptyState,
31+
SettingsQueryErrorState,
32+
} from '@/app/workspace/[workspaceId]/settings/components/settings-empty-state'
3033
import { SettingsResourceRow } from '@/app/workspace/[workspaceId]/settings/components/settings-resource-row'
3134
import { SettingsSection } from '@/app/workspace/[workspaceId]/settings/components/settings-section/settings-section'
3235
import {
@@ -523,11 +526,28 @@ function ConnectionDetails({ organizationId, connection }: ConnectionDetailsProp
523526
*/
524527
export function ScimSection({ organizationId }: ScimSectionProps) {
525528
const { features } = useDeploymentShape()
526-
const { data, isLoading } = useScimConnection(organizationId, features.scim)
529+
const { data, isLoading, isError, error, isFetching, refetch } = useScimConnection(
530+
organizationId,
531+
features.scim
532+
)
527533
const configure = useConfigureScimConnection()
528534

529535
if (!features.scim) return null
530536

537+
if (isError) {
538+
return (
539+
<SettingsSection label='Directory provisioning'>
540+
<SettingsQueryErrorState
541+
error={error}
542+
fallback='Failed to load directory provisioning settings'
543+
isRetrying={isFetching}
544+
onRetry={() => void refetch()}
545+
variant='inline'
546+
/>
547+
</SettingsSection>
548+
)
549+
}
550+
531551
const connection = data?.connection ?? null
532552
const enabled = connection?.status === 'active'
533553

apps/sim/ee/scim/hooks/scim.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ import {
1616

1717
export const SCIM_CONNECTION_STALE_TIME = 30 * 1000
1818
export const SCIM_MAPPINGS_STALE_TIME = 30 * 1000
19-
/** Activity is a debugging surface; a short window keeps it close to live. */
19+
/** Activity is a debugging surface; it polls while mounted so a sync in progress shows up without a reload. */
2020
export const SCIM_ACTIVITY_STALE_TIME = 10 * 1000
21+
export const SCIM_ACTIVITY_REFETCH_INTERVAL = 15 * 1000
2122

2223
export const scimKeys = {
2324
all: ['scim'] as const,
@@ -69,6 +70,7 @@ export function useScimActivity(organizationId?: string, enabled = true) {
6970
},
7071
enabled: Boolean(organizationId) && enabled,
7172
staleTime: SCIM_ACTIVITY_STALE_TIME,
73+
refetchInterval: SCIM_ACTIVITY_REFETCH_INTERVAL,
7274
})
7375
}
7476

apps/sim/ee/sso/components/sso-settings.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -327,9 +327,12 @@ function OrganizationSsoSettings({ organizationId }: SSOProps) {
327327
} else {
328328
if (!isLoadingProviders && isSSOProviderOwner === false && providers.length > 0) {
329329
return (
330-
<SettingsEmptyState>
331-
Only the user who configured SSO can manage these settings.
332-
</SettingsEmptyState>
330+
<>
331+
<SettingsEmptyState>
332+
Only the user who configured SSO can manage these settings.
333+
</SettingsEmptyState>
334+
<ScimSection organizationId={organizationId} />
335+
</>
333336
)
334337
}
335338
}

apps/sim/lib/api/contracts/organization-scim.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export const configureScimConnectionContract = defineRouteContract({
6565
body: z.object({
6666
status: z.enum(['active', 'disabled']).optional(),
6767
settings: scimConnectionSettingsSchema.optional(),
68-
ssoProviderId: z.string().max(128).nullable().optional(),
68+
ssoProviderId: z.string().min(1).max(128).nullable().optional(),
6969
}),
7070
response: { mode: 'json', schema: z.object({ connection: scimConnectionSchema }) },
7171
})

apps/sim/lib/api/server/routes/scim-route.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { type NextRequest, NextResponse } from 'next/server'
44
import type { AnyApiRouteContract, ContractJsonResponse } from '@/lib/api/contracts/types'
55
import { type ParsedRequest, parseRequest } from '@/lib/api/server/validation'
66
import type { ApplicationOperation, OperationUseCase } from '@/lib/core/application/operation'
7+
import { isScimEnabled } from '@/lib/core/config/env-flags'
78
import { enforceIpRateLimit, RateLimiter } from '@/lib/core/rate-limiter'
89
import { getBaseUrl } from '@/lib/core/utils/urls'
910
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -294,6 +295,8 @@ export function defineScimDiscoveryRoute(
294295
return withRouteHandler<ScimRouteContext | undefined>(
295296
async (request, context) => {
296297
try {
298+
/** A deployment without the feature exposes no provisioning surface, discovery included. */
299+
if (!isScimEnabled) throw new ScimError(404, undefined, 'Not found')
297300
if (request.method !== 'GET') {
298301
throw new ScimError(405, undefined, `${request.method} is not supported here`)
299302
}

apps/sim/lib/scim/application/admin/connection.ts

Lines changed: 51 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -73,55 +73,64 @@ export const configureScimConnection = defineAuthorizedScimAdminUseCase({
7373
await assertWorkspaceInOrganization(context.organizationId, grant.workspaceId)
7474
}
7575

76-
const [existing] = await db
77-
.select({
78-
id: scimConnection.id,
79-
settings: scimConnection.settings,
80-
status: scimConnection.status,
81-
})
82-
.from(scimConnection)
83-
.where(eq(scimConnection.organizationId, context.organizationId))
84-
.limit(1)
85-
86-
const nextSettings: ScimConnectionSettings = {
87-
/**
88-
* Locking manual membership defaults on for a new connection. Once a
89-
* directory owns membership, a change made only in Sim is reverted by the
90-
* next sync, so a member edited by hand looks like it worked and then
91-
* silently does not.
92-
*/
93-
lockManualMembership: true,
94-
...(existing?.settings ?? {}),
95-
...(input.settings ?? {}),
96-
}
76+
/**
77+
* Read, merge, and write under the row lock, so two administrators changing
78+
* different settings at once both land instead of the later write carrying
79+
* a stale copy of the earlier one's field.
80+
*/
81+
const { created, status } = await db.transaction(async (tx) => {
82+
const [existing] = await tx
83+
.select({
84+
id: scimConnection.id,
85+
settings: scimConnection.settings,
86+
status: scimConnection.status,
87+
})
88+
.from(scimConnection)
89+
.where(eq(scimConnection.organizationId, context.organizationId))
90+
.limit(1)
91+
.for('update')
9792

98-
const connectionId = existing?.id ?? generateId()
99-
const nextStatus = input.status ?? existing?.status ?? 'active'
93+
const nextSettings: ScimConnectionSettings = {
94+
/**
95+
* Locking manual membership defaults on for a new connection. Once a
96+
* directory owns membership, a change made only in Sim is reverted by
97+
* the next sync, so a member edited by hand looks like it worked and
98+
* then silently does not.
99+
*/
100+
lockManualMembership: true,
101+
...(existing?.settings ?? {}),
102+
...(input.settings ?? {}),
103+
}
104+
const nextStatus = input.status ?? existing?.status ?? 'active'
100105

101-
if (existing) {
102-
await db
103-
.update(scimConnection)
104-
.set({
106+
if (existing) {
107+
await tx
108+
.update(scimConnection)
109+
.set({
110+
status: nextStatus,
111+
settings: nextSettings,
112+
...(input.ssoProviderId !== undefined ? { ssoProviderId: input.ssoProviderId } : {}),
113+
updatedAt: new Date(),
114+
})
115+
.where(eq(scimConnection.id, existing.id))
116+
} else {
117+
await tx.insert(scimConnection).values({
118+
id: generateId(),
119+
organizationId: context.organizationId,
120+
ssoProviderId: input.ssoProviderId ?? null,
105121
status: nextStatus,
106122
settings: nextSettings,
107-
...(input.ssoProviderId !== undefined ? { ssoProviderId: input.ssoProviderId } : {}),
108-
updatedAt: new Date(),
123+
createdBy: context.actorUserId,
109124
})
110-
.where(eq(scimConnection.id, existing.id))
111-
} else {
112-
await db.insert(scimConnection).values({
113-
id: connectionId,
114-
organizationId: context.organizationId,
115-
ssoProviderId: input.ssoProviderId ?? null,
116-
status: nextStatus,
117-
settings: nextSettings,
118-
createdBy: context.actorUserId,
119-
})
120-
}
125+
}
126+
return { created: !existing, status: nextStatus }
127+
})
121128

122129
const view = await loadConnectionView(context.organizationId)
123-
if (!view) throw new OrchestrationError('internal', 'The connection could not be read back')
124-
return { connection: view, created: !existing }
130+
if (!view || view.status !== status) {
131+
throw new OrchestrationError('internal', 'The connection could not be read back')
132+
}
133+
return { connection: view, created }
125134
},
126135
projectAudit: ({ result }) => ({
127136
action:

apps/sim/lib/scim/application/admin/credentials.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,15 +111,19 @@ export const revokeScimCredential = defineAuthorizedScimAdminUseCase({
111111
)`
112112
)
113113
)
114-
.returning({ id: scimCredential.id, tokenPrefix: scimCredential.tokenPrefix })
114+
.returning({
115+
id: scimCredential.id,
116+
tokenPrefix: scimCredential.tokenPrefix,
117+
connectionId: scimCredential.connectionId,
118+
})
115119

116120
if (!revoked) throw new OrchestrationError('not_found', 'Credential not found')
117121
return { success: true as const, revoked }
118122
},
119123
projectAudit: ({ result }) => ({
120124
action: AuditAction.SCIM_CREDENTIAL_REVOKED,
121125
resourceType: AuditResourceType.SCIM_CONNECTION,
122-
resourceId: result.revoked.id,
123-
metadata: { tokenPrefix: result.revoked.tokenPrefix },
126+
resourceId: result.revoked.connectionId,
127+
metadata: { credentialId: result.revoked.id, tokenPrefix: result.revoked.tokenPrefix },
124128
}),
125129
})

0 commit comments

Comments
 (0)