feat(frontend): create RBAC API keys from apikeys page#2332
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMakes API key listing and creation RBAC-aware: adds RBAC types and fetchers, computed display helpers, RBAC-driven add-key modal (org role + optional per-app bindings), table role column and RBAC-based scope display, two i18n keys, and DialogV2 size variants. ChangesRBAC-Aware API Key Management
Sequence Diagram — high-level fetch flowsequenceDiagram
participant ApiKeys as ApiKeys.vue
participant Backend as backend.apikey
participant RoleSvc as RoleBindingsService
participant AppsSvc as AppsService
ApiKeys->>Backend: getKeys()
Backend-->>ApiKeys: apikeys
ApiKeys->>RoleSvc: fetchAllBindings(apikey principals)
ApiKeys->>RoleSvc: fetchRoles()
RoleSvc-->>ApiKeys: role_bindings, roles
ApiKeys->>AppsSvc: loadAllApps(owner_org scoped)
AppsSvc-->>ApiKeys: apps (primed caches)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
Merging this PR will not alter performance
Comparing 
Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/pages/ApiKeys.vue`:
- Around line 349-363: The title attribute is built from unescaped org names in
displayFunction (uses getHighestOrgRole, getRolesByOrg, getRoleDisplayName and
tooltipText) which allows injection if a name contains quotes; fix by escaping
attribute values or, better, by creating the span via DOM APIs and assigning the
tooltip via element.title (or setAttribute('title', ...)) and using textContent
for the visible label so values are not interpolated into HTML strings—ensure
tooltipText is properly escaped when you must build HTML strings.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: b09456b0-f518-4ee6-bf4c-5198995f5b11
📒 Files selected for processing (2)
messages/en.jsonsrc/pages/ApiKeys.vue
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@messages/en.json`:
- Line 1740: The i18n key "select-multiple-organizations-help" is unused and
misleading for the checkbox-based org selector; either remove this key from
messages/en.json, or update and wire it into the UI: modify ApiKeys.vue to
display a localized help string (use $t("select-multiple-organizations-help") or
equivalent) near the checkbox list and update the message text to describe
checkbox toggling (e.g., "Click checkboxes to select organizations") and ensure
toggleOrgSelection(...) remains the change handler for each checkbox; choose one
approach and remove the dead key if you opt to wire nothing.
In `@src/pages/ApiKeys.vue`:
- Around line 1084-1095: The dropdown trigger button lacks an aria-expanded
attribute; update the button element that toggles showOrgDropdown (the one using
`@click`="showOrgDropdown = !showOrgDropdown" and displaying
selectedOrgNamesForCreation/selectedOrgsForCreation) to include a bound
aria-expanded reflecting the open state (e.g., :aria-expanded="showOrgDropdown")
so assistive tech can read the dropdown state.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f5ccc0ee-2cfa-41d1-962e-3679be94bc6b
📒 Files selected for processing (4)
messages/en.jsonsrc/components/DialogV2.vuesrc/pages/ApiKeys.vuesrc/stores/dialogv2.ts
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/pages/ApiKeys.vue (1)
124-140:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep the table role summary org-scoped.
This helper now ranks every binding, so an app-level role can replace the org role shown in the new
rolecolumn. A key withorg_memberplus one high-rank app binding will render the app role here, which is misleading at table level.Suggested fix
function getHighestRole(key: Database['public']['Tables']['apikeys']['Row']): string | null { - const keyBindings = getBindingsForKey(key) + const keyBindings = getBindingsForKey(key) + .filter(binding => binding.scope_type === 'org') if (keyBindings.length === 0) return null🤖 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 `@src/pages/ApiKeys.vue` around lines 124 - 140, The current getHighestRole function considers all bindings (including app-level) when selecting the top role, causing app roles to override org roles in the table; update getHighestRole (and where it calls getBindingsForKey) to first filter keyBindings to only org-scoped bindings (e.g., binding.scope === 'org' or binding.org_id matching the org context) and then perform the existing priority_rank-based selection over that filtered list; if no org-scoped bindings remain, return null so the table-level role column only reflects org-scoped roles.
🤖 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 `@src/pages/ApiKeys.vue`:
- Around line 170-172: The formatDisplayApps function incorrectly deduplicates
apps by display name; update it to preserve the full list returned by
getDisplayAppIds instead of converting mapped names into a Set so different apps
with the same name and multiple unresolved apps remain distinct. Specifically,
in formatDisplayApps use getDisplayAppIds(key).map(appId =>
appCache.value.get(appId) || 'Unknown') and join the resulting array with ', '
(do not use Set), keeping reference to the function name formatDisplayApps and
the appCache and getDisplayAppIds helpers when making the change.
- Around line 149-156: getDisplayOrgIds currently only adds org ids from
bindings where scope_type === 'org', so keys that only have app-scoped bindings
miss their app's organization; modify getDisplayOrgIds to also extract and add
the org id from app-scoped bindings returned by getBindingsForKey(key) (e.g.
check b.org_id || b.app?.org_id or equivalent field on the binding/app object),
keep using the Set to dedupe, and return Array.from(orgIds) so app-binding orgs
are included in the organizations summary.
---
Outside diff comments:
In `@src/pages/ApiKeys.vue`:
- Around line 124-140: The current getHighestRole function considers all
bindings (including app-level) when selecting the top role, causing app roles to
override org roles in the table; update getHighestRole (and where it calls
getBindingsForKey) to first filter keyBindings to only org-scoped bindings
(e.g., binding.scope === 'org' or binding.org_id matching the org context) and
then perform the existing priority_rank-based selection over that filtered list;
if no org-scoped bindings remain, return null so the table-level role column
only reflects org-scoped roles.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 2149345a-f259-4728-a029-ea74a2382210
📒 Files selected for processing (1)
src/pages/ApiKeys.vue
BataraSurya
left a comment
BataraSurya
left a comment
There was a problem hiding this comment.
I think the new global create-key modal is missing the same anti-escalation filtering that the org-scoped RBAC API-key editor already applies.\n\nIn src/pages/settings/organization/ApiKeys.[id].vue, orgRoleOptions is filtered by the caller's current org-role priority (
.priority_rank <= callerOrgPriorityRank.value), so the UI only offers roles that the caller can actually grant. Here, src/pages/ApiKeys.vue builds orgRoleOptions from all assignable org roles except the explicit unsupported ones. For a user who can manage API keys in multiple orgs but has a lower-ranked role in at least one selected org, the modal can offer a role choice that createRoleBindingForPrincipal will reject on the backend. Because this create call is atomic across all selected orgs, that turns into a full create failure after the user submits, rather than a disabled/hidden role option up front.\n\nCould this compute the allowed org-role choices from the selected organizations, probably using the minimum caller priority across selectedOrgsForCreation, and filter orgRoleOptions the same way the org-scoped editor does? If a selected org makes the current role invalid, the modal should also clear or downgrade selectedOrgRole before submit.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
playwright/e2e/apikeys.spec.ts (1)
19-41:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftExpand test coverage to verify RBAC-specific behavior per PR objectives.
The current tests only verify basic creation and deletion flows. According to the PR objectives test plan, the following RBAC-specific behaviors should also be verified:
- RBAC roles are displayed in the /apikeys table for migrated keys
- Creating a key from /apikeys selects all organizations by default with the org_member role
- App-level roles can be configured across selected organizations
Consider adding test scenarios to cover these customer-facing RBAC flows. As per coding guidelines, Playwright e2e tests should cover customer-facing flows and be run locally with
bun run test:frontbefore shipping UI changes.🧪 Suggested additional test scenarios
test('should display RBAC role for newly created key', async ({ page }) => { const keyName = `Playwright RBAC ${Date.now()}` await createRbacApiKey(page, keyName) const keyRow = page.locator('tr', { hasText: keyName }) // Verify the role column shows the expected default role (e.g., org_member) await expect(keyRow.locator('[data-role-cell]')).toContainText('org_member') }) test('should select all organizations by default when creating key', async ({ page }) => { await page.click('[data-test="create-key"]') // Verify all organizations are selected by default in the modal const orgCheckboxes = page.locator('`#dialog-v2-content` input[type="checkbox"][data-org-select]') const count = await orgCheckboxes.count() for (let i = 0; i < count; i++) { await expect(orgCheckboxes.nth(i)).toBeChecked() } })Note: Adjust selectors to match actual data-test attributes in your implementation.
🤖 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 `@playwright/e2e/apikeys.spec.ts` around lines 19 - 41, The tests only cover basic create/delete flows; add RBAC-focused assertions to apikeys.spec.ts to validate migrated keys show roles, that the create-key modal selects all orgs by default with org_member, and that app-level roles can be configured: after using createRbacApiKey (or invoking the create flow via the create-key button), assert the key row (page.locator('tr', { hasText: keyName })) contains the role cell (e.g., locate via '[data-role-cell]') showing 'org_member'; open the create modal (click '[data-test="create-key"]'), locate organization checkboxes (e.g., '`#dialog-v2-content` input[type="checkbox"][data-org-select]') and assert each is checked by default; add an additional test that opens the app-level role controls in the modal and verifies selecting/deselecting roles applies across selected organizations.
🤖 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.
Outside diff comments:
In `@playwright/e2e/apikeys.spec.ts`:
- Around line 19-41: The tests only cover basic create/delete flows; add
RBAC-focused assertions to apikeys.spec.ts to validate migrated keys show roles,
that the create-key modal selects all orgs by default with org_member, and that
app-level roles can be configured: after using createRbacApiKey (or invoking the
create flow via the create-key button), assert the key row (page.locator('tr', {
hasText: keyName })) contains the role cell (e.g., locate via
'[data-role-cell]') showing 'org_member'; open the create modal (click
'[data-test="create-key"]'), locate organization checkboxes (e.g.,
'`#dialog-v2-content` input[type="checkbox"][data-org-select]') and assert each is
checked by default; add an additional test that opens the app-level role
controls in the modal and verifies selecting/deselecting roles applies across
selected organizations.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7f909dd2-5ab4-412c-97cb-6547df13ed9b
📒 Files selected for processing (3)
messages/en.jsonplaywright/e2e/apikeys.spec.tssrc/pages/ApiKeys.vue
💤 Files with no reviewable changes (1)
- messages/en.json
|
2 participants
Summary (AI generated)
Motivation (AI generated)
Martin is migrating legacy API keys to RBAC API keys, so the global API keys page needs to support the RBAC creation model instead of the old read/upload/write/all mode flow.
Business Impact (AI generated)
This keeps API key management aligned with the RBAC migration and lets users create organization-scoped and app-scoped keys from the central API keys page without falling back to legacy permissions.
Test Plan (AI generated)
bunx eslint src/pages/ApiKeys.vue src/components/DialogV2.vue src/stores/dialogv2.ts playwright/e2e/apikeys.spec.tsbun typecheckbunx playwright test playwright/e2e/apikeys.spec.tsbun test:front; observed 18/19 passing locally, with the remaining failure caused by login schema-cache retry before reaching /apikeys.Generated with AI
Summary by CodeRabbit
New Features
User Interface Changes
Localization
Tests