Skip to content

Commit 304b4e9

Browse files
waleedlatif1claude
andcommitted
fix(scim): address the first review round and CI
- Filters accept unquoted booleans for active (RFC 7644) - Group PATCH resolves order-sensitive membership deltas by last operation and refuses a non-string externalId - Full writes accept case-insensitive attribute names (Entra's `username`) - Credential issue counts and inserts under the connection row lock - Projection records provenance only for access the directory actually granted unless the directory is the source of truth; skips departed members and workspaces moved to another organization - Reconcile job verifies its lease per batch and reads settings after taking it - Group writes serialize on the organization lock; the member role route runs its managed-membership check under the same locks - Seat reconciliation targets the subscription admission validated against - Docs: FAQ import, self-hosted flags; docs manifest regenerated - Chart 1.10.0 for the new cron job; test fixtures carry column defaults Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 41ab8b1 commit 304b4e9

17 files changed

Lines changed: 327 additions & 102 deletions

File tree

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,14 @@ description: Create, update, and deactivate Sim members automatically from your
66
import { Callout } from 'fumadocs-ui/components/callout'
77
import { Step, Steps } from 'fumadocs-ui/components/steps'
88
import { Tab, Tabs } from 'fumadocs-ui/components/tabs'
9+
import { FAQ } from '@/components/ui/faq'
910

1011
Directory provisioning connects your identity provider to Sim over SCIM 2.0. Your provider creates members when someone joins, updates them when their details change, and deactivates them the moment they leave — without anyone touching Sim.
1112

1213
It pairs with [SSO](/platform/enterprise/sso). SSO proves who someone is when they sign in. Directory provisioning decides who exists and what they can reach, before and after that.
1314

1415
<Callout type="info">
15-
Enterprise plans. Requires at least one [verified domain](/platform/enterprise/verified-domains) for your organization.
16+
Enterprise plans. Requires at least one [verified domain](/platform/enterprise/verified-domains) for your organization. Self-hosted deployments turn it on with `SCIM_ENABLED=true` and `NEXT_PUBLIC_SCIM_ENABLED=true`, alongside the [SSO variables](/platform/enterprise/sso#self-hosted-setup).
1617
</Callout>
1718

1819
## What it does

apps/sim/app/api/organizations/[id]/members/[memberId]/route.ts

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { getSession } from '@/lib/auth'
1111
import { setActiveOrganizationForCurrentSession } from '@/lib/auth/active-organization'
1212
import { getOrganizationMemberUsageSnapshot } from '@/lib/billing/core/organization'
1313
import {
14+
acquireOrganizationUserMutationLocks,
1415
removeExternalUserFromOrganizationWorkspaces,
1516
removeUserFromOrganization,
1617
WORKSPACE_BILLING_ACCOUNT_REMOVAL_ERROR,
@@ -214,24 +215,21 @@ export const PUT = withRouteHandler(
214215
}
215216

216217
/**
217-
* When the organization has made its identity provider the source of
218-
* truth for membership, a role set here is reverted by the next sync.
219-
* Refusing says so instead of letting the change quietly disappear.
218+
* The member is re-read under the organization's mutation lock, so a
219+
* concurrent promotion to owner — or a directory provisioning this very
220+
* member — cannot slip between the checks and the write. When the
221+
* organization has made its identity provider the source of truth, a role
222+
* set here is reverted by the next sync; refusing says so.
220223
*/
221-
await assertMembershipNotScimManaged({
222-
organizationId,
223-
userId: memberId,
224+
const roleChange = await db.transaction(async (tx) => {
225+
await acquireOrganizationUserMutationLocks(tx, {
226+
userId: memberId,
227+
organizationIds: [organizationId],
228+
})
229+
await assertMembershipNotScimManaged({ organizationId, userId: memberId, executor: tx })
230+
return changeMemberRoleTx(tx, { organizationId, userId: memberId, role })
224231
})
225232

226-
/**
227-
* The shared primitive re-reads the member under the organization's
228-
* mutation lock, so a concurrent promotion to owner cannot slip between
229-
* the check above and the write.
230-
*/
231-
const roleChange = await db.transaction((tx) =>
232-
changeMemberRoleTx(tx, { organizationId, userId: memberId, role })
233-
)
234-
235233
/**
236234
* The audit row and analytics event fire whether or not the role actually
237235
* moved, exactly as this route did before the write went through the

apps/sim/ee/access-control/utils/permission-check.test.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,11 @@ function queueGroupResolution(
8383
workspaceGroups: WorkspaceGroupRow[] = [],
8484
defaultGroup: Array<{ config: Record<string, unknown> }> = []
8585
) {
86-
queueTableRows(permissionGroup, workspaceGroups)
86+
/** Every row carries the column default the resolver reads, as a real row would. */
87+
queueTableRows(
88+
permissionGroup,
89+
workspaceGroups.map((row) => ({ membershipMode: 'inherit', ...row }))
90+
)
8791
queueTableRows(permissionGroup, defaultGroup)
8892
}
8993

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

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,11 @@ import {
77
SCIM_MAX_PATCH_OPERATIONS,
88
SCIM_PATCH_OP_SCHEMA,
99
} from '@/lib/scim/protocol/constants'
10-
import { normalizeScimBoolean, unwrapSingleElement } from '@/lib/scim/protocol/normalize'
10+
import {
11+
canonicalizeAttributeNames,
12+
normalizeScimBoolean,
13+
unwrapSingleElement,
14+
} from '@/lib/scim/protocol/normalize'
1115

1216
/**
1317
* Wire schemas for the SCIM 2.0 surface.
@@ -63,18 +67,32 @@ const scimEnterpriseSchema = z.looseObject({
6367
* keeps the credential out of the parsed request object and therefore out of
6468
* every log line and error detail downstream.
6569
*/
66-
export const scimUserWriteSchema = z
67-
.looseObject({
68-
schemas: z.array(z.string().max(256)).min(1, 'schemas must name at least one URN').max(10),
69-
userName: z.string().trim().min(1, 'userName must not be empty').max(320),
70-
externalId: z.string().trim().max(256).optional(),
71-
active: scimBoolean.optional(),
72-
displayName: z.string().max(256).optional(),
73-
name: scimNameSchema.optional(),
74-
emails: z.array(scimEmailSchema).max(20).optional(),
75-
[SCIM_ENTERPRISE_USER_SCHEMA]: scimEnterpriseSchema.optional(),
76-
})
77-
.transform(({ password: _password, ...rest }) => rest)
70+
const USER_WRITE_ATTRIBUTES = [
71+
'schemas',
72+
'userName',
73+
'externalId',
74+
'active',
75+
'displayName',
76+
'name',
77+
'emails',
78+
SCIM_ENTERPRISE_USER_SCHEMA,
79+
] as const
80+
81+
export const scimUserWriteSchema = z.preprocess(
82+
(body) => canonicalizeAttributeNames(body, USER_WRITE_ATTRIBUTES),
83+
z
84+
.looseObject({
85+
schemas: z.array(z.string().max(256)).min(1, 'schemas must name at least one URN').max(10),
86+
userName: z.string().trim().min(1, 'userName must not be empty').max(320),
87+
externalId: z.string().trim().max(256).optional(),
88+
active: scimBoolean.optional(),
89+
displayName: z.string().max(256).optional(),
90+
name: scimNameSchema.optional(),
91+
emails: z.array(scimEmailSchema).max(20).optional(),
92+
[SCIM_ENTERPRISE_USER_SCHEMA]: scimEnterpriseSchema.optional(),
93+
})
94+
.transform(({ password: _password, ...rest }) => rest)
95+
)
7896
/** What a client may send. */
7997
export type ScimUserWrite = z.input<typeof scimUserWriteSchema>
8098
/** What the route receives after parsing, which is what the canonicalizer reads. */
@@ -86,12 +104,17 @@ const scimGroupMemberSchema = z.looseObject({
86104
type: z.string().max(64).optional(),
87105
})
88106

89-
export const scimGroupWriteSchema = z.looseObject({
90-
schemas: z.array(z.string().max(256)).min(1, 'schemas must name at least one URN').max(10),
91-
displayName: z.string().trim().min(1, 'displayName must not be empty').max(256),
92-
externalId: z.string().trim().max(256).optional(),
93-
members: z.array(scimGroupMemberSchema).max(SCIM_MAX_GROUP_MEMBERS).optional(),
94-
})
107+
const GROUP_WRITE_ATTRIBUTES = ['schemas', 'displayName', 'externalId', 'members'] as const
108+
109+
export const scimGroupWriteSchema = z.preprocess(
110+
(body) => canonicalizeAttributeNames(body, GROUP_WRITE_ATTRIBUTES),
111+
z.looseObject({
112+
schemas: z.array(z.string().max(256)).min(1, 'schemas must name at least one URN').max(10),
113+
displayName: z.string().trim().min(1, 'displayName must not be empty').max(256),
114+
externalId: z.string().trim().max(256).optional(),
115+
members: z.array(scimGroupMemberSchema).max(SCIM_MAX_GROUP_MEMBERS).optional(),
116+
})
117+
)
95118
export type ScimGroupWrite = z.input<typeof scimGroupWriteSchema>
96119
export type ScimGroupWriteParsed = z.output<typeof scimGroupWriteSchema>
97120

apps/sim/lib/copilot/generated/docs-manifest.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,7 @@ export const DOCS_MANIFEST: readonly string[] = [
375375
'platform/enterprise/data-drains.mdx',
376376
'platform/enterprise/data-retention.mdx',
377377
'platform/enterprise/forks.mdx',
378+
'platform/enterprise/scim.mdx',
378379
'platform/enterprise/self-hosted.mdx',
379380
'platform/enterprise/session-policies.mdx',
380381
'platform/enterprise/sso.mdx',

apps/sim/lib/core/config/deployment-shape.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ describe('resolveDeploymentShape', () => {
3636
dataRetention: false,
3737
inbox: true,
3838
sandboxes: true,
39+
scim: false,
3940
sessionPolicies: true,
4041
sso: true,
4142
usageMonitoring: false,

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

Lines changed: 40 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -34,43 +34,52 @@ export const issueScimCredential = defineAuthorizedScimAdminUseCase({
3434
operation: scimAdminOperations.issueCredential,
3535
async execute({ input, context }: ScimAdminUseCaseArgs<IssueScimCredentialInput>) {
3636
const connection = await requireConnection(context.organizationId)
37-
38-
const [active] = await db
39-
.select({ value: count() })
40-
.from(scimCredential)
41-
.where(activeCredentialCondition(connection.id))
42-
if ((active?.value ?? 0) >= MAX_ACTIVE_CREDENTIALS) {
43-
throw new OrchestrationError(
44-
'conflict',
45-
`At most ${MAX_ACTIVE_CREDENTIALS} credentials may be active at once. Revoke one before issuing another.`
46-
)
47-
}
48-
4937
const { secret, hash, prefix } = generateScimToken()
5038
const scopes = input.scopes ?? [...SCIM_SCOPES]
5139
const expiresAt = input.expiresInDays
5240
? new Date(Date.now() + input.expiresInDays * DAY_MS)
5341
: null
5442

55-
const [created] = await db
56-
.insert(scimCredential)
57-
.values({
58-
id: generateId(),
59-
connectionId: connection.id,
60-
tokenHash: hash,
61-
tokenPrefix: prefix,
62-
scopes,
63-
expiresAt,
64-
createdBy: context.actorUserId,
65-
})
66-
.returning({
67-
id: scimCredential.id,
68-
tokenPrefix: scimCredential.tokenPrefix,
69-
scopes: scimCredential.scopes,
70-
expiresAt: scimCredential.expiresAt,
71-
lastUsedAt: scimCredential.lastUsedAt,
72-
createdAt: scimCredential.createdAt,
73-
})
43+
const created = await db.transaction(async (tx) => {
44+
/** The connection row is the lock, so two issue requests cannot both see one free slot. */
45+
await tx
46+
.select({ id: scimConnection.id })
47+
.from(scimConnection)
48+
.where(eq(scimConnection.id, connection.id))
49+
.for('update')
50+
51+
const [active] = await tx
52+
.select({ value: count() })
53+
.from(scimCredential)
54+
.where(activeCredentialCondition(connection.id))
55+
if ((active?.value ?? 0) >= MAX_ACTIVE_CREDENTIALS) {
56+
throw new OrchestrationError(
57+
'conflict',
58+
`At most ${MAX_ACTIVE_CREDENTIALS} credentials may be active at once. Revoke one before issuing another.`
59+
)
60+
}
61+
62+
const [row] = await tx
63+
.insert(scimCredential)
64+
.values({
65+
id: generateId(),
66+
connectionId: connection.id,
67+
tokenHash: hash,
68+
tokenPrefix: prefix,
69+
scopes,
70+
expiresAt,
71+
createdBy: context.actorUserId,
72+
})
73+
.returning({
74+
id: scimCredential.id,
75+
tokenPrefix: scimCredential.tokenPrefix,
76+
scopes: scimCredential.scopes,
77+
expiresAt: scimCredential.expiresAt,
78+
lastUsedAt: scimCredential.lastUsedAt,
79+
createdAt: scimCredential.createdAt,
80+
})
81+
return row
82+
})
7483

7584
return { secret, credential: toCredentialView(created), connectionId: connection.id }
7685
},

apps/sim/lib/scim/application/groups/manage-groups.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { db } from '@sim/db'
33
import { scimGroup } from '@sim/db/schema'
44
import { and, eq, ne } from 'drizzle-orm'
55
import type { ScimPatchOperation } from '@/lib/api/contracts/scim'
6+
import { acquireOrganizationMutationLock } from '@/lib/billing/organizations/membership'
67
import type { DbOrTx } from '@/lib/db/types'
78
import {
89
defineAuthorizedScimUseCase,
@@ -164,6 +165,12 @@ export const createScimGroup = defineAuthorizedScimUseCase({
164165
}: ScimUseCaseArgs<CreateScimGroupInput>): Promise<ScimGroupWriteResult> {
165166
const { group } = input
166167
return db.transaction(async (tx) => {
168+
/**
169+
* Group writes serialize on the organization lock, which also heads the
170+
* documented lock order, so two full-membership PATCHes cannot both compute
171+
* from the same stale membership and keep members from both.
172+
*/
173+
await acquireOrganizationMutationLock(tx, context.organizationId)
167174
await assertDisplayNameAvailable(tx, {
168175
connectionId: context.connection.id,
169176
displayName: group.displayName,
@@ -226,6 +233,12 @@ export const replaceScimGroup = defineAuthorizedScimUseCase({
226233
context,
227234
}: ScimUseCaseArgs<ReplaceScimGroupInput>): Promise<ScimGroupWriteResult> {
228235
return db.transaction(async (tx) => {
236+
/**
237+
* Group writes serialize on the organization lock, which also heads the
238+
* documented lock order, so two full-membership PATCHes cannot both compute
239+
* from the same stale membership and keep members from both.
240+
*/
241+
await acquireOrganizationMutationLock(tx, context.organizationId)
229242
const current = await findScimGroupById(tx, context.connection.id, input.groupId)
230243
if (!current) throw notFound('SCIM Group not found')
231244

@@ -321,6 +334,12 @@ export const patchScimGroup = defineAuthorizedScimUseCase({
321334
const patch = parseGroupPatch(input.operations)
322335

323336
return db.transaction(async (tx) => {
337+
/**
338+
* Group writes serialize on the organization lock, which also heads the
339+
* documented lock order, so two full-membership PATCHes cannot both compute
340+
* from the same stale membership and keep members from both.
341+
*/
342+
await acquireOrganizationMutationLock(tx, context.organizationId)
324343
const current = await findScimGroupById(tx, context.connection.id, input.groupId)
325344
if (!current) throw notFound('SCIM Group not found')
326345

@@ -431,6 +450,12 @@ export const deleteScimGroup = defineAuthorizedScimUseCase({
431450
operation: scimOperations.deleteGroup,
432451
async execute({ input, context }: ScimUseCaseArgs<DeleteScimGroupInput>) {
433452
return db.transaction(async (tx) => {
453+
/**
454+
* Group writes serialize on the organization lock, which also heads the
455+
* documented lock order, so two full-membership PATCHes cannot both compute
456+
* from the same stale membership and keep members from both.
457+
*/
458+
await acquireOrganizationMutationLock(tx, context.organizationId)
434459
const current = await findScimGroupById(tx, context.connection.id, input.groupId)
435460
if (!current) throw notFound('SCIM Group not found')
436461

apps/sim/lib/scim/application/users/provision-user.ts

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ export interface ProvisionScimUserResult {
5353
createdAccount: boolean
5454
/** False when the account was already a member and only the SCIM link was new. */
5555
joinedOrganization: boolean
56+
/** The subscription seats were validated against, so the post-commit seat sync targets the same one. */
57+
subscriptionId: string | undefined
5658
organizationId: string
5759
resource: ReturnType<typeof toUserResource>
5860
}
@@ -179,7 +181,11 @@ export const provisionScimUser = defineAuthorizedScimUseCase({
179181
await deleteScimUser(db, existing.id)
180182
}
181183

182-
let provisioned: { scimUserId: string; joinedOrganization: boolean }
184+
let provisioned: {
185+
scimUserId: string
186+
joinedOrganization: boolean
187+
subscriptionId: string | undefined
188+
}
183189
try {
184190
provisioned = await db.transaction(async (tx) => {
185191
const seatPolicy = await resolveSeatPolicy(tx, context.organizationId)
@@ -231,7 +237,11 @@ export const provisionScimUser = defineAuthorizedScimUseCase({
231237
scimUserId: inserted.id,
232238
settings: context.connection.settings,
233239
})
234-
return { scimUserId: inserted.id, joinedOrganization: !membership.alreadyMember }
240+
return {
241+
scimUserId: inserted.id,
242+
joinedOrganization: !membership.alreadyMember,
243+
subscriptionId: seatPolicy.organizationSubscriptionId,
244+
}
235245
})
236246
} catch (error) {
237247
/**
@@ -253,7 +263,7 @@ export const provisionScimUser = defineAuthorizedScimUseCase({
253263
}
254264
throw error
255265
}
256-
const { scimUserId, joinedOrganization } = provisioned
266+
const { scimUserId, joinedOrganization, subscriptionId } = provisioned
257267

258268
const record = await findScimUserById(db, context.connection.id, scimUserId)
259269
if (!record) throw new ScimError(500, undefined, 'The provisioned user could not be read back')
@@ -263,6 +273,7 @@ export const provisionScimUser = defineAuthorizedScimUseCase({
263273
userId,
264274
createdAccount,
265275
joinedOrganization,
276+
subscriptionId,
266277
organizationId: context.organizationId,
267278
resource: toUserResource(toUserResourceRow(record, []), context.baseUrl),
268279
}
@@ -303,6 +314,8 @@ export const provisionScimUser = defineAuthorizedScimUseCase({
303314
await reconcileOrganizationSeats({
304315
organizationId: context.organizationId,
305316
reason: 'scim-member-added',
317+
/** The subscription admission was validated against, not whichever is newest now. */
318+
...(result.subscriptionId ? { subscriptionId: result.subscriptionId } : {}),
306319
})
307320
} catch (error) {
308321
logger.error('Failed to reconcile seats after directory provisioning', { error })

0 commit comments

Comments
 (0)