Commit 8310fb3
committed
Merge torrust#853: refactor!: secure JWT authentication with RS256 and token revocation (ADR-T-007)
6126eec fix(auth): harden token parsing and remove panics in ban logic (Peer Cat)
593276a refactor(jwt): consolidate session validation into single code path (ADR-T-007 Phase 7) (Peer Cat)
e23cd3f feat(jwt): atomic token revocation and hardened session validation (ADR-T-007) (Peer Cat)
49504f6 feat: add generate-auth-keypair CLI and container auto-generation (ADR-T-007 Phase 6) (Peer Cat)
4183c12 feat(jwt): auto-generate ephemeral RSA keys when none configured (ADR-T-007 Phase 5) (Peer Cat)
7c8ca72 fix(e2e): provision JWT PEM keys and fix health-check script (Peer Cat)
4adbe3c feat(jwt): add token revocation via per-user generation counter (ADR-T-007 Phase 4) (Peer Cat)
40fe5e5 refactor(jwt)!: switch from HMAC-HS256 to RS256 asymmetric signing (ADR-T-007 Phase 3) (Peer Cat)
8411e7d refactor(jwt): implement ADR-T-007 Phase 2 — per-purpose signing keys and RFC 7519 claims (Peer Cat)
b6f51c3 refactor!: centralise JWT handling into dedicated module (ADR-T-007 Phase 1) (Peer Cat)
Pull request description:
### Summary
Replace the JWT authentication system end-to-end: HMAC-HS256 shared secret → RS256 asymmetric signing, RFC 7519 claims, per-user token revocation with atomic database operations, ephemeral key auto-generation, a CLI for persistent key provisioning, and consolidated session validation via a single code path. Implemented across 7 phases in 9 incremental commits (ADR-T-007).
### Motivation
The previous setup used a single low-entropy HMAC-HS256 secret for all token types, had no standard registered claims, baked stale role data into payloads with no way to invalidate it, hard-coded expiration durations, panicked on encode/decode failures, and offered no mechanism to revoke tokens on password change or ban.
### Changes by phase
| Commit | Phase | Description |
|--------|-------|-------------|
| `b6f51c3` | 1 — Structural cleanup | New `jwt.rs` module centralises all `jsonwebtoken` usage; `sign()`/`parse_token()` return `Result`; configurable token lifetimes replace hard-coded durations; config key renamed to `jwt_signing_secret` |
| `8411e7d` | 2 — Claim redesign | Per-purpose signing keys (`session` / `email_verification`); RFC 7519 registered claims (`sub`, `iss`, `aud`, `iat`, `exp`); 32-byte minimum key length enforced; role/username fields marked advisory-only |
| `40fe5e5` | 3 — RS256 asymmetric signing | RSA-2048 PEM key pair replaces HMAC secrets; `kid` header (SHA-256 of public key) for future key rotation; verification is synchronous since keys are pre-loaded at startup |
| `4adbe3c` | 4 — Token revocation | `token_generation` column added to `torrust_users` (MySQL + SQLite migrations); `gen` claim in session JWTs; password changes, role grants, and bans increment the counter, invalidating all outstanding tokens |
| `7c8ca72` | — (fix) | Provision PEM keys in E2E container scripts; make health-check script directly invocable |
| `4183c12` | 5 — Ephemeral keys | Remove shipped development key pair; auto-generate RSA-2048 in-memory at startup when no keys are configured; sessions work immediately but don't survive restarts |
| `49504f6` | 6 — CLI & container keys | `torrust-generate-auth-keypair` binary writes PEM key pair to stdout; container entry script auto-generates persistent keys on first boot into `/etc/torrust/index/auth/` with restrictive permissions |
| `e23cd3f` | — (hardening) | Atomic database methods for password change, ban, and admin grant (`transaction` / single `UPDATE`); generation check tightened from `<` to `!=`; defence-in-depth `is_user_banned` fallback; `BearerToken::value()` → `as_str()`; `parse_token` requires space after `"Bearer"` prefix |
| `593276a` | 7 — Consolidate validation | `JsonWebToken::validate_session` becomes the sole entry point for session-token validation — JWT signature/expiry, generation counter, and ban check all happen in one place; removes ~45 lines of duplicated logic across three call sites |
### Breaking changes
- **Config (Phase 1):** `auth.user_claim_token_pepper` → `auth.jwt_signing_secret` (legacy alias accepted).
- **Config (Phase 3):** `auth.session_signing_key` and `auth.email_verification_signing_key` removed. Replaced by `auth.private_key_path` / `auth.public_key_path` (or inline `_pem` variants). Existing HS256 tokens are rejected after upgrade.
- **Config (Phase 5):** Key paths are now optional — omitting them triggers ephemeral key generation instead of an error.
- **Database (Phase 4):** New `token_generation BIGINT/INTEGER NOT NULL DEFAULT 0` column on `torrust_users` (migration included for both MySQL and SQLite).
- **API:** `BearerToken` extractor rejects missing or malformed `Authorization` headers at the extraction boundary. `get_optional_logged_in_user` removed; logic moved into extractors.
### New files
| File | Purpose |
|------|---------|
| jwt.rs | Centralised JWT module (370 lines) — signing, verification, claims, ephemeral key generation, `validate_session` |
| generate_auth_keypair.rs | CLI tool for RSA key pair generation (110 lines) |
| jwt.rs | Crate tests: sign/verify round-trips, audience cross-contamination, tampered tokens |
| auth.rs | Crate tests: `parse_token` whitespace trimming, empty bearer, non-ASCII rejection |
| 007-jwt-system-refactor.md | ADR documenting the full 7-phase design |
| entry_script_sh | Container auto-generation of persistent auth keys |
| `migrations/*/20260414000000_torrust_user_token_generation.sql` | Token generation column (MySQL + SQLite) |
### Testing
- New crate tests in jwt.rs and auth.rs covering sign/verify round-trips, audience enforcement, tampered tokens, bearer parsing edge cases
- All existing E2E and integration tests updated for the new config shape and passing
- Ephemeral key path exercised by default in tests
### References
- ADR-T-007: JWT System Refactor (007-jwt-system-refactor.md)
ACKs for top commit:
peer-cat:
ACK 6126eec
da2ce7:
ACK 6126eec
Tree-SHA512: 7a385c184f8ff0c3cde110f97decd62cd241d92a0671eb3479c988e8de11898e50b8631db666179775c942023ed09619ed03e012d4ae08449226f1cd51e5fd8059 files changed
Lines changed: 2167 additions & 350 deletions
File tree
- adr
- contrib/dev-tools/container
- e2e/sqlite/mode
- private
- public
- functions
- docs
- migrations
- mysql
- sqlite3
- src
- bin
- config
- v2
- databases
- models
- services
- tests
- config
- v2
- web
- web/api
- client/v1/contexts/settings
- server/v1
- contexts
- category
- proxy
- settings
- tag
- torrent
- user
- extractors
- tests
- common/contexts/settings
- e2e
- fixtures
Some content is hidden
Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | | - | |
4 | 3 | | |
5 | 4 | | |
6 | 5 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
16 | 51 | | |
17 | 52 | | |
18 | 53 | | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
19 | 63 | | |
20 | 64 | | |
21 | 65 | | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
22 | 77 | | |
23 | 78 | | |
24 | 79 | | |
| |||
28 | 83 | | |
29 | 84 | | |
30 | 85 | | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
31 | 91 | | |
32 | 92 | | |
33 | 93 | | |
| |||
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
80 | 80 | | |
81 | 81 | | |
82 | 82 | | |
| 83 | + | |
83 | 84 | | |
84 | 85 | | |
85 | 86 | | |
| |||
88 | 89 | | |
89 | 90 | | |
90 | 91 | | |
| 92 | + | |
91 | 93 | | |
92 | 94 | | |
93 | 95 | | |
| |||
102 | 104 | | |
103 | 105 | | |
104 | 106 | | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
105 | 111 | | |
106 | 112 | | |
107 | 113 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
71 | 71 | | |
72 | 72 | | |
73 | 73 | | |
74 | | - | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
75 | 77 | | |
76 | 78 | | |
77 | 79 | | |
| |||
87 | 89 | | |
88 | 90 | | |
89 | 91 | | |
90 | | - | |
| 92 | + | |
| 93 | + | |
91 | 94 | | |
92 | 95 | | |
93 | 96 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
93 | 93 | | |
94 | 94 | | |
95 | 95 | | |
96 | | - | |
97 | | - | |
98 | | - | |
| 96 | + | |
99 | 97 | | |
100 | 98 | | |
101 | | - | |
102 | | - | |
| 99 | + | |
103 | 100 | | |
104 | 101 | | |
105 | | - | |
106 | 102 | | |
107 | 103 | | |
108 | 104 | | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
109 | 124 | | |
110 | 125 | | |
111 | 126 | | |
| |||
127 | 142 | | |
128 | 143 | | |
129 | 144 | | |
| 145 | + | |
130 | 146 | | |
131 | 147 | | |
132 | 148 | | |
| |||
0 commit comments