From 8409fb250d30fdd54585a3593ef86fb5e59f065b Mon Sep 17 00:00:00 2001 From: Gustavo Valverde Date: Sat, 10 Oct 2026 11:38:31 +0000 Subject: [PATCH] fix(scim): return 409 when concurrent updates claim the same unique key (#11669) --- .changeset/scim-update-uniqueness-races.md | 5 + packages/scim/src/group-provisioning.ts | 462 ++++++++++-------- .../scim/src/scim-create-uniqueness.test.ts | 270 +++++++++- packages/scim/src/user-provisioning.ts | 261 +++++----- 4 files changed, 653 insertions(+), 345 deletions(-) create mode 100644 .changeset/scim-update-uniqueness-races.md diff --git a/.changeset/scim-update-uniqueness-races.md b/.changeset/scim-update-uniqueness-races.md new file mode 100644 index 0000000000..909dba0687 --- /dev/null +++ b/.changeset/scim-update-uniqueness-races.md @@ -0,0 +1,5 @@ +--- +"@better-auth/scim": patch +--- + +SCIM directories that send concurrent updates now get a `409` uniqueness error when a User `PATCH`, Group `PUT`, or Group `PATCH` claims a `userName`, `displayName`, `externalId`, or managed email that another request saved first. Previously the losing request failed with a `500` server error. Requests that don't race are unaffected. diff --git a/packages/scim/src/group-provisioning.ts b/packages/scim/src/group-provisioning.ts index 9cda42d5a8..133083841e 100644 --- a/packages/scim/src/group-provisioning.ts +++ b/packages/scim/src/group-provisioning.ts @@ -1247,88 +1247,102 @@ export function replaceSCIMGroup( externalIdKey, group.id, ); - const updatedGroup = await runGroupMutationTransaction( - adapter, - async (trx) => { - const currentGroup = await acquireSCIMGroupMutationLock( - trx, - connection, - group.id, - ); - const updatedAt = new Date(); + const updatedGroup = await runSCIMWriteWithUniquenessCheck( + () => + runGroupMutationTransaction(adapter, async (trx) => { + const currentGroup = await acquireSCIMGroupMutationLock( + trx, + connection, + group.id, + ); + const updatedAt = new Date(); + await assertDisplayNameAvailable( + trx, + connection.id, + displayNameKey, + currentGroup.id, + ); + await assertExternalIdAvailable( + trx, + connection.id, + externalIdKey, + currentGroup.id, + ); + const currentMemberships = await trx.findMany({ + model: "scimGroupMember", + where: [ + { field: "connectionId", value: connection.id }, + { field: "groupId", value: currentGroup.id }, + ], + }); + await projection.acquireUserLocks({ + database: trx, + provisioningDomainId: connection.provisioningDomainId, + scimUserIds: [ + ...new Set([ + ...scimUserIds, + ...currentMemberships.map( + (membership) => membership.scimUserId, + ), + ]), + ], + }); + const membershipDelta = await replaceGroupMemberships(trx, { + connectionId: connection.id, + groupId: currentGroup.id, + scimUserIds, + createdAt: updatedAt, + existingMemberships: currentMemberships, + }); + + const updated = await trx.update({ + model: "scimGroup", + where: [ + { field: "id", value: currentGroup.id }, + { field: "connectionId", value: connection.id }, + ], + update: { + displayName, + displayNameKey, + externalId: ctx.body.externalId ?? null, + externalIdKey: externalIdKey ?? null, + updatedAt, + }, + }); + if (!updated) { + throw createSCIMError("NOT_FOUND", { + detail: "SCIM Group not found", + }); + } + const affectedSCIMUserIds = new Set([ + ...scimUserIds, + ...membershipDelta.removedMemberships.map( + (membership) => membership.scimUserId, + ), + ]); + await projection.reconcileUsers({ + database: trx, + auth: ctx.context, + provisioningDomainId: connection.provisioningDomainId, + scimUserIds: [...affectedSCIMUserIds], + subjectLocksAcquired: true, + }); + await fenceActiveSCIMConnection(trx, connection.id); + return updated; + }), + async () => { await assertDisplayNameAvailable( - trx, + adapter, connection.id, displayNameKey, - currentGroup.id, + group.id, ); await assertExternalIdAvailable( - trx, + adapter, connection.id, externalIdKey, - currentGroup.id, + group.id, ); - const currentMemberships = await trx.findMany({ - model: "scimGroupMember", - where: [ - { field: "connectionId", value: connection.id }, - { field: "groupId", value: currentGroup.id }, - ], - }); - await projection.acquireUserLocks({ - database: trx, - provisioningDomainId: connection.provisioningDomainId, - scimUserIds: [ - ...new Set([ - ...scimUserIds, - ...currentMemberships.map( - (membership) => membership.scimUserId, - ), - ]), - ], - }); - const membershipDelta = await replaceGroupMemberships(trx, { - connectionId: connection.id, - groupId: currentGroup.id, - scimUserIds, - createdAt: updatedAt, - existingMemberships: currentMemberships, - }); - - const updated = await trx.update({ - model: "scimGroup", - where: [ - { field: "id", value: currentGroup.id }, - { field: "connectionId", value: connection.id }, - ], - update: { - displayName, - displayNameKey, - externalId: ctx.body.externalId ?? null, - externalIdKey: externalIdKey ?? null, - updatedAt, - }, - }); - if (!updated) { - throw createSCIMError("NOT_FOUND", { - detail: "SCIM Group not found", - }); - } - const affectedSCIMUserIds = new Set([ - ...scimUserIds, - ...membershipDelta.removedMemberships.map( - (membership) => membership.scimUserId, - ), - ]); - await projection.reconcileUsers({ - database: trx, - auth: ctx.context, - provisioningDomainId: connection.provisioningDomainId, - scimUserIds: [...affectedSCIMUserIds], - subjectLocksAcquired: true, - }); - await fenceActiveSCIMConnection(trx, connection.id); - return updated; }, ); @@ -1395,153 +1409,171 @@ export function patchSCIMGroup( resolveSCIMGroupPatchValueTarget, ); const incrementalPatch = parseIncrementalMembershipPatch(operations); - const updatedGroup = await runGroupMutationTransaction( - adapter, - async (trx) => { - const currentGroup = await acquireSCIMGroupMutationLock( - trx, - connection, - group.id, - ); - const updatedAt = new Date(); - let affectedSCIMUserIds: Set; - let resourceChanged: boolean; - let update: Partial< - Pick< - SCIMGroup, - | "displayName" - | "displayNameKey" - | "externalId" - | "externalIdKey" - | "updatedAt" - > - > = { updatedAt }; - - if (incrementalPatch) { - await projection.acquireUserLocks({ - database: trx, - provisioningDomainId: connection.provisioningDomainId, - scimUserIds: [ - ...incrementalPatch.desiredMembershipByUserId.keys(), - ], - }); - const membershipDelta = await applyIncrementalGroupMembershipPatch( + let attemptedKeys: + | { displayNameKey: string; externalIdKey: string | undefined } + | undefined; + const updatedGroup = await runSCIMWriteWithUniquenessCheck( + () => + runGroupMutationTransaction(adapter, async (trx) => { + attemptedKeys = undefined; + const currentGroup = await acquireSCIMGroupMutationLock( trx, - { + connection, + group.id, + ); + const updatedAt = new Date(); + let affectedSCIMUserIds: Set; + let resourceChanged: boolean; + let update: Partial< + Pick< + SCIMGroup, + | "displayName" + | "displayNameKey" + | "externalId" + | "externalIdKey" + | "updatedAt" + > + > = { updatedAt }; + + if (incrementalPatch) { + await projection.acquireUserLocks({ + database: trx, + provisioningDomainId: connection.provisioningDomainId, + scimUserIds: [ + ...incrementalPatch.desiredMembershipByUserId.keys(), + ], + }); + const membershipDelta = + await applyIncrementalGroupMembershipPatch(trx, { + connectionId: connection.id, + groupId: currentGroup.id, + patch: incrementalPatch, + createdAt: updatedAt, + }); + affectedSCIMUserIds = new Set([ + ...membershipDelta.addedMemberships.map( + (membership) => membership.scimUserId, + ), + ...membershipDelta.removedMemberships.map( + (membership) => membership.scimUserId, + ), + ]); + resourceChanged = affectedSCIMUserIds.size > 0; + } else { + const currentMemberships = await trx.findMany({ + model: "scimGroupMember", + where: [ + { field: "connectionId", value: connection.id }, + { field: "groupId", value: currentGroup.id }, + ], + }); + const patch = applyGroupPatch( + currentGroup, + currentMemberships.map((membership) => membership.scimUserId), + operations, + ); + await projection.acquireUserLocks({ + database: trx, + provisioningDomainId: connection.provisioningDomainId, + scimUserIds: [ + ...new Set([ + ...patch.memberIds, + ...currentMemberships.map( + (membership) => membership.scimUserId, + ), + ]), + ], + }); + const externalIdKey = createGroupExternalIdKey( + connection.id, + patch.externalId, + ); + const displayNameKey = createGroupDisplayNameKey( + connection.id, + patch.displayName, + ); + attemptedKeys = { displayNameKey, externalIdKey }; + await assertDisplayNameAvailable( + trx, + connection.id, + displayNameKey, + currentGroup.id, + ); + await assertExternalIdAvailable( + trx, + connection.id, + externalIdKey, + currentGroup.id, + ); + const membershipDelta = await replaceGroupMemberships(trx, { connectionId: connection.id, groupId: currentGroup.id, - patch: incrementalPatch, + scimUserIds: patch.memberIds, createdAt: updatedAt, - }, - ); - affectedSCIMUserIds = new Set([ - ...membershipDelta.addedMemberships.map( - (membership) => membership.scimUserId, - ), - ...membershipDelta.removedMemberships.map( - (membership) => membership.scimUserId, - ), - ]); - resourceChanged = affectedSCIMUserIds.size > 0; - } else { - const currentMemberships = await trx.findMany({ - model: "scimGroupMember", - where: [ - { field: "connectionId", value: connection.id }, - { field: "groupId", value: currentGroup.id }, - ], - }); - const patch = applyGroupPatch( - currentGroup, - currentMemberships.map((membership) => membership.scimUserId), - operations, - ); - await projection.acquireUserLocks({ - database: trx, - provisioningDomainId: connection.provisioningDomainId, - scimUserIds: [ - ...new Set([ - ...patch.memberIds, - ...currentMemberships.map( - (membership) => membership.scimUserId, - ), - ]), - ], - }); - const externalIdKey = createGroupExternalIdKey( - connection.id, - patch.externalId, - ); - const displayNameKey = createGroupDisplayNameKey( - connection.id, - patch.displayName, - ); - await assertDisplayNameAvailable( - trx, - connection.id, - displayNameKey, - currentGroup.id, - ); - await assertExternalIdAvailable( - trx, - connection.id, - externalIdKey, - currentGroup.id, - ); - const membershipDelta = await replaceGroupMemberships(trx, { - connectionId: connection.id, - groupId: currentGroup.id, - scimUserIds: patch.memberIds, - createdAt: updatedAt, - existingMemberships: currentMemberships, - }); - const metadataChanged = - patch.displayName !== currentGroup.displayName || - patch.externalId !== (currentGroup.externalId ?? undefined); - affectedSCIMUserIds = new Set([ - ...(metadataChanged ? patch.memberIds : []), - ...membershipDelta.addedMemberships.map( - (membership) => membership.scimUserId, - ), - ...membershipDelta.removedMemberships.map( - (membership) => membership.scimUserId, - ), - ]); - resourceChanged = metadataChanged || affectedSCIMUserIds.size > 0; - update = { - displayName: patch.displayName, - displayNameKey, - externalId: patch.externalId ?? null, - externalIdKey: externalIdKey ?? null, - updatedAt, - }; - } - let mutationResult = currentGroup; - if (resourceChanged) { - const updated = await trx.update({ - model: "scimGroup", - where: [ - { field: "id", value: currentGroup.id }, - { field: "connectionId", value: connection.id }, - ], - update, - }); - if (!updated) { - throw createSCIMError("NOT_FOUND", { - detail: "SCIM Group not found", + existingMemberships: currentMemberships, }); + const metadataChanged = + patch.displayName !== currentGroup.displayName || + patch.externalId !== (currentGroup.externalId ?? undefined); + affectedSCIMUserIds = new Set([ + ...(metadataChanged ? patch.memberIds : []), + ...membershipDelta.addedMemberships.map( + (membership) => membership.scimUserId, + ), + ...membershipDelta.removedMemberships.map( + (membership) => membership.scimUserId, + ), + ]); + resourceChanged = metadataChanged || affectedSCIMUserIds.size > 0; + update = { + displayName: patch.displayName, + displayNameKey, + externalId: patch.externalId ?? null, + externalIdKey: externalIdKey ?? null, + updatedAt, + }; } - await projection.reconcileUsers({ - database: trx, - auth: ctx.context, - provisioningDomainId: connection.provisioningDomainId, - scimUserIds: [...affectedSCIMUserIds], - subjectLocksAcquired: true, - }); - mutationResult = updated; - } - await fenceActiveSCIMConnection(trx, connection.id); - return mutationResult; + let mutationResult = currentGroup; + if (resourceChanged) { + const updated = await trx.update({ + model: "scimGroup", + where: [ + { field: "id", value: currentGroup.id }, + { field: "connectionId", value: connection.id }, + ], + update, + }); + if (!updated) { + throw createSCIMError("NOT_FOUND", { + detail: "SCIM Group not found", + }); + } + await projection.reconcileUsers({ + database: trx, + auth: ctx.context, + provisioningDomainId: connection.provisioningDomainId, + scimUserIds: [...affectedSCIMUserIds], + subjectLocksAcquired: true, + }); + mutationResult = updated; + } + await fenceActiveSCIMConnection(trx, connection.id); + return mutationResult; + }), + async () => { + if (!attemptedKeys) return; + await assertDisplayNameAvailable( + adapter, + connection.id, + attemptedKeys.displayNameKey, + group.id, + ); + await assertExternalIdAvailable( + adapter, + connection.id, + attemptedKeys.externalIdKey, + group.id, + ); }, ); diff --git a/packages/scim/src/scim-create-uniqueness.test.ts b/packages/scim/src/scim-create-uniqueness.test.ts index bb9536c898..f7889128c6 100644 --- a/packages/scim/src/scim-create-uniqueness.test.ts +++ b/packages/scim/src/scim-create-uniqueness.test.ts @@ -8,6 +8,7 @@ import { betterAuth } from "better-auth"; import type { MemoryDB } from "better-auth/adapters/memory"; import { memoryAdapter } from "better-auth/adapters/memory"; import { describe, expect, it } from "vitest"; +import type { SCIMIdentity } from "."; import { scim } from "."; import type { SCIMGroup, SCIMUser } from "./persistence"; import { createScopedKey } from "./resource-key"; @@ -40,12 +41,16 @@ function createData(): ResourceCreateData { }; } -function createAuth(database: BetterAuthOptions["database"]) { +function createAuth( + database: BetterAuthOptions["database"], + identity?: SCIMIdentity, +) { return betterAuth({ baseURL: "http://localhost:3000", database, plugins: [ scim({ + ...(identity ? { identity } : {}), connections: [ { id: "workforce", @@ -101,6 +106,7 @@ type UpdateInput = Parameters[0]; function createConcurrentUpdateAdapter( data: ResourceCreateData, + model: "user" | "scimUser" | "scimGroup", injectConcurrentCommit: () => void, ) { const race = { armed: false }; @@ -113,10 +119,10 @@ function createConcurrentUpdateAdapter( ) => adapter.transaction(async (transaction) => { const update = async (input: UpdateInput): Promise => { - if (race.armed && input.model === "scimUser") { + if (race.armed && input.model === model) { race.armed = false; injectConcurrentCommit(); - throw new Error("Simulated scimUser unique constraint"); + throw new Error(`Simulated ${model} unique constraint`); } return transaction.update(input); }; @@ -163,6 +169,45 @@ function createCommittedSCIMUser(userName: string): SCIMUser { }; } +function createCommittedSCIMGroup(displayName: string): SCIMGroup { + const now = new Date(); + return { + id: "concurrent-group", + connectionId: "workforce", + provisioningDomainId: "workforce", + revision: 0, + displayName, + displayNameKey: createScopedKey([ + "scim-group-display-name", + "workforce", + displayName.toLowerCase(), + ]), + orderKey: "concurrent-group-order", + createdAt: now, + updatedAt: now, + }; +} + +function createBetterAuthUser(id: string, email: string): User { + const now = new Date(); + return { + id, + name: email, + email, + emailVerified: false, + image: null, + createdAt: now, + updatedAt: now, + }; +} + +const replaceEmailsWith = (email: string) => ({ + schemas: ["urn:ietf:params:scim:api:messages:2.0:PatchOp" as const], + Operations: [ + { op: "replace", path: "emails", value: [{ value: email, primary: true }] }, + ], +}); + describe("SCIM resource create uniqueness races", () => { it("normalizes a concurrent Group displayName commit to SCIM uniqueness", async () => { const data = createData(); @@ -283,9 +328,13 @@ describe("SCIM resource create uniqueness races", () => { */ it("normalizes a concurrent userName commit during inactive User reprovisioning", async () => { const data = createData(); - const { database, race } = createConcurrentUpdateAdapter(data, () => { - data.scimUser.push(createCommittedSCIMUser("grace@example.com")); - }); + const { database, race } = createConcurrentUpdateAdapter( + data, + "scimUser", + () => { + data.scimUser.push(createCommittedSCIMUser("grace@example.com")); + }, + ); const auth = createAuth(database); const created = await auth.api.createSCIMUser({ body: { @@ -324,9 +373,13 @@ describe("SCIM resource create uniqueness races", () => { it("normalizes a concurrent userName commit during User replacement", async () => { const data = createData(); - const { database, race } = createConcurrentUpdateAdapter(data, () => { - data.scimUser.push(createCommittedSCIMUser("grace@example.com")); - }); + const { database, race } = createConcurrentUpdateAdapter( + data, + "scimUser", + () => { + data.scimUser.push(createCommittedSCIMUser("grace@example.com")); + }, + ); const auth = createAuth(database); const created = await auth.api.createSCIMUser({ body: { schemas: [USER_SCHEMA], userName: "ada@example.com" }, @@ -350,7 +403,11 @@ describe("SCIM resource create uniqueness races", () => { it("preserves a replacement failure when no committed uniqueness conflict exists", async () => { const data = createData(); - const { database, race } = createConcurrentUpdateAdapter(data, () => {}); + const { database, race } = createConcurrentUpdateAdapter( + data, + "scimUser", + () => {}, + ); const auth = createAuth(database); const created = await auth.api.createSCIMUser({ body: { schemas: [USER_SCHEMA], userName: "ada@example.com" }, @@ -366,4 +423,197 @@ describe("SCIM resource create uniqueness races", () => { }), ).rejects.toThrowError("Simulated scimUser unique constraint"); }); + + it("normalizes a concurrent userName commit during User PATCH", async () => { + const data = createData(); + const { database, race } = createConcurrentUpdateAdapter( + data, + "scimUser", + () => { + data.scimUser.push(createCommittedSCIMUser("grace@example.com")); + }, + ); + const auth = createAuth(database); + const created = await auth.api.createSCIMUser({ + body: { schemas: [USER_SCHEMA], userName: "ada@example.com" }, + headers, + }); + + race.armed = true; + await expect( + auth.api.patchSCIMUser({ + params: { userId: created.id }, + body: { + schemas: ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations: [ + { op: "replace", path: "userName", value: "grace@example.com" }, + ], + }, + headers, + }), + ).rejects.toMatchObject({ + body: expect.objectContaining({ + status: "409", + scimType: "uniqueness", + }), + }); + }); + + it("normalizes a concurrent displayName commit during Group replacement", async () => { + const data = createData(); + const { database, race } = createConcurrentUpdateAdapter( + data, + "scimGroup", + () => { + data.scimGroup.push(createCommittedSCIMGroup("Finance")); + }, + ); + const auth = createAuth(database); + const created = await auth.api.createSCIMGroup({ + body: { schemas: [GROUP_SCHEMA], displayName: "Engineering" }, + headers, + }); + + race.armed = true; + await expect( + auth.api.replaceSCIMGroup({ + params: { groupId: created.id }, + body: { schemas: [GROUP_SCHEMA], displayName: "Finance" }, + headers, + }), + ).rejects.toMatchObject({ + body: expect.objectContaining({ + status: "409", + scimType: "uniqueness", + }), + }); + }); + + it("normalizes a concurrent displayName commit during Group PATCH", async () => { + const data = createData(); + const { database, race } = createConcurrentUpdateAdapter( + data, + "scimGroup", + () => { + data.scimGroup.push(createCommittedSCIMGroup("Finance")); + }, + ); + const auth = createAuth(database); + const created = await auth.api.createSCIMGroup({ + body: { schemas: [GROUP_SCHEMA], displayName: "Engineering" }, + headers, + }); + + race.armed = true; + await expect( + auth.api.patchSCIMGroup({ + params: { groupId: created.id }, + body: { + schemas: ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations: [ + { op: "replace", path: "displayName", value: "Finance" }, + ], + }, + headers, + }), + ).rejects.toMatchObject({ + body: expect.objectContaining({ + status: "409", + scimType: "uniqueness", + }), + }); + }); + + it("preserves a Group PATCH failure when no committed uniqueness conflict exists", async () => { + const data = createData(); + const { database, race } = createConcurrentUpdateAdapter( + data, + "scimGroup", + () => {}, + ); + const auth = createAuth(database); + const created = await auth.api.createSCIMGroup({ + body: { schemas: [GROUP_SCHEMA], displayName: "Engineering" }, + headers, + }); + + race.armed = true; + await expect( + auth.api.patchSCIMGroup({ + params: { groupId: created.id }, + body: { + schemas: ["urn:ietf:params:scim:api:messages:2.0:PatchOp"], + Operations: [ + { op: "replace", path: "displayName", value: "Finance" }, + ], + }, + headers, + }), + ).rejects.toThrowError("Simulated scimGroup unique constraint"); + }); + + it("normalizes a concurrent managed email commit during User PATCH", async () => { + const data = createData(); + const { database, race } = createConcurrentUpdateAdapter( + data, + "user", + () => { + data.user.push( + createBetterAuthUser("concurrent-user", "grace@example.com"), + ); + }, + ); + const auth = createAuth(database); + const created = await auth.api.createSCIMUser({ + body: { schemas: [USER_SCHEMA], userName: "ada@example.com" }, + headers, + }); + + race.armed = true; + await expect( + auth.api.patchSCIMUser({ + params: { userId: created.id }, + body: replaceEmailsWith("grace@example.com"), + headers, + }), + ).rejects.toMatchObject({ + body: expect.objectContaining({ + status: "409", + scimType: "uniqueness", + }), + }); + }); + + it("preserves a User PATCH failure when the source does not manage the email", async () => { + const data = createData(); + data.user.push( + createBetterAuthUser("existing-user", "ada@example.com"), + createBetterAuthUser("other-user", "grace@example.com"), + ); + const { database, race } = createConcurrentUpdateAdapter( + data, + "scimUser", + () => {}, + ); + const auth = createAuth(database, { + resolveUser: () => ({ + action: "link", + userId: "existing-user", + profile: "preserve", + }), + }); + const created = await auth.api.createSCIMUser({ + body: { schemas: [USER_SCHEMA], userName: "ada@example.com" }, + headers, + }); + + race.armed = true; + await expect( + auth.api.patchSCIMUser({ + params: { userId: created.id }, + body: replaceEmailsWith("grace@example.com"), + headers, + }), + ).rejects.toThrowError("Simulated scimUser unique constraint"); + }); }); diff --git a/packages/scim/src/user-provisioning.ts b/packages/scim/src/user-provisioning.ts index 15a0aa1d09..b150d30f1d 100644 --- a/packages/scim/src/user-provisioning.ts +++ b/packages/scim/src/user-provisioning.ts @@ -364,24 +364,27 @@ async function reprovisionInactiveSCIMUser( return reprovisionedSource ?? concurrentIdentityMutation(); }), () => - assertSCIMUserReplacementAvailable(adapter, { - ...input, + assertSCIMUserUpdateAvailable(adapter, { + connection, source: retainedSource, + email: input.profile.primaryEmail, + userNameKey: input.userNameKey, + externalIdKey, }), ); } /** - * Rechecks a failed in-place replacement against committed state, so a + * Rechecks a failed in-place update against committed state, so a * concurrent `userName`, `externalId`, or managed email claim surfaces as SCIM * uniqueness instead of a storage error. */ -async function assertSCIMUserReplacementAvailable( +async function assertSCIMUserUpdateAvailable( adapter: Pick, input: { connection: SCIMConnection; source: SCIMUser; - profile: CanonicalSCIMUserProfile; + email: string; userNameKey: string; externalIdKey: string | undefined; }, @@ -399,7 +402,7 @@ async function assertSCIMUserReplacementAvailable( if (subject?.profileSourceId === input.source.id) { await assertBetterAuthEmailAvailable( adapter, - input.profile.primaryEmail, + input.email, input.source.userId, ); } @@ -1035,10 +1038,10 @@ export function replaceSCIMUser( return updatedSource; }), () => - assertSCIMUserReplacementAvailable(adapter, { + assertSCIMUserUpdateAvailable(adapter, { connection, source: scimUser, - profile, + email: profile.primaryEmail, userNameKey, externalIdKey, }), @@ -1099,122 +1102,140 @@ export function patchSCIMUser( } const connection = ctx.context.scimConnection; - const updatedSCIMUser = await runIdentityMutationTransaction( - adapter, - async (trx) => { - const sourceBeforeLock = await findSCIMUser( - trx, - connection, - scimUser.id, - ); - if (!sourceBeforeLock) { - throw createSCIMError("NOT_FOUND", { - detail: "SCIM User not found", - }); - } - const subjectBeforeLock = await requireSCIMSubject( - trx, - sourceBeforeLock.userId, - ); - const updatedAt = new Date(); - const subject = await identity.acquireSubjectRevision( - trx, - subjectBeforeLock, - updatedAt, - ); - const currentSource = await findSCIMUser( - trx, - connection, - sourceBeforeLock.id, - ); - if (!currentSource) { - throw createSCIMError("NOT_FOUND", { - detail: "SCIM User not found", - }); - } - if (currentSource.userId !== sourceBeforeLock.userId) { - throw createSCIMError("CONFLICT", { - detail: "The SCIM User identity changed concurrently", - }); - } - const patch = applySCIMUserPatch(currentSource, ctx.body.Operations); - if (!scimUserPatchChangesState(currentSource, patch)) { - await fenceActiveSCIMConnection(trx, connection.id); - return currentSource; - } - const userNameKey = createUserNameKey( - connection.id, - patch.userName.toLowerCase(), - ); - const externalIdKey = createExternalIdKey( - connection.id, - patch.externalId, - ); - await assertSCIMUserKeysAvailable(trx, { - connectionId: connection.id, - userNameKey, - externalIdKey, - excludeSCIMUserId: currentSource.id, - }); - if (subject.profileSourceId === currentSource.id) { - await updateManagedBetterAuthUser( + let attemptedUpdate: + | Parameters[1] + | undefined; + const updatedSCIMUser = await runSCIMWriteWithUniquenessCheck( + () => + runIdentityMutationTransaction(adapter, async (trx) => { + attemptedUpdate = undefined; + const sourceBeforeLock = await findSCIMUser( trx, - ctx.context.internalAdapter, - { - userId: currentSource.userId, - email: patch.primaryEmail, - name: patch.displayName, + connection, + scimUser.id, + ); + if (!sourceBeforeLock) { + throw createSCIMError("NOT_FOUND", { + detail: "SCIM User not found", + }); + } + const subjectBeforeLock = await requireSCIMSubject( + trx, + sourceBeforeLock.userId, + ); + const updatedAt = new Date(); + const subject = await identity.acquireSubjectRevision( + trx, + subjectBeforeLock, + updatedAt, + ); + const currentSource = await findSCIMUser( + trx, + connection, + sourceBeforeLock.id, + ); + if (!currentSource) { + throw createSCIMError("NOT_FOUND", { + detail: "SCIM User not found", + }); + } + if (currentSource.userId !== sourceBeforeLock.userId) { + throw createSCIMError("CONFLICT", { + detail: "The SCIM User identity changed concurrently", + }); + } + const patch = applySCIMUserPatch( + currentSource, + ctx.body.Operations, + ); + if (!scimUserPatchChangesState(currentSource, patch)) { + await fenceActiveSCIMConnection(trx, connection.id); + return currentSource; + } + const userNameKey = createUserNameKey( + connection.id, + patch.userName.toLowerCase(), + ); + const externalIdKey = createExternalIdKey( + connection.id, + patch.externalId, + ); + attemptedUpdate = { + connection, + source: currentSource, + email: patch.primaryEmail, + userNameKey, + externalIdKey, + }; + await assertSCIMUserKeysAvailable(trx, { + connectionId: connection.id, + userNameKey, + externalIdKey, + excludeSCIMUserId: currentSource.id, + }); + if (subject.profileSourceId === currentSource.id) { + await updateManagedBetterAuthUser( + trx, + ctx.context.internalAdapter, + { + userId: currentSource.userId, + email: patch.primaryEmail, + name: patch.displayName, + updatedAt, + }, + ); + } + const updatedSCIMUser = await trx.update({ + model: "scimUser", + where: [ + { field: "id", value: currentSource.id }, + { field: "connectionId", value: connection.id }, + ], + update: { + userName: patch.userName, + userNameKey, + primaryEmail: patch.primaryEmail, + workEmailValueIndex: createSCIMEmailValueIndex( + patch.emails, + "work", + ), + emailValueIndex: createSCIMEmailValueIndex(patch.emails), + displayName: patch.displayName, + formattedName: patch.formattedName, + givenName: patch.givenName ?? null, + familyName: patch.familyName ?? null, + serializedEmails: serializeSCIMEmails(patch.emails), + serializedAttributes: serializeSCIMUserAttributes( + patch.attributes, + ), + externalId: patch.externalId ?? null, + externalIdKey: externalIdKey ?? null, + active: patch.active, updatedAt, }, - ); - } - const updatedSCIMUser = await trx.update({ - model: "scimUser", - where: [ - { field: "id", value: currentSource.id }, - { field: "connectionId", value: connection.id }, - ], - update: { - userName: patch.userName, - userNameKey, - primaryEmail: patch.primaryEmail, - workEmailValueIndex: createSCIMEmailValueIndex( - patch.emails, - "work", - ), - emailValueIndex: createSCIMEmailValueIndex(patch.emails), - displayName: patch.displayName, - formattedName: patch.formattedName, - givenName: patch.givenName ?? null, - familyName: patch.familyName ?? null, - serializedEmails: serializeSCIMEmails(patch.emails), - serializedAttributes: serializeSCIMUserAttributes( - patch.attributes, - ), - externalId: patch.externalId ?? null, - externalIdKey: externalIdKey ?? null, - active: patch.active, - updatedAt, - }, - }); - if (!updatedSCIMUser) { - throw createSCIMError("NOT_FOUND", { - detail: "SCIM User not found", }); - } - await projection.reconcileUser({ - database: trx, - auth: ctx.context, - provisioningDomainId: connection.provisioningDomainId, - scimUserId: updatedSCIMUser.id, - }); - await identity.reconcileUser({ - database: trx, - auth: ctx.context, - subject, - }); - await fenceActiveSCIMConnection(trx, connection.id); - return updatedSCIMUser; + if (!updatedSCIMUser) { + throw createSCIMError("NOT_FOUND", { + detail: "SCIM User not found", + }); + } + await projection.reconcileUser({ + database: trx, + auth: ctx.context, + provisioningDomainId: connection.provisioningDomainId, + scimUserId: updatedSCIMUser.id, + }); + await identity.reconcileUser({ + database: trx, + auth: ctx.context, + subject, + }); + await fenceActiveSCIMConnection(trx, connection.id); + return updatedSCIMUser; + }), + async () => { + if (!attemptedUpdate) return; + await assertSCIMUserUpdateAvailable(adapter, attemptedUpdate); }, );