diff --git a/crates/pcb-diode-api/src/auth.rs b/crates/pcb-diode-api/src/auth.rs index 9f0e43d16..ef42789c1 100644 --- a/crates/pcb-diode-api/src/auth.rs +++ b/crates/pcb-diode-api/src/auth.rs @@ -303,11 +303,12 @@ pub fn refresh_tokens() -> Result { } pub fn get_valid_token_with_context(ctx: &WorkspaceContext) -> Result { - get_valid_token_with_sources(ctx, refresh_tokens_with_context) + get_valid_token_with_sources(ctx, None, refresh_tokens_with_context) } fn get_valid_token_with_sources( ctx: &WorkspaceContext, + refresh_fallback_lifetime: Option, refresh_tokens: impl Fn(&WorkspaceContext) -> Result, ) -> Result { let not_authenticated = || anyhow::anyhow!(NOT_AUTHENTICATED_MESSAGE); @@ -330,7 +331,16 @@ fn get_valid_token_with_sources( match refresh_tokens(ctx) { Ok(new_tokens) => Ok(new_tokens.access_token), - Err(_) => Err(not_authenticated()), + Err(_) => { + // Check after refreshing: a slow failure may consume the remaining lifetime. + if let Some(lifetime) = refresh_fallback_lifetime { + let now = SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs() as i64; + if tokens.expires_at - now > lifetime.as_secs() as i64 { + return Ok(tokens.access_token); + } + } + Err(not_authenticated()) + } } } @@ -352,6 +362,17 @@ pub fn get_api_token_with_context(ctx: &WorkspaceContext) -> Result Result> { + if api_auth_disabled() { + Ok(None) + } else { + get_valid_token_with_sources(ctx, Some(lifetime), refresh_tokens_with_context).map(Some) + } +} + pub fn get_api_token() -> Result> { let ctx = WorkspaceContext::from_cwd().unwrap_or_default(); get_api_token_with_context(&ctx) @@ -816,7 +837,7 @@ mod tests { let (_tempdir, _guard, ctx) = isolated_context(); let refresh_calls = Cell::new(0); - let err = get_valid_token_with_sources(&ctx, |_| { + let err = get_valid_token_with_sources(&ctx, None, |_| { refresh_calls.set(refresh_calls.get() + 1); anyhow::bail!("refresh should not be called") }) @@ -850,13 +871,13 @@ mod tests { #[test] #[serial] - fn expired_auth_file_refresh_failure_returns_not_authenticated() { + fn near_expiry_auth_file_refresh_failure_without_fallback_returns_not_authenticated() { let (_tempdir, _guard, ctx) = isolated_context(); save_tokens( &ctx, - "expired-token", + "still-valid-token", "refresh-token", - unix_now() - 3600, + unix_now() + 200, Some("user@example.com"), None, None, @@ -864,7 +885,7 @@ mod tests { .unwrap(); let refresh_calls = Cell::new(0); - let err = get_valid_token_with_sources(&ctx, |_| { + let err = get_valid_token_with_sources(&ctx, None, |_| { refresh_calls.set(refresh_calls.get() + 1); anyhow::bail!("refresh failed") }) @@ -890,7 +911,7 @@ mod tests { .unwrap(); let refresh_calls = Cell::new(0); - let token = get_valid_token_with_sources(&ctx, |_| { + let token = get_valid_token_with_sources(&ctx, None, |_| { refresh_calls.set(refresh_calls.get() + 1); Ok(AuthTokens { access_token: "refreshed-token".to_string(), @@ -923,7 +944,7 @@ mod tests { .unwrap(); let refresh_calls = Cell::new(0); - let token = get_valid_token_with_sources(&ctx, |_| { + let token = get_valid_token_with_sources(&ctx, None, |_| { refresh_calls.set(refresh_calls.get() + 1); anyhow::bail!("refresh should not be called") }) diff --git a/crates/pcb-diode-api/src/git_auth.rs b/crates/pcb-diode-api/src/git_auth.rs index d1ef355ca..f6f8f1abd 100644 --- a/crates/pcb-diode-api/src/git_auth.rs +++ b/crates/pcb-diode-api/src/git_auth.rs @@ -167,17 +167,19 @@ fn exchange_credential( path: &str, ) -> Result { let url = format!("{}/api/git/credentials", ctx.api_base_url()); + let timeout = Duration::from_secs(30); let client = Client::builder() .user_agent(format!("diode-pcb/{}", env!("CARGO_PKG_VERSION"))) - .timeout(Duration::from_secs(30)) + .timeout(timeout) .build() .context("Failed to create Git credential HTTP client")?; + // Only this short exchange may reuse a token after refresh failure. + let token = crate::auth::get_api_token_with_refresh_fallback(ctx, timeout)?; let request = client .post(url) .json(&GitCredentialExchangeRequest { host, path }); - - let response = crate::auth::apply_api_auth_with_context(ctx, request)? + let response = crate::auth::apply_bearer_auth(request, token.as_deref()) .send() .context("Failed to exchange Diode authentication for a Git credential")?; diff --git a/crates/pcbc/tests/auth_git.rs b/crates/pcbc/tests/auth_git.rs index 47830017d..05221ef16 100644 --- a/crates/pcbc/tests/auth_git.rs +++ b/crates/pcbc/tests/auth_git.rs @@ -662,3 +662,60 @@ fn store_and_erase_are_silent_without_exchanging_credentials() { assert!(output.stderr.is_empty()); } } + +#[test] +fn near_expiry_token_refresh() { + for (remaining_seconds, refresh_status, bearer) in [ + (200, 503, Some("Bearer still-valid-token")), + (30, 503, None), + (200, 200, Some("Bearer refreshed-token")), + ] { + let server = MockServer::start(); + let api_url = server.base_url(); + let context = TestContext::new(api_url.clone()); + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs(); + fs::write( + context + .config_dir + .join("auth") + .join(format!("{}.toml", auth_scope_slug(&api_url))), + format!( + "access_token = \"still-valid-token\"\n\ + refresh_token = \"refresh-token\"\n\ + expires_at = {}\n\ + token_endpoint = \"{api_url}/oauth/token\"\n\ + client_id = \"test-client-id\"\n", + now + remaining_seconds, + ), + ) + .unwrap(); + let refresh = server.mock(|when, then| { + when.method(POST).path("/oauth/token"); + then.status(refresh_status).json_body(json!({ + "access_token": "refreshed-token", + "refresh_token": "new-refresh-token", + "expires_in": 3600, + })); + }); + let exchange = mock_exchange(&server, GIT_API_HOST, 200, bearer); + let mut command = context.pcbc(); + command.args(["auth", "git", "--host", GIT_API_HOST, "get"]); + let fill = run_with_input(command, &credential_request()); + assert_success(&fill); + let stdout = String::from_utf8_lossy(&fill.stdout); + if bearer.is_some() { + assert!(fill.stderr.is_empty(), "{fill:?}"); + assert!(!stdout.contains("quit=true")); + assert!(stdout.contains(&format!("credential={REPOSITORY_TOKEN}"))); + } else { + assert!(stdout.contains("quit=true")); + assert!(String::from_utf8_lossy(&fill.stderr).contains("Not authenticated")); + assert!(!stdout.contains("credential=")); + } + refresh.assert_calls(1); + exchange.assert_calls(usize::from(bearer.is_some())); + } +}