Skip to content

Commit 71901dd

Browse files
committed
RBAC: scrub "enterprise" / "OSS" / cloud-side references from comments
Code comments throughout the OSS-facing RBAC surface mentioned the enterprise plugin, CASL, the cloud webapp, the cloud-side test suite, and specific cloud file paths. Two reasons not to keep that: - Reputation: comments framing the OSS code as "the OSS path" vs "the enterprise path" pollute the public repo with implementation framing that shouldn't be there. - Implementation leakage: enterprise/cloud comments give away structural details about the closed-source plugin (where its data lives, what library it uses, which Linear tickets track it). Rewrites use neutral language — "the loaded RBAC plugin (if any)", "the default fallback", "an installed plugin" — and drop references to specific cloud-side files / TRI-IDs / CASL. Plan-tier names ("Enterprise" as a public product tier in the Roles page upsell, `planCode === "enterprise"` checks, `<TierEnterprise />` in pre-existing files) are intentionally left as-is — they're the public marketing name for a paid tier, not implementation detail. Removed `.server-changes/rbac-userrole-default-assignment.md` — documented a feature that was reverted in d2bf617 (upfront UserRole inserts on create-org / acceptInvite). Verified: 162/162 OSS e2e.full pass, 31/31 OSS rbac unit pass.
1 parent d2bf617 commit 71901dd

17 files changed

Lines changed: 111 additions & 121 deletions

File tree

.changeset/rbac-assignable-role-ids.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
"@trigger.dev/plugins": patch
33
---
44

5-
RBAC plugin: new `getAssignableRoleIds(organizationId)` method on `RoleBaseAccessController`. Returns the subset of `allRoles(organizationId)` IDs that may be assigned right now — used by the Teams page UI to disable role-dropdown options outside the org's plan tier. OSS fallback returns `[]` (permissive — `allRoles` already returns `[]` so there's nothing to gate); the enterprise plugin queries its plan client and returns the plan-allowed system roles plus all custom roles. Server-side enforcement (rejecting an actual `setUserRole` to a plan-gated role) is unchanged and remains the source of truth — this method is purely a UI affordance.
5+
RBAC plugin: new `getAssignableRoleIds(organizationId)` method on `RoleBaseAccessController`. Returns the subset of `allRoles(organizationId)` IDs that may be assigned right now — used by the Teams page UI to disable role-dropdown options that aren't currently assignable. The default fallback returns `[]` (permissive — `allRoles` already returns `[]` so there's nothing to gate); a plugin may apply its own gating policy and return the assignable subset. Server-side enforcement (rejecting an actual `setUserRole` to a non-assignable role) is unchanged and remains the source of truth — this method is purely a UI affordance.

.server-changes/rbac-pat-role-selection.md

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,12 @@ type: feature
44
---
55

66
RBAC: PAT creation flow now lets users pick a system role at create
7-
time, persisted as an enterprise.TokenRole row (TRI-8749). Defaults to
8-
the caller's own role so a PAT can't be more privileged than the
9-
person creating it. Custom (org-defined) roles are out of scope for
10-
v1 — only the four global system roles are offered, and the binding
11-
is global to the PAT regardless of which org the request later
12-
targets. Compensating-delete pattern on TokenRole insert failure
13-
keeps the two writes (Prisma PAT row + Drizzle TokenRole row)
14-
consistent without cross-ORM transaction wrestling. OSS path is a
15-
no-op: when the RBAC plugin isn't installed the dropdown is hidden,
16-
no roleId is submitted, and the PAT works exactly as before.
7+
time, persisted via the RBAC plugin's `setTokenRole`. Defaults to the
8+
caller's own role so a PAT can't be more privileged than the person
9+
creating it. Custom (org-defined) roles are out of scope for v1 — only
10+
the four global system roles are offered, and the binding is global to
11+
the PAT regardless of which org the request later targets. A
12+
compensating-delete on `setTokenRole` failure keeps the PAT row and
13+
the role row consistent without cross-store transaction wrestling.
14+
With no RBAC plugin installed the dropdown is hidden, no roleId is
15+
submitted, and the PAT works exactly as before.

.server-changes/rbac-userrole-default-assignment.md

Lines changed: 0 additions & 11 deletions
This file was deleted.

apps/webapp/app/models/member.server.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -215,8 +215,8 @@ export async function acceptInvite({
215215
};
216216
});
217217

218-
// No upfront RBAC UserRole insert — the enterprise plugin's
219-
// getUserRole derives the new member's role from the legacy
218+
// No upfront RBAC UserRole insert — the loaded RBAC plugin (if any)
219+
// is responsible for deriving the new member's role from the legacy
220220
// OrgMember.role write inside the transaction above (ADMIN → Owner,
221221
// MEMBER → Admin) until an Owner explicitly changes their role on
222222
// the Teams page. Keeps the invite path tight and consistent with

apps/webapp/app/models/organization.server.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -82,12 +82,12 @@ export async function createOrganization(
8282
},
8383
});
8484

