diff --git a/crates/manager-core/src/admin_users_api.rs b/crates/manager-core/src/admin_users_api.rs index 2c8d51d..0a23f4b 100644 --- a/crates/manager-core/src/admin_users_api.rs +++ b/crates/manager-core/src/admin_users_api.rs @@ -45,6 +45,11 @@ pub struct AdminsState { /// Capability gate: every endpoint here requires /// `InstanceManageUsers` (owner / admin). pub authz: Arc, + /// Audit 2026-06-11 (PrincipalCache revocation-lag) — evicted on + /// password change / deactivation so a just-revoked session or + /// API key stops authenticating immediately instead of lingering + /// for the cache TTL. + pub principal_cache: Arc, } pub fn admins_router(state: AdminsState) -> Router { @@ -254,6 +259,12 @@ async fn patch_admin( tracing::error!(?err, "failed to expire api keys on password change"); } } + // Audit 2026-06-11 (PrincipalCache revocation-lag) — the DB + // writes above are best-effort by design (a blip must not undo + // the rotation), so evicting the in-process cache is what makes + // the rotation take effect on the very next request rather than + // after the cache TTL. + state.principal_cache.evict_user(id); } if let Some(email_patch) = input.email.as_ref() { @@ -323,6 +334,11 @@ async fn patch_admin( tracing::error!(?err, "failed to expire api keys for deactivated admin"); } } + // Audit 2026-06-11 (PrincipalCache revocation-lag) — evict + // cached principals so a deactivated admin's session / API + // keys stop authenticating immediately instead of lingering + // for the cache TTL window. + state.principal_cache.evict_user(id); } } diff --git a/crates/manager-core/src/api_keys_api.rs b/crates/manager-core/src/api_keys_api.rs index e86b40d..570fa1e 100644 --- a/crates/manager-core/src/api_keys_api.rs +++ b/crates/manager-core/src/api_keys_api.rs @@ -39,6 +39,10 @@ const NAME_MAX: usize = 64; #[derive(Clone)] pub struct ApiKeysState { pub keys: Arc, + /// Audit 2026-06-11 (PrincipalCache revocation-lag) — evicted when + /// a user deletes one of their own keys so it stops authenticating + /// from cache immediately rather than after the cache TTL. + pub principal_cache: Arc, } pub fn api_keys_router(state: ApiKeysState) -> Router { @@ -160,6 +164,12 @@ async fn delete_key( // we deliberately don't leak the distinction. return Err(ApiKeysError::NotFound(id)); } + // Audit 2026-06-11 (PrincipalCache revocation-lag) — the cache is + // keyed by token hash, which we can't derive from the key id, so + // evict every cached principal for this user. That also drops their + // session entries; re-auth on the next request is the right cost for + // a deliberate key revocation. + state.principal_cache.evict_user(principal.user_id); Ok(StatusCode::NO_CONTENT) } diff --git a/crates/manager-core/src/auth_api.rs b/crates/manager-core/src/auth_api.rs index 7cfdb93..ffb7963 100644 --- a/crates/manager-core/src/auth_api.rs +++ b/crates/manager-core/src/auth_api.rs @@ -195,6 +195,11 @@ async fn logout(State(state): State, req: Request) -> Response if let Err(err) = state.sessions.delete(&hash).await { tracing::error!(?err, "admin_sessions delete failed"); } + // Audit 2026-06-11 (PrincipalCache revocation-lag) — drop just + // this token from the cache so the logged-out session can't keep + // authenticating for the cache TTL. Other sessions for the same + // user are left untouched. + state.principal_cache.evict_token(&hash); } StatusCode::NO_CONTENT.into_response() } diff --git a/crates/manager-core/src/auth_middleware.rs b/crates/manager-core/src/auth_middleware.rs index e2b5465..551e37b 100644 --- a/crates/manager-core/src/auth_middleware.rs +++ b/crates/manager-core/src/auth_middleware.rs @@ -92,6 +92,32 @@ impl PrincipalCache { } guard.insert(token_hash, (Instant::now(), principal)); } + + /// Evict every cached principal belonging to `user_id`. + /// + /// Audit 2026-06-11 (PrincipalCache revocation-lag, Medium) — the + /// revocation paths (`set_active(false)`, password change, + /// API-key delete) flip the backing DB rows, but a cached principal + /// keeps authenticating for up to [`PRINCIPAL_CACHE_TTL`] unless it + /// is evicted. Callers invoke this right after the DB mutation so a + /// just-revoked credential stops working on the next request. The + /// cache value carries the resolved [`Principal`], so we can target + /// the user without knowing which token hashes belong to them. + pub fn evict_user(&self, user_id: AdminUserId) { + let Ok(mut guard) = self.inner.lock() else { + return; + }; + guard.retain(|_, (_, p)| p.user_id != user_id); + } + + /// Evict a single cached entry by its token hash. Used on logout, + /// where exactly one session token is being invalidated and we + /// don't want to disturb the user's other live sessions. + pub fn evict_token(&self, token_hash: &str) { + if let Ok(mut guard) = self.inner.lock() { + guard.remove(token_hash); + } + } } /// Prefix on the wire that selects the API-key path. The body that @@ -471,4 +497,46 @@ mod tests { assert!(p.scopes.is_none()); assert!(p.app_binding.is_none()); } + + fn principal_for(user_id: AdminUserId) -> Principal { + Principal { + user_id, + instance_role: InstanceRole::Owner, + scopes: None, + app_binding: None, + } + } + + #[test] + fn evict_user_drops_only_that_users_entries() { + // Audit 2026-06-11 (PrincipalCache revocation-lag). + let cache = PrincipalCache::new(); + let alice = AdminUserId::new(); + let bob = AdminUserId::new(); + cache.insert("hash-alice-1".into(), principal_for(alice)); + cache.insert("hash-alice-2".into(), principal_for(alice)); + cache.insert("hash-bob-1".into(), principal_for(bob)); + + cache.evict_user(alice); + + assert!(cache.get("hash-alice-1").is_none()); + assert!(cache.get("hash-alice-2").is_none()); + // Bob is untouched. + assert!(cache.get("hash-bob-1").is_some()); + } + + #[test] + fn evict_token_drops_only_that_entry() { + let cache = PrincipalCache::new(); + let alice = AdminUserId::new(); + cache.insert("hash-1".into(), principal_for(alice)); + cache.insert("hash-2".into(), principal_for(alice)); + + cache.evict_token("hash-1"); + + // Logout drops exactly one session; the user's other live + // session stays cached. + assert!(cache.get("hash-1").is_none()); + assert!(cache.get("hash-2").is_some()); + } } diff --git a/crates/picloud/src/lib.rs b/crates/picloud/src/lib.rs index 95faa4d..d40911d 100644 --- a/crates/picloud/src/lib.rs +++ b/crates/picloud/src/lib.rs @@ -498,12 +498,19 @@ pub async fn build_app( .and_then(|s| s.parse::().ok()) .filter(|n| *n > 0) .unwrap_or(2); + // Audit 2026-06-11 (PrincipalCache revocation-lag) — one shared + // cache instance threaded into every state that performs auth or + // revocation, so an evict on the revocation side is visible to the + // middleware's resolve side. A per-state cache would let a revoked + // principal keep authenticating from the middleware's own copy. + let principal_cache = + Arc::new(picloud_manager_core::auth_middleware::PrincipalCache::new()); let auth_state = AuthState { users: auth.users.clone(), sessions: auth.sessions.clone(), keys: auth.keys.clone(), ttl: auth.ttl, - principal_cache: Arc::new(picloud_manager_core::auth_middleware::PrincipalCache::new()), + principal_cache: principal_cache.clone(), login_rate_limiter: Arc::new( picloud_manager_core::login_rate_limit::LoginRateLimiter::new(), ), @@ -514,6 +521,7 @@ pub async fn build_app( sessions: auth.sessions, keys: auth.keys.clone(), authz: authz.clone(), + principal_cache: principal_cache.clone(), }; let app_members_state = AppMembersState { apps: apps_state.apps.clone(), @@ -526,7 +534,10 @@ pub async fn build_app( authz: authz.clone(), users: users.clone(), }; - let api_keys_state = ApiKeysState { keys: auth.keys }; + let api_keys_state = ApiKeysState { + keys: auth.keys, + principal_cache: principal_cache.clone(), + }; // /admin/auth/login + /logout are unguarded by design (login is how // you get in). /admin/auth/me applies the middleware internally so diff --git a/crates/picloud/tests/authz.rs b/crates/picloud/tests/authz.rs index 85ccc2d..fc66b64 100644 --- a/crates/picloud/tests/authz.rs +++ b/crates/picloud/tests/authz.rs @@ -380,12 +380,14 @@ async fn member_can_only_touch_apps_they_belong_to(pool: PgPool) { } // ---------------------------------------------------------------------------- -// 5. Bearer pic_ + cookie produce the same Principal +// 5. Session-token bearer + pic_ API-key bearer produce the same Principal +// (cookie auth was removed in the audit 2026-06-11 C-1 fix; both paths +// now travel through the Authorization header). // ---------------------------------------------------------------------------- #[ignore = "needs DATABASE_URL pointing at a running Postgres"] #[sqlx::test(migrations = "../manager-core/migrations")] -async fn bearer_and_cookie_produce_same_principal(pool: PgPool) { +async fn session_token_and_api_key_produce_same_principal(pool: PgPool) { let s = boot(pool).await; let session_token = login_token(&s.server, "owner", "owner-pw").await;