Skip to content

Commit 80cbeec

Browse files
committed
Merge torrust#856: feat!: refactor roles & permissions system (ADR-T-008)
1998ccb refactor(auth): post-review hardening for ADR-T-008 (Peer Cat) c461db0 feat(auth): add Moderator role, exhaustive Admin grants, and hardened diagnostics (Peer Cat) 737c4c2 feat: ADR-T-008 Phase 4 — E2E tests for permissions system (Peer Cat) 44ccd66 feat: implement ADR-T-008 Phase 3 — resource-level authorization (Peer Cat) 58c23cf refactor(auth): enforce permissions at HTTP boundary with RequirePermission<A> (Peer Cat) 70701db refactor(auth)!: replace Casbin with native PermissionMatrix (ADR-T-008) (Peer Cat) Pull request description: ## Summary Replace the Casbin-based authorization system with a native Rust permission model built on exhaustive `Role` and `Action` enums. Adding a new role or action without deciding its permission is now a **compile error**, eliminating silent policy gaps. ### Motivation The prior system had three intertwined authorization mechanisms (Casbin RBAC, a bare `administrator` boolean in the DB, and scattered inline ownership checks). This made the permission surface hard to audit, impossible to extend with intermediate roles, and left resource-level authorization as ad-hoc string comparisons sprinkled through service methods. ### What changed **Phase 1 — Native `PermissionMatrix`** (`refactor(auth)!`) - Replace `casbin` crate with a `HashSet<(Role, Action)>` table populated by exhaustive `match` arms. - Introduce `Role` enum (`Guest`, `Registered`, `Admin`) with `Display`/`FromStr`/serde support. - Rename `ACTION` → `Action` (Rust conventions). - Migrate DB from `administrator: bool` to `role: String`; backfill existing rows. - Remove the `unstable.auth.casbin` config section entirely. **Phase 2 — `RequirePermission<A>` extractor** - Generic Axum extractor resolves identity from the bearer token, looks up `Role` in the DB, and checks the matrix — all before the handler runs. - `ActionMarker` trait + `action_markers!` macro bind each handler to an `Action` at the type level (compile-time enforcement). - Remove `authorization::Service` and strip auth concerns from service methods. - Delete the old `ExtractOptionalLoggedInUser` / `ExtractLoggedInUser` extractors. **Phase 3 — Resource-level authorization & permissions discovery** - `can_on_resource()` method: Admin bypasses ownership; other roles must be the resource owner. - `GET /v1/user/me/permissions` endpoint returns the caller's resolved role and allowed actions. - `[permissions.overrides]` TOML config — operators can grant/deny `(role, action)` tuples without touching Rust source. - Drop the legacy `administrator` column (MySQL + SQLite migrations). - Remove `admin: bool` from all API responses. **Phase 4 — E2E tests** - Five E2E tests covering permission discovery (admin / registered / guest) and TOML override propagation (grant + deny paths). - `TestEnv::start_with()` config-mutator support for isolated test environments. **Phase 5 — Moderator role & hardening** - Add `Role::Moderator` with a scoped permission set (tags + `DeleteTorrent`). - Replace `Admin => true` wildcard with exhaustive action match — new actions are a compile error until granted per-role. - Emit `warn!` when TOML overrides grant high-risk actions to non-admin roles. - Improved logging in registration and `grant_admin_role` paths. ### Additional cleanup - Update all API doc examples to RS256 JWT format (post ADR-T-007). - Fix several README typos and stale "Torrust Tracker" → "Torrust Index" references. - AGENTS.md: add "Counts and Numbers" guideline. ### Breaking changes - `administrator: bool` replaced by `role: String` in API responses. - The `casbin` configuration section is removed. - `ACTION` enum renamed to `Action`. ### Stats ~64 files changed, ~3k insertions, ~1k deletions across source, tests, migrations, docs, and the ADR. ACKs for top commit: peer-cat: ACK 1998ccb da2ce7: ACK 1998ccb Tree-SHA512: 8289b69bae573e150537e453d859131968d31b8117468e7f18b36d6be0cc0ae2c7fe2ec13b17f291098ec593dde5b5c75804cb3b8b68249644c350c920ae4bfb
2 parents 7ad7866 + 1998ccb commit 80cbeec

