diff --git a/src/commands/auth/logout.test.ts b/src/commands/auth/logout.test.ts new file mode 100644 index 000000000..5e012af65 --- /dev/null +++ b/src/commands/auth/logout.test.ts @@ -0,0 +1,152 @@ +import { afterEach, describe, expect, it, mock } from "bun:test"; + +import type { CommandContext } from "~/shell/commandContext.js"; +import type { UI } from "~/shell/ui/types.js"; +import { handleLogout } from "./logout.js"; + +afterEach(() => { + mock.restore(); +}); + +type Task = { message: string; task: () => T | Promise }; + +function makeCtx( + ui: Partial & { mode: UI["mode"] }, +): CommandContext & { ui: UI } { + return { + ui: { + gap: mock(), + intro: mock(), + info: mock(), + warn: mock(), + cancel: mock(), + outro: mock(), + output: mock(), + // Run every task so the test observes the real deletion calls. + withProgress: mock( + async (tasks: Task[], summarise?: unknown) => { + const results = []; + for (const t of tasks) results.push(await t.task()); + if (typeof summarise === "function") summarise(results); + return results; + }, + ), + ...ui, + } as unknown as UI, + configDir: "/config", + } as unknown as CommandContext & { ui: UI }; +} + +function makeDeps(args: { + stored: boolean; + env?: Record; +}) { + return { + hasStoredCredentials: mock(async () => args.stored), + deleteApiKey: mock(async () => ({ + keychain: "deleted" as const, + file: "deleted" as const, + })), + deleteTokens: mock(async () => ({ + keychain: "deleted" as const, + file: "deleted" as const, + })), + env: args.env ?? {}, + }; +} + +// A factory, not a shared constant: a module-level mock would carry its call +// record from one test into the next. +function confirmed() { + return { + mode: "human" as const, + confirm: mock(async () => ({ ok: true as const, value: true })), + }; +} + +describe("handleLogout", () => { + it("clears browser tokens as well as the stored API key", async () => { + const ctx = makeCtx(confirmed()); + const deps = makeDeps({ stored: true }); + + await handleLogout(ctx, deps); + + expect(deps.deleteApiKey).toHaveBeenCalledTimes(1); + expect(deps.deleteTokens).toHaveBeenCalledTimes(1); + }); + + // The bug this replaces: deletion used to sit behind resolveApiKey, which + // refreshes a browser session over the network. Offline, or once WorkOS had + // rotated the refresh token away, logout reported "not authenticated" and + // left the credentials on disk. + it("clears credentials that can no longer be resolved", async () => { + const ctx = makeCtx(confirmed()); + const deps = makeDeps({ stored: true }); + + await handleLogout(ctx, deps); + + expect(ctx.ui.info).not.toHaveBeenCalled(); + expect(deps.deleteApiKey).toHaveBeenCalledTimes(1); + expect(deps.deleteTokens).toHaveBeenCalledTimes(1); + }); + + it("does not consult the network to decide whether to delete", async () => { + const ctx = makeCtx(confirmed()); + const deps = makeDeps({ stored: true }); + + await handleLogout(ctx, deps); + + expect(deps.hasStoredCredentials).toHaveBeenCalledTimes(1); + expect(deps.hasStoredCredentials).toHaveBeenCalledWith( + "/config", + undefined, + ); + }); + + it("deletes nothing when there is nothing stored", async () => { + const ctx = makeCtx({ mode: "human" }); + const deps = makeDeps({ stored: false }); + + await handleLogout(ctx, deps); + + expect(deps.deleteApiKey).not.toHaveBeenCalled(); + expect(deps.deleteTokens).not.toHaveBeenCalled(); + }); + + it("warns that an environment variable cannot be removed", async () => { + const ctx = makeCtx(confirmed()); + const deps = makeDeps({ + stored: false, + env: { QAWOLF_API_KEY: "qaw_env" }, + }); + + await handleLogout(ctx, deps); + + expect(ctx.ui.warn).toHaveBeenCalled(); + }); + + it("still clears storage when only an environment key is set", async () => { + const ctx = makeCtx(confirmed()); + const deps = makeDeps({ + stored: false, + env: { QAWOLF_API_KEY: "qaw_env" }, + }); + + await handleLogout(ctx, deps); + + expect(deps.deleteTokens).toHaveBeenCalledTimes(1); + }); + + it("deletes nothing when the confirmation is declined", async () => { + const ctx = makeCtx({ + mode: "human", + confirm: mock(async () => ({ ok: true as const, value: false })), + }); + const deps = makeDeps({ stored: true }); + + await handleLogout(ctx, deps); + + expect(deps.deleteApiKey).not.toHaveBeenCalled(); + expect(deps.deleteTokens).not.toHaveBeenCalled(); + }); +}); diff --git a/src/commands/auth/logout.ts b/src/commands/auth/logout.ts index 629002b03..066d30876 100644 --- a/src/commands/auth/logout.ts +++ b/src/commands/auth/logout.ts @@ -1,21 +1,49 @@ -import { deleteApiKey, resolveApiKey } from "~/domains/auth/index.js"; -import { - type CommandContext, - type CommandResult, -} from "~/shell/commandContext.js"; +import { deleteApiKey as realDeleteApiKey } from "~/domains/auth/index.js"; +import { deleteTokens as realDeleteTokens } from "~/domains/auth/store/deleteTokens.js"; +import { hasStoredCredentials as realHasStoredCredentials } from "~/domains/auth/store/index.js"; +import type { CommandContext, CommandResult } from "~/shell/commandContext.js"; import { authMessages } from "~/core/messages/index.js"; +type LogoutDeps = { + hasStoredCredentials?: ( + configDir: string, + fs: CommandContext["fs"], + ) => Promise; + deleteApiKey?: ( + configDir: string, + fs: CommandContext["fs"], + ) => Promise; + deleteTokens?: ( + configDir: string, + fs: CommandContext["fs"], + ) => Promise; + env?: Record; +}; + export async function handleLogout( ctx: CommandContext, + deps: LogoutDeps = {}, ): Promise { - const resolved = await resolveApiKey(ctx.configDir, ctx.fs); + const hasStoredCredentials = + deps.hasStoredCredentials ?? realHasStoredCredentials; + const deleteApiKey = deps.deleteApiKey ?? realDeleteApiKey; + const deleteTokens = deps.deleteTokens ?? realDeleteTokens; + const env = deps.env ?? process.env; + + // Storage is asked directly rather than through resolveApiKey. Resolving a + // browser session refreshes it over the network, so being offline or holding + // a refresh token WorkOS has already rotated away would report "not + // authenticated" and leave the credentials in place — the one case where + // clearing them matters most. + const envKey = env["QAWOLF_API_KEY"]?.trim(); + const stored = await hasStoredCredentials(ctx.configDir, ctx.fs); - if (!resolved) { + if (!envKey && !stored) { ctx.ui.info(authMessages.logout.notAuthenticated); return; } - if (resolved.source === "env") { + if (envKey) { ctx.ui.warn(authMessages.logout.envVarWarning); } @@ -34,7 +62,15 @@ export async function handleLogout( [ { message: authMessages.logout.deleting, - task: () => deleteApiKey(ctx.configDir, ctx.fs), + // Both credential kinds go, whichever one is present. Clearing only the + // one in use would leave the other to take over on the next command, + // so "logged out" would not be true. + task: async () => { + await Promise.all([ + deleteApiKey(ctx.configDir, ctx.fs), + deleteTokens(ctx.configDir, ctx.fs), + ]); + }, }, ], () => authMessages.logout.credentialsRemoved, diff --git a/src/domains/auth/resolve.test.ts b/src/domains/auth/resolve.test.ts index fa3a543e8..d1ca78f1a 100644 --- a/src/domains/auth/resolve.test.ts +++ b/src/domains/auth/resolve.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, mock } from "bun:test"; import { makeMemoryFs } from "~/shell/fs.testUtils.js"; +import type { OauthToken } from "./resolveOauthToken.js"; import type { LoadApiKeyResult } from "./types.js"; import { requireApiKey, resolveApiKey } from "./resolve.js"; @@ -10,99 +11,167 @@ afterEach(() => { const memFs = makeMemoryFs(); +/** Browser sign-in holds nothing, so only the API key paths are exercised. */ +const noOauth = async (): Promise => undefined; + +function mockLoad(result?: LoadApiKeyResult) { + const fn = mock<(configDir: string) => Promise>(); + if (result) fn.mockResolvedValue(result); + return fn; +} + describe("resolveApiKey", () => { it("returns env var when QAWOLF_API_KEY is set", async () => { - const mockLoadApiKey = - mock<(configDir: string) => Promise>(); + const loadApiKey = mockLoad(); const result = await resolveApiKey("/tmp/config", memFs, { - loadApiKey: mockLoadApiKey, + loadApiKey, + resolveOauth: noOauth, env: { QAWOLF_API_KEY: "qaw_test_key" }, }); - expect(result).toEqual({ key: "qaw_test_key", source: "env" }); - expect(mockLoadApiKey).not.toHaveBeenCalled(); + expect(result).toEqual({ + key: "qaw_test_key", + source: "env", + }); + expect(loadApiKey).not.toHaveBeenCalled(); }); it("trims whitespace from env var", async () => { - const mockLoadApiKey = - mock<(configDir: string) => Promise>(); + const loadApiKey = mockLoad(); const result = await resolveApiKey("/tmp/config", memFs, { - loadApiKey: mockLoadApiKey, + loadApiKey, + resolveOauth: noOauth, env: { QAWOLF_API_KEY: " qaw_test_key " }, }); - expect(result).toEqual({ key: "qaw_test_key", source: "env" }); - expect(mockLoadApiKey).not.toHaveBeenCalled(); + expect(result).toEqual({ + key: "qaw_test_key", + source: "env", + }); + expect(loadApiKey).not.toHaveBeenCalled(); }); it("skips whitespace-only env var", async () => { - const mockLoadApiKey = mock< - (configDir: string) => Promise - >().mockResolvedValue({ found: false }); + const loadApiKey = mockLoad({ found: false }); const result = await resolveApiKey("/tmp/config", memFs, { - loadApiKey: mockLoadApiKey, + loadApiKey, + resolveOauth: noOauth, env: { QAWOLF_API_KEY: " " }, }); expect(result).toBeUndefined(); - expect(mockLoadApiKey).toHaveBeenCalledWith("/tmp/config"); + expect(loadApiKey).toHaveBeenCalledWith("/tmp/config"); }); it("returns stored key when env var is not set", async () => { - const mockLoadApiKey = mock< - (configDir: string) => Promise - >().mockResolvedValue({ - found: true, + const result = await resolveApiKey("/tmp/config", memFs, { + loadApiKey: mockLoad({ + found: true, + key: "qaw_stored", + source: "keychain", + }), + resolveOauth: noOauth, + env: {}, + }); + + expect(result).toEqual({ key: "qaw_stored", source: "keychain", }); + }); + it("returns undefined when nothing found", async () => { const result = await resolveApiKey("/tmp/config", memFs, { - loadApiKey: mockLoadApiKey, + loadApiKey: mockLoad({ found: false }), + resolveOauth: noOauth, env: {}, }); - expect(result).toEqual({ key: "qaw_stored", source: "keychain" }); + expect(result).toBeUndefined(); }); - it("returns undefined when nothing found", async () => { - const mockLoadApiKey = mock< - (configDir: string) => Promise - >().mockResolvedValue({ found: false }); + it("falls back to browser sign-in when no API key is stored", async () => { + const result = await resolveApiKey("/tmp/config", memFs, { + loadApiKey: mockLoad({ found: false }), + resolveOauth: async () => ({ + key: "access_abc", + email: "person@example.com", + }), + env: {}, + }); + + expect(result).toEqual({ + key: "access_abc", + source: "browser", + }); + }); + + it("prefers a stored API key over browser sign-in for its team scope", async () => { + const resolveOauth = mock(async () => ({ + key: "access_abc", + email: "person@example.com", + })); const result = await resolveApiKey("/tmp/config", memFs, { - loadApiKey: mockLoadApiKey, + loadApiKey: mockLoad({ + found: true, + key: "qaw_stored", + source: "keychain", + }), + resolveOauth, env: {}, }); - expect(result).toBeUndefined(); + expect(result).toEqual({ + key: "qaw_stored", + source: "keychain", + }); + expect(resolveOauth).not.toHaveBeenCalled(); + }); + + it("prefers the environment variable over browser sign-in", async () => { + const resolveOauth = mock(async () => ({ + key: "access_abc", + email: "person@example.com", + })); + + const result = await resolveApiKey("/tmp/config", memFs, { + loadApiKey: mockLoad(), + resolveOauth, + env: { QAWOLF_API_KEY: "qaw_env" }, + }); + + expect(result).toEqual({ + key: "qaw_env", + source: "env", + }); + expect(resolveOauth).not.toHaveBeenCalled(); }); }); describe("requireApiKey", () => { it("returns the resolved ApiKeyResult when a key exists", async () => { - const mockLoad = mock<(configDir: string) => Promise>(); - const result = await requireApiKey("/tmp/config", memFs, { - loadApiKey: mockLoad, + loadApiKey: mockLoad(), + resolveOauth: noOauth, env: { QAWOLF_API_KEY: "qaw_key" }, }); - expect(result).toEqual({ key: "qaw_key", source: "env" }); + expect(result).toEqual({ + key: "qaw_key", + source: "env", + }); }); it("throws the standard message when no key is found", async () => { - const mockLoad = mock< - (configDir: string) => Promise - >().mockResolvedValue({ found: false }); - let caughtError: unknown; try { await requireApiKey("/tmp/config", memFs, { - loadApiKey: mockLoad, + loadApiKey: mockLoad({ found: false }), + resolveOauth: noOauth, env: {}, }); } catch (e) { diff --git a/src/domains/auth/resolve.ts b/src/domains/auth/resolve.ts index 64828f7f3..d32e2c1fd 100644 --- a/src/domains/auth/resolve.ts +++ b/src/domains/auth/resolve.ts @@ -1,22 +1,69 @@ import { Entry } from "@napi-rs/keyring"; +import { authErrorMessages } from "~/core/messages/authErrors.js"; import type { Fs } from "~/shell/fs.js"; +import { refreshAccessToken } from "~/shell/workos/refreshAccessToken.js"; +import { resolveWorkosConfig } from "~/shell/workos/config.js"; import { loadApiKey as realLoadApiKey } from "./store/index.js"; +import { loadTokens as realLoadTokens } from "./store/loadTokens.js"; +import { saveTokens as realSaveTokens } from "./store/saveTokens.js"; +import { + type OauthToken, + resolveOauthToken, + type ResolveOauthTokenDeps, +} from "./resolveOauthToken.js"; import type { ApiKeyResult, LoadApiKeyResult } from "./types.js"; type ResolveApiKeyDeps = { loadApiKey: (configDir: string) => Promise; + resolveOauth: (configDir: string) => Promise; env: Record; }; +function makeOauthDeps(fs: Fs): ResolveOauthTokenDeps { + return { + loadTokens: (configDir) => + realLoadTokens(configDir, { EntryClass: Entry, fs }), + // The stored session names its issuing client, so renewing a token asks + // the deployment nothing. + refreshTokens: async ({ refreshToken, organizationId, clientId }) => { + const config = resolveWorkosConfig(clientId); + if (!config.configured) { + // Nothing to retry: the session records no client to redeem against. + return { + ok: false, + error: authErrorMessages.workos.noClientForSession, + retryable: false, + }; + } + return refreshAccessToken(refreshToken, organizationId, { + fetch: globalThis.fetch, + baseUrl: config.baseUrl, + clientId: config.clientId, + }); + }, + saveTokens: (configDir, tokens) => realSaveTokens(configDir, tokens, fs), + now: () => Date.now(), + }; +} + function makeDefaultDeps(fs: Fs): ResolveApiKeyDeps { return { loadApiKey: (configDir) => realLoadApiKey(configDir, { EntryClass: Entry, fs }), + resolveOauth: (configDir) => + resolveOauthToken(configDir, makeOauthDeps(fs)), env: process.env, }; } +/** + * Finds the credential the CLI should present, highest precedence first: the + * environment variable, then a stored API key, then browser sign-in. + * + * An API key outranks browser sign-in because it carries team scope that a user + * token does not, so someone holding both keeps the broader access. + */ export async function resolveApiKey( configDir: string, fs: Fs, @@ -33,6 +80,11 @@ export async function resolveApiKey( return { key: stored.key, source: stored.source }; } + const oauth = await resolvedDeps.resolveOauth(configDir); + if (oauth) { + return { key: oauth.key, source: "browser" }; + } + return undefined; } diff --git a/src/domains/auth/resolveOauthToken.race.test.ts b/src/domains/auth/resolveOauthToken.race.test.ts new file mode 100644 index 000000000..6c5f86f4f --- /dev/null +++ b/src/domains/auth/resolveOauthToken.race.test.ts @@ -0,0 +1,156 @@ +import { describe, expect, it, mock } from "bun:test"; + +import { resolveOauthToken } from "./resolveOauthToken.js"; +import type { LoadTokensResult, StoredSession } from "./types.js"; + +const nowMs = 1_700_000_000_000; + +const spent: StoredSession = { + accessToken: "access_old", + refreshToken: "refresh_stale", + // Already past the margin, so resolving it always attempts a refresh. + expiresAt: nowMs - 1, + email: "person@example.com", + organizationId: "org_1", + clientId: "client_1", +}; + +function found(tokens: StoredSession): LoadTokensResult { + return { found: true, tokens, source: "keychain" }; +} + +const revoked = async () => ({ + ok: false as const, + error: "invalid_grant", + retryable: false, +}); + +describe("resolveOauthToken when a refresh does not succeed", () => { + // A lost refresh race must not read as "signed out". WorkOS rotates on every + // exchange, so whichever process won has already written a pair this one can + // use. + it("adopts a pair another command installed while this refresh failed", async () => { + const winner: StoredSession = { + ...spent, + accessToken: "access_from_winner", + refreshToken: "refresh_rotated", + expiresAt: nowMs + 600_000, + }; + const loadTokens = mock(async () => found(winner)); + loadTokens.mockResolvedValueOnce(found(spent)); + + const result = await resolveOauthToken("/config", { + loadTokens, + refreshTokens: revoked, + saveTokens: async () => {}, + now: () => nowMs, + }); + + expect(result).toEqual({ + key: "access_from_winner", + email: "person@example.com", + }); + expect(loadTokens).toHaveBeenCalledTimes(2); + }); + + // The store is shared. A pair written there by another person's sign-in is + // not this command's session, whatever its refresh token says. + it("does not adopt another account's pair", async () => { + const theirs: StoredSession = { + ...spent, + accessToken: "access_theirs", + refreshToken: "refresh_theirs", + expiresAt: nowMs + 600_000, + email: "someone-else@example.com", + }; + const loadTokens = mock(async () => found(theirs)); + loadTokens.mockResolvedValueOnce(found(spent)); + + const result = await resolveOauthToken("/config", { + loadTokens, + refreshTokens: revoked, + saveTokens: async () => {}, + now: () => nowMs, + }); + + expect(result).toBeUndefined(); + }); + + it("does not adopt a replacement pair that has itself already expired", async () => { + const stale: StoredSession = { + ...spent, + accessToken: "access_from_winner", + refreshToken: "refresh_rotated", + expiresAt: nowMs - 1, + }; + const loadTokens = mock(async () => found(stale)); + loadTokens.mockResolvedValueOnce(found(spent)); + + const result = await resolveOauthToken("/config", { + loadTokens, + refreshTokens: revoked, + saveTokens: async () => {}, + now: () => nowMs, + }); + + expect(result).toBeUndefined(); + }); + + // The margin is a head start, not an expiry: the token in hand still works, + // so a dropped packet inside it must not end the session. + it("keeps an unexpired token when the refresh fails transiently", async () => { + const insideMargin: StoredSession = { + ...spent, + accessToken: "access_still_good", + expiresAt: nowMs + 5_000, + }; + + const result = await resolveOauthToken("/config", { + loadTokens: async () => found(insideMargin), + refreshTokens: async () => ({ + ok: false as const, + error: "socket hang up", + retryable: true, + }), + saveTokens: async () => {}, + now: () => nowMs, + }); + + expect(result?.key).toBe("access_still_good"); + }); + + // A write that fails costs the next command a sign-in either way; failing + // this one as well would only take away a credential that works. + it("returns the refreshed token even when it cannot be persisted", async () => { + const result = await resolveOauthToken("/config", { + loadTokens: async () => found(spent), + refreshTokens: async () => ({ + ok: true as const, + value: { + accessToken: "access_new", + refreshToken: "refresh_new", + expiresAt: nowMs + 600_000, + email: "person@example.com", + organizationId: "org_1", + }, + }), + saveTokens: async () => { + throw Object.assign(Error("permission denied"), { code: "EACCES" }); + }, + now: () => nowMs, + }); + + expect(result?.key).toBe("access_new"); + }); + + it("still reports nothing when the stored pair is unchanged", async () => { + const result = await resolveOauthToken("/config", { + loadTokens: async () => found(spent), + refreshTokens: revoked, + saveTokens: async () => {}, + now: () => nowMs, + }); + + expect(result).toBeUndefined(); + }); +}); diff --git a/src/domains/auth/resolveOauthToken.test.ts b/src/domains/auth/resolveOauthToken.test.ts new file mode 100644 index 000000000..1f1a87967 --- /dev/null +++ b/src/domains/auth/resolveOauthToken.test.ts @@ -0,0 +1,181 @@ +import { describe, expect, it, mock } from "bun:test"; + +import type { DeviceTokens } from "~/core/deviceAuth/types.js"; +import { expiryMarginMs, resolveOauthToken } from "./resolveOauthToken.js"; +import type { LoadTokensResult, StoredSession } from "./types.js"; + +const nowMs = 1_700_000_000_000; + +const stored: StoredSession = { + accessToken: "access_old", + refreshToken: "refresh_old", + expiresAt: nowMs + 60_000, + email: "person@example.com", + organizationId: "org_1", + clientId: "client_1", +}; + +const refreshed: DeviceTokens = { + accessToken: "access_new", + refreshToken: "refresh_new", + expiresAt: nowMs + 600_000, + email: "person@example.com", + organizationId: "org_1", +}; + +function makeDeps( + loadResult: LoadTokensResult, + refreshResult: + | { ok: true; value: DeviceTokens } + | { ok: false; error: string; retryable: boolean } = { + ok: true, + value: refreshed, + }, +) { + const saveTokens = mock(async (_configDir: string, _tokens: DeviceTokens) => { + // storage is asserted through the spy, not through a filesystem + }); + const refreshTokens = mock( + async (_args: { + refreshToken: string; + organizationId: string | undefined; + clientId: string | undefined; + }) => refreshResult, + ); + return { + saveTokens, + refreshTokens, + deps: { + loadTokens: async () => loadResult, + refreshTokens, + saveTokens, + now: () => nowMs, + }, + }; +} + +describe("resolveOauthToken", () => { + it("uses the stored access token while it remains valid", async () => { + const { deps, refreshTokens } = makeDeps({ + found: true, + tokens: stored, + source: "keychain", + }); + + const result = await resolveOauthToken("/config", deps); + + expect(result).toEqual({ + key: "access_old", + email: "person@example.com", + }); + expect(refreshTokens).not.toHaveBeenCalled(); + }); + + it("refreshes an expired access token", async () => { + const { deps, refreshTokens } = makeDeps({ + found: true, + tokens: { ...stored, expiresAt: nowMs - 1 }, + source: "keychain", + }); + + const result = await resolveOauthToken("/config", deps); + + expect(result).toEqual({ + key: "access_new", + email: "person@example.com", + }); + expect(refreshTokens).toHaveBeenCalledWith({ + refreshToken: "refresh_old", + organizationId: "org_1", + clientId: "client_1", + }); + }); + + it("refreshes a token that expires inside the safety margin", async () => { + const { deps, refreshTokens } = makeDeps({ + found: true, + tokens: { ...stored, expiresAt: nowMs + expiryMarginMs - 1 }, + source: "keychain", + }); + + await resolveOauthToken("/config", deps); + + expect(refreshTokens).toHaveBeenCalledWith({ + refreshToken: "refresh_old", + organizationId: "org_1", + clientId: "client_1", + }); + }); + + it("refreshes when the stored expiry is unknown", async () => { + const { deps, refreshTokens } = makeDeps({ + found: true, + tokens: { ...stored, expiresAt: undefined }, + source: "keychain", + }); + + await resolveOauthToken("/config", deps); + + expect(refreshTokens).toHaveBeenCalledWith({ + refreshToken: "refresh_old", + organizationId: "org_1", + clientId: "client_1", + }); + }); + + it("asks for no particular organization when none was stored", async () => { + const { deps, refreshTokens } = makeDeps({ + found: true, + tokens: { ...stored, expiresAt: nowMs - 1, organizationId: undefined }, + source: "keychain", + }); + + await resolveOauthToken("/config", deps); + + expect(refreshTokens).toHaveBeenCalledWith({ + refreshToken: "refresh_old", + organizationId: undefined, + clientId: "client_1", + }); + }); + + it("persists the rotated refresh token, not just the access token", async () => { + const { deps, saveTokens } = makeDeps({ + found: true, + tokens: { ...stored, expiresAt: nowMs - 1 }, + source: "keychain", + }); + + await resolveOauthToken("/config", deps); + + expect(saveTokens).toHaveBeenCalledWith("/config", { + ...refreshed, + clientId: "client_1", + }); + }); + + it("returns undefined when no tokens are stored", async () => { + const { deps, refreshTokens } = makeDeps({ found: false }); + + const result = await resolveOauthToken("/config", deps); + + expect(result).toBeUndefined(); + expect(refreshTokens).not.toHaveBeenCalled(); + }); + + it("returns undefined when the refresh token has been revoked", async () => { + const { deps, saveTokens } = makeDeps( + { + found: true, + tokens: { ...stored, expiresAt: nowMs - 1 }, + source: "file", + }, + { ok: false, error: "token revoked", retryable: false }, + ); + + const result = await resolveOauthToken("/config", deps); + + expect(result).toBeUndefined(); + expect(saveTokens).not.toHaveBeenCalled(); + }); +}); diff --git a/src/domains/auth/resolveOauthToken.ts b/src/domains/auth/resolveOauthToken.ts new file mode 100644 index 000000000..28179c8f9 --- /dev/null +++ b/src/domains/auth/resolveOauthToken.ts @@ -0,0 +1,109 @@ +import type { DeviceTokens } from "~/core/deviceAuth/types.js"; +import type { LoadTokensResult, StoredSession } from "./types.js"; + +/** + * A token valid for another second will have expired by the time a slow + * request reaches the API, and the resulting 401 looks like a bug. + */ +export const expiryMarginMs = 30_000; + +export type ResolveOauthTokenDeps = { + loadTokens: (configDir: string) => Promise; + refreshTokens: (args: { + refreshToken: string; + organizationId: string | undefined; + clientId: string | undefined; + }) => Promise< + | { ok: true; value: DeviceTokens } + | { ok: false; error: string; retryable: boolean } + >; + saveTokens: (configDir: string, tokens: StoredSession) => Promise; + now: () => number; +}; + +export type OauthToken = { key: string; email: string }; + +/** + * Undefined whenever a token cannot be produced. A failed refresh means "sign + * in again", which the caller reports as not authenticated rather than as an + * error. + */ +export async function resolveOauthToken( + configDir: string, + deps: ResolveOauthTokenDeps, +): Promise { + const stored = await deps.loadTokens(configDir); + if (!stored.found) return undefined; + + const { tokens } = stored; + const expiresAt = tokens.expiresAt; + const isFresh = + expiresAt !== undefined && expiresAt - expiryMarginMs > deps.now(); + + if (isFresh) { + return { key: tokens.accessToken, email: tokens.email }; + } + + // Pin the refresh to the organization already in use. Without it WorkOS is + // free to choose again, so a session could silently move between + // organizations partway through a run of commands. + const refreshed = await deps.refreshTokens({ + refreshToken: tokens.refreshToken, + organizationId: tokens.organizationId, + clientId: tokens.clientId, + }); + if (!refreshed.ok) { + // The margin is a head start, not an expiry. A dropped packet inside it + // leaves a token that still works, so ending the session over one would + // sign someone out mid-command for nothing. + if ( + refreshed.retryable && + expiresAt !== undefined && + expiresAt > deps.now() + ) { + return { key: tokens.accessToken, email: tokens.email }; + } + + // Another command may have refreshed while this one was in flight — the + // workers of a single `flows run` all resolve at once. Adopt whatever is on + // disk before reporting a dead session: the winner's pair is valid for this + // process too, and reporting "not authenticated" over a lost race sends + // someone to sign in again for nothing. + // Only this session's replacement is adopted: the same account, client and + // organization, and a pair that has not itself lapsed. The store is + // shared, so what landed there may be another person's sign-in. + const current = await deps.loadTokens(configDir); + if ( + current.found && + current.tokens.refreshToken !== tokens.refreshToken && + current.tokens.email === tokens.email && + current.tokens.organizationId === tokens.organizationId && + current.tokens.clientId === tokens.clientId && + current.tokens.expiresAt !== undefined && + current.tokens.expiresAt > deps.now() + ) { + return { + key: current.tokens.accessToken, + email: current.tokens.email, + }; + } + return undefined; + } + + // Refresh tokens rotate, so the whole pair has to land in storage. Persisting + // only the access token would spend the refresh token and lock the next + // refresh out. + try { + await deps.saveTokens(configDir, { + ...refreshed.value, + clientId: tokens.clientId, + }); + } catch { + // The token in hand works for this command. Failing here as well would cost + // the caller a working credential and change nothing: the refresh already + // spent the stored token, so the next command has to sign in again whether + // this one succeeds or not. + } + + return { key: refreshed.value.accessToken, email: refreshed.value.email }; +} diff --git a/src/domains/auth/store/constants.ts b/src/domains/auth/store/constants.ts index c72975f2d..807c083b4 100644 --- a/src/domains/auth/store/constants.ts +++ b/src/domains/auth/store/constants.ts @@ -1,3 +1,8 @@ export const service = "qawolf-cli"; export const account = "api-key"; export const credentialsFile = "credentials.json"; + +// A separate entry, not new fields on the api-key record: moving that record to +// a JSON payload would strand every key already in a keychain. +export const tokensAccount = "oauth-tokens"; +export const tokensFile = "tokens.json"; diff --git a/src/domains/auth/store/delete.ts b/src/domains/auth/store/delete.ts index 5b5d5fc0a..5c5ec9897 100644 --- a/src/domains/auth/store/delete.ts +++ b/src/domains/auth/store/delete.ts @@ -1,12 +1,13 @@ +import { isNoEntError } from "~/core/errors.js"; import type { Fs } from "~/shell/fs.js"; import { join } from "node:path"; import { Entry } from "@napi-rs/keyring"; import { account, credentialsFile, service } from "./constants.js"; -import type { DeleteApiKeyResult } from "./types.js"; +import type { DeleteCredentialResult } from "./types.js"; -function deleteFromKeychain(): DeleteApiKeyResult["keychain"] { +function deleteFromKeychain(): DeleteCredentialResult["keychain"] { try { new Entry(service, account).deletePassword(); return "deleted"; @@ -18,19 +19,23 @@ function deleteFromKeychain(): DeleteApiKeyResult["keychain"] { async function deleteFromFile( configDir: string, fs: Fs, -): Promise { +): Promise { try { await fs.unlink(join(configDir, credentialsFile)); return "deleted"; - } catch { - return "not-found"; + } catch (err: unknown) { + // Only a missing file is "not-found". Swallowing a permission or I/O error + // would let logout report "Credentials removed" over a credential that is + // still on disk. + if (isNoEntError(err)) return "not-found"; + throw err; } } export async function deleteApiKey( configDir: string, fs: Fs, -): Promise { +): Promise { const [keychain, file] = await Promise.all([ Promise.resolve(deleteFromKeychain()), deleteFromFile(configDir, fs), diff --git a/src/domains/auth/store/deleteTokens.ts b/src/domains/auth/store/deleteTokens.ts new file mode 100644 index 000000000..8ba046623 --- /dev/null +++ b/src/domains/auth/store/deleteTokens.ts @@ -0,0 +1,44 @@ +import { isNoEntError } from "~/core/errors.js"; +import type { Fs } from "~/shell/fs.js"; +import { join } from "node:path"; + +import { Entry } from "@napi-rs/keyring"; + +import { service, tokensAccount, tokensFile } from "./constants.js"; +import type { DeleteCredentialResult } from "./types.js"; + +function deleteFromKeychain(): DeleteCredentialResult["keychain"] { + try { + new Entry(service, tokensAccount).deletePassword(); + return "deleted"; + } catch { + return "unavailable"; + } +} + +async function deleteFromFile( + configDir: string, + fs: Fs, +): Promise { + try { + await fs.unlink(join(configDir, tokensFile)); + return "deleted"; + } catch (err: unknown) { + // Only a missing file is "not-found". Swallowing a permission or I/O error + // would let logout report "Credentials removed" over a credential that is + // still on disk. + if (isNoEntError(err)) return "not-found"; + throw err; + } +} + +export async function deleteTokens( + configDir: string, + fs: Fs, +): Promise { + const [keychain, file] = await Promise.all([ + Promise.resolve(deleteFromKeychain()), + deleteFromFile(configDir, fs), + ]); + return { keychain, file }; +} diff --git a/src/domains/auth/store/hasStoredCredentials.ts b/src/domains/auth/store/hasStoredCredentials.ts new file mode 100644 index 000000000..75dc019a0 --- /dev/null +++ b/src/domains/auth/store/hasStoredCredentials.ts @@ -0,0 +1,63 @@ +import { join } from "node:path"; + +import { Entry } from "@napi-rs/keyring"; + +import { isNoEntError } from "~/core/errors.js"; +import type { Fs } from "~/shell/fs.js"; +import { + account, + credentialsFile, + service, + tokensAccount, + tokensFile, +} from "./constants.js"; + +async function fileExists( + path: string, + fs: Pick, +): Promise { + try { + await fs.readFile(path); + return true; + } catch (err: unknown) { + // Anything other than "missing" means something is there that cannot be + // read — a truncated write, a permission problem — which logout still has + // to remove. + return !isNoEntError(err); + } +} + +function keychainHolds(entryAccount: string): boolean { + try { + return Boolean(new Entry(service, entryAccount).getPassword()); + } catch { + // No usable keychain on this machine, so nothing of ours is in it. + return false; + } +} + +/** + * Whether this machine holds a credential at all. + * + * Presence, not validity: a payload that will not parse still has to be + * cleared, so this asks whether the file or the keychain entry is there rather + * than whether it loads. `resolveApiKey` cannot stand in for either question — + * it refreshes a browser session over the network, so an offline machine reads + * as "nothing stored" while the credentials are still on disk. + */ +export async function hasStoredCredentials( + configDir: string, + fs: Pick, +): Promise { + const [apiKeyFile, tokenFile] = await Promise.all([ + fileExists(join(configDir, credentialsFile), fs), + fileExists(join(configDir, tokensFile), fs), + ]); + + return ( + apiKeyFile || + tokenFile || + keychainHolds(account) || + keychainHolds(tokensAccount) + ); +} diff --git a/src/domains/auth/store/index.ts b/src/domains/auth/store/index.ts index 45ea80a0e..946045933 100644 --- a/src/domains/auth/store/index.ts +++ b/src/domains/auth/store/index.ts @@ -1,3 +1,4 @@ export { deleteApiKey } from "./delete.js"; +export { hasStoredCredentials } from "./hasStoredCredentials.js"; export { loadApiKey } from "./load.js"; export { saveApiKey } from "./save.js"; diff --git a/src/domains/auth/store/loadTokens.test.ts b/src/domains/auth/store/loadTokens.test.ts new file mode 100644 index 000000000..a81846f16 --- /dev/null +++ b/src/domains/auth/store/loadTokens.test.ts @@ -0,0 +1,119 @@ +import { describe, expect, it } from "bun:test"; + +import type { StoredSession } from "~/domains/auth/types.js"; +import type { Fs } from "~/shell/fs.js"; +import { makeMemoryFs } from "~/shell/fs.testUtils.js"; + +import { loadTokens } from "./loadTokens.js"; +import { + makeEntryClass, + makeThrowingEntryClass, + tokens, +} from "./tokens.testUtils.js"; + +describe("loadTokens", () => { + it("returns tokens held in the keychain", async () => { + const EntryClass = makeEntryClass(() => JSON.stringify(tokens)); + + const result = await loadTokens("/config", { + EntryClass, + fs: makeMemoryFs(), + }); + + expect(result).toEqual({ found: true, tokens, source: "keychain" }); + }); + + it("falls back to the token file when the keychain throws", async () => { + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + await memFs.writeFile("/config/tokens.json", JSON.stringify(tokens)); + + const result = await loadTokens("/config", { + EntryClass: makeThrowingEntryClass("keychain locked"), + fs: memFs, + }); + + expect(result).toEqual({ found: true, tokens, source: "file" }); + }); + + it("round-trips the organization, so a refresh can pin it", async () => { + const EntryClass = makeEntryClass(() => JSON.stringify(tokens)); + + const result = await loadTokens("/config", { + EntryClass, + fs: makeMemoryFs(), + }); + + if (!result.found) throw Error("expected stored tokens"); + expect(result.tokens.organizationId).toBe("org_1"); + }); + + it("round-trips tokens whose expiry is unknown", async () => { + const withoutExpiry: StoredSession = { ...tokens, expiresAt: undefined }; + const EntryClass = makeEntryClass(() => JSON.stringify(withoutExpiry)); + + const result = await loadTokens("/config", { + EntryClass, + fs: makeMemoryFs(), + }); + + expect(result).toEqual({ + found: true, + tokens: withoutExpiry, + source: "keychain", + }); + }); + + it("reports not found when neither store holds tokens", async () => { + const result = await loadTokens("/config", { + EntryClass: makeEntryClass(() => ""), + fs: makeMemoryFs(), + }); + + expect(result.found).toBe(false); + }); + + it("reports not found when the stored payload fails validation", async () => { + const EntryClass = makeEntryClass(() => + JSON.stringify({ accessToken: "only-this" }), + ); + + const result = await loadTokens("/config", { + EntryClass, + fs: makeMemoryFs(), + }); + + expect(result.found).toBe(false); + }); + + // The shape a truncated or half-flushed write actually leaves behind. The + // parse throws out of parseTokens rather than returning, so this covers the + // catch that keeps a corrupt store from crashing the command. + it("reports a store holding bytes that are not JSON", async () => { + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + await memFs.writeFile("/config/tokens.json", '{"accessToken": "trunc'); + + const result = await loadTokens("/config", { + EntryClass: makeEntryClass(() => ""), + fs: memFs as unknown as Fs, + }); + + expect(result.found).toBe(false); + if (result.found) return; + expect(result.errors?.file).toBeDefined(); + }); + + it("reports a store holding JSON that is not a session", async () => { + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + await memFs.writeFile("/config/tokens.json", '{"nonsense": true}'); + + const result = await loadTokens("/config", { + EntryClass: makeEntryClass(() => ""), + fs: memFs as unknown as Fs, + }); + + expect(result.found).toBe(false); + }); +}); diff --git a/src/domains/auth/store/loadTokens.ts b/src/domains/auth/store/loadTokens.ts new file mode 100644 index 000000000..45b869662 --- /dev/null +++ b/src/domains/auth/store/loadTokens.ts @@ -0,0 +1,58 @@ +import type { Fs } from "~/shell/fs.js"; +import { join } from "node:path"; + +import type { Entry } from "@napi-rs/keyring"; + +import { errorMessage } from "~/core/errors.js"; +import type { LoadTokensResult, StoredSession } from "~/domains/auth/types.js"; +import { service, tokensAccount, tokensFile } from "./constants.js"; +import { oauthTokensSchema } from "./types.js"; + +type LoadTokensDeps = { + EntryClass: typeof Entry; + fs: Pick; +}; + +const invalidPayload = "Invalid stored token format"; + +function parseTokens(raw: string): StoredSession | undefined { + const parsed = oauthTokensSchema.safeParse(JSON.parse(raw)); + if (!parsed.success) return undefined; + return { + accessToken: parsed.data.accessToken, + refreshToken: parsed.data.refreshToken, + expiresAt: parsed.data.expiresAt, + email: parsed.data.email, + organizationId: parsed.data.organizationId, + clientId: parsed.data.clientId, + }; +} + +export async function loadTokens( + configDir: string, + deps: LoadTokensDeps, +): Promise { + const errors: { keychain?: string; file?: string } = {}; + + try { + const raw = new deps.EntryClass(service, tokensAccount).getPassword(); + if (raw) { + const tokens = parseTokens(raw); + if (tokens) return { found: true, tokens, source: "keychain" }; + errors.keychain = invalidPayload; + } + } catch (err: unknown) { + errors.keychain = errorMessage(err); + } + + try { + const raw = await deps.fs.readFile(join(configDir, tokensFile)); + const tokens = parseTokens(raw); + if (tokens) return { found: true, tokens, source: "file" }; + errors.file = invalidPayload; + } catch (err: unknown) { + errors.file = errorMessage(err); + } + + return { found: false, errors }; +} diff --git a/src/domains/auth/store/save.ts b/src/domains/auth/store/save.ts index f82350742..9870f44eb 100644 --- a/src/domains/auth/store/save.ts +++ b/src/domains/auth/store/save.ts @@ -5,7 +5,7 @@ import { Entry } from "@napi-rs/keyring"; import { errorMessage } from "~/core/errors.js"; import { account, credentialsFile, service } from "./constants.js"; -import type { CredentialsFile, SaveApiKeyResult } from "./types.js"; +import type { CredentialsFile, SaveCredentialResult } from "./types.js"; async function saveToFile( configDir: string, @@ -27,7 +27,7 @@ export async function saveApiKey( configDir: string, key: string, fs: Fs, -): Promise { +): Promise { try { const entry = new Entry(service, account); entry.setPassword(key); diff --git a/src/domains/auth/store/saveTokens.ts b/src/domains/auth/store/saveTokens.ts new file mode 100644 index 000000000..2f5f6a835 --- /dev/null +++ b/src/domains/auth/store/saveTokens.ts @@ -0,0 +1,63 @@ +import type { StoredSession } from "~/domains/auth/types.js"; +import type { Fs } from "~/shell/fs.js"; +import { randomUUID } from "node:crypto"; +import { join } from "node:path"; + +import { Entry } from "@napi-rs/keyring"; + +import { errorMessage } from "~/core/errors.js"; +import { service, tokensAccount, tokensFile } from "./constants.js"; +import type { SaveCredentialResult } from "./types.js"; + +async function saveToFile( + configDir: string, + tokens: StoredSession, + fs: Fs, +): Promise { + // rwx------ (owner only) + await fs.mkdir(configDir, { recursive: true, mode: 0o700 }); + // Written beside the file and renamed over it: a plain write truncates + // first, and a command reading the store at that moment would parse half a + // record and report the session gone. A rename replaces it in one step. + // Per call, not per process: the workers of one `flows run` can all save + // at once, and two writes sharing a staging name would publish each other's + // pair or fail on a rename that already happened. + const target = join(configDir, tokensFile); + const staging = `${target}.${randomUUID()}.tmp`; + // rw------- (owner read/write only) + try { + await fs.writeFile(staging, JSON.stringify(tokens, undefined, 2), { + mode: 0o600, + }); + await fs.rename(staging, target); + } catch (err: unknown) { + // The staging file holds both tokens, whole or in part. Left behind, it + // outlives the session it was written for; best effort, since whatever + // refused the write or the rename may refuse the unlink too. + await fs.unlink(staging).catch(() => {}); + throw err; + } +} + +export async function saveTokens( + configDir: string, + tokens: StoredSession, + fs: Fs, +): Promise { + try { + new Entry(service, tokensAccount).setPassword(JSON.stringify(tokens)); + return { stored: "keychain" }; + } catch (err: unknown) { + // The file is about to become the newer record. A keychain entry left + // behind would still win on the next load, and its refresh token is + // spent, so the session would end for no reason. Best effort: a keychain + // that refuses to write may refuse to delete too. + try { + new Entry(service, tokensAccount).deletePassword(); + } catch { + // nothing more to clear + } + await saveToFile(configDir, tokens, fs); + return { stored: "file", keychainError: errorMessage(err) }; + } +} diff --git a/src/domains/auth/store/tokens.test.ts b/src/domains/auth/store/tokens.test.ts new file mode 100644 index 000000000..2c5c71d78 --- /dev/null +++ b/src/domains/auth/store/tokens.test.ts @@ -0,0 +1,221 @@ +import { afterEach, describe, expect, it, mock, spyOn } from "bun:test"; + +import { Entry } from "@napi-rs/keyring"; + +import type { Fs } from "~/shell/fs.js"; +import { makeMemoryFs } from "~/shell/fs.testUtils.js"; + +import { deleteTokens } from "./deleteTokens.js"; +import { saveTokens } from "./saveTokens.js"; +import { tokens } from "./tokens.testUtils.js"; + +afterEach(() => { + mock.restore(); +}); + +describe("saveTokens", () => { + it("stores tokens in the keychain when it is available", async () => { + spyOn(Entry.prototype, "setPassword").mockReturnValue(undefined); + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + + const result = await saveTokens("/config", tokens, memFs); + + expect(result.stored).toBe("keychain"); + expect(await memFs.pathExists("/config/tokens.json")).toBe(false); + }); + + it("falls back to a token file when the keychain throws", async () => { + spyOn(Entry.prototype, "setPassword").mockImplementation(() => { + throw Error("keychain unavailable"); + }); + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + + const result = await saveTokens("/config", tokens, memFs); + + expect(result.stored).toBe("file"); + const contents = await memFs.readFile("/config/tokens.json"); + expect(JSON.parse(contents)).toEqual(tokens); + }); + + it("clears a keychain entry it could not overwrite", async () => { + spyOn(Entry.prototype, "setPassword").mockImplementation(() => { + throw Error("keychain unavailable"); + }); + const deletePassword = spyOn( + Entry.prototype, + "deletePassword", + ).mockReturnValue(true); + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + + await saveTokens("/config", tokens, memFs); + + // Otherwise the stale entry keeps winning over the file on every load. + expect(deletePassword).toHaveBeenCalledTimes(1); + }); + + it("keeps two saves in one process apart", async () => { + spyOn(Entry.prototype, "setPassword").mockImplementation(() => { + throw Error("keychain unavailable"); + }); + spyOn(Entry.prototype, "deletePassword").mockReturnValue(true); + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + const other = { ...tokens, refreshToken: "refresh_other" }; + + await Promise.all([ + saveTokens("/config", tokens, memFs), + saveTokens("/config", other, memFs), + ]); + + const stored = await memFs.readFile("/config/tokens.json"); + const candidates = [tokens, other].map((t) => + JSON.stringify(t, undefined, 2), + ); + expect(candidates).toContain(stored); + expect(await memFs.readdir("/config")).toEqual(["tokens.json"]); + }); + + // The staging file holds both tokens. A rename that fails must not leave it + // on disk to outlive the session it was written for. + it("removes the staging file when it cannot be published", async () => { + spyOn(Entry.prototype, "setPassword").mockImplementation(() => { + throw Error("keychain unavailable"); + }); + spyOn(Entry.prototype, "deletePassword").mockReturnValue(true); + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + const refusing: Fs = { + ...memFs, + rename: async () => { + throw Object.assign(Error("EACCES: permission denied"), { + code: "EACCES", + }); + }, + }; + + let caught: unknown; + try { + await saveTokens("/config", tokens, refusing); + } catch (err) { + caught = err; + } + + expect((caught as Error | undefined)?.message).toContain("EACCES"); + expect(await memFs.readdir("/config")).toEqual([]); + }); + + it("removes a staging file whose write did not complete", async () => { + spyOn(Entry.prototype, "setPassword").mockImplementation(() => { + throw Error("keychain unavailable"); + }); + spyOn(Entry.prototype, "deletePassword").mockReturnValue(true); + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + const truncating: Fs = { + ...memFs, + writeFile: async (path, data) => { + // Half the record lands, then the disk gives out. + await memFs.writeFile(path, String(data).slice(0, 20)); + throw Object.assign(Error("ENOSPC: no space left"), { code: "ENOSPC" }); + }, + }; + + let caught: unknown; + try { + await saveTokens("/config", tokens, truncating); + } catch (err) { + caught = err; + } + + expect((caught as Error | undefined)?.message).toContain("ENOSPC"); + expect(await memFs.readdir("/config")).toEqual([]); + }); + + it("leaves no partial file behind, and no temporary one", async () => { + spyOn(Entry.prototype, "setPassword").mockImplementation(() => { + throw Error("keychain unavailable"); + }); + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + const written: string[] = []; + const recordingFs: Fs = { + ...memFs, + writeFile: (path, data, options) => { + written.push(path); + return memFs.writeFile(path, data, options); + }, + }; + + await saveTokens("/config", tokens, recordingFs); + + // The record was never written to its final name; it arrived by rename. + expect(written).not.toContain("/config/tokens.json"); + expect(await memFs.readdir("/config")).toEqual(["tokens.json"]); + }); + + it("writes the token file so only its owner can read it", async () => { + spyOn(Entry.prototype, "setPassword").mockImplementation(() => { + throw Error("keychain unavailable"); + }); + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + const modes: (number | undefined)[] = []; + const recordingFs: Fs = { + ...memFs, + writeFile: (path, data, options) => { + modes.push(options?.mode); + return memFs.writeFile(path, data, options); + }, + }; + + await saveTokens("/config", tokens, recordingFs); + + expect(modes).toEqual([0o600]); + }); +}); + +describe("deleteTokens", () => { + it("removes the token file", async () => { + spyOn(Entry.prototype, "deletePassword").mockReturnValue(true); + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + await memFs.writeFile("/config/tokens.json", JSON.stringify(tokens)); + + const result = await deleteTokens("/config", memFs); + + expect(result.file).toBe("deleted"); + expect(await memFs.pathExists("/config/tokens.json")).toBe(false); + }); + + it("reports not-found when there is no token file", async () => { + spyOn(Entry.prototype, "deletePassword").mockReturnValue(true); + const memFs = makeMemoryFs(); + await memFs.mkdir("/config", { recursive: true }); + + const result = await deleteTokens("/config", memFs); + + expect(result.file).toBe("not-found"); + }); + // Reporting "Credentials removed" over a file that is still there is worse + // than failing loudly. + it("propagates a deletion failure that is not a missing file", async () => { + spyOn(Entry.prototype, "deletePassword").mockReturnValue(true); + const failing = { + unlink: async () => { + throw Object.assign(Error("permission denied"), { code: "EACCES" }); + }, + } as unknown as Fs; + + let caught: unknown; + try { + await deleteTokens("/config", failing); + } catch (err) { + caught = err; + } + + expect((caught as Error | undefined)?.message).toBe("permission denied"); + }); +}); diff --git a/src/domains/auth/store/tokens.testUtils.ts b/src/domains/auth/store/tokens.testUtils.ts new file mode 100644 index 000000000..373533320 --- /dev/null +++ b/src/domains/auth/store/tokens.testUtils.ts @@ -0,0 +1,29 @@ +import type { Entry } from "@napi-rs/keyring"; + +import type { StoredSession } from "~/domains/auth/types.js"; + +export const tokens: StoredSession = { + accessToken: "access_abc", + refreshToken: "refresh_abc", + expiresAt: 1_700_000_000_000, + email: "person@example.com", + organizationId: "org_1", + clientId: "client_1", +}; + +export function makeEntryClass(getPassword: () => string): typeof Entry { + return class { + getPassword = getPassword; + } as unknown as typeof Entry; +} + +export function makeThrowingEntryClass(message: string): typeof Entry { + return class { + constructor(_service: string, _account: string) { + throw Error(message); + } + getPassword(): string { + throw Error("unreachable"); + } + } as unknown as typeof Entry; +} diff --git a/src/domains/auth/store/types.ts b/src/domains/auth/store/types.ts index 42cd3bbd0..5d82f2dfa 100644 --- a/src/domains/auth/store/types.ts +++ b/src/domains/auth/store/types.ts @@ -8,7 +8,19 @@ export const credentialsFileSchema = z.object({ export type CredentialsFile = z.infer; -export type SaveApiKeyResult = { +export const oauthTokensSchema = z.object({ + accessToken: z.string().min(1), + refreshToken: z.string().min(1), + /** Epoch ms. Absent when the access token carried no readable expiry. */ + expiresAt: z.number().int().optional(), + email: z.string().min(1), + /** WorkOS organization the session is scoped to; refreshes are pinned to it. */ + organizationId: z.string().min(1).optional(), + /** WorkOS client that issued the tokens; refreshes go back to it. */ + clientId: z.string().min(1).optional(), +}); + +export type SaveCredentialResult = { keychain: { stored: "keychain" }; file: { stored: "file"; keychainError: string }; }[StorageSource]; @@ -18,4 +30,6 @@ type DeleteOutcomeMap = { file: "deleted" | "not-found"; }; -export type DeleteApiKeyResult = { [K in StorageSource]: DeleteOutcomeMap[K] }; +export type DeleteCredentialResult = { + [K in StorageSource]: DeleteOutcomeMap[K]; +}; diff --git a/src/domains/auth/types.ts b/src/domains/auth/types.ts index 5096994aa..28f090a3f 100644 --- a/src/domains/auth/types.ts +++ b/src/domains/auth/types.ts @@ -1,9 +1,12 @@ +import type { DeviceTokens } from "~/core/deviceAuth/types.js"; + // "keychain" = OS credential store (macOS Keychain, Windows Credential Manager, etc.) // "file" = fallback JSON file in the config directory export type StorageSource = "keychain" | "file"; -// "env" = QAWOLF_API_KEY environment variable -type ApiKeySource = "env" | StorageSource; +// "env" = QAWOLF_API_KEY environment variable +// "browser" = access token from browser sign-in, held in either storage +type ApiKeySource = "env" | StorageSource | "browser"; export type ApiKeyResult = { key: string; @@ -14,6 +17,21 @@ export type LoadApiKeyResult = | { found: true; key: string; source: StorageSource } | { found: false; errors?: { keychain?: string; file?: string } }; +/** What browser sign-in persists: the WorkOS tokens plus their issuing client. */ +export type StoredSession = DeviceTokens & { + /** + * WorkOS client that issued these tokens. A refresh token is only redeemable + * against its issuing client, so the session records it rather than asking + * the deployment again — which could answer differently if the CLI has since + * been pointed elsewhere. + */ + clientId: string | undefined; +}; + +export type LoadTokensResult = + | { found: true; tokens: StoredSession; source: StorageSource } + | { found: false; errors?: { keychain?: string; file?: string } }; + export type ValidateApiKeyResult = | { valid: true } | { valid: false; error: string };