fix(cli): auto-refresh OAuth access token#4069
Merged
Merged
Conversation
Request `offline_access` scope so the token endpoint returns a refresh token, persist it, and silently exchange it for a new access token in `resolveAuth` when the current one is within 5 minutes of expiry. Only fall through to "Session expired → run login" if the refresh call itself fails. Net effect: sign in once, stay signed in indefinitely so long as `superset` is run within the refresh token's 30-day rotating window. Previously the 1h access-token TTL plus no refresh path meant the CLI forced re-login multiple times per day. No server-side changes — the prod `superset-cli` OAuth client row already has `refresh_token` in its grant types and `offline_access` in its scopes.
Contributor
📝 WalkthroughWalkthroughThe PR adds refresh token support to the Superset CLI authentication flow. OAuth requests now request ChangesRefresh Token Support
Sequence DiagramsequenceDiagram
actor User
participant CLI as CLI Login Command
participant OAuth as OAuth Provider
participant Config as Config File
User->>CLI: Run: superset auth login
CLI->>OAuth: Request auth code (offline_access scope)
OAuth-->>User: Auth code
User->>CLI: Provide auth code
CLI->>OAuth: Exchange code for tokens
OAuth-->>CLI: access_token, refresh_token, expires_in
CLI->>Config: Write auth (accessToken, refreshToken, expiresAt)
Config-->>CLI: Saved
sequenceDiagram
participant CLI as CLI Command
participant Auth as resolve-auth
participant Config as Config File
participant OAuth as OAuth Provider
participant API as Superset API
CLI->>Auth: resolveAuth()
Auth->>Config: Read stored auth config
Config-->>Auth: auth (accessToken, refreshToken, expiresAt)
Alt Token within leeway (expiring soon)
Auth->>OAuth: refreshAccessToken(refreshToken)
OAuth-->>Auth: New access_token, expires_in, refresh_token
Auth->>Config: Update auth (new tokens, expiresAt)
Config-->>Auth: Saved
Auth-->>CLI: bearer = new accessToken
Else Token valid
Auth-->>CLI: bearer = current accessToken
End
CLI->>API: API request with bearer token
API-->>CLI: Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR adds OAuth refresh-token support to the CLI:
Confidence Score: 3/5Safe to merge with a fix to the catch block; the P1 issue causes misleading error messages but no data loss or security regression. One P1 finding (catch block swallows all errors as Session expired) caps the score at 4, and two additional P2 concerns pull it to 3. packages/cli/src/lib/resolve-auth.ts (catch block), packages/cli/src/lib/auth.ts (refresh body scope)
|
| Filename | Overview |
|---|---|
| packages/cli/src/lib/resolve-auth.ts | Core auth resolution updated to silently refresh expiring tokens; catch block swallows all errors (including transient network failures) as "Session expired", which will mislead users when the real issue is not an expired token. |
| packages/cli/src/lib/auth.ts | Added refreshAccessToken function and offline_access scope; refresh body omits scope, which could cause some OIDC servers to not return a new refresh token. |
| packages/cli/src/lib/config.ts | Added optional refreshToken field to SupersetConfig.auth; straightforward type change, no issues. |
| packages/cli/src/commands/auth/login/command.ts | Persists the new refreshToken from the login result into config; minimal, correct change. |
Sequence Diagram
sequenceDiagram
participant CLI as CLI Command
participant RA as resolveAuth()
participant CFG as config.json
participant AS as Auth Server
CLI->>RA: resolveAuth(apiKey?)
RA->>CFG: readConfig()
CFG-->>RA: { auth: { accessToken, refreshToken, expiresAt } }
alt token valid (expiresAt - 5min > now)
RA-->>CLI: bearer = accessToken
else token expiring or expired
alt refreshToken present
RA->>AS: POST /oauth2/token (grant_type=refresh_token)
AS-->>RA: { access_token, refresh_token, expires_in }
RA->>CFG: writeConfig(updated tokens)
RA-->>CLI: bearer = new accessToken
else no refreshToken
RA-->>CLI: CLIError(Session expired)
end
end
note over RA: On any refresh error,<br/>all failures become Session expired
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
packages/cli/src/lib/resolve-auth.ts:54-56
**Catch block swallows non-auth errors**
All errors from `refreshAccessToken` — including transient network failures (e.g., `TypeError: fetch failed`) and server-side 5xx errors — are caught here and replaced with "Session expired / Run: superset auth login." A user experiencing a DNS outage or a momentary server error will be told their session expired and instructed to re-login, which won't fix the underlying issue.
Only HTTP 4xx responses from the token endpoint genuinely mean the refresh token is revoked. For other failures, the original error should propagate.
### Issue 2 of 3
packages/cli/src/lib/resolve-auth.ts:42-53
**Concurrent invocations can invalidate each other's refresh token**
If two CLI commands run simultaneously and both see a near-expiry token, they each call `refreshAccessToken` independently. With rotating refresh tokens, the second rotation invalidates the first command's refresh token. The config also gets written twice with a race on `expiresAt`. A file-lock or compare-and-swap on `expiresAt` before writing would mitigate this.
### Issue 3 of 3
packages/cli/src/lib/auth.ts:260-265
**`scope` omitted from refresh body**
The `refreshAccessToken` request omits a `scope` parameter. While most OIDC servers re-issue the originally granted scopes silently, stricter authorization servers may not return a new `refresh_token` without it. The fallback `data.refresh_token ?? refreshToken` will then reuse the old token until the server's rotation window closes.
Adding the same scope string used at authorization time would be safer.
Reviews (1): Last reviewed commit: "fix(cli): auto-refresh OAuth access toke..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+54
to
+56
| } catch { | ||
| throw new CLIError("Session expired", "Run: superset auth login"); | ||
| } |
There was a problem hiding this comment.
All errors from refreshAccessToken — including transient network failures (e.g., TypeError: fetch failed) and server-side 5xx errors — are caught here and replaced with "Session expired / Run: superset auth login." A user experiencing a DNS outage or a momentary server error will be told their session expired and instructed to re-login, which won't fix the underlying issue.
Only HTTP 4xx responses from the token endpoint genuinely mean the refresh token is revoked. For other failures, the original error should propagate.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/lib/resolve-auth.ts
Line: 54-56
Comment:
**Catch block swallows non-auth errors**
All errors from `refreshAccessToken` — including transient network failures (e.g., `TypeError: fetch failed`) and server-side 5xx errors — are caught here and replaced with "Session expired / Run: superset auth login." A user experiencing a DNS outage or a momentary server error will be told their session expired and instructed to re-login, which won't fix the underlying issue.
Only HTTP 4xx responses from the token endpoint genuinely mean the refresh token is revoked. For other failures, the original error should propagate.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+42
to
+53
| try { | ||
| const refreshed = await refreshAccessToken(auth.refreshToken); | ||
| config = { | ||
| ...config, | ||
| auth: { | ||
| accessToken: refreshed.accessToken, | ||
| refreshToken: refreshed.refreshToken, | ||
| expiresAt: refreshed.expiresAt, | ||
| }, | ||
| }; | ||
| writeConfig(config); | ||
| bearer = refreshed.accessToken; |
There was a problem hiding this comment.
If two CLI commands run simultaneously and both see a near-expiry token, they each call refreshAccessToken independently. With rotating refresh tokens, the second rotation invalidates the first command's refresh token. The config also gets written twice with a race on expiresAt. A file-lock or compare-and-swap on expiresAt before writing would mitigate this.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/lib/resolve-auth.ts
Line: 42-53
Comment:
**Concurrent invocations can invalidate each other's refresh token**
If two CLI commands run simultaneously and both see a near-expiry token, they each call `refreshAccessToken` independently. With rotating refresh tokens, the second rotation invalidates the first command's refresh token. The config also gets written twice with a race on `expiresAt`. A file-lock or compare-and-swap on `expiresAt` before writing would mitigate this.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+260
to
+265
| body: new URLSearchParams({ | ||
| grant_type: "refresh_token", | ||
| refresh_token: refreshToken, | ||
| client_id: CLIENT_ID, | ||
| resource: apiUrl, | ||
| }), |
There was a problem hiding this comment.
scope omitted from refresh body
The refreshAccessToken request omits a scope parameter. While most OIDC servers re-issue the originally granted scopes silently, stricter authorization servers may not return a new refresh_token without it. The fallback data.refresh_token ?? refreshToken will then reuse the old token until the server's rotation window closes.
Adding the same scope string used at authorization time would be safer.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/cli/src/lib/auth.ts
Line: 260-265
Comment:
**`scope` omitted from refresh body**
The `refreshAccessToken` request omits a `scope` parameter. While most OIDC servers re-issue the originally granted scopes silently, stricter authorization servers may not return a new `refresh_token` without it. The fallback `data.refresh_token ?? refreshToken` will then reuse the old token until the server's rotation window closes.
Adding the same scope string used at authorization time would be safer.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/lib/resolve-auth.ts`:
- Around line 38-56: The current logic treats the REFRESH_LEEWAY_MS window as a
hard expiry if refresh is missing/fails; change resolve flow so leeway only
triggers a best-effort refresh via refreshAccessToken, but does not hard-fail
until auth.expiresAt is actually past Date.now(). Specifically, keep the
proactive refresh attempt when (auth.expiresAt - REFRESH_LEEWAY_MS < Date.now())
using refreshAccessToken and writeConfig on success (refreshed ->
accessToken/refreshToken/expiresAt); but if refreshToken is missing or
refreshAccessToken throws, do NOT throw CLIError—fallback to using the existing
auth.accessToken as bearer and allow the session to remain valid until
auth.expiresAt < Date.now(), at which point throw the CLIError("Session
expired", "Run: superset auth login").
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 951aa0fd-b7c4-486e-b9be-81c169fc61fc
📒 Files selected for processing (4)
packages/cli/src/commands/auth/login/command.tspackages/cli/src/lib/auth.tspackages/cli/src/lib/config.tspackages/cli/src/lib/resolve-auth.ts
Comment on lines
+38
to
+56
| if (auth.expiresAt - REFRESH_LEEWAY_MS < Date.now()) { | ||
| if (!auth.refreshToken) { | ||
| throw new CLIError("Session expired", "Run: superset auth login"); | ||
| } | ||
| try { | ||
| const refreshed = await refreshAccessToken(auth.refreshToken); | ||
| config = { | ||
| ...config, | ||
| auth: { | ||
| accessToken: refreshed.accessToken, | ||
| refreshToken: refreshed.refreshToken, | ||
| expiresAt: refreshed.expiresAt, | ||
| }, | ||
| }; | ||
| writeConfig(config); | ||
| bearer = refreshed.accessToken; | ||
| } catch { | ||
| throw new CLIError("Session expired", "Run: superset auth login"); | ||
| } |
Contributor
There was a problem hiding this comment.
Don’t hard-fail before actual expiry when refresh is unavailable.
Line 38 currently converts the 5-minute leeway into an effective hard expiry: if refresh is missing/fails (Lines 39-56), auth fails even when auth.expiresAt is still in the future. This causes avoidable “Session expired” failures during transient refresh issues.
Proposed fix
const auth = config.auth;
- if (auth.expiresAt - REFRESH_LEEWAY_MS < Date.now()) {
- if (!auth.refreshToken) {
- throw new CLIError("Session expired", "Run: superset auth login");
- }
- try {
- const refreshed = await refreshAccessToken(auth.refreshToken);
- config = {
- ...config,
- auth: {
- accessToken: refreshed.accessToken,
- refreshToken: refreshed.refreshToken,
- expiresAt: refreshed.expiresAt,
- },
- };
- writeConfig(config);
- bearer = refreshed.accessToken;
- } catch {
- throw new CLIError("Session expired", "Run: superset auth login");
- }
+ const now = Date.now();
+ if (auth.expiresAt - REFRESH_LEEWAY_MS < now) {
+ if (auth.refreshToken) {
+ try {
+ const refreshed = await refreshAccessToken(auth.refreshToken);
+ config = {
+ ...config,
+ auth: {
+ accessToken: refreshed.accessToken,
+ refreshToken: refreshed.refreshToken,
+ expiresAt: refreshed.expiresAt,
+ },
+ };
+ writeConfig(config);
+ bearer = refreshed.accessToken;
+ } catch {
+ if (auth.expiresAt <= now) {
+ throw new CLIError("Session expired", "Run: superset auth login");
+ }
+ bearer = auth.accessToken;
+ }
+ } else if (auth.expiresAt <= now) {
+ throw new CLIError("Session expired", "Run: superset auth login");
+ } else {
+ bearer = auth.accessToken;
+ }
} else {
bearer = auth.accessToken;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (auth.expiresAt - REFRESH_LEEWAY_MS < Date.now()) { | |
| if (!auth.refreshToken) { | |
| throw new CLIError("Session expired", "Run: superset auth login"); | |
| } | |
| try { | |
| const refreshed = await refreshAccessToken(auth.refreshToken); | |
| config = { | |
| ...config, | |
| auth: { | |
| accessToken: refreshed.accessToken, | |
| refreshToken: refreshed.refreshToken, | |
| expiresAt: refreshed.expiresAt, | |
| }, | |
| }; | |
| writeConfig(config); | |
| bearer = refreshed.accessToken; | |
| } catch { | |
| throw new CLIError("Session expired", "Run: superset auth login"); | |
| } | |
| const now = Date.now(); | |
| if (auth.expiresAt - REFRESH_LEEWAY_MS < now) { | |
| if (auth.refreshToken) { | |
| try { | |
| const refreshed = await refreshAccessToken(auth.refreshToken); | |
| config = { | |
| ...config, | |
| auth: { | |
| accessToken: refreshed.accessToken, | |
| refreshToken: refreshed.refreshToken, | |
| expiresAt: refreshed.expiresAt, | |
| }, | |
| }; | |
| writeConfig(config); | |
| bearer = refreshed.accessToken; | |
| } catch { | |
| if (auth.expiresAt <= now) { | |
| throw new CLIError("Session expired", "Run: superset auth login"); | |
| } | |
| bearer = auth.accessToken; | |
| } | |
| } else if (auth.expiresAt <= now) { | |
| throw new CLIError("Session expired", "Run: superset auth login"); | |
| } else { | |
| bearer = auth.accessToken; | |
| } | |
| } else { | |
| bearer = auth.accessToken; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/cli/src/lib/resolve-auth.ts` around lines 38 - 56, The current logic
treats the REFRESH_LEEWAY_MS window as a hard expiry if refresh is
missing/fails; change resolve flow so leeway only triggers a best-effort refresh
via refreshAccessToken, but does not hard-fail until auth.expiresAt is actually
past Date.now(). Specifically, keep the proactive refresh attempt when
(auth.expiresAt - REFRESH_LEEWAY_MS < Date.now()) using refreshAccessToken and
writeConfig on success (refreshed -> accessToken/refreshToken/expiresAt); but if
refreshToken is missing or refreshAccessToken throws, do NOT throw
CLIError—fallback to using the existing auth.accessToken as bearer and allow the
session to remain valid until auth.expiresAt < Date.now(), at which point throw
the CLIError("Session expired", "Run: superset auth login").
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
saddlepaddle
added a commit
that referenced
this pull request
May 5, 2026
Three fixes since v0.2.6: - relay tunnel: carry binary WS frames as base64 so PTY output renders through cross-host workspaces (no more `[terminal] invalid server payload`) (#4066) - host-service: read remote URLs from git config instead of `git remote -v` (#4065) - cli: auto-refresh OAuth access token using the refresh token (#4069) Push cli-v0.2.7 after this lands to fire the release pipeline.
3 tasks
saddlepaddle
added a commit
that referenced
this pull request
May 10, 2026
The `oauthProvider` was using the default `accessTokenExpiresIn` of 1h. For MCP clients that don't request `offline_access` (or whose refresh token storage is flaky), this forces re-auth multiple times a day. Bump to 7d so a refresh hiccup at most loses a week of session, not a single hour. Same approach we took for the CLI in #4069 — but on the server side so it covers all MCP clients, including ones we don't control (Claude Desktop, Cursor, etc). Note: the underlying upstream issue — `@better-auth/oauth-provider` not preserving audience across refresh-token grants per RFC 8707 §2.2 — is still present in 1.6.10 and 1.7.0-beta.3. Worth filing separately.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
offline_access, persists the returned refresh token, and silently exchanges it for a new access token inresolveAuthwhen the current token is within 5 minutes of expiry.supersetis invoked within the refresh token's rotating 30d window.Previously the 1h access-token TTL with no refresh path forced users to re-login multiple times per day.
No server-side changes — the prod
superset-cliOAuth client row already hasrefresh_tokeningrant_typesandoffline_accessinscopes.Test plan
superset auth login, confirm~/.superset/config.jsonnow hasauth.refreshToken.auth.expiresAtin config to a past timestamp), run anysupersetcommand, confirm it succeeds andexpiresAtis updated.auth.refreshTokenfrom config and expireexpiresAt, confirm command fails withSession expired.superset auth logoutstill clears credentials cleanly.Summary by cubic
Auto-refresh OAuth access tokens in the CLI using a stored refresh token to keep users signed in. We request
offline_access, save the refresh token, and refresh within 5 minutes of expiry; only prompt login if refresh is missing or fails.offline_accessand persistauth.refreshTokenin~/.superset/config.json.resolveAuth, silently refresh when near expiry and updateaccessToken,expiresAt, andrefreshTokenif rotated.Written for commit e574725. Summary will update on new commits.
Summary by CodeRabbit