From c653912e099f726b76318174eff9b9bc9253800a Mon Sep 17 00:00:00 2001 From: amitbardos Date: Thu, 22 Oct 2026 14:30:32 +0300 Subject: [PATCH] fix(auth): stop parallel commands from logging the user out Refresh tokens rotate on use. When several CLI processes found an expired token at once, each refreshed with the same refresh token; every refresh after the first was rejected and its catch deleted ~/.base44/auth for all of them. The in-memory refreshPromise only guarded a single process. Hold a lock file across processes while refreshing, and use the token another process already wrote instead of refreshing again. Delete auth only when the server rejects the refresh token, not on network or server errors. Co-Authored-By: Claude Opus 5.5 --- docs/api-patterns.md | 4 +- packages/cli/src/core/auth/config.ts | 81 ++++++++++++++++---- packages/cli/tests/cli/authorization.spec.ts | 53 ++++++++++++- 3 files changed, 121 insertions(+), 17 deletions(-) diff --git a/docs/api-patterns.md b/docs/api-patterns.md index 26c88fc72..83dff8403 100644 --- a/docs/api-patterns.md +++ b/docs/api-patterns.md @@ -43,6 +43,8 @@ The `base44Client` automatically handles token refresh: 2. If expired, refreshes token and saves new tokens 3. On 401 response, attempts refresh and retries once +Refresh tokens rotate on use, so `refreshAndSaveTokens()` holds a lock file (`~/.base44/auth/auth.json.lock`) across processes. A process that waited for the lock and finds a new refresh token in `auth.json` uses it instead of refreshing again. Auth is deleted only when the server rejects the refresh token (400/401), not on network or server errors. + ## Environment-Supplied Credentials ### Workspace API keys @@ -72,7 +74,7 @@ validates), pairs it with `BASE44_REFRESH_TOKEN`, and writes a standard This overwrites an existing login when env vars are present, and no-ops when they can't form a complete record. The seeded file looks like a normal login, so -a 401 refresh applies to it too; if refresh fails it deletes the file, which +a 401 refresh applies to it too; if the refresh token is rejected it deletes the file, which self-heals on the next command (re-seeded from the still-present env vars). See `seedAuthFromEnv()` in `core/auth/config.js`. diff --git a/packages/cli/src/core/auth/config.ts b/packages/cli/src/core/auth/config.ts index d24bfc489..e524e80cf 100644 --- a/packages/cli/src/core/auth/config.ts +++ b/packages/cli/src/core/auth/config.ts @@ -1,16 +1,27 @@ +import { mkdir, open, stat, unlink } from "node:fs/promises"; +import { dirname } from "node:path"; +import { setTimeout as sleep } from "node:timers/promises"; import jwt, { type JwtPayload } from "jsonwebtoken"; import { renewAccessToken } from "@/core/auth/api.js"; import type { AuthData } from "@/core/auth/schema.js"; import { AuthDataSchema } from "@/core/auth/schema.js"; import { getAuthFilePath } from "@/core/config.js"; -import { FileReadError, SchemaValidationError } from "@/core/errors.js"; +import { + ApiError, + FileReadError, + SchemaValidationError, +} from "@/core/errors.js"; import { deleteFile, readJsonFile, writeJsonFile } from "@/core/utils/fs.js"; // Buffer time before expiration to trigger proactive refresh (60 seconds) const TOKEN_REFRESH_BUFFER_MS = 60 * 1000; const WORKSPACE_API_KEY_PREFIX = "b44k_"; -// Lock to prevent concurrent token refreshes +// A lock older than this belongs to a process that died mid-refresh +const REFRESH_LOCK_STALE_MS = 30 * 1000; +const REFRESH_LOCK_RETRY_MS = 50; + +// Lock to prevent concurrent token refreshes within this process let refreshPromise: Promise | null = null; export function getWorkspaceApiKeyFromEnv(): string | null { @@ -131,6 +142,34 @@ export function isTokenExpired(auth: AuthData): boolean { return Date.now() >= auth.expiresAt - TOKEN_REFRESH_BUFFER_MS; } +// Refresh tokens rotate on use, so parallel CLI processes must not refresh +// with the same one: the losers would be rejected and log the user out. +async function withRefreshLock(fn: () => Promise): Promise { + const lockPath = `${getAuthFilePath()}.lock`; + await mkdir(dirname(lockPath), { recursive: true }); + for (;;) { + try { + await (await open(lockPath, "wx")).close(); + break; + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "EEXIST") { + throw error; + } + const lock = await stat(lockPath).catch(() => null); + if (lock && Date.now() - lock.mtimeMs > REFRESH_LOCK_STALE_MS) { + await unlink(lockPath).catch(() => {}); + } else { + await sleep(REFRESH_LOCK_RETRY_MS); + } + } + } + try { + return await fn(); + } finally { + await unlink(lockPath).catch(() => {}); + } +} + export async function refreshAndSaveTokens(): Promise { // If a refresh is already in progress, wait for it if (refreshPromise) { @@ -139,20 +178,32 @@ export async function refreshAndSaveTokens(): Promise { refreshPromise = (async () => { try { - const auth = await readAuth(); - const tokenResponse = await renewAccessToken(auth.refreshToken); - - await writeAuth({ - ...auth, - accessToken: tokenResponse.accessToken, - refreshToken: tokenResponse.refreshToken, - expiresAt: Date.now() + tokenResponse.expiresIn * 1000, + const staleRefreshToken = (await readAuth()).refreshToken; + return await withRefreshLock(async () => { + const auth = await readAuth(); + if (auth.refreshToken !== staleRefreshToken) { + // Another process refreshed while we waited for the lock + return auth.accessToken; + } + + const tokenResponse = await renewAccessToken(auth.refreshToken); + await writeAuth({ + ...auth, + accessToken: tokenResponse.accessToken, + refreshToken: tokenResponse.refreshToken, + expiresAt: Date.now() + tokenResponse.expiresIn * 1000, + }); + return tokenResponse.accessToken; }); - - return tokenResponse.accessToken; - } catch { - // Refresh failed - delete auth, user needs to login again - await deleteAuth(); + } catch (error) { + // Only a rejected refresh token means the user must log in again; + // keep auth on network or server errors. + if ( + error instanceof ApiError && + (error.statusCode === 400 || error.statusCode === 401) + ) { + await deleteAuth(); + } return null; } finally { refreshPromise = null; diff --git a/packages/cli/tests/cli/authorization.spec.ts b/packages/cli/tests/cli/authorization.spec.ts index e15a1e6ec..7582bc99c 100644 --- a/packages/cli/tests/cli/authorization.spec.ts +++ b/packages/cli/tests/cli/authorization.spec.ts @@ -1,4 +1,4 @@ -import { describe, it } from "vitest"; +import { describe, expect, it } from "vitest"; import { fixture, setupCLITests } from "./testkit/index.js"; const APP_ID = "test-app-id"; @@ -65,6 +65,57 @@ describe("token refresh on 401", () => { t.expectResult(result).toContain("Entities pushed"); }); + it("keeps parallel commands logged in when they refresh with the same rotating token", async () => { + await t.givenLoggedInWithProject(fixture("with-agents")); + + // The server rotates refresh tokens: the original works once, then is rejected. + const tokenRequests: Array<() => void> = []; + let refreshTokenUsed = false; + t.api.mockRoute("POST", "/oauth/token", (_req, res) => { + const respond = () => { + if (refreshTokenUsed) { + res.status(400).json({ error: "invalid_grant" }); + return; + } + refreshTokenUsed = true; + res.json({ + access_token: "rotated-access-token", + refresh_token: "rotated-refresh-token", + expires_in: 3600, + token_type: "Bearer", + }); + }; + const releaseAll = () => { + for (const held of tokenRequests.splice(0)) held(); + }; + // Hold the first refresh until the second process asks too (or 2s pass). + tokenRequests.push(respond); + if (tokenRequests.length === 1) { + setTimeout(releaseAll, 2000); + } else { + releaseAll(); + } + }); + t.api.mockRoute("PUT", `/api/apps/${APP_ID}/agent-configs`, (req, res) => { + if (req.headers.authorization !== "Bearer rotated-access-token") { + res.status(401).json({ error: "Unauthorized" }); + return; + } + res.json({ created: [], updated: [], deleted: [] }); + }); + + const results = await Promise.all([ + t.run("agents", "push", "--yes"), + t.run("agents", "push", "--yes"), + ]); + + for (const result of results) { + t.expectResult(result).toSucceed(); + } + const auth = await t.readAuthFile(); + expect(auth?.refreshToken).toBe("rotated-refresh-token"); + }); + it("fails with actual error when token refresh also fails", async () => { await t.givenLoggedInWithProject(fixture("with-agents"));