Skip to content

Commit d6b077b

Browse files
waleedlatif1claude
andcommitted
fix(scim): serialize first-time configuration; reconcile members when a rename drops an automatic mapping
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
1 parent 61a1cad commit d6b077b

3 files changed

Lines changed: 15 additions & 10 deletions

File tree

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
import { generateId } from '@sim/utils/id'
1010
import { and, desc, eq } from 'drizzle-orm'
1111
import type { ScimConnectionSettingsInput } from '@/lib/api/contracts/organization-scim'
12+
import { acquireOrganizationMutationLock } from '@/lib/billing/organizations/membership'
1213
import { OrchestrationError } from '@/lib/core/orchestration/types'
1314
import {
1415
assertWorkspaceInOrganization,
@@ -74,11 +75,13 @@ export const configureScimConnection = defineAuthorizedScimAdminUseCase({
7475
}
7576

7677
/**
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.
78+
* Read, merge, and write under the organization lock, so two administrators
79+
* changing different settings at once both land instead of the later write
80+
* carrying a stale copy of the earlier one's field — and two first-time
81+
* enables, where there is no row yet to lock, cannot both insert.
8082
*/
8183
const { created, status } = await db.transaction(async (tx) => {
84+
await acquireOrganizationMutationLock(tx, context.organizationId)
8285
const [existing] = await tx
8386
.select({
8487
id: scimConnection.id,
@@ -88,7 +91,6 @@ export const configureScimConnection = defineAuthorizedScimAdminUseCase({
8891
.from(scimConnection)
8992
.where(eq(scimConnection.organizationId, context.organizationId))
9093
.limit(1)
91-
.for('update')
9294

9395
const nextSettings: ScimConnectionSettings = {
9496
/**

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,9 @@ export const replaceScimGroup = defineAuthorizedScimUseCase({
267267
scimGroupId: current.id,
268268
displayName: input.group.displayName,
269269
})
270-
/** A new mapping applies to everyone already in the group, not only to those moving today. */
271-
if (mapped === 'mapped') for (const scimUserId of before) touched.add(scimUserId)
270+
/** A mapping gained or lost applies to everyone already in the group, not only to those moving today. */
271+
if (mapped === 'mapped' || mapped === 'unmapped')
272+
for (const scimUserId of before) touched.add(scimUserId)
272273
}
273274

274275
for (const scimUserId of before) {
@@ -393,7 +394,7 @@ export const patchScimGroup = defineAuthorizedScimUseCase({
393394
scimGroupId: current.id,
394395
displayName: patch.displayName,
395396
})
396-
if (mapped === 'mapped') {
397+
if (mapped === 'mapped' || mapped === 'unmapped') {
397398
for (const scimUserId of await loadGroupMemberIds(tx, current.id)) {
398399
touched.add(scimUserId)
399400
}

apps/sim/lib/scim/projection/auto-map.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import type { DbOrTx } from '@/lib/db/types'
2424
export async function autoMapPermissionGroupByName(
2525
tx: DbOrTx,
2626
params: { organizationId: string; scimGroupId: string; displayName: string }
27-
): Promise<'mapped' | 'already-mapped' | 'no-match'> {
27+
): Promise<'mapped' | 'already-mapped' | 'unmapped' | 'no-match'> {
2828
const [target] = await tx
2929
.select({ id: permissionGroup.id, membershipMode: permissionGroup.membershipMode })
3030
.from(permissionGroup)
@@ -37,7 +37,7 @@ export async function autoMapPermissionGroupByName(
3737
)
3838
.limit(1)
3939

40-
await tx
40+
const removed = await tx
4141
.delete(scimGroupMapping)
4242
.where(
4343
and(
@@ -47,7 +47,9 @@ export async function autoMapPermissionGroupByName(
4747
...(target ? [ne(scimGroupMapping.permissionGroupId, target.id)] : [])
4848
)
4949
)
50-
if (!target) return 'no-match'
50+
.returning({ id: scimGroupMapping.id })
51+
/** `unmapped` tells the caller access changed even though nothing new was mapped. */
52+
if (!target) return removed.length > 0 ? 'unmapped' : 'no-match'
5153

5254
const [existing] = await tx
5355
.select({ id: scimGroupMapping.id })

0 commit comments

Comments
 (0)