Skip to content

Commit d4aee9d

Browse files
committed
fix(auth): restore automatic OAuth token refresh lost in Rust rewrite
Auto-refresh was implemented in Go (DataDog#68) but not ported during the Go→Rust rewrite (DataDog#103). When a stored access token is expired, the function now attempts to refresh it using the stored refresh token and client credentials before falling back to other auth methods. - Extract resolve_token() helper for testable decision logic - Refresh via tokio::task::block_in_place + Handle::block_on - Release storage mutex before network call, re-acquire to save - Add 6 unit tests covering all refresh paths
1 parent babfaaf commit d4aee9d

1 file changed

Lines changed: 167 additions & 3 deletions

File tree

src/config.rs

Lines changed: 167 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,17 +264,72 @@ pub fn parse_scopes(s: &str) -> Vec<String> {
264264
}
265265

266266
/// Try to load a valid (non-expired) access token from keychain/file storage.
267+
/// If the token is expired, attempts an automatic refresh using the stored refresh token.
267268
/// Returns None silently on any error — callers fall through to other auth methods.
268269
#[cfg(all(not(feature = "browser"), not(target_arch = "wasm32")))]
269270
pub fn load_token_from_storage(site: &str, org: Option<&str>) -> Option<String> {
270271
let guard = crate::auth::storage::get_storage().ok()?;
271272
let lock = guard.lock().ok()?;
272273
let store = lock.as_ref()?;
273274
let tokens = store.load_tokens(site, org).ok()??;
274-
if tokens.is_expired() {
275-
return None;
275+
let creds = store.load_client_credentials(site).ok().flatten();
276+
277+
drop(lock);
278+
279+
let result = resolve_token(tokens, creds.as_ref(), |refresh_token, creds| {
280+
tokio::task::block_in_place(|| {
281+
tokio::runtime::Handle::current().block_on(async {
282+
let dcr_client = crate::auth::dcr::DcrClient::new(site);
283+
dcr_client.refresh_token(refresh_token, creds).await.ok()
284+
})
285+
})
286+
});
287+
288+
match result {
289+
ResolvedToken::Valid(access_token) => Some(access_token),
290+
ResolvedToken::Refreshed(new_tokens) => {
291+
let guard = crate::auth::storage::get_storage().ok()?;
292+
let lock = guard.lock().ok()?;
293+
let store = lock.as_ref()?;
294+
store.save_tokens(site, org, &new_tokens).ok()?;
295+
eprintln!("🔄 Access token refreshed automatically.");
296+
Some(new_tokens.access_token)
297+
}
298+
ResolvedToken::Expired => None,
299+
}
300+
}
301+
302+
enum ResolvedToken {
303+
Valid(String),
304+
Refreshed(crate::auth::types::TokenSet),
305+
Expired,
306+
}
307+
308+
fn resolve_token<F>(
309+
tokens: crate::auth::types::TokenSet,
310+
creds: Option<&crate::auth::types::ClientCredentials>,
311+
refresh_fn: F,
312+
) -> ResolvedToken
313+
where
314+
F: FnOnce(&str, &crate::auth::types::ClientCredentials) -> Option<crate::auth::types::TokenSet>,
315+
{
316+
if !tokens.is_expired() {
317+
return ResolvedToken::Valid(tokens.access_token);
318+
}
319+
320+
if tokens.refresh_token.is_empty() {
321+
return ResolvedToken::Expired;
322+
}
323+
324+
let creds = match creds {
325+
Some(c) => c,
326+
None => return ResolvedToken::Expired,
327+
};
328+
329+
match refresh_fn(&tokens.refresh_token, creds) {
330+
Some(new_tokens) => ResolvedToken::Refreshed(new_tokens),
331+
None => ResolvedToken::Expired,
276332
}
277-
Some(tokens.access_token)
278333
}
279334

280335
#[cfg(not(feature = "browser"))]
@@ -626,4 +681,113 @@ profiles:
626681
let fc: FileConfig = serde_yaml::from_str(yaml).unwrap();
627682
assert_eq!(fc.scopes.as_deref(), Some("dashboards_read,monitors_read"));
628683
}
684+
685+
// --- resolve_token (auto-refresh logic) ---------------------------------
686+
687+
use crate::auth::types::{ClientCredentials, TokenSet};
688+
689+
fn make_token_set(issued_ago_secs: i64, expires_in: i64, refresh: &str) -> TokenSet {
690+
TokenSet {
691+
access_token: "old-access-token".into(),
692+
refresh_token: refresh.into(),
693+
token_type: "Bearer".into(),
694+
expires_in,
695+
issued_at: chrono::Utc::now().timestamp() - issued_ago_secs,
696+
scope: String::new(),
697+
client_id: String::new(),
698+
}
699+
}
700+
701+
fn make_creds() -> ClientCredentials {
702+
ClientCredentials {
703+
client_id: "test-client-id".into(),
704+
client_name: "test-client".into(),
705+
redirect_uris: vec![],
706+
registered_at: 0,
707+
site: "datadoghq.com".into(),
708+
}
709+
}
710+
711+
fn make_refreshed_token_set() -> TokenSet {
712+
TokenSet {
713+
access_token: "fresh-access-token".into(),
714+
refresh_token: "fresh-refresh-token".into(),
715+
token_type: "Bearer".into(),
716+
expires_in: 3600,
717+
issued_at: chrono::Utc::now().timestamp(),
718+
scope: String::new(),
719+
client_id: "test-client-id".into(),
720+
}
721+
}
722+
723+
#[test]
724+
fn test_resolve_token_valid_token() {
725+
let tokens = make_token_set(0, 3600, "refresh");
726+
let creds = make_creds();
727+
let result = super::resolve_token(tokens, Some(&creds), |_, _| {
728+
panic!("refresh_fn should not be called for valid token");
729+
});
730+
match result {
731+
super::ResolvedToken::Valid(t) => assert_eq!(t, "old-access-token"),
732+
_ => panic!("expected Valid"),
733+
}
734+
}
735+
736+
#[test]
737+
fn test_resolve_token_expired_no_refresh_token() {
738+
let tokens = make_token_set(7200, 3600, "");
739+
let creds = make_creds();
740+
let result = super::resolve_token(tokens, Some(&creds), |_, _| {
741+
panic!("refresh_fn should not be called without refresh token");
742+
});
743+
assert!(matches!(result, super::ResolvedToken::Expired));
744+
}
745+
746+
#[test]
747+
fn test_resolve_token_expired_no_client_creds() {
748+
let tokens = make_token_set(7200, 3600, "refresh");
749+
let result = super::resolve_token(tokens, None, |_, _| {
750+
panic!("refresh_fn should not be called without client credentials");
751+
});
752+
assert!(matches!(result, super::ResolvedToken::Expired));
753+
}
754+
755+
#[test]
756+
fn test_resolve_token_expired_refresh_fails() {
757+
let tokens = make_token_set(7200, 3600, "refresh");
758+
let creds = make_creds();
759+
let result = super::resolve_token(tokens, Some(&creds), |_, _| None);
760+
assert!(matches!(result, super::ResolvedToken::Expired));
761+
}
762+
763+
#[test]
764+
fn test_resolve_token_expired_refresh_succeeds() {
765+
let tokens = make_token_set(7200, 3600, "refresh");
766+
let creds = make_creds();
767+
let result = super::resolve_token(tokens, Some(&creds), |rt, c| {
768+
assert_eq!(rt, "refresh");
769+
assert_eq!(c.client_id, "test-client-id");
770+
Some(make_refreshed_token_set())
771+
});
772+
match result {
773+
super::ResolvedToken::Refreshed(t) => {
774+
assert_eq!(t.access_token, "fresh-access-token");
775+
assert_eq!(t.refresh_token, "fresh-refresh-token");
776+
}
777+
_ => panic!("expected Refreshed"),
778+
}
779+
}
780+
781+
#[test]
782+
fn test_resolve_token_near_expiry_triggers_refresh() {
783+
let tokens = make_token_set(3400, 3600, "refresh"); // 200s left < 300s buffer
784+
let creds = make_creds();
785+
let result =
786+
super::resolve_token(
787+
tokens,
788+
Some(&creds),
789+
|_, _| Some(make_refreshed_token_set()),
790+
);
791+
assert!(matches!(result, super::ResolvedToken::Refreshed(_)));
792+
}
629793
}

0 commit comments

Comments
 (0)