Skip to content

Commit ee13f2b

Browse files
matt-aitkenclaude
andcommitted
fix(rbac,webapp): isUsingPlugin() on the controller — drop sentinel-string coupling
The PAT-creation flow needed to know "is a real RBAC plugin loaded, or just the OSS fallback?" so it could skip the compensating-delete rollback path when the fallback returns ok:false from setTokenRole (no plugin → no role table to write to → that's not a failure). Previous implementation stringly-typed it: the fallback returned `{ ok: false, error: "RBAC plugin not installed" }`, and the webapp's PAT module compared `roleResult.error === FALLBACK_NOT_INSTALLED_ERROR` to detect the case. If the fallback's error string ever drifted, PAT creation would blow up with a compensating-delete on every install without an enterprise plugin. Replace with an explicit capability method on the controller: RoleBaseAccessController.isUsingPlugin(): Promise<boolean> OSS fallback returns false; cloud plugin returns true. LazyController delegates. The webapp gates the rbac.setTokenRole call on `await rbac.isUsingPlugin()`, skipping it when the fallback is in use. No string magic, decoupled from any specific error message, and the capability is generally useful — UI surfaces that gate role-pickers / admin-only sections on plugin presence can use the same method. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9f987ae commit ee13f2b

4 files changed

Lines changed: 36 additions & 32 deletions

File tree

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

Lines changed: 21 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,6 @@ const tokenGenerator = customAlphabet("123456789abcdefghijkmnopqrstuvwxyz", toke
1717
// staleness is fine.
1818
export const PAT_LAST_ACCESSED_THROTTLE_MS = 5 * 60 * 1000;
1919

20-
// The OSS fallback's setTokenRole returns this exact string when no
21-
// enterprise plugin is loaded. We treat that as "no role attached" —
22-
// the PAT is still valid; auth just falls through to legacy permissive
23-
// behaviour. Any other error is treated as a real failure and triggers
24-
// the compensating delete below.
25-
// Must match the OSS fallback's exact error string (see
26-
// `internal-packages/rbac/src/fallback.ts`'s `setTokenRole`). The
27-
// match is how we detect "no plugin installed" and skip the
28-
// compensating delete.
29-
const FALLBACK_NOT_INSTALLED_ERROR = "RBAC plugin not installed";
30-
3120
type CreatePersonalAccessTokenOptions = {
3221
name: string;
3322
userId: string;
@@ -380,34 +369,34 @@ export async function createPersonalAccessToken({
380369
// so a brief orphan window between the two writes is harmless. The
381370
// compensating delete narrows that window from "until manual cleanup"
382371
// to "until the request returns".
383-
if (roleId) {
372+
//
373+
// Skip the call entirely when no RBAC plugin is loaded — the OSS
374+
// fallback has no TokenRole table to write to. Gating on
375+
// `rbac.isUsingPlugin()` (rather than parsing the fallback's error
376+
// string) keeps the OSS-vs-cloud branch explicit and decoupled from
377+
// any specific error message.
378+
if (roleId && (await rbac.isUsingPlugin())) {
384379
const roleResult = await rbac.setTokenRole({
385380
tokenId: personalAccessToken.id,
386381
roleId,
387382
});
388383
if (!roleResult.ok) {
389-
// The default fallback always returns ok=false with this exact
390-
// message. That isn't a failure — there's no plugin to write to,
391-
// so the PAT just runs without an explicit role (matches the
392-
// pre-RBAC behaviour). Don't compensating-delete in that case.
393-
if (roleResult.error === FALLBACK_NOT_INSTALLED_ERROR) {
394-
logger.debug("createPersonalAccessToken: no RBAC plugin, skipping role assignment", {
395-
patId: personalAccessToken.id,
396-
userId,
397-
});
398-
} else {
399-
await prisma.personalAccessToken
400-
.delete({ where: { id: personalAccessToken.id } })
401-
.catch((err) => {
402-
logger.error("Failed to compensating-delete PAT after TokenRole insert failed", {
403-
patId: personalAccessToken.id,
404-
roleResultError: roleResult.error,
405-
deleteError: err instanceof Error ? err.message : String(err),
406-
});
384+
await prisma.personalAccessToken
385+
.delete({ where: { id: personalAccessToken.id } })
386+
.catch((err) => {
387+
logger.error("Failed to compensating-delete PAT after TokenRole insert failed", {
388+
patId: personalAccessToken.id,
389+
roleResultError: roleResult.error,
390+
deleteError: err instanceof Error ? err.message : String(err),
407391
});
408-
throw new Error(`Failed to assign role to access token: ${roleResult.error}`);
409-
}
392+
});
393+
throw new Error(`Failed to assign role to access token: ${roleResult.error}`);
410394
}
395+
} else if (roleId) {
396+
logger.debug("createPersonalAccessToken: no RBAC plugin, skipping role assignment", {
397+
patId: personalAccessToken.id,
398+
userId,
399+
});
411400
}
412401

413402
return {

internal-packages/rbac/src/fallback.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,10 @@ class RoleBaseAccessFallbackController implements RoleBaseAccessController {
6060
this.replica = clients.replica;
6161
}
6262

63+
async isUsingPlugin(): Promise<boolean> {
64+
return false;
65+
}
66+
6367
async authenticateBearer(
6468
request: Request,
6569
options?: { allowJWT?: boolean }

internal-packages/rbac/src/index.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,10 @@ class LazyController implements RoleBaseAccessController {
113113
return this._init;
114114
}
115115

116+
async isUsingPlugin(): Promise<boolean> {
117+
return (await this.c()).isUsingPlugin();
118+
}
119+
116120
async authenticateBearer(...args: Parameters<RoleBaseAccessController["authenticateBearer"]>) {
117121
const result = await (await this.c()).authenticateBearer(...args);
118122
return result.ok ? { ...result, ability: withActionAliases(result.ability) } : result;

packages/plugins/src/rbac.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,13 @@ export type PatAuthResult =
118118
};
119119

120120
export interface RoleBaseAccessController {
121+
// True when a real RBAC plugin is loaded (i.e. cloud); false when the
122+
// OSS fallback is in use. Hosts gate behaviour that's only meaningful
123+
// when the plugin is present (e.g. skipping role-attachment writes,
124+
// hiding role-pickers in the UI, branching on whether ability checks
125+
// are authoritative or permissive).
126+
isUsingPlugin(): Promise<boolean>;
127+
121128
// API routes (Bearer token): one DB query → identity + pre-built ability
122129
// options.allowJWT: when true, accepts PUBLIC_JWT tokens in addition to environment API keys
123130
authenticateBearer(request: Request, options?: { allowJWT?: boolean }): Promise<BearerAuthResult>;

0 commit comments

Comments
 (0)