Skip to content

refactor!: secure JWT authentication with RS256 and token revocation (ADR-T-007)#853

Merged
da2ce7 merged 10 commits into
torrust:developfrom
peer-cat:feat/jwt-refactor
Apr 16, 2026
Merged

refactor!: secure JWT authentication with RS256 and token revocation (ADR-T-007)#853
da2ce7 merged 10 commits into
torrust:developfrom
peer-cat:feat/jwt-refactor

Conversation

@peer-cat
Copy link
Copy Markdown
Contributor

@peer-cat peer-cat commented Apr 14, 2026

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_pepperauth.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)

@peer-cat peer-cat force-pushed the feat/jwt-refactor branch from d11aad4 to e96e720 Compare April 15, 2026 12:58
…hase 1)

Introduce `src/jwt.rs` — a single module that owns all `jsonwebtoken`
usage (key loading, signing, verification, algorithm selection).

Key changes:

- Move `JsonWebToken` service from `services::authentication` to `jwt`
  module, covering both session and email-verification tokens.
- Move `UserClaims` from `models::user` and `VerifyClaims` from
  `mailer` into `jwt`, co-located with the code that produces and
  consumes them.
- Rename config field `user_claim_token_pepper` → `jwt_signing_secret`
  (with legacy alias accepted during config loading).
- Add `session_token_lifetime_secs` and
  `email_verification_token_lifetime_secs` config options, replacing
  the hard-coded magic numbers.
- Make `sign()` return `Result` instead of panicking on encoding
  failure.
- Make `parse_token()` return `Result<String, AuthError>` instead of
  panicking on malformed `Authorization` headers.
- Propagate the `JsonWebToken` service via `Arc` into `mailer::Service`
  and `user::RegistrationService`, removing their direct `jsonwebtoken`
  crate dependencies.

BREAKING CHANGE: The config key `auth.user_claim_token_pepper` is
renamed to `auth.jwt_signing_secret`. The old key is still accepted
at startup but the canonical name has changed. The settings JSON
response now includes `jwt_signing_secret`,
`session_token_lifetime_secs`, and
`email_verification_token_lifetime_secs` fields.
… and RFC 7519 claims

Split the single shared JWT signing secret into two separate keys:
`session_signing_key` and `email_verification_signing_key`, so a
compromise of one key does not affect the other token type.

Session tokens (`SessionClaims`, née `UserClaims`) now carry proper
RFC 7519 registered claims: `sub` (user ID), `iss` ("torrust-index"),
`aud` ("session"), `iat`, and `exp`. The `role` and `username` fields
are advisory only — the authoritative role is always re-checked from
the database on each authenticated request.

Email-verification tokens (`VerifyClaims`) gain an `aud`
("email-verification") claim and `iat`, and are now validated with
audience + issuer checks by the `jsonwebtoken` library instead of a
manual `iss` string comparison.

Other changes:
- Enforce a 32-byte minimum length on `JwtSigningSecret`.
- Accept `jwt_signing_secret` and `user_claim_token_pepper` as legacy
  serde aliases for `session_signing_key` for backward compatibility.
- Provide `UserClaims` as a type alias so existing imports keep
  compiling.
- Update all config files, doc examples, and tests.

Ref: ADR-T-007
…DR-T-007 Phase 3)

Replace the shared HMAC secret (`session_signing_key` /
`email_verification_signing_key`) with an RSA-2048 key pair for
all JWT operations.

Key changes:

- Sign tokens with `EncodingKey` (private) and verify with
  `DecodingKey` (public), both resolved once at startup from PEM
  files or inline PEM configuration.
- Include a `kid` (Key ID) header derived from the SHA-256 hash
  of the public key, preparing for future key rotation.
- Ship a development key pair at `share/default/jwt/` with a loud
  startup warning when it is used.
- Remove `JwtSigningSecret`, the mandatory-option check for
  `auth.session_signing_key`, and all three serde aliases.
- Verification (`verify`, `verify_email_token`) is now synchronous
  since keys are pre-loaded — no config lock needed per request.
- Update all config files, container scripts, tests, and docs to
  use `private_key_path` / `public_key_path`.