85-
// No upfront RBAC UserRole insert — the enterprise plugin's
86-
// getUserRole derives the creator's role from the legacy
85+
// No upfront RBAC UserRole insert — the loaded RBAC plugin (if any)
86+
// is responsible for deriving the creator's role from the legacy
8787
// OrgMember.role = "ADMIN" write above (which maps to Owner) until an
8888
// Owner explicitly changes someone's role on the Teams page. Keeps
89-
// the create-org path tight and lets the fallback path stay the
90-
// single source of truth for "who has what role" by default.
89+
// the create-org path tight and lets the plugin stay the single
90+
// source of truth for "who has what role" by default.
9191
return { ...organization };
9292
}
9393

apps/webapp/app/presenters/TeamPresenter.server.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,9 @@ export class TeamPresenter extends BasePresenter {
1919
getLimit(organizationId, "teamMembers", 100_000_000),
2020
getCurrentPlan(organizationId),
2121
getPlans(),
22-
// RBAC role catalogue (system roles + any org-defined custom roles).
23-
// OSS fallback returns []; on cloud the enterprise plugin returns
24-
// the seeded system roles plus the org's custom roles.
22+
// RBAC role catalogue (system roles + any org-defined custom
23+
// roles). The default fallback returns []; an installed plugin
24+
// may return the seeded system roles plus any custom roles.
2525
rbac.allRoles(organizationId),
2626
// Plan-gated subset — the Teams page disables dropdown options not
2727
// in this set. Server-side enforcement is independent (setUserRole

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

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -221,10 +221,9 @@ function RoleCard({
221221
);
222222
}
223223

224-
// Permission name-prefix → display group. Mirrors PERMISSION_METADATA's
225-
// groupings server-side (cloud/enterprise/plugins/src/rbac/permissions.ts)
226-
// but lives client-side because the wire-format Permission only carries
227-
// `name` and `description`. Keep in lockstep.
224+
// Permission name-prefix → display group. Lives client-side because
225+
// the wire-format Permission only carries `name` and `description` —
226+
// the RBAC plugin doesn't ship grouping metadata over the wire.
228227
const PERMISSION_GROUP_BY_NAME: Record<string, string> = {
229228
"read:runs": "Runs",
230229
"write:runs": "Runs",
@@ -283,9 +282,8 @@ function groupPermissions(
283282
);
284283
}
285284

286-
// "Create role" upsell shown to non-Enterprise plans. Enterprise sees
287-
// nothing here for now — the actual create-role UI is a follow-up
288-
// (depends on TRI-8747's controller-level CRUD already in place).
285+
// "Create role" upsell shown to non-Enterprise plans. Enterprise plans
286+
// don't see this — the actual create-role UI is a follow-up.
289287
function CreateRoleUpsell() {
290288
const [open, setOpen] = useState(false);
291289
return (

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -581,9 +581,8 @@ function RolePicker({
581581
}) {
582582
const fetcher = useFetcher<{ ok: boolean; error?: string } | { ok: true }>();
583583
const assignable = new Set(assignableRoleIds);
584-
// OSS deployments return [] (allRoles also returns []) — when there
585-
// are no roles to pick from, render nothing rather than an empty
586-
// dropdown.
584+
// With no RBAC plugin installed, the loader returns no roles —
585+
// render nothing rather than an empty dropdown.
587586
if (roles.length === 0) return null;
588587

589588
const isSubmitting = fetcher.state === "submitting";

apps/webapp/app/routes/account.tokens/route.tsx

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,13 @@ export const meta: MetaFunction = () => {
5656
];
5757
};
5858

59-
// PATs aren't org-scoped, but role/permission catalogues are seeded
60-
// per-org in enterprise's allRoles. To get the canonical system roles
61-
// (Owner/Admin/Member/Viewer — orgId IS NULL on those rows), we hand
62-
// allRoles any orgId the user belongs to and filter down to system
59+
// PATs aren't org-scoped, but the RBAC plugin's allRoles is org-keyed
60+
// (a plugin may also expose org-defined custom roles alongside the
61+
// global system roles). To get the canonical system roles (Owner /
62+
// Admin / Member / Viewer), we hand allRoles any orgId the user
63+
// belongs to and filter down to the rows the plugin marks as system
6364
// roles. This is a UI-only convenience — the chosen role becomes a
64-
// global TokenRole row that applies wherever the PAT is used. Custom
65+
// global TokenRole that applies wherever the PAT is used. Custom
6566
// (org-defined) roles are out of scope for v1: their org-binding
6667
// semantics for a multi-org user's PAT need a separate design pass.
6768
async function loadSystemRolesForUser(userId: string) {
@@ -95,7 +96,8 @@ export const loader = async ({ request }: LoaderFunctionArgs) => {
9596
// Default the role picker to the user's own role in their primary
9697
// org so a freshly-created PAT isn't more privileged than the
9798
// person creating it. Falls back to Member if they don't have one
98-
// (new user, OSS path with no role assignments yet).
99+
// (new user, or no RBAC plugin installed so no role assignments
100+
// exist).
99101
const defaultRoleId = userRoleId ?? SYSTEM_ROLE_IDS.member;
100102

101103
return typedjson({
@@ -123,10 +125,9 @@ const CreateTokenSchema = z.discriminatedUnion("action", [
123125
.string({ required_error: "You must enter a name" })
124126
.min(2, "Your name must be at least 2 characters long")
125127
.max(50),
126-
// Optional — when the RBAC plugin isn't installed (OSS), the UI
127-
// hides the dropdown and submits no roleId; the action passes that
128-
// through and createPersonalAccessToken just doesn't write a
129-
// TokenRole row.
128+
// Optional — when no RBAC plugin is installed the UI hides the
129+
// dropdown and submits no roleId; the action passes that through
130+
// and createPersonalAccessToken just doesn't write a TokenRole.
130131
roleId: z.string().optional(),
131132
}),
132133
z.object({
@@ -284,10 +285,11 @@ function CreatePersonalAccessToken({
284285
? (lastSubmission?.payload?.token as CreatedPersonalAccessToken)
285286
: undefined;
286287

287-
// OSS path: rbac.allRoles returns []; we hide the dropdown entirely
288-
// rather than showing an empty Select. createPersonalAccessToken's
289-
// roleId is optional, so omitting it produces a working PAT with no
290-
// explicit role attached (matches pre-RBAC behaviour).
288+
// With no RBAC plugin installed, rbac.allRoles returns []; hide the
289+
// dropdown entirely rather than showing an empty Select.
290+
// createPersonalAccessToken's roleId is optional, so omitting it
291+
// produces a working PAT with no explicit role attached (matches
292+
// pre-RBAC behaviour).
291293
const showRolePicker = roles.length > 0;
292294
const [selectedRoleId, setSelectedRoleId] = useState(defaultRoleId);
293295

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,9 @@ const tokenValueLength = 40;
1111
//lowercase only, removed 0 and l to avoid confusion
1212
const tokenGenerator = customAlphabet("123456789abcdefghijkmnopqrstuvwxyz", tokenValueLength);
1313

14-
// The OSS fallback's setTokenRole returns this exact string when no
15-
// enterprise plugin is loaded. We treat that as "no role attached" —
16-
// the PAT is still valid; auth just falls through to legacy permissive
14+
// The default fallback's setTokenRole returns this exact string when
15+
// no RBAC plugin is loaded. We treat that as "no role attached" — the
16+
// PAT is still valid; auth just falls through to legacy permissive
1717
// behaviour. Any other error is treated as a real failure and triggers
1818
// the compensating delete below.
1919
const FALLBACK_NOT_INSTALLED_ERROR = "RBAC plugin not installed";
@@ -351,25 +351,25 @@ export async function createPersonalAccessToken({
351351
},
352352
});
353353

354-
// Persist the role choice in enterprise.TokenRole. This lives on a
355-
// different schema (Drizzle, not Prisma) — co-transactional inserts
356-
// across the two ORMs are awkward, so we use a compensating-delete
357-
// pattern: if setTokenRole fails, roll back the PAT row by deleting
358-
// it. The auth path treats "no role" as permissive (matches OSS
359-
// fallback) so a brief orphan window between the two writes is
360-
// harmless. The compensating delete narrows that window from "until
361-
// manual cleanup" to "until the request returns".
354+
// Persist the role choice via the RBAC plugin's setTokenRole. The
355+
// plugin may store this in a separate datastore from Prisma (e.g.
356+
// Drizzle on a different schema), so co-transactional inserts are
357+
// awkward — we use a compensating-delete pattern instead: if
358+
// setTokenRole fails, roll back the PAT row by deleting it. The auth
359+
// path treats "no role" as permissive (matches the default fallback)
360+
// so a brief orphan window between the two writes is harmless. The
361+
// compensating delete narrows that window from "until manual cleanup"
362+
// to "until the request returns".
362363
if (roleId) {
363364
const roleResult = await rbac.setTokenRole({
364365
tokenId: personalAccessToken.id,
365366
roleId,
366367
});
367368
if (!roleResult.ok) {
368-
// The OSS fallback always returns ok=false with this exact
369-
// message. That isn't a failure — there's no enterprise plugin
370-
// to write to, so the PAT just runs without an explicit role
371-
// (matches the pre-RBAC behaviour). Don't compensating-delete
372-
// in that case.
369+
// The default fallback always returns ok=false with this exact
370+
// message. That isn't a failure — there's no plugin to write to,
371+
// so the PAT just runs without an explicit role (matches the
372+
// pre-RBAC behaviour). Don't compensating-delete in that case.
373373
if (roleResult.error === FALLBACK_NOT_INSTALLED_ERROR) {
374374
logger.debug("createPersonalAccessToken: no RBAC plugin, skipping role assignment", {
375375
patId: personalAccessToken.id,

0 commit comments

Comments
 (0)