From ffef3a0a20e3bcd0a112358de551b201062325ef Mon Sep 17 00:00:00 2001 From: "Edward J. Jinotti" Date: Wed, 9 Sep 2026 11:21:58 -0400 Subject: [PATCH 1/4] claude: fix pub key caching to actually expire --- Cargo.toml | 5 +- README.md | 27 +++ src/axum_service.rs | 20 +- src/config.rs | 15 ++ src/lib.rs | 45 +++- src/protocol_test_suite.rs | 1 + src/validate_incoming.rs | 486 ++++++++++++++++++++++++++++++++++--- 7 files changed, 554 insertions(+), 45 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 7b93fdc..ae424c6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mauth-client" -version = "0.7.2" +version = "0.8.0" authors = ["Mason Gup "] edition = "2024" rust-version = "1.88" @@ -31,6 +31,7 @@ axum = { version = ">= 0.8", optional = true } futures-core = { version = "0.3", optional = true } http = "1" bytes = { version = "1", optional = true } +lru = { version = "0.18", default-features = false, optional = true } thiserror = "2" mauth-core = "0.6" tracing = { version = "0.1", optional = true } @@ -39,7 +40,7 @@ tracing = { version = "0.1", optional = true } tokio = { version = "1", features = ["rt-multi-thread", "macros"] } [features] -axum-service = ["tower", "futures-core", "axum", "bytes", "tracing"] +axum-service = ["tower", "futures-core", "axum", "bytes", "lru", "tracing"] tracing-otel-26 = ["reqwest-tracing/opentelemetry_0_26"] tracing-otel-27 = ["reqwest-tracing/opentelemetry_0_27"] tracing-otel-28 = ["reqwest-tracing/opentelemetry_0_28"] diff --git a/README.md b/README.md index 7fbee77..a745614 100644 --- a/README.md +++ b/README.md @@ -180,6 +180,33 @@ from there in order to take further actions based on the error type. This error implements Axum's `OptionalFromRequestParts`, so you can more easily retrieve it using `Option` anywhere that supports extractors. +Both layers also record the app UUID that the rejected request *claimed*, as an +`AttemptedMAuthIdentity`, so a rejection can be attributed to a caller. The Required layer +attaches it to the 401 **response** extensions, since the request itself is gone by that +point; the Optional layer attaches it to the **request** extensions alongside the error. +It is included on the logged error either way. + +`AttemptedMAuthIdentity` is deliberately a separate type from `ValidatedRequestDetails`, +and it is not an authenticated identity — it is read straight off the signature header of +a request that failed to validate, so anyone could have sent it. Use it for logging and +diagnostics; anything making a trust decision must use `ValidatedRequestDetails`. + +### Public Key Caching + +Verifying an incoming signature needs the sending app's public key, which is fetched from +MAuth and then cached in process. Entries expire according to the `Cache-Control` headers +on MAuth's own response, so rotating an application's key takes effect within the lifetime +MAuth states, without restarting anything. A response arriving without a usable directive +is cached for 60 seconds, matching the MAuth service's own default, so that MAuth does not +become a synchronous dependency of every authenticated request. Lookups that MAuth answers +with a 404 are not cached at all, so an app registered moments ago authenticates +immediately. + +The cache holds 10,000 keys by default, evicting the least recently used, which is sized +for a service with a wide range of callers rather than a handful. Set +`pubkey_cache_capacity` on the `ConfigFileSection` to change it. As with the shared HTTP +client, only the first configuration loaded in a process takes effect. + ### OpenTelemetry Integration There are also optional features `tracing-otel-26` through `tracing-otel-31` diff --git a/src/axum_service.rs b/src/axum_service.rs index cb76605..576b6fe 100644 --- a/src/axum_service.rs +++ b/src/axum_service.rs @@ -13,7 +13,9 @@ use std::task::{Context, Poll}; use tower::{Layer, Service}; use tracing::error; -use crate::validate_incoming::{MAuthValidationError, ValidatedRequestDetails}; +use crate::validate_incoming::{ + AttemptedMAuthIdentity, MAuthValidationError, ValidatedRequestDetails, +}; use crate::{ MAuthInfo, config::{ConfigFileSection, ConfigReadError}, @@ -51,12 +53,22 @@ where Ok(response) => Ok(response.into()), Err(err) => Err(err), }, - Err(err) => { + Err(rejection) => { error!( - error = ?err, + error = ?rejection.error, + app_uuid = rejection.app_uuid.map(tracing::field::display), "Failed to validate MAuth signature, rejecting request" ); - Ok(StatusCode::UNAUTHORIZED.into_response()) + let mut response = StatusCode::UNAUTHORIZED.into_response(); + // The request is gone by the time we reject it, so the + // claimed identity rides out on the response instead, for + // any logging layer sitting outside this one. + if let Some(app_uuid) = rejection.app_uuid { + response + .extensions_mut() + .insert(AttemptedMAuthIdentity { app_uuid }); + } + Ok(response) } } }) diff --git a/src/config.rs b/src/config.rs index 275291b..4f19e6f 100644 --- a/src/config.rs +++ b/src/config.rs @@ -60,6 +60,9 @@ impl MAuthInfo { signer: Signer::new(section.app_uuid.clone(), pk_data.unwrap())?, }; + #[cfg(feature = "axum-service")] + crate::init_pubkey_cache(section.pubkey_cache_capacity); + CLIENT.get_or_init(|| { let builder = ClientBuilder::new(Client::new()).with(mauth_info.clone()); #[cfg(any( @@ -89,6 +92,13 @@ pub struct ConfigFileSection { pub private_key_data: Option, pub v2_only_sign_requests: Option, pub v2_only_authenticate: Option, + /// How many verifying keys to retain when validating incoming requests. + /// `None` uses a default sized for a wide-fanout service; raise it only if + /// more distinct applications than that call this service within the + /// lifetime MAuth puts on a key. Ignored without the `axum-service` feature. + /// + /// Only the first configuration loaded in a process takes effect. + pub pubkey_cache_capacity: Option, } impl Default for ConfigFileSection { @@ -101,6 +111,7 @@ impl Default for ConfigFileSection { private_key_data: None, v2_only_sign_requests: Some(true), v2_only_authenticate: Some(true), + pubkey_cache_capacity: None, } } } @@ -155,6 +166,7 @@ mod test { private_key_data: None, v2_only_sign_requests: None, v2_only_authenticate: None, + pubkey_cache_capacity: None, }; let load_result = MAuthInfo::from_config_section(&bad_config); assert!(matches!(load_result, Err(ConfigReadError::InvalidUri(_)))); @@ -170,6 +182,7 @@ mod test { private_key_data: None, v2_only_sign_requests: None, v2_only_authenticate: None, + pubkey_cache_capacity: None, }; let load_result = MAuthInfo::from_config_section(&bad_config); assert!(matches!( @@ -190,6 +203,7 @@ mod test { private_key_data: None, v2_only_sign_requests: None, v2_only_authenticate: None, + pubkey_cache_capacity: None, }; let load_result = MAuthInfo::from_config_section(&bad_config); fs::remove_file(&filename).await.unwrap(); @@ -211,6 +225,7 @@ mod test { private_key_data: None, v2_only_sign_requests: None, v2_only_authenticate: None, + pubkey_cache_capacity: None, }; let load_result = MAuthInfo::from_config_section(&bad_config); fs::remove_file(&filename).await.unwrap(); diff --git a/src/lib.rs b/src/lib.rs index 7fab385..4a0b687 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,30 +3,61 @@ use ::reqwest_middleware::ClientWithMiddleware; use mauth_core::signer::Signer; -use mauth_core::verifier::Verifier; use reqwest::Url; -use std::collections::HashMap; -use std::sync::{LazyLock, OnceLock, RwLock}; +use std::sync::OnceLock; use uuid::Uuid; +#[cfg(feature = "axum-service")] +use crate::validate_incoming::{DEFAULT_PUBKEY_CACHE_CAPACITY, PubkeyCache}; + /// This is the primary struct of this class. It contains all of the information /// required to sign requests using the MAuth protocol and verify the responses. /// -/// Note that it contains a cache of response keys for verifying response signatures. This cache -/// makes the struct non-Sync. +/// The cache of keys used to verify incoming signatures is process-wide rather than held +/// here, so this struct stays cheap to clone -- which matters, because the validation +/// services clone it for every request they handle. #[derive(Clone)] pub struct MAuthInfo { app_id: Uuid, sign_with_v1_also: bool, signer: Signer, + // These two are only consulted when validating incoming requests, which the + // `axum-service` feature gates. They are always populated regardless, so the + // struct shape does not vary between feature sets. + #[cfg_attr(not(feature = "axum-service"), allow(dead_code))] mauth_uri_base: Url, + #[cfg_attr(not(feature = "axum-service"), allow(dead_code))] allow_v1_auth: bool, } static CLIENT: OnceLock = OnceLock::new(); -static PUBKEY_CACHE: LazyLock>> = - LazyLock::new(|| RwLock::new(HashMap::new())); +#[cfg(feature = "axum-service")] +static PUBKEY_CACHE: OnceLock = OnceLock::new(); + +/// The process-wide cache of verifier keys fetched from MAuth. +/// +/// Deliberately global rather than a field on [`MAuthInfo`]: +/// `RequiredMAuthValidationService::call` clones its `MAuthInfo` for *every* +/// request, and that `Clone` impl rebuilds the struct from config, so a +/// per-instance cache would never survive to see a second lookup. +/// +/// Falls back to [`DEFAULT_PUBKEY_CACHE_CAPACITY`] if a lookup somehow happens +/// before any config has been loaded. +#[cfg(feature = "axum-service")] +pub(crate) fn pubkey_cache() -> &'static PubkeyCache { + PUBKEY_CACHE.get_or_init(|| PubkeyCache::new(DEFAULT_PUBKEY_CACHE_CAPACITY)) +} + +/// Size the key cache from configuration. +/// +/// First caller wins, matching the behavior of `CLIENT`: a later +/// `ConfigFileSection` naming a different capacity is silently ignored. +#[cfg(feature = "axum-service")] +pub(crate) fn init_pubkey_cache(capacity: Option) { + PUBKEY_CACHE + .get_or_init(|| PubkeyCache::new(capacity.unwrap_or(DEFAULT_PUBKEY_CACHE_CAPACITY))); +} /// Tower Service and Layer to allow Tower-integrated servers to validate incoming request #[cfg(feature = "axum-service")] diff --git a/src/protocol_test_suite.rs b/src/protocol_test_suite.rs index dda691c..be75d0b 100644 --- a/src/protocol_test_suite.rs +++ b/src/protocol_test_suite.rs @@ -30,6 +30,7 @@ async fn setup_mauth_info() -> (MAuthInfo, u64) { private_key_data: None, v2_only_sign_requests: None, v2_only_authenticate: None, + pubkey_cache_capacity: None, }; ( MAuthInfo::from_config_section(&mock_config_section).unwrap(), diff --git a/src/validate_incoming.rs b/src/validate_incoming.rs index e06d90c..40c171a 100644 --- a/src/validate_incoming.rs +++ b/src/validate_incoming.rs @@ -1,12 +1,35 @@ -use crate::{CLIENT, MAuthInfo, PUBKEY_CACHE}; +use crate::{CLIENT, MAuthInfo, pubkey_cache}; use axum::extract::Request; use bytes::Bytes; use chrono::prelude::*; +use http::{HeaderMap, StatusCode, header}; +use lru::LruCache; use mauth_core::verifier::Verifier; +use std::num::NonZeroUsize; +use std::sync::{Mutex, MutexGuard}; +use std::time::{Duration, Instant}; use thiserror::Error; use tracing::error; use uuid::Uuid; +/// How many verifier keys are retained before the least recently used is +/// dropped. +/// +/// This crate is used by every MAuth-authenticating service, so this is sized +/// for a wide-fanout gateway rather than a small API. Entries cost roughly a +/// kilobyte each, and only apps that actually call the service occupy one -- +/// lookups that MAuth answers with a 404 are never stored -- so the cache +/// cannot be grown by an unregistered caller. Override with +/// `ConfigFileSection::pubkey_cache_capacity` where the default does not fit. +pub(crate) const DEFAULT_PUBKEY_CACHE_CAPACITY: usize = 10_000; + +/// How long a key is trusted when MAuth does not say. +/// +/// Matches the MAuth service's own `TOKEN_EXPIRATION` default, so a response +/// that arrives without usable cache headers is treated the way an unconfigured +/// MAuth would have asked for. +const FALLBACK_KEY_LIFETIME: Duration = Duration::from_secs(60); + /// This struct holds the app UUID for a validated request. It is meant to be used with the /// Extension setup in Hyper requests, where it is placed in requests that passed authentication. /// The custom struct makes it clearer that the request has passed and this is an authenticated @@ -18,20 +41,47 @@ pub struct ValidatedRequestDetails { pub app_uuid: Uuid, } +/// The app UUID a request *claimed*, read from its MAuth signature header +/// without verifying anything. +/// +/// This is deliberately a separate type from [`ValidatedRequestDetails`], +/// because it is not an authenticated identity: it is attached to requests that +/// **failed** validation, so that a rejection can be attributed in logs. +/// Anything making a trust decision must use [`ValidatedRequestDetails`]. +/// +/// Found in the extensions of the `401` response for the required-validation +/// layer, and in the request extensions for the optional one. +#[derive(Debug, Clone)] +#[non_exhaustive] +pub struct AttemptedMAuthIdentity { + pub app_uuid: Uuid, +} + +/// A request that failed validation, along with the identity it claimed. +pub(crate) struct RejectedRequest { + pub(crate) error: MAuthValidationError, + pub(crate) app_uuid: Option, +} + const MAUTH_V1_SIGNATURE_HEADER: &str = "X-MWS-Authentication"; const MAUTH_V2_SIGNATURE_HEADER: &str = "MCC-Authentication"; const MAUTH_V1_TIMESTAMP_HEADER: &str = "X-MWS-Time"; const MAUTH_V2_TIMESTAMP_HEADER: &str = "MCC-Time"; impl MAuthInfo { - pub(crate) async fn validate_request( - &self, - req: Request, - ) -> Result { + pub(crate) async fn validate_request(&self, req: Request) -> Result { let (mut parts, body) = req.into_parts(); + // Read the claimed identity up front. On the required-validation path + // the request is dropped when we reject it, so this is the only chance + // to capture who was turned away. + let attempted = Self::attempted_app_uuid(&parts.headers); + let reject = |error| RejectedRequest { + error, + app_uuid: attempted, + }; let body_bytes = axum::body::to_bytes(body, usize::MAX) .await - .map_err(|_| MAuthValidationError::InvalidBody)?; + .map_err(|_| reject(MAuthValidationError::InvalidBody))?; match self.validate_request_v2(&parts, &body_bytes).await { Ok(host_app_uuid) => { parts.extensions.insert(ValidatedRequestDetails { @@ -52,10 +102,10 @@ impl MAuthInfo { let new_request = Request::from_parts(parts, new_body); Ok(new_request) } - Err(err) => Err(err), + Err(err) => Err(reject(err)), } } else { - Err(err) + Err(reject(err)) } } } @@ -82,6 +132,8 @@ impl MAuthInfo { } }; + let attempted = Self::attempted_app_uuid(&parts.headers); + match self.validate_request_v2(&parts, &body_bytes).await { Ok(host_app_uuid) => { parts.extensions.insert(ValidatedRequestDetails { @@ -100,13 +152,20 @@ impl MAuthInfo { error!( ?error_v2, ?error_v1, + app_uuid = attempted.map(tracing::field::display), "Error attempting to validate MAuth signatures" ); + Self::record_attempted_identity(&mut parts, attempted); parts.extensions.insert(error_v1); } } } else { - error!(?error_v2, "Error attempting to validate MAuth V2 signature"); + error!( + ?error_v2, + app_uuid = attempted.map(tracing::field::display), + "Error attempting to validate MAuth V2 signature" + ); + Self::record_attempted_identity(&mut parts, attempted); parts.extensions.insert(error_v2); } } @@ -247,33 +306,167 @@ impl MAuthInfo { Ok((host_app_uuid, signature_encoded_string.into())) } + /// The app UUID a request claims, taken straight from its signature header + /// with no verification at all. + /// + /// Prefers the V2 header when both are present, matching the order the + /// validation paths try them in. Never use the result as an identity. + fn attempted_app_uuid(headers: &HeaderMap) -> Option { + let (header_name, token) = if headers.contains_key(MAUTH_V2_SIGNATURE_HEADER) { + (MAUTH_V2_SIGNATURE_HEADER, "MWSV2") + } else { + (MAUTH_V1_SIGNATURE_HEADER, "MWS") + }; + let header_value = headers.get(header_name)?.to_str().ok()?; + Self::split_auth_string(header_value, token) + .ok() + .map(|(app_uuid, _)| app_uuid) + } + + /// Attach the claimed identity to a request that failed validation, so an + /// outer logging layer can attribute the rejection. + fn record_attempted_identity(parts: &mut http::request::Parts, app_uuid: Option) { + if let Some(app_uuid) = app_uuid { + parts.extensions.insert(AttemptedMAuthIdentity { app_uuid }); + } + } + async fn get_app_pub_key(&self, app_uuid: &Uuid) -> Option { - { - let key_store = PUBKEY_CACHE.read().unwrap(); - if let Some(pub_key) = key_store.get(app_uuid) { - return Some(pub_key.clone()); - } + if let Some(verifier) = pubkey_cache().get(app_uuid, Instant::now()) { + return Some(verifier); } + let uri = self.mauth_uri_base.join(&format!("{}", app_uuid)).unwrap(); - let mauth_response = CLIENT.get().unwrap().get(uri).send().await; - match mauth_response { - Err(_) => None, - Ok(response) => { - if let Ok(response_obj) = response.json::().await - && let Some(pub_key_str) = response_obj - .pointer("/security_token/public_key_str") - .and_then(|s| s.as_str()) - .map(|st| st.to_owned()) - && let Ok(verifier) = Verifier::new(*app_uuid, pub_key_str) - { - let mut key_store = PUBKEY_CACHE.write().unwrap(); - key_store.insert(*app_uuid, verifier.clone()); - Some(verifier) - } else { - None - } - } + let request_started = Instant::now(); + let response = CLIENT.get().unwrap().get(uri).send().await.ok()?; + + // Only a 200 carries a key. Checking explicitly keeps a 404 -- which + // means "no such app" -- from being distinguished only by a later + // pointer lookup happening to miss. + if response.status() != StatusCode::OK { + return None; + } + // `json` consumes the response, so decide the freshness first. + let freshness = cacheable_until(response.headers(), request_started, Instant::now()); + + let response_obj = response.json::().await.ok()?; + let pub_key_str = response_obj + .pointer("/security_token/public_key_str") + .and_then(|s| s.as_str())?; + let verifier = Verifier::new(*app_uuid, pub_key_str.to_owned()).ok()?; + + if let Freshness::Until(expires_at) = freshness { + pubkey_cache().put(*app_uuid, verifier.clone(), expires_at); + } + Some(verifier) + } +} + +/// How long a fetched key may be reused, if at all. +#[derive(Debug, PartialEq, Eq)] +enum Freshness { + /// Reusable until this moment. + Until(Instant), + /// The origin forbade reuse, or the response was already stale on arrival. + DoNotStore, +} + +/// When a MAuth response stops being reusable. +/// +/// MAuth states its own policy -- `max-age=300, public` on a hit today, though +/// the service reads that from an environment variable and its own default is +/// 60 -- so this honors what the origin says rather than assuming a number. A +/// response with no usable directive falls back to [`FALLBACK_KEY_LIFETIME`], +/// which keeps MAuth from becoming a synchronous dependency of every single +/// authenticated request if those headers ever regress. +fn cacheable_until(headers: &HeaderMap, request_started: Instant, now: Instant) -> Freshness { + let fallback = Freshness::Until(now + FALLBACK_KEY_LIFETIME); + + let Some(cache_control) = headers.get(header::CACHE_CONTROL) else { + return fallback; + }; + let Ok(cache_control) = cache_control.to_str() else { + return fallback; + }; + + let mut max_age: Option = None; + for directive in cache_control.split(',') { + let directive = directive.trim(); + // `no-cache` permits storage with revalidation, which this cache has no + // way to do -- MAuth sends no ETag on the responses that carry it -- so + // treat it the same as `no-store`. This is the path a 404 takes. + if directive.eq_ignore_ascii_case("no-store") || directive.eq_ignore_ascii_case("no-cache") + { + return Freshness::DoNotStore; + } + if let Some((name, seconds)) = directive.split_once('=') + && name.trim().eq_ignore_ascii_case("max-age") + && let Ok(seconds) = seconds.trim().trim_matches('"').parse::() + { + // Take the shortest of any repeats, so `max-age=0, max-age=300` + // is not treated as reusable. + let stated = Duration::from_secs(seconds); + max_age = Some(max_age.map_or(stated, |seen: Duration| seen.min(stated))); + } + } + + let Some(max_age) = max_age else { + return fallback; + }; + + // RFC 9111 4.2.3, simplified: what the origin says the response has + // already aged, plus how long it spent reaching us. + let age = headers + .get(header::AGE) + .and_then(|age| age.to_str().ok()) + .and_then(|age| age.trim().parse::().ok()) + .map_or(Duration::ZERO, Duration::from_secs) + + now.saturating_duration_since(request_started); + + match max_age.checked_sub(age) { + Some(remaining) if !remaining.is_zero() => Freshness::Until(now + remaining), + _ => Freshness::DoNotStore, + } +} + +/// A bounded, expiring store of verifier keys. +/// +/// Both bounds matter. The expiry is the point: without it a key rotation +/// leaves every long-lived process rejecting the rotated app forever. The +/// capacity is a safety net -- entries can only be created for registered apps +/// that actually call this service, since failed lookups are never stored. +pub(crate) struct PubkeyCache(Mutex>); + +impl PubkeyCache { + pub(crate) fn new(capacity: usize) -> Self { + let capacity = NonZeroUsize::new(capacity) + .unwrap_or(NonZeroUsize::new(DEFAULT_PUBKEY_CACHE_CAPACITY).unwrap()); + Self(Mutex::new(LruCache::new(capacity))) + } + + fn lock(&self) -> MutexGuard<'_, LruCache> { + // A panic while holding this lock cannot leave the cache unsound -- at + // worst an insert did not land -- so recover rather than propagating + // the poison and taking down authentication with it. + self.0 + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) + } + + /// The key for `app_uuid`, if one is cached and still fresh. A hit promotes + /// recency, so a steady caller is not evicted by a burst of one-off ones. + fn get(&self, app_uuid: &Uuid, now: Instant) -> Option { + let mut cache = self.lock(); + let expires_at = cache.peek(app_uuid).map(|(_, expires_at)| *expires_at)?; + if now >= expires_at { + cache.pop(app_uuid); + return None; } + cache.get(app_uuid).map(|(verifier, _)| verifier.clone()) + } + + fn put(&self, app_uuid: Uuid, verifier: Verifier, expires_at: Instant) { + self.lock().put(app_uuid, (verifier, expires_at)); } } @@ -306,3 +499,232 @@ pub enum MAuthValidationError { #[error("The body of the response did not match the signature")] SignatureVerifyFailure, } + +#[cfg(test)] +mod tests { + use super::*; + use http::HeaderName; + + const APP_UUID: &str = "66de0549-f7ea-4f17-9984-10643f3357d1"; + const OTHER_UUID: &str = "b0021882-1049-495d-99d7-9df99a78c0e0"; + + fn headers(pairs: &[(&str, &str)]) -> HeaderMap { + let mut headers = HeaderMap::new(); + for (name, value) in pairs { + headers.insert( + HeaderName::from_bytes(name.as_bytes()).unwrap(), + value.parse().unwrap(), + ); + } + headers + } + + fn verifier(app_uuid: Uuid) -> Verifier { + let public_key = + std::fs::read_to_string("mauth-protocol-test-suite/signing-params/rsa-key-pub") + .unwrap(); + Verifier::new(app_uuid, public_key).unwrap() + } + + #[test] + fn cache_lifetime_follows_what_mauth_states() { + let now = Instant::now(); + let secs = Duration::from_secs; + let cases: [(&[(&str, &str)], Freshness); 10] = [ + // What MAuth actually sends on a hit today. + ( + &[("cache-control", "max-age=300, public")], + Freshness::Until(now + secs(300)), + ), + // What it sends on a 404, and the reason unknown apps are not + // negatively cached. + (&[("cache-control", "no-cache")], Freshness::DoNotStore), + (&[("cache-control", "no-store")], Freshness::DoNotStore), + // An explicit directive beats any max-age beside it. + ( + &[("cache-control", "no-cache, max-age=300")], + Freshness::DoNotStore, + ), + // Shortest repeat wins, so this is not treated as reusable. + ( + &[("cache-control", "max-age=0, max-age=300")], + Freshness::DoNotStore, + ), + ( + &[("cache-control", "max-age=300, max-age=60")], + Freshness::Until(now + secs(60)), + ), + // Already partly aged upstream. + ( + &[("cache-control", "max-age=300"), ("age", "60")], + Freshness::Until(now + secs(240)), + ), + // Aged past its own lifetime before reaching us. + ( + &[("cache-control", "max-age=60"), ("age", "300")], + Freshness::DoNotStore, + ), + // No usable directive falls back rather than making MAuth a + // dependency of every authenticated request. + (&[], Freshness::Until(now + FALLBACK_KEY_LIFETIME)), + ( + &[("cache-control", "max-age=not-a-number")], + Freshness::Until(now + FALLBACK_KEY_LIFETIME), + ), + ]; + + for (pairs, expected) in cases { + assert_eq!( + cacheable_until(&headers(pairs), now, now), + expected, + "headers: {pairs:?}" + ); + } + } + + #[test] + fn time_in_flight_counts_against_the_stated_lifetime() { + let started = Instant::now(); + let now = started + Duration::from_secs(10); + + assert_eq!( + cacheable_until(&headers(&[("cache-control", "max-age=300")]), started, now), + Freshness::Until(now + Duration::from_secs(290)), + ); + } + + #[test] + fn a_cached_key_is_returned_until_it_expires() { + let app_uuid = Uuid::parse_str(APP_UUID).unwrap(); + let base = Instant::now(); + let cache = PubkeyCache::new(4); + + cache.put( + app_uuid, + verifier(app_uuid), + base + Duration::from_secs(300), + ); + + assert!(cache.get(&app_uuid, base).is_some()); + assert!( + cache + .get(&app_uuid, base + Duration::from_secs(299)) + .is_some() + ); + } + + #[test] + fn an_expired_key_is_a_miss_and_is_dropped() { + let app_uuid = Uuid::parse_str(APP_UUID).unwrap(); + let base = Instant::now(); + let cache = PubkeyCache::new(4); + + cache.put( + app_uuid, + verifier(app_uuid), + base + Duration::from_secs(300), + ); + + // This is the rotation case: the key is present but no longer trusted, + // so the caller re-fetches instead of rejecting the app forever. + assert!( + cache + .get(&app_uuid, base + Duration::from_secs(300)) + .is_none() + ); + assert!(cache.lock().peek(&app_uuid).is_none()); + } + + #[test] + fn capacity_evicts_the_least_recently_used() { + let base = Instant::now(); + let expires_at = base + Duration::from_secs(300); + let cache = PubkeyCache::new(1); + + let first = Uuid::parse_str(APP_UUID).unwrap(); + let second = Uuid::parse_str(OTHER_UUID).unwrap(); + cache.put(first, verifier(first), expires_at); + cache.put(second, verifier(second), expires_at); + + assert!(cache.get(&first, base).is_none()); + assert!(cache.get(&second, base).is_some()); + } + + #[test] + fn reading_a_key_protects_it_from_eviction() { + let base = Instant::now(); + let expires_at = base + Duration::from_secs(300); + let cache = PubkeyCache::new(2); + + let steady = Uuid::parse_str(APP_UUID).unwrap(); + let other = Uuid::parse_str(OTHER_UUID).unwrap(); + let one_off = Uuid::new_v4(); + + cache.put(steady, verifier(steady), expires_at); + cache.put(other, verifier(other), expires_at); + // A steady caller stays resident even as one-off callers churn through. + assert!(cache.get(&steady, base).is_some()); + cache.put(one_off, verifier(one_off), expires_at); + + assert!(cache.get(&steady, base).is_some()); + assert!(cache.get(&other, base).is_none()); + } + + #[test] + fn zero_capacity_falls_back_to_the_default() { + let app_uuid = Uuid::parse_str(APP_UUID).unwrap(); + let base = Instant::now(); + let cache = PubkeyCache::new(0); + + cache.put( + app_uuid, + verifier(app_uuid), + base + Duration::from_secs(300), + ); + + assert!(cache.get(&app_uuid, base).is_some()); + } + + #[test] + fn attempted_identity_is_read_from_either_signature_header() { + let expected = Uuid::parse_str(APP_UUID).unwrap(); + let v2 = format!("MWSV2 {APP_UUID}:some-signature;"); + let v1 = format!("MWS {APP_UUID}:some-signature"); + + assert_eq!( + MAuthInfo::attempted_app_uuid(&headers(&[("mcc-authentication", &v2)])), + Some(expected), + ); + assert_eq!( + MAuthInfo::attempted_app_uuid(&headers(&[("x-mws-authentication", &v1)])), + Some(expected), + ); + } + + #[test] + fn attempted_identity_prefers_v2_and_tolerates_junk() { + let v2 = format!("MWSV2 {APP_UUID}:some-signature;"); + let v1 = format!("MWS {OTHER_UUID}:some-signature"); + + assert_eq!( + MAuthInfo::attempted_app_uuid(&headers(&[ + ("mcc-authentication", &v2), + ("x-mws-authentication", &v1), + ])), + Some(Uuid::parse_str(APP_UUID).unwrap()), + "V2 is tried first, so it names the identity", + ); + + // A rejection is still a rejection when we cannot say who sent it -- + // these must report no identity rather than fail. + assert_eq!(MAuthInfo::attempted_app_uuid(&HeaderMap::new()), None); + assert_eq!( + MAuthInfo::attempted_app_uuid(&headers(&[("mcc-authentication", "MWSV2 nonsense:x;")])), + None, + ); + assert_eq!( + MAuthInfo::attempted_app_uuid(&headers(&[("mcc-authentication", "garbage")])), + None, + ); + } +} From c26cd1b94d03821f8870d062cb51cac5f89715ac Mon Sep 17 00:00:00 2001 From: "Edward J. Jinotti" Date: Wed, 9 Sep 2026 11:48:50 -0400 Subject: [PATCH 2/4] claude: update comments/readme --- README.md | 4 ++-- src/validate_incoming.rs | 22 ++++++++++------------ 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index a745614..330c0d4 100644 --- a/README.md +++ b/README.md @@ -197,8 +197,8 @@ Verifying an incoming signature needs the sending app's public key, which is fet MAuth and then cached in process. Entries expire according to the `Cache-Control` headers on MAuth's own response, so rotating an application's key takes effect within the lifetime MAuth states, without restarting anything. A response arriving without a usable directive -is cached for 60 seconds, matching the MAuth service's own default, so that MAuth does not -become a synchronous dependency of every authenticated request. Lookups that MAuth answers +is cached for a conservative 60 seconds, so that MAuth does not become a synchronous +dependency of every authenticated request. Lookups that MAuth answers with a 404 are not cached at all, so an app registered moments ago authenticates immediately. diff --git a/src/validate_incoming.rs b/src/validate_incoming.rs index 40c171a..63cf866 100644 --- a/src/validate_incoming.rs +++ b/src/validate_incoming.rs @@ -25,9 +25,9 @@ pub(crate) const DEFAULT_PUBKEY_CACHE_CAPACITY: usize = 10_000; /// How long a key is trusted when MAuth does not say. /// -/// Matches the MAuth service's own `TOKEN_EXPIRATION` default, so a response -/// that arrives without usable cache headers is treated the way an unconfigured -/// MAuth would have asked for. +/// Deliberately short. It exists so that a response arriving without usable +/// cache headers cannot make MAuth a synchronous dependency of every +/// authenticated request, while still letting a rotated key converge quickly. const FALLBACK_KEY_LIFETIME: Duration = Duration::from_secs(60); /// This struct holds the app UUID for a validated request. It is meant to be used with the @@ -373,12 +373,10 @@ enum Freshness { /// When a MAuth response stops being reusable. /// -/// MAuth states its own policy -- `max-age=300, public` on a hit today, though -/// the service reads that from an environment variable and its own default is -/// 60 -- so this honors what the origin says rather than assuming a number. A -/// response with no usable directive falls back to [`FALLBACK_KEY_LIFETIME`], -/// which keeps MAuth from becoming a synchronous dependency of every single -/// authenticated request if those headers ever regress. +/// The serving side decides how long its keys may be reused and states that in +/// the response, so this honors what the origin says rather than assuming a +/// number that could drift out of agreement with it. A response carrying no +/// usable directive falls back to [`FALLBACK_KEY_LIFETIME`]. fn cacheable_until(headers: &HeaderMap, request_started: Instant, now: Instant) -> Freshness { let fallback = Freshness::Until(now + FALLBACK_KEY_LIFETIME); @@ -531,13 +529,13 @@ mod tests { let now = Instant::now(); let secs = Duration::from_secs; let cases: [(&[(&str, &str)], Freshness); 10] = [ - // What MAuth actually sends on a hit today. + // The ordinary case: a key that may be reused for a while. ( &[("cache-control", "max-age=300, public")], Freshness::Until(now + secs(300)), ), - // What it sends on a 404, and the reason unknown apps are not - // negatively cached. + // How a not-found response is marked, and the reason an unknown + // app is never negatively cached. (&[("cache-control", "no-cache")], Freshness::DoNotStore), (&[("cache-control", "no-store")], Freshness::DoNotStore), // An explicit directive beats any max-age beside it. From 2b0bedb01d3078cf49a19517c4eadebdc00d5f13 Mon Sep 17 00:00:00 2001 From: "Edward J. Jinotti" Date: Wed, 9 Sep 2026 12:13:46 -0400 Subject: [PATCH 3/4] claude: address copilot review --- src/validate_incoming.rs | 203 ++++++++++++++++++++++++++++++++------- 1 file changed, 170 insertions(+), 33 deletions(-) diff --git a/src/validate_incoming.rs b/src/validate_incoming.rs index 63cf866..e1b202c 100644 --- a/src/validate_incoming.rs +++ b/src/validate_incoming.rs @@ -378,33 +378,37 @@ enum Freshness { /// number that could drift out of agreement with it. A response carrying no /// usable directive falls back to [`FALLBACK_KEY_LIFETIME`]. fn cacheable_until(headers: &HeaderMap, request_started: Instant, now: Instant) -> Freshness { - let fallback = Freshness::Until(now + FALLBACK_KEY_LIFETIME); - - let Some(cache_control) = headers.get(header::CACHE_CONTROL) else { - return fallback; - }; - let Ok(cache_control) = cache_control.to_str() else { - return fallback; - }; + let fallback = expires_after(now, FALLBACK_KEY_LIFETIME); let mut max_age: Option = None; - for directive in cache_control.split(',') { - let directive = directive.trim(); - // `no-cache` permits storage with revalidation, which this cache has no - // way to do -- MAuth sends no ETag on the responses that carry it -- so - // treat it the same as `no-store`. This is the path a 404 takes. - if directive.eq_ignore_ascii_case("no-store") || directive.eq_ignore_ascii_case("no-cache") - { + // `Cache-Control` is a list header, and HTTP permits a list to arrive split + // across several field lines. Reading only the first would let a `no-store` + // on a later line be silently ignored. + for value in headers.get_all(header::CACHE_CONTROL) { + // A value we cannot read leaves the origin's instruction unknown, which + // is not the same as it having given none. + let Ok(value) = value.to_str() else { return Freshness::DoNotStore; - } - if let Some((name, seconds)) = directive.split_once('=') - && name.trim().eq_ignore_ascii_case("max-age") - && let Ok(seconds) = seconds.trim().trim_matches('"').parse::() - { - // Take the shortest of any repeats, so `max-age=0, max-age=300` - // is not treated as reusable. - let stated = Duration::from_secs(seconds); - max_age = Some(max_age.map_or(stated, |seen: Duration| seen.min(stated))); + }; + for directive in value.split(',') { + let directive = directive.trim(); + // `no-cache` permits storage with revalidation, which this cache has + // no way to do -- MAuth sends no ETag on the responses that carry it + // -- so treat it the same as `no-store`. A 404 takes this path. + if directive.eq_ignore_ascii_case("no-store") + || directive.eq_ignore_ascii_case("no-cache") + { + return Freshness::DoNotStore; + } + if let Some((name, seconds)) = directive.split_once('=') + && name.trim().eq_ignore_ascii_case("max-age") + && let Ok(seconds) = seconds.trim().trim_matches('"').parse::() + { + // Take the shortest of any repeats, so `max-age=0, max-age=300` + // is not treated as reusable. + let stated = Duration::from_secs(seconds); + max_age = Some(max_age.map_or(stated, |seen: Duration| seen.min(stated))); + } } } @@ -412,21 +416,52 @@ fn cacheable_until(headers: &HeaderMap, request_started: Instant, now: Instant) return fallback; }; + // `Age` is a singleton field, so a repeated or unreadable one leaves the + // true age unknown. Decline rather than assume the response is fresh. + let mut ages = headers.get_all(header::AGE).into_iter(); + let stated_age = match (ages.next(), ages.next()) { + (None, _) => Duration::ZERO, + (Some(_), Some(_)) => return Freshness::DoNotStore, + (Some(age), None) => { + match age + .to_str() + .ok() + .and_then(|age| age.trim().parse::().ok()) + { + Some(seconds) => Duration::from_secs(seconds), + None => return Freshness::DoNotStore, + } + } + }; + // RFC 9111 4.2.3, simplified: what the origin says the response has - // already aged, plus how long it spent reaching us. - let age = headers - .get(header::AGE) - .and_then(|age| age.to_str().ok()) - .and_then(|age| age.trim().parse::().ok()) - .map_or(Duration::ZERO, Duration::from_secs) - + now.saturating_duration_since(request_started); + // already aged, plus how long it spent reaching us. Both halves are + // response-controlled, so this is checked rather than assumed to fit. + let Some(age) = stated_age.checked_add(now.saturating_duration_since(request_started)) else { + return Freshness::DoNotStore; + }; match max_age.checked_sub(age) { - Some(remaining) if !remaining.is_zero() => Freshness::Until(now + remaining), + Some(remaining) if !remaining.is_zero() => expires_after(now, remaining), _ => Freshness::DoNotStore, } } +/// `now + lifetime`, or [`Freshness::DoNotStore`] when that is not +/// representable. +/// +/// Lifetimes here are derived from response headers, so a malformed origin or +/// proxy must not be able to panic the process that is authenticating a +/// request. A duration too large to add is refused rather than clamped: a +/// response claiming it is reusable for longer than time itself is not one to +/// trust. +fn expires_after(now: Instant, lifetime: Duration) -> Freshness { + match now.checked_add(lifetime) { + Some(expires_at) => Freshness::Until(expires_at), + None => Freshness::DoNotStore, + } +} + /// A bounded, expiring store of verifier keys. /// /// Both bounds matter. The expiry is the point: without it a key rotation @@ -506,10 +541,12 @@ mod tests { const APP_UUID: &str = "66de0549-f7ea-4f17-9984-10643f3357d1"; const OTHER_UUID: &str = "b0021882-1049-495d-99d7-9df99a78c0e0"; + /// Builds a header map, appending rather than replacing so that a repeated + /// name produces the multiple field lines HTTP allows for list headers. fn headers(pairs: &[(&str, &str)]) -> HeaderMap { let mut headers = HeaderMap::new(); for (name, value) in pairs { - headers.insert( + headers.append( HeaderName::from_bytes(name.as_bytes()).unwrap(), value.parse().unwrap(), ); @@ -580,6 +617,106 @@ mod tests { } } + #[test] + fn every_cache_control_field_line_is_considered() { + let now = Instant::now(); + + // A list header may arrive as several field lines. Reading only the + // first would cache this against the origin's explicit instruction. + assert_eq!( + cacheable_until( + &headers(&[ + ("cache-control", "max-age=300"), + ("cache-control", "no-store"), + ]), + now, + now, + ), + Freshness::DoNotStore, + ); + assert_eq!( + cacheable_until( + &headers(&[ + ("cache-control", "max-age=300"), + ("cache-control", "max-age=60"), + ]), + now, + now, + ), + Freshness::Until(now + Duration::from_secs(60)), + ); + } + + #[test] + fn absurd_cache_values_are_refused_rather_than_panicking() { + let now = Instant::now(); + let huge = u64::MAX.to_string(); + + // These are response-controlled, so overflow must not be able to take + // down the process that is trying to authenticate a request. + assert_eq!( + cacheable_until( + &headers(&[("cache-control", &format!("max-age={huge}"))]), + now, + now + ), + Freshness::DoNotStore, + ); + assert_eq!( + cacheable_until( + &headers(&[("cache-control", "max-age=300"), ("age", &huge)]), + now, + now + Duration::from_secs(1), + ), + Freshness::DoNotStore, + ); + } + + #[test] + fn an_unreadable_directive_is_not_the_same_as_no_directive() { + let now = Instant::now(); + let mut unreadable = HeaderMap::new(); + unreadable.insert( + header::CACHE_CONTROL, + http::HeaderValue::from_bytes(b"max-age=\xff").unwrap(), + ); + + // No header at all means the origin stated no policy, and the fallback + // applies. A header we cannot read means it stated one we failed to + // understand, which is not something to guess at. + assert_eq!( + cacheable_until(&unreadable, now, now), + Freshness::DoNotStore, + ); + } + + #[test] + fn an_unusable_age_is_declined_rather_than_assumed_fresh() { + let now = Instant::now(); + + // Age is a singleton field; two of them leave the real age unknown. + assert_eq!( + cacheable_until( + &headers(&[ + ("cache-control", "max-age=300"), + ("age", "60"), + ("age", "90") + ]), + now, + now, + ), + Freshness::DoNotStore, + ); + assert_eq!( + cacheable_until( + &headers(&[("cache-control", "max-age=300"), ("age", "not-a-number")]), + now, + now, + ), + Freshness::DoNotStore, + ); + } + #[test] fn time_in_flight_counts_against_the_stated_lifetime() { let started = Instant::now(); From a74e639fa56a725729c68350948a767e57afc941 Mon Sep 17 00:00:00 2001 From: "Edward J. Jinotti" Date: Wed, 9 Sep 2026 13:13:58 -0400 Subject: [PATCH 4/4] gpt: improvements --- Cargo.toml | 1 + src/lib.rs | 13 +- src/validate_incoming.rs | 253 +++++++++++++++++++++++++++++++++++++-- 3 files changed, 253 insertions(+), 14 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index ae424c6..2552f44 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,6 +38,7 @@ tracing = { version = "0.1", optional = true } [dev-dependencies] tokio = { version = "1", features = ["rt-multi-thread", "macros"] } +rsa = "0.9" [features] axum-service = ["tower", "futures-core", "axum", "bytes", "lru", "tracing"] diff --git a/src/lib.rs b/src/lib.rs index 4a0b687..8a129e1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -13,9 +13,8 @@ use crate::validate_incoming::{DEFAULT_PUBKEY_CACHE_CAPACITY, PubkeyCache}; /// This is the primary struct of this class. It contains all of the information /// required to sign requests using the MAuth protocol and verify the responses. /// -/// The cache of keys used to verify incoming signatures is process-wide rather than held -/// here, so this struct stays cheap to clone -- which matters, because the validation -/// services clone it for every request they handle. +/// The cache of keys used to verify incoming signatures is process-wide and is +/// shared by all instances of this struct. #[derive(Clone)] pub struct MAuthInfo { app_id: Uuid, @@ -37,10 +36,10 @@ static PUBKEY_CACHE: OnceLock = OnceLock::new(); /// The process-wide cache of verifier keys fetched from MAuth. /// -/// Deliberately global rather than a field on [`MAuthInfo`]: -/// `RequiredMAuthValidationService::call` clones its `MAuthInfo` for *every* -/// request, and that `Clone` impl rebuilds the struct from config, so a -/// per-instance cache would never survive to see a second lookup. +/// The validation services clone themselves for every request. Their custom +/// `Clone` implementations rebuild `MAuthInfo` from configuration; `MAuthInfo` +/// itself derives `Clone`. Keeping the cache global preserves entries across +/// those service clones without changing the existing service implementation. /// /// Falls back to [`DEFAULT_PUBKEY_CACHE_CAPACITY`] if a lookup somehow happens /// before any config has been loaded. diff --git a/src/validate_incoming.rs b/src/validate_incoming.rs index e1b202c..a70e130 100644 --- a/src/validate_incoming.rs +++ b/src/validate_incoming.rs @@ -15,11 +15,9 @@ use uuid::Uuid; /// How many verifier keys are retained before the least recently used is /// dropped. /// -/// This crate is used by every MAuth-authenticating service, so this is sized -/// for a wide-fanout gateway rather than a small API. Entries cost roughly a -/// kilobyte each, and only apps that actually call the service occupy one -- -/// lookups that MAuth answers with a 404 are never stored -- so the cache -/// cannot be grown by an unregistered caller. Override with +/// This default accommodates services with many distinct callers. Only +/// successful public-key lookups create entries; 404 responses are not cached. +/// Override with /// `ConfigFileSection::pubkey_cache_capacity` where the default does not fit. pub(crate) const DEFAULT_PUBKEY_CACHE_CAPACITY: usize = 10_000; @@ -466,8 +464,8 @@ fn expires_after(now: Instant, lifetime: Duration) -> Freshness { /// /// Both bounds matter. The expiry is the point: without it a key rotation /// leaves every long-lived process rejecting the rotated app forever. The -/// capacity is a safety net -- entries can only be created for registered apps -/// that actually call this service, since failed lookups are never stored. +/// capacity limits memory use, since expired entries are removed lazily and +/// successful lookups can introduce additional application UUIDs. pub(crate) struct PubkeyCache(Mutex>); impl PubkeyCache { @@ -536,11 +534,252 @@ pub enum MAuthValidationError { #[cfg(test)] mod tests { use super::*; + use crate::axum_service::{OptionalMAuthValidationLayer, RequiredMAuthValidationLayer}; + use crate::config::ConfigFileSection; + use axum::{Router, routing::get}; use http::HeaderName; + use rsa::{ + RsaPrivateKey, + pkcs1::EncodeRsaPrivateKey, + pkcs8::{EncodePublicKey, LineEnding}, + rand_core::OsRng, + }; + use std::sync::{ + Arc, LazyLock, RwLock, + atomic::{AtomicUsize, Ordering}, + }; + use tower::{Layer, Service}; const APP_UUID: &str = "66de0549-f7ea-4f17-9984-10643f3357d1"; const OTHER_UUID: &str = "b0021882-1049-495d-99d7-9df99a78c0e0"; + // Generate disposable test keys in memory; no credentials are persisted. + static TEST_KEYS: LazyLock<[(String, String); 2]> = LazyLock::new(|| { + std::array::from_fn(|_| { + let key = RsaPrivateKey::new(&mut OsRng, 2048).unwrap(); + let public = key + .to_public_key() + .to_public_key_pem(LineEnding::LF) + .unwrap(); + let private = key.to_pkcs1_pem(LineEnding::LF).unwrap().to_string(); + (private, public) + }) + }); + + fn test_config(app_uuid: Uuid, base_url: &str, key_index: usize) -> ConfigFileSection { + ConfigFileSection { + app_uuid: app_uuid.to_string(), + mauth_baseurl: base_url.to_owned(), + private_key_data: Some(TEST_KEYS[key_index].0.clone()), + ..ConfigFileSection::default() + } + } + + fn signed_request(signer: &MAuthInfo) -> Request { + let mut outgoing = reqwest::Request::new( + http::Method::GET, + "http://localhost/resource?value=1".parse().unwrap(), + ); + signer.sign_request(&mut outgoing).unwrap(); + let mut incoming = Request::builder() + .uri("/resource?value=1") + .body(axum::body::Body::empty()) + .unwrap(); + *incoming.headers_mut() = outgoing.headers().clone(); + incoming + } + + #[tokio::test] + async fn expired_key_is_refetched_and_validates_a_rotated_signature() { + let app_uuid = Uuid::new_v4(); + let served_key = Arc::new(RwLock::new(TEST_KEYS[0].1.clone())); + let fetches = Arc::new(AtomicUsize::new(0)); + let endpoint = Router::new().route( + &format!("/mauth/v1/security_tokens/{app_uuid}"), + get({ + let served_key = Arc::clone(&served_key); + let fetches = Arc::clone(&fetches); + move || { + let public_key = served_key.read().unwrap().clone(); + fetches.fetch_add(1, Ordering::SeqCst); + async move { + ( + [(header::CACHE_CONTROL, "max-age=300")], + axum::Json(serde_json::json!({ + "security_token": { "public_key_str": public_key } + })), + ) + } + } + }), + ); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let base_url = format!("http://{}", listener.local_addr().unwrap()); + let server = tokio::spawn(async move { axum::serve(listener, endpoint).await.unwrap() }); + let original = + MAuthInfo::from_config_section(&test_config(app_uuid, &base_url, 0)).unwrap(); + let rotated = MAuthInfo::from_config_section(&test_config(app_uuid, &base_url, 1)).unwrap(); + + for _ in 0..2 { + let validated = original + .validate_request(signed_request(&original)) + .await + .ok() + .unwrap(); + assert_eq!( + validated + .extensions() + .get::() + .unwrap() + .app_uuid, + app_uuid + ); + assert!( + validated + .extensions() + .get::() + .is_none() + ); + } + assert_eq!(fetches.load(Ordering::SeqCst), 1); + + *served_key.write().unwrap() = TEST_KEYS[1].1.clone(); + let rejection = original + .validate_request(signed_request(&rotated)) + .await + .err() + .unwrap(); + assert!(matches!( + rejection.error, + MAuthValidationError::SignatureVerifyFailure + )); + assert_eq!(fetches.load(Ordering::SeqCst), 1); + + // Advance this entry to its expiry boundary without sleeping or + // changing the process-wide cache used by other tests. + { + let mut cache = pubkey_cache().lock(); + let (_, expires_at) = cache.get_mut(&app_uuid).unwrap(); + assert!(*expires_at > Instant::now()); + *expires_at = Instant::now(); + } + for _ in 0..2 { + let validated = original + .validate_request(signed_request(&rotated)) + .await + .ok() + .unwrap(); + assert_eq!( + validated + .extensions() + .get::() + .unwrap() + .app_uuid, + app_uuid + ); + } + assert_eq!(fetches.load(Ordering::SeqCst), 2); + pubkey_cache().lock().pop(&app_uuid); + server.abort(); + } + + fn rejected_request(app_uuid: Uuid, v1: bool) -> Request { + let (name, value) = if v1 { + (MAUTH_V1_SIGNATURE_HEADER, format!("MWS {app_uuid}:invalid")) + } else { + ( + MAUTH_V2_SIGNATURE_HEADER, + format!("MWSV2 {app_uuid}:invalid;"), + ) + }; + // A missing timestamp rejects locally, without a public-key lookup. + Request::builder() + .uri("/") + .header(name, value) + .body(axum::body::Body::empty()) + .unwrap() + } + + #[tokio::test] + async fn required_layer_exposes_rejected_identity_on_response() { + let app_uuid = Uuid::new_v4(); + let mut config = test_config(app_uuid, "http://127.0.0.1:1", 0); + config.v2_only_authenticate = Some(false); + let handler_calls = Arc::new(AtomicUsize::new(0)); + let inner = Router::new().route( + "/", + get({ + let handler_calls = Arc::clone(&handler_calls); + move || { + handler_calls.fetch_add(1, Ordering::SeqCst); + async { StatusCode::NO_CONTENT } + } + }), + ); + let mut service = RequiredMAuthValidationLayer::from_config_section(config) + .unwrap() + .layer(inner); + for v1 in [false, true] { + let response = service.call(rejected_request(app_uuid, v1)).await.unwrap(); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert_eq!( + response + .extensions() + .get::() + .unwrap() + .app_uuid, + app_uuid + ); + assert!( + response + .extensions() + .get::() + .is_none() + ); + } + let response = service + .call(Request::new(axum::body::Body::empty())) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + assert!( + response + .extensions() + .get::() + .is_none() + ); + assert_eq!(handler_calls.load(Ordering::SeqCst), 0); + } + + #[tokio::test] + async fn optional_layer_exposes_rejected_identity_and_error_to_handler() { + let app_uuid = Uuid::new_v4(); + let mut config = test_config(app_uuid, "http://127.0.0.1:1", 0); + config.v2_only_authenticate = Some(false); + let inner = Router::new().route( + "/", + get(move |req: Request| async move { + assert_eq!( + req.extensions() + .get::() + .unwrap() + .app_uuid, + app_uuid + ); + assert!(req.extensions().get::().is_some()); + assert!(req.extensions().get::().is_none()); + StatusCode::NO_CONTENT + }), + ); + let mut service = OptionalMAuthValidationLayer::from_config_section(config) + .unwrap() + .layer(inner); + for v1 in [false, true] { + let response = service.call(rejected_request(app_uuid, v1)).await.unwrap(); + assert_eq!(response.status(), StatusCode::NO_CONTENT); + } + } + /// Builds a header map, appending rather than replacing so that a repeated /// name produces the multiple field lines HTTP allows for list headers. fn headers(pairs: &[(&str, &str)]) -> HeaderMap {