153 lines
14 KiB
Markdown
153 lines
14 KiB
Markdown
# 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. (Superseded: implemented by roles_permissions_content_scope, which also makes owner-only keys effective only on a locked role.)
|
|
- 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.
|
|
- the removed 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 the `owner_only` clause in routes.md "SEO", disagree with this slice. Admin has `settings.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 `user` role" that does not exist.
|
|
- The docs name `GET /v1/me`. The implemented route is `GET /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.export` plus `activity.view`.
|
|
- change-list item 22 says GET /v1/author works for any role; under 2a a role without `authors.view` gets 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
|
|
|
|
1. `enforcementRoleRepo` implements the role `Repository` in memory, seeded with the three built-ins through the same Go function the migration uses. It supports editing grants and a switchable failure.
|
|
2. 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).
|
|
3. A helper `mintCookie(userID, tokenVersion)` and a helper to mint API keys backed by the existing `apikey` fakes (see `test/api_keys_http_test.go`).
|
|
4. HTTP harness. Call `validator.InitValidator()`. Build each router with the same middleware chain as `api/router.go` for 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`, `NewMediaRouter` and `NewImageVariantRouter` need the role repository added to their fixtures.
|
|
- Mount `authtoken.Verifier` and `container.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.
|
|
5. A gate-matrix helper `assertGate(t, rec, want)`. For "ok" assert the status is not the `missing_permission` 403 and the handler or use case spy was reached. For "403" assert status 403, body `missing_permission\n`, header `X-Missing-Permission` equals the key, and the spy was not reached.
|
|
|
|
## A. Domain and use cases (fakes only)
|
|
|
|
6. Permission check use case (name left to coder, for example `CheckPermission`): table-test owner, admin, writer, a custom role, unresolvable role (expect `role.ErrMissingPermission` via `errors.Is`; the failing key travels in a typed error via `errors.As`), a failing repository (expect a non-sentinel error), and a live grant change between two calls.
|
|
7. `ListActivity`: all, own (self filter forced, other actor returns `audit.ErrActorNotAllowed`), none. No `user.Role`.
|
|
8. `ListAPIKeys` and `RevokeAPIKey`: all, own, none. `created_by_name` follows scope all. Own on another's key returns `apikey.ErrAPIKeyNotFound`.
|
|
9. `ListAuthors` and `UpdateAuthor`: `user_email` follows `users.view`. `authors.edit` own, all, none, including `author.ErrAuthorRights` for own violations.
|
|
10. The `can_apply` computation in the update status follows `system.apply_updates`.
|
|
11. Hygiene tests, in the style of the existing `*_hygiene_test.go`:
|
|
- `api/router.go` and `api/middleware` contain no `RequireOwner`.
|
|
- Non-test Go under `api/` and `internal/infrastructure/useCases` has no `user.RoleOwner` or `user.RoleAdmin` comparison apart from the allowed spots listed in the feature.
|
|
- `internal/domain/role` still imports only the stdlib.
|
|
|
|
## B. HTTP
|
|
|
|
12. Gate matrices: run every Scenario Outline row. One test per outline, subtests per row, named after the row.
|
|
13. Contract: status 403, body `missing_permission\n`, `X-Missing-Permission`, no side effect (no invite stored, no mail sent, no site created, retention unchanged).
|
|
14. 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/variants` 401, all before the permission check. Limiter order: 6 admin update checks give 6 times 403; exhausted invite limiter gives 403 for a Writer.
|
|
15. Scoped: activity (list, summary, export), api keys (list, revoke), authors (edit, `user_email`) per the Outlines.
|
|
16. API key live role: change `role_id` between requests, edit grants between requests, and unresolvable role. Use the same key value throughout.
|
|
17. Session live role: edit grants between two requests with the same cookie.
|
|
18. Legacy string: the six rows of the "legacy role string" Outline.
|
|
19. Ungated: public routes, `/v1/auth/me`, `/v1/permission`, `/v1/site`, `/v1/user/lookup`, `PATCH /v1/user/me`, `POST /v1/author/me` answer for a user whose role has no grants. Article and entry routes behave as in their existing tests.
|
|
20. 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:api` on 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_id` in 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.
|