BREAKING CHANGE: The `auth.session_signing_key` and
`auth.email_verification_signing_key` config fields are removed.
Deployers must provide an RSA key pair via `auth.private_key_path` /
`auth.public_key_path` (or the inline `_pem` variants). Existing
HS256 tokens will be rejected after upgrade.
@peer-cat peer-cat force-pushed the feat/jwt-refactor branch from e96e720 to b7ec439 Compare April 15, 2026 13:14
@peer-cat peer-cat marked this pull request as ready for review April 15, 2026 13:14
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 15, 2026

Codecov Report

❌ Patch coverage is 56.46766% with 175 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.54%. Comparing base (20ab763) to head (6126eec).
⚠️ Report is 11 commits behind head on develop.

Files with missing lines Patch % Lines
src/databases/mysql.rs 0.00% 43 Missing ⚠️
src/bin/generate_auth_keypair.rs 0.00% 41 Missing ⚠️
src/databases/sqlite.rs 65.11% 15 Missing ⚠️
src/config/v2/mod.rs 0.00% 4 Missing and 8 partials ⚠️
src/services/authentication.rs 42.10% 6 Missing and 5 partials ⚠️
src/config/v2/auth.rs 73.68% 8 Missing and 2 partials ⚠️
src/jwt.rs 91.07% 7 Missing and 3 partials ⚠️
src/mailer.rs 40.00% 9 Missing ⚠️
src/services/user.rs 68.18% 6 Missing and 1 partial ⚠️
src/web/api/server/v1/extractors/user_id.rs 0.00% 7 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #853      +/-   ##
===========================================
- Coverage    53.76%   53.54%   -0.23%     
===========================================
  Files          116      118       +2     
  Lines         6182     6441     +259     
  Branches      6182     6441     +259     
===========================================
+ Hits          3324     3449     +125     
- Misses        2776     2893     +117     
- Partials        82       99      +17     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…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).
Copy the RS256 key pair into the volume-mounted storage path during
E2E install for both SQLite and MySQL setups, aligning the container
environment with the HMAC→RS256 migration (ADR-T-007 Phase 3).

Also make `wait_for_container_to_be_healthy.sh` directly invocable by
forwarding arguments to the function it defines.
…-T-007 Phase 5)

Remove the development RSA key pair shipped at `share/default/jwt/`
and replace it with on-demand ephemeral key generation. When no
`private_key_path`/`public_key_path` or inline PEM values are
configured, the server auto-generates an RSA-2048 key pair in memory
at startup via the `rsa` crate. Sessions work immediately but do not
survive restarts — deployers who want persistent sessions supply
their own key pair.

Key changes:

- `Auth::resolve_{private,public}_key_pem()` now return `Option`
  instead of panicking, letting `JsonWebToken::new()` decide whether
  to use host-supplied or ephemeral keys.
- `JsonWebToken::new()` matches on (Some, Some) / (None, None) and
  panics on mismatched key configuration.
- `generate_ephemeral_key_pair()` offloads CPU-heavy RSA generation
  to a blocking Tokio task.
- Default config no longer includes key paths; all container configs,
  e2e scripts, compose.yaml, and docs updated accordingly.
- Tests now exercise the ephemeral code path by default.
- ADR-T-007 updated with Phase 5/6 design, migration notes, and
  testing strategy.�
@peer-cat peer-cat marked this pull request as draft April 15, 2026 18:16
@peer-cat peer-cat marked this pull request as ready for review April 15, 2026 18:52
…R-T-007 Phase 6)

Add the `torrust-generate-auth-keypair` binary that generates an
RSA-2048 key pair and writes both PEM blocks to stdout. The tool
refuses to run when stdout is a terminal, uses structured JSON
tracing on stderr, and supports a `--debug` flag for verbose output.

The container entry script now auto-generates persistent auth keys on
first boot into `/etc/torrust/index/auth/` with restrictive
permissions (0400 private, 0440 public). A `mktemp` + `trap` pattern
ensures key material is never world-readable and the temp file is
cleaned up on interruption.

