Skip to content

Commit b7ec439

Browse files
committed
feat(jwt): add token revocation via per-user generation counter (ADR-T-007 Phase 4)
Add a `token_generation` column to `torrust_users` and embed a `gen` claim in every session JWT. Password changes, admin-role grants, and bans increment the counter, instantly invalidating all outstanding tokens for that user. Revocation is checked at three entry points (defence in depth): - `Authentication::get_user_id_from_bearer_token` - `verify_token_handler` - `authentication::Service::renew_token` Rework the `BearerToken` extractor to reject missing or malformed `Authorization` headers at the extraction boundary instead of deferring the check downstream. Remove the `bearer_token::Extract` wrapper and `get_optional_logged_in_user` free function; the logic now lives directly in the extractors. Add `AuthError::TokenRevoked` for revoked-token responses. Add crate tests for the JWT module (sign/verify round-trips, audience cross-contamination, tampered tokens) and for `parse_token` (whitespace trimming, empty bearer, non-ASCII rejection).
1 parent 40fe5e5 commit b7ec439

22 files changed

Lines changed: 580 additions & 116 deletions

CHANGELOG.md

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,60 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313
- 188 crate-level tests for the domain error system (`src/tests/errors/`):
1414
status-code mapping, display messages, `From` impl coverage, and
1515
`ApiError` delegation (ADR-T-006 §1–§4).
16+
- ADR-T-007: Document rationale for JWT system refactor.
17+
- Centralised JWT module (`src/jwt.rs`) consolidating all `jsonwebtoken` usage:
18+
key loading, signing, verification, and algorithm configuration.
19+
- `SessionClaims` with RFC 7519 registered claims (`sub`, `iss`, `aud`, `iat`,
20+
`exp`) plus advisory `role`, `username`, and revocation `gen` fields.
21+
- `VerifyClaims` with `aud: "email-verification"` for purpose separation.
22+
- RSA key pair configuration: `auth.private_key_path` / `auth.public_key_path`
23+
(or inline PEM via `auth.private_key_pem` / `auth.public_key_pem`).
24+
- Development RSA key pair shipped at `share/default/jwt/` with loud startup
25+
warning when the default dev keys are detected.
26+
- `kid` (Key ID) header in every JWT for future key rotation support.
27+
- Configurable token lifetimes: `auth.session_token_lifetime_secs` (default:
28+
2 weeks) and `auth.email_verification_token_lifetime_secs` (default: ~10 years).
29+
- `token_generation` column on `torrust_users` (migration for SQLite and MySQL).
30+
- Token revocation: password changes, role changes (admin grant), and bans
31+
increment `token_generation`; tokens with an older `gen` claim are rejected.
32+
- Revocation checks at three entry points (defence in depth):
33+
`Authentication::get_user_id_from_bearer_token`, `verify_token_handler`,
34+
and `authentication::Service::renew_token`.
35+
- `BearerToken` extractor rejects missing/malformed `Authorization` headers at
36+
the extraction boundary (`AuthError::TokenNotFound` / `AuthError::TokenInvalid`).
37+
- `ExtractOptionalLoggedInUser` catches extraction rejection and returns `None`
38+
for anonymous requests.
39+
- `AuthError::TokenRevoked` variant for revoked-token responses.
40+
- Crate tests for the JWT module (session + email-verification round-trips,
41+
audience cross-contamination, tampered/garbage tokens).
42+
- Crate tests for `parse_token` (valid extraction, whitespace trimming,
43+
empty bearer, missing prefix, non-ASCII rejection).
1644

1745
### Changed
1846

