14 KiB
QA procedure: roles_permissions_enforcement
Package: test (black-box). Name new fakes uniquely, for example enforcementRoleRepo, enforcementUserRepo.
Do not copy the old Role == checks into new code.
Deferred to later slices
- 2b: own/all enforcement after loading an article or entry, publish separate from update, Draft behaviour. Articles and entries stay ungated here.
- 3: /v1/role CRUD, no-escalation, ValidateGrants at write time, per-type override keys.
- 4: role change, invite with a role, transfer ownership, removal of the legacy role column.
- SPA (sidebar, read-only pages, 403 toasts) is out of scope. It reacts to the status code only.
Design notes for the user
- docs/design/screens/roles.md "Owner-only items" disables View roles and Manage roles for an admin. This slice follows the user's decision (Admin has 29 keys). The design needs a fix by the user.
- docs/design/change-list.md item 21 ("Sites: owner only, hidden for other roles") and its SEO "Permissions" paragraph ("only the owner edits site SEO defaults", 403
owner_only), plus theowner_onlyclause in routes.md "SEO", disagree with this slice. Admin hassettings.manage_sites, so an admin can now edit sites and their SEO defaults. - roles.md is inconsistent about Writer and Schema. The "Writer: Articles list" state hides Schema from the sidebar, while the catalogue, the built-in table and change-list ("Schema read-only for view-only roles") give Writer
schema.view. - change-list line 100 mentions a "restricted
userrole" that does not exist. - The docs name
GET /v1/me. The implemented route isGET /v1/auth/me(resolved in slice 1). - change-list item 15 and routes.md word activity export as "owner and admin"; it is now
activity.exportplusactivity.view. - change-list item 22 says GET /v1/author works for any role; under 2a a role without
authors.viewgets 403. - The Home activity card may call activity summary, so a Writer would get 403 there; check before the SPA slice.
Inventory (route, current gate, new key, Owner / Admin / Writer)
Routes gated today by RequireOwner or RequireOwnerOrAdmin:
| Route | New key (scope) | Owner / Admin / Writer | Change |
|---|---|---|---|
| POST /v1/user/invite, DELETE /v1/user/invite/{id}, POST /v1/user/invite/{id}/renew | users.invite | ok / ok / 403 | Admin gains |
| PATCH /v1/settings | system.edit_retention | ok / 403 / 403 | none |
| POST /v1/site, PATCH /v1/site/{id}, PATCH /v1/site/{id}/default | settings.manage_sites | ok / ok / 403 | Admin gains |
| DELETE /v1/site/{id} | settings.delete_site | ok / 403 / 403 | none |
| POST /v1/author | authors.create | ok / ok / 403 | none |
| DELETE /v1/author/{id} | authors.delete | ok / ok / 403 | none |
| POST, PUT /v1/content-type | schema.edit | ok / ok / 403 | none |
| DELETE /v1/content-type/{key} | schema.delete | ok / ok / 403 | none |
| GET /v1/builtin-type, /{key} | schema.view | ok / ok / ok | Writer gains |
| GET /v1/activity/summary | activity.view at all | ok / ok / 403 | none |
| GET /v1/activity/export | activity.export plus activity.view (same scoping as the list) | ok / ok / 403 | none |
| POST /v1/system/update/check, /download, /restart | system.apply_updates | ok / 403 / 403 | none |
| GET /v1/media/purge-settings | settings.view | ok / ok / 403 | none |
| PATCH /v1/media/purge-settings | settings.edit_general | ok / ok / 403 | none |
| GET /v1/media/purge-preview, POST /v1/media/purge | media.delete at all | ok / ok / 403 | none |
| GET /v1/media/{id}/variants (after RejectAPIKey) | media.view | ok / ok / ok | Writer gains |
| POST, DELETE /v1/media/{id}/variants... (after RejectAPIKey) | settings.edit_general | ok / ok / 403 | none |
| GET /v1/image-variants | media.view | ok / ok / ok | Writer gains |
| GET /v1/image-variants/cache | settings.view | ok / ok / 403 | none |
| other image-variants mutations and cache purge | settings.edit_general | ok / ok / 403 | none |
Gated by a legacy Role == inside a use case or handler:
| Place | New |
|---|---|
| ListActivity.scopeToViewer, activityHandler.writeActivityError | activity.view: all unfiltered, own forced to self (another actor gives 403 missing_permission), none gives 403 |
| ListAPIKeys | apikeys.view: all lists everything with created_by_name, own only the caller's keys without it, none 403 |
| RevokeAPIKey | apikeys.revoke: all any key, own only the caller's (404 API key not found on another's), none 403 |
| ListAuthors (CallerRole, isAdminRole) | gate authors.view; user_email only if the caller holds users.view |
| UpdateAuthor / AuthorCaller.isAdmin | authors.edit: all any author and the link, own the linked author only (else author_rights), none 403 missing_permission |
| updateHandler.isOwner | can_apply true only with system.apply_updates (and helper installed, as today) |
| middleware.RequireOwner, RequireOwnerOrAdmin | deleted; replaced by RequirePermission(key) and a scope-aware variant resolving the live role |
Previously ungated or verification-only:
| Route | New |
|---|---|
| GET /v1/user | users.view |
| GET /v1/settings | settings.view |
| GET /v1/system/update | settings.view |
| GET /v1/author | authors.view |
| GET /v1/content-type, /{key} | schema.view |
| GET /v1/media, /{id}, /{id}/usage | media.view |
| POST /v1/media | media.upload |
| PATCH /v1/media/{id} | media.upload |
| DELETE /v1/media/{id}, POST /v1/media/merge | media.delete at all (own fails closed) |
| POST /v1/api-key | apikeys.create (RequireSession stays) |
| GET /v1/api-key, DELETE /v1/api-key/{id} | apikeys.view, apikeys.revoke (scoped) |
| GET /v1/activity | activity.view (scoped) |
| GET /v1/site, GET /v1/user/lookup, PATCH /v1/user/me, POST /v1/author/me, GET /v1/permission, GET /v1/auth/me | ungated |
| /v1/article/, /v1/content-type/{key}/entry/, public reads | ungated in 2a |
Test infrastructure first
enforcementRoleRepoimplements the roleRepositoryin memory, seeded with the three built-ins through the same Go function the migration uses. It supports editing grants and a switchable failure.- The user fake carries
RoleID. Add users 3 (Owner), 5 (Admin), 9 (Writer), 12 (unverified Admin), 13 (role_id 99), 14-19 (legacy and role_id combinations from the Outline), 20 (custom role). - A helper
mintCookie(userID, tokenVersion)and a helper to mint API keys backed by the existingapikeyfakes (seetest/api_keys_http_test.go). - HTTP harness. Call
validator.InitValidator(). Build each router with the same middleware chain asapi/router.gofor that group, now with the permission middleware, over use cases wired to fakes.- Follow whichever approach the closest existing HTTP test uses. Existing tests for
NewUserRouter,NewSettingsRouter,NewSiteRouter,NewAuthorRouter,NewContentTypeRouter,NewBuiltinTypeRouter,NewActivityRouter,NewSystemRouter,NewAPIKeyRouter,NewMediaRouterandNewImageVariantRouterneed the role repository added to their fixtures. - Mount
authtoken.Verifierandcontainer.apiKeyAuth()equivalents where the real router has them, so key scenarios traverse the same order. - No live server, no real SQLite file, no real SMTP, no real update helper.
- Follow whichever approach the closest existing HTTP test uses. Existing tests for
- A gate-matrix helper
assertGate(t, rec, want). For "ok" assert the status is not themissing_permission403 and the handler or use case spy was reached. For "403" assert status 403, bodymissing_permission\n, headerX-Missing-Permissionequals the key, and the spy was not reached.
A. Domain and use cases (fakes only)
- Permission check use case (name left to coder, for example
CheckPermission): table-test owner, admin, writer, a custom role, unresolvable role (expectrole.ErrMissingPermissionviaerrors.Is; the failing key travels in a typed error viaerrors.As), a failing repository (expect a non-sentinel error), and a live grant change between two calls. ListActivity: all, own (self filter forced, other actor returnsaudit.ErrActorNotAllowed), none. Nouser.Role.ListAPIKeysandRevokeAPIKey: all, own, none.created_by_namefollows scope all. Own on another's key returnsapikey.ErrAPIKeyNotFound.ListAuthorsandUpdateAuthor:user_emailfollowsusers.view.authors.editown, all, none, includingauthor.ErrAuthorRightsfor own violations.- The
can_applycomputation in the update status followssystem.apply_updates. - Hygiene tests, in the style of the existing
*_hygiene_test.go:api/router.goandapi/middlewarecontain noRequireOwner.- Non-test Go under
api/andinternal/infrastructure/useCaseshas nouser.RoleOwneroruser.RoleAdmincomparison apart from the allowed spots listed in the feature. internal/domain/rolestill imports only the stdlib.
B. HTTP
- Gate matrices: run every Scenario Outline row. One test per outline, subtests per row, named after the row.
- Contract: status 403, body
missing_permission\n,X-Missing-Permission, no side effect (no invite stored, no mail sent, no site created, retention unchanged). - Order: stale token 401, no cookie 401, unverified 403 verification, API key on session-only routes 403
session_required, API key on/v1/media/1/variants401, all before the permission check. Limiter order: 6 admin update checks give 6 times 403; exhausted invite limiter gives 403 for a Writer. - Scoped: activity (list, summary, export), api keys (list, revoke), authors (edit,
user_email) per the Outlines. - API key live role: change
role_idbetween requests, edit grants between requests, and unresolvable role. Use the same key value throughout. - Session live role: edit grants between two requests with the same cookie.
- Legacy string: the six rows of the "legacy role string" Outline.
- Ungated: public routes,
/v1/auth/me,/v1/permission,/v1/site,/v1/user/lookup,PATCH /v1/user/me,POST /v1/author/meanswer for a user whose role has no grants. Article and entry routes behave as in their existing tests. - Regression: run the existing suites for the owner and admin cases in the "does not regress" Outline unchanged, after the assertion edits listed below.
Existing tests that must change (found by grep, so recheck with go vet)
Assertions of the removed constants become missing_permission, or the expectation flips where Admin gains access.
| File (lines) | Change |
|---|---|
| test/roles_permissions_foundation_http_test.go (around 320, the "Existing role checks are untouched" Outline) | Superseded. The admin rows for invite and /system/update/check move to this slice (invite becomes ok, update check becomes missing_permission). Keep the owner rows. |
| test/update_http_test.go:248; test/update_download_http_test.go:187; test/update_restart_http_test.go:263-264 | Admin 403 body becomes missing_permission |
| test/activity_log_backend_http_test.go:280, 735; test/activity_filter_cards_and_export_http_test.go:75; test/activity_next_cleanup_test.go:199 | Message becomes missing_permission. The "editor" fixtures need a Writer or custom role_id. Admin retention read stays 200. |
| test/user_roles_test.go:280; test/invite_management_test.go:410; test/api_keys_http_test.go:434, 573 | Admin invite is now allowed. Other Owner-required assertions become missing_permission. API key list and revoke fixtures need roles. |
| test/seo_output_sitemap_http_test.go:167; test/seo_data_site_http_test.go; test/site_acceptance_test.go; test/authors_backend_http_test.go | Admin PATCH /v1/site/{id} is now allowed. Message assertions for non-admins become missing_permission. |
| test/custom_content_types_test.go:131 | "member without a role" gets 403 missing_permission. The fixture needs an unresolvable or empty role. |
| test/media_purge_http_test.go:160-167; test/media_purge_qa_test.go:31; test/image_variants_per_image_http_test.go:152-155; test/image_variants_cache_http_test.go:32-33; test/image_variants_admin_http_test.go:42-47 | The "editor", "plain" and "empty role" users need role_ids. Expected body becomes missing_permission. |
every test that builds users with a legacy Role and no RoleID |
Add RoleID (or rely on the fallback for "owner" and "admin" only). Users with legacy roles like "editor" or "" are now unresolvable. |
Existing features to amend or mark superseded
Edit the owner/admin 403 clauses in place or add a "superseded by roles_permissions_enforcement" note.
- roles_permissions_foundation (last Outline)
- user_roles, invite_management, invitations
- sites_backend, sites_authors_integration, seo_data_backend, seo_output_backend, seo_sites_articles_integration
- authors_backend
- activity-log-backend, activity_filter_cards_and_export, activity_home_card_and_retention
- api_keys, integration_api_key_routes, integration
- media_purge, media_cleanup_spa, image-variants, image_variants_cache, image_variants_per_image_api
- updates_download_stage, updates_restart_helper, updates_release_feed, container_update_guidance
- custom_content_types, custom_content_types_support
Also check the matching .qa.md files and these non-test texts: web/src/data/apiCatalogue.ts (OWNER_REQUIRED_MESSAGE, OWNER_OR_ADMIN_MESSAGE), web/src/views/IntegrationView.vue ("Owner and admin routes answer 403…") and anything under internal/infrastructure/llmdocs that documents the old messages. The SPA stores handle 403 by status only, so they should be unaffected; this is a manual check.
Manual verification
- Start
task dev:apion a copy of a migrated database. As the admin, check that settings are readable and software and retention updates return 403. As the owner, check everything still works. - Create an API key as an admin, change that user to Writer by editing
users.role_idin the database copy, and confirm the same key now gets 403 on a schema write. - The SPA is untouched, so the admin will still see owner-only UI hidden. That is expected until the SPA slices.