Skip to content

Commit 62db886

Browse files
0skiTrigger.dev RepoOps
authored andcommitted
fix(sso): re-add Directory Sync users the IdP re-creates and report plugin errors
Directory Sync now handles a user who is removed from the identity provider app and later re-assigned. The IdP creates them under a new directory user id, and they now rejoin as the same Trigger.dev user with the role from their groups, instead of failing on every retry. Out-of-order directory events for replaced identities can no longer restore access that was revoked later. Deleting a directory user now also removes members whose account was created by Directory Sync. SSO login and Directory Sync now resolve the same account for an email, and match an existing account even when its stored email uses different letter case, so they no longer create duplicate accounts. The SSO plugin contract gains an optional `reportError` hook, so errors the plugin logs reach the host's error tracker. Mono-RevId: d2c87da245541ecbbca7bf4eed20a856cc277d3c
1 parent de02ce2 commit 62db886

7 files changed

Lines changed: 139 additions & 36 deletions

File tree

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { Prisma, type PrismaClient } from "@trigger.dev/database";
2+
import { tryCatch } from "@trigger.dev/core/utils";
3+
4+
type DirectoryUserParams = {
5+
email: string;
6+
firstName: string | null;
7+
lastName: string | null;
8+
};
9+
10+
// Matches on the lowercased email. New rows are marked SSO since the user will
11+
// authenticate via the org's IdP.
12+
export async function findOrCreateDirectoryUser(
13+
client: PrismaClient,
14+
params: DirectoryUserParams
15+
): Promise<{ userId: string }> {
16+
const email = params.email.toLowerCase().trim();
17+
const existingId = await findUserIdByEmail(client, email);
18+
if (existingId) return { userId: existingId };
19+
20+
const name = [params.firstName, params.lastName].filter(Boolean).join(" ").trim() || null;
21+
const [error, created] = await tryCatch(
22+
client.user.create({
23+
data: { email, authenticationMethod: "SSO", name, displayName: name },
24+
select: { id: true },
25+
})
26+
);
27+
if (!error) return { userId: created.id };
28+
29+
// Concurrent directory events for one email race on create; the loser reuses
30+
// the winner's row.
31+
const isUniqueViolation =
32+
error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2002";
33+
if (!isUniqueViolation) throw error;
34+
35+
const winnerId = await findUserIdByEmail(client, email);
36+
if (!winnerId) throw error;
37+
return { userId: winnerId };
38+
}
39+
40+
// Same rule as SSO login: an exact match on the lowercased email (unique
41+
// index) wins, otherwise a single case-insensitive match. That fallback scans
42+
// User, which is fine for this rare first-contact lookup. Several casings
43+
// without an exact match are ambiguous, so nothing matches.
44+
async function findUserIdByEmail(client: PrismaClient, email: string): Promise<string | null> {
45+
const exact = await client.user.findFirst({ where: { email }, select: { id: true } });
46+
if (exact) return exact.id;
47+
48+
const folded = await client.$queryRaw<Array<{ id: string }>>`
49+
SELECT "id" FROM "User" WHERE lower("email") = ${email} LIMIT 2
50+
`;
51+
if (folded.length !== 1) return null;
52+
return folded[0].id;
53+
}

‎apps/webapp/app/models/orgMember.server.ts‎

Lines changed: 4 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { $transaction, Prisma, prisma } from "~/db.server";
22
import { deleteOrgMember } from "./deleteOrgMember.server";
3+
import { findOrCreateDirectoryUser } from "./directoryUser.server";
34
import type { MembershipSource } from "~/models/member.server";
45
import { logger } from "~/services/logger.server";
56
import { enqueueMemberDevelopmentEnvironments } from "~/services/memberDevEnvironments.server";
@@ -174,45 +175,14 @@ export async function ensureOrgMember(
174175
return { created: true, orgMemberId: member.id, devEnvironmentsQueued: enqueued };
175176
}
176177

