permissions: backend-driven permission catalog endpoint#1799
Conversation
Adds GET /permissions/available, derived from CEDAR_SCHEMA.RocketAdmin.actions so the catalog stays in sync with the Cedar action enum. Replaces the hardcoded POLICY_ACTION_GROUPS list in the frontend with a signal-backed httpResource on UsersService, and switches PolicyAction's needsTable/needsDashboard booleans to a single resource discriminator. New ActionEvent and Panel categories now appear in the Cedar policy editor. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds a dynamic permission catalog endpoint that serves available Cedar permissions enriched with display metadata, and refactors the frontend to fetch permissions from the service instead of using static constants. The backend builds permission groups with labels, icons, and optional resource scoping; the frontend consumes these via UsersService computed properties. ChangesPermission Catalog System
Sequence DiagramsequenceDiagram
participant Client
participant Controller as PermissionController
participant Builder as buildPermissionCatalog()
participant CedarSchema
participant Response as AvailablePermissionsResponseDs
Client->>Controller: GET /permissions/available
Controller->>Builder: buildPermissionCatalog()
Builder->>CedarSchema: Read RocketAdmin.actions
CedarSchema-->>Builder: All Cedar actions
Builder->>Builder: Group by prefix, enrich metadata, add wildcards
Builder-->>Controller: Array<AvailablePermissionGroupDs>
Controller->>Response: Wrap groups
Controller-->>Client: AvailablePermissionsResponseDs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
backend/src/entities/permission/permission-catalog.builder.ts (1)
7-12: ⚡ Quick winUse interfaces for object-shape declarations.
ActionMetadataOverrideandSchemaShapeare object shapes declared astype; guideline requiresinterfacefor object shapes.Suggested fix
-type ActionMetadataOverride = { +interface ActionMetadataOverride { label?: string; shortLabel?: string; icon?: string; resourceOverride?: 'none' | string; -}; +} -type SchemaShape = { +interface SchemaShape { RocketAdmin: { actions: Record<string, { appliesTo: { principalTypes: Array<string>; resourceTypes: Array<string> } }>; }; -}; +}As per coding guidelines, "Use interfaces for object shapes and type for unions and primitives".
Also applies to: 177-181
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/src/entities/permission/permission-catalog.builder.ts` around lines 7 - 12, Change the object-shape declarations from type aliases to interfaces: replace the `type ActionMetadataOverride = { ... }` declaration with `interface ActionMetadataOverride { ... }` and likewise convert the `SchemaShape` type alias to an `interface SchemaShape { ... }`; keep the same property names and optional markers (label, shortLabel, icon, resourceOverride, etc.) and preserve exported/visibility modifiers and any JSDoc so all usages of ActionMetadataOverride and SchemaShape continue to work.backend/test/ava-tests/non-saas-tests/non-saas-permission-catalog-e2e.test.ts (1)
76-79: ⚡ Quick winAssert
panel:*wildcard to lock the full wildcard contract.The catalog builder includes a Panel wildcard path; adding this assertion prevents silent regressions for panel permissions.
Suggested fix
t.true(values.has('*'), 'catalog must include the General * wildcard'); t.true(values.has('table:*'), 'catalog must include the table:* wildcard'); t.true(values.has('dashboard:*'), 'catalog must include the dashboard:* wildcard'); +t.true(values.has('panel:*'), 'catalog must include the panel:* wildcard');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/test/ava-tests/non-saas-tests/non-saas-permission-catalog-e2e.test.ts` around lines 76 - 79, The test currently checks that the catalog includes '*' , 'table:*' and 'dashboard:*' but misses the panel wildcard; update the test in non-saas-permission-catalog-e2e.test.ts to also assert that values.has('panel:*') is true by adding an assertion using the existing t.true(values.has('panel:*'), 'catalog must include the panel:* wildcard') so the catalog builder's panel wildcard is covered and prevents regressions (look for the variable values and the nearby t.true assertions to place this check).frontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.ts (1)
67-68: ⚡ Quick winAlign private signal fields with frontend naming/modifier rules.
Use underscore-prefixed private members and mark computed signal fields as readonly.
✏️ Suggested cleanup
- private availableActions = computed(() => this._users.availablePermissions()); - private availableGroups = computed(() => this._users.availablePermissionGroups()); + private readonly _availableActions = computed(() => this._users.availablePermissions()); + private readonly _availableGroups = computed(() => this._users.availablePermissionGroups());As per coding guidelines,
frontend/**/*.ts: "Prefix private members with underscore (e.g.,_privateMethod,_dataService)" andfrontend/**/*.component.ts: "Signals for component state must be declared asprotectedorprivate readonlyand initialized withsignal()orcomputed()".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.ts` around lines 67 - 68, The signals availableActions and availableGroups must follow component naming/modifier rules: rename them to private readonly _availableActions and private readonly _availableGroups and update their usages accordingly; keep the computed initialization but change the declarations from "private" to "private readonly" and prefix the identifiers with an underscore so computed(() => this._users.availablePermissions()) and computed(() => this._users.availablePermissionGroups()) are assigned to _availableActions and _availableGroups respectively.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/src/entities/permission/permission-catalog.builder.ts`:
- Around line 52-53: The code currently filters emitted groups to only those in
GROUP_ORDER, causing any new/unknown Cedar action groups to be dropped; update
the grouping logic (where GROUP_ORDER is used and where groups are emitted —
e.g., the code that builds groupedActions/resultGroups around lines referencing
GROUP_ORDER) so that after adding groups in GROUP_ORDER order you append any
remaining group keys from the actual grouping map (preserve their original names
and either original detection order or sorted lexicographically) rather than
omitting them; ensure the same change is applied to the other occurrence noted
(lines 83–86) so unknown groups are included in API output.
- Around line 62-175: Convert the standalone function declarations into const
arrow function expressions while preserving their names, signatures and return
types: replace appendAction, buildAction, buildWildcardAllAction,
buildPrefixWildcard, deriveResource, resolveGroupName, autoLabel,
autoShortLabel, and capitalize with const <name> = (<params>): <returnType> => {
... } forms; ensure existing type annotations (e.g., AvailablePermissionDs,
Array<string>, Map types) remain intact and behavior is unchanged (including use
of throw/return and side effects like map mutation in appendAction).
In
`@backend/test/ava-tests/non-saas-tests/non-saas-permission-catalog-e2e.test.ts`:
- Line 41: Remove the explicit call to app.getHttpServer().listen(0) in the test
setup; supertest directly uses app.getHttpServer(), so delete the listen
invocation (reference: the call to app.getHttpServer().listen(0)) to avoid
redundant server startup and teardown flakiness and let tests use the provided
server instance without awaiting or manually listening.
In
`@frontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.ts`:
- Around line 281-283: _actionResource currently returns undefined when
_findAction(value) is missing, causing downstream flags
(needsTable/needsDashboard) to be false; modify _actionResource to return a
deterministic fallback resource when _findAction(value) is undefined by deriving
a resource object from the action string (for example parse the value to infer
resource type and id or return a stable default resource shape such as { type:
inferredType, id: inferredId } ), so code that relies on
PolicyAction['resource'] (and the needsTable/needsDashboard logic) always
receives a predictable resource; reference the _actionResource and _findAction
symbols and ensure the fallback logic covers missing catalog/availability states
instead of returning undefined.
---
Nitpick comments:
In `@backend/src/entities/permission/permission-catalog.builder.ts`:
- Around line 7-12: Change the object-shape declarations from type aliases to
interfaces: replace the `type ActionMetadataOverride = { ... }` declaration with
`interface ActionMetadataOverride { ... }` and likewise convert the
`SchemaShape` type alias to an `interface SchemaShape { ... }`; keep the same
property names and optional markers (label, shortLabel, icon, resourceOverride,
etc.) and preserve exported/visibility modifiers and any JSDoc so all usages of
ActionMetadataOverride and SchemaShape continue to work.
In
`@backend/test/ava-tests/non-saas-tests/non-saas-permission-catalog-e2e.test.ts`:
- Around line 76-79: The test currently checks that the catalog includes '*' ,
'table:*' and 'dashboard:*' but misses the panel wildcard; update the test in
non-saas-permission-catalog-e2e.test.ts to also assert that
values.has('panel:*') is true by adding an assertion using the existing
t.true(values.has('panel:*'), 'catalog must include the panel:* wildcard') so
the catalog builder's panel wildcard is covered and prevents regressions (look
for the variable values and the nearby t.true assertions to place this check).
In
`@frontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.ts`:
- Around line 67-68: The signals availableActions and availableGroups must
follow component naming/modifier rules: rename them to private readonly
_availableActions and private readonly _availableGroups and update their usages
accordingly; keep the computed initialization but change the declarations from
"private" to "private readonly" and prefix the identifiers with an underscore so
computed(() => this._users.availablePermissions()) and computed(() =>
this._users.availablePermissionGroups()) are assigned to _availableActions and
_availableGroups respectively.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 605fe83c-45ad-45e0-810e-39fdda36adca
📒 Files selected for processing (9)
backend/src/entities/permission/application/data-structures/available-permissions.ds.tsbackend/src/entities/permission/permission-catalog.builder.tsbackend/src/entities/permission/permission.controller.tsbackend/src/entities/permission/permission.module.tsbackend/test/ava-tests/non-saas-tests/non-saas-permission-catalog-e2e.test.tsfrontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.spec.tsfrontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.tsfrontend/src/app/lib/cedar-policy-items.tsfrontend/src/app/services/users.service.ts
| const GROUP_ORDER = ['General', 'Connection', 'Group', 'Table', 'ActionEvent', 'Dashboard', 'Panel']; | ||
|
|
There was a problem hiding this comment.
Don't drop unknown Cedar action groups from the response.
The current return path only emits groups listed in GROUP_ORDER. If Cedar adds a new action prefix, actions get grouped but then omitted from API output, which breaks the “schema-driven sync” goal.
Suggested fix
- return GROUP_ORDER.filter((name) => grouped.has(name)).map((name) => ({
- group: name,
- actions: grouped.get(name)!,
- }));
+ const orderedKnownGroups = GROUP_ORDER.filter((name) => grouped.has(name));
+ const unknownGroups = Array.from(grouped.keys()).filter((name) => !GROUP_ORDER.includes(name)).sort();
+ const orderedGroups = [...orderedKnownGroups, ...unknownGroups];
+
+ return orderedGroups.map((name) => ({
+ group: name,
+ actions: grouped.get(name)!,
+ }));Also applies to: 83-86
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/entities/permission/permission-catalog.builder.ts` around lines
52 - 53, The code currently filters emitted groups to only those in GROUP_ORDER,
causing any new/unknown Cedar action groups to be dropped; update the grouping
logic (where GROUP_ORDER is used and where groups are emitted — e.g., the code
that builds groupedActions/resultGroups around lines referencing GROUP_ORDER) so
that after adding groups in GROUP_ORDER order you append any remaining group
keys from the actual grouping map (preserve their original names and either
original detection order or sorted lexicographically) rather than omitting them;
ensure the same change is applied to the other occurrence noted (lines 83–86) so
unknown groups are included in API output.
| export function buildPermissionCatalog(): Array<AvailablePermissionGroupDs> { | ||
| const schemaActions = (CEDAR_SCHEMA as SchemaShape).RocketAdmin.actions; | ||
| const grouped = new Map<string, Array<AvailablePermissionDs>>(); | ||
|
|
||
| for (const [value, definition] of Object.entries(schemaActions)) { | ||
| const action = buildAction(value, definition.appliesTo.resourceTypes); | ||
| const groupName = resolveGroupName(value); | ||
| appendAction(grouped, groupName, action); | ||
| } | ||
|
|
||
| appendAction(grouped, 'General', buildWildcardAllAction()); | ||
|
|
||
| for (const groupName of WILDCARD_GROUPS) { | ||
| const actions = grouped.get(groupName); | ||
| if (actions && actions.length > 1) { | ||
| const prefix = groupName.toLowerCase(); | ||
| const resource = actions.find((a) => a.resource)?.resource; | ||
| actions.unshift(buildPrefixWildcard(prefix, groupName, resource)); | ||
| } | ||
| } | ||
|
|
||
| return GROUP_ORDER.filter((name) => grouped.has(name)).map((name) => ({ | ||
| group: name, | ||
| actions: grouped.get(name)!, | ||
| })); | ||
| } | ||
|
|
||
| function appendAction( | ||
| target: Map<string, Array<AvailablePermissionDs>>, | ||
| groupName: string, | ||
| action: AvailablePermissionDs, | ||
| ): void { | ||
| const list = target.get(groupName); | ||
| if (list) { | ||
| list.push(action); | ||
| } else { | ||
| target.set(groupName, [action]); | ||
| } | ||
| } | ||
|
|
||
| function buildAction(value: string, resourceTypes: Array<string>): AvailablePermissionDs { | ||
| const meta = ACTION_DISPLAY_METADATA[value] ?? {}; | ||
| const derivedResource = deriveResource(resourceTypes); | ||
| const resource = | ||
| meta.resourceOverride === NONE_RESOURCE_OVERRIDE ? undefined : (meta.resourceOverride ?? derivedResource); | ||
|
|
||
| const action: AvailablePermissionDs = { | ||
| value, | ||
| label: meta.label ?? autoLabel(value), | ||
| shortLabel: meta.shortLabel ?? autoShortLabel(value), | ||
| icon: meta.icon ?? 'help_outline', | ||
| }; | ||
| if (resource) { | ||
| action.resource = resource; | ||
| } | ||
| return action; | ||
| } | ||
|
|
||
| function buildWildcardAllAction(): AvailablePermissionDs { | ||
| return { | ||
| value: '*', | ||
| label: 'Full access (all permissions)', | ||
| shortLabel: 'Full access', | ||
| icon: 'shield', | ||
| }; | ||
| } | ||
|
|
||
| function buildPrefixWildcard(prefix: string, groupName: string, resource: string | undefined): AvailablePermissionDs { | ||
| const labels = WILDCARD_LABELS[groupName] ?? { label: `Full ${prefix} access`, shortLabel: 'Full access' }; | ||
| const action: AvailablePermissionDs = { | ||
| value: `${prefix}:*`, | ||
| label: labels.label, | ||
| shortLabel: labels.shortLabel, | ||
| icon: 'shield', | ||
| }; | ||
| if (resource) { | ||
| action.resource = resource; | ||
| } | ||
| return action; | ||
| } | ||
|
|
||
| function deriveResource(resourceTypes: Array<string>): string | undefined { | ||
| for (const type of resourceTypes) { | ||
| const candidate = type.charAt(0).toLowerCase() + type.slice(1); | ||
| if (UI_RENDERABLE_RESOURCES.has(candidate)) { | ||
| return candidate; | ||
| } | ||
| } | ||
| return undefined; | ||
| } | ||
|
|
||
| function resolveGroupName(actionValue: string): string { | ||
| const [prefix] = actionValue.split(':'); | ||
| return PREFIX_TO_GROUP[prefix] ?? capitalize(prefix); | ||
| } | ||
|
|
||
| function autoLabel(value: string): string { | ||
| const [prefix, verb = ''] = value.split(':'); | ||
| const groupName = PREFIX_TO_GROUP[prefix] ?? capitalize(prefix); | ||
| const verbWords = verb.split('-').map(capitalize).join(' '); | ||
| return verbWords ? `${groupName} ${verbWords.toLowerCase()}` : groupName; | ||
| } | ||
|
|
||
| function autoShortLabel(value: string): string { | ||
| const verb = value.split(':')[1] ?? value; | ||
| return verb | ||
| .split('-') | ||
| .map((part, index) => (index === 0 ? capitalize(part) : part)) | ||
| .join(' '); | ||
| } | ||
|
|
||
| function capitalize(text: string): string { | ||
| return text ? text.charAt(0).toUpperCase() + text.slice(1) : text; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify function declarations in this file
rg -nP '^\s*function\s+\w+\s*\(' backend/src/entities/permission/permission-catalog.builder.tsRepository: rocket-admin/rocketadmin
Length of output: 665
Use arrow functions for helper declarations in backend/src/entities/permission/permission-catalog.builder.ts.
This file still defines multiple helpers with function declarations (appendAction, buildAction, buildWildcardAllAction, buildPrefixWildcard, deriveResource, resolveGroupName, autoLabel, autoShortLabel, capitalize); convert them to const ... = (...) => {} to match the TS style guideline.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/src/entities/permission/permission-catalog.builder.ts` around lines
62 - 175, Convert the standalone function declarations into const arrow function
expressions while preserving their names, signatures and return types: replace
appendAction, buildAction, buildWildcardAllAction, buildPrefixWildcard,
deriveResource, resolveGroupName, autoLabel, autoShortLabel, and capitalize with
const <name> = (<params>): <returnType> => { ... } forms; ensure existing type
annotations (e.g., AvailablePermissionDs, Array<string>, Map types) remain
intact and behavior is unchanged (including use of throw/return and side effects
like map mutation in appendAction).
| }), | ||
| ); | ||
| await app.init(); | ||
| app.getHttpServer().listen(0); |
There was a problem hiding this comment.
Remove the explicit server listen in this E2E setup.
supertest uses app.getHttpServer() directly; listen(0) here is redundant and may create teardown flakiness because it isn’t awaited.
Suggested fix
- app.getHttpServer().listen(0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| app.getHttpServer().listen(0); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/test/ava-tests/non-saas-tests/non-saas-permission-catalog-e2e.test.ts`
at line 41, Remove the explicit call to app.getHttpServer().listen(0) in the
test setup; supertest directly uses app.getHttpServer(), so delete the listen
invocation (reference: the call to app.getHttpServer().listen(0)) to avoid
redundant server startup and teardown flakiness and let tests use the provided
server instance without awaiting or manually listening.
| private _actionResource(value: string): PolicyAction['resource'] | undefined { | ||
| return this._findAction(value)?.resource; | ||
| } |
There was a problem hiding this comment.
Add a deterministic fallback when action metadata is missing.
At Line 282, _actionResource relies only on catalog lookup. If an action is not present in availablePermissions (loading/error/stale catalog), needsTable / needsDashboard become false and save paths can emit policies without required tableName/dashboardId.
💡 Proposed fix
private _actionResource(value: string): PolicyAction['resource'] | undefined {
- return this._findAction(value)?.resource;
+ const resource = this._findAction(value)?.resource;
+ if (resource != null) return resource;
+ if (value.startsWith('table:')) return 'table';
+ if (value.startsWith('dashboard:')) return 'dashboard';
+ return undefined;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@frontend/src/app/components/users/cedar-policy-list/cedar-policy-list.component.ts`
around lines 281 - 283, _actionResource currently returns undefined when
_findAction(value) is missing, causing downstream flags
(needsTable/needsDashboard) to be false; modify _actionResource to return a
deterministic fallback resource when _findAction(value) is undefined by deriving
a resource object from the action string (for example parse the value to infer
resource type and id or return a stable default resource shape such as { type:
inferredType, id: inferredId } ), so code that relies on
PolicyAction['resource'] (and the needsTable/needsDashboard logic) always
receives a predictable resource; reference the _actionResource and _findAction
symbols and ensure the fallback logic covers missing catalog/availability states
instead of returning undefined.
Drop labels, short labels, icons, and synthesized wildcards from the backend
response. Each action is now { value, resource } only, where resource is the
first appliesTo.resourceTypes entry lowercased. Wildcards (*, table:*, etc.)
and display copy move to the frontend: a new lib/permission-display.ts derives
labels/icons algorithmically, and CedarPolicyListComponent synthesizes the
General and per-prefix wildcard entries at render time.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Backend now returns { actions: [...] } instead of pre-grouped categories.
Group names ('Connection', 'ActionEvent', etc.) and group ordering live in
permission-display.ts on the frontend, where groupNameForAction(value)
derives them from the action prefix. UsersService computes
availablePermissionGroups from the flat list at read time.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds GET /permissions/available, derived from CEDAR_SCHEMA.RocketAdmin.actions so the catalog stays in sync with the Cedar action enum. Replaces the hardcoded POLICY_ACTION_GROUPS list in the frontend with a signal-backed httpResource on UsersService, and switches PolicyAction's needsTable/needsDashboard booleans to a single resource discriminator. New ActionEvent and Panel categories now appear in the Cedar policy editor.
Summary by CodeRabbit
New Features
GET /permissions/availableendpoint that returns available permissions organized by groups with metadata including labels, short labels, icons, and optional resource scopes.Refactor