mirror of
https://github.com/better-auth/better-auth.git
synced 2026-10-18 11:32:15 -05:00
fix(scim): return 409 when concurrent updates claim the same unique key (#11669)
This commit is contained in:
Gustavo Valverde
@@ -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.
|
||||
@@ -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<SCIMGroupMember>({
|
||||
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<SCIMGroup>({
|
||||
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<SCIMGroupMember>({
|
||||
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<SCIMGroup>({
|
||||
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<string>;
|
||||
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<string>;
|
||||
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<SCIMGroupMember>({
|
||||
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<SCIMGroupMember>({
|
||||
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<SCIMGroup>({
|
||||
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<SCIMGroup>({
|
||||
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,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
@@ -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<DBTransactionAdapter["update"]>[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<unknown> => {
|
||||
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");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<DBAdapter, "findOne">,
|
||||
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<typeof assertSCIMUserUpdateAvailable>[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<SCIMUser>({
|
||||
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<SCIMUser>({
|
||||
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);
|
||||
},
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user