177-
// Find-or-create a User for a directory-provisioned member. Directory Sync
178-
// can provision a user before they have ever logged in, so the User row may
179-
// not exist yet. Email is the natural key (lowercased). New rows are marked
180-
// SSO since the user will authenticate via the org's IdP.
178+
// Directory Sync can provision a user before they have ever logged in, so
179+
// the User row may not exist yet.
181180
export async function ensureUserForDirectory(params: {
182181
email: string;
183182
firstName: string | null;
184183
lastName: string | null;
185184
}): Promise<{ userId: string }> {
186-
const email = params.email.toLowerCase().trim();
187-
const existing = await prisma.user.findFirst({ where: { email }, select: { id: true } });
188-
if (existing) return { userId: existing.id };
189-
190-
const name = [params.firstName, params.lastName].filter(Boolean).join(" ").trim() || null;
191-
// `User.email` is unique, so two concurrent directory events for the same
192-
// email can both miss the lookup above and race on create; the loser gets
193-
// P2002. Treat that as the idempotent "already exists" case (same pattern as
194-
// `ensureOrgMember`) rather than throwing and burning a webhook retry.
195-
try {
196-
const created = await prisma.user.create({
197-
data: {
198-
email,
199-
authenticationMethod: "SSO",
200-
name,
201-
displayName: name,
202-
},
203-
select: { id: true },
204-
});
205-
return { userId: created.id };
206-
} catch (error) {
207-
if (error instanceof Prisma.PrismaClientKnownRequestError && error.code === "P2002") {
208-
const existingAfterConflict = await prisma.user.findFirst({
209-
where: { email },
210-
select: { id: true },
211-
});
212-
if (existingAfterConflict) return { userId: existingAfterConflict.id };
213-
}
214-
throw error;
215-
}
185+
return findOrCreateDirectoryUser(prisma, params);
216186
}
217187

218188
// Whether the user holds the Owner system role in this org. Owner is the one

‎apps/webapp/app/services/sso.server.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { $replica, prisma } from "~/db.server";
22
import type { PrismaClient } from "@trigger.dev/database";
33
import sso from "@trigger.dev/sso";
4+
import { Logger } from "@trigger.dev/core/logger";
45
import { env } from "~/env.server";
56

67
// sso.create() is synchronous — returns a lazy controller that resolves
@@ -30,5 +31,6 @@ export const ssoController = sso.create(
3031
writerConnectionLimit: env.SSO_DATABASE_WRITER_CONNECTION_LIMIT,
3132
readerConnectionLimit: env.SSO_DATABASE_READER_CONNECTION_LIMIT,
3233
},
34+
reportError: (message, ...args) => Logger.onError?.(message, ...args),
3335
}
3436
);

