feat: granular permission system (groups, overrides, per-entity hiding)#3605
feat: granular permission system (groups, overrides, per-entity hiding)#3605zurdi15 wants to merge 10 commits into
Conversation
…issions - Introduced a new permission model with `PermissionGroup`, `UserPermissionOverride`, and `HiddenEntity` to manage access control. - Added `DBPermissionsHandler` for handling permission-related database operations. - Updated `User` model to include a foreign key to `PermissionGroup` and modified `oauth_scopes` to derive from the new permission model. - Implemented tests to ensure the new permission model maintains parity with legacy access controls. - Created documentation outlining the new permission system architecture and migration strategy.
…mission groups - Added new permission handling in `backend/handler/auth/dependencies.py` to support fine-grained, DB-backed permission checks. - Enhanced user role update logic in `backend/endpoints/user.py` to prevent demotion of the last admin. - Introduced `hidden_platform_ids` and `hidden_rom_ids` parameters in various database handlers to manage visibility based on admin settings. - Created new endpoints for managing permission groups, user memberships, and hidden entities in `backend/tests/endpoints/test_permissions_admin.py`. - Added tests for permissions visibility and CRUD operations in `backend/tests/endpoints/test_permissions_visibility.py` and `backend/tests/endpoints/test_permissions_me.py`. - Updated archive handling in `backend/utils/archives.py` to improve error logging and timeout management during extraction.
…e new permission models
Collapse the Role enum from viewer/editor/admin to two kinds (admin/user);
non-admin access now comes entirely from permission groups + overrides.
Backend:
- Role -> StrEnum {USER, ADMIN}, VARCHAR-backed (native_enum=False); Role.coerce
maps legacy/unknown strings (incl. in-flight invites) to USER.
- Resolver: group-less users fall back to the default group (dropped the
role-based legacy fallback); kiosk caps all non-admins to read-only.
- OIDC editor/viewer claims both map to USER (env var names unchanged).
- Migration 0092 converts the native enum to VARCHAR and normalizes
ADMIN->admin, VIEWER/EDITOR->user.
- Updated endpoints, conftest fixtures, role/oidc/parity/db-handler tests, tools.
Frontend:
- v2 + v1 user management now use admin/user (dropdowns, defaults, getRoleIcon).
- Regenerated types (Role = 'user' | 'admin'); added role-user i18n to all locales.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Test Results (mariadb) 1 files 1 suites 2m 39s ⏱️ Results for commit 7d45795. ♻️ This comment has been updated with latest results. |
Test Results (postgresql) 1 files 1 suites 2m 33s ⏱️ Results for commit 7d45795. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
This PR replaces the legacy role-based authorization model with a DB-backed, request-resolved permission system (groups + per-user overrides + per-entity hiding), while projecting effective permissions back into coarse OAuth scopes for backward compatibility across existing clients/UIs.
Changes:
- Introduces backend models, resolver/projection, enforcement helpers, and endpoints for granular permissions (including hide/visibility filtering and socket-driven refresh).
- Updates v2 admin UI to manage permission groups, per-user access (admin vs user + group + overrides), and hidden platforms/games; removes v2’s hardcoded role map.
- Regenerates OpenAPI-derived frontend types and updates v1/v2 role selectors + translations to the narrowed
admin/userroles.
Reviewed changes
Copilot reviewed 84 out of 106 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/views/Settings/UserProfile.vue | Updates v1 role picker items to admin/user. |
| frontend/src/v2/views/Settings/UserProfile.vue | Updates v2 role items/tone mapping for admin/user. |
| frontend/src/v2/views/Settings/Administration.vue | Adds “Permission Groups” settings tab and dialogs. |
| frontend/src/v2/utils/groupColor.ts | Adds group color palette + fallback resolver for UI pills/dots. |
| frontend/src/v2/lib/forms/RCheckbox/types.ts | Introduces multi-state checkbox state typing. |
| frontend/src/v2/lib/forms/RCheckbox/RCheckbox.stories.ts | Adds Storybook coverage for multi-state checkbox behavior. |
| frontend/src/v2/lib/forms/RCheckbox/index.ts | Re-exports RCheckboxState type. |
| frontend/src/v2/lib/data/RTable/RTable.vue | Adds one-time row entrance animation + stagger variable. |
| frontend/src/v2/composables/useCan/role-map.ts | Removes legacy hardcoded role→actions mapping. |
| frontend/src/v2/composables/useCan/index.ts | Hydrates permissions from /permissions/me + socket refresh; admin short-circuit. |
| frontend/src/v2/composables/useCan/actions.ts | Switches ActionKey to OpenAPI-generated type; documents scope modeling. |
| frontend/src/v2/components/Settings/UsersSection.vue | Displays access via admin/group pill; loads groups for labels/colors; adds access sorting. |
| frontend/src/v2/components/Settings/PermissionsMatrix.vue | Adds tri-state group grants matrix with per-column select-all. |
| frontend/src/v2/components/Settings/PermissionGroupsSection.vue | Adds admin table for permission groups with edit/delete actions. |
| frontend/src/v2/components/Settings/OverridesMatrix.vue | Adds per-user override matrix (inherit/grant/grant-own/revoke) with select-all. |
| frontend/src/v2/components/Settings/InviteLinkDialog.vue | Updates invite role options to admin/user. |
| frontend/src/v2/components/Settings/HiddenPlatformsPicker.vue | Adds multi-select platform picker for hide lists. |
| frontend/src/v2/components/Settings/HiddenGamesPicker.vue | Adds debounced ROM search + selectable hidden ROM list UI. |
| frontend/src/v2/components/Settings/CreateUserDialog.vue | Replaces role select with admin toggle + initial group assignment. |
| frontend/src/utils/index.ts | Adds icon mapping for user role while keeping legacy fallbacks. |
| frontend/src/types/emitter.d.ts | Adds showGroupFormDialog event typing for v2 settings UI. |
| frontend/src/stores/permissions.ts | Reworks permissions store to hydrate from /permissions/me with admin/hidden state. |
| frontend/src/stores/permissionGroups.ts | Adds shared Pinia store for permission groups list/default. |
| frontend/src/services/api/permissions.ts | Adds permissions API client (catalog, groups CRUD, user assignment, hiding, /me). |
| frontend/src/locales/zh_TW/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/zh_CN/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/tr_TR/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/ru_RU/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/ro_RO/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/pt_BR/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/pl_PL/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/ko_KR/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/ja_JP/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/it_IT/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/hu_HU/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/fr_FR/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/es_ES/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/en_US/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/en_GB/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/de_DE/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/cs_CZ/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/locales/bg_BG/settings.json | Adds new permission system UI strings (and role-user). |
| frontend/src/components/Settings/Administration/Users/Dialog/InviteLink.vue | Updates v1 invite role options to admin/user. |
| frontend/src/components/Settings/Administration/Users/Dialog/EditUser.vue | Updates v1 edit-user role options to admin/user. |
| frontend/src/components/Settings/Administration/Users/Dialog/CreateUser.vue | Defaults v1 create-user role to user and updates role options. |
| frontend/src/generated/models/UserSchema.ts | Adds permission_group_id field to generated user schema. |
| frontend/src/generated/models/UserPermissionsUpdate.ts | Adds generated schema for updating user permissions. |
| frontend/src/generated/models/UserPermissionsSchema.ts | Adds generated schema for reading user permissions (admin view). |
| frontend/src/generated/models/Role.ts | Narrows generated Role union to user/admin. |
| frontend/src/generated/models/PermissionsResponse.ts | Adds generated /permissions/me response schema. |
| frontend/src/generated/models/PermissionScopeSchema.ts | Adds generated permission scope schema. |
| frontend/src/generated/models/PermissionGroupUpdate.ts | Adds generated schema for updating permission groups. |
| frontend/src/generated/models/PermissionGroupSchema.ts | Adds generated schema for permission group details. |
| frontend/src/generated/models/PermissionGroupCreate.ts | Adds generated schema for creating permission groups. |
| frontend/src/generated/models/PermissionCatalogSchema.ts | Adds generated schema for permission vocabulary catalog. |
| frontend/src/generated/models/PermEntity.ts | Adds generated granular permission entity vocabulary. |
| frontend/src/generated/models/PermAction.ts | Adds generated granular permission action vocabulary. |
| frontend/src/generated/models/OverrideSchemaIO.ts | Adds generated schema for per-user permission overrides. |
| frontend/src/generated/models/HiddenEntitySchema.ts | Adds generated schema for hidden entities. |
| frontend/src/generated/models/HiddenEntityCreate.ts | Adds generated schema for creating hidden entities. |
| frontend/src/generated/models/HiddenEntitiesSchema.ts | Adds generated schema for hidden platforms/roms lists. |
| frontend/src/generated/models/GrantSchemaIO.ts | Adds generated schema for group grant cells (IO). |
| frontend/src/generated/models/GrantSchema.ts | Adds generated schema for effective grants in /permissions/me. |
| frontend/src/generated/models/Body_remove_hidden_entity_api_permissions_hidden_delete.ts | Adds generated request body schema for hidden entity deletion. |
| frontend/src/generated/models/ActionKey.ts | Adds generated UI-facing ActionKey enum from OpenAPI. |
| frontend/src/generated/index.ts | Re-exports new generated types. |
| frontend/.storybook/preview.ts | Seeds permissions store via /permissions/me-shaped payload for stories. |
| backend/utils/archives.py | Adjusts 7z extraction timeout handling to avoid per-member log spam. |
| backend/tools/generate_test_data.py | Updates seeded users to admin/user roles. |
| backend/tests/utils/test_archives.py | Adds unit tests for new 7z streaming/timeout behavior. |
| backend/tests/handler/test_db_handler.py | Updates user role expectations for narrowed roles. |
| backend/tests/handler/auth/test_permissions_resolver.py | Adds resolver/projection tests for groups, overrides, hiding, kiosk, admin bypass. |
| backend/tests/handler/auth/test_permissions_parity.py | Adds parity tests proving access preservation vs legacy roles and seeded migration rows. |
| backend/tests/handler/auth/test_permissions_dependencies.py | Adds unit tests for new fine-grained enforcement helpers. |
| backend/tests/handler/auth/test_oidc.py | Updates OIDC tests for collapsed user kind and admin mapping. |
| backend/tests/endpoints/test_permissions_visibility.py | Adds end-to-end tests for hide/visibility and delete enforcement wiring. |
| backend/tests/endpoints/test_permissions_me.py | Adds endpoint tests for /permissions/me action vocabulary + hidden IDs. |
| backend/tests/endpoints/test_permissions_admin.py | Adds admin CRUD tests for permissions API and effective behavior. |
| backend/tests/endpoints/test_identity.py | Updates identity endpoint tests for new roles. |
| backend/tests/endpoints/test_client_tokens.py | Updates demotion test to demote admin→user. |
| backend/tests/conftest.py | Updates fixtures to assign legacy editor/viewer access via groups under Role.USER. |
| backend/models/user.py | Narrows Role, adds permission_group_id, projects oauth scopes from granular resolver. |
| backend/models/permission.py | Introduces permission group, grants, overrides, and hidden entity models. |
| backend/main.py | Registers new permissions router. |
| backend/handler/database/roms_handler.py | Adds hidden-platform/rom filtering to ROM queries. |
| backend/handler/database/platforms_handler.py | Adds hidden-platform filtering to platform queries. |
| backend/handler/database/firmware_handler.py | Adds platform-based hide filtering to firmware queries. |
| backend/handler/database/init.py | Exposes db_permission_handler. |
| backend/handler/auth/permissions.py | Implements per-request permission resolution and scope projection. |
| backend/handler/auth/permissions_map.py | Adds pure mapping from granular grants to legacy OAuth scopes + legacy matrices. |
| backend/handler/auth/dependencies.py | Adds fine-grained request-cached enforcement helpers (assert_can, etc.). |
| backend/handler/auth/base_handler.py | Collapses OIDC editor/viewer roles to user, preserves admin mapping. |
| backend/endpoints/user.py | Coerces legacy role strings; prevents last-admin demotion; emits permissions-changed on role change. |
| backend/endpoints/roms/init.py | Applies hidden filtering + 404 masking; enforces delete via fine-grained permission. |
| backend/endpoints/responses/permission.py | Adds /permissions/me response schemas + ActionKey vocabulary and admin CRUD schemas. |
| backend/endpoints/responses/identity.py | Adds permission_group_id to user response schema. |
| backend/endpoints/platform.py | Applies hidden filtering + 404 masking; enforces delete via fine-grained permission. |
| backend/endpoints/firmware.py | Applies hidden filtering + 404 masking; enforces delete via fine-grained permission. |
| backend/alembic/versions/0092_narrow_role_to_admin_user.py | Migrates users.role to VARCHAR and normalizes values to admin/user. |
Files not reviewed (21)
- frontend/src/generated/index.ts: Generated file
- frontend/src/generated/models/ActionKey.ts: Generated file
- frontend/src/generated/models/Body_remove_hidden_entity_api_permissions_hidden_delete.ts: Generated file
- frontend/src/generated/models/GrantSchema.ts: Generated file
- frontend/src/generated/models/GrantSchemaIO.ts: Generated file
- frontend/src/generated/models/HiddenEntitiesSchema.ts: Generated file
- frontend/src/generated/models/HiddenEntityCreate.ts: Generated file
- frontend/src/generated/models/HiddenEntitySchema.ts: Generated file
- frontend/src/generated/models/OverrideSchemaIO.ts: Generated file
- frontend/src/generated/models/PermAction.ts: Generated file
- frontend/src/generated/models/PermEntity.ts: Generated file
- frontend/src/generated/models/PermissionCatalogSchema.ts: Generated file
- frontend/src/generated/models/PermissionGroupCreate.ts: Generated file
- frontend/src/generated/models/PermissionGroupSchema.ts: Generated file
- frontend/src/generated/models/PermissionGroupUpdate.ts: Generated file
- frontend/src/generated/models/PermissionScopeSchema.ts: Generated file
- frontend/src/generated/models/PermissionsResponse.ts: Generated file
- frontend/src/generated/models/Role.ts: Generated file
- frontend/src/generated/models/UserPermissionsSchema.ts: Generated file
- frontend/src/generated/models/UserPermissionsUpdate.ts: Generated file
- frontend/src/generated/models/UserSchema.ts: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code review: granular permission systemReviewed adversarially across four dimensions (auth core/resolver, migrations, endpoint enforcement + query visibility, frontend). The architecture is sound and the parity story (legacy groups reproducing old roles, projection feeding the coarse layer) holds up well. There is, however, one CI blocker and a cluster of visibility "fail-open" gaps that should be closed before this ships, since opt-out hiding is a core promise of the feature. Items marked (verified) I reproduced or confirmed against the code/CI directly. Items marked (reported) come from the review pass and are worth a maintainer look but I did not individually reproduce. Blocker (CI is red)1. Backend
High (security: hidden entities leak / privilege escalation)2. Default-group hides fail open for users with no explicit group. (verified) 3. 4. Granting the Medium (secondary read paths not visibility-filtered; one parity change)These all leak the existence/metadata/counts of hidden platforms or ROMs to a user they are hidden from (content downloads remain 404-masked, but ids, names, and counts leak). (all reported)
OIDC Low
Nits
Investigated and refuted (so nobody chases these)
Solid / verified OK
Nice work overall. The blocker is a one-line fix; the High visibility gaps are the main thing to close before merge given hiding is a headline feature. Review assisted by Claude Code (Claude Opus): four parallel adversarial review agents plus manual verification of the blocker, both High visibility findings, the escalation chain, and the Postgres-migration refutation. |
Resolves the CI blocker and a cluster of opt-out visibility "fail-open" gaps surfaced in review of the granular permission system. Security / correctness: - admin oauth_scopes projection keeps canonical FULL_SCOPES order (order_scopes) instead of sorting alphabetically, fixing the red test_user.py::test_admin on MariaDB + Postgres. - default-group hides no longer fail open: the resolver resolves the effective (own-or-default) group before the hidden-entity lookup. - /roms/by-hash and /roms/by-metadata-provider now 404-mask hidden roms. - USERS-entity grant no longer enables admin creation: add_user and invite-link require a real admin to mint admin accounts. Visibility leaks closed on secondary read paths: - feeds, sibling roms (list query + single-rom schemas), /stats counts and per-platform breakdowns, collection rom_ids/rom_count, search_rom. Hardening / cleanups: - firmware/platform PUT 404-mask hidden entities; group rename conflict returns 400 not 500; guard against removing the last default group; kiosk read-only enforced at the fine layer; add_hidden_entity rejects non-cascading entity types. Frontend: - permissionGroups.ensureLoaded coalesces concurrent callers on one in-flight request; permissions.setGrants resets isAdmin/hidden; CreateUserDialog no longer orphans a user when group assignment fails; HiddenGamesPicker search rows are native buttons (keyboard/gamepad); invite-role labels and group swatch aria-label use i18n; drop dead code (originalRole, unused permissionsApi export). AI assistance: changes authored with Claude Code (Claude Opus), driven by the Copilot review and a multi-agent adversarial review, then verified (backend pytest, frontend typecheck/vitest, i18n parity, trunk). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
☂️ Python Coverage
Overall Coverage
New Files
Modified Files
|
gantoine
left a comment
gantoine
left a comment
There was a problem hiding this comment.
Reviewed the authorization core in depth — resolver, scope projection, enforcement helpers, per-entity visibility/404-masking, the admin CRUD, and the migrations. (I left the v2 admin UI mostly aside; backend is the source of truth.) This is a well-architected rework and the core is sound; the issues are visibility-coverage gaps on a few endpoints plus one Postgres migration concern.
Verified correct
- Resolver is fail-closed (
handler/auth/permissions.py): no group / no default group / unknown role / missing overrides all resolve to an empty grant set → everyallows()returns False. Admin bypass is exact (user.role == Role.ADMIN) and a normalusernever constructsis_admin=True. - Override precedence is right: explicit revoke
pops the group grant, explicit grant adds. (One sharp edge: a per-user grant override silently replaces the group'sown_onlybecause override identity is(entity, action)withoutown_only; both directions are admin-initiated and the narrowing direction fails closed, so not exploitable — just a granularity limitation worth a comment.) - Scope projection is a strict subset (
permissions_map.py):DELETEprojects to no coarse scope, so delete is double-gated (coarse*_WRITEnecessary + authoritativeassert_can(DELETE)). The legacy parity matrices match the0091seed grants exactly. - Permission-admin CRUD is admin-only: every mutating route in
endpoints/permissions.pycallsassert_admin(request)in its body, so the coarse[Scope.USERS_WRITE]decorator can't be abused by a non-admin who's merely been grantedUSERSwrite.assert_admincorrectly 403s unlessperms.is_admin. - Read-path masking is broad and consistent on the endpoints this PR touched: list (
get_romsthreadshidden_*into the query; sidecars/counts/rom_id_indexderive from the filtered query; sidecar cache key is per-user), single-entity GETs/HEAD/content (roms, platform, firmware),download_roms, allfeeds.pyvariants,collections(incl. fixingrom_count),search, andstatscounts via_exclude_hidden. Delete paths combineassert_can(DELETE)+can_see_*. Kiosk mode locks non-admins to read-only at both the scope and fine layers.
Should fix — visibility-coverage gaps
These mutation endpoints fetch by id and 404 only on non-existence, with no can_see_* mask, unlike their siblings (delete_roms, update_platform) — so a hidden entity is editable and its existence is confirmed:
update_rom(PUT/roms/{id},endpoints/roms/__init__.py:1316-1319) — medium: an editor can edit metadata/cover of a ROM hidden from them. Mirrordelete_roms:if not rom or not perms.can_see_rom(rom.id, rom.platform_id): raise RomNotFoundInDatabaseException(id).update_rom_user(PUT/roms/{id}/props,:1828-1831) — low/medium: writes the caller's own RomUser row + confirms existence of a hidden ROM.add_firmware(endpoints/firmware.py:48-52) — low: upload firmware to a hidden platform (platform-hide should cascade to firmware).
Should fix — gaps in pre-existing endpoints the new model doesn't cover
These aren't in this PR's diff (so no inline comment), but the hidden-entity feature is incomplete without them — flagging because they defeat the feature's purpose:
patch_rom(endpoints/roms/patch.py, gated only[Scope.ROMS_READ]) — high: looks up aRomFileby id and streams the patched ROM bytes back with nocan_see_romcheck. Any user can download the content of a hidden ROM by file id. This is the only path that leaks actual file bytes; worth resolving in this PR since it's the feature's headline guarantee. Fix: resolverom_file.romand 404 unlesscan_see_rom(rom.id, rom.platform_id)(and the same for the patch file).get_all_activity/get_rom_activity(endpoints/activity.py) — medium: returnrom_name/rom_cover_path/platform_*for live sessions across all users with no hidden filter, leaking hidden ROM/platform metadata.
Migration — 0092 is not Postgres-safe
alembic/versions/0092_narrow_role_to_admin_user.py converts role from a native Enum to VARCHAR with no postgresql_using and never drops the orphaned PG role type. The repo's own migrations show both required patterns (0010_igdb_id_integerr.py uses postgresql_using="...::text"; 0026/0033/0075 do ENUM(name=...).drop(connection)). Consequences on Postgres: the upgrade likely fails the enum→varchar cast without postgresql_using="role::text" (and migrations auto-apply on startup, so that would block boot), and downgrade()'s sa.Enum(name="role") recreation collides with the lingering type ("type role already exists"). The PR notes verification on MariaDB only — please test 0092 up+down on a real Postgres instance and mirror the existing precedent. (Chain itself is a single linear head; only the dialect handling is the issue. Minor: two files share the 0091_ filename prefix — harmless but confusing.)
Net: the design and the bulk of enforcement are solid. The can_see_* masking just needs to be applied uniformly across every entity read/write path (the three in-diff endpoints above, plus patch_rom/activity), and 0092 needs Postgres dialect handling, before this is safe to merge.
Generated by Claude Code
…ation
Visibility-coverage gaps (404-mask hidden entities, mirroring the existing
delete/read paths):
- update_rom (PUT /roms/{id}) and update_rom_user (PUT /roms/{id}/props)
- add_firmware: platform-hide now cascades to firmware uploads
- patch_rom: resolve the parent rom of both the base and patch files and
404 when hidden, so a hidden rom's bytes can no longer be streamed back
- activity feeds (get_all_activity / get_rom_activity): drop sessions whose
rom is hidden from the caller
Migration: make the role enum -> varchar narrowing Postgres-safe. The cast
now uses postgresql_using, the orphaned native role type is dropped on
upgrade, and downgrade recreates it explicitly (create_type=False) before
re-typing the column. Verified up/down/up on Postgres 16 and MariaDB.
Also collapses the two permission migrations into a single 0092 and notes
the override own_only replacement granularity limit in the resolver.
AI assistance: implemented with Claude Code (review-fix pass).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@gantoine thanks for the thorough review. All points addressed in Visibility-coverage gaps (in-diff)
Pre-existing endpoints
Migration Postgres-safetyThe role enum -> VARCHAR narrowing is now Postgres-safe:
Verified locally on a real Postgres 16 with an up -> down -> up cycle: column ends as The two migrations are also collapsed into a single Added tests for the newly-masked AI assistance: these fixes were implemented with Claude Code. |
| - **Backend**: `pytest` on resolver (precedence, ownership, own_only), projection-parity, migration up/down + backfill assertions, visibility filtering (list + count + 404-mask), last-admin guard, OIDC/invite mapping, kiosk read-only. | ||
| - **The parity test is the linchpin**: for a DB seeded with admin/editor/viewer users, assert each user's projected `oauth_scopes` equals their pre-migration scopes exactly — this proves the upgrade changes nobody's access. | ||
| - **Frontend** (CLAUDE.md §VII): `npm run typecheck`, `npm run lint`, `npm run test`, `npm run generate` after backend API changes. Browser-test `uiVersion=v2`: a `user` with restricted group sees gated controls hidden (`v-if`), a hidden platform absent from their gallery; admin sees everything; live update when an admin changes their permissions (socket). Both themes, four modalities. | ||
| - **Manual upgrade test**: run the migration against a populated dev DB; confirm existing editors/viewers retain identical capabilities (including delete) and visibility. |
There was a problem hiding this comment.
we don't store these typically
| # Derived from the granular permission model (groups + overrides), | ||
| # projected onto the legacy coarse scopes. Resolved per access via its | ||
| # own session; admins short-circuit without touching the DB. | ||
| from handler.auth.permissions import compute_oauth_scopes |
There was a problem hiding this comment.
can be imported at the top of the file?
| entity: Mapped[PermEntity] = mapped_column(_str_enum(PermEntity, 20)) | ||
| action: Mapped[PermAction] = mapped_column(_str_enum(PermAction, 10)) | ||
| granted: Mapped[bool] = mapped_column(Boolean) | ||
| own_only: Mapped[bool] = mapped_column(Boolean, default=False) |
There was a problem hiding this comment.
why an override instead of directly editing the user's matrix?
| if not request.user.is_authenticated: | ||
| return siblings | ||
|
|
||
| from handler.auth.dependencies import get_permissions |
| if request.user.is_authenticated and not get_permissions(request).can_see_rom( | ||
| rom.id, rom.platform_id | ||
| ): | ||
| raise RomNotFoundInDatabaseException(id) |
There was a problem hiding this comment.
this is repeated a lot, should probably be a decorator
| visible = set(s.rom_ids) - hidden | ||
| if len(visible) != len(s.rom_ids): | ||
| s.rom_ids = visible | ||
| s.rom_count = len(visible) |
There was a problem hiding this comment.
dunno how i feel about this pattern, but let's merge and get the AI to refactor later if needed!
| raise HTTPException( | ||
| status_code=status.HTTP_404_NOT_FOUND, | ||
| detail=f"Firmware with ID {id} not found", | ||
| ) |
Summary
Replaces RomM's role + fixed-JWT-scopes authorization with a DB-backed, per-request model:
admin(bypasses everything) anduser(granular).User.oauth_scopesbecomes a coarse projection of the new model, so the existing scope-based enforcement chain, client tokens, OAuth/token, kiosk, and the legacy v1 UI keep working. Upgrade is access-preserving: seeded "Viewer (legacy)" / "Editor (legacy)" groups reproduce each old role exactly (including delete, which previously gated on*_WRITE).What's included
Backend
permission_groups,permission_group_grants,user_permission_overrides,hidden_entities(single consolidated migration0091_permission_system).handler/auth/permissions.py), enforcement helpers (assert_can,require_permission, visibility filtering + 404-masking)./permissions/me, admin CRUD under/api/permissions(catalog, groups, user assignment/overrides, hidden entities),permissions:changedsocket broadcast,ActionKeyexposed in OpenAPI.admin/user(migration0092): VARCHAR-backed enum,Role.coercefor legacy strings, group-default fallback for group-less users, OIDC editor/viewer claims map touser.Frontend
permissionGroupsPinia store.RCheckboxgains a generic multi-state mode (used by the permission matrices);RTablegains a subtle row entrance animation + per-column select-all.admin/user. Regenerated types; i18n added across all 18 locales.Migration / deploy notes
0092converts the nativeroleenum to VARCHAR and normalizesADMIN→admin,VIEWER/EDITOR→user(verified on MariaDB).Verification
tests/endpoints+tests/handlersuite green on MariaDB (incl. parity, resolver, OIDC, visibility, admin CRUD). Migration conversion probe-tested.vue-tsctypecheck, Vitest + Storybook play tests, Trunk lint, and i18n parity all pass; browser-verified in light + dark themes.AI assistance disclosure
This PR was developed with substantial assistance from an AI agent (Claude Code / Claude Opus). The AI did the bulk of the implementation, test updates, and verification under human direction and review. Per
CONTRIBUTING.md, disclosing this is required.🤖 Generated with Claude Code