Skip to content

Commit 77ab141

Browse files
committed
fix(webapp): keep the last Owner on directory-sync role changes
Applying a directory-sync effect that would demote the org's last Owner (a group remap, or a provision) previously threw and 500'd the settings save. Now rbac.setUserRole reports code:"last_owner" and applyEffect skips just that member (they keep Owner) while the rest of the batch applies. Adds the machine-readable RoleAssignmentResult.code to the plugin contract so callers can tell the last-owner guard apart from a real failure.
1 parent b64b54c commit 77ab141

2 files changed

Lines changed: 29 additions & 2 deletions

File tree

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,19 @@ async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
9595
roleId: effect.roleId,
9696
});
9797
if (!result.ok) {
98-
throw retryableEffectError(`directorySync provision setUserRole failed: ${result.error}`);
98+
// The org must keep one Owner: skip the role overwrite for the last
99+
// Owner (they keep Owner) instead of failing the whole batch. Applies
100+
// to a directory burst and to a dashboard group remap alike.
101+
if (result.code === "last_owner") {
102+
logger.info("directorySync: kept last Owner, skipped provision role overwrite", {
103+
userId,
104+
organizationId: effect.organizationId,
105+
});
106+
} else {
107+
throw retryableEffectError(
108+
`directorySync provision setUserRole failed: ${result.error}`
109+
);
110+
}
99111
}
100112
}
101113
return;
@@ -107,6 +119,15 @@ async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
107119
roleId: effect.roleId,
108120
});
109121
if (!result.ok) {
122+
// Keeping the org's last Owner is expected, not a failure — skip this
123+
// one member and let the rest of the remap apply (no server error).
124+
if (result.code === "last_owner") {
125+
logger.info("directorySync: kept last Owner, skipped set_role", {
126+
userId: effect.userId,
127+
organizationId: effect.organizationId,
128+
});
129+
return;
130+
}
110131
throw retryableEffectError(`directorySync set_role failed: ${result.error}`);
111132
}
112133
return;

packages/plugins/src/rbac.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -422,7 +422,13 @@ export interface RoleBaseAccessController {
422422
export type RoleMutationResult = { ok: true; role: Role } | { ok: false; error: string };
423423

424424
// Result for assignment / deletion mutations that don't return a value.
425-
export type RoleAssignmentResult = { ok: true } | { ok: false; error: string };
425+
// `code` is an optional machine-readable reason so callers can branch on
426+
// expected outcomes (e.g. `last_owner`, the guard that keeps an org from
427+
// losing its final Owner) instead of matching the free-text `error`.
428+
export type RoleAssignmentErrorCode = "last_owner";
429+
export type RoleAssignmentResult =
430+
| { ok: true }
431+
| { ok: false; error: string; code?: RoleAssignmentErrorCode };
426432

427433
import type { PluginDatabaseConfig } from "./databaseConfig.js";
428434

0 commit comments

Comments
 (0)