66 files changed

Lines changed: 3347 additions & 1147 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

AGENTS.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,12 @@ suite, for example when finishing up.)
2222
When running tests, tee to a temp file (`/tmp/...`) and then grep that
2323
file after the tests have completed.
2424

25+
## Counts and Numbers
26+
27+
Do not use exact numbers, i.e. not: "100 tests passed".
28+
Instead, use fuzzy numbers, e.g. "A hundred tests passed" or "~320 functions in total".
29+
Precise numbers may be used for qualified historical records. e.g. "on 21.12.1992 phase 3 was marked as accepted with 465 passing tests and 3 failures."
30+
2531
## Test Locations
2632

2733
We use three levels of tests based upon viability. In general, crate tests

CHANGELOG.md

Lines changed: 66 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Added
1111

12+
- ADR-T-008: Document rationale for roles and permissions refactor.
1213
- ADR-T-006: Document rationale for error system refactor.
1314
- 188 crate-level tests for the domain error system (`src/tests/errors/`):
1415
status-code mapping, display messages, `From` impl coverage, and
1516
`ApiError` delegation (ADR-T-006 §1–§4).
17+
- Native `PermissionMatrix` replacing Casbin: compile-time checked `Role` and
18+
`Action` enums with an exhaustive default-deny policy table (ADR-T-008).
19+
- `Permissions` trait abstraction for the authorization backend, consumed by
20+
the `RequirePermission<A>` extractor via `AppData.permissions`.
21+
- `role: TEXT` column on `torrust_users` (migration for SQLite and MySQL);
22+
existing `administrator = true` rows migrated to `role = 'admin'`, others
23+
to `role = 'registered'`.
24+
- `role: String` field on `TokenResponse`, `UserCompact`, `UserProfile`, and
25+
`UserFull` API response models.
26+
- `RequirePermission<A>` Axum extractor enforcing role-based authorization at
27+
the HTTP boundary before the handler runs (ADR-T-008 Phase 2).
28+
- `ActionMarker` trait and `action_markers!` macro mapping zero-sized types to
29+
`Action` enum variants for compile-time handler–permission binding.
30+
- `Actor` struct yielded by `RequirePermission` carrying the resolved
31+
`user_id` and `Role` for downstream handler use.
32+
- `Actor::try_user_id()` non-panicking accessor returning `Option<UserId>`,
33+
safe for handlers that may serve guests (ADR-T-008).
34+
- `Actor::is_authenticated()` convenience predicate (ADR-T-008).
35+
- Compile-time `action_markers!``Action::ALL` sync assertion: adding an
36+
`Action` variant without a matching marker (or vice versa) is a compile
37+
error (ADR-T-008).
38+
- E2E tests for non-owner update and delete denial (`and_non_owners` module
39+
in `tests/e2e/web/api/v1/contexts/torrent/contract.rs`) (ADR-T-008 Phase 4).
1640
- ADR-T-007: Document rationale for JWT system refactor.
1741
- Centralised JWT module (`src/jwt.rs`) consolidating all `jsonwebtoken` usage:
1842
key loading, signing, verification, and algorithm configuration.
@@ -52,6 +76,25 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5276
### Changed
5377

