From 42655ff5ef3e4eae8ff90c623683bb321ae7d06b Mon Sep 17 00:00:00 2001 From: AlexRV12 <71396855+AlexRV12@users.noreply.github.com> Date: Tue, 22 Sep 2026 19:07:21 +0200 Subject: [PATCH] fix: memoize the resolved authed so one request resolves identity once (#11299) * fix: memoize the resolved authed so one request resolves identity once Co-Authored-By: Claude Opus 5 (1M context) * test: pin that the memo preserves job token provenance Co-Authored-By: Claude Opus 5 (1M context) --------- Co-authored-by: Claude Opus 5 (1M context) --- backend/windmill-api-auth/src/auth.rs | 81 +++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 5 deletions(-) diff --git a/backend/windmill-api-auth/src/auth.rs b/backend/windmill-api-auth/src/auth.rs index 6e5aa6b1da..869f40e14c 100644 --- a/backend/windmill-api-auth/src/auth.rs +++ b/backend/windmill-api-auth/src/auth.rs @@ -1073,7 +1073,8 @@ fn no_auth_admin_authed() -> ApiAuthed { } /// Resolves OptJobAuthed from request parts. -/// Takes ownership of Parts and returns them back. +/// Takes ownership of Parts and returns them back, memoized into the extensions so a +/// second extraction in the same request is free. #[allow(unreachable_code, unused_mut)] pub async fn resolve_opt_job_authed( mut parts: Parts, @@ -1089,10 +1090,11 @@ pub async fn resolve_opt_job_authed( )); } - let already_authed = parts.extensions.get::().cloned(); - - if let Some(authed) = already_authed { - return Ok((authed, parts)); + // Safe only while what the route checks below read is fixed before the first + // extraction: path and method always are, `workspace_id` only while every + // `GatewayWorkspaceId` writer stays layered outside auth extraction. + if let Some(opt_job_authed) = parts.extensions.get::().cloned() { + return Ok((opt_job_authed, parts)); } let already_tokened = parts.extensions.get::().cloned(); @@ -1185,6 +1187,9 @@ pub async fn resolve_opt_job_authed( if let Some(workspace_id) = workspace_id { Span::current().record("workspace_id", &workspace_id); } + // Separate from the ApiAuthed insert above, which handlers taking + // `Extension` read. + parts.extensions.insert(opt_job_authed.clone()); return Ok((opt_job_authed, parts)); } } @@ -1311,3 +1316,69 @@ pub async fn list_tokens_internal( Ok(Json(tokens)) } + +#[cfg(test)] +mod tests { + use super::*; + + /// Dropping the `AUTH_CACHE` entry between the two resolutions is what makes a memo + /// hit observable: only a working memo can answer the second one. + #[tokio::test] + async fn resolve_opt_job_authed_is_memoized() { + // Never connected: an AUTH_CACHE hit resolves without a query, and AuthCache + // only needs the handle to exist. Without the timeout a regression — which + // falls through to a real lookup — spends sqlx's 30s default failing. + let db = sqlx::postgres::PgPoolOptions::new() + .acquire_timeout(std::time::Duration::from_millis(50)) + .connect_lazy("postgres://memo@127.0.0.1:1/memo") + .expect("lazy pool"); + + let token = "memo_guard_token"; + // Workspace-scoped: a job token on a workspace-less route is refused by + // check_job_token_for_global_route before the memo is ever taken. + let key = ("memo_workspace".to_string(), token.to_string()); + let job_id = uuid::Uuid::from_u128(0x6d656d6f); + AUTH_CACHE.insert( + key.clone(), + ExpiringAuthCache { + authed: ApiAuthed { + email: "memo@windmill.dev".to_string(), + username: "memo".to_string(), + ..Default::default() + }, + expiry: chrono::Utc::now() + chrono::Duration::hours(1), + job_id: Some(job_id), + }, + ); + + #[cfg(feature = "enterprise")] + let cache = AuthCache::new(db, None, None); + #[cfg(not(feature = "enterprise"))] + let cache = AuthCache::new(db, None); + + let mut parts = http::Request::builder() + .uri("/api/w/memo_workspace/jobs/list") + .header(http::header::AUTHORIZATION, format!("Bearer {token}")) + .body(()) + .unwrap() + .into_parts() + .0; + parts.extensions.insert(Arc::new(cache)); + + let (_, parts) = resolve_opt_job_authed(parts) + .await + .map_err(|(e, _)| e) + .expect("first resolution"); + + AUTH_CACHE.remove(&key); + + let (second, _) = resolve_opt_job_authed(parts) + .await + .map_err(|(e, _)| e) + .expect("second resolution must hit the memo, not re-resolve"); + assert_eq!(second.authed.username, "memo"); + // The wrapper must survive whole: memoizing only the inner ApiAuthed and + // rebuilding around it drops the provenance forbid_elevated_job_token gates on. + assert_eq!(second.job_id, Some(job_id)); + } +}