47+
- **BREAKING:** JWT signing algorithm changed from HMAC-HS256 to RS256
48+
(RSA + SHA-256). Existing HS256 tokens are invalidated; users must re-login.
49+
- **BREAKING:** JWT claims redesigned from `UserClaims { user, exp }` to
50+
`SessionClaims { sub, iss, aud, iat, exp, role, username, gen }`. Existing
51+
tokens without the new claims fail deserialization.
52+
- **BREAKING:** Configuration keys changed — `auth.user_claim_token_pepper` /
53+
`auth.session_signing_key` / `auth.email_verification_signing_key` replaced
54+
by `auth.private_key_path` and `auth.public_key_path` (or inline PEM).
55+
Deployers must generate an RSA key pair.
1956
- **BREAKING:** Replace `ServiceError` (41 variants) and `ServiceResult` with
2057
domain-scoped error enums: `AuthError`, `UserError`, `TorrentError`,
2158
`CategoryTagError`, and a thin `ApiError` wrapper (ADR-T-006).
59+
- `Authentication::get_user_id_from_bearer_token` now takes `BearerToken`
60+
directly instead of `Option<BearerToken>`.
61+
- `ExtractLoggedInUser` and `ExtractOptionalLoggedInUser` use `BearerToken`
62+
directly instead of the old `Extract` wrapper.
63+
- `parse_token` returns `Result` instead of panicking on malformed headers.
64+
- JWT `exp` validation relies solely on the `jsonwebtoken` library; redundant
65+
manual expiration check removed.
66+
- Token signing uses `Result` propagation instead of `.unwrap()` / `.expect()`.
67+
- `UserClaims` is now a type alias for `SessionClaims` (backward-compatible).
68+
- `VerifyClaims` moved from `mailer` into the `jwt` module (re-exported for
69+
backward compatibility).
2270
- Service functions now return domain-specific `Result<T, DomainError>` instead
2371
of `Result<T, ServiceError>`.
2472
- Each domain error co-locates its HTTP status-code mapping via a
@@ -28,6 +76,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2876

2977
### Removed
3078

79+
- `bearer_token::Extract` wrapper struct (replaced by `BearerToken` directly).
80+
- `get_optional_logged_in_user` free function (logic moved into extractors).
81+
- `get_claims_from_bearer_token` private method on `Authentication` (inlined).
82+
- `ClaimTokenPepper` / `JwtSigningSecret` / `user_claim_token_pepper` config
83+
keys (replaced by RSA key pair configuration).
3184
- `ServiceError` enum and `ServiceResult` type alias from `src/errors.rs`.
3285
- `http_status_code_for_service_error` and `map_database_error_to_service_error`
3386
helper functions.

adr/007-jwt-system-refactor.md

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# ADR-T-007: Refactor the JWT System
22

3-
**Status:** Phase 3 implemented
3+
**Status:** Phase 4 implemented
44
**Date:** 2026-04-14
55

66
## Context
@@ -340,9 +340,8 @@ phased rollout that subsumes Options A and B.
340340
does not currently need instant revocation badly enough to
341341
justify the infrastructure cost and loss of statelessness.
342342
- **Option E (hybrid revocation):** The `token_generation` column
343-
approach is elegant but adds complexity that can be layered on
344-
later without changing the token format. It remains a valid
345-
follow-up if revocation becomes a priority.
343+
approach was originally deferred but has since been implemented
344+
in Phase 4. See the Phase 4 section below.
346345

347346
### Implementation Phases
348347

@@ -384,7 +383,9 @@ phased rollout that subsumes Options A and B.
384383
authenticated request (the authorization service already does
385384
this via `get_role`) so the token role is advisory only.
386385
- ✅ Enforce a minimum secret length (32 bytes) at config
387-
validation time.
386+
validation time. *(With Phase 3's move to RS256, this is now
387+
enforced implicitly: `EncodingKey::from_rsa_pem` /
388+
`DecodingKey::from_rsa_pem` reject invalid PEM at startup.)*
388389
- **Breaking change:** existing HS256 tokens are invalidated;
389390
users must re-login.
390391

@@ -408,14 +409,23 @@ phased rollout that subsumes Options A and B.
408409
no longer supported. Deployers must generate an RSA key pair
409410
and update their configuration.
410411

411-
#### Future — Optional Revocation (Option E scope)
412-
413-
- Add a `token_generation` column to `torrust_users`.
414-
- Include `gen` in `SessionClaims`; reject stale generations on
415-
verify.
416-
- Increment generation on password change, role change, or ban.
417-
- This phase is independent and can be shipped whenever revocation
418-
becomes a priority.
412+
#### Phase 4 — Optional Revocation (Option E scope) ✅ Implemented
413+
414+
- ✅ Add a `token_generation` column (default `0`) to
415+
`torrust_users`.
416+
- ✅ Include `gen` in `SessionClaims`; reject tokens whose `gen`
417+
is older than the current database value.
418+
- ✅ Increment `token_generation` on password change, role change
419+
(admin grant), and ban.
420+
- ✅ Validation performed in the `Authentication` web layer
421+
(`get_user_id_from_bearer_token`), the `verify_token_handler`,
422+
and the `renew_token` service method.
423+
*Defence in depth:* the generation check is intentionally
424+
repeated at each entry point rather than consolidated into a
425+
single layer, so that no call path can accidentally bypass
426+
revocation.
427+
- **Breaking change:** existing tokens without a `gen` claim will
428+
fail deserialization and be rejected (users re-login once).
419429

