[chore] Clean up RBAC and tighten API key and data export access#4076
[chore] Clean up RBAC and tighten API key and data export access#4076
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR standardizes RBAC role naming across the API, web app, and test tooling (moving to the canonical set owner/admin/manager/evaluator/auditor) and tightens API Keys UI visibility/editing based on project permissions.
Changes:
- Introduce a reusable frontend permission hook and apply it to settings routing/sidebar + API Keys page gating.
- Rename legacy role strings to canonical names across backend surfaces (defaults, routers/services) and add role-migration helpers/migrations.
- Update test tooling/markers and add design docs for the RBAC cleanup plan and verification.
Reviewed changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| web/oss/src/pages/w/[workspace_id]/p/[project_id]/settings/index.tsx | Prevents navigating to API Keys tab without view_api_keys permission and aligns breadcrumbs with resolved tab. |
| web/oss/src/hooks/useWorkspacePermissions.ts | Simplifies workspace permission checks to use the new project-permission helper. |
| web/oss/src/hooks/useProjectPermissions.ts | Adds a frontend permission/role helper backed by current member permissions. |
| web/oss/src/components/pages/settings/WorkspaceManage/Modals/InviteUsersModal.tsx | Updates invite default role to admin. |
| web/oss/src/components/pages/settings/APIKeys/APIKeys.tsx | Hides API Keys UI for users without view/edit permissions; conditions list/create/delete accordingly. |
| web/oss/src/components/Sidebar/SettingsSidebar.tsx | Hides API Keys settings nav item and guards invalid requested tabs. |
| sdk/run-tests.py | Updates CLI --role dimension to canonical roles. |
| sdk/pytest.ini | Replaces role markers to match canonical roles. |
| sdk/oss/tests/legacy/new_tests/vault_router/test_vault_secrets_apis.py | Updates “viewer” role references to “auditor” in tests. |
| sdk/oss/tests/legacy/new_tests/conftest.py | Updates programmatic user role setup to new role names. |
| docs/design/rbac-role-cleanup/sql.md | Adds SQL queries for validating/migrating RBAC role cleanup. |
| docs/design/rbac-role-cleanup/research.md | Documents current RBAC/role state and API key permission implications. |
| docs/design/rbac-role-cleanup/proposal.md | Proposes canonical role model and API key tightening strategy. |
| docs/design/rbac-role-cleanup/plan.md | Execution plan/checklist for the role cleanup and migrations. |
| docs/design/rbac-role-cleanup/gap.md | Gap analysis between current and desired role/permission state. |
| docs/design/rbac-role-cleanup/context.md | Context and clarified objectives/non-goals for the cleanup. |
| api/run-tests.py | Updates CLI --role dimension to canonical roles. |
| api/pytest.ini | Replaces role markers to match canonical roles. |
| api/oss/tests/legacy/vault_router/test_vault_secrets_apis.py | Updates “viewer” role references to “auditor” in tests. |
| api/oss/tests/legacy/conftest.py | Updates programmatic user role setup to new role names. |
| api/oss/src/services/organization_service.py | Updates default invitation role from editor to admin. |
| api/oss/src/services/admin_manager.py | Updates accepted role literals to canonical roles. |
| api/oss/src/routers/workspace_router.py | Updates OSS role list output and replaces WORKSPACE_ADMIN checks with ADMIN. |
| api/oss/src/routers/projects_router.py | Renames OSS non-owner role from editor to admin. |
| api/oss/src/routers/organization_router.py | Updates organization roles output to admin. |
| api/oss/src/core/auth/service.py | Updates auto-provisioned membership default role from editor to admin. |
| api/oss/databases/postgres/migrations/core/versions/b1c2d3e4f5a6_migrate_roles_to_canonical_names.py | Adds OSS migration to rewrite invitation role strings to canonical names. |
| api/oss/databases/postgres/migrations/core/data_migrations/roles.py | Adds shared role-migration helpers including API key cleanup for EE migration path. |
| api/ee/src/services/workspace_manager.py | Updates EE invite default role from editor to admin. |
| api/ee/src/services/db_manager_ee.py | Updates workspace admin resolution logic to use ADMIN. |
| api/ee/src/services/commoners.py | Updates demo role default from viewer to auditor. |
| api/ee/src/services/admin_manager.py | Updates accepted role literals to canonical roles. |
| api/ee/src/models/shared_models.py | Replaces legacy roles with canonical roles and tightens API key permissions per role. |
| api/ee/src/models/db_models.py | Updates membership default role from viewer to auditor. |
| api/ee/databases/postgres/migrations/core/versions/b1c2d3e4f5a6_migrate_roles_to_canonical_names.py | Adds EE migration to rewrite membership role strings + delete disallowed API keys. |
| api/ee/databases/postgres/migrations/core/data_migrations/demos.py | Updates demo role constant to auditor. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
api/oss/databases/postgres/migrations/core/data_migrations/roles.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 46 out of 46 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...databases/postgres/migrations/core/versions/b1c2d3e4f5a6_migrate_roles_to_canonical_names.py
Show resolved
Hide resolved
...databases/postgres/migrations/core/versions/b1c2d3e4f5a6_migrate_roles_to_canonical_names.py
Show resolved
Hide resolved
Railway Preview Environment
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 51 out of 51 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| viewer -> auditor | ||
|
|
||
| Revision ID: b1c2d3e4f5a6 | ||
| Revises: fd77265d65dc |
There was a problem hiding this comment.
The migration header docstring says it revises fd77265d65dc, but down_revision is e5f6a1b2c3d4. Please update the docstring so it matches the actual Alembic dependency chain (helps avoid confusion when troubleshooting migrations).
| Revises: fd77265d65dc | |
| Revises: e5f6a1b2c3d4 |
| are not permitted to hold API keys. | ||
|
|
||
| Revision ID: b1c2d3e4f5a6 | ||
| Revises: fd77265d65dc |
There was a problem hiding this comment.
The migration header docstring says it revises fd77265d65dc, but down_revision is e5f6a1b2c3d4. Please update the docstring to match the actual Alembic down_revision so the migration history is self-consistent.
| Revises: fd77265d65dc | |
| Revises: e5f6a1b2c3d4 |
| OWNER = CANONICAL_WORKSPACE_ROLES[0] | ||
| ADMIN = CANONICAL_WORKSPACE_ROLES[1] | ||
| MANAGER = CANONICAL_WORKSPACE_ROLES[2] | ||
| EVALUATOR = CANONICAL_WORKSPACE_ROLES[3] | ||
| AUDITOR = CANONICAL_WORKSPACE_ROLES[4] |
There was a problem hiding this comment.
WorkspaceRole enum values are pulled from CANONICAL_WORKSPACE_ROLES by numeric index. This makes the enum fragile (a tuple reorder would silently change role values) and harder to grep. Prefer assigning explicit string literals (or named constants) for each enum member instead of relying on positional indexing.
| OWNER = CANONICAL_WORKSPACE_ROLES[0] | |
| ADMIN = CANONICAL_WORKSPACE_ROLES[1] | |
| MANAGER = CANONICAL_WORKSPACE_ROLES[2] | |
| EVALUATOR = CANONICAL_WORKSPACE_ROLES[3] | |
| AUDITOR = CANONICAL_WORKSPACE_ROLES[4] | |
| OWNER = "owner" | |
| ADMIN = "admin" | |
| MANAGER = "manager" | |
| EVALUATOR = "evaluator" | |
| AUDITOR = "auditor" |
Summary
This PR cleans up the RBAC role model across the API, database, frontend, and test tooling, and standardizes on the canonical role set:
owneradminmanagerevaluatorauditorIt also tightens API key access so only
owner,admin, andmanagercan view/manage API keys, and hides data export affordances for the same roleset in the web UI.What Changed
evaluatorandauditorno longer have API key access, whilemanagernow does.editor->adminworkspace_admin->admindeployment_manager->managerviewer->auditorNotes