Skip to content

Commit 9d7a60b

Browse files
0skiTrigger.dev RepoOps
authored andcommitted
fix(webapp,plugins): additive Directory Sync roles + retryable account webhooks
Directory Sync is now additive: connecting a directory no longer resets or removes the roles of members you added yourself. A member's role changes only when they belong to a group you've explicitly mapped to a role — new groups start unmapped ("Inherit") and change nothing until you map them. Owner can now be assigned from the Directory Sync and SSO role menus. Out-of-order webhooks from your identity provider are retried instead of dropped, so directory setup is more reliable. Mono-RevId: cd148230d7708141c30a13a4aae0e5ba2a4bfa5e
1 parent f3aeba9 commit 9d7a60b

5 files changed

Lines changed: 44 additions & 27 deletions

File tree

‎apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.sso/route.tsx‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,8 @@ export const loader = dashboardLoader(
168168
const status = statusResult.isOk() ? statusResult.value : EMPTY_SSO_STATUS;
169169
const directorySync = dsyncResult.isOk() ? dsyncResult.value : EMPTY_DIRECTORY_SYNC_STATUS;
170170

171-
// JIT can't grant Owner (reserved), and non-assignable/plan-gated roles
172-
// are filtered out.
173171
const assignable = new Set(assignableIds);
174-
const jitRoles = allRoles.filter((r) => r.name !== "Owner" && assignable.has(r.id));
172+
const jitRoles = allRoles.filter((r) => assignable.has(r.id));
175173

176174
return typedjson({
177175
status,
@@ -810,7 +808,7 @@ function ActiveConnectionState({
810808
/>
811809
<SettingsRow
812810
title="Default role for new users"
813-
description="Assigned to users created by just-in-time provisioning. Owner can't be granted automatically."
811+
description="Assigned to users created by just-in-time provisioning."
814812
action={
815813
<Select<string, Role>
816814
value={draftJitRoleId}
@@ -1085,7 +1083,11 @@ function DirectorySyncSection({
10851083
setDraftGroupRoles((prev) => ({ ...prev, [group.groupId]: v }))
10861084
}
10871085
items={[
1088-
{ id: NULL_ROLE_VALUE, name: "No access", description: "" },
1086+
{
1087+
id: NULL_ROLE_VALUE,
1088+
name: "Inherit",
1089+
description: "Uses the Default role for unmapped users",
1090+
},
10891091
...jitRoles,
10901092
]}
10911093
variant="secondary/small"
@@ -1094,7 +1096,7 @@ function DirectorySyncSection({
10941096
placement="bottom-end"
10951097
text={(v) =>
10961098
v === NULL_ROLE_VALUE
1097-
? "No access"
1099+
? "Inherit"
10981100
: (jitRoles.find((r) => r.id === v)?.name ?? "Select a role")
10991101
}
11001102
>

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

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -88,9 +88,7 @@ async function applyEffect(effect: DirectorySyncEffect): Promise<string | null>
8888
source: "directory_sync",
8989
});
9090

91-
// Directory owns the role: overwrite even an existing member
92-
// (ensureOrgMember only sets it on create).
93-
if (effect.roleId) {
91+
if (effect.roleId && effect.roleAuthoritative) {
9492
const result = await rbac.setUserRole({
9593
userId,
9694
organizationId: effect.organizationId,

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ function initializeWorker() {
7474
data: payload.data,
7575
});
7676
if (result.isErr()) {
77-
throw new Error(`account webhook processing failed: ${result.error}`);
77+
const error = new Error(`account webhook processing failed: ${result.error}`);
78+
if (result.error === "not_ready") {
79+
Object.assign(error, { logLevel: "warn" as const });
80+
}
81+
throw error;
7882
}
7983
// Directory-sync events return membership effects to apply against
8084
// public.* tables (the plugin never writes those). A throw here

‎apps/webapp/test/directorySyncEffects.server.test.ts‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,11 @@ import { applyDirectorySyncEffects } from "~/services/directorySyncEffects.serve
3131
const ENTITLED_ORG = "org_entitled";
3232
const UNENTITLED_ORG = "org_unentitled";
3333

34-
function provision(organizationId: string, email = "someone@acme.com"): DirectorySyncEffect {
34+
function provision(
35+
organizationId: string,
36+
email = "someone@acme.com",
37+
roleAuthoritative = false
38+
): DirectorySyncEffect {
3539
return {
3640
kind: "provision",
3741
userId: "user_1",
@@ -40,6 +44,7 @@ function provision(organizationId: string, email = "someone@acme.com"): Director
4044
lastName: null,
4145
organizationId,
4246
roleId: null,
47+
roleAuthoritative,
4348
};
4449
}
4550

@@ -165,7 +170,11 @@ describe("applyDirectorySyncEffects — SSO entitlement gate", () => {
165170
devEnvironmentsQueued: false,
166171
});
167172

168-
const effect = { ...provision(ENTITLED_ORG), roleId: "role_restricted" };
173+
const effect = {
174+
...provision(ENTITLED_ORG),
175+
roleId: "role_restricted",
176+
roleAuthoritative: true,
177+
};
169178

170179
const { unqueuedUserIds } = await applyDirectorySyncEffects([effect]);
171180

@@ -175,6 +184,23 @@ describe("applyDirectorySyncEffects — SSO entitlement gate", () => {
175184
);
176185
});
177186

187+
it("does not overwrite an existing role for a non-authoritative provision", async () => {
188+
getSsoEntitlement.mockResolvedValue("entitled");
189+
190+
const effect = {
191+
...provision(ENTITLED_ORG),
192+
roleId: "role_restricted",
193+
roleAuthoritative: false,
194+
};
195+
196+
await applyDirectorySyncEffects([effect]);
197+
198+
expect(ensureOrgMember).toHaveBeenCalledWith(
199+
expect.objectContaining({ roleId: "role_restricted", organizationId: ENTITLED_ORG })
200+
);
201+
expect(setUserRole).not.toHaveBeenCalled();
202+
});
203+
178204
it("reports nothing to retry when every provision was queued", async () => {
179205
getSsoEntitlement.mockResolvedValue("entitled");
180206

‎packages/plugins/src/sso.ts‎

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -99,17 +99,6 @@ export type DirectorySyncStatus = {
9999
groups: ReadonlyArray<DirectoryGroupMapping>;
100100
};
101101

102-
// A host-actionable membership mutation derived from a directory-sync event.
103-
// The plugin owns all `enterprise.*` writes; these effects describe the
104-
// `public.*` (User / OrgMember / role / token) writes the host must perform —
105-
// the plugin never touches those tables. The host worker applies them
106-
// idempotently.
107-
//
108-
// - `provision`: ensure the User exists (create when `userId === null`),
109-
// ensure the OrgMember exists, and set its role.
110-
// - `deprovision`: remove the membership (guarded against last-Owner), force
111-
// logout, and revoke tokens per host policy.
112-
// - `set_role`: overwrite the member's role (directory-authoritative).
113102
export type DirectorySyncEffect =
114103
| {
115104
kind: "provision";
@@ -119,6 +108,7 @@ export type DirectorySyncEffect =
119108
lastName: string | null;
120109
organizationId: string;
121110
roleId: string | null;
111+
roleAuthoritative: boolean;
122112
}
123113
| { kind: "deprovision"; userId: string; organizationId: string }
124114
| { kind: "set_role"; userId: string; organizationId: string; roleId: string };
@@ -147,10 +137,7 @@ export type SsoPortalError = "idp_org_unavailable" | "internal";
147137
// session is NOT an error: it's a successful result of `{ valid: false }`.
148138
export type SsoValidateError = "internal";
149139

150-
// Inbound webhook handling. `invalid_signature` → reject (4xx, no retry);
151-
// `feature_disabled` → no plugin installed (host returns 404); `internal`
152-
// → transient, the host returns 5xx so the provider retries.
153-
export type SsoWebhookError = "invalid_signature" | "feature_disabled" | "internal";
140+
export type SsoWebhookError = "invalid_signature" | "feature_disabled" | "internal" | "not_ready";
154141

155142
// A verified, JSON-serializable inbound event. Vendor-neutral envelope —
156143
// `event` is the provider's event-type string, `data` its opaque payload.

0 commit comments

Comments
 (0)