Changes:
- New binary: src/bin/generate_auth_keypair.rs
- Containerfile: copy new binary in both debug and release stages
- entry_script_sh: key-generation block before `exec su-exec`
- All container configs: set auth key paths to /etc/torrust/index/auth/
- ADR-007: update Phase 6 to reflect implementation
- docs/containers.md: document auth/ directory and volume layout
- README: note container auto-generation for new users
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 57 out of 59 changed files in this pull request and generated 7 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/databases/mysql.rs
Comment thread src/web/api/server/v1/contexts/user/handlers.rs Outdated
Comment thread src/web/api/server/v1/auth.rs
Comment thread src/web/api/server/v1/extractors/bearer_token.rs
Comment thread src/config/v2/mod.rs Outdated
Comment thread tests/e2e/environment.rs
Comment thread src/databases/sqlite.rs
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 57 out of 59 changed files in this pull request and generated 9 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/web/api/server/v1/contexts/user/handlers.rs
Comment thread src/web/api/server/v1/contexts/user/mod.rs Outdated
Comment thread src/web/api/server/v1/contexts/torrent/mod.rs
Comment thread src/web/api/server/v1/contexts/settings/mod.rs
Comment thread src/databases/mysql.rs Outdated
Comment thread src/web/api/server/v1/contexts/tag/mod.rs
Comment thread src/web/api/server/v1/contexts/proxy/mod.rs
Comment thread src/web/api/server/v1/contexts/category/mod.rs
Comment thread src/config/v2/auth.rs
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 57 out of 59 changed files in this pull request and generated 6 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/web/api/server/v1/auth.rs Outdated
Comment thread src/web/api/server/v1/contexts/user/handlers.rs Outdated
Comment thread src/services/authentication.rs Outdated
Comment thread src/services/user.rs Outdated
Comment thread src/services/user.rs
Comment thread src/services/user.rs Outdated
…DR-T-007)

Introduce atomic database methods that combine state changes with
`token_generation` bumps in a single transaction, closing the race
window where a password change or ban could commit but the generation
increment could fail — leaving revoked credentials with valid tokens:

- `change_user_password_and_revoke_tokens` (transaction)
- `ban_user_and_revoke_tokens` (transaction)
- `grant_admin_role_and_revoke_tokens` (single UPDATE)

Harden token-generation checking from `<` to `!=` (exact match) at
all three validation sites so tokens are also rejected when the
database generation *decreases* (e.g. restore from backup).

Add defence-in-depth `is_user_banned` checks after generation
validation — if `token_generation` somehow matches despite an active
ban, the ban table catches it.

Other fixes included in this changeset:

- `BearerToken::value()` → `as_str()`: return `&str` instead of
  cloning.
- `parse_token`: require the space in `"Bearer "` prefix
  (`strip_prefix("Bearer")` → `strip_prefix("Bearer ")`).
- `get_token_generation`: distinguish `RowNotFound` from other
  errors; replace `unwrap_or(0)` with proper error propagation.
- `Settings::remove_secrets_from_settings`: conditionally redact
  `Option` key fields instead of unconditionally overwriting `None`
  with `Some("***")`. Also redact public key pem/path.
- Warn when configured key paths don't exist (falling back to
  ephemeral keys).
- Replace hardcoded JWT tokens in doc examples with `<JWT_TOKEN>`.
- Fix doc typo: "JWT is not invalid" → "JWT is invalid".

Condense ADR-T-007 and add Phase 7 (consolidate session validation)
design.
…ADR-T-007 Phase 7)

Introduce `JsonWebToken::validate_session` as the sole entry point for
session-token validation — JWT signature/expiry verification, token-
generation counter check, and banned-user rejection all happen in one
place.

Three call sites that previously re-implemented this sequence now
delegate to `validate_session`:
  - `Authentication::get_user_id_from_bearer_token`
  - `verify_token_handler`
  - `authentication::Service::renew_token`

This removes ~45 lines of duplicated validation logic and eliminates
the private `Authentication::validate_token_generation` helper.

Mark ADR-T-007 Phase 7 as complete.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 57 out of 59 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/jwt.rs Outdated
Comment thread src/services/authentication.rs
Comment thread src/web/api/server/v1/auth.rs
Comment thread src/services/user.rs Outdated
- Parse Bearer scheme case-insensitively per RFC 7235 §2.1
- Replace `parse_from_str` + `expect` with a `LazyLock` constant for
  the permanent-ban expiry date, removing two panic sites
- Propagate `is_user_banned` errors with `?` instead of swallowing
  them via `unwrap_or(false)`
- Use `saturating_sub` for token-expiry arithmetic to avoid underflow
@peer-cat
Copy link
Copy Markdown
Contributor Author

ACK 6126eec

1 similar comment
@da2ce7
Copy link
Copy Markdown
Contributor

da2ce7 commented Apr 16, 2026

ACK 6126eec

@da2ce7 da2ce7 merged commit 8310fb3 into torrust:develop Apr 16, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants