diff --git a/.cargo/config.toml b/.cargo/config.toml new file mode 100644 index 0000000..a81ce1d --- /dev/null +++ b/.cargo/config.toml @@ -0,0 +1,23 @@ +# Local development configuration. +# +# On Windows a running executable cannot be relinked, and `cargo test` always rebuilds the +# package's binaries — an integration test may reference `env!("CARGO_BIN_EXE_ism")`, which is +# resolved at compile time, so Cargo has to produce `ism.exe` before it can compile a single test. +# A dev server started from `target/debug/ism.exe` therefore blocks every `cargo test` and +# `cargo build` for as long as it runs, with `os error 5`. +# +# The fix is to keep the long-running server out of the shared build directory: +# +# cargo serve instead of `cargo run` +# cargo attack the black-box auth suite in tests/auth_attacks.rs +# +# `cargo serve` builds into `target/server/`, leaving `target/` free for tests, clippy and the +# IDE. Extra arguments are appended, so `cargo attack rejects_kid` filters by test name and +# `cargo attack -- --test-threads=1` still works. +# +# The first `cargo serve` rebuilds every dependency into the new directory, librdkafka included — +# that one is slow. It happens once. + +[alias] +serve = ["run", "--target-dir", "target/server"] +attack = ["test", "--test", "auth_attacks", "--", "--nocapture"] diff --git a/.claude/rules/handlers.md b/.claude/rules/handlers.md index 2a72b04..93e2ec0 100644 --- a/.claude/rules/handlers.md +++ b/.claude/rules/handlers.md @@ -12,25 +12,49 @@ paths: ## Auth Extraction -Every protected handler extracts the caller's identity via: +Every protected handler takes the caller as: ```rust -Extension(claims): Extension +use crate::auth::CurrentUser; + +user: CurrentUser // = KeycloakToken ``` -The caller's UUID is available as `claims.sub`. +`CurrentUser` is the full validated token. The caller's id is `user.subject` (`Uuid`, `Copy`); +also available are `user.roles`, `user.extra.profile.preferred_username`, `user.extra.email` and +the standard JWT claims. + +Name the binding `user` and use `user.subject` inline. Only bind a local `let user_id = +user.subject;` when the id outlives the handler body — a `move` closure or a spawned task — so the +capture is a `Copy` id rather than the whole token (see `messaging/notifications.rs`). + +Path parameters are named after what they refer to (`target_id`, `friend_id`, `sender_id`, +`invited_user_id`), never `user_id`. + +**Roles** are not enforced anywhere yet. To require one in a handler: + +```rust +expect_role!(&user, AppRole::Admin); // returns 403 early if absent +``` + +See `../../.docs/auth.md`. ## Return Type -All handlers return `Result, HttpError>`. On success: `Ok(Json(...))`. On failure: `Err(HttpError)`. +All handlers return `AppResponse>` (= `Result, AppError>`, from +`crate::core::errors`). On success: `Ok(Json(...))`. On failure: `Err(AppError)`. -`HttpError` serializes to: +`AppError` serializes to: ```json -{ "status": 404, "errorCode": "NOT_FOUND", "message": "...", "timestamp": "...", "path": "/api/..." } +{ "timestamp": "...", "status": 404, "error": "Not Found", "message": "...", "path": "/api/v1/...", "errorCode": "CONTENT_NOT_FOUND" } ``` The `path` field is injected automatically by the `inject_request_path` middleware — do not set it manually. +Pick the variant by audience: `Validation` / `NotFound` / `Forbidden` pass their message through to +the caller; `Database` / `Cache` / `Serialization` / `S3` / `Processing` are logged in full and +answered with a generic message. + ## No unwrap() -Never use `unwrap()` or `expect()` in handlers. Propagate errors with `?` and convert via `HttpError`. \ No newline at end of file +Never use `unwrap()` or `expect()` in handlers. Propagate errors with `?` and convert via `AppError`. \ No newline at end of file diff --git a/.docs/auth.md b/.docs/auth.md new file mode 100644 index 0000000..e16040c --- /dev/null +++ b/.docs/auth.md @@ -0,0 +1,277 @@ +# Authentication + +Every route under `/api/v1` is protected by a Keycloak JWT. This document covers how the +middleware is wired, what handlers get, and the extension points that exist but are not currently +used. + +The code lives in `../src/auth`. Submodules are private; everything a caller needs is re-exported +from `crate::auth` directly. + +## How a request is authenticated + +``` +router::init_router + └─ ServiceBuilder + ├─ TraceLayer + ├─ CorsLayer + ├─ KeycloakAuthLayer ← built in router::init_auth + └─ DefaultBodyLimit +``` + +| Step | File | What happens | +|---|---|---| +| 1 | `extract.rs` | The raw JWT is pulled out of the `Authorization: Bearer …` header. | +| 2 | `instance.rs` | The realm's signing keys, fetched at startup via OIDC discovery and refreshed only on demand, are looked up by the token's `kid`. A `kid` outside the cached set is the one case that re-runs discovery — see [Key rotation](#key-rotation). | +| 3 | `decode.rs` | Signature, algorithm, issuer, audience, expiry, `nbf`, token type and authorized party are checked against the `ValidationPolicy`. | +| 4 | `service.rs` | On success the `KeycloakToken` is inserted into the request extensions; on failure the request is answered with an error and never reaches the handler. | + +The `ValidationPolicy` is built once at startup from the `[token_issuer]` config section, and a +misconfiguration (unknown algorithm, symmetric algorithm, empty audience list) panics at startup +rather than turning into a blanket 401 at runtime. + +### Expiry and clock drift + +Expiry is checked twice per request — once by `jsonwebtoken` as part of signature validation, once +explicitly by `KeycloakToken::assert_not_expired` — and the stricter of the two decides. Both read +the same `EXPIRY_LEEWAY_SECS` (`decode.rs`, currently **5 seconds**), so a token is accepted up to +five seconds past its `exp`. The same leeway applies to `nbf`. + +That window exists to absorb clock drift between the Keycloak host and this one; it deliberately +replaces `jsonwebtoken`'s 60-second default. If you change it, change the constant — never one of +the two call sites, or the looser check silently becomes dead code. `security_tests.rs` pins the +boundary from both sides (expired by 2s must pass, expired by 10s must fail). + +Beyond that window, correct expiry depends on both hosts having a synchronised clock. Neither +`chrono` nor any other date library affects this: every expiry check ultimately reads the host's +`CLOCK_REALTIME`, so NTP on both machines is the actual control. + +### Startup + +`KeycloakAuthInstance::new` runs the first OIDC discovery to completion and returns an error if it +does not succeed; `router::init_auth` turns that into a panic. Without discovered keys not one +token can be verified, and nothing re-runs discovery on a timer, so a process that started anyway +would answer every authenticated request with a 503 for as long as it stayed up — a crash an +orchestrator can restart is the more useful outcome. + +`KeycloakConfig::startup_retry` (default 18 attempts, 5s apart ≈ 90s) is deliberately patient: in a +compose stack Keycloak is routinely slower to become ready than ISM is. It only delays the abort — a +wrong realm or an unreachable host still brings the process down. + +### Key rotation + +There is no refresh timer and no background task. Discovery re-runs on exactly one condition: the +token's header names a `kid` that the cached key set does not contain. `decode_and_validate` then +refreshes once and retries within the same request, so rotation costs added latency on a single +request and is never visible to a client. + +**Why `kid` and not the signature result.** The trigger used to key off `jsonwebtoken`'s +`ErrorKind`, which cannot answer the question. `InvalidSignature` has a single construction site in +the crate and means only "the verify operation returned false" — a payload-tampered token and a +token signed by a rotated key produce a byte-identical error. That is inherent to what a signature +is, not a gap in the crate. The result was that *every* invalid token reached for Keycloak, expired +sessions included. The `kid` decides it before any crypto runs, because Keycloak stamps a per-key +thumbprint and does not reuse it across rotations: + +| Token | Outcome | +|---|---| +| No `kid` in the header | 401. Nothing to match a key against, and not something Keycloak issues. | +| `kid` is cached | Verified against exactly those keys. Any failure is terminal — we hold the key it names, so rediscovery cannot change the answer. | +| `kid` is unknown | One refresh, then retry. Still unknown afterwards → 401, without a single public-key operation spent on it. | + +Three bounds keep the trigger safe even though a caller controls it: + +- `min_refresh_interval` (default 30s) rate-limits discovery, so a flood of fabricated `kid`s + cannot become a request storm against Keycloak. +- `refresh_timeout` (default 2s) caps how long a request is held while a refresh runs, and + `refresh_retry` (default one attempt) keeps a slow Keycloak from being waited on. Previously the + in-request refresh inherited the full startup retry budget and could stall a request ~40s. +- A refresh that fails leaves the previously discovered keys in place (`action.rs`). Overwriting + them with the failure took authentication down until the next successful discovery — which, with + no timer, could be indefinitely. + +## What handlers get + +Handlers take `CurrentUser`, which is a type alias for `KeycloakToken` — the whole +validated token, not just an id: + +```rust +use crate::auth::CurrentUser; + +pub async fn handle_get_friends( + State(state): State>, + user: CurrentUser, + Query(params): Query, +) -> AppResponse>> { + let results = UserService::get_friends(state, &user.subject, /* … */).await?; + Ok(Json(results)) +} +``` + +| Field | Type | | +|---|---|---| +| `subject` | `Uuid` | the Keycloak user id — what nearly every handler passes to a service | +| `roles` | `Vec>` | realm and client roles | +| `extra.profile` | `Profile` | `preferred_username` | +| `extra.email` | `Email` | `email`, `email_verified` | +| `expires_at`, `issued_at` | `DateTime` | | +| `issuer`, `audience`, `authorized_party`, `jwt_id` | | standard JWT claims | + +`subject` is `Copy`, so `user.subject` inline is free. Bind a local only when the id has to +outlive the handler body — a `move` closure or a spawned task — so the capture is the id rather +than the whole token: + +```rust +pub async fn websocket_server_events( + websocket: WebSocketUpgrade, + user: CurrentUser, + Query(params): Query, +) -> impl IntoResponse { + let user_id = user.subject; + websocket.on_upgrade(move |socket| handle_socket(socket, user_id, params.last_seq)) +} +``` + +Because `CurrentUser` pins the `` generic to `AppRole`, no handler and no route ever spells the +role type out. + +## Errors + +`AuthError` covers everything that can go wrong. It is logged in full server-side and then +sanitised before it reaches the client — the response never says *which* check failed, only that +authentication failed. The wire format is the same `ErrorResponse` every other endpoint produces: + +```json +{ + "timestamp": "…", + "status": 401, + "error": "Unauthorized", + "message": "…", + "path": "/api/v1/users/friends", + "errorCode": "UNAUTHORIZED" +} +``` + +## Roles + +`AppRole` (`auth/app_role.rs`) is the realm's role set and the concrete `Role` the whole +application is generic over: + +```rust +pub enum AppRole { + Admin, // ADMIN + User, // USER + LocalGuide, // LOCAL_GUIDE + Unknown(String), +} +``` + +`Unknown` is not an error case. Keycloak gives every account roles nobody here asked for — +`offline_access`, `uma_authorization`, `default-roles-`, plus the client roles of the +`account` client. Those land in `Unknown`, keep their original name for log messages, and never +match a check. + +Role names are matched **case-sensitively** against the realm spelling, and `Display` emits that +same spelling — so `AppRole::from(role.to_string())` round-trips for every variant. + +### Enforcing a role + +**No route enforces one today.** Every authenticated user may call every endpoint. Two ways to +change that: + +**Per handler**, with the macros from `role.rs`. They are `#[macro_export]`ed and therefore live +at the crate root, but resolve `ExpectRoles` through the `auth` facade: + +```rust +use crate::auth::{AppRole, CurrentUser}; +use crate::expect_role; + +pub async fn admin_only(user: CurrentUser) -> Response { + expect_role!(&user, AppRole::Admin); // returns 403 early if absent + StatusCode::OK.into_response() +} +``` + +Available: `expect_role!`, `expect_roles!`, `not_expect_role!`, `not_expect_roles!`. Each returns +early from the handler with an error response when the assertion fails — 403 with +`errorCode: "INSUFFICIENT_PERMISSIONS"`. The role that was missing is logged server-side but never +returned to the caller. + +**Layer-wide**, when every route behind the layer needs the same role: + +```rust +KeycloakAuthLayer::::builder() + .instance(instance) + .required_roles(vec![AppRole::Admin]) + .build() +``` + +Be careful with this one: it applies to every protected route at once, so requiring a role that +not every account actually holds locks those users out of the whole API. + +### Adding a role + +Add the variant, its realm spelling in `as_str`, and the `From` arm — the tests in +`app_role.rs` cover the mapping and the round-trip. Nothing else changes: `CurrentUser` and +`init_auth` already carry the type. + +## Extension points + +The two sections below describe capabilities the middleware has but ISM does not currently use. + +### Passthrough modes + +`KeycloakAuthLayer::passthrough_mode` decides what happens to a request that fails validation: + +- `PassthroughMode::Block` — answer immediately with an error response. This is the default and + what ISM uses. On success the handler finds a `KeycloakToken` in the request extensions. +- `PassthroughMode::Pass` — always call the handler, and store a `KeycloakAuthStatus` + (`Success(token)` or `Failure(error)`) in the extensions instead. Use this when a route needs + fine-grained handling of the failure, or when a deeper layer might still authenticate the user. + +Note that `CurrentUser` assumes `Block`: under `Pass` there is no `KeycloakToken` extension to +read, and the extractor rejects with 401. + +### Custom token extractors + +By default the layer looks for `Authorization: Bearer `. `token_extractors` takes a list of +`TokenExtractor` implementations, tried in order; the first one to yield a token wins and the rest +are skipped. An empty list fails closed — no extractor produces a token, so every request is +rejected with 401. + +Two implementations ship in `extract.rs`: + +- `AuthHeaderTokenExtractor` — the `Authorization` header. The default. +- `QueryParamTokenExtractor` — a query parameter, `?token=…` by default. + +```rust +KeycloakAuthLayer::::builder() + .instance(instance) + .validation_policy(policy) + .token_extractors(vec![ + Arc::new(AuthHeaderTokenExtractor::default()) as Arc, + Arc::new(QueryParamTokenExtractor::default()), + Arc::new(QueryParamTokenExtractor::extracting_key("jwt")), + ]) + .build() +``` + +**Security note on query parameters**: URLs end up in access logs, proxy logs and `Referer` +headers, so a token passed this way is far more likely to be recorded somewhere than one in a +header. The reason it exists at all is `/api/wss`: the browser `WebSocket` API cannot set request +headers, so a browser client has no way to send `Authorization` on the upgrade request. If that +becomes necessary, prefer a short-lived single-use ticket over the access token itself. + +## Tests + +`../src/auth/security_tests.rs` runs the validation path against deliberately malformed and +malicious tokens — `alg=none`, RS256→HS256 confusion, tampered payloads, foreign issuers and +audiences, expired and not-yet-valid tokens, ID and refresh tokens replayed as access tokens, +non-UUID subjects. It signs with an in-file test key and touches no network. + +It also pins the rediscovery trigger, which is reachable by anyone who can send a request: a +missing `kid`, a forgery reusing a known `kid`, and an expired token must all be terminal, and only +an unknown `kid` may cost a discovery. Those tests exercise the lookup directly, so they stay +network-free too. + +**When you add a validation rule, add the attack it defeats.** A rule with no test that failed +before it existed is a rule nobody can prove still works. diff --git a/.docs/share-targets.md b/.docs/share-targets.md new file mode 100644 index 0000000..c85fb75 --- /dev/null +++ b/.docs/share-targets.md @@ -0,0 +1,203 @@ +# Share Targets — Frontend Integration Guide + +The **Share Targets** feature powers a "share to chat" sheet (think Instagram's share +sheet): the user picks where to send a piece of content — an existing chat or a friend +they don't have a chat with yet — and the client delivers it there. + +This endpoint **only returns the list of destinations**. Actually delivering the content +is a separate, existing call (`send-msg` or `create-room`) that depends on the kind of +target the user picked. See [Delivering content](#delivering-content-to-a-target). + +--- + +## Endpoint + +``` +GET /api/rooms/share-targets +``` + +Protected — requires a valid Keycloak bearer token like every `/api/*` route. The list +is always scoped to the authenticated caller (their friends and their rooms). + +### Query parameters + +| Param | Type | Required | Description | +|----------|----------|----------|-------------| +| `name` | string | no | Case-insensitive filter. Matches the friend's display name (1-1) or the group room name. | +| `cursor` | string | no | Opaque pagination cursor. Omit for the first page; pass back the `cursor` from the previous response for the next page. | +| `limit` | integer | no | Page size. Clamped server-side to `[1, 50]`, defaults to `20`. Never assume the server honored your exact value. | + +### Response + +`200 OK` with a standard cursor-paginated envelope: + +```jsonc +{ + "cursor": "eyJwaGFzZSI6ImFjdGl2ZSIsLi4u", // pass to the next request; null = no more pages + "content": [ /* array of ShareTarget */ ] +} +``` + +- `cursor: null` means you've reached the end of the list — stop paging. +- `content` is the page of share targets, already in display order (see + [Ordering](#ordering)). + +--- + +## The `ShareTarget` object + +Every item in `content` looks like this: + +```jsonc +{ + "name": "Alice", // string | null — see below + "imageUrl": "https://.../a.png", // string | null — avatar (user) or room image (group) + "target": { /* ShareTargetRef */ } +} +``` + +| Field | Type | Notes | +|------------|-----------------|-------| +| `name` | string \| null | Friend's display name (1-1) or the room name (group). **Can be `null`** for an unnamed group room — render a fallback (e.g. participant names or a placeholder). | +| `imageUrl` | string \| null | Avatar / room image. `null` ⇒ render a placeholder. | +| `target` | object | Tells you *how* to deliver content. Discriminated by `kind`. See below. | + +### `target` — the `ShareTargetRef` + +This is a tagged union; **switch on `kind`**: + +#### `kind: "room"` — an existing chat + +The destination already exists (a group, or a friend you already have a 1-1 chat with). +Send straight into it. + +```jsonc +{ + "kind": "room", + "roomId": "550e8400-e29b-41d4-a716-446655440000", + "roomType": "Single" // "Single" | "Group" +} +``` + +#### `kind: "user"` — a friend with no chat yet + +There is no room to send to yet. You must **create the 1-1 room first**, then send into +the room you get back. + +```jsonc +{ + "kind": "user", + "userId": "7b2d1f90-1c3e-4a55-9b21-0f8e2a4c1d77" +} +``` + +> Rule of thumb: `kind === "room"` → one call (`send-msg`). +> `kind === "user"` → two calls (`create-room`, then `send-msg` — or embed the content as +> the room's first message, see below). + +--- + +## Ordering + +The list is **two-phase**, and the server stitches both phases into a single paginated +stream — you do not need to merge anything client-side, just render `content` in order: + +1. **Active targets first** — group rooms you're in and friends you already chat 1-1 with, + sorted by most recent activity (newest first). These always come back as + `kind: "room"`. +2. **Inactive targets after** — friends you have *no* 1-1 room with yet, sorted + alphabetically by display name. These come back as `kind: "user"`. + +A page near the boundary may contain the tail of the active section followed by the start +of the inactive section. The cursor transparently tracks which phase the next page resumes +in — just keep passing the returned `cursor` back. + +Every friend appears in **exactly one** of the two sections (either you have a 1-1 room +with them or you don't), so there are no duplicates across pages. + +--- + +## Pagination + +Standard ISM cursor pagination — there are no `page`/`pageSize` params anywhere. + +``` +1. GET /api/rooms/share-targets?limit=20 +2. render content; if response.cursor != null: +3. GET /api/rooms/share-targets?limit=20&cursor= +4. repeat until cursor == null +``` + +The `cursor` is an opaque base64 string — do not parse, construct, or persist assumptions +about it. Combine it with `name` (the filter is re-applied on every page) and `limit` +consistently across a paging session. + +--- + +## Delivering content to a target + +Once the user taps a target, deliver the shared content based on `target.kind`. + +### Existing room (`kind: "room"`) + +`POST /api/send-msg` with the target's `roomId`: + +```jsonc +{ + "chatRoomId": "550e8400-e29b-41d4-a716-446655440000", + "msgType": "Media", // "Text" | "Media" | "Reply" + "msgBody": { + "mediaUrl": "https://app.example.com/post/123", + "mediaType": "link" + } +} +``` + +For a plain text share use `msgType: "Text"` with `msgBody: { "text": "..." }`. + +### Friend without a room (`kind: "user"`) + +`POST /api/rooms/create-room`. The caller must include **their own id** in +`invitedUsers` alongside the friend. You can optionally embed the shared content as the +room's `firstMessage` so it is delivered atomically with room creation (and pushed to the +recipient in the `NewRoom` broadcast event) — no separate `send-msg` needed: + +```jsonc +{ + "roomType": "Single", + "roomName": null, + "invitedUsers": ["", "7b2d1f90-1c3e-4a55-9b21-0f8e2a4c1d77"], + "firstMessage": { // optional; Text or Media only (never Reply) + "mediaUrl": "https://app.example.com/post/123", + "mediaType": "link" + } +} +``` + +The response is the created `ChatRoomDto` (includes the new `id`). If you did **not** use +`firstMessage`, follow up with `POST /api/send-msg` using that `id` as `chatRoomId`. + +> `firstMessage` accepts only `Text` or `Media` bodies — a brand-new room has no prior +> messages, so a `Reply` is rejected. + +--- + +## Field validation limits (for the delivery calls) + +| Field | Limit | +|----------------------|-------| +| Text body `text` | 1–4000 characters | +| Media `mediaUrl` | 1–250 characters | +| Media `mediaType` | 1–80 characters | + +--- + +## Quick reference + +| Concern | Value | +|----------------------|-------| +| List endpoint | `GET /api/rooms/share-targets` | +| Params | `name?`, `cursor?`, `limit?` (clamped to 1–50, default 20) | +| Response | `{ cursor: string\|null, content: ShareTarget[] }` | +| Deliver to room | `POST /api/send-msg` (`kind: "room"`) | +| Deliver to friend | `POST /api/rooms/create-room` then send, or embed `firstMessage` (`kind: "user"`) | diff --git a/CLAUDE.md b/CLAUDE.md index 10bfc2d..85187c5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,7 +62,7 @@ Set `DATABASE_URL` in `.env` for the sqlx CLI. The `.sqlx/` directory holds pre- ``` Routes (router.rs) - ↓ Keycloak JWT middleware → injects KeycloakClaims into request extensions + ↓ Keycloak JWT middleware → injects KeycloakToken into request extensions Handlers (rooms/handler.rs, messaging/handler.rs, users/handler.rs) ↓ Services (room_service.rs, timeline_service.rs, message_service.rs, user_service.rs) @@ -120,7 +120,7 @@ BroadcastChannel::get().unsubscribe(user_id).await; - `send_event` / `send_event_to_all` assign a monotonic **per-user** `seq` (Redis `INCR`), cache durable events in a per-user Redis Stream (`user_notifications:{id}`, entry ID `-0`, length-capped via `XADD ... MAXLEN ~ N` — no background cleanup), and fall back to Kafka push notifications for offline users. - **Ephemeral** events (`NotificationEvent::is_ephemeral()`) get no `seq` and are never cached — live-only (e.g. `Resync`, future typing indicators). - Push notifications are only sent for: `ChatMessage`, `FriendRequestReceived`, `NewRoom`. -- Wire envelope: `{ v, seq, type, createdAt, ...payload }`. Clients reconnect with `?last_seq=` on `/api/sse` and `/api/wss`; the server replays missing durable events or emits a `Resync` when the gap was trimmed out of the retained window. See `docs/streaming-sequencing.md`. +- Wire envelope: `{ v, seq, type, createdAt, ...payload }`. Clients reconnect with `?last_seq=` on `/api/v1/sse` and `/api/v1/wss`; the server replays missing durable events or emits a `Resync` when the gap was trimmed out of the retained window. See `docs/streaming-sequencing.md`. **`NotificationEvent` variants** (defined in `broadcast/notification.rs`): @@ -142,12 +142,19 @@ All data lives in PostgreSQL. SQLx macros provide compile-time query type-checki For function signatures involving transactions or shared executors, follow `docs/sqlx-executor-pattern.md` — this documents when to use `impl Executor<'_, Database = Postgres>` vs `&PgPool` vs `&mut PgTransaction`. -### Authentication +### Authentication (`auth/`) -Keycloak middleware validates the JWT on every protected request (JWKS endpoint cached). Valid tokens inject `KeycloakClaims` into request extensions. Handlers extract the caller's UUID via: +Keycloak middleware validates the JWT on every protected request (JWKS cached, refreshed on demand when a token fails to verify). Valid tokens inject a `KeycloakToken` into request extensions. + +Submodules of `auth` are private; everything a caller needs is re-exported from `crate::auth` directly. Handlers take the caller as: ```rust -Extension(claims): Extension +user: CurrentUser // = KeycloakToken ``` +`CurrentUser` is the whole validated token: `user.subject` (the caller's `Uuid`), `user.roles`, `user.extra.profile.preferred_username`, `user.extra.email`, plus `expires_at` / `issued_at` / `issuer` / `audience` / `authorized_party` / `jwt_id`. + +**Roles**: `AppRole` (`auth/app_role.rs`) is the realm's role set — `Admin`, `User`, `LocalGuide`, and `Unknown(String)` for everything Keycloak hands out that ISM has no rule for. It is the concrete `Role` the whole app is generic over, so `` appears nowhere. No route enforces a role yet; assert one in a handler with `expect_role!(&user, AppRole::Admin)` or layer-wide via `required_roles`. + +See `.docs/auth.md` for the full request path, roles, passthrough modes and custom token extractors. ### Cursor Pagination (`core/cursor.rs`) @@ -185,53 +192,56 @@ Existing cursor types: **Read Receipts**: - `last_message_read_at` per (user, room) on `chat_room_participant` -- Updated via `POST /api/rooms/{id}/mark-read`; broadcast as `UserReadChat` so all user devices sync +- Updated via `POST /api/v1/rooms/{room_id}/mark-read`; broadcast as `UserReadChat` so all user devices sync ### Routing ``` GET /health -POST /api/rooms/create-room -GET /api/rooms -GET /api/rooms/search -GET /api/rooms/{id} -GET /api/rooms/{id}/detailed -GET /api/rooms/{id}/users -GET /api/rooms/{id}/timeline -POST /api/rooms/{id}/leave -POST /api/rooms/{id}/invite/{user_id} -POST /api/rooms/{id}/upload-img -POST /api/rooms/{id}/mark-read -GET /api/rooms/{id}/read-states - -POST /api/send-msg -GET /api/notifications -GET /api/notifications/cursor -GET /api/sse -ANY /api/wss - -GET /api/users/{user_id} -GET /api/users/search -GET /api/users/friends -GET /api/users/friends/requests -POST /api/users/friends/add/{user_id} -POST /api/users/friends/accept-request/{sender_id} -DELETE /api/users/friends/reject-request/{sender_id} -DELETE /api/users/friends/{friend_id} -POST /api/users/ignore/{user_id} -DELETE /api/users/ignore/{user_id} +POST /api/v1/rooms/create-room +GET /api/v1/rooms +GET /api/v1/rooms/search +GET /api/v1/rooms/share-targets +GET /api/v1/rooms/{room_id} +GET /api/v1/rooms/{room_id}/detailed +GET /api/v1/rooms/{room_id}/users +GET /api/v1/rooms/{room_id}/timeline +POST /api/v1/rooms/{room_id}/leave +POST /api/v1/rooms/{room_id}/invite/{user_id} +POST /api/v1/rooms/{room_id}/upload-img +POST /api/v1/rooms/{room_id}/mark-read +GET /api/v1/rooms/{room_id}/read-states + +POST /api/v1/send-msg +GET /api/v1/notifications +GET /api/v1/notifications/cursor +GET /api/v1/sse +ANY /api/v1/wss + +GET /api/v1/users/{user_id} +GET /api/v1/users/search +GET /api/v1/users/friends +GET /api/v1/users/friends/requests +POST /api/v1/users/friends/add/{user_id} +POST /api/v1/users/friends/accept-request/{sender_id} +DELETE /api/v1/users/friends/reject-request/{sender_id} +DELETE /api/v1/users/friends/{friend_id} +POST /api/v1/users/ignore/{user_id} +DELETE /api/v1/users/ignore/{user_id} ``` Middleware stack (protected routes): `TraceLayer` → `CorsLayer` → `KeycloakAuthLayer` → `DefaultBodyLimit` (5 MB) → `inject_request_path` ### Error Handling -All handlers return `Result, HttpError>`. `HttpError` serializes to: +All handlers return `AppResponse>` (= `Result, AppError>`, `core/errors.rs`). `AppError` serializes to: ```json -{ "status": 404, "errorCode": "NOT_FOUND", "message": "...", "timestamp": "...", "path": "/api/..." } +{ "timestamp": "...", "status": 404, "error": "Not Found", "message": "...", "path": "/api/v1/...", "errorCode": "CONTENT_NOT_FOUND" } ``` `path` is injected by `inject_request_path` middleware on error responses. +`AppError` variants split into **client-facing** (`Validation`, `NotFound`, `Forbidden` — message passed through) and **internal** (`Database`, `Cache`, `Serialization`, `S3`, `Processing` — logged in full, generic message returned). The auth middleware has its own `AuthError`, sanitised the same way. + ## Development Patterns **New endpoint**: handler in `handler.rs` → service logic → repository query → register in `routes.rs`. No business logic in handlers. diff --git a/Cargo.lock b/Cargo.lock index 96265cc..13294e8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -138,13 +138,13 @@ dependencies = [ [[package]] name = "async-trait" -version = "0.1.89" +version = "0.1.91" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" +checksum = "ae36dc4177970ef04fde5178d3e2429882def40e57a451f919c098f72baa6cec" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 3.0.3", ] [[package]] @@ -156,15 +156,6 @@ dependencies = [ "num-traits", ] -[[package]] -name = "atomic-time" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "75821c8282c0e622f3892087c1eeb8d4e3964b92467a263a44afa7d79dec7f3c" -dependencies = [ - "portable-atomic", -] - [[package]] name = "atomic-waker" version = "1.1.2" @@ -206,7 +197,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "31b698c5f9a010f6573133b09e0de5408834d0c82f8d7475a89fc1867a71cd90" dependencies = [ "axum-core", - "base64", + "base64 0.22.1", "bytes", "form_urlencoded", "futures-util", @@ -276,6 +267,12 @@ version = "0.22.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "72b3254f16251a8381aa12e40e3c4d2f0199f8c6508fbecb9d91f575e0fbb8c6" +[[package]] +name = "base64" +version = "0.23.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b25655df2c3cdd83c5e5b293b88acd880332b2ddadd7c30ac43144fdc0033da9" + [[package]] name = "base64ct" version = "1.8.3" @@ -350,9 +347,9 @@ checksum = "8f1fe948ff07f4bd06c30984e69f5b4899c516a3ef74f34df92a2df2ab535495" [[package]] name = "bytes" -version = "1.12.0" +version = "1.12.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ae3f5d315924270530207e2a68396c3cc547f6dca3fbdca317cfb1a51edb593" +checksum = "fc652a48c352aef3ea3aed32080501cf3ef6ed5da78602a020c991775b0aff04" [[package]] name = "cc" @@ -455,9 +452,9 @@ dependencies = [ [[package]] name = "config" -version = "0.15.24" +version = "0.15.25" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0b34d0237145f33580b89724f75d16950efd3e2c91b2d823917ecb69ec7f84f0" +checksum = "b85f248a4de22d204ceabc6299d89d2c70fbd7f09fea53c06c852369652d8139" dependencies = [ "async-trait", "convert_case", @@ -917,9 +914,9 @@ dependencies = [ [[package]] name = "educe" -version = "0.6.0" +version = "0.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d7bc049e1bd8cdeb31b68bbd586a9464ecf9f3944af3958a7a9d0f8b9799417" +checksum = "92c3e1715a2bf74bc8f68cd7bae12ff144f02669c602106ad1fa16f2ba62e646" dependencies = [ "enum-ordinalize", "proc-macro2", @@ -968,18 +965,18 @@ dependencies = [ [[package]] name = "enum-ordinalize" -version = "4.3.2" +version = "4.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4a1091a7bb1f8f2c4b28f1fe2cef4980ca2d410a3d727d67ecc3178c9b0800f0" +checksum = "07f808d588c10e464ea6f7d3eaed500049eff30aaac103460f61828c2d65b3eb" dependencies = [ "enum-ordinalize-derive", ] [[package]] name = "enum-ordinalize-derive" -version = "4.3.2" +version = "4.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8ca9601fb2d62598ee17836250842873a413586e5d7ed88b356e38ddbb0ec631" +checksum = "42e528e2d34ba8a67a1a650b86beae8ef69fc5fdb638016f386b973226590432" dependencies = [ "proc-macro2", "quote", @@ -1175,9 +1172,9 @@ checksum = "42703706b716c37f96a77aea830392ad231f44c9e9a67872fa5548707e11b11c" [[package]] name = "futures" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8b147ee9d1f6d097cef9ce628cd2ee62288d963e16fb287bd9286455b241382d" +checksum = "a88cf1f829d945f548cf8fec32c61b1f202b6d93b45848602fc02af4b12ad218" dependencies = [ "futures-channel", "futures-core", @@ -1190,9 +1187,9 @@ dependencies = [ [[package]] name = "futures-channel" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "07bbe89c50d7a535e539b8c17bc0b49bdb77747034daa8087407d655f3f7cc1d" +checksum = "262590f4fe6afeb0bc83be1daa64e52657fe185690a958af7f3ad0e92085c5ae" dependencies = [ "futures-core", "futures-sink", @@ -1200,15 +1197,15 @@ dependencies = [ [[package]] name = "futures-core" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7e3450815272ef58cec6d564423f6e755e25379b217b0bc688e295ba24df6b1d" +checksum = "2cd50c473c80f6d7c3670a752354b8e569b1a7cbfdc0419ec88e5edad85e0dc7" [[package]] name = "futures-executor" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "baf29c38818342a3b26b5b923639e7b1f4a61fc5e76102d4b1981c6dc7a7579d" +checksum = "6754879cc9f2c66f88c6e5c35344bb0bdb0708b0352b1201815667c7eabc7458" dependencies = [ "futures-core", "futures-task", @@ -1228,15 +1225,15 @@ dependencies = [ [[package]] name = "futures-io" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cecba35d7ad927e23624b22ad55235f2239cfa44fd10428eecbeba6d6a717718" +checksum = "4577ecaa3c4f96589d473f679a71b596316f6641bc350038b962a5daf0085d7a" [[package]] name = "futures-macro" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e835b70203e41293343137df5c0664546da5745f82ec9b84d40be8336958447b" +checksum = "2d6d3cde68c518367be28956066ddfef33813991b77a55005a69dae04bf3b10b" dependencies = [ "proc-macro2", "quote", @@ -1245,21 +1242,21 @@ dependencies = [ [[package]] name = "futures-sink" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c39754e157331b013978ec91992bde1ac089843443c49cbc7f46150b0fad0893" +checksum = "e34418ac499d6305c2fb5ad0ed2f6ac998c5f8ca209b4510f7f94242c647e307" [[package]] name = "futures-task" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "037711b3d59c33004d3856fbdc83b99d4ff37a24768fa1be9ce3538a1cde4393" +checksum = "b231ed28831efb4a61a08580c4bc233ec56bc009f4cd8f52da2c3cb97df0c109" [[package]] name = "futures-util" -version = "0.3.32" +version = "0.3.33" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "389ca41296e6190b48053de0321d02a77f32f8a5d2461dd38762c0593805c6d6" +checksum = "a77a90a256fce34da66415271e30f94ee91c57b04b8a2c042d9cf3220179deaa" dependencies = [ "futures-channel", "futures-core", @@ -1572,7 +1569,7 @@ version = "0.1.20" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96547c2556ec9d12fb1578c4eaf448b04993e7fb79cbaad930a656880a6bdfa0" dependencies = [ - "base64", + "base64 0.22.1", "bytes", "futures-channel", "futures-util", @@ -1799,13 +1796,12 @@ checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695" [[package]] name = "ism" -version = "0.8.0" +version = "0.8.1" dependencies = [ "assertr", "async-trait", - "atomic-time", "axum", - "base64", + "base64 0.23.0", "bytes", "chrono", "config", @@ -1814,27 +1810,21 @@ dependencies = [ "http", "image", "jsonwebtoken", - "log", "minio", - "nonempty", "rdkafka", "redis", "reqwest 0.13.4", "serde", - "serde-querystring", "serde_json", "serde_with", - "snafu", "sqlx", "thiserror", - "time", "tokio", "tokio-stream", "tower", "tower-http 0.7.0", "tracing", "tracing-subscriber", - "try-again", "typed-builder", "url", "uuid", @@ -1955,11 +1945,11 @@ dependencies = [ [[package]] name = "jsonwebtoken" -version = "10.4.0" +version = "11.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eba32bfb4ffdeaca3e34431072faf01745c9b26d25504aa7a6cf5684334fc4fc" +checksum = "881733cbc631fc9e472e24447ce32a64bedf2da498d6d8570b08edc87de71f65" dependencies = [ - "base64", + "base64 0.22.1", "ed25519-dalek", "getrandom 0.2.17", "hmac 0.12.1", @@ -1986,72 +1976,6 @@ dependencies = [ "spin", ] -[[package]] -name = "lexical" -version = "7.0.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1bc8a009b2ff1f419ccc62706f04fe0ca6e67b37460513964a3dfdb919bb37d6" -dependencies = [ - "lexical-core", -] - -[[package]] -name = "lexical-core" -version = "1.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d8d125a277f807e55a77304455eb7b1cb52f2b18c143b60e766c120bd64a594" -dependencies = [ - "lexical-parse-float", - "lexical-parse-integer", - "lexical-util", - "lexical-write-float", - "lexical-write-integer", -] - -[[package]] -name = "lexical-parse-float" -version = "1.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "52a9f232fbd6f550bc0137dcb5f99ab674071ac2d690ac69704593cb4abbea56" -dependencies = [ - "lexical-parse-integer", - "lexical-util", -] - -[[package]] -name = "lexical-parse-integer" -version = "1.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a7a039f8fb9c19c996cd7b2fcce303c1b2874fe1aca544edc85c4a5f8489b34" -dependencies = [ - "lexical-util", -] - -[[package]] -name = "lexical-util" -version = "1.0.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2604dd126bb14f13fb5d1bd6a66155079cb9fa655b37f875b3a742c705dbed17" - -[[package]] -name = "lexical-write-float" -version = "1.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50c438c87c013188d415fbabbb1dceb44249ab81664efbd31b14ae55dabb6361" -dependencies = [ - "lexical-util", - "lexical-write-integer", -] - -[[package]] -name = "lexical-write-integer" -version = "1.0.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "409851a618475d2d5796377cad353802345cba92c867d9fbcde9cf4eac4e14df" -dependencies = [ - "lexical-util", -] - [[package]] name = "libc" version = "0.2.186" @@ -2170,7 +2094,7 @@ checksum = "3824101357fa899d01c729e4a245776e20a03f2f6645979e86b9d3d5d9c42741" dependencies = [ "async-recursion", "async-trait", - "base64", + "base64 0.22.1", "byteorder", "bytes", "chrono", @@ -2276,12 +2200,6 @@ dependencies = [ "tempfile", ] -[[package]] -name = "nonempty" -version = "0.12.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9737e026353e5cd0736f98eddae28665118eb6f6600902a7f50db585621fecb6" - [[package]] name = "nu-ansi-term" version = "0.50.3" @@ -2539,7 +2457,7 @@ version = "3.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d30c53c26bc5b31a98cd02d20f25a7c8567146caf63ed593a9d87b2775291be" dependencies = [ - "base64", + "base64 0.22.1", "serde_core", ] @@ -2933,9 +2851,9 @@ dependencies = [ [[package]] name = "redis" -version = "1.2.4" +version = "1.4.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bae41a63fd0b8a5372f82b21e810e09a316f5dd7efd96bf08e678fb240fc1918" +checksum = "b0b9503711b03773e43b31668c7b5bd279ee7cd9b7d18cff7c23a42cc1d08e5a" dependencies = [ "arc-swap", "arcstr", @@ -3023,7 +2941,7 @@ version = "0.12.28" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "eddd3ca559203180a307f12d114c268abf583f59b03cb906fd0b3ff8646c1147" dependencies = [ - "base64", + "base64 0.22.1", "bytes", "futures-core", "futures-util", @@ -3062,7 +2980,7 @@ version = "0.13.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "219c5811de6525e5416c7d5d53bb656d3afdbc6c5af816e0802bcfa42dbdc1c3" dependencies = [ - "base64", + "base64 0.22.1", "bytes", "encoding_rs", "futures-core", @@ -3084,6 +3002,7 @@ dependencies = [ "rustls-platform-verifier", "serde", "serde_json", + "serde_urlencoded", "sync_wrapper", "tokio", "tokio-rustls", @@ -3372,24 +3291,14 @@ checksum = "8a7852d02fc848982e0c167ef163aaff9cd91dc640ba85e263cb1ce46fae51cd" [[package]] name = "serde" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e" +checksum = "4148590afebada386688f18773da617792bf2ef03ffc1e4cbd2b1d45b023e0ba" dependencies = [ "serde_core", "serde_derive", ] -[[package]] -name = "serde-querystring" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4ae1940bc2612f641456fc715125e4002cbd235d040188a1994e64b734054c2e" -dependencies = [ - "lexical", - "serde", -] - [[package]] name = "serde-untagged" version = "0.1.9" @@ -3404,29 +3313,29 @@ dependencies = [ [[package]] name = "serde_core" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad" +checksum = "67dca2c9c51e58a4791a4b1ed58308b39c64224d349a935ab5039aa360942a48" dependencies = [ "serde_derive", ] [[package]] name = "serde_derive" -version = "1.0.228" +version = "1.0.229" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" +checksum = "e7a5d71263a5a7d47b41f6b3f06ba276f10cc18b0931f1799f710578e2309348" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 3.0.3", ] [[package]] name = "serde_json" -version = "1.0.150" +version = "1.0.151" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e8014e44b4736ed0538adeecded0fce2a272f22dc9578a7eb6b2d9993c74cfb9" +checksum = "c841b55ecdae098c80dcae9cf767f6f8a0c2cdb3416bbef72181df4d0fe73f14" dependencies = [ "itoa", "memchr", @@ -3473,7 +3382,7 @@ version = "3.21.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "76a5c54c7310e7b8b9577c286d7e399ddd876c3e12b3ed917a8aabc4b96e9e8c" dependencies = [ - "base64", + "base64 0.22.1", "bs58", "chrono", "hex", @@ -3633,27 +3542,6 @@ dependencies = [ "serde", ] -[[package]] -name = "snafu" -version = "0.9.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d1a012328be2e3f5d5f6f3218147ca02588cea4cb865e876849ab6debcf36522" -dependencies = [ - "snafu-derive", -] - -[[package]] -name = "snafu-derive" -version = "0.9.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5f103c50866b8743da9429b8a581d81a27c2d3a9c4ac7df8f8571c1dd7896eda" -dependencies = [ - "heck", - "proc-macro2", - "quote", - "syn 2.0.118", -] - [[package]] name = "socket2" version = "0.6.4" @@ -3702,7 +3590,7 @@ version = "0.9.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "05b44e85bf579a8eeb4ceaa77a3a523baf2bf0e9bac7e40f405d537b5d2d5ccb" dependencies = [ - "base64", + "base64 0.22.1", "bytes", "cfg-if", "chrono", @@ -3806,7 +3694,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "87a2bdd6e83f6b3ea525ca9fee568030508b58355a43d0b2c1674d5f79dcd65e" dependencies = [ "atoi", - "base64", + "base64 0.22.1", "bitflags 2.13.0", "byteorder", "chrono", @@ -3913,6 +3801,17 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "syn" +version = "3.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "53e9bae58849f64dfa4f5d5ae372c8341f7305f82a3868709269343628b659a3" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "sync_wrapper" version = "1.0.2" @@ -3969,22 +3868,22 @@ dependencies = [ [[package]] name = "thiserror" -version = "2.0.18" +version = "2.0.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4288b5bcbc7920c07a1149a35cf9590a2aa808e0bc1eafaade0b80947865fbc4" +checksum = "09a43598840e33d5b0331f38c5e30d13bb11c11210a4b58f0d9b18a5a5eefcd9" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "2.0.18" +version = "2.0.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ebc4ee7f67670e9b64d05fa4253e753e016c6c95ff35b89b7941d6b856dec1d5" +checksum = "43cbfe0cf76104d42a574802844187e84a305e531ed54455f11fbde0f10541cd" dependencies = [ "proc-macro2", "quote", - "syn 2.0.118", + "syn 3.0.3", ] [[package]] @@ -4076,9 +3975,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.52.3" +version = "1.53.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8fc7f01b389ac15039e4dc9531aa973a135d7a4135281b12d7c1bc79fd57fffe" +checksum = "202caea871b69668250d242070849eb495be178ed697a3e98aebce5bc81a0bed" dependencies = [ "bytes", "libc", @@ -4124,9 +4023,9 @@ dependencies = [ [[package]] name = "tokio-stream" -version = "0.1.18" +version = "0.1.19" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32da49809aab5c3bc678af03902d4ccddea2a87d028d86392a4b1560c6906c70" +checksum = "a3d06f0b082ba57c26b79407372e57cf2a1e28124f78e9479fe80322cf53420b" dependencies = [ "futures-core", "pin-project-lite", @@ -4327,16 +4226,6 @@ dependencies = [ "tracing-log", ] -[[package]] -name = "try-again" -version = "0.2.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7840f01e68d609b64f3d6954d52846fa42b5dad9403eaecd00d2658d85f0cbff" -dependencies = [ - "tokio", - "tracing", -] - [[package]] name = "try-lock" version = "0.2.5" @@ -4468,9 +4357,9 @@ checksum = "06abde3611657adf66d383f00b093d7faecc7fa57071cce2578660c9f1010821" [[package]] name = "uuid" -version = "1.23.3" +version = "1.24.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "144d6b123cef80b301b8f72a9e2ca4370ddec21950d0a103dd22c437006d2db7" +checksum = "bf3923a6f5c4c6382e0b653c4117f48d631ea17f38ed86e2a828e6f7412f5239" dependencies = [ "getrandom 0.4.3", "js-sys", @@ -4480,12 +4369,11 @@ dependencies = [ [[package]] name = "validator" -version = "0.20.0" +version = "0.21.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43fb22e1a008ece370ce08a3e9e4447a910e92621bb49b85d6e48a45397e7cfa" +checksum = "c3d68c6633c483df6780cc5277a417c7c2d1bceee2649d06c8ab6b0fd2dd3c81" dependencies = [ "idna", - "once_cell", "regex", "serde", "serde_derive", diff --git a/Cargo.toml b/Cargo.toml index e8348ba..dee98ea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,49 +1,53 @@ [package] name = "ism" -version = "0.8.0" +version = "0.8.1" +description = "Social backend services for user relationships and chatting" edition = "2024" [dependencies] -log = "0.4.33" axum = { version = "0.8.9", features = ["multipart", "ws"] } -tokio = {version = "1.52.3", features = ["full"]} +tokio = {version = "1.53.1", features = ["full"]} tower = "0.5.3" -config = "0.15.24" -serde = "1.0.228" -futures = "0.3.32" -uuid = { version = "1.23.3", features = ["v4", "serde", "v7"] } +config = "0.15.25" +serde = "1.0.229" +futures = "0.3.33" +uuid = { version = "1.24.0", features = ["v4", "serde", "v7"] } chrono = { version = "0.4.45", features = ["serde"] } tower-http = { version = "0.7.0", features = ["cors", "trace"] } tracing = "0.1.44" tracing-subscriber = { version = "0.3.23", features = ["env-filter"] } sqlx = {version = "0.9.0", features = ["runtime-tokio", "postgres", "chrono", "uuid", "macros", "json"]} -serde_json = "1.0.150" -tokio-stream = { version = "0.1.18", features = ["sync"] } +serde_json = "1.0.151" +tokio-stream = { version = "0.1.19", features = ["sync"] } rdkafka = { version = "0.39.0", features = ["cmake-build", "tokio"] } minio = { version = "0.3.0", features = ["default"] } image = { version = "0.25.10", default-features = false, features = ["jpeg", "png", "gif", "webp", "bmp", "ico", "tiff"] } -bytes = "1.12.0" -base64 = "0.22.1" -validator = { version = "0.20.0", features = ["derive"] } -redis = { version = "1.2.4", features = ["tokio-comp", "connection-manager"] } -thiserror = "2.0.18" -async-trait = "0.1.89" +bytes = "1.12.1" +base64 = "0.23.0" +validator = { version = "0.21.0", features = ["derive"] } +redis = { version = "1.4.1", features = ["tokio-comp", "connection-manager"] } +thiserror = "2.0.19" +async-trait = "0.1.91" -#KEYCLOAK: -atomic-time = "0.2.1" -educe = { version = "0.6.0", default-features = false, features = ["Debug"] } +#used by the auth-domain: +educe = { version = "0.7.4", default-features = false, features = ["Debug"] } http = "1.4.2" -jsonwebtoken = { version = "10.4.0", features = ["rust_crypto"] } -nonempty = { version = "0.12.0", features = ["std"] } +jsonwebtoken = { version = "11.0.0", features = ["rust_crypto"] } reqwest = { version = "0.13.4", features = ["json"] } -serde-querystring = "0.3.0" serde_with = "3.21.0" -snafu = "0.9.1" -time = "0.3.51" -try-again = "0.2.2" typed-builder = "0.23.2" url = "2.5.8" [dev-dependencies] assertr = "0.6.0" + +# Used by tests/auth_attacks.rs, the black-box attack suite. Same versions as the runtime +# dependencies above, so they add nothing to the build graph — integration tests are separate +# crates and do not inherit `[dependencies]`. +base64 = "0.23.0" +jsonwebtoken = { version = "11.0.0", features = ["rust_crypto"] } +# `form` is what the Keycloak direct access grant is posted with; it affects the test build only. +reqwest = { version = "0.13.4", features = ["json", "form"] } +serde_json = "1.0.151" +tokio = { version = "1.53.1", features = ["full"] } diff --git a/default.config.toml b/default.config.toml index 2207792..e17c031 100644 --- a/default.config.toml +++ b/default.config.toml @@ -1,6 +1,12 @@ ism_url = "localhost" ism_port= 5403 -log_level = "debug" +# Tracing filter: a global default plus any number of `target=level` overrides, comma-separated. +# Targets are module paths, so they can be narrowed arbitrarily (`ism::auth::decode=trace`). +# Overridden entirely by the ISM_LOG_LEVEL env var. Noisy targets worth pinning: +# sqlx=warn - sqlx logs every executed statement at INFO +# hyper=info,h2=info - wire-level noise at DEBUG (already covered by a global `info`) +# tower_http=info - HTTP request spans +log_level = "info,ism=info,sqlx=warn" cors_origin = "http://localhost:4200" use_kafka = true @@ -14,6 +20,21 @@ db_name = "postgres" [token_issuer] iss_host = "http://localhost:8180/" iss_realm = "meventure" +# Accepted `aud` values. "account" is added by Keycloak's default audience-resolve mapper to every +# access token of every client in the realm, so on its own it does NOT restrict access to this +# application. To tighten: add a dedicated audience mapper in Keycloak, then set +# expected_audiences = [""] +# expected_azp = [""] +expected_audiences = ["account"] +# Accepted `azp` (authorized party) values. Empty = no check. +expected_azp = [] +# Signature algorithms accepted for incoming tokens. Never taken from the JWT header. +allowed_algorithms = ["RS256"] +# The JWKS is refreshed on demand: a token that fails to verify against the cached keys triggers +# a re-discovery and a retry within the same request, so key rotation costs one slow request and +# is never visible to a client. This is the rate limit between two discoveries, so a flood of +# invalid tokens cannot hammer Keycloak. +jwks_min_refresh_interval_secs = 30 [object_db_config] access_key = "minioadmin" diff --git a/rustfmt.toml b/rustfmt.toml new file mode 100644 index 0000000..3cbc7f7 --- /dev/null +++ b/rustfmt.toml @@ -0,0 +1,8 @@ +# rustfmt configuration for ISM +# Only stable options — usable with the standard `cargo fmt`. + +edition = "2024" + +# Allow wider lines than the 100-column default: breaking long expressions +# and signatures across lines often hurts readability more than it helps. +max_width = 160 \ No newline at end of file diff --git a/src/auth/LICENSE-MIT b/src/auth/LICENSE-MIT deleted file mode 100644 index 468cd79..0000000 --- a/src/auth/LICENSE-MIT +++ /dev/null @@ -1,23 +0,0 @@ -Permission is hereby granted, free of charge, to any -person obtaining a copy of this software and associated -documentation files (the "Software"), to deal in the -Software without restriction, including without -limitation the rights to use, copy, modify, merge, -publish, distribute, sublicense, and/or sell copies of -the Software, and to permit persons to whom the Software -is furnished to do so, subject to the following -conditions: - -The above copyright notice and this permission notice -shall be included in all copies or substantial portions -of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF -ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED -TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A -PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT -SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY -CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION -OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR -IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER -DEALINGS IN THE SOFTWARE. \ No newline at end of file diff --git a/src/auth/action.rs b/src/auth/action.rs index f3c69c9..8b696e2 100644 --- a/src/auth/action.rs +++ b/src/auth/action.rs @@ -1,3 +1,17 @@ +//! A single shared async operation whose latest result can be read while it re-runs. +//! +//! Used for exactly one thing: holding the OIDC discovery result inside a +//! `KeycloakAuthInstance` (see `instance.rs`), so that requests keep reading the cached keys +//! while a refresh is in flight and concurrent callers coalesce onto one run. +//! +//! The action reports failure as `None` rather than storing it, so an unsuccessful run leaves the +//! last good value in place. For the one caller that matters this is the difference between a +//! failed JWKS refresh costing nothing and it taking authentication down until the next successful +//! discovery — there is no timer, so "the next one" may be a long way off. + +use educe::Educe; +use futures::Future; +use std::sync::atomic::Ordering; use std::{ fmt::Debug, option::Option, @@ -7,137 +21,122 @@ use std::{ atomic::{AtomicBool, AtomicUsize}, }, }; - -use atomic_time::AtomicOptionInstant; -use educe::Educe; -use futures::Future; use tokio::{ sync::Notify, sync::{RwLock, futures::Notified}, task::JoinHandle, }; -pub(crate) trait ActionInput: Debug + Clone + Send + Sync + 'static {} -pub(crate) trait ActionOutput: Debug + Send + Sync + 'static {} +pub trait ActionInput: Debug + Clone + Send + Sync + 'static {} +pub trait ActionOutput: Debug + Send + Sync + 'static {} impl ActionInput for T where T: Debug + Clone + Send + Sync + 'static {} impl ActionOutput for T where T: Debug + Send + Sync + 'static {} #[derive(Educe)] #[educe(Debug)] -pub(crate) struct Action { +pub struct Action { /// The current argument that was dispatched to the `async` function. /// `Some` while we are waiting for it to resolve, `None` if it has resolved. input: Arc>>, - /// Last staring time at which the operation was dispatched. - #[educe(Debug(ignore))] - input_send: Arc, - + /// Yields `None` when the run failed, which `dispatch` treats as "keep what we have". #[educe(Debug(ignore))] #[allow(clippy::complexity)] - action_fn: Arc Pin + Send + Sync>> + Send + Sync>, + action_fn: + Arc Pin> + Send + Sync>> + Send + Sync>, /// Might be Some if there still is an ongoing operation. pending: Arc, notify: Arc, - /// The most recent return value of the `async` function. + /// The most recent *successful* return value of the `async` function. + /// + /// A run that yields `None` leaves this untouched — see `dispatch`. value: Arc>>, - /// Time the last value was received. None if we never received a value. - #[educe(Debug(ignore))] - value_received: Arc, - - /// How many times the action has successfully resolved. - /// Version 0 indicates that no value was received yet. + /// How many times the action has resolved, successfully or not. + /// Version 0 indicates that no run has completed yet. version: Arc, } -#[allow(dead_code)] impl Action { - pub(crate) fn new(action_fn: F) -> Self + pub fn new(action_fn: F) -> Self where F: Fn(&I) -> Fu + Send + Sync + 'static, - Fu: Future + Send + Sync + 'static, + Fu: Future> + Send + Sync + 'static, { let action_fn = Arc::new(move |input: &I| { let fut = action_fn(input); - Box::pin(fut) as Pin + Send + Sync>> + Box::pin(fut) as Pin> + Send + Sync>> }); Self { input: Arc::new(RwLock::new(None)), - input_send: Arc::new(AtomicOptionInstant::none()), action_fn, pending: Arc::new(AtomicBool::new(false)), notify: Arc::new(Notify::new()), value: Arc::new(RwLock::new(None)), - value_received: Arc::new(AtomicOptionInstant::none()), version: Arc::new(AtomicUsize::new(0)), } } /// Await the next completion of this action. /// Useful if the action is already pending, and you are interested in its upcoming value. - pub(crate) fn notified(&self) -> Notified<'_> { + pub fn notified(&self) -> Notified<'_> { self.notify.notified() } - pub(crate) fn is_pending(&self) -> bool { - self.pending.load(std::sync::atomic::Ordering::Acquire) - } - - pub(crate) fn pending_for(&self) -> std::time::Duration { - let started_at: std::time::Instant = self.input_send().expect("Start time when pending"); - std::time::Instant::now() - started_at + pub fn is_pending(&self) -> bool { + self.pending.load(Ordering::Acquire) } - pub(crate) async fn input(&self) -> tokio::sync::RwLockReadGuard<'_, Option> { + pub async fn input(&self) -> tokio::sync::RwLockReadGuard<'_, Option> { self.input.read().await } - pub(crate) fn input_send(&self) -> Option { - self.input_send.load(std::sync::atomic::Ordering::Acquire) - } - - pub(crate) async fn value(&self) -> tokio::sync::RwLockReadGuard<'_, Option> { + pub async fn value(&self) -> tokio::sync::RwLockReadGuard<'_, Option> { self.value.read().await } - pub(crate) fn value_received(&self) -> Option { - self.value_received - .load(std::sync::atomic::Ordering::Acquire) + pub fn version(&self) -> usize { + self.version.load(Ordering::Acquire) } - pub(crate) fn version(&self) -> usize { - self.version.load(std::sync::atomic::Ordering::Acquire) + /// Installs a value without running the action, as the result of a run performed by the caller. + /// + /// Exists so the initial OIDC discovery can be awaited directly — and its failure turned into a + /// startup abort — rather than being dispatched into a task whose outcome nobody can observe. + pub async fn seed(&self, value: O) { + *self.value.write().await = Some(value); + self.version.fetch_add(1, Ordering::Release); } - pub(crate) fn dispatch(&self, action_input: I) -> JoinHandle<()> { + pub fn dispatch(&self, action_input: I) -> JoinHandle<()> { let fut = (self.action_fn)(&action_input); let input = self.input.clone(); - let input_send = self.input_send.clone(); let version = self.version.clone(); let pending = self.pending.clone(); let notify = self.notify.clone(); let value = self.value.clone(); - let value_received = self.value_received.clone(); - tokio::spawn(async move { - let started = Some(std::time::Instant::now()); + // Mark pending *before* spawning. Setting it inside the task leaves a window in which the + // action is dispatched but not yet observably pending, so concurrent callers checking + // `is_pending()` would each dispatch their own duplicate run. + pending.store(true, Ordering::Release); + tokio::spawn(async move { *input.write().await = Some(action_input.clone()); - input_send.store(started, std::sync::atomic::Ordering::Release); - pending.store(true, std::sync::atomic::Ordering::Release); - let new_value: O = fut.await; - let new_value_received_at = Some(std::time::Instant::now()); - *value.write().await = Some(new_value); - value_received.store(new_value_received_at, std::sync::atomic::Ordering::Release); - version.fetch_add(1, std::sync::atomic::Ordering::Release); + // A failed run must not evict the value a successful one left behind: the stored value + // is what every request reads, and replacing it with nothing turns one unlucky refresh + // into a total outage. The run is still counted below so waiters wake either way. + if let Some(new_value) = fut.await { + *value.write().await = Some(new_value); + } + version.fetch_add(1, Ordering::Release); *input.write().await = None; - pending.store(false, std::sync::atomic::Ordering::Release); + pending.store(false, Ordering::Release); notify.notify_waiters(); }) } diff --git a/src/auth/app_role.rs b/src/auth/app_role.rs new file mode 100644 index 0000000..c26c478 --- /dev/null +++ b/src/auth/app_role.rs @@ -0,0 +1,107 @@ +//! The realm roles ISM knows about. +//! +//! This is the concrete `Role` implementation the whole application is generic over — see +//! `CurrentUser` in `current_user.rs`, which pins `KeycloakToken` to it. + +use std::fmt::{Display, Formatter}; + +use crate::auth::role::Role; + +/// A Keycloak realm role, resolved to the set ISM cares about. +/// +/// Keycloak hands every user a number of roles nobody here asked for — `offline_access`, +/// `uma_authorization`, `default-roles-`, plus the client roles of the `account` client. +/// Those land in `Unknown` and simply never match a check, which is why this enum has a catch-all +/// instead of failing to parse: an unrecognised role is normal, not an error. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AppRole { + Admin, + User, + LocalGuide, + /// Any role the realm hands out that ISM has no rule for. Carries the original name. + Unknown(String), +} + +impl AppRole { + /// The role name exactly as the realm spells it. + /// + /// `Display` and `From` both go through this, so `AppRole::from(role.to_string())` + /// round-trips for every variant — including `Unknown`. + pub fn as_str(&self) -> &str { + match self { + AppRole::Admin => "ADMIN", + AppRole::User => "USER", + AppRole::LocalGuide => "LOCAL_GUIDE", + AppRole::Unknown(name) => name, + } + } +} + +impl Role for AppRole {} + +impl Display for AppRole { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + +impl From for AppRole { + fn from(value: String) -> Self { + match value.as_str() { + "ADMIN" => AppRole::Admin, + "USER" => AppRole::User, + "LOCAL_GUIDE" => AppRole::LocalGuide, + _ => AppRole::Unknown(value), + } + } +} + +#[cfg(test)] +mod tests { + use super::AppRole; + + #[test] + fn maps_the_realm_role_names() { + assert_eq!(AppRole::from("ADMIN".to_owned()), AppRole::Admin); + assert_eq!(AppRole::from("USER".to_owned()), AppRole::User); + assert_eq!(AppRole::from("LOCAL_GUIDE".to_owned()), AppRole::LocalGuide); + } + + #[test] + fn keeps_unknown_roles_verbatim() { + // Keycloak assigns these to every account; they must survive as-is rather than being + // dropped, so a log line naming the role is still readable. + for name in [ + "offline_access", + "uma_authorization", + "default-roles-meventure", + ] { + assert_eq!( + AppRole::from(name.to_owned()), + AppRole::Unknown(name.to_owned()) + ); + } + } + + #[test] + fn role_names_are_case_sensitive() { + // Keycloak role names are case-sensitive, so "admin" is genuinely a different role than + // "ADMIN" and must not silently be treated as one. + assert_eq!( + AppRole::from("admin".to_owned()), + AppRole::Unknown("admin".to_owned()) + ); + } + + #[test] + fn display_round_trips_through_from_string() { + for role in [ + AppRole::Admin, + AppRole::User, + AppRole::LocalGuide, + AppRole::Unknown("something-else".to_owned()), + ] { + assert_eq!(AppRole::from(role.to_string()), role); + } + } +} diff --git a/src/auth/current_user.rs b/src/auth/current_user.rs new file mode 100644 index 0000000..e24c483 --- /dev/null +++ b/src/auth/current_user.rs @@ -0,0 +1,116 @@ +//! The handler-facing extractor for the authenticated caller. + +use axum::extract::FromRequestParts; +use axum::http::request::Parts; + +use crate::auth::app_role::AppRole; +use crate::auth::error::AuthError; +use crate::auth::token::KeycloakToken; + +/// The authenticated caller — the full validated token, not just an id. +/// +/// This is the concrete instantiation of `KeycloakToken` the whole application uses, which is why +/// no handler ever writes the `` generic. Available on it: +/// +/// - `subject` — the Keycloak user UUID, what almost every handler passes on to a service +/// - `roles` — `Vec>`, realm and client roles +/// - `extra.profile` — `preferred_username` +/// - `extra.email` — `email`, `email_verified` +/// - `expires_at`, `issued_at`, `issuer`, `audience`, `authorized_party`, `jwt_id` +/// +/// ```ignore +/// pub async fn handle_get_friends( +/// State(state): State>, +/// user: CurrentUser, +/// ) -> AppResponse>> { +/// expect_role!(&user, AppRole::Admin); // optional, see docs/auth.md +/// let results = UserService::get_friends(state, &user.subject, /* … */).await?; +/// Ok(Json(results)) +/// } +/// ``` +pub type CurrentUser = KeycloakToken; + +/// Hands the handler the token the middleware already validated and stashed. +/// +/// The `KeycloakAuthLayer` runs in `PassthroughMode::Block`, so a handler behind it is only +/// reached once that token exists. Under `PassthroughMode::Pass` there is no token extension to +/// read and this rejects with 401 — extract `KeycloakAuthStatus` there instead. +impl FromRequestParts for KeycloakToken +where + S: Send + Sync, +{ + type Rejection = AuthError; + + async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result { + // Cloned rather than taken out of the extensions: a handler may name `CurrentUser` more + // than once, and later layers still expect the extension to be there. + parts + .extensions + .get::>() + .cloned() + .ok_or(AuthError::MissingToken) + } +} + +#[cfg(test)] +mod tests { + use axum::http::StatusCode; + use axum::response::{IntoResponse, Response}; + use uuid::Uuid; + + use super::CurrentUser; + use crate::auth::app_role::AppRole; + use crate::auth::role::KeycloakRole; + use crate::auth::token::{Email, Profile, ProfileAndEmail}; + use crate::expect_role; + + fn token_with(roles: Vec) -> CurrentUser { + CurrentUser { + expires_at: chrono::Utc::now() + chrono::TimeDelta::minutes(5), + issued_at: chrono::Utc::now(), + jwt_id: "b7c1e5a2-3f4d-4e5a-9b8c-7d6e5f4a3b2c".to_owned(), + issuer: "https://keycloak.example/realms/meventure".to_owned(), + audience: vec!["account".to_owned()], + subject: Uuid::now_v7(), + authorized_party: "ism-app".to_owned(), + roles: roles + .into_iter() + .map(|role| KeycloakRole::Realm { role }) + .collect(), + extra: ProfileAndEmail { + profile: Profile { + preferred_username: "tim".to_owned(), + }, + email: Email { + email: None, + email_verified: false, + }, + }, + } + } + + /// Shaped like a real handler, so the macro's early `return` is exercised the way it is at an + /// actual call site — that is the whole point of the test. + fn admin_only(user: &CurrentUser) -> Response { + expect_role!(user, AppRole::Admin); + StatusCode::OK.into_response() + } + + #[test] + fn expect_role_macro_resolves_through_the_auth_facade() { + // The `expect_role!` family expands to `$crate::auth::ExpectRoles`. Nothing in the + // application calls these macros yet, so without this test a broken path — which is + // exactly what a private `role` module would cause — would go unnoticed until the first + // real use. + assert_eq!( + admin_only(&token_with(vec![AppRole::Admin])).status(), + StatusCode::OK + ); + } + + #[test] + fn expect_role_macro_rejects_a_caller_without_the_role() { + let response = admin_only(&token_with(vec![AppRole::User, AppRole::LocalGuide])); + assert_eq!(response.status(), StatusCode::FORBIDDEN); + } +} diff --git a/src/auth/decode.rs b/src/auth/decode.rs index fa390fb..73d7adf 100644 --- a/src/auth/decode.rs +++ b/src/auth/decode.rs @@ -1,117 +1,282 @@ +//! Signature and claim validation. +//! +//! `ValidationPolicy` fixes the rules at startup from configuration; `decode_and_validate` runs +//! them against the keys cached in the `KeycloakAuthInstance` and hands back a `KeycloakToken` +//! (defined in `token.rs`). Every rule in here has a matching attack in `security_tests.rs`. + use std::collections::HashMap; use std::sync::Arc; -use super::{error::AuthError, role::ExtractRoles, role::Role}; -use crate::auth::error::DecodeHeaderSnafu; -use crate::auth::error::DecodeSnafu; -use crate::auth::instance::KeycloakAuthInstance; -use crate::auth::role::{ExpectRoles, KeycloakRole, NumRoles}; -use jsonwebtoken::Header; -use jsonwebtoken::errors::ErrorKind; +use crate::auth::error::AuthError; +use crate::auth::instance::{KeycloakAuthInstance, keys_for_kid}; +use crate::auth::role::{ExpectRoles, Role}; +use crate::auth::token::{KeycloakToken, StandardClaims}; +use chrono::TimeDelta; +use jsonwebtoken::{Algorithm, AlgorithmFamily, DecodingKey, Header, Validation, decode}; use serde::de::DeserializeOwned; -use serde::{Deserialize, Serialize}; -use serde_with::{OneOrMany, serde_as}; -use snafu::ResultExt; +use std::str::FromStr; use tracing::debug; -use uuid::Uuid; pub type RawClaims = HashMap; -pub(crate) struct RawToken<'a>(pub(crate) &'a str); +type DecodedTokenResult = Result>, AuthError>; + +/// Token type Keycloak stamps onto access tokens. ID and refresh tokens carry a different `typ` +/// but are signed by the same realm key, so this claim is what separates them. +const ACCESS_TOKEN_TYP: &str = "Bearer"; + +/// How far past its `exp` a token is still accepted, absorbing clock drift between the Keycloak +/// host and this one. +/// +/// Applied in two places that both run on every request: `jsonwebtoken`'s own `exp`/`nbf` +/// validation below, and the explicit `assert_not_expired` in `decode_and_validate`. The stricter +/// of the two decides, so they must agree — this constant is what makes them agree. It deliberately +/// replaces `jsonwebtoken`'s 60-second default, which was previously dead anyway because the +/// explicit check ran with no leeway at all. +pub const EXPIRY_LEEWAY_SECS: i64 = 5; + +/// Longest `kid` accepted from a token header. +/// +/// Keycloak publishes a base64url thumbprint of around 43 characters. The limit is generous next to +/// that and exists only to bound what an unauthenticated caller can push into a log line — `kid` is +/// read before any signature is verified, so its content is entirely attacker-chosen. +const MAX_KID_LEN: usize = 256; + +/// The rules incoming tokens are validated against, fixed at startup from configuration. +/// +/// Deliberately independent of anything the caller controls — see `RawToken::decode_and_validate`. +#[derive(Debug, Clone)] +pub struct ValidationPolicy { + /// Accepted `aud` values. + pub expected_audiences: Vec, + /// Accepted `azp` values. Empty disables the check. + pub expected_azp: Vec, + /// Accepted signature algorithms. + pub allowed_algorithms: Vec, + /// Built once here and reused for every token, rather than reassembled per request — it costs + /// several hash sets and string allocations. Covers everything fixed at startup; the issuer is + /// checked separately in `decode_and_validate`, since it only becomes known through discovery. + validation: Validation, +} + +impl ValidationPolicy { + /// Builds a policy from configuration, rejecting unusable algorithm lists up front so a + /// misconfiguration surfaces at startup rather than as a blanket 401 at runtime. + pub fn new( + expected_audiences: Vec, + expected_azp: Vec, + algorithm_names: &[String], + ) -> Result { + let allowed_algorithms = algorithm_names + .iter() + .map(|name| { + Algorithm::from_str(name) + .map_err(|_| format!("unknown signature algorithm: {name}")) + }) + .collect::, _>>()?; + + let Some(first) = allowed_algorithms.first().copied() else { + return Err("allowed_algorithms must not be empty".to_owned()); + }; + + if first.family() == AlgorithmFamily::Hmac { + return Err( + "allowed_algorithms must not contain a symmetric (HS*) algorithm: Keycloak signs \ + with asymmetric keys, and accepting HMAC invites key-confusion forgery" + .to_owned(), + ); + } + + // `jsonwebtoken` rejects verification outright when the allow-list spans more than one + // family, so catch that here where we can explain it. + if let Some(mismatch) = allowed_algorithms.iter().find(|alg| alg.family() != first.family()) { + return Err(format!( + "allowed_algorithms must all belong to the same family, but {first:?} and \ + {mismatch:?} do not" + )); + } + + if expected_audiences.is_empty() { + return Err("expected_audiences must not be empty".to_owned()); + } + + // The algorithm allow-list comes from configuration, never from `header.alg`. Deriving it + // from the header lets the caller choose the family their token is verified under, which + // is the setup for RS256 -> HS256 key-confusion forgery. + let mut validation = Validation::new_for_family(first.family()); + validation.validate_exp = true; + validation.validate_nbf = true; + validation.leeway = EXPIRY_LEEWAY_SECS as u64; + validation.algorithms = allowed_algorithms.clone(); + validation.set_audience(&expected_audiences); + + // `iss` is required here even though it is compared by hand below, so a token that omits + // it is rejected before the comparison ever runs. + validation.set_required_spec_claims(&["exp", "iss", "aud", "sub"]); + + Ok(Self { + expected_audiences, + expected_azp, + allowed_algorithms, + validation, + }) + } + + /// Rejects tokens minted for a different Keycloak client in the same realm. + fn assert_authorized_party(&self, azp: &str) -> Result<(), AuthError> { + if self.expected_azp.is_empty() || self.expected_azp.iter().any(|it| it == azp) { + return Ok(()); + } + Err(AuthError::InvalidToken { + reason: format!("unexpected authorized party: {azp}"), + }) + } +} + +/// A bearer token as it came off the wire, before any validation. +pub struct RawToken<'a>(pub &'a str); impl RawToken<'_> { - pub(crate) fn decode_header(&self) -> Result { - let jwt_header = jsonwebtoken::decode_header(self.0).context(DecodeHeaderSnafu {})?; + pub fn decode_header(&self) -> Result { + let jwt_header = jsonwebtoken::decode_header(self.0) + .map_err(|source| AuthError::DecodeHeader { source })?; debug!(?jwt_header, "Decoded JWT header"); Ok(jwt_header) } - pub(crate) fn decode_and_validate<'d>( + /// Verifies the signature against `decoding_keys` and every claim rule in `policy`. + pub fn decode_and_validate( &self, - header: &Header, - expected_audiences: &[String], - decoding_keys: impl Iterator, + policy: &ValidationPolicy, + issuer: &str, + decoding_keys: &[&DecodingKey], ) -> Result { - let mut validation = jsonwebtoken::Validation::new(header.alg); - validation.set_audience(expected_audiences); + let mut token_data: DecodedTokenResult = Err(AuthError::NoDecodingKeys); - let mut token_data: Result< - jsonwebtoken::TokenData>, - AuthError, - > = Err(AuthError::NoDecodingKeys); for key in decoding_keys { - token_data = - jsonwebtoken::decode::(self.0, key, &validation).context(DecodeSnafu {}); + token_data = decode::(self.0, key, &policy.validation) + .map_err(|source| AuthError::Decode { source }); if token_data.is_ok() { break; } } let token_data = token_data?; let raw_claims = token_data.claims; - debug!(?raw_claims, "Decoded JWT data"); + // Pinned here rather than through `Validation::set_issuer`, because the expected issuer + // only becomes known through OIDC discovery: folding it into the policy's `Validation` + // would force that whole struct — several hash sets and string allocations — to be rebuilt + // on every request. This is the same set-membership test `jsonwebtoken` performs, over a + // set of one. + let token_issuer = raw_claims + .get("iss") + .and_then(|it| it.as_str()) + .unwrap_or_default(); + if token_issuer != issuer { + return Err(AuthError::InvalidToken { + reason: format!("unexpected issuer: {token_issuer}"), + }); + } + + // Only the subject is logged. The full claim set carries the user's email, username and + // roles, and this runs at an operator-settable log level. + debug!(sub = ?raw_claims.get("sub"), "Decoded JWT claims"); Ok(raw_claims) } } -pub(crate) async fn decode_and_validate( +/// Validates a token against the currently discovered keys, re-running discovery once if the +/// token names a signing key we do not know. +/// +/// The refresh is gated on the header's `kid`, never on the outcome of the signature check. That +/// check cannot answer the question: `ErrorKind::InvalidSignature` has a single construction site +/// in `jsonwebtoken` and means only "the verify operation returned false", so a payload-tampered +/// token and a token signed by a rotated key produce the identical error. Keying off it therefore +/// let any invalid token — every expired session, every replayed token — reach for Keycloak. +/// +/// The `kid` does answer it. Keycloak stamps a per-key thumbprint on every access token and does +/// not reuse it across rotations, so: +/// +/// - **known `kid`** — we hold the exact key the token names. Whatever fails now is a property of +/// the token, not of our key set; rediscovery cannot change the outcome. Terminal. +/// - **unknown `kid`** — the only shape a rotation can take. Worth one refresh. +/// - **no `kid`** — nothing to match a key against, and not something Keycloak issues. Rejected +/// before any signature verification runs. +pub async fn decode_and_validate( kc_instance: &KeycloakAuthInstance, raw_token: RawToken<'_>, - expected_audiences: &[String], + policy: &ValidationPolicy, ) -> Result { let header = raw_token.decode_header()?; + let Some(kid) = header.kid.as_deref() else { + return Err(AuthError::InvalidToken { + reason: "token header names no signing key (kid)".to_owned(), + }); + }; + + // `kid` reaches this point unauthenticated — it is read before any signature is verified — and + // from here it flows into log fields and into `UnknownSigningKey`. Unchecked, a `kid` carrying + // newlines or control characters can forge log lines, and an arbitrarily long one turns every + // rejected request into as much log volume as the sender cares to send. Keycloak's own `kid` is + // a base64url thumbprint, so printable ASCII with a length bound loses nothing. + // + // The rejection deliberately does not echo the `kid` back into its own message. + if kid.len() > MAX_KID_LEN || !kid.bytes().all(|b| b.is_ascii_graphic()) { + return Err(AuthError::InvalidToken { + reason: "token header names a malformed signing key (kid)".to_owned(), + }); + } + async fn try_decode( kc_instance: &KeycloakAuthInstance, - header: &Header, + kid: &str, raw_token: &RawToken<'_>, - expected_audiences: &[String], + policy: &ValidationPolicy, ) -> Result { - let decoding_keys = kc_instance.decoding_keys().await; - raw_token.decode_and_validate(header, expected_audiences, decoding_keys.iter()) - } - - // First decode. This may fail if known decoding keys are out of date (for example if the Keycloak server changed). - let mut raw_claims = try_decode(kc_instance, &header, &raw_token, expected_audiences).await; - - if raw_claims.is_err() { - // If it makes sense to do so, refresh the decoding keys through a new discovery process - // and try to decode again. - // This may delay handling of the request in flight by a non-marginal amount of time - // but may allow us to acknowledge it in the end without rejecting the call immediately, - // which would then (probably) require a retry from our caller anyway! - #[allow(clippy::unwrap_used)] - let retry = match raw_claims.as_ref().unwrap_err() { - AuthError::NoDecodingKeys => true, - AuthError::Decode { source } => match source.kind() { - // While rare, if this occurs, a valid key can be retrieved from Keycloak. - ErrorKind::InvalidRsaKey(_) => true, - // Added for completeness, though its relevance is uncertain. - ErrorKind::InvalidEcdsaKey => true, - // May occur after a private key change in Keycloak. - // However, such changes are infrequent, and without rate limiting, - // this can lead to excessive requests to the Keycloak server - // through our Axum backend. - ErrorKind::RsaFailedSigning => true, - _ => false, - }, - _ => false, + let discovered = kc_instance.discovered().await; + // The issuer we pin against is the one Keycloak advertises in its discovery document, + // not one rebuilt from `iss_host`/`iss_realm` — a configured frontend URL makes those + // two differ. + let issuer = discovered.issuer().ok_or(AuthError::NoOidcDiscovery)?; + let Some(keys) = keys_for_kid(discovered.decoding_keys(), kid) else { + return Err(AuthError::UnknownSigningKey { + kid: kid.to_owned(), + }); }; + raw_token.decode_and_validate(policy, issuer, &keys) + } - // Second decode - if retry { - kc_instance.perform_oidc_discovery().await; - raw_claims = try_decode(kc_instance, &header, &raw_token, expected_audiences).await; - } + // First decode. A separate function so the read guard it holds is released here on return: + // installing a refreshed key set below needs a write guard on that same lock, and Tokio's + // `RwLock` is write-preferring, so holding the read guard across it would deadlock. + let raw_claims = try_decode(kc_instance, kid, &raw_token, policy).await; + + // Only an unknown `kid` is worth re-running discovery for. Every other failure came from a key + // we hold, which makes it a property of the token rather than of our key set — rediscovery + // cannot change the verdict, and reaching for Keycloak on each one meant every expired session + // did so too. + if !matches!(raw_claims, Err(AuthError::UnknownSigningKey { .. })) { + return raw_claims; } - raw_claims + // Second decode, against a freshly discovered key set. Either Keycloak rotated its signing key + // or someone made a `kid` up; both look the same from here, so the refresh is rate-limited and + // time-boxed rather than trusted — see `KeycloakAuthInstance::refresh_for_request`. If the + // `kid` is still unknown afterwards, this returns `UnknownSigningKey` again without spending a + // single public-key operation on it. + debug!(kid,"Token names an unknown signing key. Re-running discovery."); + kc_instance.refresh_for_request().await; + try_decode(kc_instance, kid, &raw_token, policy).await } -pub(crate) async fn parse_raw_claims( +/// Turns a validated claim map into a `KeycloakToken`, applying the checks that need the parsed +/// claims: token type, expiry, authorized party and the layer's required roles. +pub async fn parse_raw_claims( raw_claims: RawClaims, persist_raw_claims: bool, required_roles: &[R], + policy: &ValidationPolicy, ) -> Result< ( Option>, @@ -127,260 +292,25 @@ where true => Some(raw_claims.clone()), false => None, }; - let value = serde_json::Value::from_iter(raw_claims.into_iter()); + let value = serde_json::Value::from_iter(raw_claims); + + let standard_claims: StandardClaims = + serde_json::from_value(value).map_err(|err| AuthError::JsonParse { + source: Arc::new(err), + })?; + + // Reject anything that is not an access token. ID and refresh tokens are signed by the same + // realm key and pass every check above, so without this they could be replayed as bearer + // credentials. + if standard_claims.typ != ACCESS_TOKEN_TYP { + return Err(AuthError::InvalidToken { + reason: format!("unexpected token type: {}", standard_claims.typ), + }); + } - let standard_claims = serde_json::from_value(value).map_err(|err| AuthError::JsonParse { - source: Arc::new(err), - })?; let keycloak_token = KeycloakToken::::parse(standard_claims)?; - keycloak_token.assert_not_expired()?; + keycloak_token.assert_not_expired(TimeDelta::seconds(EXPIRY_LEEWAY_SECS))?; + policy.assert_authorized_party(&keycloak_token.authorized_party)?; keycloak_token.expect_roles(required_roles)?; Ok((raw_claims_clone, keycloak_token)) } - -#[serde_as] -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct StandardClaims { - /// Expiration time (unix timestamp). - pub exp: i64, - /// Issued at time (unix timestamp). - pub iat: i64, - /// JWT ID (unique identifier for this token). - pub jti: String, - /// Issuer (who created and signed this token). This is the UUID which uniquely identifies this user inside Keycloak. - pub iss: String, - /// Audience (who or what the token is intended for). - #[serde_as(deserialize_as = "OneOrMany<_>")] - #[serde(default)] - pub aud: Vec, - /// Subject (whom the token refers to). - pub sub: String, - /// Type of token. - pub typ: String, - /// Authorized party (the party to which this token was issued). - pub azp: String, - - /// Keycloak: Optional realm roles from Keycloak. - pub realm_access: Option, - /// Keycloak: Optional client roles from Keycloak. - pub resource_access: Option, - - #[serde(flatten)] - pub extra: Extra, -} - -/// Access details. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct Access { - /// A list of role names. - pub roles: Vec, -} - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct RealmAccess(pub Access); - -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct ResourceAccess(pub HashMap); - -impl NumRoles for RealmAccess { - fn num_roles(&self) -> usize { - self.0.roles.len() - } -} - -impl NumRoles for ResourceAccess { - fn num_roles(&self) -> usize { - self.0.values().map(|access| access.roles.len()).sum() - } -} - -impl ExtractRoles for RealmAccess { - fn extract_roles(self, target: &mut Vec>) { - for role in self.0.roles { - target.push(KeycloakRole::Realm { role: role.into() }); - } - } -} - -impl ExtractRoles for ResourceAccess { - fn extract_roles(self, target: &mut Vec>) { - for (res_name, access) in &self.0 { - for role in &access.roles { - target.push(KeycloakRole::Client { - client: res_name.to_owned(), - role: role.to_owned().into(), - }); - } - } - } -} - -/// Token data parsed from the request and added as an `axum::Extension` through our middleware. -/// -/// This only exists if the `KeycloakAuthLayer` is configured to use `PassthroughMode::Block`. -/// -/// If you want to manually check whether a request was authenticated, configure -/// `PassthroughMode::Pass` (potentially on a separate `axum::Router`) and inject -/// `KeycloakAuthState` instead of `KeycloakToken`! -/// -/// Can be extracted like this: -/// ``` -/// use axum::{Extension, Json}; -/// use axum::response::{IntoResponse, Response}; -/// use http::StatusCode; -/// use serde::Serialize;/// -/// use ism::auth::decode::KeycloakToken; -/// -/// -/// pub async fn who_am_i(Extension(token): Extension>) -> Response { -/// #[derive(Debug, Serialize)] -/// struct Response { -/// name: String, -/// keycloak_uuid: uuid::Uuid, -/// token_valid_for_whole_seconds: i64, -/// } -/// -/// ( -/// StatusCode::OK, -/// Json(Response { -/// name: token.extra.profile.preferred_username, -/// keycloak_uuid: uuid::Uuid::try_parse(&token.subject).expect("uuid"), -/// token_valid_for_whole_seconds: (token.expires_at - time::OffsetDateTime::now_utc()) -/// .whole_seconds(), -/// }), -/// ).into_response() -/// } -/// ``` -#[derive(Debug, PartialEq, Clone)] -pub struct KeycloakToken -where - R: Role, - Extra: DeserializeOwned + Clone, -{ - /// Expiration time (UTC). - pub expires_at: time::OffsetDateTime, - /// Issued at time (UTC). - pub issued_at: time::OffsetDateTime, - /// JWT ID (unique identifier for this token). - pub jwt_id: String, - /// Issuer (who created and signed this token). - pub issuer: String, - /// Audience (who or what the token is intended for). - pub audience: Vec, - /// Subject (whom the token refers to). This is the UUID which uniquely identifies this user inside Keycloak. - pub subject: Uuid, - /// Authorized party (the party to which this token was issued). - pub authorized_party: String, - - // Keycloak: Roles of the user. - pub roles: Vec>, - - pub extra: Extra, -} - -impl KeycloakToken -where - R: Role, - Extra: DeserializeOwned + Clone, -{ - pub(crate) fn parse(raw: StandardClaims) -> Result { - Ok(Self { - expires_at: time::OffsetDateTime::from_unix_timestamp(raw.exp).map_err(|err| { - AuthError::InvalidToken { - reason: format!( - "Could not parse 'exp' (expires_at) field as unix timestamp: {err}" - ), - } - })?, - issued_at: time::OffsetDateTime::from_unix_timestamp(raw.iat).map_err(|err| { - AuthError::InvalidToken { - reason: format!( - "Could not parse 'iat' (issued_at) field as unix timestamp: {err}" - ), - } - })?, - jwt_id: raw.jti, - issuer: raw.iss, - audience: raw.aud, - subject: Uuid::try_parse(&raw.sub).map_err(|err| AuthError::InvalidToken { - reason: format!("Could not parse 'sub' (subject) field as uuid: {err}"), - })?, - authorized_party: raw.azp, - roles: { - let mut roles = Vec::new(); - (raw.realm_access, raw.resource_access).extract_roles(&mut roles); - roles - }, - extra: raw.extra, - }) - } - - pub fn is_expired(&self) -> bool { - time::OffsetDateTime::now_utc() > self.expires_at - } - - pub fn assert_not_expired(&self) -> Result<(), AuthError> { - match self.is_expired() { - true => Err(AuthError::TokenExpired), - false => Ok(()), - } - } -} - -impl ExpectRoles for KeycloakToken -where - R: Role, - Extra: DeserializeOwned + Clone, -{ - type Rejection = AuthError; - - fn expect_roles + Clone>(&self, roles: &[I]) -> Result<(), Self::Rejection> { - for expected in roles { - let expected: R = expected.clone().into(); - if !self.roles.iter().any(|role| role.role() == &expected) { - return Err(AuthError::MissingExpectedRole { - role: expected.to_string(), - }); - } - } - Ok(()) - } - - fn not_expect_roles + Clone>(&self, roles: &[I]) -> Result<(), Self::Rejection> { - for expected in roles { - let expected: R = expected.clone().into(); - if let Some(_role) = self.roles.iter().find(|role| role.role() == &expected) { - return Err(AuthError::UnexpectedRole); - } - } - Ok(()) - } -} - -#[derive(serde::Deserialize, Debug, Clone)] -pub struct Profile { - /// Keycloak: First name. - pub given_name: Option, - /// Keycloak: Combined name. Assume this to equal `format!("{given_name} {family name}")`. - pub full_name: Option, - /// Keycloak: Last name. - pub family_name: Option, - /// Keycloak: Username of the user. - pub preferred_username: String, -} - -#[derive(serde::Deserialize, Debug, Clone)] -pub struct Email { - /// Keycloak: Email address of the user. - pub email: Option, - /// Keycloak: Whether the users email is verified. - pub email_verified: bool, -} - -#[derive(serde::Deserialize, Debug, Clone)] -pub struct ProfileAndEmail { - #[serde(flatten)] - pub profile: Profile, - #[serde(flatten)] - pub email: Email, -} diff --git a/src/auth/error.rs b/src/auth/error.rs index 123394c..7034db2 100644 --- a/src/auth/error.rs +++ b/src/auth/error.rs @@ -1,200 +1,255 @@ -use std::{borrow::Cow, sync::Arc}; +//! The one error type the auth middleware produces, and how it reaches the client. +//! +//! Every variant is logged in full server-side and then sanitised by `classify` into the small +//! set of (status, `ErrorCode`, message) triples a caller is allowed to see — the reason a +//! token failed must not tell an attacker which check tripped. + +use std::sync::Arc; use axum::{ Json, http::StatusCode, response::{IntoResponse, Response}, }; -use serde_json::json; -use snafu::Snafu; +use thiserror::Error; use crate::auth::oidc_discovery; +use crate::core::errors::{ErrorCode, ErrorResponse}; -#[derive(Debug, Clone, Snafu)] -#[snafu(visibility(pub(crate)))] +/// Everything that can go wrong while authenticating a request. +#[derive(Debug, Clone, Error)] pub enum AuthError { /// OIDC discovery never happened. - #[snafu(display("Never discovered a OIDC configuration."))] + #[error("Never discovered a OIDC configuration.")] NoOidcDiscovery, /// OIDC discovery failed. - #[snafu(display("Could not discover OIDC configuration."))] + #[error("Could not discover OIDC configuration.")] OidcDiscovery { - #[snafu(backtrace)] source: oidc_discovery::RequestError, }, /// JWK set discovery never happened. - #[snafu(display("Never discovered a JWK set."))] + #[error("Never discovered a JWK set.")] NoJwkSetDiscovery, /// JWK endpoint was not a valid URL. - #[snafu(display("Could not parse the JWK endpoint."))] + #[error("Could not parse the JWK endpoint.")] JwkEndpoint { source: url::ParseError }, /// JWK set discovery failed. - #[snafu(display("Could not discover the JWK set."))] + #[error("Could not discover the JWK set.")] JwkSetDiscovery { source: oidc_discovery::RequestError, }, /// The 'Authorization' header was not present on a request. - #[snafu(display("The 'Authorization' header was not present on a request."))] + #[error("The 'Authorization' header was not present on a request.")] MissingAuthorizationHeader, /// The 'Authorization' header was present on a request but its value could not be parsed. /// This can occur if the header value did not solely contain visible ASCII characters. - #[snafu(display( + #[error( "The 'Authorization' header was present on a request but its value could not be parsed. Reason: {reason}" - ))] + )] InvalidAuthorizationHeader { reason: String }, /// The 'Authorization' header was present and could be parsed, but it did not contain the expected "Bearer {token}" format. - #[snafu(display( - "The 'Authorization' header did not contain the expected 'Bearer ...token' format." - ))] + #[error("The 'Authorization' header did not contain the expected 'Bearer ...token' format.")] MissingBearerToken, /// No query parameters were found on the request. - #[snafu(display("No query parameters were found on the request."))] + #[error("No query parameters were found on the request.")] MissingQueryParams, /// Query parameters were found on the request, but the expected token parameter wasn't. - #[snafu(display( - "Query parameters were found on the request, but the expected token parameter wasn't." - ))] + #[error("Query parameters were found on the request, but the expected token parameter wasn't.")] MissingTokenQueryParam, /// Query parameters were found on the request, and the expected token parameter was found, but it had no value assigned ("?token="). - #[snafu(display( + #[error( "Query parameters were found on the request, and the expected token parameter was found, but it had no value assigned (\"?token=\")." - ))] + )] EmptyTokenQueryParam, /// No JWT could be extracted from the request. - #[snafu(display("No JWT could be extracted from the request."))] + #[error("No JWT could be extracted from the request.")] MissingToken, /// The DecodingKey, required for decoding tokens, could not be created. - #[snafu(display( + #[error( "The DecodingKey, required for decoding tokens, could not be created. Source: {source}" - ))] + )] CreateDecodingKey { source: jsonwebtoken::errors::Error }, /// The JWT header could not be decoded. - #[snafu(display("The JWT header could not be decoded. Source: {source}"))] + #[error("The JWT header could not be decoded. Source: {source}")] DecodeHeader { source: jsonwebtoken::errors::Error }, - /// No decoding keys were fetched jet. - #[snafu(display("There were no decoding keys available."))] + /// No decoding keys were available to verify against. + /// + /// Unreachable via `Discovered::keys_for_kid`, which never yields an empty set. Kept as the + /// guard for a direct caller of `RawToken::decode_and_validate` handing over an empty slice. + #[error("There were no decoding keys available.")] NoDecodingKeys, + /// The token names a `kid` that the discovered key set does not contain. + /// + /// The one failure a signing-key rotation produces, and therefore the only one + /// `decode_and_validate` re-runs OIDC discovery for. Still returned after that refresh when the + /// `kid` was fabricated rather than rotated. + /// + /// Note: the `kid` is only ever logged server-side, never returned to the client. + #[error("The token names an unknown signing key: {kid}")] + UnknownSigningKey { kid: String }, + /// The JWT could not be decoded. - #[snafu(display("The JWT could not be decoded. Source: {source}"))] + #[error("The JWT could not be decoded. Source: {source}")] Decode { source: jsonwebtoken::errors::Error }, /// Parts of the JWT could not be parsed. - #[snafu(display("Parts of the JWT could not be parsed. Source: {source}"))] + #[error("Parts of the JWT could not be parsed. Source: {source}")] JsonParse { source: Arc }, /// The tokens lifetime is expired. - #[snafu(display("The tokens lifetime is expired."))] + #[error("The tokens lifetime is expired.")] TokenExpired, /// For a not further known reason, the token was deemed invalid - #[snafu(display( - "For a not further known reason, the token was deemed invalid: Reason: {reason}" - ))] + #[error("For a not further known reason, the token was deemed invalid: Reason: {reason}")] InvalidToken { reason: String }, - /// Note: The `IntoResponse` implementation will only show the provided role in a debug build! - #[snafu(display("An expected role (omitted for security reasons) was missing."))] + /// Note: The role is only ever logged server-side, never returned to the client. + #[error("An expected role was missing: {role}")] MissingExpectedRole { role: String }, /// An unexpected role was present. - #[snafu(display("An unexpected role was present."))] + #[error("An unexpected role was present.")] UnexpectedRole, } -impl IntoResponse for AuthError { - fn into_response(self) -> Response { - let (status, error_message) = match self { - err @ AuthError::NoOidcDiscovery => ( - StatusCode::INTERNAL_SERVER_ERROR, - Cow::Owned(err.to_string()), - ), - err @ AuthError::OidcDiscovery { source: _ } => ( - StatusCode::INTERNAL_SERVER_ERROR, - Cow::Owned(err.to_string()), - ), - err @ AuthError::NoJwkSetDiscovery => ( - StatusCode::INTERNAL_SERVER_ERROR, - Cow::Owned(err.to_string()), +/// Renders an error together with its full `source` chain as `outer: middle: inner`. +/// +/// Several variants above deliberately keep their own `Display` free of the source (so the +/// sanitised client message and the log line can differ), which means a plain `{err}` would drop +/// the actual cause. Used for the discovery failures logged in `instance.rs`. +pub(crate) fn error_chain(err: &dyn std::error::Error) -> String { + let mut chain = err.to_string(); + let mut source = err.source(); + while let Some(cause) = source { + chain.push_str(": "); + chain.push_str(&cause.to_string()); + source = cause.source(); + } + chain +} + +impl AuthError { + /// Stable, low-cardinality discriminant used as a structured log field. + fn kind(&self) -> &'static str { + match self { + AuthError::NoOidcDiscovery => "no_oidc_discovery", + AuthError::OidcDiscovery { .. } => "oidc_discovery", + AuthError::NoJwkSetDiscovery => "no_jwk_set_discovery", + AuthError::JwkEndpoint { .. } => "jwk_endpoint", + AuthError::JwkSetDiscovery { .. } => "jwk_set_discovery", + AuthError::MissingAuthorizationHeader => "missing_authorization_header", + AuthError::InvalidAuthorizationHeader { .. } => "invalid_authorization_header", + AuthError::MissingBearerToken => "missing_bearer_token", + AuthError::MissingQueryParams => "missing_query_params", + AuthError::MissingTokenQueryParam => "missing_token_query_param", + AuthError::EmptyTokenQueryParam => "empty_token_query_param", + AuthError::MissingToken => "missing_token", + AuthError::CreateDecodingKey { .. } => "create_decoding_key", + AuthError::DecodeHeader { .. } => "decode_header", + AuthError::NoDecodingKeys => "no_decoding_keys", + AuthError::UnknownSigningKey { .. } => "unknown_signing_key", + AuthError::Decode { .. } => "decode", + AuthError::JsonParse { .. } => "json_parse", + AuthError::TokenExpired => "token_expired", + AuthError::InvalidToken { .. } => "invalid_token", + AuthError::MissingExpectedRole { .. } => "missing_expected_role", + AuthError::UnexpectedRole => "unexpected_role", + } + } + + /// Maps the error onto its client-visible representation. + /// + /// The message deliberately never depends on the underlying source. Reporting *why* a token + /// was rejected — bad signature vs. wrong audience vs. expired — turns this endpoint into an + /// oracle for crafting tokens, so all rejections of the same class are indistinguishable. + /// The full detail is logged server-side instead. + fn classify(&self) -> (StatusCode, ErrorCode, &'static str) { + const UNAVAILABLE_MSG: &str = "Authentication service unavailable. Please try again later."; + const UNAUTHORIZED_MSG: &str = "Authentication required."; + + match self { + // The identity provider is unreachable or misconfigured — this is our fault, not the + // caller's, and 503 matches how `AppError::Database`/`Cache` signal upstream trouble. + AuthError::NoOidcDiscovery + | AuthError::OidcDiscovery { .. } + | AuthError::NoJwkSetDiscovery + | AuthError::JwkEndpoint { .. } + | AuthError::JwkSetDiscovery { .. } + | AuthError::CreateDecodingKey { .. } + | AuthError::NoDecodingKeys => ( + StatusCode::SERVICE_UNAVAILABLE, + ErrorCode::AuthUnavailable, + UNAVAILABLE_MSG, ), - err @ AuthError::JwkEndpoint { source: _ } => ( - StatusCode::INTERNAL_SERVER_ERROR, - Cow::Owned(err.to_string()), + + AuthError::TokenExpired => ( + StatusCode::UNAUTHORIZED, + ErrorCode::TokenExpired, + "Token expired.", ), - err @ AuthError::JwkSetDiscovery { source: _ } => ( - StatusCode::INTERNAL_SERVER_ERROR, - Cow::Owned(err.to_string()), + + // Everything from "no header" through "signature did not verify" collapses into one + // indistinguishable 401. `DecodeHeader` used to be a 400, which leaked that the token + // was malformed rather than merely invalid. + AuthError::MissingAuthorizationHeader + | AuthError::InvalidAuthorizationHeader { .. } + | AuthError::MissingBearerToken + | AuthError::MissingQueryParams + | AuthError::MissingTokenQueryParam + | AuthError::EmptyTokenQueryParam + | AuthError::MissingToken + | AuthError::DecodeHeader { .. } + | AuthError::UnknownSigningKey { .. } + | AuthError::Decode { .. } + | AuthError::InvalidToken { .. } => ( + StatusCode::UNAUTHORIZED, + ErrorCode::Unauthorized, + UNAUTHORIZED_MSG, ), - err @ AuthError::MissingAuthorizationHeader => { - (StatusCode::UNAUTHORIZED, Cow::Owned(err.to_string())) - } - err @ AuthError::InvalidAuthorizationHeader { reason: _ } => { - (StatusCode::UNAUTHORIZED, Cow::Owned(err.to_string())) - } - err @ AuthError::MissingBearerToken => { - (StatusCode::UNAUTHORIZED, Cow::Owned(err.to_string())) - } - err @ AuthError::MissingQueryParams => { - (StatusCode::UNAUTHORIZED, Cow::Owned(err.to_string())) - } - err @ AuthError::MissingTokenQueryParam => { - (StatusCode::UNAUTHORIZED, Cow::Owned(err.to_string())) - } - err @ AuthError::EmptyTokenQueryParam => { - (StatusCode::UNAUTHORIZED, Cow::Owned(err.to_string())) - } - err @ AuthError::MissingToken => { - (StatusCode::UNAUTHORIZED, Cow::Owned(err.to_string())) - } - err @ AuthError::CreateDecodingKey { source: _ } => ( - StatusCode::INTERNAL_SERVER_ERROR, - Cow::Owned(err.to_string()), + + AuthError::MissingExpectedRole { .. } | AuthError::UnexpectedRole => ( + StatusCode::FORBIDDEN, + ErrorCode::InsufficientPermissions, + "Insufficient permissions.", ), - err @ AuthError::DecodeHeader { source: _ } => { - (StatusCode::BAD_REQUEST, Cow::Owned(err.to_string())) - } - err @ AuthError::NoDecodingKeys => { - (StatusCode::UNAUTHORIZED, Cow::Owned(err.to_string())) - } - err @ AuthError::Decode { source: _ } => { - (StatusCode::UNAUTHORIZED, Cow::Owned(err.to_string())) - } - err @ AuthError::JsonParse { source: _ } => ( + + AuthError::JsonParse { .. } => ( StatusCode::INTERNAL_SERVER_ERROR, - Cow::Owned(err.to_string()), - ), - err @ AuthError::TokenExpired => { - (StatusCode::UNAUTHORIZED, Cow::Owned(err.to_string())) - } - err @ AuthError::InvalidToken { reason: _ } => { - (StatusCode::UNAUTHORIZED, Cow::Owned(err.to_string())) - } - AuthError::MissingExpectedRole { role } => ( - StatusCode::FORBIDDEN, - match cfg!(debug_assertions) { - true => Cow::Owned(format!("Missing expected role: {role}")), - false => Cow::Borrowed("Missing expected role"), - }, + ErrorCode::UnexpectedError, + "An unexpected error occurred.", ), - err @ AuthError::UnexpectedRole => (StatusCode::FORBIDDEN, Cow::Owned(err.to_string())), - }; - let body = Json(json!({ - "error": error_message, - })); - (status, body).into_response() + } + } +} + +impl IntoResponse for AuthError { + fn into_response(self) -> Response { + let (status, error_code, message) = self.classify(); + + // Full detail stays server-side; the client only ever sees the sanitised message above. + if status.is_server_error() { + tracing::error!(error.kind = self.kind(), error = %self, "Authentication failed"); + } else { + tracing::debug!(error.kind = self.kind(), error = %self, "Rejected request"); + } + + let body = ErrorResponse::new(status, error_code, message); + (status, Json(body)).into_response() } } diff --git a/src/auth/extract.rs b/src/auth/extract.rs index 16da52d..ae550c9 100644 --- a/src/auth/extract.rs +++ b/src/auth/extract.rs @@ -1,7 +1,12 @@ +//! Strategies for locating the raw JWT in an incoming request. +//! +//! The layer holds a list of these and uses the first one that yields a token. ISM keeps the +//! default (`Authorization: Bearer …`) only. + use std::{borrow::Cow, sync::Arc}; use axum::extract::Request; -use nonempty::NonEmpty; +use url::form_urlencoded; use crate::auth::error::AuthError; @@ -45,12 +50,19 @@ impl TokenExtractor for AuthHeaderTokenExtractor { /// /// SECURITY: This extractor should be used with caution! /// Only use it if you are informed about the security implication of providing tokens through query parameters. +/// +/// Not wired up in ISM — the layer uses the header extractor only. Kept because a browser +/// `WebSocket` cannot set request headers, so authenticating `/api/wss` will need this or an +/// equivalent. See `docs/auth.md`. +#[allow(dead_code)] #[derive(Debug, Clone)] pub struct QueryParamTokenExtractor { + /// Name of the query parameter carrying the token. pub key: String, } impl QueryParamTokenExtractor { + /// Builds an extractor reading the token from the query parameter named `key`. pub fn extracting_key(key: impl Into) -> Self { Self { key: key.into() } } @@ -66,25 +78,38 @@ impl TokenExtractor for QueryParamTokenExtractor { fn extract<'a>(&self, request: &'a Request) -> Result, AuthError> { let query = request.uri().query().ok_or(AuthError::MissingQueryParams)?; - let mut tokens = serde_querystring::DuplicateQS::parse(query.as_bytes()) - .values(self.key.as_bytes()) - .unwrap_or_default() - .into_iter(); + // First occurrence wins, matching how a duplicated parameter was resolved before. + let (_, token) = form_urlencoded::parse(query.as_bytes()) + .find(|(key, _)| key.as_ref() == self.key.as_str()) + .ok_or(AuthError::MissingTokenQueryParam)?; - let first_token = tokens - .next() - .ok_or(AuthError::MissingTokenQueryParam)? - .ok_or(AuthError::EmptyTokenQueryParam)?; + if token.is_empty() { + return Err(AuthError::EmptyTokenQueryParam); + } - let first_token = std::str::from_utf8(first_token.as_ref()).expect("Valid UTF-8"); + // `form_urlencoded` decodes lossily, so `?token=%FF` arrives as U+FFFD instead of failing. + // A JWT is base64url segments joined by dots and therefore pure ASCII, so anything outside + // that range is rejected here rather than handed to the decoder as a silently mangled + // string. (The previous implementation returned raw bytes and had to guard UTF-8 itself — + // before that it `expect`ed, which made a malformed query string a remote panic.) + if !token.is_ascii() { + return Err(AuthError::InvalidToken { + reason: "token query parameter contained non-ASCII characters".to_owned(), + }); + } - Ok(ExtractedToken::Owned(first_token.to_owned())) + // Borrowed when the value needed no unescaping, owned otherwise — see `ExtractedToken`. + Ok(token) } } -pub(crate) fn extract_jwt<'a>( +/// Returns the token from the first extractor that succeeds, or `None` if all of them fail. +/// +/// An empty slice yields `None`, which the caller turns into `AuthError::MissingToken` — a layer +/// configured without extractors rejects every request rather than letting one through. +pub fn extract_jwt<'a>( request: &'a Request, - extractors: &NonEmpty>, + extractors: &[Arc], ) -> Option> { for extractor in extractors { match extractor.extract(request) { @@ -96,3 +121,145 @@ pub(crate) fn extract_jwt<'a>( } None } + +#[cfg(test)] +mod test { + use super::*; + use axum::body::Body; + + fn request(uri: &str) -> Request { + Request::builder() + .uri(uri) + .body(Body::empty()) + .expect("valid test request") + } + + #[test] + fn reads_the_configured_query_parameter() { + let request = request("http://localhost/api/v1/wss?token=abc.def.ghi"); + let token = QueryParamTokenExtractor::default() + .extract(&request) + .expect("token to be extracted"); + assert_eq!(token, "abc.def.ghi"); + } + + #[test] + fn percent_decodes_the_value() { + let request = request("http://localhost/?token=a%2Eb"); + let token = QueryParamTokenExtractor::default() + .extract(&request) + .expect("token to be extracted"); + assert_eq!(token, "a.b"); + } + + #[test] + fn ignores_other_parameters_and_takes_the_first_duplicate() { + let request = request("http://localhost/?last_seq=7&token=first&token=second"); + let token = QueryParamTokenExtractor::default() + .extract(&request) + .expect("token to be extracted"); + assert_eq!(token, "first"); + } + + #[test] + fn honours_a_custom_key() { + let request = request("http://localhost/?token=wrong&jwt=right"); + let token = QueryParamTokenExtractor::extracting_key("jwt") + .extract(&request) + .expect("token to be extracted"); + assert_eq!(token, "right"); + } + + #[test] + fn distinguishes_absent_missing_and_empty_parameters() { + let no_query = request("http://localhost/api/v1/wss"); + assert!(matches!( + QueryParamTokenExtractor::default().extract(&no_query), + Err(AuthError::MissingQueryParams) + )); + + let other_key = request("http://localhost/?last_seq=7"); + assert!(matches!( + QueryParamTokenExtractor::default().extract(&other_key), + Err(AuthError::MissingTokenQueryParam) + )); + + let empty_value = request("http://localhost/?token="); + assert!(matches!( + QueryParamTokenExtractor::default().extract(&empty_value), + Err(AuthError::EmptyTokenQueryParam) + )); + } + + #[test] + fn rejects_a_non_ascii_token_instead_of_mangling_it() { + // `%FF` is not valid UTF-8; percent-decoding it lossily yields U+FFFD, which must not be + // passed on to the JWT decoder as if it were the token the caller sent. + let request = request("http://localhost/?token=%FF"); + assert!(matches!( + QueryParamTokenExtractor::default().extract(&request), + Err(AuthError::InvalidToken { .. }) + )); + } + + #[test] + fn header_extractor_requires_the_bearer_prefix() { + let bearer = Request::builder() + .uri("http://localhost/") + .header(http::header::AUTHORIZATION, "Bearer abc.def.ghi") + .body(Body::empty()) + .expect("valid test request"); + assert_eq!( + AuthHeaderTokenExtractor {} + .extract(&bearer) + .expect("token to be extracted"), + "abc.def.ghi" + ); + + let basic = Request::builder() + .uri("http://localhost/") + .header(http::header::AUTHORIZATION, "Basic abc") + .body(Body::empty()) + .expect("valid test request"); + assert!(matches!( + AuthHeaderTokenExtractor {}.extract(&basic), + Err(AuthError::MissingBearerToken) + )); + + let none = request("http://localhost/"); + assert!(matches!( + AuthHeaderTokenExtractor {}.extract(&none), + Err(AuthError::MissingAuthorizationHeader) + )); + } + + #[test] + fn extract_jwt_falls_through_to_the_next_extractor() { + let extractors: Vec> = vec![ + Arc::new(AuthHeaderTokenExtractor {}), + Arc::new(QueryParamTokenExtractor::default()), + ]; + + let with_query = request("http://localhost/api/v1/wss?token=from.query"); + assert_eq!( + extract_jwt(&with_query, &extractors).as_deref(), + Some("from.query") + ); + + let neither = request("http://localhost/api/v1/wss"); + assert_eq!(extract_jwt(&neither, &extractors), None); + } + + #[test] + fn extract_jwt_without_extractors_fails_closed() { + // The list is a plain `Vec` rather than a non-empty type, so the empty case is + // representable. It must reject, never pass a request through unauthenticated. + let with_credentials = Request::builder() + .uri("http://localhost/?token=from.query") + .header(http::header::AUTHORIZATION, "Bearer abc.def.ghi") + .body(Body::empty()) + .expect("valid test request"); + + assert_eq!(extract_jwt(&with_credentials, &[]), None); + } +} diff --git a/src/auth/instance.rs b/src/auth/instance.rs index 9b1ffc8..7157db6 100644 --- a/src/auth/instance.rs +++ b/src/auth/instance.rs @@ -1,24 +1,41 @@ -use std::ops::Deref; +//! OIDC discovery and the cached verification keys every request is validated against. +//! +//! A `KeycloakAuthInstance` performs its first discovery while it is being built, and fails +//! construction if that does not succeed — a process that cannot verify a single token is more +//! useful dead, where an orchestrator will restart it, than alive serving blanket 503s. +//! +//! Afterwards the key set refreshes purely on demand: no timer, no background task, and no traffic +//! towards Keycloak while nothing is rotating. A request whose token names an unknown `kid` +//! triggers one re-discovery and retries within that same request, bounded by +//! `KeycloakConfig::refresh_timeout` so a slow Keycloak cannot stall it, and rate-limited by +//! `KeycloakConfig::min_refresh_interval` so a flood of fabricated `kid`s cannot become a request +//! storm. A refresh that fails leaves the previously discovered keys in place. use educe::Educe; -use snafu::ResultExt; +use jsonwebtoken::DecodingKey; +use jsonwebtoken::jwk::{AlgorithmParameters, JwkSet, PublicKeyUse}; +use std::future::Future; +use std::ops::Deref; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::time::{Duration, Instant}; use tokio::sync::RwLockReadGuard; -use tracing::Instrument; -use try_again::{StdDuration, delay, retry_async}; +use tokio::time::timeout; +use tracing::{debug, error, info, warn}; use typed_builder::TypedBuilder; use url::Url; use crate::auth::{ action::Action, - error::{AuthError, JwkEndpointSnafu, JwkSetDiscoverySnafu, OidcDiscoverySnafu}, + error::{AuthError, error_chain}, oidc::OidcConfig, oidc_discovery, }; +/// The realm's `.well-known/openid-configuration` URL. #[derive(Debug, Clone)] -pub(crate) struct OidcDiscoveryEndpoint(pub(crate) Url); +pub struct OidcDiscoveryEndpoint(pub Url); impl OidcDiscoveryEndpoint { - pub(crate) fn from_server_and_realm(server: Url, realm: &str) -> Self { + pub fn from_server_and_realm(server: Url, realm: &str) -> Self { let mut url = server; url.path_segments_mut() .expect("URL not to be a 'cannot-be-a-base' URL. We have to append segments.") @@ -43,27 +60,66 @@ pub struct KeycloakConfig { /// The realm of you Keycloak server. pub realm: String, - /// The retry strategy to be used: (maximum tries, delay in seconds). - #[builder(default = (5, 1))] - pub retry: (usize, u64), + /// Retry strategy for the *initial* discovery: (maximum attempts, delay in seconds). + /// + /// `maximum attempts` counts the first try, so `(5, 1)` means five requests one second apart, + /// not five retries on top of an initial one. + /// + /// Generous by default, because failing here aborts startup: in a compose stack Keycloak is + /// routinely slower to become ready than ISM is, and crash-looping through that is noise. It + /// only delays the abort — a wrong realm or an unreachable host still brings the process down, + /// just ~90s later. + #[builder(default = (18, 5))] + pub startup_retry: (usize, u64), + + /// Retry strategy for an on-demand refresh, in the same shape as `startup_retry`. + /// + /// One attempt, because this runs inside a request. A reachable Keycloak answers in + /// milliseconds; an unreachable one must not be waited on while a caller holds a connection + /// open. If the single attempt fails the cached keys still stand and the request gets a 401. + #[builder(default = (1, 0))] + pub refresh_retry: (usize, u64), + + /// Hard ceiling on how long a request may be held while an on-demand refresh runs. + /// + /// Without it, `refresh_retry` bounds the number of attempts but not the time each one takes, + /// and a Keycloak that accepts connections without answering would hang the request for as + /// long as the HTTP client allows. + #[builder(default = std::time::Duration::from_secs(2))] + pub refresh_timeout: Duration, + + /// Minimum time between two OIDC discoveries. + /// + /// The JWKS is refreshed on demand, when a token names a `kid` the cached key set does not + /// contain. Since that trigger is caller-controlled, this cooldown is what keeps a flood of + /// fabricated `kid`s from turning into a request storm against Keycloak. + #[builder(default = std::time::Duration::from_secs(30))] + pub min_refresh_interval: Duration, +} + +/// A verification key together with the `kid` it was published under, so incoming tokens can be +/// matched to a single key instead of being tried against every key in the set. +pub struct JwkDecodingKey { + pub kid: Option, + pub key: DecodingKey, } fn debug_decoding_keys( - decoding_keys: &[jsonwebtoken::DecodingKey], + decoding_keys: &[JwkDecodingKey], f: &mut std::fmt::Formatter<'_>, ) -> std::fmt::Result { f.write_fmt(format_args!("len: {}", decoding_keys.len())) } -#[derive(TypedBuilder, Educe)] +/// One successful discovery: the raw documents plus the keys parsed out of them. +#[derive(Educe)] #[educe(Debug)] -pub(crate) struct DiscoveredData { +pub struct DiscoveredData { + pub oidc_config: OidcConfig, #[allow(dead_code)] - pub(crate) oidc_config: OidcConfig, - #[allow(dead_code)] - pub(crate) jwk_set: jsonwebtoken::jwk::JwkSet, + pub jwk_set: JwkSet, #[educe(Debug(method(debug_decoding_keys)))] - pub(crate) decoding_keys: Vec, + pub decoding_keys: Vec, } /// The KeycloakAuthInstance is responsible for performing OIDC discovery @@ -75,162 +131,305 @@ pub(crate) struct DiscoveredData { #[derive(Debug)] pub struct KeycloakAuthInstance { #[allow(dead_code)] - pub(crate) id: uuid::Uuid, - #[allow(dead_code)] - pub(crate) config: KeycloakConfig, - pub(crate) oidc_discovery_endpoint: OidcDiscoveryEndpoint, - pub(crate) discovery: Action>, + pub id: uuid::Uuid, + pub config: KeycloakConfig, + pub oidc_discovery_endpoint: OidcDiscoveryEndpoint, + /// Holds the last *successful* discovery. A failed refresh leaves the previous one in place, + /// so this is `Some` from construction onwards — `KeycloakAuthInstance::new` does not return + /// until one discovery has succeeded. + pub discovery: Action, + /// Monotonic base for `last_refresh_ms`. + started_at: Instant, + /// Millis since `started_at` at which the last discovery was started. Drives the cooldown that + /// keeps a flood of unknown `kid`s from hammering Keycloak. + /// + /// Starts at `0` against a `started_at` stamped once the initial discovery has succeeded, so + /// the cooldown applies from startup — nothing can have rotated in the moment since. + /// + /// An atomic rather than a lock: this is read on *every* authenticated request, and a relaxed + /// load neither suspends nor bounces a cache line between cores the way even an uncontended + /// mutex acquire does. The compare-exchange in `refresh_for_request` also expresses the + /// "exactly one caller starts the refresh" rule more directly than holding a lock across the + /// check and the stamp. + last_refresh_ms: AtomicU64, } impl KeycloakAuthInstance { - /// Creates a new KeycloakAuthInstance. This immediately starts an initial OIDC discovery process. - /// The `is_operational` method will tell you if discovery has taken place. - /// This may be useful in determining service health. - pub fn new(kc_config: KeycloakConfig) -> Self { + /// Creates a new `KeycloakAuthInstance`, running the initial OIDC discovery to completion + /// before returning. + /// + /// Deliberately fallible and deliberately awaited here rather than dispatched into a task: + /// without discovered keys not one token can be verified, so a process that starts anyway only + /// serves blanket 503s, forever, since nothing re-runs discovery on a timer. Returning `Err` — + /// which the caller turns into a startup abort — surfaces a bad realm or an unreachable + /// Keycloak where it can actually be seen and restarted. + /// + /// Afterwards the key set is refreshed purely on demand: a token naming a `kid` outside the + /// cached set makes `decode_and_validate` re-run discovery and retry within the same request. + /// Signing-key rotation therefore costs added latency on exactly one request and is never + /// visible to a client — no timer, no background task, and no traffic towards Keycloak while + /// nothing is rotating. + pub async fn new(kc_config: KeycloakConfig) -> Result { let id = uuid::Uuid::now_v7(); let oidc_discovery_endpoint = OidcDiscoveryEndpoint::from_server_and_realm( kc_config.server.clone(), &kc_config.realm, ); - let kc_server = kc_config.server.to_string(); - let kc_realm = kc_config.realm.clone(); + // The initial run, on the generous startup budget. Its result is installed directly, so a + // failure is returned to the caller instead of being buried in a spawned task. + let discovered = perform_oidc_discovery( + oidc_discovery_endpoint.clone(), + kc_config.startup_retry.0, + Duration::from_secs(kc_config.startup_retry.1), + ) + .await?; let discovery = Action::new(move |oidc_discovery_endpoint: &OidcDiscoveryEndpoint| { - let kc_server = kc_server.clone(); - let kc_realm = kc_realm.clone(); let oidc_discovery_endpoint = oidc_discovery_endpoint.clone(); async move { - let span = tracing::span!( - tracing::Level::INFO, - "perform_oidc_discovery", - kc_instance_id = ?id, - kc_server, - kc_realm, - oidc_discovery_endpoint = ?oidc_discovery_endpoint.0.to_string() - ); perform_oidc_discovery( oidc_discovery_endpoint, - kc_config.retry.0, - std::time::Duration::from_secs(kc_config.retry.1), + kc_config.refresh_retry.0, + Duration::from_secs(kc_config.refresh_retry.1), ) - .instrument(span) .await + .ok() } }); - discovery.dispatch(oidc_discovery_endpoint.clone()); + discovery.seed(discovered).await; - Self { + Ok(Self { id, config: kc_config, oidc_discovery_endpoint, discovery, - } + // Discovery was just dispatched above, so the key set counts as fresh from now on. + started_at: Instant::now(), + last_refresh_ms: AtomicU64::new(0), + }) + } + + /// Milliseconds elapsed since this instance was created. + fn now_ms(&self) -> u64 { + self.started_at.elapsed().as_millis() as u64 } - pub(crate) async fn perform_oidc_discovery(&self) { - // Wait for an ongoing discovery or dispatch a new discovery process. + /// Refreshes the OIDC configuration and JWKS, subject to the configured cooldown. + /// + /// Callers may invoke this per failing request; the cooldown is what keeps that safe. + pub async fn refresh_for_request(&self) { + // Created *before* testing `is_pending`, and that order is the point. `notify_waiters` + // stores no permit and wakes only waiters that already exist when it fires; a `Notified` + // counts as registered from the moment it is created, not when it is first polled. + // Reversed, a discovery completing in the gap between the check and this line would leave + // the caller waiting for the *next* discovery — and nothing here runs on a timer, so there + // may not be one. + let notified = self.discovery.notified(); + + // Wait for an ongoing discovery rather than starting a competing one. The timeout is a + // second, independent guard: it bounds this wait no matter what happens to that discovery. if self.discovery.is_pending() { - self.discovery.notified().await; - } else { - self.discovery - .dispatch(self.oidc_discovery_endpoint.clone()) + if timeout(self.config.refresh_timeout, notified) .await - .expect("No Join error"); + .is_err() + { + debug!("Gave up waiting for an in-flight OIDC discovery."); + } + return; } - } - /// Returns true after a successful OIDC discovery. - pub async fn is_operational(&self) -> bool { - self.discovery - .value() - .await - .as_ref() - .is_some_and(|it| it.is_ok()) + let now = self.now_ms(); + let last = self.last_refresh_ms.load(Ordering::Acquire); + if now.saturating_sub(last) < self.config.min_refresh_interval.as_millis() as u64 { + debug!("Skipping OIDC discovery: still within the refresh cooldown."); + return; + } + + // Claim the refresh before dispatching. Exactly one of any number of concurrent callers + // wins the exchange and starts discovery; the rest return and keep serving the current + // keys. This also covers callers arriving before the dispatched task flips `pending`. + if self + .last_refresh_ms + .compare_exchange(last, now, Ordering::AcqRel, Ordering::Acquire) + .is_err() + { + return; + } + + let dispatched = self + .discovery + .dispatch(self.oidc_discovery_endpoint.clone()); + + match timeout(self.config.refresh_timeout, dispatched).await { + Ok(Ok(())) => {} + Ok(Err(err)) => { + // Happens when the runtime is shutting down and the task is aborted. The next + // request will retry; there is nothing to recover here. + warn!(error = %err, "OIDC discovery task did not complete."); + } + Err(_) => { + // The discovery keeps running in the background and will install its result if it + // succeeds. We simply stop holding the request open for it. + debug!( + timeout = ?self.config.refresh_timeout, + "OIDC discovery outlasted the request budget. Continuing with the cached keys." + ); + } + } } - pub(crate) async fn decoding_keys(&self) -> DecodingKeys<'_> { - DecodingKeys { + /// A read guard over the most recent discovery result. + pub async fn discovered(&self) -> Discovered<'_> { + Discovered { // Note: Tokio's RwLock implementation prioritizes write access to prevent starvation. This is fine and will not block writes. lock: self.discovery.value().await, } } + + /// Builds an instance without contacting Keycloak, for tests that only need the surrounding + /// wiring rather than a working key set. + /// + /// The key set stays empty, so every token validated through it is rejected — which is why + /// this is test-only: it is precisely the state `new` exists to make unreachable. + #[cfg(test)] + pub(crate) fn without_discovery(kc_config: KeycloakConfig) -> Self { + let oidc_discovery_endpoint = OidcDiscoveryEndpoint::from_server_and_realm( + kc_config.server.clone(), + &kc_config.realm, + ); + + Self { + id: uuid::Uuid::now_v7(), + config: kc_config, + oidc_discovery_endpoint, + discovery: Action::new(|_: &OidcDiscoveryEndpoint| async move { None }), + started_at: Instant::now(), + last_refresh_ms: AtomicU64::new(0), + } + } } -pub(crate) struct DecodingKeys<'a> { - lock: RwLockReadGuard<'a, Option>>, +/// Borrowed view onto the cached discovery result, held for the duration of one validation. +pub struct Discovered<'a> { + lock: RwLockReadGuard<'a, Option>, } -impl DecodingKeys<'_> { - /// Iterate over the currently known decoding keys. - /// This may return an empty iterator if no keys are known! - pub(crate) fn iter(&self) -> impl Iterator { +impl Discovered<'_> { + /// The issuer Keycloak advertises in its discovery document. + /// + /// Only `None` if the cached data were somehow never installed, which construction rules out. + pub fn issuer(&self) -> Option<&str> { self.lock .as_ref() - .map(|r| r.as_ref()) - .and_then(|r| r.ok()) - .map(|d| d.decoding_keys.iter()) - .unwrap_or_default() + .map(|d| d.oidc_config.standard_claims.issuer.as_str()) } + + /// The keys installed by the last successful discovery. Pass them to `keys_for_kid`. + pub fn decoding_keys(&self) -> &[JwkDecodingKey] { + self.lock.as_ref().map_or(&[], |d| &d.decoding_keys) + } +} + +/// The keys published under `kid`, or `None` when none of them carries it. +/// +/// A miss is the single failure shape a signing-key rotation can produce, and it is what +/// `decode_and_validate` gates rediscovery on — the signature check itself cannot tell a rotated +/// key from a forged token, so the decision has to be made here, before any crypto. +/// +/// Deliberately no fall back to the whole key set on a miss: that both destroyed the signal (every +/// unknown `kid` then failed the same way a tampered token does) and turned each such request into +/// N public-key operations. +/// +/// A free function over the key slice rather than a method on `Discovered`, so it can be tested +/// against a hand-built key set instead of a live discovery. +pub fn keys_for_kid<'a>(keys: &'a [JwkDecodingKey], kid: &str) -> Option> { + let matching: Vec<_> = keys + .iter() + .filter(|it| it.kid.as_deref() == Some(kid)) + .map(|it| &it.key) + .collect(); + + (!matching.is_empty()).then_some(matching) +} + +/// Runs `op` until it succeeds, at most `max_attempts` times, sleeping `delay` in between. +/// +/// Every error is retried, without inspecting it: the only caller talks to Keycloak during +/// discovery, where a misconfiguration and a dropped connection are equally worth another attempt. +/// Intermediate failures are logged at DEBUG; the caller decides what a terminal failure means. +/// +/// Spelled with an explicit `Fn() -> Future` bound rather than `AsyncFn`: the latter cannot express +/// that the returned future is `Send`, which the spawned discovery task requires. +async fn retry_fixed(max_attempts: usize, delay: Duration, op: F) -> Result +where + E: std::error::Error, + F: Fn() -> Fut, + Fut: Future>, +{ + // The final attempt is the one after the loop, so it is not followed by a pointless sleep. + for attempt in 1..max_attempts { + match op().await { + Ok(value) => return Ok(value), + Err(err) => { + warn!( + attempt, + max_attempts, + err = error_chain(&err), + "Attempt failed. Retrying after delay." + ); + tokio::time::sleep(delay).await; + } + } + } + op().await } async fn perform_oidc_discovery( oidc_discovery_endpoint: OidcDiscoveryEndpoint, - num_retries: usize, - fixed_delay: StdDuration, + max_attempts: usize, + fixed_delay: Duration, ) -> Result { - tracing::info!("Starting OIDC discovery."); + // A successful run is routine and stays at DEBUG. Failures below remain at ERROR: discovery + // only ever runs at startup or on a suspected rotation, so a failure always means either the + // process is about to abort or a rotation went unpicked-up. + debug!("Starting OIDC discovery."); // Load OIDC config. - let oidc_config = retry_async(async move || { + let oidc_config = retry_fixed(max_attempts, fixed_delay, async || { oidc_discovery::retrieve_oidc_config(oidc_discovery_endpoint.0.clone()) .await - .context(OidcDiscoverySnafu {}) + .map_err(|source| AuthError::OidcDiscovery { source }) }) - .delayed_by(delay::Fixed::of(fixed_delay).take(num_retries)) .await .inspect_err(|err| { - tracing::error!( - err = snafu::Report::from_error(err.clone()).to_string(), - "Could not retrieve OIDC config." - ); + error!(err = error_chain(err), "Could not retrieve OIDC config."); })?; // Parse JWK endpoint if OIDC config is available. let jwk_set_endpoint = Url::parse(&oidc_config.standard_claims.jwks_uri) - .context(JwkEndpointSnafu {}) + .map_err(|source| AuthError::JwkEndpoint { source }) .inspect_err(|err| { - tracing::error!( - err = snafu::Report::from_error(err.clone()).to_string(), + error!( + err = error_chain(err), "Could not retrieve jwk_set_endpoint_url." ); })?; // Load JWK set if endpoint was parsable. - let jwk_set = retry_async(async move || { + let jwk_set = retry_fixed(max_attempts, fixed_delay, async || { oidc_discovery::retrieve_jwk_set(jwk_set_endpoint.clone()) .await - .context(JwkSetDiscoverySnafu {}) + .map_err(|source| AuthError::JwkSetDiscovery { source }) }) - .delayed_by(delay::Fixed::of(fixed_delay).take(num_retries)) .await .inspect_err(|err| { - tracing::error!( - err = snafu::Report::from_error(err.clone()).to_string(), - "Could not retrieve jwk_set." - ); + error!(err = error_chain(err), "Could not retrieve jwk_set."); })?; - let num_keys = jwk_set.keys.len(); - tracing::info!( - "Received new jwk_set containing {num_keys} {}.", - match num_keys { - 1 => "key", - _ => "keys", - } - ); + debug!(num_keys = jwk_set.keys.len(), "Received new jwk_set."); // Create DecodingKey instances from received JWKs. let decoding_keys = parse_jwks(&jwk_set); @@ -242,14 +441,52 @@ async fn perform_oidc_discovery( }) } -fn parse_jwks(jwk_set: &jsonwebtoken::jwk::JwkSet) -> Vec { - jwk_set.keys.iter().filter_map(|jwk| { - match jsonwebtoken::DecodingKey::from_jwk(jwk) { - Ok(decoding_key) => Some(decoding_key), +fn parse_jwks(jwk_set: &JwkSet) -> Vec { + let mut decoding_keys = Vec::with_capacity(jwk_set.keys.len()); + let mut usable_keys = Vec::with_capacity(jwk_set.keys.len()); + + for jwk in &jwk_set.keys { + // Keycloak publishes its RSA-OAEP encryption key in the same set. Loading it as a + // verification key is pointless and widens what a token can be verified against. + if !matches!(jwk.common.public_key_use,Some(PublicKeyUse::Signature) | None) { + continue; + } + + match DecodingKey::from_jwk(jwk) { + Ok(key) => { + usable_keys.push(format!( + "kid={} kty={} alg={:?}", + jwk.common.key_id.as_deref().unwrap_or(""), + key_type_name(&jwk.algorithm), + jwk.common.key_algorithm, + )); + decoding_keys.push(JwkDecodingKey { kid: jwk.common.key_id.clone(), key, }); + } Err(err) => { - tracing::error!(?err, "Received JWK from Keycloak which could not be parsed as a DecodingKey. Ignoring the JWK."); - None + error!( + ?err, + kid = ?jwk.common.key_id, + "Received JWK from Keycloak which could not be parsed as a DecodingKey. Ignoring the JWK." + ); } } - }).collect::>() + } + + if decoding_keys.is_empty() { + warn!("No public key for signature verification available. Every token verification will fail."); + } else { + info!(keys = ?usable_keys,"Public key(s) for signature verification available."); + } + decoding_keys +} + +/// Short `kty` label for a JWK, so key details can be logged without dumping the key material. +fn key_type_name(algorithm: &AlgorithmParameters) -> &'static str { + match algorithm { + AlgorithmParameters::EllipticCurve(_) => "EC", + AlgorithmParameters::RSA(_) => "RSA", + AlgorithmParameters::OctetKey(_) => "oct", + AlgorithmParameters::OctetKeyPair(_) => "OKP", + _ => "unknown", + } } diff --git a/src/auth/layer.rs b/src/auth/layer.rs index 602e4d5..69ab061 100644 --- a/src/auth/layer.rs +++ b/src/auth/layer.rs @@ -1,4 +1,11 @@ -use nonempty::NonEmpty; +//! The tower `Layer` carrying the auth configuration. +//! +//! Built once in `router::init_auth`. `KeycloakAuthService` holds it behind an `Arc`, so nothing +//! in here is copied per request — keep it that way when adding fields. +//! +//! `instance` stays an `Arc` in its own right: a `KeycloakAuthInstance` is meant to be shared +//! across several layers so they do not each run their own OIDC discovery. + use serde::de::DeserializeOwned; use std::collections::HashMap; use std::marker::PhantomData; @@ -6,20 +13,17 @@ use std::{fmt::Debug, sync::Arc}; use tower::Layer; use typed_builder::TypedBuilder; -use crate::auth::decode::{ - KeycloakToken, ProfileAndEmail, RawToken, decode_and_validate, parse_raw_claims, -}; +use crate::auth::decode::{RawToken, ValidationPolicy, decode_and_validate, parse_raw_claims}; use crate::auth::error::AuthError; use crate::auth::extract::TokenExtractor; +use crate::auth::token::{KeycloakToken, ProfileAndEmail}; use crate::auth::{instance::KeycloakAuthInstance, role::Role, service::KeycloakAuthService}; use super::PassthroughMode; -extern crate alloc; - /// Add this layer to a router to protect the contained route handlers. /// Authentication happens by looking for the `Authorization` header on requests and parsing the contained JWT bearer token. -/// See the crate level documentation for how this layer can be created and used. +/// See the module level documentation and `docs/auth.md` for how this layer can be created and used. #[derive(Clone, TypedBuilder)] pub struct KeycloakAuthLayer where @@ -38,8 +42,10 @@ where #[builder(default = false)] pub persist_raw_claims: bool, - /// Allowed values of the JWT 'aud' (audiences) field. Token validation will fail immediately if this is left empty! - pub expected_audiences: Vec, + /// Rules incoming tokens are validated against: accepted audiences, authorized parties and + /// signature algorithms. See `ValidationPolicy`. + #[builder(setter(into))] + pub validation_policy: ValidationPolicy, /// These roles are always required. /// Should a route protected by this layer be accessed by a user not having this role, an error is generated. @@ -48,9 +54,12 @@ where #[builder(default = vec![], setter(into))] pub required_roles: Vec, - /// Specifies where the token is expected to be found. - #[builder(default = nonempty::nonempty![Arc::new(crate::auth::extract::AuthHeaderTokenExtractor {})])] - pub token_extractors: NonEmpty>, + /// Specifies where the token is expected to be found. Tried in order, first hit wins. + /// + /// An empty list is accepted and fails closed: no extractor yields a token, so every request + /// is rejected with `AuthError::MissingToken`. + #[builder(default = vec![Arc::new(crate::auth::extract::AuthHeaderTokenExtractor {})], setter(into))] + pub token_extractors: Vec>, #[builder(default = uuid::Uuid::now_v7(), setter(skip))] id: uuid::Uuid, @@ -80,12 +89,17 @@ where let raw_claims = decode_and_validate( self.instance.as_ref(), RawToken(raw_token), - &self.expected_audiences, + &self.validation_policy, ) .await?; - parse_raw_claims::(raw_claims, self.persist_raw_claims, &self.required_roles) - .await + parse_raw_claims::( + raw_claims, + self.persist_raw_claims, + &self.required_roles, + &self.validation_policy, + ) + .await } } @@ -119,56 +133,104 @@ where mod test { use std::sync::Arc; - use nonempty::NonEmpty; use url::Url; use crate::auth::{ - PassthroughMode, + AppRole, PassthroughMode, + decode::ValidationPolicy, extract::{AuthHeaderTokenExtractor, QueryParamTokenExtractor, TokenExtractor}, instance::{KeycloakAuthInstance, KeycloakConfig}, layer::KeycloakAuthLayer, }; + fn test_policy() -> ValidationPolicy { + ValidationPolicy::new( + vec![String::from("account")], + vec![], + &[String::from("RS256")], + ) + .expect("valid policy") + } + #[tokio::test] async fn build_basic_layer() { - let instance = KeycloakAuthInstance::new( + // `without_discovery`, because `new` now insists on reaching Keycloak and this test is + // about the builder wiring, not about discovery. + let instance = KeycloakAuthInstance::without_discovery( KeycloakConfig::builder() .server(Url::parse("https://localhost:8443/").expect("invalid url")) .realm(String::from("MyRealm")) - .retry((10, 2)) .build(), ); - let _layer = KeycloakAuthLayer::::builder() + let _layer = KeycloakAuthLayer::::builder() .instance(instance) .passthrough_mode(PassthroughMode::Block) - .expected_audiences(vec![String::from("account")]) + .validation_policy(test_policy()) .build(); } + #[test] + fn policy_rejects_symmetric_and_mixed_algorithms() { + // An `oct` JWK in the key set plus an HS entry here is the RS256 -> HS256 key-confusion + // setup, so the symmetric family is refused outright. + assert!( + ValidationPolicy::new( + vec![String::from("account")], + vec![], + &[String::from("HS256")] + ) + .is_err() + ); + + // `jsonwebtoken` fails verification when the allow-list spans families; reject it here + // with an explanation instead of at request time with a blanket 401. + assert!( + ValidationPolicy::new( + vec![String::from("account")], + vec![], + &[String::from("RS256"), String::from("ES256")], + ) + .is_err() + ); + + assert!(ValidationPolicy::new(vec![String::from("account")], vec![], &[]).is_err()); + assert!( + ValidationPolicy::new(vec![], vec![], &[String::from("RS256")]).is_err(), + "an empty audience list would disable the audience check" + ); + + // Same family, multiple entries: allowed. + assert!( + ValidationPolicy::new( + vec![String::from("account")], + vec![], + &[String::from("RS256"), String::from("PS512")], + ) + .is_ok() + ); + } + #[tokio::test] async fn build_full_layer() { - let instance = KeycloakAuthInstance::new( + let instance = KeycloakAuthInstance::without_discovery( KeycloakConfig::builder() .server(Url::parse("https://localhost:8443/").expect("invalid url")) .realm(String::from("MyRealm")) - .retry((10, 2)) .build(), ); - let _layer = KeycloakAuthLayer::::builder() + let _layer = KeycloakAuthLayer::::builder() .instance(instance) .passthrough_mode(PassthroughMode::Block) .persist_raw_claims(false) - .expected_audiences(vec![String::from("account")]) - .required_roles(vec![String::from("administrator")]) - .token_extractors(NonEmpty::> { - head: Arc::new(AuthHeaderTokenExtractor::default()), - tail: vec![ - Arc::new(QueryParamTokenExtractor::default()), - Arc::new(QueryParamTokenExtractor::extracting_key("jwt")), - ], - }) + .validation_policy(test_policy()) + .required_roles(vec![AppRole::Admin]) + .token_extractors(vec![ + Arc::new(AuthHeaderTokenExtractor::default()) as Arc, + Arc::new(QueryParamTokenExtractor::default()), + Arc::new(QueryParamTokenExtractor::extracting_key("jwt")), + ]) .build(); } } diff --git a/src/auth/mod.rs b/src/auth/mod.rs index aef65ba..bc45466 100644 --- a/src/auth/mod.rs +++ b/src/auth/mod.rs @@ -1,273 +1,61 @@ -//! Protect axum routes with a JWT emitted by Keycloak. +//! Keycloak JWT authentication for the protected axum routes. //! -//! # Usage +//! The layer is built once in `router::init_auth` and wrapped around the protected router. In +//! `PassthroughMode::Block` — the mode ISM uses — a request only reaches a handler once its +//! bearer token has been validated against the realm's published signing keys. //! -//! This library provides the `KeycloakAuthInstance` which manages OIDC discovery and hold onto decoding keys -//! and the `KeycloakAuthLayer`, a tower layer / service implementation that parses and validates incoming JWTs. +//! Request path: //! -//! Let's set up a protected Axum route! +//! | File | Responsibility | +//! |---|---| +//! | `layer.rs` | tower `Layer`, holds the config; built once at startup | +//! | `service.rs` | per-request tower `Service`; blocks or passes through | +//! | `extract.rs` | pulls the raw JWT out of the request (header, query param) | +//! | `decode.rs` | signature and claim validation | +//! | `token.rs` | the claim types validation produces, `KeycloakToken` among them | +//! | `instance.rs` | OIDC discovery, cached JWKS, on-demand refresh on key rotation | +//! | `oidc.rs` / `oidc_discovery.rs` | discovery document DTOs / the HTTP calls fetching them | +//! | `role.rs` | the generic `Role` trait and the `expect_role!` family of macros | +//! | `app_role.rs` | `AppRole` — the realm's roles, the concrete `Role` this app uses | +//! | `current_user.rs` | `CurrentUser` = `KeycloakToken`, plus its extractor | //! -//! To demonstrate the likely case of still requiring some (e.g. /health) public routes, -//! let us define two functions to create the respective public and protected routers, -//! adding a `KeycloakAuthLayer` only to the router whose routes should be protected. -//! -//! Specifying the `required_roles` is optional. If omitted, role-presence can be checked in each route-handler individually. -//! The library will then only check that a request was performed with a valid JWT. -//! Consider using this builder field if you have a long list of route-handlers -//! which all require the same roles to be present. -//! -//! ```rust -//! use std::sync::Arc; -//! use axum::{http::StatusCode, response::{Response, IntoResponse}, routing::get, Extension, Router}; -//! use ism::auth::{Url, error::AuthError, instance::KeycloakConfig, instance::KeycloakAuthInstance, layer::KeycloakAuthLayer, decode::KeycloakToken, PassthroughMode}; -//! use ism::expect_role; -//! -//! pub fn public_router() -> Router { -//! Router::new() -//! .route("/health", get(health)) -//! } -//! -//! pub fn protected_router(instance: KeycloakAuthInstance) -> Router { -//! Router::new() -//! .route("/protected", get(protected)) -//! .layer( -//! KeycloakAuthLayer::::builder() -//! .instance(instance) -//! .passthrough_mode(PassthroughMode::Block) -//! .persist_raw_claims(false) -//! .expected_audiences(vec![String::from("account")]) -//! .required_roles(vec![String::from("administrator")]) -//! .build(), -//! ) -//! } -//! -//! // You may have multiple routers that you want to see protected by a `KeycloakAuthLayer`. -//! // You can safely attach new `KeycloakAuthLayer`s to different routers, but consider using only a single `KeycloakAuthInstance` for all of these layers. -//! // Remember: The `KeycloakAuthInstance` manages the keys used to decode incoming JWTs and dynamically fetches them from your Keycloak server. -//! // Having multiple instances simoultaniously would incease pressure on your Keycloak instance on service startup and unnecesssarily store duplicated data. -//! // The `KeycloakAuthLayer` therefore really takes an `Arc` in its `instance` method! -//! // Presence of the `Into` trait in the `instance` methods argument let us hide that fact in the previous example. -//! -//! #[allow(dead_code)] -//! pub fn protect(router:Router, instance: Arc) -> Router { -//! router.layer( -//! KeycloakAuthLayer::::builder() -//! .instance(instance) -//! .passthrough_mode(PassthroughMode::Block) -//! .persist_raw_claims(false) -//! .expected_audiences(vec![String::from("account")]) -//! .required_roles(vec![String::from("administrator")]) -//! .build(), -//! ) -//! } -//! -//! // Lets also define the handlers ('health' and 'protected') defined in our routers. -//! -//! // The `health` handler can always be called without a JWT, -//! // as we only attached an instance of the `KeycloakAuthLayer` to the protected router. -//! -//! // The `KeycloakAuthLayer` makes the parsed token data available using axum's `Extension`'s, -//! // including the users roles, the uuid of the user, its name, email, ... -//! // The `protected` handler will (in the default `PassthroughMode::Block` case) only be called -//! // if the request contained a valid JWT which not already expired. -//! // It may then access that data (as `KeycloakToken`) through an Extension -//! // to get access to the decoded auth user information as shown below. -//! -//! pub async fn health() -> impl IntoResponse { -//! StatusCode::OK -//! } -//! -//! pub async fn protected(Extension(token): Extension>) -> Response { -//! -//! expect_role!(&token, "administrator"); -//! -//! tracing::info!("Token payload is {token:#?}"); -//! -//! ( -//! StatusCode::OK, -//! format!( -//! "Hello {name} ({subject}). Your token is valid for another {valid_for} seconds.", -//! name = token.extra.profile.preferred_username, -//! subject = token.subject, -//! valid_for = (token.expires_at - time::OffsetDateTime::now_utc()).whole_seconds() -//! ), -//! ).into_response() -//! } -//! -//! // You can construct a `KeycloakAuthInstance` using a single value of type `KeycloakConfig`, which is constructed using the builder pattern. -//! // You may want to immediately wrap it inside an `Arc` if you intend to pass it to multiple `KeycloakAuthLayer`s. We are not doing this in this example. -//! -//! // Your final router can be created by merging the public and protected routers. -//! -//! #[tokio::main] -//! async fn main() { -//! let keycloak_auth_instance = KeycloakAuthInstance::new( -//! KeycloakConfig::builder() -//! .server(Url::parse("https://localhost:8443/").unwrap()) -//! .realm(String::from("MyRealm")) -//! .build(), -//! ); -//! let router = public_router().merge(protected_router(keycloak_auth_instance)); -//! -//! // let addr_and_port = String::from("0.0.0.0:8080"); -//! // let socket_addr: std::net::SocketAddr = addr_and_port.parse().unwrap(); -//! // println!("Listening on: {}", addr_and_port); -//! -//! // let tcp_listener = tokio::net::TcpListener::bind(socket_addr).await.unwrap(); -//! // axum::serve(tcp_listener, router.into_make_service()).await.unwrap(); -//! } -//! ``` -//! -//! # Using a custom role type -//! -//! You probably noticed a generic `` when creating the `KeycloakAuthLayer` and defining the handler extension. -//! -//! This is the type representing a single role and can be replaced with any type implementing the `axum_keycloak_auth::role::Role` trait. -//! -//! You could for example create an enum containing all your known roles as variants with a special variant for unknown role names. -//! -//! ```rust -//! #[derive(Debug, PartialEq, Eq, Clone)] -//! pub enum Role { -//! Administrator, -//! Unknown(String), -//! } -//! -//! impl auth::role::Role for Role {} -//! -//! impl std::fmt::Display for Role { -//! fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { -//! match self { -//! Role::Administrator => f.write_str("Administrator"), -//! Role::Unknown(unknown) => f.write_fmt(format_args!("Unknown role: {unknown}")), -//! } -//! } -//! } -//! -//! impl From for Role { -//! fn from(value: String) -> Self { -//! match value.as_ref() { -//! "administrator" => Role::Administrator, -//! _ => Role::Unknown(value), -//! } -//! } -//! } -//! -//! // You could then (remember to update both locations of the generic type) check for roles using your enum: -//! -//! use axum::{http::StatusCode, response::{Response, IntoResponse}, Extension}; -//! use ism::auth::{decode::KeycloakToken};//! -//! -//! use ism::{expect_role, auth}; -//! -//! pub async fn protected(Extension(token): Extension>) -> Response { -//! expect_role!(&token, Role::Administrator); -//! StatusCode::OK.into_response() -//! } -//! ``` -//! -//! # Passthrough modes -//! -//! The `KeycloakAuthLayer` provides a `passthrough_mode` field, allowing you to choose between the following modes: -//! -//! - `PassthroughMode::Block`: Immediately return an error-response should authentication fail. This is the preferred mode and the default if omitted. -//! - `PassthroughMode::Pass`: Always store a `KeycloakAuthStatus` containing the authentication result and defer the response generation to the handler or any deeper layers. You may want to use this mode i fine-grained error handling is required or you want to use additional layers which could still prove the user authenticated. -//! -//! # Using custom token extractors -//! -//! By default, request headers are checked for presence of an "authorization" header, -//! which is expected to contain the typical "`Bearer `" string. -//! -//! You have the ability to change this behavior to your liking through use of the `TokenExtractor` trait, -//! which allows for customized strategies on how to retrieve the token from an axum request. -//! -//! The `token_extractors` field on the `KeycloakAuthLayer` builder accepts a non-empty vec of extractors. -//! -//! ```rust,no_run -//! use std::sync::Arc; -//! use ism::auth::{ -//! NonEmpty, PassthroughMode, -//! instance::KeycloakAuthInstance, -//! layer::KeycloakAuthLayer, -//! extract::{AuthHeaderTokenExtractor, QueryParamTokenExtractor, TokenExtractor} -//! }; -//! -//! let instance: KeycloakAuthInstance = todo!(); -//! -//! let layer = KeycloakAuthLayer::::builder() -//! .instance(instance) -//! .passthrough_mode(PassthroughMode::Block) -//! .expected_audiences(vec![String::from("account")]) -//! // ... -//! .token_extractors(NonEmpty::> { -//! head: Arc::new(AuthHeaderTokenExtractor::default()), -//! tail: vec![ -//! Arc::new(QueryParamTokenExtractor::default()), -//! Arc::new(QueryParamTokenExtractor::extracting_key("jwt")), -//! ], -//! }) -//! .build(); -//! ``` -//! -//! Extractors are called in order of their definition in the `token_extractors` vec. -//! The token from the first extractor able to successfully extract one is used to further validate the request. -//! Other extractors are no longer considered. -//! -//! This crate implements two extraction strategies: -//! - `AuthHeaderTokenExtractor`: Extracts the token from the `http::header::AUTHORIZATION` header. -//! - `QueryParamTokenExtractor`: Extracts the token from a query parameter (by default named "token"). Use with caution! -//! -//! By default, when not explicitly setting `token_extractors`, a single `AuthHeaderTokenExtractor::default()` is used. +//! Handlers behind the layer take `CurrentUser`, which is the whole validated token: caller id, +//! roles, profile and email claims. The `` generic is pinned to `AppRole` there, so no handler +//! and no route ever spells it out. //! +//! Full usage guide, including custom role types and custom token extractors: `docs/auth.md`. #![forbid(unsafe_code)] -//#![warn(missing_docs)] #![deny(clippy::unwrap_used)] -use std::sync::Arc; - -use role::Role; - mod action; -pub mod decode; -pub mod error; -pub mod extract; -pub mod instance; -pub mod layer; -pub mod oidc; -pub mod oidc_discovery; -pub mod role; -pub mod service; - -// Re-export the Url struct used when configuring a `KeycloakAuthInstance`. -pub use url::Url; - -// Re-export the NonEmpty struct used when configuring a `KeycloakAuthLayer`. -pub use nonempty::NonEmpty; - -use serde::de::DeserializeOwned; - -/// The mode in which the authentication middleware may operate in. -/// -/// ```PassthroughMode::Block```: Immediately return a `Response` if authentication failed. -/// On successful authentication, the parsed token content is stored as an axum extension as a `KeycloakToken`. -/// -/// ```PassthroughMode::Pass```: Forward to the response handler regardless of whether there was an authentication failure. -/// In this mode, the authentication status is stored as an axum extension as a `KeycloakAuthStatus`. -#[derive(Debug, PartialEq, Eq, Clone, Copy)] -pub enum PassthroughMode { - Block, - Pass, -} - -#[derive(Debug, Clone)] -#[allow(clippy::large_enum_variant)] -pub enum KeycloakAuthStatus -where - R: Role, - Extra: DeserializeOwned + Clone, -{ - // This variant is fairly large, but probably used most of the time. Leaving this non-boxed results in one less allocation each request. - Success(decode::KeycloakToken), - Failure(Arc), -} +mod app_role; +mod current_user; +mod decode; +mod error; +mod extract; +mod instance; +mod layer; +mod mode; +mod oidc; +mod oidc_discovery; +mod role; +#[cfg(test)] +mod security_tests; +mod service; +mod token; + +pub use app_role::AppRole; +pub use current_user::CurrentUser; +pub use decode::ValidationPolicy; +pub use error::AuthError; +// Several `AuthError` variants keep their `Display` free of the source, so `{err}` alone drops the +// actual cause. Startup needs the full chain to make a failed discovery diagnosable from a crash log. +pub(crate) use error::error_chain; +pub use instance::{KeycloakAuthInstance, KeycloakConfig}; +pub use layer::KeycloakAuthLayer; +pub use mode::{KeycloakAuthStatus, PassthroughMode}; +pub use token::KeycloakToken; +// Re-exported for the `expect_role!` family of macros, which expand to `$crate::auth::ExpectRoles` +// at the call site and therefore cannot reach through the private `role` module. +pub use role::{ExpectRoles, KeycloakRole, Role}; diff --git a/src/auth/mode.rs b/src/auth/mode.rs new file mode 100644 index 0000000..284d58f --- /dev/null +++ b/src/auth/mode.rs @@ -0,0 +1,35 @@ +//! What the middleware does with a request whose token failed validation. + +use std::sync::Arc; + +use serde::de::DeserializeOwned; + +use crate::auth::error::AuthError; +use crate::auth::role::Role; +use crate::auth::token::KeycloakToken; + +/// The mode in which the authentication middleware may operate in. +/// +/// ```PassthroughMode::Block```: Immediately return a `Response` if authentication failed. +/// On successful authentication, the parsed token content is stored as an axum extension as a `KeycloakToken`. +/// +/// ```PassthroughMode::Pass```: Forward to the response handler regardless of whether there was an authentication failure. +/// In this mode, the authentication status is stored as an axum extension as a `KeycloakAuthStatus`. +#[derive(Debug, PartialEq, Eq, Clone, Copy)] +pub enum PassthroughMode { + Block, + Pass, +} + +/// The authentication result handed to the handler under `PassthroughMode::Pass`. +#[derive(Debug, Clone)] +#[allow(clippy::large_enum_variant)] +pub enum KeycloakAuthStatus +where + R: Role, + Extra: DeserializeOwned + Clone, +{ + // This variant is fairly large, but probably used most of the time. Leaving this non-boxed results in one less allocation each request. + Success(KeycloakToken), + Failure(Arc), +} diff --git a/src/auth/oidc.rs b/src/auth/oidc.rs index 9ecaaf8..26491d9 100644 --- a/src/auth/oidc.rs +++ b/src/auth/oidc.rs @@ -1,3 +1,8 @@ +//! Serde models of Keycloak's `.well-known/openid-configuration` document. +//! +//! Modelled in full against the OIDC/OAuth discovery specs, even though ISM only reads `issuer` +//! and `jwks_uri` today. The HTTP calls that fetch these live in `oidc_discovery.rs`. + use std::collections::HashMap; use serde::{Deserialize, Serialize}; @@ -44,7 +49,7 @@ pub struct OidcConfig { pub struct OAuthDiscoveryClaims { /// OPTIONAL. URL of the authorization server's OAuth 2.0 /// introspection endpoint [RFC7662]. - introspection_endpoint: Option, + pub introspection_endpoint: Option, /// OPTIONAL. JSON array containing a list of client authentication /// methods supported by this introspection endpoint. The valid @@ -55,7 +60,7 @@ pub struct OAuthDiscoveryClaims { /// values are and will remain distinct, due to Section 7.2.) If /// omitted, the set of supported authentication methods MUST be /// determined by other means. - introspection_endpoint_auth_methods_supported: Option>, + pub introspection_endpoint_auth_methods_supported: Option>, /// OPTIONAL. JSON array containing a list of the JWS signing /// algorithms ("alg" values) supported by the introspection endpoint @@ -66,7 +71,7 @@ pub struct OAuthDiscoveryClaims { /// specified in the "introspection_endpoint_auth_methods_supported" /// entry. No default algorithms are implied if this entry is /// omitted. The value "none" MUST NOT be used. - introspection_endpoint_auth_signing_alg_values_supported: Option>, + pub introspection_endpoint_auth_signing_alg_values_supported: Option>, } /// See: `https://openid.net/specs/openid-connect-frontchannel-1_0.html#OPLogout` diff --git a/src/auth/oidc_discovery.rs b/src/auth/oidc_discovery.rs index bc80780..f432a41 100644 --- a/src/auth/oidc_discovery.rs +++ b/src/auth/oidc_discovery.rs @@ -1,35 +1,55 @@ +//! The HTTP calls that fetch Keycloak's discovery document and JWK set. +//! +//! The documents themselves are modelled in `oidc.rs`; this module only does the fetching. + use std::sync::Arc; use crate::auth::oidc::OidcConfig; use reqwest::IntoUrl; use serde::Deserialize; -use snafu::{ResultExt, Snafu}; +use thiserror::Error; -#[derive(Debug, Clone, Snafu)] +/// A discovery request that never produced a usable document. +#[derive(Debug, Clone, Error)] pub enum RequestError { - #[snafu(display("RequestError: Could not send request"))] + #[error("RequestError: Could not send request")] Send { source: Arc }, - #[snafu(display("RequestError: Could not decode payload"))] + #[error("RequestError: Could not decode payload")] Decode { source: Arc }, } -pub(crate) async fn retrieve_oidc_config( +impl RequestError { + fn send(source: reqwest::Error) -> Self { + Self::Send { + source: Arc::new(source), + } + } + + fn decode(source: reqwest::Error) -> Self { + Self::Decode { + source: Arc::new(source), + } + } +} + +/// Fetches the realm's `.well-known/openid-configuration` document. +pub async fn retrieve_oidc_config( discovery_endpoint: impl IntoUrl, ) -> Result { reqwest::Client::new() .get(discovery_endpoint) .send() .await - .map_err(Arc::new) - .context(SendSnafu {})? + .map_err(RequestError::send)? .json::() .await - .map_err(Arc::new) - .context(DecodeSnafu {}) + .map_err(RequestError::decode) } -pub(crate) async fn retrieve_jwk_set( +/// Fetches the JWK set, parsing each key on its own so one unusable entry does not discard the +/// whole set. +pub async fn retrieve_jwk_set( jwk_set_endpoint: impl IntoUrl, ) -> Result { #[derive(Deserialize)] @@ -40,12 +60,10 @@ pub(crate) async fn retrieve_jwk_set( .get(jwk_set_endpoint) .send() .await - .map_err(Arc::new) - .context(SendSnafu {})? + .map_err(RequestError::send)? .json::() .await - .map_err(Arc::new) - .context(DecodeSnafu {})?; + .map_err(RequestError::decode)?; let mut set = jsonwebtoken::jwk::JwkSet { keys: Vec::new() }; for key in raw_set.keys { match serde_json::from_value::(key) { diff --git a/src/auth/role.rs b/src/auth/role.rs index f8ef49a..f13f8c0 100644 --- a/src/auth/role.rs +++ b/src/auth/role.rs @@ -1,3 +1,9 @@ +//! Realm and client roles, and the macros that assert them inside a handler. +//! +//! ISM does not check roles yet — every handler is reachable by any authenticated user — but the +//! `Role` trait is what the `` generic on `KeycloakToken` and `KeycloakAuthLayer` binds, and +//! `String` is the default role type. See `docs/auth.md` for using a custom enum instead. + use std::fmt::{Debug, Display}; use axum::response::IntoResponse; @@ -29,14 +35,44 @@ pub enum KeycloakRole { } impl KeycloakRole { + /// The role name, whether it came from the realm or from a client. + /// + /// Deliberately *not* what an access check should compare against — see `realm_role`. Keycloak + /// puts the roles of every client the user holds roles on into `resource_access`, and + /// `ExtractRoles` flattens those into the same list as the realm roles. Matching on the bare + /// name therefore treats a client role named `ADMIN`, defined by some unrelated service in the + /// realm, as if the realm itself had granted it. pub fn role(&self) -> &R { match self { KeycloakRole::Realm { role } => role, KeycloakRole::Client { client: _, role } => role, } } + + /// The role name if the *realm* granted it, `None` for a client role. + /// + /// This is what `ExpectRoles::expect_roles` matches on, so that granting a client role can + /// never widen a caller's access here. + pub fn realm_role(&self) -> Option<&R> { + match self { + KeycloakRole::Realm { role } => Some(role), + KeycloakRole::Client { .. } => None, + } + } + + /// The role name if `client` granted it, `None` otherwise. + pub fn client_role(&self, client: &str) -> Option<&R> { + match self { + KeycloakRole::Client { + client: owner, + role, + } if owner == client => Some(role), + _ => None, + } + } } +/// How many roles a claim section carries, so `ExtractRoles` can size its target vec once. pub trait NumRoles { fn num_roles(&self) -> usize; } @@ -47,6 +83,7 @@ impl NumRoles for Option { } } +/// Flattens a claim section (`realm_access`, `resource_access`) into a single role list. pub trait ExtractRoles { fn extract_roles(self, target: &mut Vec>); } @@ -75,6 +112,15 @@ where } } +/// Asserts the presence (or absence) of roles on something that carries them. +/// +/// The two directions deliberately consult different role sources, so that both fail closed: +/// +/// - `expect_roles` grants access and therefore only accepts **realm** roles. A client role of the +/// same name must never be enough, or any service in the realm could widen access to ISM by +/// naming one of its own roles `ADMIN`. +/// - `not_expect_roles` denies access and therefore considers **every** role source. A role that +/// should keep a caller out must do so no matter who granted it. pub trait ExpectRoles { type Rejection: IntoResponse; @@ -82,38 +128,45 @@ pub trait ExpectRoles { fn not_expect_roles + Clone>(&self, roles: &[I]) -> Result<(), Self::Rejection>; } +// The four macros below are `#[macro_export]`ed, so they land at the crate root +// (`crate::expect_role!`) regardless of this module being private. They must therefore reach +// `ExpectRoles` through the `auth` facade rather than through `auth::role`. + +/// Returns from the handler with an error response unless the token carries all `$roles`. #[macro_export] macro_rules! expect_roles { ($token: expr, $roles: expr) => { - if let Err(err) = axum_keycloak_auth::role::ExpectRoles::expect_roles($token, $roles) { + if let Err(err) = $crate::auth::ExpectRoles::expect_roles($token, $roles) { return axum::response::IntoResponse::into_response(err); } }; } +/// Returns from the handler with an error response unless the token carries `$role`. #[macro_export] macro_rules! expect_role { ($token: expr, $role: expr) => { - if let Err(err) = axum_keycloak_auth::role::ExpectRoles::expect_roles($token, &[$role]) { + if let Err(err) = $crate::auth::ExpectRoles::expect_roles($token, &[$role]) { return axum::response::IntoResponse::into_response(err); } }; } +/// Returns from the handler with an error response if the token carries any of `$roles`. #[macro_export] macro_rules! not_expect_roles { ($token: expr, $roles: expr) => { - if let Err(err) = axum_keycloak_auth::role::ExpectRoles::not_expect_roles($token, $roles) { + if let Err(err) = $crate::auth::ExpectRoles::not_expect_roles($token, $roles) { return axum::response::IntoResponse::into_response(err); } }; } +/// Returns from the handler with an error response if the token carries `$role`. #[macro_export] macro_rules! not_expect_role { ($token: expr, $role: expr) => { - if let Err(err) = axum_keycloak_auth::role::ExpectRoles::not_expect_roles($token, &[$role]) - { + if let Err(err) = $crate::auth::ExpectRoles::not_expect_roles($token, &[$role]) { return axum::response::IntoResponse::into_response(err); } }; diff --git a/src/auth/security_tests.rs b/src/auth/security_tests.rs new file mode 100644 index 0000000..ba33fe4 --- /dev/null +++ b/src/auth/security_tests.rs @@ -0,0 +1,666 @@ +//! Adversarial tests for the JWT validation path. +//! +//! Each test encodes one attack that the validation must reject. They run entirely in-process: +//! tokens are signed with a test key below and verified through the same code the middleware +//! uses, so no Keycloak and no network are involved. +//! +//! The entry points under test are `RawToken::decode_header` / +//! `RawToken::decode_and_validate` (signature, algorithm, issuer, audience, expiry, required +//! claims) and `parse_raw_claims` (token type, authorized party, subject shape). +//! +//! When adding a validation rule, add the attack it defeats here — a rule with no failing test +//! before it existed is a rule nobody can prove still works. + +use base64::Engine; +use base64::engine::general_purpose::URL_SAFE_NO_PAD; +use jsonwebtoken::errors::ErrorKind; +use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header}; +use serde_json::{Value, json}; + +use url::Url; + +use crate::auth::app_role::AppRole; +use crate::auth::decode::{ + RawClaims, RawToken, ValidationPolicy, decode_and_validate, parse_raw_claims, +}; +use crate::auth::error::AuthError; +use crate::auth::instance::{JwkDecodingKey, KeycloakAuthInstance, KeycloakConfig, keys_for_kid}; +use crate::auth::token::ProfileAndEmail; + +/// Test-only RSA-2048 keypair. Not a secret and never used outside this file. +const TEST_PRIVATE_KEY_PEM: &str = "-----BEGIN PRIVATE KEY----- +MIIEvgIBADANBgkqhkiG9w0BAQEFAASCBKgwggSkAgEAAoIBAQC6Lsy40iI95RfA +t/V3F3/qaNEfI2qv15zRkCDjwCZyHTbR8YALCO55kcGC1ljWG6crA4f6kfZoarEB +6tfLpQQvQvtIozurIt5oNaZOnMK1jFSbBxLvCzWW6MVpZmOGDDN+bRlhl73kj8bq +sEu/puRBhEeRrIPT6Y0TxZwrH9iX6gULWKBYWhKp/o7+9iq2q28s3aa8h1MAI/Xi +V0+Yvn+gi25IzuRooNh1M6+ONSDjqqtzT/OWikK4ckY+BkzPGA17jE5fMbDO4AGj +yaYU+1K4Pk0qwwYnBwayBkfKnfMVxQEc5QrgsCZLDerzSzpTfXnygMKS8XPkDSwb +6a1Y114PAgMBAAECggEAC6KEEaK0GBkacGUulkgmKsBtHRyJ/L4lIyV2ILVv0Z7I +v7rvTQE8YeV9ac86Uvr8aeA5HawEcYcFU8DYxnWj+s4dRO9KecneizWbFHuQYWcJ +HH0HLmANc8ZNG+aVnplhmGt59BLW/5MKk7z7ptjnl76L+GsG+/Wy5sLpHPrK/sc6 +PyLapU5p+QD9KaI2HCOFm/HFMGy48P0A+Gh4tkj8DgNNa2yWDo7bEGtw0SlFi//a +aS4S5thwC4bJKJRQTKxwiA9jiPL9ddpBxyt2E1XR8drzLUjpySqob/mF+al2A3Te +VzxABfNAnING9rq8IafAW+Bc5q9odI2D6Rr0molH6QKBgQDngBn76ypZGrPERcLh +hWeu7uglC4erbPgRuxdr9UHRTJJ9m9lvMD9JiKjm2Y2WwfUsUNQ8BqWtcwwMWyi+ +DnUcceDyQLj0lIgjN45h+JaNqUMrgQFtkXy20YxMRQZva5LKpQSERjwpRvqp1YLd +TrhdYNvTEIabfBxWoQx546aZSQKBgQDN4u/Fc6b40pWCZPqrhrxQCv3X0JIOSwKD +haPE1msgV5NWlISZ/zkrtQj43Iddy/xqyVEs7NoOscRR3cXX8yCxr7HSzMXIFTs7 +cE+Q0gwdzvLQ43n1NyhDJfTIKQWkTIksbebCqSAJaDgqpPloxuUNVx+O5qEH2gze +8TsLDA9UlwKBgQCsK7ynfEW5kT9jWNLQcSwkkS/75TBYkSmJ3lBUDUqPA9jrLE6w +//wBj262idRg7A2QkOjXX8Y2UpsCUYXim9QDfLpk0Tf9Rr5dGsN9H6mw39LB9yb9 +uzc6rGwgiTF5ClNY/RN34Nh7hnuEdfPm7dX2NMQonGDQIKTe1NX3jRTpaQKBgQCc +qjvDZv6+RheockhgbxUqX0LLjxUktSVDiVSV+obnxFwEPN0uBYyuWoJqQ/zpfcgk +Re50HgLLva9ikDv02Defnc7VViaF2soIr6yLyZmYsRoJo57w3jjP57j8+mIlpGuZ +GEPJCkKrhdd/c6upc/dlkE8eQRZ10BGNL8i63kFoHwKBgG2xY1iOJuW/NbtPsN9Z +zdiNso3tYHLzSB3Oxz5B6GguiucHl/kQt8L6Ho4t/LhjIx1YViUPt6ONPi8v42// +7b1HXvk28rxnd5crUPxajuDUaIcECIz0pixnOIigkc+DLrH4tTFc2PIHxmwAUaQ6 +3wr2jjWHZYQByCDxwzzU5oA5 +-----END PRIVATE KEY-----"; + +const TEST_PUBLIC_KEY_PEM: &str = "-----BEGIN PUBLIC KEY----- +MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAui7MuNIiPeUXwLf1dxd/ +6mjRHyNqr9ec0ZAg48Amch020fGACwjueZHBgtZY1hunKwOH+pH2aGqxAerXy6UE +L0L7SKM7qyLeaDWmTpzCtYxUmwcS7ws1lujFaWZjhgwzfm0ZYZe95I/G6rBLv6bk +QYRHkayD0+mNE8WcKx/Yl+oFC1igWFoSqf6O/vYqtqtvLN2mvIdTACP14ldPmL5/ +oItuSM7kaKDYdTOvjjUg46qrc0/zlopCuHJGPgZMzxgNe4xOXzGwzuABo8mmFPtS +uD5NKsMGJwcGsgZHyp3zFcUBHOUK4LAmSw3q80s6U3158oDCkvFz5A0sG+mtWNde +DwIDAQAB +-----END PUBLIC KEY-----"; + +const KID: &str = "test-signing-key"; +const ISSUER: &str = "https://keycloak.example/realms/meventure"; +const OTHER_ISSUER: &str = "https://attacker.example/realms/meventure"; +const AUDIENCE: &str = "account"; +const CLIENT_ID: &str = "ism-app"; +const SUBJECT: &str = "0193f3a0-1c2d-7e4f-8a9b-0c1d2e3f4a5b"; + +// ── Fixtures ──────────────────────────────────────────────────────────────── + +fn policy() -> ValidationPolicy { + ValidationPolicy::new(vec![AUDIENCE.to_owned()], vec![], &[String::from("RS256")]) + .expect("valid policy") +} + +/// Policy that additionally pins the authorized party, i.e. the tightened production setup. +fn policy_pinning_azp() -> ValidationPolicy { + ValidationPolicy::new( + vec![AUDIENCE.to_owned()], + vec![CLIENT_ID.to_owned()], + &[String::from("RS256")], + ) + .expect("valid policy") +} + +fn signing_key() -> EncodingKey { + EncodingKey::from_rsa_pem(TEST_PRIVATE_KEY_PEM.as_bytes()).expect("private key parses") +} + +fn verification_key() -> DecodingKey { + DecodingKey::from_rsa_pem(TEST_PUBLIC_KEY_PEM.as_bytes()).expect("public key parses") +} + +fn now() -> u64 { + jsonwebtoken::get_current_timestamp() +} + +/// A claim set that passes every check, as the baseline each attack deviates from. +fn base_claims() -> Value { + json!({ + "exp": now() + 300, + "iat": now(), + "nbf": now() - 10, + "jti": "b7c1e5a2-3f4d-4e5a-9b8c-7d6e5f4a3b2c", + "iss": ISSUER, + "aud": AUDIENCE, + "sub": SUBJECT, + "typ": "Bearer", + "azp": CLIENT_ID, + "preferred_username": "tim", + "email": "tim@example.test", + "email_verified": true, + }) +} + +/// `base_claims` with `overrides` applied. A `null` value removes the claim entirely, which is +/// how the "missing required claim" cases are expressed. +fn claims_with(overrides: Value) -> Value { + let mut claims = base_claims(); + let target = claims.as_object_mut().expect("claims are an object"); + for (key, value) in overrides.as_object().expect("overrides are an object") { + match value.is_null() { + true => target.remove(key), + false => target.insert(key.clone(), value.clone()), + }; + } + claims +} + +fn rs256_header() -> Header { + let mut header = Header::new(Algorithm::RS256); + header.kid = Some(KID.to_owned()); + header +} + +fn sign(header: &Header, claims: &Value) -> String { + jsonwebtoken::encode(header, claims, &signing_key()).expect("token encodes") +} + +/// Runs a token through exactly what the middleware runs: header decode, then verification. +fn verify_with(token: &str, policy: &ValidationPolicy) -> Result { + let raw_token = RawToken(token); + raw_token.decode_header()?; + let key = verification_key(); + raw_token.decode_and_validate(policy, ISSUER, &[&key]) +} + +fn verify(token: &str) -> Result { + verify_with(token, &policy()) +} + +/// Full pipeline including the claim-level checks that run after signature verification. +async fn authenticate(token: &str, policy: &ValidationPolicy) -> Result<(), AuthError> { + authenticate_expecting_roles(token, policy, &[]).await +} + +/// `authenticate`, additionally requiring `required_roles` — the layer-wide role gate. +async fn authenticate_expecting_roles( + token: &str, + policy: &ValidationPolicy, + required_roles: &[AppRole], +) -> Result<(), AuthError> { + let raw_claims = verify_with(token, policy)?; + parse_raw_claims::(raw_claims, false, required_roles, policy) + .await + .map(|_| ()) +} + +// ── Control: the baseline must actually pass ──────────────────────────────── + +#[tokio::test] +async fn accepts_a_well_formed_token() { + // Without this, every rejection test below could pass for the wrong reason. + let token = sign(&rs256_header(), &base_claims()); + authenticate(&token, &policy()) + .await + .expect("a well-formed token is accepted"); +} + +#[tokio::test] +async fn accepts_matching_authorized_party_when_pinned() { + let token = sign(&rs256_header(), &base_claims()); + authenticate(&token, &policy_pinning_azp()) + .await + .expect("the configured client is accepted"); +} + +// ── Algorithm attacks ─────────────────────────────────────────────────────── + +#[test] +fn rejects_alg_none() { + // The unsigned-token attack: claim `alg: none` and append an empty signature. + let header = URL_SAFE_NO_PAD.encode(br#"{"alg":"none","typ":"JWT"}"#); + let payload = URL_SAFE_NO_PAD.encode(base_claims().to_string()); + let token = format!("{header}.{payload}."); + + let err = verify(&token).expect_err("an unsigned token must be rejected"); + assert!( + matches!(err, AuthError::DecodeHeader { .. }), + "expected the header decode to fail, got {err:?}" + ); +} + +#[test] +fn rejects_rs256_to_hs256_key_confusion() { + // The classic: re-sign with HMAC using the *public* key as the shared secret, betting that + // the verifier picks its algorithm from the token header. + let header = Header::new(Algorithm::HS256); + let forged = jsonwebtoken::encode( + &header, + &base_claims(), + &EncodingKey::from_secret(TEST_PUBLIC_KEY_PEM.as_bytes()), + ) + .expect("forged token encodes"); + + let err = verify(&forged).expect_err("an HMAC-signed token must be rejected"); + assert!( + matches!(err, AuthError::Decode { .. }), + "expected verification to fail, got {err:?}" + ); +} + +#[test] +fn config_refuses_symmetric_and_mixed_algorithm_families() { + // The forgery above only stays impossible while the allow-list holds no symmetric algorithm, + // so the policy refuses to be built that way at all. + assert!( + ValidationPolicy::new(vec![AUDIENCE.to_owned()], vec![], &[String::from("HS256")]).is_err() + ); + assert!( + ValidationPolicy::new( + vec![AUDIENCE.to_owned()], + vec![], + &[String::from("RS256"), String::from("ES256")] + ) + .is_err() + ); + assert!(ValidationPolicy::new(vec![AUDIENCE.to_owned()], vec![], &[]).is_err()); +} + +// ── Signature attacks ─────────────────────────────────────────────────────── + +#[test] +fn rejects_tampered_payload() { + // Keep a genuine signature, swap the payload underneath it. + let token = sign(&rs256_header(), &base_claims()); + let mut parts = token.split('.'); + let header = parts.next().expect("header"); + let _original = parts.next().expect("payload"); + let signature = parts.next().expect("signature"); + + let escalated = claims_with(json!({ "sub": "ffffffff-ffff-4fff-8fff-ffffffffffff" })); + let forged = format!( + "{header}.{}.{signature}", + URL_SAFE_NO_PAD.encode(escalated.to_string()) + ); + + let err = verify(&forged).expect_err("a tampered payload must be rejected"); + assert!( + matches!(err, AuthError::Decode { .. }), + "expected verification to fail, got {err:?}" + ); +} + +#[test] +fn rejects_token_signed_by_an_unknown_key() { + // A token that is internally consistent but signed by a key we do not trust. + let foreign_key = EncodingKey::from_rsa_pem(FOREIGN_PRIVATE_KEY_PEM.as_bytes()) + .expect("foreign private key parses"); + let forged = jsonwebtoken::encode(&rs256_header(), &base_claims(), &foreign_key) + .expect("forged token encodes"); + + let err = verify(&forged).expect_err("a foreign signature must be rejected"); + assert!( + matches!(err, AuthError::Decode { .. }), + "expected verification to fail, got {err:?}" + ); +} + +// ── Rediscovery trigger: rotation vs. forgery ─────────────────────────────── +// +// `decode_and_validate` may re-run OIDC discovery when a token fails to verify, which makes the +// trigger reachable by anyone who can send a request. The signature check cannot decide who +// deserves it: `InvalidSignature` means only "the verify operation returned false", so a forged +// token and a token signed by a rotated key are indistinguishable there. The decision is therefore +// made from the header's `kid`, before any crypto — and these tests pin that split. + +/// The cached key set as it looks after a successful discovery: one key, published under `KID`. +fn cached_keys() -> Vec { + vec![JwkDecodingKey { + kid: Some(KID.to_owned()), + key: verification_key(), + }] +} + +/// An instance that never contacted Keycloak, for the header checks that must reject a token +/// before the key set is consulted at all. +fn test_instance() -> KeycloakAuthInstance { + KeycloakAuthInstance::without_discovery( + KeycloakConfig::builder() + .server(Url::parse("https://localhost:8443/").expect("valid url")) + .realm(String::from("meventure")) + .build(), + ) +} + +#[tokio::test] +async fn rejects_token_without_kid_before_touching_the_key_set() { + // Keycloak stamps a `kid` on every access token. Without one there is nothing to match a key + // against, so this is refused outright rather than tried against every key we hold — which is + // what the old fall-back did, turning one such request into N public-key operations. + let mut header = Header::new(Algorithm::RS256); + header.kid = None; + let token = sign(&header, &base_claims()); + + let err = decode_and_validate(&test_instance(), RawToken(&token), &policy()) + .await + .expect_err("a token without a kid must be rejected"); + assert!( + matches!(&err, AuthError::InvalidToken { reason } if reason.contains("kid")), + "expected rejection for the missing kid, got {err:?}" + ); +} + +#[test] +fn a_forgery_reusing_a_known_kid_does_not_look_like_rotation() { + // The realistic forgery: copy a genuine token's header — `kid` and all — and sign a payload of + // your own. The verify step fails exactly as a rotated key would, so keying rediscovery off + // that error let every forged token reach for Keycloak. The `kid` tells them apart: we hold + // the key it names, so the failure is a property of the token and rediscovery cannot help. + let foreign_key = EncodingKey::from_rsa_pem(FOREIGN_PRIVATE_KEY_PEM.as_bytes()) + .expect("foreign private key parses"); + let forged = jsonwebtoken::encode(&rs256_header(), &base_claims(), &foreign_key) + .expect("forged token encodes"); + + verify(&forged).expect_err("a foreign signature must be rejected"); + assert!( + keys_for_kid(&cached_keys(), KID).is_some(), + "the forged token names a kid we already hold, so it must not trigger rediscovery" + ); +} + +#[test] +fn an_unknown_kid_is_the_only_rediscovery_trigger() { + // The one shape a genuine rotation takes: Keycloak signs with a key whose thumbprint we have + // never seen. This is the *only* case worth spending a discovery on. + assert!( + keys_for_kid(&cached_keys(), "rotated-signing-key").is_none(), + "a kid outside the cached set must be reported as a miss" + ); + assert!( + keys_for_kid(&cached_keys(), KID).is_some(), + "a kid inside the cached set must resolve" + ); +} + +#[test] +fn an_unknown_kid_does_not_fall_back_to_the_other_keys() { + // The lookup used to return the whole key set on a miss. That destroyed the signal — an + // unknown `kid` then failed identically to a tampered token — and made each such request cost + // one public-key operation per cached key. + let mut keys = cached_keys(); + keys.push(JwkDecodingKey { + kid: Some(String::from("second-signing-key")), + key: verification_key(), + }); + + assert!( + keys_for_kid(&keys, "neither-of-them").is_none(), + "a miss must stay a miss, never widen to the full key set" + ); + assert_eq!( + keys_for_kid(&keys, KID).expect("kid resolves").len(), + 1, + "a hit must yield only the keys published under that kid" + ); +} + +#[tokio::test] +async fn rejects_a_kid_carrying_control_characters() { + // `kid` is read before any signature is verified, so its content is entirely attacker-chosen, + // and it flows into log fields from there. A newline lets an unauthenticated caller forge log + // lines in any plain-text subscriber. + let mut header = rs256_header(); + header.kid = Some(String::from("real-key\nlevel=INFO msg=\"forged log line\"")); + let token = sign(&header, &base_claims()); + + let err = decode_and_validate(&test_instance(), RawToken(&token), &policy()) + .await + .expect_err("a kid with control characters must be rejected"); + let AuthError::InvalidToken { reason } = &err else { + panic!("expected InvalidToken, got {err:?}"); + }; + assert!( + !reason.contains('\n'), + "the rejection must not echo the kid back into its own message: {reason}" + ); +} + +#[tokio::test] +async fn rejects_an_overlong_kid() { + // Unbounded, every rejected request costs as much log volume as the sender chooses. Keycloak's + // own kid is a ~43 character base64url thumbprint. + let mut header = rs256_header(); + header.kid = Some("A".repeat(4096)); + let token = sign(&header, &base_claims()); + + let err = decode_and_validate(&test_instance(), RawToken(&token), &policy()) + .await + .expect_err("an overlong kid must be rejected"); + assert!( + matches!(err, AuthError::InvalidToken { .. }), + "expected InvalidToken, got {err:?}" + ); +} + +// ── Role attacks ──────────────────────────────────────────────────────────── + +#[tokio::test] +async fn a_client_role_does_not_satisfy_a_realm_role_check() { + // Keycloak ships the roles of *every* client the user holds roles on in `resource_access`, and + // `ExtractRoles` flattens those into the same list as the realm roles. Matching on the bare + // name would therefore let any other service in the realm grant access to ISM by naming one of + // its own roles `ADMIN` — no cooperation from this realm's admins required. + let token = sign( + &rs256_header(), + &claims_with(json!({ + "realm_access": { "roles": ["USER"] }, + "resource_access": { "some-other-service": { "roles": ["ADMIN"] } }, + })), + ); + + let err = authenticate_expecting_roles(&token, &policy(), &[AppRole::Admin]) + .await + .expect_err("a client role must not satisfy a realm role check"); + assert!( + matches!(err, AuthError::MissingExpectedRole { .. }), + "expected MissingExpectedRole, got {err:?}" + ); +} + +#[tokio::test] +async fn a_realm_role_satisfies_a_realm_role_check() { + // The control for the test above: without this, that one could pass because role checking is + // broken outright rather than because the realm/client split works. + let token = sign( + &rs256_header(), + &claims_with(json!({ "realm_access": { "roles": ["ADMIN"] } })), + ); + + authenticate_expecting_roles(&token, &policy(), &[AppRole::Admin]) + .await + .expect("a realm role must satisfy a realm role check"); +} + +// ── Claim attacks ─────────────────────────────────────────────────────────── + +#[test] +fn rejects_foreign_issuer() { + // A token from a Keycloak we do not trust, correctly signed by *that* server. + let token = sign( + &rs256_header(), + &claims_with(json!({ "iss": OTHER_ISSUER })), + ); + verify(&token).expect_err("a foreign issuer must be rejected"); +} + +#[test] +fn rejects_wrong_audience() { + let token = sign( + &rs256_header(), + &claims_with(json!({ "aud": "some-other-service" })), + ); + verify(&token).expect_err("a foreign audience must be rejected"); +} + +#[test] +fn rejects_expired_token() { + // Well beyond `EXPIRY_LEEWAY_SECS`. + let token = sign( + &rs256_header(), + &claims_with(json!({ "exp": now() - 3600 })), + ); + verify(&token).expect_err("an expired token must be rejected"); +} + +#[tokio::test] +async fn accepts_a_token_expired_within_the_leeway() { + // Two checks decide expiry — `jsonwebtoken`'s own validation and `assert_not_expired` — and + // the stricter one wins. This passes only while both apply `EXPIRY_LEEWAY_SECS`, so it is what + // catches the two drifting apart. + let token = sign(&rs256_header(), &claims_with(json!({ "exp": now() - 2 }))); + authenticate(&token, &policy()) + .await + .expect("expiry within the leeway must still authenticate"); +} + +#[tokio::test] +async fn rejects_a_token_expired_beyond_the_leeway() { + // Ten seconds is outside the five the leeway grants, but far inside `jsonwebtoken`'s 60-second + // default — so this fails the moment someone drops the explicit leeway and lets that default + // back in. + let token = sign(&rs256_header(), &claims_with(json!({ "exp": now() - 10 }))); + authenticate(&token, &policy()) + .await + .expect_err("expiry beyond the leeway must be rejected"); +} + +#[test] +fn expired_token_does_not_look_like_key_rotation() { + // Ordinary expiry is the single most common auth failure there is, so it must never reach for + // Keycloak. It cannot: an expired token still names the `kid` it was signed with, we still + // hold that key, and a known `kid` makes the failure terminal. Note this holds regardless of + // the `ErrorKind` — the trigger no longer consults it, which is the point. The kind is + // asserted only to document that expiry is what actually tripped. + let token = sign( + &rs256_header(), + &claims_with(json!({ "exp": now() - 3600 })), + ); + + let err = verify(&token).expect_err("an expired token must be rejected"); + let AuthError::Decode { source } = &err else { + panic!("expected a decode error, got {err:?}"); + }; + assert!( + matches!(source.kind(), ErrorKind::ExpiredSignature), + "expected ExpiredSignature, got {:?}", + source.kind() + ); + assert!( + keys_for_kid(&cached_keys(), KID).is_some(), + "the expired token names a kid we hold, so it must not trigger rediscovery" + ); +} + +#[test] +fn rejects_token_not_yet_valid() { + let token = sign( + &rs256_header(), + &claims_with(json!({ "nbf": now() + 3600 })), + ); + verify(&token).expect_err("a not-yet-valid token must be rejected"); +} + +#[test] +fn rejects_missing_required_claims() { + // Dropping a claim must not be a way to skip the check that reads it. + for claim in ["exp", "iss", "aud", "sub"] { + let token = sign(&rs256_header(), &claims_with(json!({ claim: null }))); + assert!( + verify(&token).is_err(), + "a token missing '{claim}' must be rejected" + ); + } +} + +#[tokio::test] +async fn rejects_id_token_replayed_as_access_token() { + // An ID token is signed by the same realm key and passes every signature and claim check + // above; only `typ` separates it from a bearer credential. + let token = sign(&rs256_header(), &claims_with(json!({ "typ": "ID" }))); + let err = authenticate(&token, &policy()) + .await + .expect_err("an ID token must not authenticate a request"); + assert!( + matches!(err, AuthError::InvalidToken { .. }), + "expected the token type to be rejected, got {err:?}" + ); +} + +#[tokio::test] +async fn rejects_refresh_token_replayed_as_access_token() { + let token = sign(&rs256_header(), &claims_with(json!({ "typ": "Refresh" }))); + authenticate(&token, &policy()) + .await + .expect_err("a refresh token must not authenticate a request"); +} + +#[tokio::test] +async fn rejects_token_minted_for_another_realm_client() { + // The reason `aud: "account"` alone is not access control: Keycloak puts that audience on + // every access token of every client in the realm. Only `azp` distinguishes them. + let token = sign( + &rs256_header(), + &claims_with(json!({ "azp": "some-other-client" })), + ); + + authenticate(&token, &policy_pinning_azp()) + .await + .expect_err("a token from another realm client must be rejected when azp is pinned"); + + // Documents the default posture: without `expected_azp` configured, this token is accepted. + authenticate(&token, &policy()) + .await + .expect("without azp pinning any realm client is accepted"); +} + +#[tokio::test] +async fn rejects_non_uuid_subject() { + let token = sign( + &rs256_header(), + &claims_with(json!({ "sub": "../../admin" })), + ); + let err = authenticate(&token, &policy()) + .await + .expect_err("a non-UUID subject must be rejected"); + assert!( + matches!(err, AuthError::InvalidToken { .. }), + "expected the subject to be rejected, got {err:?}" + ); +} + +// ── Structural attacks ────────────────────────────────────────────────────── + +#[test] +fn rejects_malformed_tokens() { + for token in [ + "", + "not-a-jwt", + "a.b", + "a.b.c.d", + "....", + &"A".repeat(8192), + "eyJhbGciOiJSUzI1NiJ9..", + ] { + verify(token).expect_err("a malformed token must be rejected"); + } +} + +/// A second, unrelated RSA-2048 key, standing in for "signed by someone else". +const FOREIGN_PRIVATE_KEY_PEM: &str = "-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQD7q21UQWXygUvE +Ws+NDvTevym687EsUSdOQUFJDL1MESpWFeeRl+z6nlcoOd0AkyAqa652FL/Pir0d +KK2lRtHa1PeNiENJMxjRG+ivfueh1TiRAi6Z5QyquyydDQ0e22K2X0CrfXxxke2h +N9a0YjdNWmUnPSy46QGwulHh2Muc+d0Wj4Trbf2RCPQ1gHKBJBONirmHO+T5KJyT +Q1a0M98wlHrh5rxRwVaKyREFma/5G5osNVnXdeCcVAnlNRQSg62p7PGrmqa+PShW +cVeczAGbIurdU8bOdTgPfuLNoN12jePSzn5bf+lpZ5bNHiK1Or8T2ULtoCOwdx9a +wNylSJ9zAgMBAAECggEAdT6WRufWukTRCu9xfNooavMs2js4YZiHErZk10bXk231 +xrgasyHPlawZl5RpaKCiHhEfbERbXbFZTBHM39Af6O5JS8bc7eefmp+BZezdtW+T +lD6rfieOoKVlcd8IK0VyddrnUl06EeC1j2Nno46UC/XeZQrjYFuw3WfXyLsKlJyI +ibyTHDCg3HQm3Y5+y1x2uLJ8vmOOU8q72hvfqM8eoyHMQeGOK2iBEDgmpPaiGtIE +Jpgju7/OxRmBenu0lQ5oszByl+4pVaUUpEAN5tFClQTdO30H0AOkiz26ClwrhR/K +wqBttRRUl+tyQ68lgKVuhsNdaQDB7jA0o61GAwNiQQKBgQD/Bayd3ZXtOw0JaVoT +WrdLG4wiAY/4bFfoQIWISHr6Y4NcTEsMbbvgnIj7UzJ4rXDV1ZYUZ/pUOCUMxURK +YNh8G8GT0WcWKsdRUjTFqf8dMFFRNw2fQZv80895T0EQLrmiUuZR99/A7YRBNxe1 +a+Fse5YzS8JrGYltiuTZxq7xwQKBgQD8onZFRYvapb1wggI0heeMT/4RfhBmuMjX +qphlNyOQvuSXz2KT8LodgLwhYD0F8F5LNOigf9Uk0sJ/VhrP1WvoQ10Csr2RUuo7 +GlmOFpxD+wEBcis2L3nantZRvjbVt6eEAAIQE++y7VvktG3X3xW9ZgRd2L47iQEh +x3Vmnd/2MwKBgQDadLXllYd07HzCbyjmI3OYN0TXbJczqzuyjHLWx5/xFYXVbtVr +FCU4x17gS+iUT560zn39hQR/WIkEY4eYX1WTGwO76ElyR7ruAomKOZF8I4PFGm/k +2IMTFS5JMIb/occLMhBybu+RiOUeKF963asBDu0fi+pDbGC5IZ3gn74FAQKBgGDW +YWFiLB6Og1P58aByZ3QoQWoxGVZWpF3OvYWmohJcqcDrNI0irCSc8QAWJK3/GhXX +3QeQmIH566XlundKBofMMn3TR8jJsJEhI4zMa+++6f7E5X1qq1m6ospIkDpRoHt/ +iUriaXH7e8rpwmUJ1Qp5bVkPuLOXa4CoNP81quBzAoGAIZSk1e/aSn6UGcBsEZpY +FYPIvuWckExme91xq9f5mgdL1bhJ7kzTtTVpP1/7du+82qhioD+BZyob+4sgJeAB +XyACnmWLM2aB3lUI5muQ5YSe7TMrtMBlvqNSFGQmk6/ioT4Bs9THGfUhrjqWvVav +c0iY5r5mMKIEUzAfRGdCyJE= +-----END PRIVATE KEY-----"; diff --git a/src/auth/service.rs b/src/auth/service.rs index 27c1f28..5150b1b 100644 --- a/src/auth/service.rs +++ b/src/auth/service.rs @@ -1,3 +1,10 @@ +//! The per-request tower `Service` the auth layer produces. +//! +//! `poll_ready` holds requests back until the first OIDC discovery has succeeded, so no request +//! is ever rejected merely because the key set had not arrived yet. `call` then extracts and +//! validates the token and, in `PassthroughMode::Block`, either inserts the `KeycloakToken` into +//! the request extensions or answers the error itself. + use std::{ sync::Arc, task::{Context, Poll}, @@ -12,6 +19,7 @@ use futures::future::BoxFuture; use http::Request; use serde::de::DeserializeOwned; +/// Wraps the inner service with token validation. Created by `KeycloakAuthLayer::layer`. #[derive(Clone)] pub struct KeycloakAuthService where @@ -19,7 +27,10 @@ where Extra: DeserializeOwned + Clone, { inner: S, - layer: KeycloakAuthLayer, + /// Shared rather than owned: tower clones the whole service per request, and `call` needs a + /// `'static` handle to move into the response future. Both are then a single refcount bump + /// instead of a structural copy of every field the layer holds. + layer: Arc>, } impl KeycloakAuthService @@ -27,10 +38,11 @@ where R: Role, Extra: DeserializeOwned + Clone, { + /// Clones the layer exactly once, when the router is built — never per request. pub fn new(inner: S, layer: &KeycloakAuthLayer) -> Self { Self { inner, - layer: layer.clone(), + layer: Arc::new(layer.clone()), } } } @@ -47,48 +59,25 @@ where type Future = BoxFuture<'static, Result>; fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll> { - // Once ready, we shall always be ready, independent of future discovery requests, - // satisfying a `poll_ready` requirement! - let is_ready = self.layer.instance.discovery.version() > 0; - - if is_ready { - tracing::debug!("Ready to process requests."); - } else { - tracing::debug!("Not ready to process requests. Waiting for initial discovery..."); - - // We have to assume that the discovery action was initialized. - // Our waker handling in would otherwise be wrong! - assert!(self.layer.instance.discovery.is_pending()); - let instance = self.layer.instance.clone(); - let waker = cx.waker().clone(); - tokio::spawn(async move { - instance.discovery.notified().await; - waker.wake(); - }); - } - - match (is_ready, self.inner.poll_ready(cx)) { - (true, Poll::Ready(t)) => Poll::Ready(t), - (false, _) => Poll::Pending, - (_, Poll::Pending) => Poll::Pending, - } + // No discovery gate here: `KeycloakAuthInstance::new` does not return until one discovery + // has succeeded, and a later failed refresh keeps the keys it already had. There is + // therefore no state in which this service exists without a usable key set to wait for. + self.inner.poll_ready(cx) } fn call(&mut self, mut request: Request) -> Self::Future { tracing::debug!("Validating request..."); let clone = self.inner.clone(); - let cloned_layer = self.layer.clone(); + let layer = Arc::clone(&self.layer); // Take the service that was ready! let mut inner = std::mem::replace(&mut self.inner, clone); - let passthrough_mode = cloned_layer.passthrough_mode; - Box::pin(async move { // Process the request. - let result = match extract::extract_jwt(&request, &cloned_layer.token_extractors) { - Some(token) => cloned_layer.validate_raw_token(&token).await, + let result = match extract::extract_jwt(&request, &layer.token_extractors) { + Some(token) => layer.validate_raw_token(&token).await, None => Err(AuthError::MissingToken), }; @@ -97,7 +86,7 @@ where if let Some(raw_claims) = raw_claims { request.extensions_mut().insert(raw_claims); } - match cloned_layer.passthrough_mode { + match layer.passthrough_mode { PassthroughMode::Block => { request.extensions_mut().insert(keycloak_token); } @@ -109,7 +98,7 @@ where }; inner.call(request).await } - Err(err) => match passthrough_mode { + Err(err) => match layer.passthrough_mode { PassthroughMode::Block => Ok(err.into_response()), PassthroughMode::Pass => { request diff --git a/src/auth/token.rs b/src/auth/token.rs new file mode 100644 index 0000000..5447dbf --- /dev/null +++ b/src/auth/token.rs @@ -0,0 +1,270 @@ +//! The claim types a validated JWT is parsed into. +//! +//! `StandardClaims` mirrors the JSON Keycloak emits; `KeycloakToken` is the typed form handed to +//! handlers through the request extensions. The validation that produces them lives in +//! `decode.rs`. + +use std::collections::HashMap; + +use chrono::{DateTime, TimeDelta, Utc}; +use serde::de::DeserializeOwned; +use serde::{Deserialize, Serialize}; +use serde_with::{OneOrMany, serde_as}; +use uuid::Uuid; + +use crate::auth::error::AuthError; +use crate::auth::role::{ExpectRoles, ExtractRoles, KeycloakRole, NumRoles, Role}; + +#[serde_as] +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct StandardClaims { + /// Expiration time (unix timestamp). + pub exp: i64, + /// Issued at time (unix timestamp). + pub iat: i64, + /// JWT ID (unique identifier for this token). + pub jti: String, + /// Issuer (who created and signed this token). + pub iss: String, + /// Audience (who or what the token is intended for). + #[serde_as(deserialize_as = "OneOrMany<_>")] + #[serde(default)] + pub aud: Vec, + /// Subject (whom the token refers to). + pub sub: String, + /// Type of token. + pub typ: String, + /// Authorized party (the party to which this token was issued). + pub azp: String, + + /// Keycloak: Optional realm roles from Keycloak. + pub realm_access: Option, + /// Keycloak: Optional client roles from Keycloak. + pub resource_access: Option, + + #[serde(flatten)] + pub extra: Extra, +} + +/// Access details. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Access { + /// A list of role names. + pub roles: Vec, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RealmAccess(pub Access); + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ResourceAccess(pub HashMap); + +impl NumRoles for RealmAccess { + fn num_roles(&self) -> usize { + self.0.roles.len() + } +} + +impl NumRoles for ResourceAccess { + fn num_roles(&self) -> usize { + self.0.values().map(|access| access.roles.len()).sum() + } +} + +impl ExtractRoles for RealmAccess { + fn extract_roles(self, target: &mut Vec>) { + for role in self.0.roles { + target.push(KeycloakRole::Realm { role: role.into() }); + } + } +} + +impl ExtractRoles for ResourceAccess { + fn extract_roles(self, target: &mut Vec>) { + for (res_name, access) in &self.0 { + for role in &access.roles { + target.push(KeycloakRole::Client { + client: res_name.to_owned(), + role: role.to_owned().into(), + }); + } + } + } +} + +/// Token data parsed from the request and added as an `axum::Extension` through our middleware. +/// +/// This only exists if the `KeycloakAuthLayer` is configured to use `PassthroughMode::Block`. +/// +/// If you want to manually check whether a request was authenticated, configure +/// `PassthroughMode::Pass` (potentially on a separate `axum::Router`) and inject +/// `KeycloakAuthStatus` instead of `KeycloakToken`! +/// +/// Handlers do not name this type directly — they take `CurrentUser`, which pins `R` to +/// `AppRole`: +/// +/// ```ignore +/// use crate::auth::CurrentUser; +/// +/// pub async fn who_am_i(user: CurrentUser) -> Response { +/// let name = &user.extra.profile.preferred_username; +/// // ... +/// } +/// ``` +#[derive(Debug, PartialEq, Clone)] +pub struct KeycloakToken +where + R: Role, + Extra: DeserializeOwned + Clone, +{ + /// Expiration time (UTC). + pub expires_at: DateTime, + /// Issued at time (UTC). + pub issued_at: DateTime, + /// JWT ID (unique identifier for this token). + pub jwt_id: String, + /// Issuer (who created and signed this token). + pub issuer: String, + /// Audience (who or what the token is intended for). + pub audience: Vec, + /// Subject (whom the token refers to). This is the UUID which uniquely identifies this user inside Keycloak. + pub subject: Uuid, + /// Authorized party (the party to which this token was issued). + pub authorized_party: String, + + // Keycloak: Roles of the user. + pub roles: Vec>, + + pub extra: Extra, +} + +impl KeycloakToken +where + R: Role, + Extra: DeserializeOwned + Clone, +{ + pub fn parse(raw: StandardClaims) -> Result { + Ok(Self { + expires_at: DateTime::from_timestamp(raw.exp, 0).ok_or_else(|| { + AuthError::InvalidToken { + reason: format!( + "Could not parse 'exp' (expires_at) field as unix timestamp: {}", + raw.exp + ), + } + })?, + issued_at: DateTime::from_timestamp(raw.iat, 0).ok_or_else(|| { + AuthError::InvalidToken { + reason: format!( + "Could not parse 'iat' (issued_at) field as unix timestamp: {}", + raw.iat + ), + } + })?, + jwt_id: raw.jti, + issuer: raw.iss, + audience: raw.aud, + subject: Uuid::try_parse(&raw.sub).map_err(|err| AuthError::InvalidToken { + reason: format!("Could not parse 'sub' (subject) field as uuid: {err}"), + })?, + authorized_party: raw.azp, + roles: { + let mut roles = Vec::new(); + (raw.realm_access, raw.resource_access).extract_roles(&mut roles); + roles + }, + extra: raw.extra, + }) + } + + /// Whether the **realm** granted this role. + /// + /// The check every access decision should use. Client roles are deliberately not consulted: + /// Keycloak ships the roles of every client the user holds roles on in `resource_access`, so + /// accepting those would let an unrelated service in the realm grant access to ISM simply by + /// naming one of its own roles `ADMIN`. + pub fn has_realm_role(&self, role: &R) -> bool { + self.roles.iter().any(|it| it.realm_role() == Some(role)) + } + + /// Whether `client` granted this role, for the rare case where a client-scoped role is meant. + pub fn has_client_role(&self, client: &str, role: &R) -> bool { + self.roles + .iter() + .any(|it| it.client_role(client) == Some(role)) + } + + /// Whether the token is past its `exp`, allowing `leeway` of clock drift. + /// + /// `leeway` must match what `ValidationPolicy` hands to `jsonwebtoken` — see + /// `decode::EXPIRY_LEEWAY_SECS`. Both checks run on every request, and the stricter one decides, + /// so a mismatch would silently make one of them dead. + pub fn is_expired(&self, leeway: TimeDelta) -> bool { + // An `exp` so far out that adding the leeway leaves the representable range is, by any + // reading, not expired. + self.expires_at + .checked_add_signed(leeway) + .is_some_and(|deadline| Utc::now() > deadline) + } + + pub fn assert_not_expired(&self, leeway: TimeDelta) -> Result<(), AuthError> { + match self.is_expired(leeway) { + true => Err(AuthError::TokenExpired), + false => Ok(()), + } + } +} + +impl ExpectRoles for KeycloakToken +where + R: Role, + Extra: DeserializeOwned + Clone, +{ + type Rejection = AuthError; + + /// Realm roles only — a client role of the same name does not grant access. See the trait docs. + fn expect_roles + Clone>(&self, roles: &[I]) -> Result<(), Self::Rejection> { + for expected in roles { + let expected: R = expected.clone().into(); + if !self.has_realm_role(&expected) { + return Err(AuthError::MissingExpectedRole { + role: expected.to_string(), + }); + } + } + Ok(()) + } + + /// Every role source — a denial must hold regardless of who granted the role. See the trait docs. + fn not_expect_roles + Clone>(&self, roles: &[I]) -> Result<(), Self::Rejection> { + for expected in roles { + let expected: R = expected.clone().into(); + if self.roles.iter().any(|role| role.role() == &expected) { + return Err(AuthError::UnexpectedRole); + } + } + Ok(()) + } +} + +#[derive(serde::Deserialize, Debug, Clone)] +pub struct Profile { + /// Keycloak: Username of the user. + pub preferred_username: String, +} + +#[derive(serde::Deserialize, Debug, Clone)] +pub struct Email { + /// Keycloak: Email address of the user. + pub email: Option, + /// Keycloak: Whether the users email is verified. + pub email_verified: bool, +} + +#[derive(serde::Deserialize, Debug, Clone)] +pub struct ProfileAndEmail { + #[serde(flatten)] + pub profile: Profile, + #[serde(flatten)] + pub email: Email, +} diff --git a/src/broadcast/event_broadcast.rs b/src/broadcast/event_broadcast.rs index 5aab86f..5a7d388 100644 --- a/src/broadcast/event_broadcast.rs +++ b/src/broadcast/event_broadcast.rs @@ -1,11 +1,11 @@ use crate::broadcast::{Notification, NotificationEvent}; use crate::cache::redis_cache::{Cache, ReplayResult}; use crate::kafka::{EventProducer, PushNotificationProducer}; -use log::{debug, error, info}; use std::collections::HashMap; use std::sync::Arc; use tokio::sync::broadcast::{Receiver, Sender, channel}; use tokio::sync::{OnceCell, RwLock}; +use tracing::{debug, error, info}; use uuid::Uuid; static BROADCAST_INSTANCE: OnceCell> = OnceCell::const_new(); @@ -115,12 +115,14 @@ impl BroadcastChannel { .add_notification_for_user(user_id, ¬ification) .await { - error!("Failed to cache notification: {}", error); + error!(%user_id, error = %error, "Failed to cache notification"); } } // No sequencing (no Redis): deliver best-effort without replay support. Ok(None) => {} - Err(err) => error!("Failed to allocate sequence for user {}: {}", user_id, err), + Err(err) => { + error!(%user_id, error = %err, "Failed to allocate notification sequence") + } } } @@ -130,11 +132,14 @@ impl BroadcastChannel { // `send` only errors when there are no active receivers, i.e. the user is offline. Some(sender) => match sender.send(notification.clone()) { Ok(sc) => { - info!("Successfully sent {:?} broadcast event.", sc); + debug!(%user_id, receivers = sc, "Broadcast event delivered"); true } Err(err) => { - error!("Unable to broadcast notification: {}", err); + // `send` only fails when nobody is listening, i.e. the user is offline. + // That is expected, not an error: the push-notification path below picks + // it up. + debug!(%user_id, error = %err, "No active receiver for notification"); false } }, @@ -162,25 +167,26 @@ impl BroadcastChannel { ); if should_send { + let recipients = to_user.len(); if let Err(error) = self .push_notification_producer .send_notification(notification, to_user) .await { - error!("Failed to send push notification: {}", error); + error!(recipients, error = %error, "Failed to send push notification"); } } } pub async fn unsubscribe(&self, user_id: Uuid) { - debug!("Unsubscribing user {:?} from broadcasting events.", user_id); + debug!(%user_id, "Unsubscribing user from broadcast events"); let mut lock = self.channel.write().await; if let Some(sender) = lock.get(&user_id) { if sender.receiver_count() > 0 { return; } else { lock.remove(&user_id); - debug!("Removed stale sender for user {:?}", user_id); + debug!(%user_id, "Removed stale broadcast sender"); } } } diff --git a/src/cache/redis_cache.rs b/src/cache/redis_cache.rs index acdd27c..9221a1c 100644 --- a/src/cache/redis_cache.rs +++ b/src/cache/redis_cache.rs @@ -3,10 +3,10 @@ use crate::cache::redis_subscriber::run_event_processor; use crate::cache::util::{CHAT_CHANNEL, ROOM_CONTEXT, USER_NOTIFICATIONS, USER_SEQUENCE}; use crate::rooms::room_member::RoomContext; use async_trait::async_trait; -use log::info; use redis::aio::ConnectionManager; use redis::aio::ConnectionManagerConfig; use redis::{AsyncTypedCommands, Client, ErrorKind, RedisError, RedisResult}; +use tracing::info; use uuid::Uuid; /// TTL for the per-user sequence counter and notification stream. Refreshed on every write, so a diff --git a/src/cache/redis_subscriber.rs b/src/cache/redis_subscriber.rs index ea13172..34dbb7f 100644 --- a/src/cache/redis_subscriber.rs +++ b/src/cache/redis_subscriber.rs @@ -1,12 +1,11 @@ use crate::broadcast::{BroadcastChannel, Notification, NotificationEvent}; use crate::cache::util::ROOM_CONTEXT; use crate::rooms::room_member::RoomContext; -use log::info; use redis::aio::ConnectionManager; use redis::{AsyncTypedCommands, PushInfo, RedisError, from_redis_value}; use thiserror::Error; use tokio::sync::mpsc::UnboundedReceiver; -use tracing::{error, warn}; +use tracing::{debug, error, info, warn}; use uuid::Uuid; #[derive(Debug, Error)] @@ -29,20 +28,17 @@ pub async fn run_event_processor(mut rx: UnboundedReceiver, mut conn: info!("Redis Event-Processing active."); while let Some(push_message) = rx.recv().await { - info!("Received push message: {:?}", push_message); + debug!(?push_message, "Received push message"); let notification = match parse_push_message(push_message) { Ok(message) => message, Err(error) => { - warn!( - "Parsing of received push message failed. Ignoring. Push message: {:?}", - error - ); + warn!(?error, "Parsing of received push message failed, ignoring"); continue; } }; if let Err(e) = handle_notification(notification, &mut conn).await { - error!("Fehler bei der Verarbeitung der Notification: {}", e); + error!(error = %e, "Failed to process notification"); } } } diff --git a/src/core/app_state.rs b/src/core/app_state.rs index 6d09db3..6cdaade 100644 --- a/src/core/app_state.rs +++ b/src/core/app_state.rs @@ -6,9 +6,9 @@ use crate::messaging::chat_repository::ChatRepository; use crate::object_storage::ObjectStorage; use crate::rooms::room_repository::RoomRepository; use crate::users::user_repository::UserRepository; -use log::info; use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; use std::sync::Arc; +use tracing::info; #[derive(Clone)] pub struct AppState { diff --git a/src/core/config.rs b/src/core/config.rs index f1481aa..a3466f7 100644 --- a/src/core/config.rs +++ b/src/core/config.rs @@ -37,6 +37,48 @@ pub struct RoomDbConfig { pub struct TokenIssuer { pub iss_host: String, pub iss_realm: String, + + /// Accepted values of the JWT `aud` claim. + /// + /// The default `["account"]` is what Keycloak's built-in `audience-resolve` mapper adds to + /// *every* access token of *every* client in the realm — it therefore does not scope access to + /// a particular client. Configure a dedicated audience mapper in Keycloak and set this to that + /// audience to make the check meaningful. + #[serde(default = "default_expected_audiences")] + pub expected_audiences: Vec, + + /// Accepted values of the JWT `azp` (authorized party) claim. Empty disables the check. + /// + /// Set this to your application's Keycloak client id to reject tokens minted for other clients + /// in the same realm. + #[serde(default)] + pub expected_azp: Vec, + + /// Signature algorithms accepted for incoming tokens. + /// + /// This is a fixed allow-list: the algorithm named in the (attacker-controlled) JWT header is + /// never used to decide which algorithms are acceptable. + #[serde(default = "default_allowed_algorithms")] + pub allowed_algorithms: Vec, + + /// Minimum time between two OIDC discoveries, in seconds. + /// + /// Rate-limits on-demand refreshes so a flood of invalid tokens cannot turn into a request + /// storm against Keycloak. + #[serde(default = "default_jwks_min_refresh_interval_secs")] + pub jwks_min_refresh_interval_secs: u64, +} + +fn default_expected_audiences() -> Vec { + vec![String::from("account")] +} + +fn default_allowed_algorithms() -> Vec { + vec![String::from("RS256")] +} + +fn default_jwks_min_refresh_interval_secs() -> u64 { + 30 } #[derive(Deserialize, Debug, Clone)] diff --git a/src/core/errors.rs b/src/core/errors.rs index 22088a1..5dfd6b7 100644 --- a/src/core/errors.rs +++ b/src/core/errors.rs @@ -33,18 +33,18 @@ impl ErrorResponse { #[derive(Serialize, Debug)] #[serde(rename_all = "SCREAMING_SNAKE_CASE")] -#[allow(dead_code)] pub enum ErrorCode { // Authentication & Authorization + /// The request carried no usable credentials, or they were rejected. + Unauthorized, + /// The presented token was well-formed but has expired. + TokenExpired, + /// The caller is authenticated but lacks the required permission. InsufficientPermissions, - - // User & Profile Errors - UserNotFound, + /// The identity provider could not be reached or its configuration is unusable. + AuthUnavailable, // Content & Interaction Errors - RoomNotFound, - MessageNotFound, - InvalidContent, FileProcessingError, ContentNotFound, @@ -106,6 +106,22 @@ impl From for AppError { } } +impl AppError { + /// Stable, low-cardinality discriminant used as a structured log field. + fn kind(&self) -> &'static str { + match self { + AppError::Validation(_) => "validation", + AppError::NotFound(_) => "not_found", + AppError::Forbidden(_) => "forbidden", + AppError::Database(_) => "database", + AppError::Cache(_) => "cache", + AppError::Serialization(_) => "serialization", + AppError::S3(_) => "s3", + AppError::Processing(_) => "processing", + } + } +} + impl IntoResponse for AppError { fn into_response(self) -> Response { // Log every internal error with its full details before the message is sanitised. @@ -114,7 +130,9 @@ impl IntoResponse for AppError { | AppError::Cache(_) | AppError::Serialization(_) | AppError::S3(_) - | AppError::Processing(_) => tracing::error!("{}", self), + | AppError::Processing(_) => { + tracing::error!(error.kind = self.kind(), error = ?self, "Internal error") + } _ => {} } diff --git a/src/main.rs b/src/main.rs index ea9dc1a..746691a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -6,7 +6,6 @@ use tokio::net::TcpListener; use tokio::signal; use tracing::info; use tracing_subscriber::EnvFilter; -use tracing_subscriber::filter::LevelFilter; //learn to code rust axum here: //https://gitlab.com/famedly/conduit/-/tree/next?ref_type=heads @@ -66,12 +65,29 @@ fn init_configuration() -> ISMConfig { let config = ISMConfig::new(&run_mode).unwrap_or_else(|err| panic!("Missing needed env: {}", err)); + init_tracing(&config.log_level); + + config +} + +/// Installs the tracing subscriber. +/// +/// `configured` is a full `EnvFilter` directive list, not a single level: a global default +/// followed by any number of `target=level` overrides, e.g. `"info,sqlx=warn,ism::auth=debug"`. +/// Targets are module paths, so they can be narrowed as far as needed. +/// +/// `ISM_LOG_LEVEL` replaces the configured value entirely when set and non-empty. +fn init_tracing(configured: &str) { + let directives = env::var("ISM_LOG_LEVEL") + .ok() + .filter(|it| !it.trim().is_empty()) + .unwrap_or_else(|| configured.to_owned()); + + // Parse strictly and fail at startup. A typo previously fell back to a bare INFO default, + // silently discarding every per-target override the operator had configured. let filter = EnvFilter::builder() - .with_env_var("ISM_LOG_LEVEL") - .with_default_directive(LevelFilter::INFO.into()) - .from_env_lossy(); + .parse(&directives) + .unwrap_or_else(|err| panic!("Invalid log filter {directives:?}: {err}")); tracing_subscriber::fmt().with_env_filter(filter).init(); - - config } diff --git a/src/messaging/handler.rs b/src/messaging/handler.rs index 005bf62..bc1e33d 100644 --- a/src/messaging/handler.rs +++ b/src/messaging/handler.rs @@ -1,19 +1,19 @@ -use crate::auth::decode::KeycloakToken; +use crate::auth::CurrentUser; use crate::core::AppState; use crate::core::errors::AppError; use crate::messaging::message_service::MessageService; use crate::messaging::model::{MessageDto, NewMessage}; +use axum::Json; use axum::extract::State; -use axum::{Extension, Json}; use std::sync::Arc; use validator::Validate; pub async fn handle_send_message( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, Json(payload): Json, ) -> Result, AppError> { payload.validate().map_err(AppError::from)?; - let response_msg = MessageService::send_message(state, payload, token.subject).await?; + let response_msg = MessageService::send_message(state, payload, user.subject).await?; Ok(Json(response_msg)) } diff --git a/src/messaging/notifications.rs b/src/messaging/notifications.rs index 8f13a65..65e4ba7 100644 --- a/src/messaging/notifications.rs +++ b/src/messaging/notifications.rs @@ -1,16 +1,15 @@ -use crate::auth::decode::KeycloakToken; +use crate::auth::CurrentUser; use crate::broadcast::{BroadcastChannel, Notification, NotificationEvent}; use crate::cache::redis_cache::ReplayResult; use crate::core::AppState; use crate::core::errors::AppResponse; +use axum::Json; use axum::extract::ws::{Message, WebSocket, WebSocketUpgrade}; use axum::extract::{Query, State}; use axum::response::sse::Event; use axum::response::{IntoResponse, Sse}; -use axum::{Extension, Json}; use bytes::Bytes; use futures::Stream; -use log::{debug, error}; use serde::{Deserialize, Serialize}; use std::sync::Arc; use std::time::Duration; @@ -18,7 +17,7 @@ use tokio::sync::broadcast::error::RecvError; use tokio::time; use tokio_stream::wrappers::BroadcastStream; use tokio_stream::wrappers::errors::BroadcastStreamRecvError; -use tracing::warn; +use tracing::{debug, error, warn}; use uuid::Uuid; /// Handshake parameters shared by the SSE and WebSocket endpoints. The client passes the @@ -89,7 +88,7 @@ async fn resolve_handshake( 0, ), Err(err) => { - error!("Failed to fetch replay for {}: {}", user_id, err); + error!(%user_id, error = %err, "Failed to fetch notification replay"); ( vec![resync_notification("replay error, please resync via REST")], 0, @@ -99,12 +98,13 @@ async fn resolve_handshake( } pub async fn stream_server_events( - Extension(token): Extension>, + user: CurrentUser, Query(params): Query, ) -> Sse>> { use futures::StreamExt; - let user_id = token.subject; + // Bound out of the token: the live stream below outlives this scope and only needs the id. + let user_id = user.subject; let bc = BroadcastChannel::get(); // Subscribe before reading the replay so live events produced during the handshake are @@ -131,10 +131,7 @@ pub async fn stream_server_events( } } Err(BroadcastStreamRecvError::Lagged(n)) => { - warn!( - "SSE client {} lagged by {} events, signalling resync", - user_id, n - ); + warn!(%user_id, lagged_events = n, "SSE client lagged, signalling resync"); Some(Ok(notification_to_sse(&resync_notification( "stream lagged, please resync via REST", )))) @@ -154,12 +151,14 @@ pub async fn stream_server_events( pub async fn websocket_server_events( websocket: WebSocketUpgrade, - Extension(token): Extension>, + user: CurrentUser, Query(params): Query, ) -> impl IntoResponse { + // Bound out of the token so the upgrade closure captures a `Copy` id, not the whole token. + let user_id = user.subject; websocket .on_failed_upgrade(|error| warn!("Error upgrading websocket: {}", error)) - .on_upgrade(move |socket| handle_socket(socket, token.subject, params.last_seq)) + .on_upgrade(move |socket| handle_socket(socket, user_id, params.last_seq)) } async fn handle_socket(mut socket: WebSocket, user_id: Uuid, last_seq: Option) { @@ -195,7 +194,7 @@ async fn handle_socket(mut socket: WebSocket, user_id: Uuid, last_seq: Option { - warn!("WS client {} lagged by {} events, signalling resync", user_id, n); + warn!(%user_id, lagged_events = n, "WS client lagged, signalling resync"); let resync = serde_json::to_string(&resync_notification("stream lagged, please resync via REST")).unwrap_or_default(); if socket.send(Message::text(resync)).await.is_err() { break; @@ -269,11 +268,11 @@ pub struct NotificationCursor { pub async fn get_notification_cursor( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, ) -> AppResponse> { let seq = state .cache - .current_sequence(&token.subject) + .current_sequence(&user.subject) .await? .unwrap_or(0); Ok(Json(NotificationCursor { seq })) @@ -281,12 +280,12 @@ pub async fn get_notification_cursor( pub async fn get_latest_notification_events( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, Query(params): Query, ) -> AppResponse>> { let notifications = match state .cache - .get_notifications_since_seq(&token.subject, params.last_seq) + .get_notifications_since_seq(&user.subject, params.last_seq) .await? { ReplayResult::Events(events) => events, diff --git a/src/object_storage/object_storage.rs b/src/object_storage/object_storage.rs index c04ef4e..6fafce3 100644 --- a/src/object_storage/object_storage.rs +++ b/src/object_storage/object_storage.rs @@ -1,6 +1,5 @@ use crate::core::ObjectStorageConfig; use bytes::Bytes; -use log::{debug, info}; use minio::s3::builders::{ObjectContent, ObjectToDelete}; use minio::s3::creds::StaticProvider; use minio::s3::http::BaseUrl; @@ -8,6 +7,7 @@ use minio::s3::segmented_bytes::SegmentedBytes; use minio::s3::types::S3Api; use minio::s3::{Client, ClientBuilder}; use std::sync::Arc; +use tracing::{debug, info}; #[derive(Debug, Clone)] pub struct ObjectStorage { @@ -75,7 +75,7 @@ impl ObjectStorage { .delete_object(&self.config.bucket_name, ObjectToDelete::from(object_id)) .send() .await?; - debug!("Deleted object, marker: {:?}", response.version_id); + debug!(version_id = ?response.version_id, "Deleted object"); Ok(()) } @@ -91,7 +91,7 @@ impl ObjectStorage { .content_type("image/jpeg".to_string()) .send() .await?; - debug!("Saved object with name: {:?}", response.object); + debug!(object = ?response.object, "Saved object"); Ok(()) } } diff --git a/src/rooms/handler.rs b/src/rooms/handler.rs index 409bca0..d314b84 100644 --- a/src/rooms/handler.rs +++ b/src/rooms/handler.rs @@ -1,4 +1,4 @@ -use crate::auth::decode::KeycloakToken; +use crate::auth::CurrentUser; use crate::core::AppState; use crate::core::cursor::{CursorResults, clamp_page_size, decode_cursor}; use crate::core::errors::AppError; @@ -13,14 +13,14 @@ use crate::rooms::share_service::{ShareService, ShareTarget, ShareTargetCursor}; use crate::rooms::timeline_service::TimelineService; use crate::users::user_service::UserService; use crate::utils::check_user_in_room; +use axum::Json; use axum::extract::{Multipart, Path, Query, State}; -use axum::{Extension, Json}; use bytes::Bytes; use chrono::{DateTime, Utc}; -use log::error; use serde::Deserialize; use std::collections::HashSet; use std::sync::Arc; +use tracing::error; use uuid::Uuid; use validator::Validate; @@ -44,29 +44,29 @@ pub struct TimelineQueryParam { } pub async fn handle_scroll_chat_timeline( - Extension(token): Extension>, + user: CurrentUser, State(state): State>, Path(room_id): Path, Query(params): Query, ) -> Result, AppError> { - check_user_in_room(&state, &token.subject, &room_id).await?; + check_user_in_room(&state, &user.subject, &room_id).await?; let page = TimelineService::scroll_chat_timeline(state, room_id, params.timestamp).await?; Ok(Json(page)) } pub async fn handle_get_users_in_room( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, Path(room_id): Path, ) -> Result>, AppError> { - check_user_in_room(&state, &token.subject, &room_id).await?; + check_user_in_room(&state, &user.subject, &room_id).await?; let users = RoomService::get_users_in_room(state, room_id).await?; Ok(Json(users)) } pub async fn handle_get_joined_rooms( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, Query(params): Query, ) -> Result>, AppError> { let cursor: RoomPaginationCursor = decode_cursor(params.cursor) @@ -74,13 +74,13 @@ pub async fn handle_get_joined_rooms( let page_size = clamp_page_size(params.limit); let rooms = - RoomService::get_joined_rooms(state, token.subject, params.name, cursor, page_size).await?; + RoomService::get_joined_rooms(state, user.subject, params.name, cursor, page_size).await?; Ok(Json(rooms)) } pub async fn handle_get_share_targets( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, Query(params): Query, ) -> Result>, AppError> { let cursor: ShareTargetCursor = decode_cursor(params.cursor) @@ -88,35 +88,35 @@ pub async fn handle_get_share_targets( let page_size = clamp_page_size(params.limit); let targets = - ShareService::get_share_targets(state, token.subject, params.name, cursor, page_size) + ShareService::get_share_targets(state, user.subject, params.name, cursor, page_size) .await?; Ok(Json(targets)) } pub async fn handle_get_room_with_details( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, Path(room_id): Path, ) -> Result, AppError> { - let room = RoomService::get_room_with_details(state, token.subject, room_id).await?; + let room = RoomService::get_room_with_details(state, user.subject, room_id).await?; Ok(Json(room)) } pub async fn mark_room_as_read( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, Path(room_id): Path, ) -> Result<(), AppError> { - RoomService::mark_room_as_read(state, token.subject, room_id).await?; + RoomService::mark_room_as_read(state, user.subject, room_id).await?; Ok(()) } pub async fn handle_create_room( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, Json(mut payload): Json, ) -> Result, AppError> { - if !payload.invited_users.contains(&token.subject) { + if !payload.invited_users.contains(&user.subject) { return Err(AppError::Validation( "Sender ID is not in the list of invited users.".to_string(), )); @@ -128,7 +128,7 @@ pub async fn handle_create_room( //filter out all users that have an ignore-relationship with the sender let ignored = - UserService::get_blocked_users(state.clone(), &token.subject, &payload.invited_users) + UserService::get_blocked_users(state.clone(), &user.subject, &payload.invited_users) .await?; let filter_set: HashSet<_> = ignored.iter().collect(); payload @@ -145,12 +145,12 @@ pub async fn handle_create_room( let other_user = payload .invited_users .iter() - .find(|&&el| el != token.subject) + .find(|&&el| el != user.subject) .ok_or_else(|| { AppError::Validation("Personal rooms must contain another user.".to_string()) })?; let has_active_chat = - RoomService::find_existing_single_room(state.clone(), &token.subject, other_user) + RoomService::find_existing_single_room(state.clone(), &user.subject, other_user) .await?; if has_active_chat.is_some() { return Err(AppError::Validation( @@ -166,60 +166,61 @@ pub async fn handle_create_room( } } } - let room = RoomService::create_room(state, token.subject, payload).await?; + let room = RoomService::create_room(state, user.subject, payload).await?; Ok(Json(room)) } pub async fn handle_get_room_list_item_by_id( - Extension(token): Extension>, + user: CurrentUser, State(state): State>, Path(room_id): Path, ) -> Result, AppError> { - let room = RoomService::get_room_list_item_by_id(state, token.subject, room_id).await?; + let room = RoomService::get_room_list_item_by_id(state, user.subject, room_id).await?; Ok(Json(room)) } pub async fn handle_leave_room( - Extension(token): Extension>, + user: CurrentUser, State(state): State>, Path(room_id): Path, ) -> Result<(), AppError> { - RoomService::leave_room(state, token.subject, room_id).await?; + RoomService::leave_room(state, user.subject, room_id).await?; Ok(()) } pub async fn handle_invite_to_room( - Extension(token): Extension>, + user: CurrentUser, State(state): State>, - Path((room_id, user_id)): Path<(Uuid, Uuid)>, + Path((room_id, invited_user_id)): Path<(Uuid, Uuid)>, ) -> Result<(), AppError> { let ignored = - UserService::get_blocked_users(state.clone(), &token.subject, &vec![user_id]).await?; - if ignored.contains(&user_id) { + UserService::get_blocked_users(state.clone(), &user.subject, &vec![invited_user_id]) + .await?; + if ignored.contains(&invited_user_id) { return Err(AppError::Forbidden("User is blocked.".to_string())); } - RoomService::invite_to_room(state, token.subject, room_id, user_id).await?; + RoomService::invite_to_room(state, user.subject, room_id, invited_user_id).await?; Ok(()) } pub async fn handle_search_existing_single_room( - Extension(token): Extension>, + user: CurrentUser, State(state): State>, Query(params): Query, ) -> Result>, AppError> { let result = - RoomService::find_existing_single_room(state, &token.subject, ¶ms.with_user).await?; + RoomService::find_existing_single_room(state, &user.subject, ¶ms.with_user).await?; Ok(Json(result)) } pub async fn handle_save_room_image( - Extension(token): Extension>, + user: CurrentUser, State(state): State>, Path(room_id): Path, mut multipart: Multipart, ) -> Result, AppError> { - check_user_in_room(&state, &token.subject, &room_id).await?; + check_user_in_room(&state, &user.subject, &room_id).await?; let mut image_data: Option = None; loop { match multipart.next_field().await { @@ -242,7 +243,7 @@ pub async fn handle_save_room_image( } Err(err) => { //read error - error!("Bad image upload: {}", err.to_string()); + error!(error = %err, "Bad image upload"); return Err(AppError::Validation( "Error reading the image byte stream.".to_string(), )); @@ -261,11 +262,11 @@ pub async fn handle_save_room_image( } pub async fn handle_get_read_states( - Extension(token): Extension>, + user: CurrentUser, State(state): State>, Path(room_id): Path, ) -> Result>, AppError> { - check_user_in_room(&state, &token.subject, &room_id).await?; + check_user_in_room(&state, &user.subject, &room_id).await?; let read_states = RoomService::get_read_states(state, room_id).await?; Ok(Json(read_states)) } diff --git a/src/rooms/room_service.rs b/src/rooms/room_service.rs index 96856ed..4953a1d 100644 --- a/src/rooms/room_service.rs +++ b/src/rooms/room_service.rs @@ -14,8 +14,8 @@ use crate::rooms::room::{ use crate::rooms::room_member::RoomMember; use crate::utils::crop_image_from_center; use bytes::Bytes; -use log::error; use std::sync::Arc; +use tracing::error; use uuid::Uuid; pub struct RoomService; @@ -403,7 +403,7 @@ impl RoomService { image_data: Bytes, ) -> Result { let img = crop_image_from_center(&image_data, 500, 500).map_err(|err| { - error!("Unable to crop image: {}", err.to_string()); + error!(error = %err, "Unable to crop image"); AppError::Processing("Unable to crop image.".to_string()) })?; @@ -413,7 +413,7 @@ impl RoomService { .insert_object(&room_id.to_string(), img) .await { - error!("{}", err.to_string()); + error!(error = %err, "Image processing failed"); return Err(AppError::S3("Unable save image in s3 bucket.".to_string())); }; state diff --git a/src/router.rs b/src/router.rs index 0edb31f..e129131 100644 --- a/src/router.rs +++ b/src/router.rs @@ -1,6 +1,7 @@ -use crate::auth::PassthroughMode; -use crate::auth::instance::{KeycloakAuthInstance, KeycloakConfig}; -use crate::auth::layer::KeycloakAuthLayer; +use crate::auth::{ + AppRole, KeycloakAuthInstance, KeycloakAuthLayer, KeycloakConfig, PassthroughMode, + ValidationPolicy, error_chain, +}; use crate::core::{AppState, TokenIssuer}; use crate::messaging::routes::create_messaging_routes; use crate::rooms::routes::create_room_routes; @@ -17,9 +18,11 @@ use axum::response::{IntoResponse, Response}; use axum::routing::get; use http::header::{CONNECTION, CONTENT_LENGTH, ORIGIN}; use std::sync::Arc; +use std::time::Duration; use tower::ServiceBuilder; use tower_http::cors::CorsLayer; -use tower_http::trace::TraceLayer; +use tower_http::trace::{DefaultOnFailure, TraceLayer}; +use tracing::Level; use url::Url; /** @@ -27,7 +30,7 @@ use url::Url; */ pub async fn init_router(app_state: AppState) -> Router { let origin = app_state.env.cors_origin.clone(); - let cors = CorsLayer::new() + let cors_layer = CorsLayer::new() .allow_origin(origin.parse::().expect("Invalid CORS Origin")) .allow_headers([ AUTHORIZATION, @@ -40,12 +43,11 @@ pub async fn init_router(app_state: AppState) -> Router { .allow_credentials(true) .allow_methods([Method::GET, Method::POST, Method::OPTIONS, Method::DELETE]); + let auth_layer = init_auth(app_state.env.token_issuer.clone()).await; + let public_routing = Router::new() .route("/", get(|| async { "Hello, world! I'm your new ISM. 🤗" })) - .route( - "/health", - get(|| async { (StatusCode::OK, "Healthy").into_response() }), - ); + .route("/health", get(|| async { (StatusCode::OK, "Healthy").into_response() }), ); let protected_routing = Router::new() .nest( @@ -58,16 +60,45 @@ pub async fn init_router(app_state: AppState) -> Router { //layering bottom to top middleware .layer( ServiceBuilder::new() //layering top to bottom middleware - .layer(TraceLayer::new_for_http()) //1 - .layer(cors) //2 - .layer(init_auth(app_state.env.token_issuer.clone())) //3.. + .layer(http_trace_layer()) //1 + .layer(cors_layer) //2 + .layer(auth_layer) //3.. .layer(DefaultBodyLimit::max(5 * 1024 * 1024)), //max 5mb files ) .layer(axum::middleware::from_fn(inject_request_path)) .with_state(Arc::new(app_state)); + public_routing.merge(protected_routing) } +/// HTTP request tracing. +/// +/// The defaults of `TraceLayer::new_for_http()` put both the span and the response event at DEBUG, +/// which means nothing at all is emitted at the default INFO level — and application errors then +/// arrive with no request context attached. Spans are built explicitly here instead. +fn http_trace_layer() -> TraceLayer< + tower_http::classify::SharedClassifier, + impl Fn(&Request) -> tracing::Span + Clone, +> { + TraceLayer::new_for_http() + .make_span_with(|request: &Request| { + // The matched route, not the raw URI: path parameters would give the span unbounded + // cardinality. + let path = request + .extensions() + .get::() + .map(|mp| mp.as_str()) + .unwrap_or_else(|| request.uri().path()); + + tracing::info_span!( + "HTTP_REQUEST", + method = %request.method(), + path = %path, + ) + }) + .on_failure(DefaultOnFailure::new().level(Level::ERROR)) +} + async fn inject_request_path( matched_path: Option, uri: Uri, @@ -103,17 +134,57 @@ async fn inject_request_path( Response::from_parts(parts, axum::body::Body::from(bytes)) } -fn init_auth(config: TokenIssuer) -> KeycloakAuthLayer { +/// Builds the auth middleware, performing the initial OIDC discovery on the way. +/// +/// Panics if that discovery does not succeed, in line with the other startup-configuration +/// failures here and in `main`. Without discovered keys not a single token can be verified and +/// nothing re-runs discovery on a timer, so the alternative is a process that answers every +/// authenticated request with a 503 for as long as it stays up. +async fn init_auth(config: TokenIssuer) -> KeycloakAuthLayer { + let server = Url::parse(&config.iss_host).expect("Invalid Keycloak Host"); + + if server.scheme() != "https" { + tracing::warn!( + keycloak_host = %server, + "Keycloak is configured over plaintext HTTP. Tokens and the JWKS are exchanged in the \ + clear — use HTTPS outside of local development." + ); + } + + let policy = ValidationPolicy::new( + config.expected_audiences, + config.expected_azp, + &config.allowed_algorithms, + ) + .unwrap_or_else(|err| panic!("Invalid [token_issuer] configuration: {err}")); + + if policy.expected_audiences.iter().any(|it| it == "account") && policy.expected_azp.is_empty() + { + tracing::warn!( + "Accepting the 'account' audience with no 'expected_azp' restriction: Keycloak adds \ + this audience to every access token of every client in the realm, so any client in \ + the realm is accepted. Configure an audience mapper and set expected_audiences / \ + expected_azp." + ); + } + let keycloak_auth_instance = KeycloakAuthInstance::new( KeycloakConfig::builder() - .server(Url::parse(&config.iss_host).expect("Invalid Keycloak Host")) + .server(server) .realm(config.iss_realm) + .min_refresh_interval(Duration::from_secs(config.jwks_min_refresh_interval_secs)) .build(), - ); - KeycloakAuthLayer::::builder() + ) + .await + .unwrap_or_else(|err| { + panic!("Initial OIDC discovery failed, refusing to start: {}", error_chain(&err)) + }); + + KeycloakAuthLayer::::builder() .instance(keycloak_auth_instance) + // Nothing reads the raw claim map; persisting it clones the whole map per request. + .persist_raw_claims(false) .passthrough_mode(PassthroughMode::Block) - .persist_raw_claims(true) - .expected_audiences(vec![String::from("account")]) + .validation_policy(policy) .build() } diff --git a/src/users/handler.rs b/src/users/handler.rs index 665ce84..a375c8c 100644 --- a/src/users/handler.rs +++ b/src/users/handler.rs @@ -1,4 +1,4 @@ -use crate::auth::decode::KeycloakToken; +use crate::auth::CurrentUser; use crate::core::AppState; use crate::core::cursor::{CursorResults, clamp_page_size, decode_cursor}; use crate::core::errors::{AppError, AppResponse}; @@ -8,24 +8,24 @@ use crate::users::model::{ }; use crate::users::query_param::{RelationshipQueryParams, UserSearchParams}; use crate::users::user_service::UserService; +use axum::Json; use axum::extract::{Path, Query, State}; -use axum::{Extension, Json}; use std::sync::Arc; use uuid::Uuid; pub async fn handle_search_user_by_id( State(state): State>, - Path(user_id): Path, - Extension(token): Extension>, + Path(target_id): Path, + user: CurrentUser, ) -> AppResponse> { - let user_dto = UserService::query_user_by_id(state, &token.subject, &user_id).await?; + let user_dto = UserService::query_user_by_id(state, &user.subject, &target_id).await?; Ok(Json(user_dto)) } pub async fn handle_search_user_by_name( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, Query(params): Query, ) -> AppResponse>> { let cursor: UserPaginationCursor = decode_cursor(params.cursor) @@ -33,7 +33,7 @@ pub async fn handle_search_user_by_name( let page_size = clamp_page_size(params.limit); let search_results = - UserService::query_user_by_name(state, &token.subject, ¶ms.username, cursor, page_size) + UserService::query_user_by_name(state, &user.subject, ¶ms.username, cursor, page_size) .await?; Ok(Json(search_results)) @@ -41,7 +41,7 @@ pub async fn handle_search_user_by_name( pub async fn handle_get_open_friend_requests( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, Query(params): Query, ) -> AppResponse>> { let cursor: UserPaginationCursor = decode_cursor(params.cursor) @@ -50,7 +50,7 @@ pub async fn handle_get_open_friend_requests( let results = UserService::get_open_friend_requests( state, - &token.subject, + &user.subject, params.username, cursor, page_size, @@ -62,7 +62,7 @@ pub async fn handle_get_open_friend_requests( pub async fn handle_get_friends( State(state): State>, - Extension(token): Extension>, + user: CurrentUser, Query(params): Query, ) -> AppResponse>> { let cursor: UserPaginationCursor = decode_cursor(params.cursor) @@ -70,65 +70,64 @@ pub async fn handle_get_friends( let page_size = clamp_page_size(params.limit); let results = - UserService::get_friends(state, &token.subject, params.username, cursor, page_size).await?; + UserService::get_friends(state, &user.subject, params.username, cursor, page_size).await?; Ok(Json(results)) } pub async fn handle_add_friend( State(state): State>, - Path(user_id): Path, - Extension(token): Extension>, + Path(target_id): Path, + user: CurrentUser, ) -> AppResponse<()> { - if token.subject == user_id { + if user.subject == target_id { return Err(AppError::Validation( "Cannot friendship yourself.".to_string(), )); } - UserService::add_friend(state, token.subject, user_id).await?; + UserService::add_friend(state, user.subject, target_id).await?; Ok(()) } pub async fn handle_accept_friend_request( State(state): State>, Path(sender_id): Path, - Extension(token): Extension>, + user: CurrentUser, ) -> AppResponse<()> { - UserService::accept_friend_request(state, token.subject, sender_id).await?; + UserService::accept_friend_request(state, user.subject, sender_id).await?; Ok(()) } pub async fn handle_reject_friend_request( State(state): State>, Path(sender_id): Path, - Extension(token): Extension>, + user: CurrentUser, ) -> AppResponse<()> { - UserService::reject_friend_request(state, token.subject, sender_id).await?; + UserService::reject_friend_request(state, user.subject, sender_id).await?; Ok(()) } pub async fn handle_remove_friend( State(state): State>, Path(friend_id): Path, - Extension(token): Extension>, + user: CurrentUser, ) -> AppResponse<()> { - UserService::remove_friend(state, token.subject, friend_id).await?; + UserService::remove_friend(state, user.subject, friend_id).await?; Ok(()) } pub async fn handle_ignore_user( State(state): State>, - Path(user_id): Path, - Extension(token): Extension>, + Path(target_id): Path, + user: CurrentUser, ) -> AppResponse> { - if token.subject == user_id { + if user.subject == target_id { return Err(AppError::Validation("Cannot ignore yourself.".to_string())); } - let updated_state = - UserService::ignore_user(state.clone(), token.subject.clone(), user_id.clone()).await?; + let updated_state = UserService::ignore_user(state.clone(), user.subject, target_id).await?; let room = - RoomService::find_existing_single_room(state.clone(), &token.subject, &user_id).await?; + RoomService::find_existing_single_room(state.clone(), &user.subject, &target_id).await?; if let Some(room) = room { - RoomService::leave_room(state, token.subject, room).await?; + RoomService::leave_room(state, user.subject, room).await?; } let response = RelationshipStateResponse { state: Some(updated_state), @@ -138,10 +137,10 @@ pub async fn handle_ignore_user( pub async fn handle_undo_ignore_user( State(state): State>, - Path(user_id): Path, - Extension(token): Extension>, + Path(target_id): Path, + user: CurrentUser, ) -> AppResponse> { - let updated_state = UserService::undo_ignore(state, token.subject, user_id).await?; + let updated_state = UserService::undo_ignore(state, user.subject, target_id).await?; let response = RelationshipStateResponse { state: updated_state, }; diff --git a/tests/auth_attacks.rs b/tests/auth_attacks.rs new file mode 100644 index 0000000..6ae2e9e --- /dev/null +++ b/tests/auth_attacks.rs @@ -0,0 +1,1557 @@ +//! Black-box attack suite against a **running** ISM. +//! +//! Where `src/auth/security_tests.rs` verifies the validation rules in-process — signing tokens +//! with a test key and calling the decoder directly — this suite has no access to any of that. It +//! speaks HTTP to a live instance, exactly like an attacker would, and therefore covers what the +//! unit tests structurally cannot: the wiring. Which layer sits in front of which route, whether +//! the configured algorithm allow-list is the one actually in force, whether a rejected request +//! leaks *why* it was rejected, whether the token is read from anywhere but the `Authorization` +//! header, and whether anything in the request can steer the server towards a key set of the +//! caller's choosing. +//! +//! # Running it +//! +//! ```sh +//! docker compose up -d # Keycloak, PostgreSQL, Redis, MinIO, Redpanda +//! cargo run # ISM itself, in another shell +//! cargo test --test auth_attacks -- --nocapture +//! ``` +//! +//! `--nocapture` matters: several tests print what they observed (skips, timings, the exact +//! response an attack produced) and cargo swallows that for passing tests otherwise. +//! +//! The target is resolved the same way the server resolves its own listen address — through +//! `ISMConfig::new(ISM_MODE)`, so it follows `default.config.toml`, the mode file and the `ISM_*` +//! environment overrides. `ISM_ATTACK_BASE_URL` overrides it outright. +//! +//! If ISM or Keycloak is not reachable, every test **skips** rather than fails, so `cargo test` +//! stays green on a machine without the stack. Set `ISM_ATTACK_STRICT=1` to turn every skip into a +//! failure — do that in CI, otherwise a suite that silently tested nothing looks exactly like a +//! suite that passed. +//! +//! # What needs credentials +//! +//! Everything an attacker can do without an account runs unconditionally. Some attacks only mean +//! anything with a *genuine* Keycloak token — an ID token replayed as a bearer credential, a +//! refresh token in the `Authorization` header, a real token with its payload swapped — because +//! forging those without the realm's private key produces a token that dies at the signature check +//! for the wrong reason. Those tests skip unless you point them at a realm account: +//! +//! ```sh +//! ISM_ATTACK_CLIENT_ID= \ +//! ISM_ATTACK_USERNAME= ISM_ATTACK_PASSWORD= \ +//! cargo test --test auth_attacks -- --nocapture +//! # optional: ISM_ATTACK_CLIENT_SECRET for a confidential client +//! ``` +//! +//! A token minted by a *different* realm is the cross-issuer case; it needs a second account and is +//! configured with the same variables under `ISM_ATTACK_FOREIGN_*` (plus `..._FOREIGN_REALM`). +//! +//! Two tests are `#[ignore]`d because they disturb the environment or take real time: +//! +//! ```sh +//! ISM_ATTACK_ALLOW_DOCKER=1 cargo test --test auth_attacks -- --ignored --nocapture --test-threads=1 +//! ``` +//! +//! # Reading a failure +//! +//! Every attack asserts the same thing: **401, with the generic body**. A 200 is a breach. A 500 is +//! a parser or panic reached by unauthenticated input. A 503 means the auth layer could not talk to +//! Keycloak — usually the stack, not the code. A 401 whose body differs from every other 401 is an +//! oracle: it tells the sender which check tripped, which is how forged tokens get iterated into +//! working ones. + +#![allow(clippy::expect_used)] + +use std::collections::BTreeSet; +use std::net::SocketAddr; +use std::sync::Arc; +use std::sync::atomic::{AtomicUsize, Ordering}; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; + +use base64::Engine; +use base64::engine::general_purpose::{STANDARD, URL_SAFE_NO_PAD}; +use ism::core::ISMConfig; +use jsonwebtoken::{Algorithm, EncodingKey}; +use serde_json::{Value, json}; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::net::{TcpListener, TcpStream}; +use tokio::sync::OnceCell; + +/// The protected endpoint every attack is fired at. Any route behind the auth layer would do; this +/// one takes no path parameters and no body, so nothing but the token decides the outcome. +const PROTECTED_PATH: &str = "/api/v1/rooms"; + +/// The rejection every attack must produce, verbatim. See `AuthError::classify`. +const REJECTION_STATUS: u16 = 401; +const REJECTION_CODE: &str = "UNAUTHORIZED"; +const REJECTION_MESSAGE: &str = "Authentication required."; + +/// `azp` on forged tokens. Irrelevant while `expected_azp` is unset (the default), which is +/// precisely what `rejects_claim_manipulation` documents. +const FORGED_AZP: &str = "ism-attack-suite"; + +/// A UUID-shaped subject, so `sub` parsing is never what a forged token dies on — the point is to +/// reach the check under test. +const FORGED_SUBJECT: &str = "0193f3a0-1c2d-7e4f-8a9b-0c1d2e3f4a5b"; + +// ── Target discovery ──────────────────────────────────────────────────────── + +/// Everything the suite needs to know about the instance under attack. +struct Target { + /// e.g. `http://127.0.0.1:7800` + base_url: String, + /// `host:port` of `base_url`, for the raw-socket requests. + authority: String, + /// The issuer Keycloak advertises — not one rebuilt from config, which is what ISM pins against. + issuer: String, + token_endpoint: String, + /// First accepted audience from the config. Forged tokens carry it so `aud` is never the reason + /// they are rejected. + audience: String, + /// Keycloak's active RSA signing key: the `kid` a genuine token names, and the public modulus + /// and exponent behind it. Public information — which is the entire premise of the RS256→HS256 + /// confusion attack. + keycloak_kid: String, + keycloak_n: Vec, + keycloak_e: Vec, + realm: String, + http: reqwest::Client, +} + +static TARGET: OnceCell> = OnceCell::const_new(); + +async fn target() -> Option<&'static Target> { + TARGET.get_or_init(discover_target).await.as_ref() +} + +async fn discover_target() -> Option { + let mode = std::env::var("ISM_MODE").unwrap_or_else(|_| String::from("development")); + let config = match ISMConfig::new(&mode) { + Ok(config) => config, + Err(err) => { + println!("Could not load {mode}.config.toml from the package root: {err}"); + return None; + } + }; + + let base_url = std::env::var("ISM_ATTACK_BASE_URL").unwrap_or_else(|_| format!("http://{}:{}", config.ism_url, config.ism_port)); + let base_url = base_url.trim_end_matches('/').to_owned(); + let authority = base_url.split_once("://").map_or_else(|| base_url.clone(), |(_, rest)| rest.to_owned()); + + let http = reqwest::Client::builder() + .timeout(Duration::from_secs(15)) + // A system proxy that swallows loopback traffic would turn every attack into a false pass. + .no_proxy() + // One connection per request. Keep-alive reuse races the server's idle timeout, and a + // connection reset on that race is indistinguishable here from the server dropping a + // request — a flaky failure in a suite whose whole output is "was this refused?". + .pool_max_idle_per_host(0) + .build() + .expect("HTTP client builds"); + + // Is anything listening, and is it ISM? + match http.get(format!("{base_url}/health")).send().await { + Ok(response) if response.status().as_u16() == 200 => {} + Ok(response) => { + println!("{base_url}/health answered {} — not ISM?", response.status()); + return None; + } + Err(err) => { + println!("ISM is not reachable at {base_url}: {err}"); + return None; + } + } + + // The realm's own view of itself. ISM pins the issuer to what this document advertises, so + // rebuilding it from `iss_host`/`iss_realm` would produce forged tokens that fail for a reason + // we did not intend whenever a frontend URL is configured. + let issuer_host = config.token_issuer.iss_host.trim_end_matches('/').to_owned(); + let realm = config.token_issuer.iss_realm.clone(); + let discovery_url = format!("{issuer_host}/realms/{realm}/.well-known/openid-configuration"); + let discovery: Value = match http.get(&discovery_url).send().await { + Ok(response) => match response.json().await { + Ok(json) => json, + Err(err) => { + println!("Keycloak discovery document at {discovery_url} was unreadable: {err}"); + return None; + } + }, + Err(err) => { + println!("Keycloak is not reachable at {discovery_url}: {err}"); + return None; + } + }; + + let issuer = discovery["issuer"].as_str()?.to_owned(); + let jwks_uri = discovery["jwks_uri"].as_str()?.to_owned(); + let token_endpoint = discovery["token_endpoint"].as_str()?.to_owned(); + + let jwks: Value = http.get(&jwks_uri).send().await.ok()?.json().await.ok()?; + let (keycloak_kid, keycloak_n, keycloak_e) = active_signing_key(&jwks)?; + + Some(Target { + base_url, + authority, + issuer, + token_endpoint, + audience: config + .token_issuer + .expected_audiences + .first() + .cloned() + .unwrap_or_else(|| String::from("account")), + keycloak_kid, + keycloak_n, + keycloak_e, + realm, + http, + }) +} + +/// Picks the RSA key Keycloak signs access tokens with out of its JWK set. +fn active_signing_key(jwks: &Value) -> Option<(String, Vec, Vec)> { + for key in jwks["keys"].as_array()? { + if key["kty"].as_str() != Some("RSA") { + continue; + } + // Keycloak publishes its RSA-OAEP *encryption* key in the same document. + if matches!(key["use"].as_str(), Some(other) if other != "sig") { + continue; + } + if matches!(key["alg"].as_str(), Some(alg) if !alg.starts_with("RS")) { + continue; + } + let kid = key["kid"].as_str()?.to_owned(); + let n = URL_SAFE_NO_PAD.decode(key["n"].as_str()?).ok()?; + let e = URL_SAFE_NO_PAD.decode(key["e"].as_str()?).ok()?; + return Some((kid, n, e)); + } + None +} + +// ── Skipping ──────────────────────────────────────────────────────────────── + +/// Reports that a test could not run. Fails instead when `ISM_ATTACK_STRICT` is set: a suite that +/// skipped everything is indistinguishable from a suite that passed, which is not a state CI should +/// be able to reach quietly. +fn skip(reason: &str) { + assert!(std::env::var("ISM_ATTACK_STRICT").is_err(), "SKIPPED under ISM_ATTACK_STRICT: {reason}"); + println!("SKIPPED: {reason}"); +} + +/// Binds the live target or leaves the test, having said why. +macro_rules! target_or_skip { + () => { + match target().await { + Some(target) => target, + None => { + skip("no reachable ISM + Keycloak (see the message above)"); + return; + } + } + }; +} + +/// Binds a genuine Keycloak token set or leaves the test. +macro_rules! tokens_or_skip { + ($target:expr) => { + match genuine_tokens($target).await { + Some(tokens) => tokens, + None => { + skip( + "no realm credentials: set ISM_ATTACK_CLIENT_ID / ISM_ATTACK_USERNAME / \ + ISM_ATTACK_PASSWORD to run the attacks that need a genuine token", + ); + return; + } + } + }; +} + +// ── Talking to the target ─────────────────────────────────────────────────── + +/// One response, reduced to what an attacker learns from it. +#[derive(Debug, Clone, PartialEq, Eq)] +struct Outcome { + status: u16, + code: String, + message: String, + body: String, +} + +impl Outcome { + /// The triple a caller can distinguish responses by. Two different attacks producing two + /// different fingerprints is an oracle. + fn fingerprint(&self) -> (u16, String, String) { + (self.status, self.code.clone(), self.message.clone()) + } +} + +/// Sends `token` as a bearer credential to the protected route. +async fn call_protected(target: &Target, token: &str) -> Outcome { + call_with_headers(target, PROTECTED_PATH, &[("Authorization", token_header(token))]).await +} + +fn token_header(token: &str) -> String { + format!("Bearer {token}") +} + +async fn call_with_headers(target: &Target, path: &str, headers: &[(&str, String)]) -> Outcome { + let send = || async { + let mut request = target.http.get(format!("{}{path}", target.base_url)); + for (name, value) in headers { + request = request.header(*name, value); + } + request.send().await + }; + + // One retry, for the transport rather than the target: a refused or reset connection says + // nothing about how the token was judged, and letting it fail the assertion would report a + // breach that never happened. + let response = match send().await { + Ok(response) => response, + Err(first) => { + tokio::time::sleep(Duration::from_millis(100)).await; + send() + .await + .unwrap_or_else(|second| panic!("request to {path} could not be sent: {first} (retry: {second})")) + } + }; + + let status = response.status().as_u16(); + let body = response.text().await.unwrap_or_default(); + let parsed: Value = serde_json::from_str(&body).unwrap_or(Value::Null); + + Outcome { + status, + code: parsed["errorCode"].as_str().unwrap_or_default().to_owned(), + message: parsed["message"].as_str().unwrap_or_default().to_owned(), + body, + } +} + +/// The assertion every attack ends in. +/// +/// Checks three separate properties, because they fail independently: +/// 1. the request was refused — a 200 here is the breach the whole suite exists to find; +/// 2. it was refused with the *generic* body, so the rejection is not an oracle; +/// 3. nothing the caller put in the token came back out, so error responses cannot be used to +/// reflect content at a third party or into a log-scraping pipeline. +async fn assert_rejected(target: &Target, attack: &str, token: &str) -> Outcome { + let outcome = call_protected(target, token).await; + + assert_eq!( + outcome.status, REJECTION_STATUS, + "{attack}: expected {REJECTION_STATUS}, got {} — body: {}", + outcome.status, outcome.body + ); + assert_eq!( + outcome.code, REJECTION_CODE, + "{attack}: rejection carried errorCode {:?} instead of {REJECTION_CODE}, which tells the \ + sender which check tripped — body: {}", + outcome.code, outcome.body + ); + assert_eq!( + outcome.message, REJECTION_MESSAGE, + "{attack}: rejection message differed from the generic one — body: {}", + outcome.body + ); + // Length-gated: a short or empty "token" is a substring of any body by accident, and this + // check is about reflection, not coincidence. + assert!( + token.len() < ECHO_MIN_LEN || !outcome.body.contains(token), + "{attack}: the response echoed the token back — body: {}", + outcome.body + ); + + outcome +} + +/// Shortest attacker-controlled string worth checking for reflection. Below this, a match says more +/// about the alphabet than about the server. +const ECHO_MIN_LEN: usize = 4; + +/// Asserts a rejection *and* that no fragment of attacker-chosen text survived into the response. +async fn assert_rejected_without_echo(target: &Target, attack: &str, token: &str, needle: &str) -> Outcome { + let outcome = assert_rejected(target, attack, token).await; + assert!( + needle.len() < ECHO_MIN_LEN || !outcome.body.contains(needle), + "{attack}: the response echoed attacker-controlled input ({needle:?}) — body: {}", + outcome.body + ); + outcome +} + +/// Writes a handcrafted request onto a socket, for the cases a well-behaved HTTP client refuses to +/// produce: control characters inside a header, a header far past any sane size, two `Authorization` +/// headers on one request. +/// +/// Returns the status line's code, or `None` when the server answered by closing the connection — +/// itself an acceptable way to refuse. +async fn raw_request(target: &Target, raw: &str) -> Option { + let addr = &target.authority; + let mut stream = TcpStream::connect(addr) + .await + .unwrap_or_else(|err| panic!("could not connect to {addr}: {err}")); + stream + .write_all(raw.as_bytes()) + .await + .unwrap_or_else(|err| panic!("could not write to {addr}: {err}")); + + let mut buffer = vec![0_u8; 4096]; + let read = tokio::time::timeout(Duration::from_secs(10), stream.read(&mut buffer)).await; + let read = read.expect("the server answered or closed within 10s"); + let read = read.ok()?; + if read == 0 { + return None; + } + + let head = String::from_utf8_lossy(&buffer[..read]); + head.split_whitespace().nth(1)?.parse().ok() +} + +fn raw_get(target: &Target, authorization_lines: &str) -> String { + format!( + "GET {PROTECTED_PATH} HTTP/1.1\r\nHost: {}\r\n{authorization_lines}Connection: close\r\n\r\n", + target.authority + ) +} + +// ── Forging tokens ────────────────────────────────────────────────────────── + +/// A throwaway RSA-2048 key, standing in for "the attacker's own key". Not a secret and used +/// nowhere but here: the point of every token it signs is that ISM must not know it. +const ATTACKER_PRIVATE_KEY_PEM: &str = "-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQD7q21UQWXygUvE +Ws+NDvTevym687EsUSdOQUFJDL1MESpWFeeRl+z6nlcoOd0AkyAqa652FL/Pir0d +KK2lRtHa1PeNiENJMxjRG+ivfueh1TiRAi6Z5QyquyydDQ0e22K2X0CrfXxxke2h +N9a0YjdNWmUnPSy46QGwulHh2Muc+d0Wj4Trbf2RCPQ1gHKBJBONirmHO+T5KJyT +Q1a0M98wlHrh5rxRwVaKyREFma/5G5osNVnXdeCcVAnlNRQSg62p7PGrmqa+PShW +cVeczAGbIurdU8bOdTgPfuLNoN12jePSzn5bf+lpZ5bNHiK1Or8T2ULtoCOwdx9a +wNylSJ9zAgMBAAECggEAdT6WRufWukTRCu9xfNooavMs2js4YZiHErZk10bXk231 +xrgasyHPlawZl5RpaKCiHhEfbERbXbFZTBHM39Af6O5JS8bc7eefmp+BZezdtW+T +lD6rfieOoKVlcd8IK0VyddrnUl06EeC1j2Nno46UC/XeZQrjYFuw3WfXyLsKlJyI +ibyTHDCg3HQm3Y5+y1x2uLJ8vmOOU8q72hvfqM8eoyHMQeGOK2iBEDgmpPaiGtIE +Jpgju7/OxRmBenu0lQ5oszByl+4pVaUUpEAN5tFClQTdO30H0AOkiz26ClwrhR/K +wqBttRRUl+tyQ68lgKVuhsNdaQDB7jA0o61GAwNiQQKBgQD/Bayd3ZXtOw0JaVoT +WrdLG4wiAY/4bFfoQIWISHr6Y4NcTEsMbbvgnIj7UzJ4rXDV1ZYUZ/pUOCUMxURK +YNh8G8GT0WcWKsdRUjTFqf8dMFFRNw2fQZv80895T0EQLrmiUuZR99/A7YRBNxe1 +a+Fse5YzS8JrGYltiuTZxq7xwQKBgQD8onZFRYvapb1wggI0heeMT/4RfhBmuMjX +qphlNyOQvuSXz2KT8LodgLwhYD0F8F5LNOigf9Uk0sJ/VhrP1WvoQ10Csr2RUuo7 +GlmOFpxD+wEBcis2L3nantZRvjbVt6eEAAIQE++y7VvktG3X3xW9ZgRd2L47iQEh +x3Vmnd/2MwKBgQDadLXllYd07HzCbyjmI3OYN0TXbJczqzuyjHLWx5/xFYXVbtVr +FCU4x17gS+iUT560zn39hQR/WIkEY4eYX1WTGwO76ElyR7ruAomKOZF8I4PFGm/k +2IMTFS5JMIb/occLMhBybu+RiOUeKF963asBDu0fi+pDbGC5IZ3gn74FAQKBgGDW +YWFiLB6Og1P58aByZ3QoQWoxGVZWpF3OvYWmohJcqcDrNI0irCSc8QAWJK3/GhXX +3QeQmIH566XlundKBofMMn3TR8jJsJEhI4zMa+++6f7E5X1qq1m6ospIkDpRoHt/ +iUriaXH7e8rpwmUJ1Qp5bVkPuLOXa4CoNP81quBzAoGAIZSk1e/aSn6UGcBsEZpY +FYPIvuWckExme91xq9f5mgdL1bhJ7kzTtTVpP1/7du+82qhioD+BZyob+4sgJeAB +XyACnmWLM2aB3lUI5muQ5YSe7TMrtMBlvqNSFGQmk6/ioT4Bs9THGfUhrjqWvVav +c0iY5r5mMKIEUzAfRGdCyJE= +-----END PRIVATE KEY-----"; + +/// `kid` the attacker publishes their own key under. +const ATTACKER_KID: &str = "attacker-signing-key"; + +fn attacker_key() -> EncodingKey { + EncodingKey::from_rsa_pem(ATTACKER_PRIVATE_KEY_PEM.as_bytes()).expect("the attacker's private key parses") +} + +/// The attacker's *public* key, as `(n, e)`. Needed to publish it as a JWK the target could pick +/// up — without which the `jwk`/`jku` injections would fail for the trivial reason that the key in +/// the header does not match the signature, and would prove nothing. +fn attacker_public_components() -> (Vec, Vec) { + let key = attacker_key(); + (jsonwebtoken::crypto::rust_crypto::DEFAULT_PROVIDER + .key_utils + .rsa_pub_components_from_private_key)(key.as_bytes()) + .expect("the attacker's public components are derivable") +} + +fn attacker_jwk() -> Value { + let (n, e) = attacker_public_components(); + json!({ + "kty": "RSA", + "use": "sig", + "alg": "RS256", + "kid": ATTACKER_KID, + "n": URL_SAFE_NO_PAD.encode(n), + "e": URL_SAFE_NO_PAD.encode(e), + }) +} + +fn now() -> i64 { + SystemTime::now().duration_since(UNIX_EPOCH).expect("the clock is past 1970").as_secs() as i64 +} + +/// A claim set that is plausible in every respect — right issuer, right audience, live expiry, +/// UUID subject, `typ: Bearer` — so that whatever a given attack changes is the only thing wrong +/// with it. +fn claims(target: &Target, overrides: Value) -> Value { + let mut claims = json!({ + "exp": now() + 300, + "iat": now(), + "auth_time": now(), + "jti": "b7c1e5a2-3f4d-4e5a-9b8c-7d6e5f4a3b2c", + "iss": target.issuer, + "aud": target.audience, + "sub": FORGED_SUBJECT, + "typ": "Bearer", + "azp": FORGED_AZP, + "acr": "1", + "scope": "openid profile email", + "realm_access": { "roles": ["USER"] }, + "email_verified": true, + "preferred_username": "attacker", + "email": "attacker@example.test", + }); + + let target_object = claims.as_object_mut().expect("claims are an object"); + for (key, value) in overrides.as_object().expect("overrides are an object") { + match value.is_null() { + // A `null` override removes the claim, which is how the "missing claim" cases are said. + true => target_object.remove(key), + false => target_object.insert(key.clone(), value.clone()), + }; + } + claims +} + +/// A header naming Keycloak's *real* signing key. Attacks that want to reach the signature check +/// must use this: an unknown `kid` is rejected before any crypto runs, so a made-up one would +/// short-circuit the very check under test. +fn header_with_real_kid(target: &Target, alg: &str) -> Value { + json!({ "alg": alg, "typ": "JWT", "kid": target.keycloak_kid }) +} + +fn b64_json(value: &Value) -> String { + URL_SAFE_NO_PAD.encode(serde_json::to_vec(value).expect("value serializes")) +} + +/// Assembles a token and signs it for real, over exactly the bytes that were assembled. Header and +/// claims stay `Value`s rather than typed structs so any shape can be expressed — a numeric `alg`, +/// a missing one, an `exp` sent as a string. +fn signed(header: &Value, claims: &Value, key: &EncodingKey, alg: Algorithm) -> String { + let signing_input = format!("{}.{}", b64_json(header), b64_json(claims)); + let signature = jsonwebtoken::crypto::sign(signing_input.as_bytes(), key, alg).expect("the forged token signs"); + format!("{signing_input}.{signature}") +} + +/// Signed by the attacker's key: internally consistent, verifiable by anyone holding the attacker's +/// public key, and worthless against a server that only trusts the realm's. +fn forged(target: &Target, claims_overrides: Value) -> String { + signed( + &header_with_real_kid(target, "RS256"), + &claims(target, claims_overrides), + &attacker_key(), + Algorithm::RS256, + ) +} + +/// Header and payload, no signature at all. +fn unsigned(header: &Value, claims: &Value, signature: &str) -> String { + format!("{}.{}.{signature}", b64_json(header), b64_json(claims)) +} + +// ── DER / PEM, for the key-confusion secrets ──────────────────────────────── +// +// The RS256→HS256 forgery re-signs a token with HMAC, using the *public* key as the shared secret. +// Whether it works on a vulnerable server depends on which byte representation of that key the +// server would hand to the HMAC — so the attack is run against every representation that is +// plausibly in play, rather than one guess. + +fn der_length(len: usize) -> Vec { + if len < 0x80 { + return vec![len as u8]; + } + let bytes = len.to_be_bytes(); + let significant: Vec = bytes.iter().copied().skip_while(|b| *b == 0).collect(); + let mut out = vec![0x80 | significant.len() as u8]; + out.extend(significant); + out +} + +fn der_tlv(tag: u8, contents: &[u8]) -> Vec { + let mut out = vec![tag]; + out.extend(der_length(contents.len())); + out.extend(contents); + out +} + +/// DER INTEGER from a big-endian magnitude: leading zeros dropped, one re-added when the high bit +/// would otherwise make the value negative. +fn der_integer(magnitude: &[u8]) -> Vec { + let trimmed: &[u8] = match magnitude.iter().position(|b| *b != 0) { + Some(first) => &magnitude[first..], + None => &[0], + }; + let mut contents = Vec::with_capacity(trimmed.len() + 1); + if trimmed[0] & 0x80 != 0 { + contents.push(0x00); + } + contents.extend(trimmed); + der_tlv(0x02, &contents) +} + +/// PKCS#1 `RSAPublicKey ::= SEQUENCE { modulus INTEGER, publicExponent INTEGER }`. +fn rsa_pkcs1_der(n: &[u8], e: &[u8]) -> Vec { + let mut contents = der_integer(n); + contents.extend(der_integer(e)); + der_tlv(0x30, &contents) +} + +/// X.509 `SubjectPublicKeyInfo` wrapping the PKCS#1 key — the `BEGIN PUBLIC KEY` form, and the one +/// a JWKS-driven verifier most likely reconstructs internally. +fn rsa_spki_der(n: &[u8], e: &[u8]) -> Vec { + // AlgorithmIdentifier { OID 1.2.840.113549.1.1.1 (rsaEncryption), NULL } + const RSA_ENCRYPTION_OID: [u8; 11] = [0x06, 0x09, 0x2A, 0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x01]; + let mut algorithm = RSA_ENCRYPTION_OID.to_vec(); + algorithm.extend([0x05, 0x00]); + + let mut key_bits = vec![0x00]; // unused-bits count + key_bits.extend(rsa_pkcs1_der(n, e)); + + let mut contents = der_tlv(0x30, &algorithm); + contents.extend(der_tlv(0x03, &key_bits)); + der_tlv(0x30, &contents) +} + +fn pem(label: &str, der: &[u8]) -> String { + let encoded = STANDARD.encode(der); + let body = encoded + .as_bytes() + .chunks(64) + .map(|line| String::from_utf8_lossy(line).into_owned()) + .collect::>() + .join("\n"); + format!("-----BEGIN {label}-----\n{body}\n-----END {label}-----\n") +} + +// ── The JWKS honeypot ─────────────────────────────────────────────────────── + +/// A local HTTP server that serves a JWK set containing the *attacker's* key and counts every +/// connection it receives. +/// +/// It is pointed at by the `jku` and `x5u` headers of the forged tokens below. A single hit means +/// the server let a token nominate where its verification keys come from, which is total +/// compromise — and it would be invisible from the status code alone, because the forged tokens +/// would then verify and return 200. The counter catches even a fetch that happens to fail. +struct Honeypot { + url: String, + hits: Arc, +} + +impl Honeypot { + async fn start() -> Self { + let listener = TcpListener::bind::(([127, 0, 0, 1], 0).into()) + .await + .expect("the honeypot binds a loopback port"); + let addr = listener.local_addr().expect("the honeypot has an address"); + let hits = Arc::new(AtomicUsize::new(0)); + + let body = json!({ "keys": [attacker_jwk()] }).to_string(); + let counter = Arc::clone(&hits); + tokio::spawn(async move { + while let Ok((mut stream, _)) = listener.accept().await { + counter.fetch_add(1, Ordering::SeqCst); + let body = body.clone(); + tokio::spawn(async move { + let mut scratch = vec![0_u8; 2048]; + let _ = stream.read(&mut scratch).await; + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}", + body.len() + ); + let _ = stream.write_all(response.as_bytes()).await; + }); + } + }); + + Self { + url: format!("http://{addr}/certs"), + hits, + } + } + + fn hits(&self) -> usize { + self.hits.load(Ordering::SeqCst) + } +} + +// ── Genuine tokens ────────────────────────────────────────────────────────── + +struct Tokens { + access: String, + id: Option, + refresh: Option, +} + +struct Credentials { + realm: Option, + client_id: String, + client_secret: Option, + username: String, + password: String, +} + +fn credentials_from_env(prefix: &str) -> Option { + Some(Credentials { + realm: std::env::var(format!("{prefix}REALM")).ok(), + client_id: std::env::var(format!("{prefix}CLIENT_ID")).ok()?, + client_secret: std::env::var(format!("{prefix}CLIENT_SECRET")).ok(), + username: std::env::var(format!("{prefix}USERNAME")).ok()?, + password: std::env::var(format!("{prefix}PASSWORD")).ok()?, + }) +} + +/// Direct access grant against Keycloak. `scope=openid` is what makes it hand out an ID token as +/// well, which is one of the credentials that must *not* be accepted as a bearer token. +async fn login(target: &Target, credentials: &Credentials) -> Option { + let endpoint = match &credentials.realm { + Some(realm) if *realm != target.realm => target + .token_endpoint + .replace(&format!("/realms/{}/", target.realm), &format!("/realms/{realm}/")), + _ => target.token_endpoint.clone(), + }; + + let mut form = vec![ + ("grant_type", "password"), + ("client_id", credentials.client_id.as_str()), + ("username", credentials.username.as_str()), + ("password", credentials.password.as_str()), + ("scope", "openid"), + ]; + if let Some(secret) = &credentials.client_secret { + form.push(("client_secret", secret.as_str())); + } + + let response = target.http.post(&endpoint).form(&form).send().await.ok()?; + let status = response.status(); + let payload: Value = response.json().await.ok()?; + if !status.is_success() { + println!("Keycloak refused the direct access grant at {endpoint}: {status} {payload}"); + return None; + } + + Some(Tokens { + access: payload["access_token"].as_str()?.to_owned(), + id: payload["id_token"].as_str().map(str::to_owned), + refresh: payload["refresh_token"].as_str().map(str::to_owned), + }) +} + +static GENUINE: OnceCell> = OnceCell::const_new(); + +async fn genuine_tokens(target: &'static Target) -> Option<&'static Tokens> { + GENUINE + .get_or_init(|| async { + let credentials = credentials_from_env("ISM_ATTACK_")?; + login(target, &credentials).await + }) + .await + .as_ref() +} + +// ── Control: is this actually a protected ISM? ────────────────────────────── + +#[tokio::test] +async fn the_target_is_a_live_ism_behind_the_auth_layer() { + // Every rejection assertion below is worthless if the route was never protected, or if the + // thing answering is not ISM. This is what makes the rest of the suite mean something. + let target = target_or_skip!(); + + let health = call_with_headers(target, "/health", &[]).await; + assert_eq!(health.status, 200, "/health must be public"); + + let unauthenticated = call_with_headers(target, PROTECTED_PATH, &[]).await; + assert_eq!( + unauthenticated.status, REJECTION_STATUS, + "{PROTECTED_PATH} answered {} without a token — it is not behind the auth layer", + unauthenticated.status + ); + assert_eq!(unauthenticated.code, REJECTION_CODE); + + println!("Target: {} | issuer: {} | signing kid: {}", target.base_url, target.issuer, target.keycloak_kid); +} + +#[tokio::test] +async fn rejects_credentials_that_are_not_bearer_tokens() { + let target = target_or_skip!(); + + for (name, header) in [ + ("basic auth", String::from("Basic YWRtaW46YWRtaW4=")), + ("bare token, no scheme", forged(target, json!({}))), + ( + // The extractor matches `Bearer ` exactly. Documented here so a future relaxation to + // case-insensitive matching is a deliberate, visible change. + "lowercase scheme", + format!("bearer {}", forged(target, json!({}))), + ), + ("scheme only", String::from("Bearer")), + ("empty value", String::new()), + ("negotiate", String::from("Negotiate YII=")), + ] { + let outcome = call_with_headers(target, PROTECTED_PATH, &[("Authorization", header)]).await; + assert_eq!( + outcome.status, REJECTION_STATUS, + "{name}: expected {REJECTION_STATUS}, got {} — body: {}", + outcome.status, outcome.body + ); + } +} + +// ── Algorithm confusion ───────────────────────────────────────────────────── + +#[tokio::test] +async fn rejects_unsigned_tokens() { + // The unsigned-token attack: claim there is no algorithm and append an empty signature. Every + // spelling, because a case-insensitive comparison somewhere is exactly how this gets through. + let target = target_or_skip!(); + + for alg in ["none", "None", "NONE", "nOnE", "nonE", ""] { + let header = json!({ "alg": alg, "typ": "JWT", "kid": target.keycloak_kid }); + let payload = claims(target, json!({})); + + for (variant, signature) in [ + ("empty signature", ""), + ("garbage signature", "aGVsbG8"), + // A signature copied off a genuine-looking token, in case emptiness is what is checked. + ("borrowed signature", "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"), + ] { + let token = unsigned(&header, &payload, signature); + assert_rejected(target, &format!("alg={alg:?} with a {variant}"), &token).await; + } + } + + // ...and the same with no `alg` at all, or one that is not even a string. + for header in [ + json!({ "typ": "JWT", "kid": target.keycloak_kid }), + json!({ "alg": null, "typ": "JWT", "kid": target.keycloak_kid }), + json!({ "alg": 0, "typ": "JWT", "kid": target.keycloak_kid }), + json!({ "alg": ["none", "RS256"], "typ": "JWT", "kid": target.keycloak_kid }), + json!({ "alg": "RS256 ", "typ": "JWT", "kid": target.keycloak_kid }), + json!({ "alg": "rs256", "typ": "JWT", "kid": target.keycloak_kid }), + ] { + let token = unsigned(&header, &claims(target, json!({})), ""); + assert_rejected(target, &format!("malformed alg: {header}"), &token).await; + } +} + +#[tokio::test] +async fn rejects_rs256_to_hs256_key_confusion() { + // The classic. Keycloak's signing key is public — it is published at the JWKS endpoint — so if + // the verifier takes its algorithm from the token header, anyone can re-sign a token of their + // choosing with HMAC, keyed by that public key, and be believed. + // + // The token names the real `kid`, so the server looks up the real RSA key; the only thing + // deciding the outcome is whether the header's `alg` is allowed to select the algorithm. + let target = target_or_skip!(); + + let (n, e) = (target.keycloak_n.as_slice(), target.keycloak_e.as_slice()); + let spki_der = rsa_spki_der(n, e); + let pkcs1_der = rsa_pkcs1_der(n, e); + + // Every representation of that public key a vulnerable implementation might use as the secret. + let secrets: Vec<(&str, Vec)> = vec![ + ("SPKI PEM", pem("PUBLIC KEY", &spki_der).into_bytes()), + ("PKCS#1 PEM", pem("RSA PUBLIC KEY", &pkcs1_der).into_bytes()), + ("SPKI DER", spki_der.clone()), + ("PKCS#1 DER", pkcs1_der.clone()), + ("raw modulus", n.to_vec()), + ("base64url modulus", URL_SAFE_NO_PAD.encode(n).into_bytes()), + ( + "JWK JSON", + json!({ "kty": "RSA", "n": URL_SAFE_NO_PAD.encode(n), "e": URL_SAFE_NO_PAD.encode(e) }) + .to_string() + .into_bytes(), + ), + ]; + + for (algorithm_name, algorithm) in [("HS256", Algorithm::HS256), ("HS384", Algorithm::HS384), ("HS512", Algorithm::HS512)] { + for (description, secret) in &secrets { + let token = signed( + &header_with_real_kid(target, algorithm_name), + &claims(target, json!({})), + &EncodingKey::from_secret(secret), + algorithm, + ); + assert_rejected(target, &format!("{algorithm_name} signed with the public key as {description}"), &token).await; + } + } + + // The same confusion one family over: an EC or EdDSA header against an RSA key set. + for alg in ["ES256", "ES384", "PS256", "EdDSA", "RS384", "RS512"] { + let token = signed( + &header_with_real_kid(target, alg), + &claims(target, json!({})), + &attacker_key(), + Algorithm::RS256, + ); + assert_rejected(target, &format!("{alg} header over an RS256 signature"), &token).await; + } +} + +// ── Header injection ──────────────────────────────────────────────────────── + +#[tokio::test] +async fn rejects_kid_header_injection() { + // `kid` is read before a single signature is verified, so its content is entirely + // attacker-chosen. Anything that resolves it — a file path, a cache key, a database lookup, a + // log field — inherits that. Each payload here targets one such sink; all of them must produce + // the same flat rejection, and none of them may come back out in the response. + let target = target_or_skip!(); + + let payloads = [ + "../../../../dev/null", + "../../../../../../etc/passwd", + "..\\..\\..\\..\\windows\\win.ini", + "/dev/null", + "file:///dev/null", + "' OR '1'='1", + "'; DROP TABLE app_user;--", + "\" OR \"\"=\"", + "1 UNION SELECT null,null--", + "${jndi:ldap://127.0.0.1:1389/a}", + "{{7*7}}", + "%00", + "\u{0}truncated", + "real-key\nlevel=ERROR msg=\"forged log line\"", + "real-key\r\nSet-Cookie: session=stolen", + "", + "", + "null", + "undefined", + ]; + + for payload in payloads { + let header = json!({ "alg": "RS256", "typ": "JWT", "kid": payload }); + let token = signed(&header, &claims(target, json!({})), &attacker_key(), Algorithm::RS256); + assert_rejected_without_echo(target, &format!("kid = {payload:?}"), &token, payload).await; + } + + // An unbounded `kid` is a log-volume amplifier: one request, as much output as the sender likes. + for length in [1024_usize, 8 * 1024, 64 * 1024] { + let payload = "A".repeat(length); + let header = json!({ "alg": "RS256", "typ": "JWT", "kid": payload }); + let token = signed(&header, &claims(target, json!({})), &attacker_key(), Algorithm::RS256); + assert_rejected(target, &format!("{length} byte kid"), &token).await; + } +} + +#[tokio::test] +async fn never_fetches_verification_keys_named_by_the_token() { + // `jku`, `x5u` and `jwk` let a token say where its own verification key lives. Honouring any of + // them is unconditional compromise, and it is also an SSRF primitive: the server would fetch a + // URL of the caller's choosing from inside its own network. + // + // A status code alone cannot prove this — a server might fetch the URL and still reject the + // token. So the URLs point at a local listener that counts connections, and the assertion is + // that it was never touched. + let target = target_or_skip!(); + let honeypot = Honeypot::start().await; + + let attacker_headers = [ + json!({ "alg": "RS256", "typ": "JWT", "kid": ATTACKER_KID, "jku": honeypot.url }), + json!({ "alg": "RS256", "typ": "JWT", "kid": ATTACKER_KID, "x5u": honeypot.url }), + json!({ "alg": "RS256", "typ": "JWT", "kid": ATTACKER_KID, "jwk": attacker_jwk() }), + // The `jwk` smuggled in under the real `kid`, in case the key is looked up by `kid` and the + // embedded key is then trusted for it. + json!({ "alg": "RS256", "typ": "JWT", "kid": target.keycloak_kid, "jwk": attacker_jwk() }), + json!({ + "alg": "RS256", + "typ": "JWT", + "kid": ATTACKER_KID, + "jku": honeypot.url, + "x5u": honeypot.url, + "jwk": attacker_jwk(), + }), + // Relative and scheme-relative forms, which sometimes slip past an allow-list built on + // string prefixes. + json!({ "alg": "RS256", "typ": "JWT", "kid": ATTACKER_KID, "jku": format!("{}/../../{}", target.issuer, honeypot.url) }), + ]; + + for header in attacker_headers { + let token = signed(&header, &claims(target, json!({})), &attacker_key(), Algorithm::RS256); + assert_rejected(target, &format!("key-source injection: {header}"), &token).await; + } + + // Give an in-flight fetch a moment to land before reading the counter. + tokio::time::sleep(Duration::from_millis(500)).await; + assert_eq!( + honeypot.hits(), + 0, + "the server fetched a key URL named by the token ({} hits on {}) — a token must never be \ + able to nominate its own verification keys", + honeypot.hits(), + honeypot.url + ); +} + +// ── Claim manipulation ────────────────────────────────────────────────────── + +#[tokio::test] +async fn rejects_claim_manipulation() { + // Read this one for what it is: without the realm's private key these tokens all carry an + // invalid signature, so a rejection does not prove the *claim* rule fired — only that nothing + // about the claim shape (a string `exp`, an absent `aud`, an array where a string belongs) + // makes the pipeline fall over or take a shortcut before the signature is checked. + // + // The claim rules themselves are proven in `src/auth/security_tests.rs`, where the token is + // signed by a key the verifier trusts and the claim is genuinely the only thing wrong. + let target = target_or_skip!(); + + let cases = [ + ("expired an hour ago", json!({ "exp": now() - 3600 })), + ("expired just now", json!({ "exp": now() - 30 })), + ("no exp at all", json!({ "exp": null })), + ("exp = 0", json!({ "exp": 0 })), + ("negative exp", json!({ "exp": -1 })), + ("exp as a string", json!({ "exp": "9999999999" })), + ("exp beyond i64", json!({ "exp": 9_223_372_036_854_775_807_i64 })), + ("exp as a float", json!({ "exp": 1.0e18 })), + ("nbf in the future", json!({ "nbf": now() + 3600 })), + ("iat in the future", json!({ "iat": now() + 3600 })), + ("no issuer", json!({ "iss": null })), + ( + "foreign realm", + json!({ "iss": format!("{}/realms/master", target.issuer.rsplit_once("/realms/").map(|(host, _)| host.to_owned()).unwrap_or_default()) }), + ), + ("attacker issuer", json!({ "iss": "https://attacker.example/realms/meventure" })), + ("issuer with a trailing slash", json!({ "iss": format!("{}/", target.issuer) })), + ("issuer as an array", json!({ "iss": [target.issuer.clone()] })), + ("no audience", json!({ "aud": null })), + ("foreign audience", json!({ "aud": "some-other-service" })), + ("empty audience array", json!({ "aud": [] })), + ( + "audience smuggled into an array", + json!({ "aud": ["some-other-service", target.audience.clone()] }), + ), + ("another realm client", json!({ "azp": "account-console" })), + ("no azp", json!({ "azp": null })), + ("no subject", json!({ "sub": null })), + ("path traversal subject", json!({ "sub": "../../admin" })), + ("non-uuid subject", json!({ "sub": "admin" })), + ("sql injection subject", json!({ "sub": "1' OR '1'='1" })), + ("subject as a number", json!({ "sub": 1 })), + ("id token replayed", json!({ "typ": "ID" })), + ("refresh token replayed", json!({ "typ": "Refresh" })), + ("offline token replayed", json!({ "typ": "Offline" })), + ("no token type", json!({ "typ": null })), + ("self-granted realm admin", json!({ "realm_access": { "roles": ["ADMIN", "USER"] } })), + ( + "admin borrowed from another client", + json!({ "resource_access": { "some-other-service": { "roles": ["ADMIN"] } } }), + ), + ("roles as a string", json!({ "realm_access": { "roles": "ADMIN" } })), + ]; + + for (name, overrides) in cases { + let token = forged(target, overrides); + assert_rejected(target, name, &token).await; + } +} + +// ── Structural attacks ────────────────────────────────────────────────────── + +#[tokio::test] +async fn rejects_structurally_broken_tokens() { + let target = target_or_skip!(); + + let genuine_shape = forged(target, json!({})); + let mut parts = genuine_shape.split('.'); + let header = parts.next().expect("header").to_owned(); + let payload = parts.next().expect("payload").to_owned(); + let signature = parts.next().expect("signature").to_owned(); + + let claims_json = serde_json::to_vec(&claims(target, json!({}))).expect("claims serialize"); + + // A payload nested deeply enough to blow a recursive JSON parser's stack. It never reaches the + // parser on a correct server — the signature check comes first — but a 500 here would say it did. + let deep_json = format!("{}1{}", "[".repeat(20_000), "]".repeat(20_000)); + + let tokens = [ + ("empty", String::new()), + ("a single dot", String::from(".")), + ("two dots", String::from("..")), + ("four dots", String::from("....")), + ("not a jwt", String::from("not-a-jwt")), + ("two segments", format!("{header}.{payload}")), + ("signature stripped", format!("{header}.{payload}.")), + ("trailing dot removed", format!("{header}.{payload}")), + ("four segments", format!("{genuine_shape}.{signature}")), + ("signature swapped for the header", format!("{header}.{payload}.{header}")), + ("payload and signature swapped", format!("{header}.{signature}.{payload}")), + ("standard base64 padding", format!("{header}=.{payload}=.{signature}=")), + ("non-base64 characters", format!("{header}!.{payload}?.{signature}#")), + ("whitespace inside", format!("{header} . {payload} . {signature}")), + ("header only", header.clone()), + ( + "payload is a json array", + format!("{header}.{}.{signature}", URL_SAFE_NO_PAD.encode(b"[1,2,3]")), + ), + ( + "payload is a json string", + format!("{header}.{}.{signature}", URL_SAFE_NO_PAD.encode(b"\"admin\"")), + ), + ("payload is not json", format!("{header}.{}.{signature}", URL_SAFE_NO_PAD.encode(b"admin"))), + ( + "payload nested 20k deep", + format!("{header}.{}.{signature}", URL_SAFE_NO_PAD.encode(deep_json.as_bytes())), + ), + ("header is not json", format!("{}.{payload}.{signature}", URL_SAFE_NO_PAD.encode(b"not-json"))), + ("header is an empty object", format!("{}.{payload}.{signature}", URL_SAFE_NO_PAD.encode(b"{}"))), + ( + "claims re-encoded with a valid signature over the old ones", + format!("{header}.{}.{signature}", URL_SAFE_NO_PAD.encode(&claims_json)), + ), + ]; + + for (name, token) in tokens { + assert_rejected(target, name, &token).await; + } +} + +#[tokio::test] +async fn oversized_tokens_are_refused_without_stalling() { + // Unbounded input in the one header that is parsed before authentication. The server may answer + // 401, 400, 431 or simply close the connection — all fine. What it must not do is accept it, + // fall over with a 5xx, or hold the connection open while it thinks about it. + let target = target_or_skip!(); + + for size in [8 * 1024_usize, 64 * 1024, 512 * 1024, 1024 * 1024] { + let padding = "A".repeat(size); + let token = signed( + &header_with_real_kid(target, "RS256"), + &claims(target, json!({ "padding": padding })), + &attacker_key(), + Algorithm::RS256, + ); + + let started = Instant::now(); + let status = raw_request(target, &raw_get(target, &format!("Authorization: Bearer {token}\r\n"))).await; + let elapsed = started.elapsed(); + + println!("{size} byte token → {status:?} in {elapsed:?}"); + if let Some(status) = status { + assert_ne!(status, 200, "a {size} byte token was accepted"); + assert!( + status < 500, + "a {size} byte token produced {status} — unauthenticated input must not reach a \ + server error" + ); + } + assert!(elapsed < Duration::from_secs(10), "a {size} byte token held the connection for {elapsed:?}"); + } +} + +#[tokio::test] +async fn rejects_malformed_authorization_headers_at_the_wire_level() { + // Shapes no HTTP client will produce for you, and which therefore never get exercised by + // anything but a handcrafted socket. + let target = target_or_skip!(); + let token = forged(target, json!({})); + + let cases = [ + ( + "two Authorization headers, forged first", + format!("Authorization: Bearer {token}\r\nAuthorization: Bearer {token}\r\n"), + ), + ( + "Authorization with a folded continuation line", + format!("Authorization: Bearer\r\n\t{token}\r\n"), + ), + ("Bearer with a tab separator", format!("Authorization:\tBearer\t{token}\r\n")), + ("two spaces after the scheme", format!("Authorization: Bearer {token}\r\n")), + ("trailing whitespace", format!("Authorization: Bearer {token} \r\n")), + ("duplicated scheme", format!("Authorization: Bearer Bearer {token}\r\n")), + ("header name in mixed case", format!("AUTHORIZATION: Bearer {token}\r\n")), + ("non-ascii in the value", String::from("Authorization: Bearer ünïcödé.tökén.hërë\r\n")), + ]; + + for (name, header_lines) in cases { + let status = raw_request(target, &raw_get(target, &header_lines)).await; + println!("{name} → {status:?}"); + match status { + // Closing the connection is a legitimate refusal. + None => {} + Some(status) => { + assert_ne!(status, 200, "{name} was accepted"); + assert!(status < 500, "{name} produced {status}"); + } + } + } +} + +// ── JWKS handling ─────────────────────────────────────────────────────────── + +#[tokio::test] +async fn unknown_signing_keys_do_not_become_a_request_storm() { + // An unknown `kid` is the one failure a key rotation produces, so it is the one failure worth + // re-running OIDC discovery for — which makes it a caller-controlled trigger. Fifty fabricated + // `kid`s must not turn into fifty discoveries, must not exhaust anything, and must not start + // producing 503s: a service that answers "auth unavailable" under a trivial flood is a + // denial-of-service away from being down for everyone. + let target = target_or_skip!(); + + let started = Instant::now(); + let mut slowest = Duration::ZERO; + + for index in 0..50 { + let kid = format!("rotated-{}-{index}", SystemTime::now().duration_since(UNIX_EPOCH).expect("clock").as_nanos()); + let header = json!({ "alg": "RS256", "typ": "JWT", "kid": kid }); + let token = signed(&header, &claims(target, json!({})), &attacker_key(), Algorithm::RS256); + + let request_started = Instant::now(); + let outcome = call_protected(target, &token).await; + slowest = slowest.max(request_started.elapsed()); + + assert_eq!( + outcome.status, REJECTION_STATUS, + "unknown kid #{index} produced {} — body: {}", + outcome.status, outcome.body + ); + } + + println!("50 unknown-kid requests in {:?}, slowest single request {:?}", started.elapsed(), slowest); + + // The service is still healthy afterwards. + let health = call_with_headers(target, "/health", &[]).await; + assert_eq!(health.status, 200, "the flood left /health unhealthy"); +} + +#[tokio::test] +async fn rejections_are_indistinguishable_from_one_another() { + // The reason a token was refused is information: "wrong audience" narrows the search space, + // "bad signature" tells you the claims were fine. Every class of failure must therefore look + // identical from outside — same status, same code, same message. + // + // Expiry is deliberately not in this set: `TOKEN_EXPIRED` is distinguishable on purpose, so a + // client knows to refresh rather than to re-authenticate. + let target = target_or_skip!(); + + let mut fingerprints: BTreeSet<(u16, String, String)> = BTreeSet::new(); + let mut observed: Vec<(&str, Outcome)> = Vec::new(); + + let attacks: Vec<(&str, String)> = vec![ + ("garbage", String::from("not-a-jwt")), + ("alg none", unsigned(&header_with_real_kid(target, "none"), &claims(target, json!({})), "")), + ("bad signature", forged(target, json!({}))), + ( + "unknown kid", + signed( + &json!({ "alg": "RS256", "typ": "JWT", "kid": "no-such-key" }), + &claims(target, json!({})), + &attacker_key(), + Algorithm::RS256, + ), + ), + ("foreign issuer", forged(target, json!({ "iss": "https://attacker.example/realms/x" }))), + ("foreign audience", forged(target, json!({ "aud": "some-other-service" }))), + ("id token", forged(target, json!({ "typ": "ID" }))), + ("no subject", forged(target, json!({ "sub": null }))), + ]; + + for (name, token) in &attacks { + let outcome = call_protected(target, token).await; + fingerprints.insert(outcome.fingerprint()); + observed.push((name, outcome)); + } + + // ...and the no-credentials case, which must look the same as a bad credential. + let missing = call_with_headers(target, PROTECTED_PATH, &[]).await; + fingerprints.insert(missing.fingerprint()); + observed.push(("no authorization header", missing)); + + assert_eq!( + fingerprints.len(), + 1, + "rejections are distinguishable, which turns the endpoint into an oracle for crafting \ + tokens: {observed:#?}" + ); +} + +// ── Attacks that need a genuine token ─────────────────────────────────────── + +#[tokio::test] +async fn a_genuine_access_token_is_accepted() { + // The control for every test below it: without this, "rejected" proves nothing, because a + // server that rejects everything passes the entire suite. + let target = target_or_skip!(); + let tokens = tokens_or_skip!(target); + + let outcome = call_protected(target, &tokens.access).await; + assert!( + outcome.status != 401 && outcome.status != 403, + "a genuine Keycloak access token was refused with {} — body: {}", + outcome.status, + outcome.body + ); + println!("genuine access token → {} (auth passed)", outcome.status); +} + +#[tokio::test] +async fn rejects_other_genuine_keycloak_tokens_as_bearer_credentials() { + // ID and refresh tokens come from the same realm and, in the ID token's case, from the same + // signing key — every signature and claim check passes. Only `typ` separates a bearer + // credential from something that was never meant to be one. + let target = target_or_skip!(); + let tokens = tokens_or_skip!(target); + + match &tokens.id { + Some(id_token) => { + assert_rejected(target, "id token as a bearer credential", id_token).await; + } + None => println!("no id_token in the grant response (add `openid` to the client's scopes)"), + } + + match &tokens.refresh { + Some(refresh_token) => { + assert_rejected(target, "refresh token as a bearer credential", refresh_token).await; + } + None => println!("no refresh_token in the grant response"), + } +} + +#[tokio::test] +async fn rejects_a_tampered_genuine_token() { + // Take a token the server *does* accept and change one thing about it. This is the only way to + // prove the signature is verified at all rather than merely parsed. + let target = target_or_skip!(); + let tokens = tokens_or_skip!(target); + + let mut parts = tokens.access.split('.'); + let header = parts.next().expect("header").to_owned(); + let payload = parts.next().expect("payload").to_owned(); + let signature = parts.next().expect("signature").to_owned(); + + let decoded = URL_SAFE_NO_PAD.decode(&payload).expect("the genuine payload is base64url"); + let mut genuine_claims: Value = serde_json::from_slice(&decoded).expect("the genuine payload is json"); + + // Become someone else, keeping the genuine signature. + let claims_object = genuine_claims.as_object_mut().expect("claims are an object"); + claims_object.insert(String::from("sub"), json!("ffffffff-ffff-4fff-8fff-ffffffffffff")); + let elevated = URL_SAFE_NO_PAD.encode(genuine_claims.to_string()); + + assert_rejected(target, "genuine token, subject swapped", &format!("{header}.{elevated}.{signature}")).await; + + // Grant yourself a realm role. + let mut escalated_claims: Value = serde_json::from_slice(&decoded).expect("the genuine payload is json"); + escalated_claims + .as_object_mut() + .expect("claims are an object") + .insert(String::from("realm_access"), json!({ "roles": ["ADMIN"] })); + assert_rejected( + target, + "genuine token, realm roles rewritten", + &format!("{header}.{}.{signature}", URL_SAFE_NO_PAD.encode(escalated_claims.to_string())), + ) + .await; + + // Same claims, no signature. + assert_rejected(target, "genuine token with the signature stripped", &format!("{header}.{payload}.")).await; + + // Same claims, re-declared as unsigned. + let mut genuine_header: Value = + serde_json::from_slice(&URL_SAFE_NO_PAD.decode(&header).expect("the genuine header is base64url")).expect("the genuine header is json"); + genuine_header + .as_object_mut() + .expect("the header is an object") + .insert(String::from("alg"), json!("none")); + assert_rejected( + target, + "genuine claims re-declared as alg=none", + &format!("{}.{payload}.", b64_json(&genuine_header)), + ) + .await; + + // Flipped bits inside the signature itself. + let mut broken = signature.clone().into_bytes(); + if let Some(last) = broken.last_mut() { + *last = if *last == b'A' { b'B' } else { b'A' }; + } + assert_rejected( + target, + "genuine token with one signature byte changed", + &format!("{header}.{payload}.{}", String::from_utf8(broken).expect("still ascii")), + ) + .await; +} + +#[tokio::test] +async fn does_not_read_the_token_from_the_query_string() { + // ISM wires up the header extractor only. A token in a URL leaks into access logs, proxy logs, + // browser history and `Referer` headers, so if the WebSocket route ever needs the query + // extractor, this test is what will notice it being switched on globally. + let target = target_or_skip!(); + let tokens = tokens_or_skip!(target); + + for parameter in ["token", "access_token", "jwt", "bearer", "auth"] { + let path = format!("{PROTECTED_PATH}?{parameter}={}", tokens.access); + let outcome = call_with_headers(target, &path, &[]).await; + assert_eq!( + outcome.status, REJECTION_STATUS, + "a genuine token in ?{parameter}= authenticated the request ({}) — body: {}", + outcome.status, outcome.body + ); + } +} + +#[tokio::test] +async fn rejects_a_genuine_token_from_another_realm() { + // A correctly signed, unexpired, entirely legitimate token — from the wrong issuer. Nothing + // about it is malformed; only the `iss` pin and the key set stand between it and access. + let target = target_or_skip!(); + + let Some(credentials) = credentials_from_env("ISM_ATTACK_FOREIGN_") else { + skip( + "no second realm configured: set ISM_ATTACK_FOREIGN_REALM / _CLIENT_ID / _USERNAME / \ + _PASSWORD to run the cross-issuer attack", + ); + return; + }; + + let Some(foreign) = login(target, &credentials).await else { + skip("the configured second realm did not issue a token"); + return; + }; + + assert_rejected(target, "access token from another realm", &foreign.access).await; + if let Some(id_token) = &foreign.id { + assert_rejected(target, "id token from another realm", id_token).await; + } +} + +// ── Opt-in: environment-disturbing and slow checks ────────────────────────── + +#[tokio::test] +#[ignore = "stops and restarts the Keycloak container; run with --test-threads=1 and ISM_ATTACK_ALLOW_DOCKER=1"] +async fn a_keycloak_outage_does_not_open_the_door() { + // The question is which way the service fails when its identity provider disappears. Serving + // cached keys is correct — a rotation that never happened cannot invalidate them, and dropping + // every session because Keycloak restarted would be its own outage. Accepting anything it + // cannot verify is not. + let target = target_or_skip!(); + + if std::env::var("ISM_ATTACK_ALLOW_DOCKER").is_err() { + skip("set ISM_ATTACK_ALLOW_DOCKER=1 to let this test stop and start the Keycloak container"); + return; + } + + let genuine = genuine_tokens(target).await.map(|tokens| tokens.access.clone()); + + compose(&["stop", "keycloak"]); + + // Collected rather than asserted, so Keycloak is restarted even when something fails. + let mut failures: Vec = Vec::new(); + + let forged_token = forged(target, json!({})); + let outcome = call_protected(target, &forged_token).await; + if outcome.status != REJECTION_STATUS { + failures.push(format!( + "with Keycloak down, a forged token produced {} — body: {}", + outcome.status, outcome.body + )); + } + + let unknown_kid = signed( + &json!({ "alg": "RS256", "typ": "JWT", "kid": "rotated-during-the-outage" }), + &claims(target, json!({})), + &attacker_key(), + Algorithm::RS256, + ); + let outcome = call_protected(target, &unknown_kid).await; + if outcome.status != REJECTION_STATUS { + failures.push(format!( + "with Keycloak down, an unknown kid produced {} instead of {REJECTION_STATUS} — body: {}", + outcome.status, outcome.body + )); + } + + let health = call_with_headers(target, "/health", &[]).await; + if health.status != 200 { + failures.push(format!("with Keycloak down, /health answered {}", health.status)); + } + + if let Some(access) = &genuine { + let outcome = call_protected(target, access).await; + println!( + "with Keycloak down, a previously issued genuine token → {} (cached keys still verify \ + it, which is the intended availability behaviour)", + outcome.status + ); + } + + compose(&["start", "keycloak"]); + + // Wait for the realm to answer again, so the next test does not run against a half-started + // Keycloak. + let discovery_url = format!("{}/.well-known/openid-configuration", target.issuer); + for _ in 0..60 { + if target + .http + .get(&discovery_url) + .send() + .await + .is_ok_and(|response| response.status().is_success()) + { + break; + } + tokio::time::sleep(Duration::from_secs(1)).await; + } + + assert!(failures.is_empty(), "{}", failures.join("\n")); +} + +fn compose(arguments: &[&str]) { + let status = std::process::Command::new("docker") + .arg("compose") + .args(arguments) + .status() + .expect("docker compose runs"); + assert!(status.success(), "docker compose {arguments:?} failed"); +} + +#[tokio::test] +#[ignore = "reports timings rather than asserting; run explicitly"] +async fn reports_the_timing_of_each_rejection_path() { + // Informational on purpose. Over a network, per-request noise dwarfs the difference this would + // have to detect, so a threshold here would either never fire or flake constantly. What it is + // good for is spotting an order-of-magnitude gap — a rejection path that hits the network, or a + // claim compared with a short-circuiting string comparison — which is visible by eye. + let target = target_or_skip!(); + + let samples = 40; + let probes: Vec<(&str, String)> = vec![ + ("no token", String::new()), + ("garbage", String::from("not-a-jwt")), + ("alg none", unsigned(&header_with_real_kid(target, "none"), &claims(target, json!({})), "")), + ("bad signature, real kid", forged(target, json!({}))), + ( + "unknown kid", + signed( + &json!({ "alg": "RS256", "typ": "JWT", "kid": "no-such-key" }), + &claims(target, json!({})), + &attacker_key(), + Algorithm::RS256, + ), + ), + ("expired", forged(target, json!({ "exp": now() - 3600 }))), + ("foreign audience", forged(target, json!({ "aud": "elsewhere" }))), + ]; + + for (name, token) in &probes { + let mut timings = Vec::with_capacity(samples); + for _ in 0..samples { + let started = Instant::now(); + if token.is_empty() { + call_with_headers(target, PROTECTED_PATH, &[]).await; + } else { + call_protected(target, token).await; + } + timings.push(started.elapsed()); + } + timings.sort_unstable(); + println!("{name:<26} median {:>9?} p95 {:>9?}", timings[samples / 2], timings[samples * 95 / 100]); + } +}