Skip to content

fix(cli): auto-refresh OAuth access token #4069

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saddlepaddle merged 1 commit into from
May 5, 2026
Merged

fix(cli): auto-refresh OAuth access token #4069

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/cli/src/commands/auth/login/command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ export default command({

config.auth = {
accessToken: result.accessToken,
refreshToken: result.refreshToken,
expiresAt: result.expiresAt,
};
writeConfig(config);
Expand Down
45 changes: 43 additions & 2 deletions packages/cli/src/lib/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@ import { env } from "./env";

const CLIENT_ID = "superset-cli";
const PASTE_REDIRECT_PATH = "/cli/auth/code";
const SCOPE = "openid profile email";
const SCOPE = "openid profile email offline_access";
const LOOPBACK_PORTS = [51789, 51790, 51791, 51792, 51793];
const CALLBACK_TIMEOUT_MS = 5 * 60 * 1000;

export interface LoginResult {
accessToken: string;
refreshToken?: string;
expiresAt: number;
}

Expand Down Expand Up @@ -238,11 +239,51 @@ async function exchangeCodeForToken({
access_token: string;
token_type: string;
expires_in?: number;
refresh_token?: string;
};

const expiresIn = data.expires_in ?? 60 * 60 * 24 * 30;
const expiresIn = data.expires_in ?? 60 * 60;
return {
accessToken: data.access_token,
refreshToken: data.refresh_token,
expiresAt: Date.now() + expiresIn * 1000,
};
}

export async function refreshAccessToken(
refreshToken: string,
): Promise<LoginResult> {
const apiUrl = env.SUPERSET_API_URL;
const response = await fetch(`${apiUrl}/api/auth/oauth2/token`, {
method: "POST",
headers: { "Content-Type": "application/x-www-form-urlencoded" },
body: new URLSearchParams({
grant_type: "refresh_token",
refresh_token: refreshToken,
client_id: CLIENT_ID,
resource: apiUrl,
}),
Comment on lines +260 to +265
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

});

if (!response.ok) {
const body = await response.text();
throw new CLIError(
`Token refresh failed: ${response.status}`,
body || "Run `superset auth login` again.",
);
}

const data = (await response.json()) as {
access_token: string;
token_type: string;
expires_in?: number;
refresh_token?: string;
};

const expiresIn = data.expires_in ?? 60 * 60;
return {
accessToken: data.access_token,
refreshToken: data.refresh_token ?? refreshToken,
expiresAt: Date.now() + expiresIn * 1000,
};
}
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/lib/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { env } from "./env";
export type SupersetConfig = {
auth?: {
accessToken: string;
refreshToken?: string;
expiresAt: number;
};
organizationId?: string;
Expand Down
34 changes: 28 additions & 6 deletions packages/cli/src/lib/resolve-auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { CLIError } from "@superset/cli-framework";
import { type ApiClient, createApiClient } from "./api-client";
import { readConfig, type SupersetConfig } from "./config";
import { refreshAccessToken } from "./auth";
import { readConfig, type SupersetConfig, writeConfig } from "./config";

export type AuthSource = "flag" | "env" | "oauth";

Expand All @@ -11,10 +12,12 @@ export type ResolvedAuth = {
authSource: AuthSource;
};

const REFRESH_LEEWAY_MS = 5 * 60 * 1000;

export async function resolveAuth(
apiKeyOption: string | undefined,
): Promise<ResolvedAuth> {
const config = readConfig();
let config = readConfig();

let bearer = apiKeyOption?.trim();
let authSource: AuthSource = bearer ? "flag" : "oauth";
Expand All @@ -30,11 +33,30 @@ export async function resolveAuth(
"Run: superset auth login (or set SUPERSET_API_KEY)",
);
}
const CLOCK_SKEW_MS = 5 * 60 * 1000;
if (config.auth.expiresAt + CLOCK_SKEW_MS < Date.now()) {
throw new CLIError("Session expired", "Run: superset auth login");

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;
Comment on lines +42 to +53
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 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.

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.

} catch {
throw new CLIError("Session expired", "Run: superset auth login");
}
Comment on lines +54 to +56
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 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.

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 +38 to +56
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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").

} else {
bearer = auth.accessToken;
}
bearer = config.auth.accessToken;
}

const api = createApiClient({
Expand Down
Loading