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