5478
- **BREAKING:** Raise MSRV from 1.85 to 1.88.
79+
- **BREAKING:** `administrator: bool` replaced by `role: String` in API
80+
responses (`TokenResponse`, `UserCompact`, etc.). The legacy `admin: bool`
81+
field has been removed entirely (ADR-T-008).
82+
- **BREAKING:** `administrator` column dropped from `torrust_users`; the
83+
`role: TEXT` column is now the sole authority. Migration
84+
`20260415000001_torrust_drop_administrator_column` handles both SQLite
85+
(table-rebuild) and MySQL (`DROP COLUMN`).
86+
- **BREAKING:** `ACTION` enum renamed to `Action`; variants unchanged.
87+
- All HTTP handlers that require authorization now use
88+
`RequirePermission<A>` extractors instead of calling
89+
`authorization::Service::authorize()` (ADR-T-008 Phase 2).
90+
- Service methods no longer receive `maybe_user_id` for authorization
91+
purposes — they receive an already-authorized `Actor` or are called
92+
unconditionally.
93+
- Unauthorized requests are rejected at the extractor boundary before
94+
reaching the service layer (fail-fast).
95+
- First-user auto-admin grant in `RegistrationService::register` now logs
96+
a `warn!` on failure instead of silently discarding the `Result` via
97+
`drop()`.
5598
- **BREAKING:** JWT signing algorithm changed from HMAC-HS256 to RS256
5699
(RSA + SHA-256). Existing HS256 tokens are invalidated; users must re-login.
57100
- **BREAKING:** JWT claims redesigned from `UserClaims { user, exp }` to
@@ -66,8 +109,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
66109
`CategoryTagError`, and a thin `ApiError` wrapper (ADR-T-006).
67110
- `Authentication::get_user_id_from_bearer_token` now takes `BearerToken`
68111
directly instead of `Option<BearerToken>`.
69-
- `ExtractLoggedInUser` and `ExtractOptionalLoggedInUser` use `BearerToken`
70-
directly instead of the old `Extract` wrapper.
71112
- `parse_token` returns `Result` instead of panicking on malformed headers.
72113
- JWT `exp` validation relies solely on the `jsonwebtoken` library; redundant
73114
manual expiration check removed.
@@ -80,10 +121,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
80121
- Each domain error co-locates its HTTP status-code mapping via a
81122
`status_code()` method.
82123
- Error `From` impls use `tracing::error!` instead of `eprintln!`.
124+
- JWT session token `role` claim now carries the database `role` value
125+
directly (`"registered"`, `"admin"`) instead of the previous mapping
126+
(`"user"`, `"admin"`).
127+
- v1→v2 upgrade path: `insert_imported_user` now writes the `role` column
128+
(`"admin"` / `"registered"`) instead of the removed `administrator` column.
83129
- Standardise all error derives on `thiserror`.
84130

85131
### Removed
86132

133+
- `admin: bool` field from `TokenResponse`, `LoggedInUserData`, and
134+
`TokenRenewalData` — superseded by `role: String` (ADR-T-008).
135+
- `UserCompact::is_admin()` convenience method — no longer needed after
136+
`admin: bool` removal.
137+
- `administrator` column from `torrust_users` schema (migration for both
138+
SQLite and MySQL).
139+
- `casbin` crate dependency and all Casbin-related code
140+
(`CasbinConfiguration`, `CasbinEnforcer`, the `ACTION` enum in
141+
SCREAMING_CASE) — replaced by the native `PermissionMatrix` (ADR-T-008).
142+
- `unstable.auth.casbin` configuration section (`Unstable`, `Auth`, `Casbin`
143+
config structs in `src/config/v2/unstable.rs`).
87144
- `bearer_token::Extract` wrapper struct (replaced by `BearerToken` directly).
88145
- `get_optional_logged_in_user` free function (logic moved into extractors).
89146
- `get_claims_from_bearer_token` private method on `Authentication` (inlined).
@@ -93,6 +150,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
93150
- `http_status_code_for_service_error` and `map_database_error_to_service_error`
94151
helper functions.
95152
- `IntoResponse` impl for `database::Error` (now handled by domain errors).
153+
- `authorization::Service` struct — replaced by `RequirePermission<A>`
154+
extractors consulting `PermissionMatrix` directly (ADR-T-008 Phase 2).
155+
- `ExtractLoggedInUser` (`user_id.rs`) and `ExtractOptionalLoggedInUser`
156+
(`optional_user_id.rs`) extractors — replaced by `RequirePermission<A>`
157+
(ADR-T-008 Phase 2).
158+
- Dead `Action` variants `GetSettings` and `GetCanonicalInfoHash` (no
159+
corresponding handlers existed).
96160

97161
## [4.0.0] - 2026-03-23
98162

0 commit comments

Comments
 (0)