‎apps/webapp/app/v3/accountsWebhookWorker.server.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,9 @@ function initializeWorker() {
7474
data: payload.data,
7575
});
7676
if (result.isErr()) {
77-
const error = new Error(`account webhook processing failed: ${result.error}`);
77+
const error = new Error(
78+
`account webhook processing failed for ${payload.event}: ${result.error}`
79+
);
7880
if (result.error === "not_ready") {
7981
Object.assign(error, { logLevel: "warn" as const });
8082
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { postgresTest } from "@internal/testcontainers";
2+
import { describe, expect, vi } from "vitest";
3+
import { findOrCreateDirectoryUser } from "~/models/directoryUser.server";
4+
5+
vi.setConfig({ testTimeout: 60_000 });
6+
7+
describe("findOrCreateDirectoryUser", () => {
8+
postgresTest("reuses the user with the lowercased email", async ({ prisma }) => {
9+
const existing = await prisma.user.create({
10+
data: { email: "kai@example.com", authenticationMethod: "GITHUB" },
11+
});
12+
13+
const result = await findOrCreateDirectoryUser(prisma, {
14+
email: "Kai@Example.com",
15+
firstName: "Kai",
16+
lastName: null,
17+
});
18+
19+
expect(result.userId).toBe(existing.id);
20+
expect(await prisma.user.count()).toBe(1);
21+
});
22+
23+
postgresTest("reuses a user whose stored email differs only in case", async ({ prisma }) => {
24+
const existing = await prisma.user.create({
25+
data: { email: "Kai.Smith@Example.com", authenticationMethod: "GITHUB" },
26+
});
27+
28+
const result = await findOrCreateDirectoryUser(prisma, {
29+
email: "kai.smith@example.com",
30+
firstName: "Kai",
31+
lastName: null,
32+
});
33+
34+
expect(result.userId).toBe(existing.id);
35+
expect(await prisma.user.count()).toBe(1);
36+
});
37+
38+
postgresTest("creates a separate user when only ambiguous casings exist", async ({ prisma }) => {
39+
const upper = await prisma.user.create({
40+
data: { email: "Amb@example.com", authenticationMethod: "GITHUB" },
41+
});
42+
const shout = await prisma.user.create({
43+
data: { email: "AMB@example.com", authenticationMethod: "GOOGLE" },
44+
});
45+
46+
const result = await findOrCreateDirectoryUser(prisma, {
47+
email: "amb@example.com",
48+
firstName: null,
49+
lastName: null,
50+
});
51+
52+
expect([upper.id, shout.id]).not.toContain(result.userId);
53+
const created = await prisma.user.findFirstOrThrow({ where: { id: result.userId } });
54+
expect(created.email).toBe("amb@example.com");
55+
});
56+
57+
postgresTest("creates a lowercased SSO user when none matches", async ({ prisma }) => {
58+
const result = await findOrCreateDirectoryUser(prisma, {
59+
email: " New.User@Example.com ",
60+
firstName: "New",
61+
lastName: "User",
62+
});
63+
64+
const created = await prisma.user.findFirstOrThrow({ where: { id: result.userId } });
65+
expect(created).toMatchObject({
66+
email: "new.user@example.com",
67+
authenticationMethod: "SSO",
68+
name: "New User",
69+
});
70+
});
71+
});

‎internal-packages/sso/src/index.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import type {
1010
SsoFlow,
1111
SsoMutationError,
1212
SsoPlugin,
13+
SsoPluginConfig,
1314
SsoPortalError,
1415
SsoProfile,
1516
SsoResolutionDecision,
@@ -38,6 +39,8 @@ export type SsoCreateOptions = {
3839
// follows the host's writer/replica topology. The fallback ignores this —
3940
// it queries through the Prisma clients passed as `SsoPrismaInput`.
4041
database?: PluginDatabaseConfig;
42+
// Forwarded to the plugin so the errors it logs reach the host's error tracker.
43+
reportError?: SsoPluginConfig["reportError"];
4144
};
4245

4346
// Loads the cloud plugin lazily; falls back to the OSS no-op
@@ -62,7 +65,7 @@ export class LazyController implements SsoController {
6265
const module = await importer(moduleName);
6366
const plugin: SsoPlugin = module.default;
6467
console.log("SSO: using plugin implementation");
65-
return plugin.create({ database: options?.database });
68+
return plugin.create({ database: options?.database, reportError: options?.reportError });
6669
} catch (err) {
6770
// Distinguish the two failure modes the dynamic import can hit:
6871
//

‎packages/plugins/src/sso.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,8 @@ export type SsoPluginConfig = {
362362
// Database connections for a plugin that owns its own client. Omitted →
363363
// the plugin falls back to its own defaults.
364364
database?: PluginDatabaseConfig;
365+
// Receives every error the plugin logs, so it reaches the host's error tracker.
366+
reportError?: (message: string, ...args: Array<Record<string, unknown> | undefined>) => void;
365367
};
366368

367369
export interface SsoPlugin {

0 commit comments

Comments
 (0)