420430
### Configuration Migration
421431

@@ -438,10 +448,25 @@ A migration guide will accompany the release that ships Phase 3.
438448
- Deployers must generate and manage an RSA key pair (Phase 3).
439449
A development-mode auto-generated key reduces friction for
440450
local setups.
441-
- Token revocation is **not** included in the initial scope but
442-
the architecture cleanly supports adding it later (Phase 4 /
443-
Option E).
451+
- Token revocation via a `token_generation` counter is included
452+
(Phase 4 / Option E). Password changes, role changes, and bans
453+
increment the counter and invalidate outstanding tokens.
444454
- The centralised `jwt` module makes future algorithm changes
445455
(e.g., migrating to EdDSA) a localised, single-module change.
446456
- External services can verify tokens using only the public key,
447457
enabling zero-trust verification without secret sharing.
458+
459+
## Remaining Issues
460+
461+
- **Problem #11 (`BearerToken` extractor returns `Ok(None)`).**
462+
**Resolved.** The `BearerToken` extractor now implements
463+
`FromRequestParts` directly and **rejects** missing
464+
(`AuthError::TokenNotFound`) or malformed
465+
(`AuthError::TokenInvalid`) `Authorization` headers at the
466+
extraction boundary. The `Extract` wrapper has been removed.
467+
`ExtractLoggedInUser` uses `BearerToken` directly (fails if
468+
missing). `ExtractOptionalLoggedInUser` catches the rejection
469+
and returns `None` for anonymous requests.
470+
`Authentication::get_user_id_from_bearer_token` now takes
471+
`BearerToken` (not `Option<BearerToken>`), eliminating the
472+
`None`-handling indirection.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE torrust_users ADD COLUMN token_generation BIGINT NOT NULL DEFAULT 0
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
ALTER TABLE torrust_users ADD COLUMN token_generation INTEGER NOT NULL DEFAULT 0

src/app.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ pub async fn run(configuration: Configuration, api_version: &Version) -> Running
7474

7575
let database = Arc::new(database::connect(&database_connect_url).await.expect("Database error."));
7676
let json_web_token = Arc::new(JsonWebToken::new(configuration.clone()).await);
77-
let auth = Arc::new(Authentication::new(json_web_token.clone()));
77+
let auth = Arc::new(Authentication::new(json_web_token.clone(), database.clone()));
7878

