fix(auth): implement automatic OAuth token refresh - #68
Conversation
…tokens When an access token is expired and a valid refresh token exists, the client now automatically refreshes via the DCR token endpoint before falling back to API keys. This matches the behavior documented in docs/OAUTH2.md and docs/ARCHITECTURE.md. - Add auto-refresh logic to NewWithOptions in pkg/client/client.go - Add test hooks (getStorageFunc, newDCRClientFunc) for dependency injection - Add 8 unit tests covering all refresh paths (success, failure, fallback) Closes DataDog#67 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Cursor <cursoragent@cursor.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e630971be3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dcrClient := newDCRClientFunc(cfg.Site) | ||
| newTokens, refreshErr := dcrClient.RefreshToken(tokens.RefreshToken, creds) | ||
| if refreshErr == nil { | ||
| _ = store.SaveTokens(cfg.Site, newTokens) |
There was a problem hiding this comment.
When refresh succeeds, the code ignores store.SaveTokens errors and continues with the in-memory token. This creates a latent auth regression whenever keychain/file writes fail (e.g., locked keychain, permission issues): the current command appears to work, but the next process reloads stale tokens and may fail to refresh (especially if the refresh token rotated), forcing unexpected re-login or auth errors. This path was introduced by the new auto-refresh flow, so persistence failures should be handled explicitly instead of being silently dropped.
Useful? React with 👍 / 👎.
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
2 participants
Summary
Implement automatic OAuth token refresh when access tokens expire, eliminating the need
for users to manually run
pup auth refreshorpup auth loginevery hour.Changes
NewWithOptionsinpkg/client/client.go— when a token is expiredand a refresh token exists, call
dcr.Client.RefreshToken()and persist the new tokensgetStorageFunc,newDCRClientFunc) for dependency injection in testspkg/client/client_oauth_test.gocovering all refresh pathsTest plan
TestNewWithOptions_ValidToken_UsesOAuth— fresh token uses OAuth directlyTestNewWithOptions_ExpiredToken_NoRefreshToken_FallsBackToAPIKeys— no refresh token, falls backTestNewWithOptions_ExpiredToken_NoClientCreds_FallsBackToAPIKeys— missing creds, falls backTestNewWithOptions_ExpiredToken_RefreshFails_FallsBackToAPIKeys— server rejects, falls backTestNewWithOptions_ExpiredToken_RefreshSucceeds_UsesNewToken— uses refreshed tokenTestNewWithOptions_ExpiredToken_RefreshSucceeds_PersistsNewToken— saves refreshed tokenTestNewWithOptions_ExpiredToken_NoAPIKeys_ReturnsError— clear error when no auth availableTestNewWithOptions_ForceAPIKeys_SkipsOAuth— forceAPIKeys bypasses OAuth entirelypup monitors listsucceeds after silent refreshCloses #67
Made with Cursor