From 9e1709772d1017cdc37de4520bd12d768171a2a8 Mon Sep 17 00:00:00 2001 From: Trezy Date: Tue, 7 Jul 2026 15:15:36 -0500 Subject: [PATCH] fix: validate JWT aud in middleware Signed-off-by: Trezy --- src/auth/middleware.rs | 14 +++- src/auth/service_auth.rs | 23 +++++- tests/e2e_service_identity.rs | 129 ++++++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+), 5 deletions(-) diff --git a/src/auth/middleware.rs b/src/auth/middleware.rs index 2a5d672..aecc3df 100644 --- a/src/auth/middleware.rs +++ b/src/auth/middleware.rs @@ -123,10 +123,18 @@ impl FromRequestParts for Claims { }); } - // Otherwise, try service auth JWT - let service_auth = super::service_auth::ServiceAuth::from_bearer(token, state).await?; + // Otherwise, try service auth JWT. Route through the same helper the + // XRPC path uses so the token's `aud` is verified against this + // instance's service DID — otherwise a JWT the user minted for a + // different audience would authenticate here and impersonate them + // (H6). `from_bearer` alone checks only the signature and `exp`. + let host = parts + .headers + .get(axum::http::header::HOST) + .and_then(|v| v.to_str().ok()); + let service_claims = try_parse_service_auth(token, state, host).await?; return Ok(Claims { - did: service_auth.did, + did: service_claims.did, client_key: None, dpop_key_id: None, }); diff --git a/src/auth/service_auth.rs b/src/auth/service_auth.rs index fa9f0d9..7edaaa8 100644 --- a/src/auth/service_auth.rs +++ b/src/auth/service_auth.rs @@ -7,6 +7,11 @@ use serde::Deserialize; use crate::AppState; use crate::error::AppError; +/// Maximum accepted `exp - now` for a service-auth JWT (1 hour). atproto tokens +/// are typically valid for ~60s; this generous cap bounds the replay window of a +/// captured token without breaking well-behaved clients. +const MAX_SERVICE_JWT_LIFETIME_SECS: u64 = 3600; + /// Authenticated ATProto user identity extracted from a service auth JWT. /// /// Used for XRPC endpoints that receive proxied requests from PDSes. @@ -132,8 +137,22 @@ fn verify_service_jwt<'a>( return Err(AppError::Auth("JWT expired".into())); } - // Check audience if SERVICE_DID is configured (optional for HappyView). - // For now, accept any audience. + // Bound the acceptance window: atproto service-auth tokens are short-lived + // (~60s). Reject tokens valid absurdly far into the future so a captured + // token isn't replayable for months/years. + if payload.exp > now.saturating_add(MAX_SERVICE_JWT_LIFETIME_SECS) { + tracing::warn!( + exp = payload.exp, + now, + "service auth JWT lifetime exceeds maximum" + ); + return Err(AppError::Auth("JWT lifetime exceeds maximum".into())); + } + + // Audience is validated by the caller against this instance's service DID + // (see `try_parse_service_auth`) — every service-auth entry point routes + // through that check, so a token minted for a different audience is + // rejected rather than trusted here. let _ = &payload.aud; // Check lxm if present (optional validation). diff --git a/tests/e2e_service_identity.rs b/tests/e2e_service_identity.rs index 68af8d6..0941e98 100644 --- a/tests/e2e_service_identity.rs +++ b/tests/e2e_service_identity.rs @@ -2203,3 +2203,132 @@ async fn jwt_with_allowed_typ_accepted() { "JWT with typ=JWT should be accepted" ); } + +// --------------------------------------------------------------------------- +// Service auth — audience validation on the admin/UserAuth path (H6) +// --------------------------------------------------------------------------- + +/// Seed a super user directly, so an authenticated request reaches a 200 rather +/// than a permission error. +async fn seed_super_user(app: &TestApp, did: &str) { + let sql = happyview::db::adapt_sql( + "INSERT INTO happyview_users (id, did, is_super, created_at) VALUES (?, ?, ?, ?)", + app.state.db_backend, + ); + sqlx::query(&sql) + .bind(uuid::Uuid::new_v4().to_string()) + .bind(did) + .bind(1_i32) + .bind(happyview::db::now_rfc3339()) + .execute(&app.state.db) + .await + .expect("seed super user"); +} + +/// A service-auth JWT addressed to THIS instance authenticates on an admin route. +#[tokio::test] +#[serial] +async fn service_auth_correct_aud_accepted_on_admin_route() { + common::require_db!(); + let mut app = TestApp::new().await; + let plc_store = plc::setup_mock_plc(&app.mock_server).await; + let instance_did = app.setup_did_plc().await; + + let issuer = "did:plc:h6-good-issuer"; + seed_super_user(&app, issuer).await; + + let exp = chrono::Utc::now().timestamp() as u64 + 60; + let auth = app + .raw_service_auth_jwt(&plc_store, issuer, &format!("{instance_did}#appview"), exp) + .await; + + let resp = app + .router + .clone() + .oneshot( + Request::builder() + .uri("/admin/stats") + .header("authorization", &auth) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::OK); +} + +/// A service-auth JWT the user minted for a DIFFERENT audience must NOT +/// authenticate on the admin path — otherwise it impersonates the issuer. +#[tokio::test] +#[serial] +async fn service_auth_wrong_aud_rejected_on_admin_route() { + common::require_db!(); + let mut app = TestApp::new().await; + let plc_store = plc::setup_mock_plc(&app.mock_server).await; + let _instance_did = app.setup_did_plc().await; + + // The issuer is even a super user — so if the token authenticated, it would + // return 200. The point is that the wrong audience blocks it entirely. + let issuer = "did:plc:h6-bad-issuer"; + seed_super_user(&app, issuer).await; + + let exp = chrono::Utc::now().timestamp() as u64 + 60; + let auth = app + .raw_service_auth_jwt(&plc_store, issuer, "did:web:evil.example#appview", exp) + .await; + + let resp = app + .router + .clone() + .oneshot( + Request::builder() + .uri("/admin/stats") + .header("authorization", &auth) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!( + resp.status(), + StatusCode::UNAUTHORIZED, + "a service-auth JWT for a different audience must not authenticate here" + ); +} + +/// A service-auth JWT valid absurdly far into the future is rejected even when +/// correctly addressed to this instance (bounds the replay window). +#[tokio::test] +#[serial] +async fn service_auth_excessive_lifetime_rejected() { + common::require_db!(); + let mut app = TestApp::new().await; + let plc_store = plc::setup_mock_plc(&app.mock_server).await; + let instance_did = app.setup_did_plc().await; + + let issuer = "did:plc:h6-longlived"; + seed_super_user(&app, issuer).await; + + // exp two hours out — well beyond the accepted max. + let exp = chrono::Utc::now().timestamp() as u64 + 7200; + let auth = app + .raw_service_auth_jwt(&plc_store, issuer, &format!("{instance_did}#appview"), exp) + .await; + + let resp = app + .router + .clone() + .oneshot( + Request::builder() + .uri("/admin/stats") + .header("authorization", &auth) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); +} -- 2.51.2