7979
// Repositories
8080
let category_repository = Arc::new(DbCategoryRepository::new(database.clone()));
@@ -154,6 +154,7 @@ pub async fn run(configuration: Configuration, api_version: &Version) -> Running
154154
let authentication_service = Arc::new(Service::new(
155155
configuration.clone(),
156156
json_web_token.clone(),
157+
database.clone(),
157158
user_repository.clone(),
158159
user_profile_repository.clone(),
159160
user_authentication_repository.clone(),

src/config/v2/auth.rs

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,13 @@ const DEFAULT_PUBLIC_KEY_PATH: &str = "./share/default/jwt/public.pem";
1515

1616
/// Authentication options.
1717
///
18-
/// ## Phase 3 (ADR-T-007)
18+
/// ## JWT signing (ADR-T-007)
1919
///
20-
/// JWT signing has moved from HMAC-HS256 with shared secrets to
21-
/// RS256 (RSA + SHA-256) with a public/private key pair.
20+
/// JWT signing uses RS256 (RSA + SHA-256) with a public/private key
21+
/// pair. Session and email-verification tokens share the same key
22+
/// pair; purpose separation is via the `aud` claim. A per-user
23+
/// `token_generation` counter enables near-instant revocation on
24+
/// password change, role change, or ban.
2225
///
2326
/// Configuration supports two mechanisms (in priority order):
2427
///

src/databases/database.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,13 @@ pub trait Database: Sync + Send {
238238
/// Grant a user the administrator role.
239239
async fn grant_admin_role(&self, user_id: i64) -> Result<(), Error>;
240240

241+
/// Get the current `token_generation` counter for a user.
242+
async fn get_token_generation(&self, user_id: i64) -> Result<u64, Error>;
243+
244+
/// Increment the `token_generation` counter for a user, invalidating
245+
/// all outstanding session tokens.
246+
async fn increment_token_generation(&self, user_id: i64) -> Result<(), Error>;
247+
241248
/// Verify a user's email with `user_id`.
242249
async fn verify_email(&self, user_id: i64) -> Result<(), Error>;
243250

src/databases/mysql.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,30 @@ impl Database for Mysql {
299299
})
300300
}
301301

302+
async fn get_token_generation(&self, user_id: i64) -> Result<u64, database::Error> {
303+
query_as("SELECT token_generation FROM torrust_users WHERE user_id = ?")
304+
.bind(user_id)
305+
.fetch_one(&self.pool)
306+
.await
307+
.map(|(v,): (i64,)| u64::try_from(v).unwrap_or(0))
308+
.map_err(|_| database::Error::UserNotFound)
309+
}
310+
311+
async fn increment_token_generation(&self, user_id: i64) -> Result<(), database::Error> {
312+
query("UPDATE torrust_users SET token_generation = token_generation + 1 WHERE user_id = ?")
313+
.bind(user_id)
314+
.execute(&self.pool)
315+
.await
316+
.map_err(|_| database::Error::Error)
317+
.and_then(|v| {
318+
if v.rows_affected() > 0 {
319+
Ok(())
320+
} else {
321+
Err(database::Error::UserNotFound)
322+
}
323+
})
324+
}
325+
302326
async fn verify_email(&self, user_id: i64) -> Result<(), database::Error> {
303327
query("UPDATE torrust_user_profiles SET email_verified = TRUE WHERE user_id = ?")
304328
.bind(user_id)

src/databases/sqlite.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,30 @@ impl Database for Sqlite {
295295
})
296296
}
297297

298+
async fn get_token_generation(&self, user_id: i64) -> Result<u64, database::Error> {
299+
query_as("SELECT token_generation FROM torrust_users WHERE user_id = ?")
300+
.bind(user_id)
301+
.fetch_one(&self.pool)
302+
.await
303+
.map(|(v,): (i64,)| u64::try_from(v).unwrap_or(0))
304+
.map_err(|_| database::Error::UserNotFound)
305+
}
306+
307+
async fn increment_token_generation(&self, user_id: i64) -> Result<(), database::Error> {
308+
query("UPDATE torrust_users SET token_generation = token_generation + 1 WHERE user_id = ?")
309+
.bind(user_id)
310+
.execute(&self.pool)
311+
.await
312+
.map_err(|_| database::Error::Error)
313+
.and_then(|v| {
314+
if v.rows_affected() > 0 {
315+
Ok(())
316+
} else {
317+
Err(database::Error::UserNotFound)
318+
}
319+
})
320+
}
321+
298322
async fn verify_email(&self, user_id: i64) -> Result<(), database::Error> {
299323
query("UPDATE torrust_user_profiles SET email_verified = TRUE WHERE user_id = ?")
300324
.bind(user_id)

src/errors.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ pub enum AuthError {
3636
#[error("Token invalid.")]
3737
TokenInvalid,
3838

39+
#[error("Token has been revoked. Please sign in again.")]
40+
TokenRevoked,
41+
3942
#[error("Unauthorized action.")]
4043
UnauthorizedAction,
4144

@@ -70,6 +73,7 @@ impl AuthError {
7073
Self::TokenNotFound
7174
| Self::TokenExpired
7275
| Self::TokenInvalid
76+
| Self::TokenRevoked
7377
| Self::LoggedInUserNotFound
7478
| Self::UnauthorizedActionForGuests => StatusCode::UNAUTHORIZED,
7579
Self::InternalServerError | Self::DatabaseError => StatusCode::INTERNAL_SERVER_ERROR,

0 commit comments

Comments
 (0)