fix(webapp): keep the last Owner on directory-sync role changes (#4230)

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.
This commit is contained in:
Oskar Otwinowski
2026-07-10 19:28:16 +02:00
committed by GitHub
parent b64b54c74e
commit 4be32d411c
2 changed files with 29 additions and 2 deletions
@@ -95,7 +95,19 @@ async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
roleId: effect.roleId,
});
if (!result.ok) {
throw retryableEffectError(`directorySync provision setUserRole failed: ${result.error}`);
// The org must keep one Owner: skip the role overwrite for the last
// Owner (they keep Owner) instead of failing the whole batch. Applies
// to a directory burst and to a dashboard group remap alike.
if (result.code === "last_owner") {
logger.info("directorySync: kept last Owner, skipped provision role overwrite", {
userId,
organizationId: effect.organizationId,
});
} else {
throw retryableEffectError(
`directorySync provision setUserRole failed: ${result.error}`
);
}
}
}
return;
@@ -107,6 +119,15 @@ async function applyEffect(effect: DirectorySyncEffect): Promise<void> {
roleId: effect.roleId,
});
if (!result.ok) {
// Keeping the org's last Owner is expected, not a failure — skip this
// one member and let the rest of the remap apply (no server error).
if (result.code === "last_owner") {
logger.info("directorySync: kept last Owner, skipped set_role", {
userId: effect.userId,
organizationId: effect.organizationId,
});
return;
}
throw retryableEffectError(`directorySync set_role failed: ${result.error}`);
}
return;
+7 -1
View File
@@ -422,7 +422,13 @@ export interface RoleBaseAccessController {
export type RoleMutationResult = { ok: true; role: Role } | { ok: false; error: string };
// Result for assignment / deletion mutations that don't return a value.
export type RoleAssignmentResult = { ok: true } | { ok: false; error: string };
// `code` is an optional machine-readable reason so callers can branch on
// expected outcomes (e.g. `last_owner`, the guard that keeps an org from
// losing its final Owner) instead of matching the free-text `error`.
export type RoleAssignmentErrorCode = "last_owner";
export type RoleAssignmentResult =
| { ok: true }
| { ok: false; error: string; code?: RoleAssignmentErrorCode };
import type { PluginDatabaseConfig } from "./databaseConfig.js";