feat: /v1/role list, get, create, update and delete. Reassign members on delete. Write-time validation (ValidateGrants, duplicates, typed keys against real content types). Name uniqueness (role_name_taken), role_locked, role_has_members, role_conflict on version mismatch. No-escalation: nobody grants a permission they don't hold, and roles.edit_owner_only guards the Owner role. Activity events for role changes. Typed-key exposure in /auth/me. Cleanup of typed rows when a content type is deleted. | Backend only. Will carry the deferred ListGrants decision.
This commit is contained in:
Florian Sylvain committed
1 parent
a043f7f41a
commit
094df63a7f
51 files changed
+4822
-115
No files matched your search
@@ -0,0 +1,110 @@
|
||||
# QA procedure: roles_permissions_roles_api
|
||||
|
||||
Package: `test` (black-box). New fakes need unique names, for example `rolesAPIRoleRepo`, `rolesAPIUserRepo`, `rolesAPITypeRepo`, `rolesAPIRecorder`.
|
||||
Reuse `mintCookie`, `assertGate`, the API key minting helper and the seeded fakes of `test/role_support_test.go` and `test/roles_content_scope_world_test.go` where they fit.
|
||||
Do not copy any scope-rank comparison into tests: call `GrantSet.Covers`, `ScopeFor` and the exported helpers.
|
||||
|
||||
## Deferred to later slices
|
||||
|
||||
- 4: PUT /v1/user/{id}/role, invites with a role (and counting pending invites as members), ownership transfer, removal of the legacy role string.
|
||||
- 5-7: SPA (roles list, editor, preview, delete dialog, role-aware shell). The SPA reacts to status, body code and the X-Missing-Permission / X-Escalation-Key headers.
|
||||
|
||||
## Design notes for the user
|
||||
|
||||
- docs/design says the conflict copy names who saved and when: this slice adds updated_at and updated_by to the role (question 1). The 409 body stays plain text.
|
||||
- docs/design names the events role.created / updated / deleted. The code convention is create / edit / delete (question 8). The SPA activity copy keys on the action, so the design board needs the code names.
|
||||
- docs/design has no code for "built-in role can't be deleted", "own role" and the escalation key: role_built_in, own_role, X-Escalation-Key are new.
|
||||
- The design asks the Owner role to be uneditable: role_locked is checked first, before own_role and the built-in rule.
|
||||
- Agents never edit docs/design: these four items are for you to relay to Claude Design.
|
||||
|
||||
## Inventory (route, key, session only, checks after the load, Owner / Admin / Writer)
|
||||
|
||||
| Route | Key | Session only | After the load | Owner / Admin / Writer |
|
||||
|---|---|---|---|---|
|
||||
| GET /v1/role | roles.view | no | none | ok / ok / 403 |
|
||||
| GET /v1/role/{id} | roles.view | no | 404 role_not_found | ok / ok / 403 |
|
||||
| POST /v1/role | roles.manage | yes | ValidateGrants, type keys, covers, name taken | 201 / 201 / 403 |
|
||||
| PUT /v1/role/{id} | roles.manage | yes | locked, built-in rule, own role, covers existing, version, validate, covers new, name taken | ok / ok (not built-in) / 403 |
|
||||
| POST /v1/role/{id}/reset | roles.manage and roles.edit_owner_only | yes | locked, built-in only, version, restore BuiltInRoles() | 200 / 403 / 403 |
|
||||
| DELETE /v1/role/{id} | roles.manage | yes | locked, built-in, own role, covers existing, members | ok / ok / 403 |
|
||||
| POST /v1/role/{id}/reassign | roles.manage | yes | locked, built-in, own role, covers source, target valid, covers target | ok / ok / 403 |
|
||||
| DELETE /v1/content-type/{key} | schema.delete | as before | removes typed rows after the delete | as before |
|
||||
| POST /v1/content-type | schema.edit | as before | clears stale typed rows for the key | as before |
|
||||
| GET /v1/auth/me | none | as before | lists typed grants | as before |
|
||||
|
||||
## Test infrastructure first
|
||||
|
||||
1. `rolesAPIRoleRepo`: in-memory `role.Repository` seeded with the three built-ins (same Go function the migration uses) plus the Background roles. Implements `List` (with grants), `Get`, `GetByName`, `Create` (new id, version 1), `Save` (compare-and-swap on version, error `role.ErrRoleConflict`, also sets updated_at/updated_by), `Delete` (`role.ErrRoleHasMembers` if a user in the shared user fake has the role_id), `ReassignMembers`, `RemoveTypedGrants`. A switchable failure per method, call counters for `List`, `Get`, `GetByName`, and a hook to run code between `Get` and `Save` (race tests).
|
||||
2. User fake with `GetAll`, a role mover shared with the role fake, and users 3, 5, 9, 10 (role_id 0, legacy admin), 12 (unverified Admin), 31-35, 41-45.
|
||||
3. Content type fake with "events" and "venues". Recording `audit.Recorder` to assert event count, action, category, target, params, changes.
|
||||
4. HTTP harness: `validator.InitValidator()`, a fresh `chi.NewRouter()` mirroring `NewRoleRouter` in api/router.go (Verifier, apiKeyAuth equivalent, Authenticator, RequireCurrentToken, RequireVerifiedUser, RequireSession on writes, `RequirePermission(role.KeyRolesView|KeyRolesManage)`, AuditContext) over real use cases and the fakes. No live server, no real SQLite, no real SMTP.
|
||||
5. `assertGate(t, rec, key)` for the 403 contract. New helper `assertRefusal(t, rec, status, code)` for plain-text codes, and `assertEscalation(t, rec, key)` for the `escalation` body plus `X-Escalation-Key`.
|
||||
6. `grantsOf("content.update=own", "content.update:events=none", ...)` from the content-scope tests, so rows read like the feature.
|
||||
|
||||
## A. Domain (no fakes)
|
||||
|
||||
7. `GrantSet.Covers`: table of caller grants x candidate grants x type keys, including typed none, caller `update=all, update:article=none` against candidate `update=all` (violation key `content.update:article`), caller with typed-only read, toggles, and the first-violation order. Symmetry check: Covers(x, x) is always true, Owner covers every role, Admin does not cover Owner.
|
||||
8. Normalised grant list function: duplicates, owner-only on unlocked, unknown keys, typed none kept, order (wildcard then typed by type key).
|
||||
9. New sentinels exist and map with `errors.Is`: role.ErrRoleHasMembers, ErrRoleNameTaken, ErrRoleLocked, ErrBuiltInRole, ErrBuiltInName, ErrOwnRole, ErrInvalidRoleName, ErrUnknownContentType, and the typed `EscalationError{Key}` read with `errors.As`.
|
||||
10. Hygiene tests in the style of `*_hygiene_test.go`: `internal/domain/role` imports the stdlib only; `Repository` has no `ListGrants`; no scope-rank comparison outside `internal/domain/role`; `api/router.go` and `api/handlers/role` hold no catalogue key literal and no import of persistence, models, gorm or mailer; the drift test for role.Key constants still passes.
|
||||
|
||||
## B. Use cases (fakes only)
|
||||
|
||||
11. `ListRoles`/`GetRole`: members through the legacy fallback (user 10 counted in Admin), order by id, normalised grants, updated_by name (display name, else username, null for a deleted user), `List` called once, `ListGrants` does not exist.
|
||||
12. `CreateRole`: every row of the validation Outline, with `errors.Is` on the sentinel; type keys against the content type fake plus "article"; the covers rows; name taken (case-insensitive, trimmed); the stored role is built_in false, locked false, version 1; one `role.create` event with the right target and params.
|
||||
13. `UpdateRole`: every row of "Editing a role", "No-escalation" and "above their own grants" Outlines. Assert the order of checks with counters: for a refusal before the version check the fake's `Save` and the content type `List` are never called. No-op PUT: no `Save`, no event, same version. Race: the hook bumps the version between `Get` and `Save`, expect `role.ErrRoleConflict`. Own role resolved for a legacy user (role_id 0).
|
||||
14. Diff: the example edit gives added/removed/changed exactly as in the feature, plus name and description text changes; created role: added only; deleted role: members param.
|
||||
15. `DeleteRole`, `ReassignRoleMembers`: every row of their Outlines; `moved` counts; no event when moved is 0; repository failure gives a failed event and no partial move; delete race (`Delete` returns `role.ErrRoleHasMembers`).
|
||||
16. `DeleteContentType`: removes typed rows after the type delete, bumps only affected roles, does nothing when the delete is refused (`contenttype.ErrHasEntries`, `ErrReferenced`). `CreateContentType`: clears stale typed rows. Both with the role fake's `RemoveTypedGrants` counter.
|
||||
17. `GetMyAccess`: typed entries incl. none, order, wildcard none omitted.
|
||||
18. Activity: `audit.Categories()` contains "roles"; `audit.Event.Validate()` accepts the new category; the filter accepts `category=roles`.
|
||||
|
||||
## C. HTTP
|
||||
|
||||
19. Gate matrix: every row of the first Outline, one subtest per row, stores reset per row. API key writes get `apperr.SessionRequiredErrorMessage`. Live-role scenario (grant added to Viewer between two requests with the same cookie).
|
||||
20. Each Outline of the feature: validation, name rules, no-escalation, above-own-grants, editing, delete, reassign. Assert status, exact body text, headers (`X-Missing-Permission`, `X-Escalation-Key`), the store after the call, and the recorder.
|
||||
21. Order: for a Writer, `PUT /v1/role/abc` and `DELETE /v1/role/999` are 403 before 400/404; for a Viewer too. For user 31 an unknown key in the body is 400 before the escalation 403; an escalation on the existing role (Content lead) is 403 before a stale version 409.
|
||||
22. Responses: list shape, one-role shape (members, updated_at, updated_by), 201 on create, 204 on delete, `{"moved": n}` on reassign. Content-Type header is JSON for success bodies, errors are plain text.
|
||||
23. Live effect: after PUT on Contributor, user 41's next request on a gated route sees the new grants with the same cookie and an API key created by user 41.
|
||||
24. `/auth/me`: the Events editor example.
|
||||
25. Content type delete/create through the real content type router with the role fake attached (rows removed, versions bumped, refused delete leaves rows).
|
||||
26. Loads per request: a gated request on a role route still loads the user at most once and the role once (`Get`), `List` 0 times by the permission check.
|
||||
27. Regression: the 2a and 2b suites (enforcement, content scope) stay green after the interface change.
|
||||
|
||||
## D. Persistence (existing repository-test style, in-memory migrated DB helper `openMigratedRolesDB`)
|
||||
|
||||
28. `RoleRepository`: `List` returns grants for every role in one pass; `Save` compare-and-swap (two saves of version 1: one wins), writes updated_at and updated_by; `Delete` removes rows and refuses with `ErrRoleHasMembers`; `ReassignMembers` returns the count and touches only role_id; `RemoveTypedGrants` removes the five `content.*:<key>` forms only, bumps version once per affected role, leaves wildcard and other types; failure paths (DB closed) for every new method; `Create` of a role with typed rows; seed idempotency unchanged (72 rows, edited built-in kept).
|
||||
29. `role` and `role_permission` columns: updated_at and updated_by exist after `Migrate` (AutoMigrate list in `persistence/database/migrate.go`), seeded roles have updated_by NULL.
|
||||
|
||||
## Existing tests that must change (recheck with `go vet -tags nofrontend ./test/...`)
|
||||
|
||||
| File (lines) | Change |
|
||||
|---|---|
|
||||
| test/role_support_test.go:85 | `mockRoleRepo.ListGrants` removed. Add `Delete`, `ReassignMembers`, `RemoveTypedGrants`; `List` returns grants; `Save` keeps its conflict behaviour |
|
||||
| test/roles_content_scope_world_test.go:79-81 and test/roles_content_scope_http_test.go:587, 599, 604, 616, 621 | `listGrantsN` and the `ListGrants` wrapper go away. Replace "0 ListGrants" with "0 List calls by the permission check and /auth/me" |
|
||||
| test/role_repository_gaps_test.go:45-48 | The `ListGrants` failure case becomes a `List` failure case |
|
||||
| test/roles_content_scope_usecases_test.go:481, test/roles_content_scope_world_test.go:222, test/custom_content_types_support_test.go:271, 275, test/custom_content_types_support_http_test.go:220, 426, test/custom_content_types_usecases_test.go:154, 596, test/relation_field_type_test.go:127, 234 | `NewCreateContentTypeUseCase` and `NewDeleteContentTypeUseCase` take the role repository (clearing typed rows). Pass a role fake or a no-op one |
|
||||
| test/activity_domain_test.go:59 | Categories list gains "roles" |
|
||||
| test/roles_content_scope_http_test.go (the /auth/me typed-grants test) and the feature scenario "/auth/me lists the effective grants" | "typed grants are not listed" is superseded by typed entries incl. none |
|
||||
| test/api_catalogue_drift_test.go:52 | Add the 6 new routes (GET /v1/role, GET /v1/role/{id}, POST /v1/role, PUT /v1/role/{id}, DELETE /v1/role/{id}, POST /v1/role/{id}/reassign) to `routesNotYetInTheCatalogue`. The existing failure of `TestRouterRoutesAreInTheCatalogue` is not this slice's: report, do not fix |
|
||||
| any test that lists `role.Repository` method sets or builds a `role.Role` literal with positional fields | recheck after adding UpdatedAt and UpdatedBy |
|
||||
|
||||
## Existing features and docs to amend
|
||||
|
||||
- features/roles_permissions_content_scope.feature: the scenario "/auth/me lists the effective grants" last line ("typed grants are not listed in this slice") and "Out of scope here" (typed grants written by slice 3: now done); add a "superseded by roles_permissions_roles_api" note in place. Same for its `.qa.md` "Deferred" line 3 and the manual item about editing `role_permission` by hand.
|
||||
- features/roles_permissions_foundation.feature: the Background sentence "per-type overrides ... deferred to slice 3" and the Repository interface line (list, get by id, get by name, list grants, create, save).
|
||||
- features/activity-log-producers.qa.md: add the roles category and the four actions.
|
||||
- docs/api.md: new "Roles" section (routes, codes, headers, response shapes), the `/v1/auth/me` typed entries, the activity category and actions, and the content-type delete/create side effect. docs/architecture.md: role repository methods.
|
||||
- docs/design: for you to relay, not edited by agents (see Design notes).
|
||||
|
||||
## Manual verification
|
||||
|
||||
- Edit the Writer role, then POST /v1/role/{id}/reset as Owner with the current version: grants are back to the seed and a second reset is a no-op.
|
||||
|
||||
- `task dev:api` on a copy of a migrated database. As the Owner: create "Contributor" (Write own preset grants), edit it, move a member to Writer, delete it. Check the activity log shows role.create, role.edit (with added/removed/changed), role.reassign and role.delete.
|
||||
- As an Admin: editing Writer answers 403 with X-Missing-Permission roles.edit_owner_only; editing Owner answers role_locked.
|
||||
- With a role manager who publishes only own entries: granting publish all answers escalation with the key in X-Escalation-Key.
|
||||
- Open two sessions on the same role, save from both: the second gets role_conflict.
|
||||
- Restart the server after editing Writer: the edit is still there (seed does not overwrite).
|
||||
- Delete a content type that has a typed row and check the role_permission rows are gone in the database copy.
|
||||
- No secret appears in any scenario. Env vars are referenced by name only (JWT_SECRET).
|
||||
Reference in new issue
Block a user