diff --git a/package-lock.json b/package-lock.json index 06d0718..c633551 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "hasInstallScript": true, "license": "MIT", "dependencies": { - "@doist/cli-core": "0.12.0", + "@doist/cli-core": "0.16.1", "chalk": "5.6.2", "commander": "14.0.2", "marked": "18.0.3", @@ -132,9 +132,9 @@ } }, "node_modules/@doist/cli-core": { - "version": "0.12.0", - "resolved": "https://registry.npmjs.org/@doist/cli-core/-/cli-core-0.12.0.tgz", - "integrity": "sha512-6dk9mL+HMmkfijcO9t+AtXO+2vxV6Gj5thD91sExNJfInnmQzFhMSYOJ/8u/k5gzpD9wyVAxL+q+bzt8s9eeQw==", + "version": "0.16.1", + "resolved": "https://registry.npmjs.org/@doist/cli-core/-/cli-core-0.16.1.tgz", + "integrity": "sha512-OoHWYM8Rv+eoPhVR1BUMwFIejZGk5Wyxheva4Cp9x3zTh4RURE+OIJ03G95oBegbXCA2O9LRtZZ9Ww0D6vU1dQ==", "license": "MIT", "dependencies": { "chalk": "5.6.2", @@ -143,6 +143,9 @@ "engines": { "node": ">=20.18.1" }, + "optionalDependencies": { + "@napi-rs/keyring": "1.3.0" + }, "peerDependencies": { "commander": ">=14", "marked": ">=18", @@ -639,6 +642,226 @@ "integrity": "sha512-dXn3FZhPv0US+7dtJsIi2R+c7qWYiReoEh5zUntWCf4oSpMNib8FDhSoed6m3QyZdx5hK7iLFkYk3rNxwt8vTA==", "license": "MIT" }, + "node_modules/@napi-rs/keyring": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring/-/keyring-1.3.0.tgz", + "integrity": "sha512-WrOw/bcXm0f9qHkumlT1QlArXSTWqaY9sunsDpOk+yCCorCKMxvWT/a3xko4EYHVdeZoh00yI2TydXn6eyICDA==", + "license": "MIT", + "optional": true, + "engines": { + "node": ">= 10" + }, + "funding": { + "type": "github", + "url": "https://github.com/sponsors/Brooooooklyn" + }, + "optionalDependencies": { + "@napi-rs/keyring-darwin-arm64": "1.3.0", + "@napi-rs/keyring-darwin-x64": "1.3.0", + "@napi-rs/keyring-freebsd-x64": "1.3.0", + "@napi-rs/keyring-linux-arm-gnueabihf": "1.3.0", + "@napi-rs/keyring-linux-arm64-gnu": "1.3.0", + "@napi-rs/keyring-linux-arm64-musl": "1.3.0", + "@napi-rs/keyring-linux-riscv64-gnu": "1.3.0", + "@napi-rs/keyring-linux-x64-gnu": "1.3.0", + "@napi-rs/keyring-linux-x64-musl": "1.3.0", + "@napi-rs/keyring-win32-arm64-msvc": "1.3.0", + "@napi-rs/keyring-win32-ia32-msvc": "1.3.0", + "@napi-rs/keyring-win32-x64-msvc": "1.3.0" + } + }, + "node_modules/@napi-rs/keyring-darwin-arm64": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-darwin-arm64/-/keyring-darwin-arm64-1.3.0.tgz", + "integrity": "sha512-pl76hJvdYUBn6I24bXiOBMA9nbDapo3I5B+f3OorjDU4dUMSypXeKbOVehJe8fhgTiH24flMyTS3aAIy43xegQ==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-darwin-x64": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-darwin-x64/-/keyring-darwin-x64-1.3.0.tgz", + "integrity": "sha512-YcJtEV5LA3cvA4z3BurgxH5IhTsW1JfIvcAAcqcecwk06Si9F9NqkxbZVIfDwQ8oRHgaBmT3zZJnLAotCrVahw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-freebsd-x64": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-freebsd-x64/-/keyring-freebsd-x64-1.3.0.tgz", + "integrity": "sha512-vlLf31TGhfRAaxLDBhg8b89ss0HHD/lyNmL5F3UjSaz5CUXElsJmKYq9fqA/B+cZKUEUcLHHGhF0I/CqcFdaVw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "freebsd" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm-gnueabihf": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm-gnueabihf/-/keyring-linux-arm-gnueabihf-1.3.0.tgz", + "integrity": "sha512-KiWdMMu/Inz/bHHIAGrnF7r54FZDYXuHO6UFF/rhIrshUsxbMG1Rl9lEymNtqqsVo927G0VYcb02FzWQ3iBQRQ==", + "cpu": [ + "arm" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm64-gnu": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm64-gnu/-/keyring-linux-arm64-gnu-1.3.0.tgz", + "integrity": "sha512-eyKGpY40lm9Jvs1aD294XRH4y7+TlJM0YVAryZeXA6TX0mb4gMkxVXwSQv7MCwgah7raeUd0dKUb4BPAYIgcMg==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-arm64-musl": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-arm64-musl/-/keyring-linux-arm64-musl-1.3.0.tgz", + "integrity": "sha512-iIK6JWHXAJqDrEyLY3TmswwloVyt2vj+04TZnew+uSJ9gnDO8EwRbp3/iw3LpWaXiDO7VomGO6y8I0Id8uBZSw==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-riscv64-gnu": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-riscv64-gnu/-/keyring-linux-riscv64-gnu-1.3.0.tgz", + "integrity": "sha512-/PGqrwn6EwgtK6vccASSXJRfOSP4vN1F4ASsIQ+7MdrK6hNvAJ1FZPrIuD5gGGdxezo3F++To2Wq7DbuGIeuNQ==", + "cpu": [ + "riscv64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-x64-gnu": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-x64-gnu/-/keyring-linux-x64-gnu-1.3.0.tgz", + "integrity": "sha512-2PDK1WKWTu9lBGq9VvNEkSlQD3O7YwVpmnyN2M3cy4v7NJ/8gDMd9GXv3G+FVXN13uhp4gnnPBS+ScefmEeD2A==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-linux-x64-musl": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-linux-x64-musl/-/keyring-linux-x64-musl-1.3.0.tgz", + "integrity": "sha512-oJ2HkX8YUo46QBkn0pG+HuIKQNqr523q6vBobCn+P95s4C4K6/kLBqHY/1bg5J4ap31DzsznhnFKcfBNBsjCnw==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-arm64-msvc": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-arm64-msvc/-/keyring-win32-arm64-msvc-1.3.0.tgz", + "integrity": "sha512-tOd3c/uAaeoE4ycVlmAdSvygz0Zt3zdca6Y7gokBeIbaRDWpjDIUOpU3MvML59XAaqyuKGsVVu0F/DZb1lHPmw==", + "cpu": [ + "arm64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-ia32-msvc": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-ia32-msvc/-/keyring-win32-ia32-msvc-1.3.0.tgz", + "integrity": "sha512-sPSqeAFZMGqP1R++M2JTza7GQJJ/TpCo6JU6Vcd4jnebvOaEDs9b7eipakU1PJdSvhpC2yXMCNRk9gXfrhuwHQ==", + "cpu": [ + "ia32" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, + "node_modules/@napi-rs/keyring-win32-x64-msvc": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/@napi-rs/keyring-win32-x64-msvc/-/keyring-win32-x64-msvc-1.3.0.tgz", + "integrity": "sha512-4DnCWXwDc0HRKwyRlG5y0VhKZW2tNRQfKKfyj6IX/KWfDNyq9hn4n+GL1auyDcOO/v8PwnhmYo2+rOOqCkvvOg==", + "cpu": [ + "x64" + ], + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">= 10" + } + }, "node_modules/@napi-rs/wasm-runtime": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/@napi-rs/wasm-runtime/-/wasm-runtime-1.1.4.tgz", diff --git a/package.json b/package.json index 3df1cb5..b5f0cb9 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "node": ">=20.18.1" }, "dependencies": { - "@doist/cli-core": "0.12.0", + "@doist/cli-core": "0.16.1", "chalk": "5.6.2", "commander": "14.0.2", "marked": "18.0.3", diff --git a/src/__tests__/_fixtures/auth.ts b/src/__tests__/_fixtures/auth.ts new file mode 100644 index 0000000..dc868ce --- /dev/null +++ b/src/__tests__/_fixtures/auth.ts @@ -0,0 +1,54 @@ +import type { MigrateAuthResult } from '@doist/cli-core/auth' +import type { OutlineAccount } from '../../lib/outline-account.js' + +/** Canonical persisted `OutlineAccount` used across auth tests. */ +export const STORED_ACCOUNT: OutlineAccount = { + id: 'user-uuid', + label: 'Ada', + baseUrl: 'https://wiki.example.com', + oauthClientId: 'cid-xyz', + teamName: 'Analytics', +} + +/** v1 plaintext config snapshot that round-trips to `STORED_ACCOUNT`. */ +export const LEGACY_CONFIG = { + api_token: 'tk_legacy_plaintext', + base_url: 'https://wiki.example.com', + oauth_client_id: 'cid-xyz', + auth_user_id: 'user-uuid', + auth_user_name: 'Ada', + auth_team_name: 'Analytics', +} as const + +/** `updateConfig` payload that wipes every v1 auth key. */ +export const LEGACY_CLEAR_PAYLOAD = { + api_token: undefined, + base_url: undefined, + oauth_client_id: undefined, + auth_user_id: undefined, + auth_user_name: undefined, + auth_team_name: undefined, +} as const + +/** Outline `auth.info` response body. Richer than `STORED_ACCOUNT` (carries email). */ +export const AUTH_INFO = { + user: { id: 'user-uuid', name: 'Ada Lovelace', email: 'ada@example.com' }, + team: { name: 'Analytics', subdomain: 'analytics' }, +} as const + +/** Stand-in for a cli-core `migrateLegacyAuth` skip result. */ +export const SKIPPED_RESULT: MigrateAuthResult = { + status: 'skipped', + reason: 'identify-failed', + detail: 'offline', +} + +/** 200 `Response` with `body` as JSON, using the native static helper. */ +export function okResponse(body: unknown): Response { + return Response.json(body) +} + +/** Error `Response` with the given status (default 500). */ +export function errResponse(status: number, statusText = 'Error', body?: unknown): Response { + return Response.json(body ?? {}, { status, statusText }) +} diff --git a/src/__tests__/auth-command.test.ts b/src/__tests__/auth-command.test.ts index 1b2016f..9c87d11 100644 --- a/src/__tests__/auth-command.test.ts +++ b/src/__tests__/auth-command.test.ts @@ -1,12 +1,13 @@ import { Command } from 'commander' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { AUTH_INFO } from './_fixtures/auth.js' vi.mock('../lib/auth.js', () => ({ getApiToken: async () => 'test-token', getBaseUrl: async () => 'https://test.outline.com', getOAuthClientId: async () => undefined, - getTokenSource: async () => 'config' as const, - clearConfig: vi.fn(), + getActiveTokenSource: async () => + process.env.OUTLINE_API_TOKEN ? ('env' as const) : ('secure-store' as const), })) vi.mock('../lib/api.js', () => ({ apiRequest: vi.fn() })) @@ -27,6 +28,19 @@ vi.mock('@doist/cli-core/auth', async () => ({ attachLoginCommand: vi.fn(), })) +/** + * Replace `console.log` with a recorder. Tests read `logs` to assert on + * stdout-bound output. Lines are joined with spaces, matching how chalk's + * styled fragments arrive at the spy. + */ +function captureLogs(): { logs: string[] } { + const logs: string[] = [] + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + logs.push(args.join(' ')) + }) + return { logs } +} + async function captureAttachOptions() { const { attachLoginCommand } = await import('@doist/cli-core/auth') const login = new Command('login') @@ -59,10 +73,7 @@ afterEach(() => { describe('registerAuthCommand', () => { it('wires --base-url / --client-id, env-driven port, and prints success only in human output mode', async () => { process.env.OUTLINE_OAUTH_CALLBACK_PORT = '7000' - const logs: string[] = [] - vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { - logs.push(args.join(' ')) - }) + const { logs } = captureLogs() const { options, login } = await captureAttachOptions() @@ -93,11 +104,6 @@ describe('registerAuthCommand', () => { }) describe('auth status subcommand', () => { - const AUTH_INFO = { - user: { id: 'user-uuid', name: 'Ada Lovelace', email: 'ada@example.com' }, - team: { name: 'Analytics', subdomain: 'analytics' }, - } - async function importApiMock() { const { apiRequest } = await import('../lib/api.js') return vi.mocked(apiRequest) @@ -105,10 +111,7 @@ describe('auth status subcommand', () => { it('renders the human status from the env-token snapshot path', async () => { process.env.OUTLINE_API_TOKEN = 'env-token' - const logs: string[] = [] - vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { - logs.push(args.join(' ')) - }) + const { logs } = captureLogs() const apiRequest = await importApiMock() apiRequest.mockResolvedValue({ data: AUTH_INFO }) @@ -130,10 +133,7 @@ describe('auth status subcommand', () => { it('emits a PII-free JSON envelope under --json', async () => { process.env.OUTLINE_API_TOKEN = 'env-token' - const logs: string[] = [] - vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { - logs.push(args.join(' ')) - }) + const { logs } = captureLogs() const apiRequest = await importApiMock() apiRequest.mockResolvedValue({ data: AUTH_INFO }) @@ -154,10 +154,7 @@ describe('auth status subcommand', () => { it('emits a single newline-free NDJSON line under --ndjson', async () => { process.env.OUTLINE_API_TOKEN = 'env-token' - const logs: string[] = [] - vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { - logs.push(args.join(' ')) - }) + const { logs } = captureLogs() const apiRequest = await importApiMock() apiRequest.mockResolvedValue({ data: AUTH_INFO }) @@ -194,25 +191,17 @@ describe('auth status subcommand', () => { }) describe('auth logout subcommand', () => { - it('clears the token and prints the registrar success line', async () => { - const logs: string[] = [] - vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { - logs.push(args.join(' ')) - }) - const { clearConfig } = await import('../lib/auth.js') + it('prints the registrar success line in human mode', async () => { + const { logs } = captureLogs() const program = await buildProgram() await program.parseAsync(['node', 'ol', 'auth', 'logout']) - expect(clearConfig).toHaveBeenCalledTimes(1) expect(logs).toContain('✓ Logged out') }) it('emits {"ok": true} under --json and skips the human success line', async () => { - const logs: string[] = [] - vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { - logs.push(args.join(' ')) - }) + const { logs } = captureLogs() const program = await buildProgram() await program.parseAsync(['node', 'ol', 'auth', 'logout', '--json']) @@ -221,11 +210,13 @@ describe('auth logout subcommand', () => { expect(JSON.parse(logs[0])).toEqual({ ok: true }) }) - it('stays silent on stdout under --ndjson', async () => { - const logs: string[] = [] - vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { - logs.push(args.join(' ')) - }) + it('stays silent on stdout under --ndjson (no human storage-result line leaks)', async () => { + // Critical: logout now surfaces a storage-result confirmation via + // `logTokenStorageResult` on success. NDJSON consumers expect a + // clean stdout — any human-readable line here would corrupt the + // stream. Guards the `isMachineOutput` branch in + // `logTokenStorageResult`. + const { logs } = captureLogs() const program = await buildProgram() await program.parseAsync(['node', 'ol', 'auth', 'logout', '--ndjson']) @@ -233,3 +224,51 @@ describe('auth logout subcommand', () => { expect(logs).toEqual([]) }) }) + +describe('logTokenStorageResult', () => { + function captureStreams() { + const logs: string[] = [] + const errs: string[] = [] + vi.spyOn(console, 'log').mockImplementation((...a: unknown[]) => { + logs.push(a.join(' ')) + }) + vi.spyOn(console, 'error').mockImplementation((...a: unknown[]) => { + errs.push(a.join(' ')) + }) + return { logs, errs } + } + + it('prints the secure-store confirmation to stdout in human mode', async () => { + const { logs, errs } = captureStreams() + const { logTokenStorageResult } = await import('../commands/auth.js') + + logTokenStorageResult({ storage: 'secure-store' }, 'Token stored securely', false) + + expect(logs.some((l) => l.includes('Token stored securely'))).toBe(true) + expect(errs).toEqual([]) + }) + + it('suppresses the stdout confirmation in machine-output mode', async () => { + const { logs } = captureStreams() + const { logTokenStorageResult } = await import('../commands/auth.js') + + logTokenStorageResult({ storage: 'secure-store' }, 'Token stored securely', true) + + expect(logs).toEqual([]) + }) + + it('routes the keyring-fallback warning to stderr (in both human and machine modes)', async () => { + const { logs, errs } = captureStreams() + const { logTokenStorageResult } = await import('../commands/auth.js') + + logTokenStorageResult( + { storage: 'config-file', warning: 'system credential manager unavailable' }, + 'Token stored securely', + true, + ) + + // No stdout in machine mode, but warning still reaches operator on stderr. + expect(logs).toEqual([]) + expect(errs.some((e) => e.includes('system credential manager unavailable'))).toBe(true) + }) +}) diff --git a/src/__tests__/auth-provider.test.ts b/src/__tests__/auth-provider.test.ts index 46b8578..2f666f4 100644 --- a/src/__tests__/auth-provider.test.ts +++ b/src/__tests__/auth-provider.test.ts @@ -1,31 +1,77 @@ -import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs' -import { tmpdir } from 'node:os' -import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' - -const TEST_XDG = join(tmpdir(), `outline-cli-test-${process.pid}-auth-provider`) -const TEST_CONFIG_DIR = join(TEST_XDG, 'outline-cli') -const TEST_CONFIG_PATH = join(TEST_CONFIG_DIR, 'config.json') +import { + LEGACY_CLEAR_PAYLOAD, + LEGACY_CONFIG, + okResponse, + SKIPPED_RESULT, + STORED_ACCOUNT, +} from './_fixtures/auth.js' vi.mock('../transport/fetch-with-retry.js', () => ({ fetchWithRetry: vi.fn() })) vi.mock('../lib/api.js', () => ({ apiRequest: vi.fn() })) -beforeEach(() => { - process.env.XDG_CONFIG_HOME = TEST_XDG - mkdirSync(TEST_CONFIG_DIR, { recursive: true }) - delete process.env.OUTLINE_API_TOKEN - delete process.env.OUTLINE_URL - delete process.env.OUTLINE_OAUTH_CLIENT_ID - vi.resetModules() - vi.clearAllMocks() +const migrateMocks = vi.hoisted(() => ({ + runMigrateLegacyAuth: vi.fn(), +})) + +vi.mock('../lib/migrate-auth.js', () => migrateMocks) + +const keyringMocks = vi.hoisted(() => ({ + createKeyringTokenStore: vi.fn(), + inner: { + active: vi.fn(), + set: vi.fn(), + clear: vi.fn(), + list: vi.fn(), + setDefault: vi.fn(), + getLastStorageResult: vi.fn(), + getLastClearResult: vi.fn(), + }, +})) + +vi.mock('@doist/cli-core/auth', async (importOriginal) => { + const actual = await importOriginal() + keyringMocks.createKeyringTokenStore.mockImplementation(() => keyringMocks.inner) + return { + ...actual, + createKeyringTokenStore: keyringMocks.createKeyringTokenStore, + } }) -afterEach(() => { - if (existsSync(TEST_XDG)) rmSync(TEST_XDG, { recursive: true }) - delete process.env.XDG_CONFIG_HOME +const configMocks = vi.hoisted(() => ({ + getConfig: vi.fn(), + updateConfig: vi.fn(), +})) + +vi.mock('../lib/config.js', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + getConfigPath: () => '/home/user/.config/outline-cli/config.json', + getConfig: configMocks.getConfig, + updateConfig: configMocks.updateConfig, + } }) -describe('OutlineAuthProvider', () => { +const TOKEN_ENV_VAR = 'OUTLINE_API_TOKEN' + +/** Reset the module-level migration memo for each test by re-importing. */ +async function loadCreateOutlineTokenStore(): Promise< + typeof import('../lib/auth-provider.js').createOutlineTokenStore +> { + vi.resetModules() + const mod = await import('../lib/auth-provider.js') + return mod.createOutlineTokenStore +} + +describe('createOutlineAuthProvider', () => { + beforeEach(() => { + delete process.env.OUTLINE_URL + delete process.env.OUTLINE_OAUTH_CLIENT_ID + configMocks.getConfig.mockReset().mockResolvedValue({}) + configMocks.updateConfig.mockReset().mockResolvedValue(undefined) + }) + it('authorize builds an outline /oauth/authorize URL with PKCE params from flags', async () => { const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') const result = await createOutlineAuthProvider().authorize({ @@ -58,10 +104,7 @@ describe('OutlineAuthProvider', () => { it('exchangeCode posts via fetchWithRetry and surfaces provider errors', async () => { const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') - vi.mocked(fetchWithRetry).mockResolvedValueOnce({ - ok: true, - json: async () => ({ access_token: 'tok-abc' }), - } as Response) + vi.mocked(fetchWithRetry).mockResolvedValueOnce(okResponse({ access_token: 'tok-abc' })) const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') const provider = createOutlineAuthProvider() @@ -135,151 +178,279 @@ describe('OutlineAuthProvider', () => { }) }) -describe('OutlineTokenStore', () => { - const sampleAccount = { - id: 'user-uuid', - label: 'Ada', - baseUrl: 'https://wiki.example.com', - oauthClientId: 'cid-xyz', - teamName: 'Analytics', - } +describe('createOutlineTokenStore', () => { + beforeEach(() => { + delete process.env[TOKEN_ENV_VAR] + delete process.env.OUTLINE_URL + keyringMocks.createKeyringTokenStore.mockClear() + keyringMocks.inner.active.mockReset().mockResolvedValue(null) + keyringMocks.inner.set.mockReset().mockResolvedValue(undefined) + keyringMocks.inner.clear.mockReset().mockResolvedValue(undefined) + keyringMocks.inner.list.mockReset().mockResolvedValue([]) + keyringMocks.inner.setDefault.mockReset().mockResolvedValue(undefined) + migrateMocks.runMigrateLegacyAuth + .mockReset() + .mockResolvedValue({ status: 'no-legacy-state' }) + configMocks.getConfig.mockReset().mockResolvedValue({}) + configMocks.updateConfig.mockReset().mockResolvedValue(undefined) + }) - /** Config-file shape that round-trips to `sampleAccount`. */ - const sampleConfig = { - api_token: 'tok', - base_url: sampleAccount.baseUrl, - oauth_client_id: sampleAccount.oauthClientId, - auth_user_id: sampleAccount.id, - auth_user_name: sampleAccount.label, - auth_team_name: sampleAccount.teamName, - } + afterEach(() => { + vi.unstubAllEnvs() + }) + + it('passes outline-cli wiring to cli-core: serviceName, records location, and the id-or-label matcher', async () => { + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + createOutlineTokenStore() + + const options = keyringMocks.createKeyringTokenStore.mock.calls[0][0] + expect(options.serviceName).toBe('outline-cli') + expect(options.recordsLocation).toBe('/home/user/.config/outline-cli/config.json') + const { matchOutlineAccount } = await import('../lib/auth-provider.js') + expect(options.matchAccount).toBe(matchOutlineAccount) + }) - it('round-trips token + account through the config file', async () => { - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') + it('active() env-token short-circuit: returns env token, honours OUTLINE_URL, bypasses migration + inner.active', async () => { + vi.stubEnv(TOKEN_ENV_VAR, 'env_token_value') + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + + const defaultBase = await createOutlineTokenStore().active() + expect(defaultBase?.token).toBe('env_token_value') + expect(defaultBase?.account.id).toBe('') + expect(defaultBase?.account.baseUrl).toBe('https://app.getoutline.com') + expect(keyringMocks.inner.active).not.toHaveBeenCalled() + expect(migrateMocks.runMigrateLegacyAuth).not.toHaveBeenCalled() + + vi.stubEnv('OUTLINE_URL', 'https://custom.example.com/') + const customBase = await createOutlineTokenStore().active() + expect(customBase?.account.baseUrl).toBe('https://custom.example.com') + }) + + it('active() ignores OUTLINE_API_TOKEN when an explicit ref targets a stored account', async () => { + vi.stubEnv(TOKEN_ENV_VAR, 'env_token_value') + keyringMocks.inner.active.mockResolvedValue({ + token: 'tk_stored', + account: STORED_ACCOUNT, + }) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + + await createOutlineTokenStore().active('user-uuid') + + expect(keyringMocks.inner.active).toHaveBeenCalledWith('user-uuid') + }) + + it('runs runMigrateLegacyAuth on the first store access and memoises across subsequent calls', async () => { + const createOutlineTokenStore = await loadCreateOutlineTokenStore() const store = createOutlineTokenStore() - await store.set(sampleAccount, 'tok-persisted') - const got = await store.active() - expect(got).toEqual({ token: 'tok-persisted', account: sampleAccount }) + + await store.active('user-uuid') + await store.list() + await store.clear('user-uuid') + await store.set(STORED_ACCOUNT, 'tk') + await store.setDefault('user-uuid') + + expect(migrateMocks.runMigrateLegacyAuth).toHaveBeenCalledTimes(1) + expect(migrateMocks.runMigrateLegacyAuth).toHaveBeenCalledWith({ silent: true }) }) - it('active returns null when the saved token predates the persisted-identity fields', async () => { - writeFileSync(TEST_CONFIG_PATH, JSON.stringify({ api_token: 'legacy-tok' })) - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - await expect(createOutlineTokenStore().active()).resolves.toBeNull() + it('falls back to the legacy plaintext snapshot only when the v2 store is empty', async () => { + migrateMocks.runMigrateLegacyAuth.mockResolvedValue(SKIPPED_RESULT) + configMocks.getConfig.mockResolvedValue(LEGACY_CONFIG) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + + const snapshot = await createOutlineTokenStore().active() + + expect(snapshot).toEqual({ + token: LEGACY_CONFIG.api_token, + account: STORED_ACCOUNT, + }) + // v2 consulted first (returned null per the beforeEach default), + // then the legacy snapshot served the answer. + expect(keyringMocks.inner.active).toHaveBeenCalledTimes(1) }) - it('clear strips every auth field but preserves unrelated config keys', async () => { - writeFileSync( - TEST_CONFIG_PATH, - JSON.stringify({ - api_token: 'tok', - base_url: 'https://x', - oauth_client_id: 'c', - auth_user_id: 'u', - auth_user_name: 'l', - auth_team_name: 't', - update_channel: 'pre-release', - }), + it('delegates to the v2 store when migration is conclusive (no-legacy-state) — no legacy read attempt', async () => { + migrateMocks.runMigrateLegacyAuth.mockResolvedValue({ status: 'no-legacy-state' }) + keyringMocks.inner.active.mockResolvedValue({ token: 'tk_v2', account: STORED_ACCOUNT }) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + + const snapshot = await createOutlineTokenStore().active() + + expect(snapshot).toEqual({ token: 'tk_v2', account: STORED_ACCOUNT }) + expect(configMocks.getConfig).not.toHaveBeenCalled() + }) + + it('falls back to legacy when runMigrateLegacyAuth rejects (catch branch of ensureMigrated)', async () => { + migrateMocks.runMigrateLegacyAuth.mockRejectedValue(new Error('boom')) + configMocks.getConfig.mockResolvedValue(LEGACY_CONFIG) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + + const snapshot = await createOutlineTokenStore().active() + + expect(snapshot?.token).toBe(LEGACY_CONFIG.api_token) + expect(snapshot?.account.id).toBe('user-uuid') + }) + + it('legacy snapshot synthesises empty id/label when the v1 config never carried persisted identity fields', async () => { + migrateMocks.runMigrateLegacyAuth.mockResolvedValue(SKIPPED_RESULT) + configMocks.getConfig.mockResolvedValue({ api_token: 'tk_old' }) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + + const snapshot = await createOutlineTokenStore().active() + + expect(snapshot?.token).toBe('tk_old') + expect(snapshot?.account.id).toBe('') + expect(snapshot?.account.label).toBe('') + expect(snapshot?.account.baseUrl).toBe('https://app.getoutline.com') + }) + + it('active(ref) returns the legacy snapshot when ref matches, falls through to v2 when it does not', async () => { + migrateMocks.runMigrateLegacyAuth.mockResolvedValue(SKIPPED_RESULT) + configMocks.getConfig.mockResolvedValue(LEGACY_CONFIG) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + const store = createOutlineTokenStore() + + const matched = await store.active('user-uuid') + expect(matched?.token).toBe(LEGACY_CONFIG.api_token) + + const mismatched = await store.active('other-user') + expect(mismatched).toBeNull() + expect(keyringMocks.inner.active).toHaveBeenCalledWith('other-user') + }) + + it('set() / clear() discharge legacy state on disk when migration is inconclusive', async () => { + migrateMocks.runMigrateLegacyAuth.mockResolvedValue(SKIPPED_RESULT) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + const store = createOutlineTokenStore() + + await store.set(STORED_ACCOUNT, 'tk_new') + await store.clear('user-uuid') + + expect(configMocks.updateConfig).toHaveBeenCalledWith(LEGACY_CLEAR_PAYLOAD) + expect(keyringMocks.inner.set).toHaveBeenCalledWith(STORED_ACCOUNT, 'tk_new') + expect(keyringMocks.inner.clear).toHaveBeenCalledWith('user-uuid') + }) + + it('set() / clear() do NOT touch legacy state when migration is conclusive', async () => { + migrateMocks.runMigrateLegacyAuth.mockResolvedValue({ status: 'no-legacy-state' }) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + const store = createOutlineTokenStore() + + await store.set(STORED_ACCOUNT, 'tk_new') + await store.clear('user-uuid') + + expect(configMocks.updateConfig).not.toHaveBeenCalled() + }) + + it('set() does NOT discharge legacy state when the v2 write fails (atomicity)', async () => { + // Regression test for the pre-fix order where legacy fields were + // erased before the v2 write — a failing keyring call would leave + // the user with no recoverable credentials. + migrateMocks.runMigrateLegacyAuth.mockResolvedValue(SKIPPED_RESULT) + keyringMocks.inner.set.mockRejectedValue(new Error('keyring boom')) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + + await expect(createOutlineTokenStore().set(STORED_ACCOUNT, 'tk_new')).rejects.toThrow( + 'keyring boom', ) - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - await createOutlineTokenStore().clear() - const after = JSON.parse(readFileSync(TEST_CONFIG_PATH, 'utf8')) - expect(after).toEqual({ update_channel: 'pre-release' }) + expect(configMocks.updateConfig).not.toHaveBeenCalled() }) - describe('ref-aware lookups', () => { - it('active(ref) returns the snapshot when the id ref matches', async () => { - writeFileSync(TEST_CONFIG_PATH, JSON.stringify(sampleConfig)) - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - expect(await createOutlineTokenStore().active(sampleAccount.id)).toEqual({ - token: sampleConfig.api_token, - account: sampleAccount, - }) - }) + it('clear() does NOT discharge legacy state when the v2 clear fails (atomicity)', async () => { + migrateMocks.runMigrateLegacyAuth.mockResolvedValue(SKIPPED_RESULT) + keyringMocks.inner.clear.mockRejectedValue(new Error('keyring boom')) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() - it('active(ref) matches the stored label case-insensitively', async () => { - writeFileSync(TEST_CONFIG_PATH, JSON.stringify(sampleConfig)) - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - expect(await createOutlineTokenStore().active('ADA')).toEqual({ - token: sampleConfig.api_token, - account: sampleAccount, - }) - }) + await expect(createOutlineTokenStore().clear('user-uuid')).rejects.toThrow('keyring boom') + expect(configMocks.updateConfig).not.toHaveBeenCalled() + }) - it('active(ref) throws ACCOUNT_NOT_FOUND on mismatch', async () => { - writeFileSync(TEST_CONFIG_PATH, JSON.stringify(sampleConfig)) - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - await expect(createOutlineTokenStore().active('other')).rejects.toMatchObject({ - code: 'ACCOUNT_NOT_FOUND', - }) - }) + it('clear() PROPAGATES updateConfig failures (logout must be atomic, not silently partial)', async () => { + // If the v2 clear succeeded but the legacy discharge silently + // failed, the next `active()` call would fall back to the + // surviving plaintext `api_token` — i.e. "logout" leaves the + // user authenticated. Logout has to fail loudly instead. + migrateMocks.runMigrateLegacyAuth.mockResolvedValue(SKIPPED_RESULT) + configMocks.updateConfig.mockRejectedValue(new Error('disk full')) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() - it('active(ref) throws ACCOUNT_NOT_FOUND when no token is stored', async () => { - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - await expect(createOutlineTokenStore().active(sampleAccount.id)).rejects.toMatchObject({ - code: 'ACCOUNT_NOT_FOUND', - }) - }) + await expect(createOutlineTokenStore().clear('user-uuid')).rejects.toThrow('disk full') + expect(keyringMocks.inner.clear).toHaveBeenCalledWith('user-uuid') + }) - it('clear(ref) clears the config when the ref matches', async () => { - writeFileSync( - TEST_CONFIG_PATH, - JSON.stringify({ ...sampleConfig, update_channel: 'stable' }), - ) - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - await createOutlineTokenStore().clear(sampleAccount.id) - const after = JSON.parse(readFileSync(TEST_CONFIG_PATH, 'utf8')) - expect(after).toEqual({ update_channel: 'stable' }) + it('set() runs migration BEFORE the v2 write (no post-write race that re-grabs the legacy token)', async () => { + // Regression test for the race where ensureMigrated only fired + // after inner.set/clear: migration would then see the stale + // api_token still on disk and migrate it on top of the fresh + // login, either duplicating accounts or reviving auth after logout. + const callOrder: string[] = [] + migrateMocks.runMigrateLegacyAuth.mockImplementation(async () => { + callOrder.push('migrate') + return { status: 'no-legacy-state' } }) - - it('clear(ref) throws ACCOUNT_NOT_FOUND on mismatch and does not touch storage', async () => { - writeFileSync(TEST_CONFIG_PATH, JSON.stringify(sampleConfig)) - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - await expect(createOutlineTokenStore().clear('other')).rejects.toMatchObject({ - code: 'ACCOUNT_NOT_FOUND', - }) - const after = JSON.parse(readFileSync(TEST_CONFIG_PATH, 'utf8')) - expect(after).toEqual(sampleConfig) + keyringMocks.inner.set.mockImplementation(async () => { + callOrder.push('set') }) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() - it('clear(ref) throws ACCOUNT_NOT_FOUND when no token is stored at all', async () => { - // Guards against a regression to the old silent no-op when the - // store is empty — `attachLogoutCommand` would otherwise emit - // ✓ Logged out for a ref that never had any backing account. - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - await expect(createOutlineTokenStore().clear(sampleAccount.id)).rejects.toMatchObject({ - code: 'ACCOUNT_NOT_FOUND', - }) - expect(existsSync(TEST_CONFIG_PATH)).toBe(false) - }) + await createOutlineTokenStore().set(STORED_ACCOUNT, 'tk_new') - it('list() returns the stored account flagged as default', async () => { - writeFileSync(TEST_CONFIG_PATH, JSON.stringify(sampleConfig)) - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - expect(await createOutlineTokenStore().list()).toEqual([ - { account: sampleAccount, isDefault: true }, - ]) - }) + expect(callOrder).toEqual(['migrate', 'set']) + }) +}) - it('list() returns an empty array when no token is stored', async () => { - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - expect(await createOutlineTokenStore().list()).toEqual([]) - }) +describe('matchOutlineAccount', () => { + it('matches the UUID exactly and the label case-insensitively', async () => { + const { matchOutlineAccount } = await import('../lib/auth-provider.js') + expect(matchOutlineAccount(STORED_ACCOUNT, 'user-uuid')).toBe(true) + expect(matchOutlineAccount(STORED_ACCOUNT, 'ADA')).toBe(true) + expect(matchOutlineAccount(STORED_ACCOUNT, 'ada')).toBe(true) + expect(matchOutlineAccount(STORED_ACCOUNT, 'other-user')).toBe(false) + // Case-sensitive on the UUID — would never collide with a label. + expect(matchOutlineAccount(STORED_ACCOUNT, 'USER-UUID')).toBe(false) + }) +}) + +describe('getActiveTokenSource', () => { + beforeEach(() => { + delete process.env[TOKEN_ENV_VAR] + configMocks.getConfig.mockReset() + }) + + afterEach(() => { + vi.unstubAllEnvs() + }) - it('setDefault(ref) resolves silently when the ref matches', async () => { - writeFileSync(TEST_CONFIG_PATH, JSON.stringify(sampleConfig)) - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - await expect( - createOutlineTokenStore().setDefault(sampleAccount.id), - ).resolves.toBeUndefined() + it('reports the storage location of the active token, mirroring active()s resolution order', async () => { + const { getActiveTokenSource } = await import('../lib/auth-provider.js') + + vi.stubEnv(TOKEN_ENV_VAR, 'tk') + configMocks.getConfig.mockResolvedValue({}) + await expect(getActiveTokenSource()).resolves.toBe('env') + vi.unstubAllEnvs() + + configMocks.getConfig.mockResolvedValue({ + users: [{ id: 'u', name: 'Ada', token: 'plaintext' }], }) + await expect(getActiveTokenSource()).resolves.toBe('config-file') + + configMocks.getConfig.mockResolvedValue({ users: [{ id: 'u', name: 'Ada' }] }) + await expect(getActiveTokenSource()).resolves.toBe('secure-store') - it('setDefault(ref) throws ACCOUNT_NOT_FOUND when the ref does not match', async () => { - writeFileSync(TEST_CONFIG_PATH, JSON.stringify(sampleConfig)) - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - await expect(createOutlineTokenStore().setDefault('other')).rejects.toMatchObject({ - code: 'ACCOUNT_NOT_FOUND', - }) + // v2 record (even without fallbackToken) wins over a lingering v1 + // plaintext slot — `active()` ignores `api_token` once a record + // exists, so this classifier must too. Regression guard for the + // pre-fix order where the v1 check ran first. + configMocks.getConfig.mockResolvedValue({ + api_token: 'stale-v1', + users: [{ id: 'u', name: 'Ada' }], }) + await expect(getActiveTokenSource()).resolves.toBe('secure-store') + + configMocks.getConfig.mockResolvedValue({ api_token: 'tk' }) + await expect(getActiveTokenSource()).resolves.toBe('config-file') + + configMocks.getConfig.mockResolvedValue({}) + await expect(getActiveTokenSource()).resolves.toBe('secure-store') }) }) diff --git a/src/__tests__/auth.test.ts b/src/__tests__/auth.test.ts index a87beb0..c4dea1d 100644 --- a/src/__tests__/auth.test.ts +++ b/src/__tests__/auth.test.ts @@ -2,11 +2,21 @@ import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { SKIPPED_RESULT } from './_fixtures/auth.js' -const TEST_XDG = join(tmpdir(), `outline-cli-test-${process.pid}`) +const TEST_XDG = join(tmpdir(), `outline-cli-test-${process.pid}-auth`) const TEST_CONFIG_DIR = join(TEST_XDG, 'outline-cli') const TEST_CONFIG_PATH = join(TEST_CONFIG_DIR, 'config.json') +// Force a `skipped` migration so the runtime falls back to the legacy +// plaintext snapshot — the v1 token in these fixtures has no live API +// behind it. Mocking `runMigrateLegacyAuth` directly is more robust than +// stubbing transitive network deps: tests don't have to know how +// migration internally decides to skip. +vi.mock('../lib/migrate-auth.js', () => ({ + runMigrateLegacyAuth: vi.fn(async () => SKIPPED_RESULT), +})) + describe('auth', () => { beforeEach(() => { process.env.XDG_CONFIG_HOME = TEST_XDG @@ -30,7 +40,7 @@ describe('auth', () => { await expect(getApiToken()).resolves.toBe('env-token') }) - it('getApiToken reads from config file', async () => { + it('getApiToken falls back to the legacy plaintext config token', async () => { writeFileSync(TEST_CONFIG_PATH, JSON.stringify({ api_token: 'file-token' })) const { getApiToken } = await import('../lib/auth.js') await expect(getApiToken()).resolves.toBe('file-token') @@ -58,18 +68,29 @@ describe('auth', () => { await expect(getBaseUrl()).resolves.toBe('https://app.getoutline.com') }) - it('clearConfig removes the saved token', async () => { + it('getBaseUrl reads from the default user record (v2)', async () => { writeFileSync( TEST_CONFIG_PATH, JSON.stringify({ - api_token: 'test-token', - base_url: 'https://wiki.test.com', - oauth_client_id: 'client-id', + config_version: 2, + users: [ + { + id: 'u', + name: 'Ada', + base_url: 'https://wiki.example.com', + token: 'tok', + }, + ], }), ) - const { clearConfig, getApiToken } = await import('../lib/auth.js') - await clearConfig() - await expect(getApiToken()).rejects.toThrow() + const { getBaseUrl } = await import('../lib/auth.js') + await expect(getBaseUrl()).resolves.toBe('https://wiki.example.com') + }) + + it('getBaseUrl reads from the legacy v1 base_url slot', async () => { + writeFileSync(TEST_CONFIG_PATH, JSON.stringify({ base_url: 'https://legacy.example.com' })) + const { getBaseUrl } = await import('../lib/auth.js') + await expect(getBaseUrl()).resolves.toBe('https://legacy.example.com') }) it('getOAuthClientId returns env var first', async () => { @@ -78,9 +99,28 @@ describe('auth', () => { await expect(getOAuthClientId()).resolves.toBe('env-client-id') }) - it('getOAuthClientId reads from config file', async () => { + it('getOAuthClientId reads from the legacy v1 config slot', async () => { writeFileSync(TEST_CONFIG_PATH, JSON.stringify({ oauth_client_id: 'file-client-id' })) const { getOAuthClientId } = await import('../lib/auth.js') await expect(getOAuthClientId()).resolves.toBe('file-client-id') }) + + it('getOAuthClientId reads from the default user record (v2)', async () => { + writeFileSync( + TEST_CONFIG_PATH, + JSON.stringify({ + config_version: 2, + users: [ + { + id: 'u', + name: 'Ada', + oauth_client_id: 'cid-record', + token: 'tok', + }, + ], + }), + ) + const { getOAuthClientId } = await import('../lib/auth.js') + await expect(getOAuthClientId()).resolves.toBe('cid-record') + }) }) diff --git a/src/__tests__/migrate-auth.test.ts b/src/__tests__/migrate-auth.test.ts new file mode 100644 index 0000000..39e525d --- /dev/null +++ b/src/__tests__/migrate-auth.test.ts @@ -0,0 +1,86 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { errResponse, okResponse } from './_fixtures/auth.js' + +const fetchMock = vi.hoisted(() => ({ fetchWithRetry: vi.fn() })) +vi.mock('../transport/fetch-with-retry.js', () => fetchMock) + +const cliCoreMock = vi.hoisted(() => ({ migrateLegacyAuth: vi.fn() })) +vi.mock('@doist/cli-core/auth', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, migrateLegacyAuth: cliCoreMock.migrateLegacyAuth } +}) + +const configMock = vi.hoisted(() => ({ + getConfig: vi.fn(), + updateConfig: vi.fn(), +})) +vi.mock('../lib/config.js', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, getConfig: configMock.getConfig, updateConfig: configMock.updateConfig } +}) + +/** Run the wrapper once, return the options handed to cli-core. */ +async function captureCliCoreOptions() { + cliCoreMock.migrateLegacyAuth.mockResolvedValue({ status: 'no-legacy-state' }) + const { runMigrateLegacyAuth } = await import('../lib/migrate-auth.js') + await runMigrateLegacyAuth() + return cliCoreMock.migrateLegacyAuth.mock.calls[0][0] +} + +describe('runMigrateLegacyAuth', () => { + beforeEach(() => { + cliCoreMock.migrateLegacyAuth.mockReset() + fetchMock.fetchWithRetry.mockReset() + configMock.getConfig.mockReset().mockResolvedValue({}) + configMock.updateConfig.mockReset().mockResolvedValue(undefined) + }) + + afterEach(() => { + vi.resetModules() + }) + + it('hands cli-core the outline-cli wiring (service, legacy slot, records store, log prefix)', async () => { + const opts = await captureCliCoreOptions() + expect(opts.serviceName).toBe('outline-cli') + expect(opts.legacyAccount).toBe('api-token') + expect(opts.logPrefix).toBe('outline-cli') + expect(opts.silent).toBe(true) + expect(typeof opts.userRecords.list).toBe('function') + }) + + it('identifyAccount POSTs auth.info to the configured base URL and builds an OutlineAccount', async () => { + const opts = await captureCliCoreOptions() + configMock.getConfig.mockResolvedValue({ + base_url: 'https://wiki.example.com/', + oauth_client_id: 'cid-xyz', + }) + fetchMock.fetchWithRetry.mockResolvedValue( + okResponse({ + data: { user: { id: 'user-uuid', name: 'Ada' }, team: { name: 'Analytics' } }, + }), + ) + + const account = await opts.identifyAccount('tk_v1') + + const args = fetchMock.fetchWithRetry.mock.calls[0][0] + expect(args.url).toBe('https://wiki.example.com/api/auth.info') + expect(args.options.headers.Authorization).toBe('Bearer tk_v1') + // Critical: timeout must be set or a stalled connection during + // lazy migration can hang every CLI invocation. Guards the + // `IDENTIFY_TIMEOUT_MS` constant in migrate-auth.ts. + expect(args.options.timeout).toBe(10_000) + expect(account).toEqual({ + id: 'user-uuid', + label: 'Ada', + baseUrl: 'https://wiki.example.com', + oauthClientId: 'cid-xyz', + teamName: 'Analytics', + }) + }) + + it('identifyAccount throws when auth.info responds with !ok (triggers identify-failed)', async () => { + const opts = await captureCliCoreOptions() + fetchMock.fetchWithRetry.mockResolvedValue(errResponse(401, 'Unauthorized')) + await expect(opts.identifyAccount('tk')).rejects.toThrow('auth.info failed: 401') + }) +}) diff --git a/src/__tests__/user-records.test.ts b/src/__tests__/user-records.test.ts new file mode 100644 index 0000000..acce45d --- /dev/null +++ b/src/__tests__/user-records.test.ts @@ -0,0 +1,180 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { STORED_ACCOUNT } from './_fixtures/auth.js' + +const configMock = vi.hoisted(() => ({ + getConfig: vi.fn(), + updateConfig: vi.fn(), +})) + +vi.mock('../lib/config.js', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, getConfig: configMock.getConfig, updateConfig: configMock.updateConfig } +}) + +const ADA = { + id: 'user-uuid', + name: 'Ada', + base_url: 'https://wiki.example.com', + oauth_client_id: 'cid-xyz', + team_name: 'Analytics', +} as const + +const GRACE = { + id: 'grace-uuid', + name: 'Grace', + base_url: 'https://other.example.com', +} as const + +describe('createOutlineUserRecordStore', () => { + beforeEach(() => { + configMock.getConfig.mockReset().mockResolvedValue({}) + configMock.updateConfig.mockReset().mockResolvedValue(undefined) + }) + + it('list() maps StoredUser → UserRecord, surfacing token as fallbackToken when present', async () => { + configMock.getConfig.mockResolvedValue({ + users: [ADA, { ...GRACE, token: ' plaintext-token ' }], + }) + const { createOutlineUserRecordStore } = await import('../lib/user-records.js') + + const records = await createOutlineUserRecordStore().list() + + expect(records).toHaveLength(2) + expect(records[0]).toEqual({ account: STORED_ACCOUNT }) + expect(records[0].fallbackToken).toBeUndefined() + expect(records[1].account.id).toBe('grace-uuid') + expect(records[1].fallbackToken).toBe('plaintext-token') + }) + + it('upsert() appends a brand-new record', async () => { + configMock.getConfig.mockResolvedValue({ users: [ADA] }) + const { createOutlineUserRecordStore } = await import('../lib/user-records.js') + + await createOutlineUserRecordStore().upsert({ + account: { + id: 'grace-uuid', + label: 'Grace', + baseUrl: 'https://other.example.com', + oauthClientId: '', + }, + }) + + expect(configMock.updateConfig).toHaveBeenCalledWith({ + users: [ + ADA, + { id: 'grace-uuid', name: 'Grace', base_url: 'https://other.example.com' }, + ], + }) + }) + + it('upsert() REPLACES an existing record in-place (preserves order, drops absent fallbackToken)', async () => { + // Critical: an absent `fallbackToken` on the new record must + // erase any prior plaintext slot. Cli-core's contract says + // `upsert` is replace-not-merge so a stale fallback can't + // shadow a fresh keyring-backed write. + configMock.getConfig.mockResolvedValue({ + users: [{ ...ADA, token: 'stale-plaintext' }, GRACE], + }) + const { createOutlineUserRecordStore } = await import('../lib/user-records.js') + + await createOutlineUserRecordStore().upsert({ account: STORED_ACCOUNT }) + + expect(configMock.updateConfig).toHaveBeenCalledWith({ + users: [ADA, GRACE], + }) + }) + + it('upsert() persists fallbackToken back to the StoredUser.token slot', async () => { + configMock.getConfig.mockResolvedValue({ users: [] }) + const { createOutlineUserRecordStore } = await import('../lib/user-records.js') + + await createOutlineUserRecordStore().upsert({ + account: STORED_ACCOUNT, + fallbackToken: 'wsl-plaintext', + }) + + expect(configMock.updateConfig).toHaveBeenCalledWith({ + users: [{ ...ADA, token: 'wsl-plaintext' }], + }) + }) + + it('remove() drops the matching record', async () => { + configMock.getConfig.mockResolvedValue({ users: [ADA, GRACE] }) + const { createOutlineUserRecordStore } = await import('../lib/user-records.js') + + await createOutlineUserRecordStore().remove('user-uuid') + + expect(configMock.updateConfig).toHaveBeenCalledWith({ users: [GRACE] }) + }) + + it('remove() also clears default_user_id when it matched the removed record', async () => { + configMock.getConfig.mockResolvedValue({ + users: [ADA, GRACE], + default_user_id: 'user-uuid', + }) + const { createOutlineUserRecordStore } = await import('../lib/user-records.js') + + await createOutlineUserRecordStore().remove('user-uuid') + + expect(configMock.updateConfig).toHaveBeenCalledWith({ + users: [GRACE], + default_user_id: undefined, + }) + }) + + it('remove() is a no-op when the id is unknown (does not touch config)', async () => { + configMock.getConfig.mockResolvedValue({ users: [ADA] }) + const { createOutlineUserRecordStore } = await import('../lib/user-records.js') + + await createOutlineUserRecordStore().remove('nobody') + + expect(configMock.updateConfig).not.toHaveBeenCalled() + }) + + it('getDefaultId / setDefaultId round-trip via the default_user_id config field', async () => { + const { createOutlineUserRecordStore } = await import('../lib/user-records.js') + const store = createOutlineUserRecordStore() + + configMock.getConfig.mockResolvedValueOnce({}) + await expect(store.getDefaultId()).resolves.toBeNull() + + configMock.getConfig.mockResolvedValueOnce({ default_user_id: 'user-uuid' }) + await expect(store.getDefaultId()).resolves.toBe('user-uuid') + + await store.setDefaultId('grace-uuid') + expect(configMock.updateConfig).toHaveBeenCalledWith({ default_user_id: 'grace-uuid' }) + + await store.setDefaultId(null) + expect(configMock.updateConfig).toHaveBeenLastCalledWith({ default_user_id: undefined }) + }) +}) + +describe('getDefaultUserRecord', () => { + it('returns null when no users are stored', async () => { + const { getDefaultUserRecord } = await import('../lib/user-records.js') + expect(getDefaultUserRecord({})).toBeNull() + expect(getDefaultUserRecord({ users: [] })).toBeNull() + }) + + it('returns the pinned default when default_user_id matches', async () => { + const { getDefaultUserRecord } = await import('../lib/user-records.js') + const record = getDefaultUserRecord({ + users: [ADA, GRACE], + default_user_id: 'grace-uuid', + }) + expect(record?.account.id).toBe('grace-uuid') + }) + + it('falls back to the first record when default_user_id is absent or stale', async () => { + const { getDefaultUserRecord } = await import('../lib/user-records.js') + + const noDefault = getDefaultUserRecord({ users: [ADA, GRACE] }) + expect(noDefault?.account.id).toBe('user-uuid') + + const staleDefault = getDefaultUserRecord({ + users: [ADA, GRACE], + default_user_id: 'nobody', + }) + expect(staleDefault?.account.id).toBe('user-uuid') + }) +}) diff --git a/src/commands/auth.ts b/src/commands/auth.ts index 8e40c0d..a2dc509 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -1,4 +1,9 @@ -import { attachLoginCommand, attachLogoutCommand, attachStatusCommand } from '@doist/cli-core/auth' +import { + attachLoginCommand, + attachLogoutCommand, + attachStatusCommand, + type TokenStorageResult, +} from '@doist/cli-core/auth' import chalk from 'chalk' import type { Command } from 'commander' import { apiRequest } from '../lib/api.js' @@ -7,7 +12,9 @@ import { type AuthInfoResponse, createOutlineAuthProvider, createOutlineTokenStore, + getActiveTokenSource, type OutlineAccount, + type OutlineTokenStore, } from '../lib/auth-provider.js' import { CliError } from '../lib/errors.js' @@ -15,7 +22,7 @@ const DEFAULT_OAUTH_CALLBACK_PORT = 54969 type StatusData = { email: string - source: 'env' | 'config' + source: 'env' | 'secure-store' | 'config-file' } function resolvePreferredCallbackPort(): number { @@ -28,11 +35,34 @@ function resolvePreferredCallbackPort(): number { return parsed } +/** + * Surface a `TokenStorageResult` from a save/clear: the human-readable + * confirmation goes to stdout, any keyring-fallback warning goes to stderr. + * Pass `isMachineOutput: true` to suppress the stdout confirmation in + * `--json` / `--ndjson` mode while still routing the warning to stderr. + * + * Exported for direct unit testing — the alternative (driving this via + * mocked cli-core login/logout hooks) would require stubbing the entire + * store contract just to assert two console calls. + */ +export function logTokenStorageResult( + result: TokenStorageResult, + secureStoreMessage: string, + isMachineOutput = false, +): void { + if (!isMachineOutput && result.storage === 'secure-store') { + console.log(chalk.dim(secureStoreMessage)) + } + if (result.warning) { + console.error(chalk.yellow('Warning:'), result.warning) + } +} + export function registerAuthCommand(program: Command): void { const auth = program.command('auth').description('Manage authentication') const provider = createOutlineAuthProvider() - const store = createOutlineTokenStore() + const store: OutlineTokenStore = createOutlineTokenStore() attachLoginCommand(auth, { provider, @@ -42,8 +72,18 @@ export function registerAuthCommand(program: Command): void { renderSuccess, renderError, onSuccess({ view, account }) { - if (view.json || view.ndjson) return - console.log(chalk.green(`Authenticated to ${account.teamName} as ${account.label}`)) + const isMachineOutput = view.json || view.ndjson + if (!isMachineOutput) { + console.log(chalk.green(`Authenticated to ${account.teamName} as ${account.label}`)) + } + const result = store.getLastStorageResult() + if (result) { + logTokenStorageResult( + result, + 'Token stored securely in the system credential manager', + isMachineOutput, + ) + } }, }) .description('Authenticate with an Outline instance via OAuth') @@ -66,15 +106,15 @@ export function registerAuthCommand(program: Command): void { description: 'Show current authentication state', async fetchLive({ token, account }) { try { - const { data: info } = await apiRequest( - 'auth.info', - {}, - { token, baseUrl: account.baseUrl }, - ) - statusData = { - email: info.user.email, - source: process.env.OUTLINE_API_TOKEN ? 'env' : 'config', - } + const [{ data: info }, source] = await Promise.all([ + apiRequest( + 'auth.info', + {}, + { token, baseUrl: account.baseUrl }, + ), + getActiveTokenSource(), + ]) + statusData = { email: info.user.email, source } return { ...account, id: info.user.id, @@ -118,5 +158,14 @@ export function registerAuthCommand(program: Command): void { attachLogoutCommand(auth, { store, description: 'Clear saved authentication', + onCleared({ view }) { + const result = store.getLastClearResult() + if (!result) return + logTokenStorageResult( + result, + 'Stored token removed from the system credential manager', + view.json || view.ndjson, + ) + }, }) } diff --git a/src/lib/auth-constants.ts b/src/lib/auth-constants.ts new file mode 100644 index 0000000..360a950 --- /dev/null +++ b/src/lib/auth-constants.ts @@ -0,0 +1,29 @@ +import type { Config } from './config.js' + +/** OS keyring `service` identifier for every outline-cli secret. */ +export const SECURE_STORE_SERVICE = 'outline-cli' + +/** Env var that short-circuits the token store with a manually-supplied token. */ +export const TOKEN_ENV_VAR = 'OUTLINE_API_TOKEN' + +/** + * Legacy single-user keyring slot. Outline never wrote a token to the + * keyring before this migration, so there is no real legacy slot to read — + * `migrateLegacyAuth` still expects the option, and its best-effort delete + * after migration is a harmless no-op on an empty entry. + */ +export const LEGACY_KEYRING_ACCOUNT = 'api-token' + +/** + * `updateConfig` payload that wipes every v1 auth key. Used by both the + * one-shot migration `cleanupLegacyConfig` and the runtime + * `dischargeLegacyState` so both paths stay in lockstep. + */ +export const LEGACY_CLEAR_PAYLOAD: Partial = { + api_token: undefined, + base_url: undefined, + oauth_client_id: undefined, + auth_user_id: undefined, + auth_user_name: undefined, + auth_team_name: undefined, +} diff --git a/src/lib/auth-provider.ts b/src/lib/auth-provider.ts index 4254e40..24045ad 100644 --- a/src/lib/auth-provider.ts +++ b/src/lib/auth-provider.ts @@ -1,32 +1,30 @@ import { createInterface } from 'node:readline/promises' import { type AccountRef, - type AuthAccount, type AuthProvider, + createKeyringTokenStore, deriveChallenge, generateVerifier, - type TokenStore, + type KeyringTokenStore, + type MigrateAuthResult, } from '@doist/cli-core/auth' import { fetchWithRetry } from '../transport/fetch-with-retry.js' import { apiRequest } from './api.js' -import { clearConfig, getBaseUrl, getOAuthClientId } from './auth.js' -import { type Config, getConfig, updateConfig } from './config.js' -import { CliError } from './errors.js' +import { LEGACY_CLEAR_PAYLOAD, SECURE_STORE_SERVICE, TOKEN_ENV_VAR } from './auth-constants.js' +import { getBaseUrl, getOAuthClientId } from './auth.js' +import { getConfig, getConfigPath, updateConfig } from './config.js' +import { runMigrateLegacyAuth } from './migrate-auth.js' +import { makeOutlineAccount, type OutlineAccount } from './outline-account.js' +import { createOutlineUserRecordStore, getDefaultUserRecord } from './user-records.js' -const DEFAULT_BASE_URL = 'https://app.getoutline.com' +export type { OutlineAccount } from './outline-account.js' export type AuthInfoResponse = { user: { id: string; name: string; email: string } team: { name: string; subdomain: string } } -export type OutlineAccount = AuthAccount & { - id: string - label: string - baseUrl: string - oauthClientId: string - teamName?: string -} +export type OutlineTokenStore = KeyringTokenStore type OutlineHandshake = Record & { baseUrl: string @@ -150,115 +148,204 @@ export function createOutlineAuthProvider(): AuthProvider { baseUrl: hs.baseUrl, }, ) - return { + return makeOutlineAccount({ id: data.user.id, label: data.user.name, baseUrl: hs.baseUrl, oauthClientId: hs.clientId, teamName: data.team.name, - } + }) }, } } -export function createOutlineTokenStore(): TokenStore { - /** - * Derive a snapshot from an already-loaded config. Pure, so ref-aware - * callers can validate without a second config read. - */ - function deriveSnapshot( - config: Partial, - ): { token: string; account: OutlineAccount } | null { - if (!config.api_token) return null - const id = config.auth_user_id - const label = config.auth_user_name - if (!id || !label) { - // Stored token predates this adapter (env var, pre-upgrade - // config). No persisted identity to round-trip. - return null - } - return { - token: config.api_token, - account: { - id, - label, - baseUrl: config.base_url ?? DEFAULT_BASE_URL, - oauthClientId: config.oauth_client_id ?? '', - teamName: config.auth_team_name, - }, - } - } +/** + * Accepts the Outline user UUID or display name. Id matches are + * case-sensitive (UUIDs are canonical); label matches are + * case-insensitive so users can pass the name they see in `auth status`. + */ +export function matchOutlineAccount(account: OutlineAccount, ref: AccountRef): boolean { + if (account.id === ref) return true + return account.label.toLowerCase() === ref.toLowerCase() +} + +/** True when the v2 store is the authoritative source. */ +function migrationIsConclusive(result: MigrateAuthResult): boolean { + return ( + result.status === 'migrated' || + result.status === 'already-migrated' || + result.status === 'no-legacy-state' + ) +} - /** - * Match the stored account against `--user `. Outline accounts use - * UUID ids and a display name — id matches are case-sensitive (UUIDs - * are canonical), label matches are case-insensitive so users can pass - * the name they see in `auth status` regardless of casing. - */ - function matchesRef(account: OutlineAccount, ref: AccountRef): boolean { - if (account.id === ref) return true - return account.label.toLowerCase() === ref.toLowerCase() +/** + * Synthesise a snapshot from v1 plaintext state still on disk. Fallback for + * when migration can't complete (offline `identifyAccount`, WSL + * `legacy-keyring-unreachable`). Returns `null` when no legacy token is + * present. The synthesised account uses whatever identity fields the v1 + * config has — for pre-#71 configs the id/label may be empty, but the + * runtime never renders those (status's `fetchLive` re-derives from the + * API), so empty placeholders are safe. + */ +async function readLegacyTokenSnapshot(): Promise<{ + token: string + account: OutlineAccount +} | null> { + const config = await getConfig() + const token = config.api_token?.trim() || null + if (!token) return null + return { + token, + account: makeOutlineAccount({ + id: config.auth_user_id ?? '', + label: config.auth_user_name ?? '', + baseUrl: config.base_url, + oauthClientId: config.oauth_client_id, + teamName: config.auth_team_name, + }), } +} + +/** + * Discharge v1 plaintext state. Runs **after** a successful v2 write/clear + * — never before — so a v2-op failure doesn't strand the user with no + * recoverable credentials. Caller decides whether to swallow or propagate + * the `updateConfig` failure: + * - `set()` swallows (v2 record will win in `active()` regardless). + * - `clear()` propagates (v2 is empty, so a stale legacy token would + * shadow the logout via the fallback). + */ +async function dischargeLegacyState(): Promise { + await updateConfig(LEGACY_CLEAR_PAYLOAD) +} - function refMismatch(ref: AccountRef): CliError { - return new CliError('ACCOUNT_NOT_FOUND', `No stored account matches "${ref}".`) +/** + * Memoised one-shot migration trigger. Resolves with `null` on rejection so + * the CLI never fails to start because of a migration error — the legacy + * snapshot fallback handles that case. Tests reset the memo with + * `vi.resetModules()` + a dynamic re-import. + */ +let migrationPromise: Promise | null> | undefined +function ensureMigrated(): Promise | null> { + if (!migrationPromise) { + migrationPromise = runMigrateLegacyAuth({ silent: true }).catch(() => null) } + return migrationPromise +} + +/** + * True when the v2 store is empty but a legacy v1 token snapshot is still + * the only thing keeping the CLI authenticated — typically because + * `migrateLegacyAuth` couldn't reach the Outline API to identify the + * account (`MigrateSkipReason: 'identify-failed'`) or the OS keyring is + * unreachable (`'legacy-keyring-unreachable'`, although outline has no + * v1 keyring slot to read from). Useful for downstream diagnostics. + */ +export async function isLegacyAuthActive(): Promise { + const result = await ensureMigrated() + if (result !== null && migrationIsConclusive(result)) return false + const legacy = await readLegacyTokenSnapshot() + return legacy !== null +} +/** + * `OUTLINE_API_TOKEN` short-circuits `active()` only when no explicit ref + * is supplied — cli-core's `KeyringTokenStore` doesn't know about the env + * var, and an explicit ref means the caller targets a specific stored + * account. + * + * `ensureMigrated()` runs **before** every mutating v2 op so the post-op + * legacy discharge can't race a still-pending migration into re-grabbing + * the legacy `api_token` we just consumed. `set()` / `clear()` then + * discharge legacy state **after** the v2 op succeeds. `set()` swallows + * the cleanup failure (v2 wins in `active()` regardless); `clear()` + * propagates it so a failed logout fails loudly instead of leaving the + * user authenticated via the legacy fallback. + */ +export function createOutlineTokenStore(): OutlineTokenStore { + const inner = createKeyringTokenStore({ + serviceName: SECURE_STORE_SERVICE, + userRecords: createOutlineUserRecordStore(), + recordsLocation: getConfigPath(), + matchAccount: matchOutlineAccount, + }) + async function migrationIsInconclusive(): Promise { + const result = await ensureMigrated() // memoised + return result === null || !migrationIsConclusive(result) + } return { async active(ref?: AccountRef) { - // Env token wins per the `getApiToken` cascade. Surface it as a - // snapshot with placeholder identity fields — `status`'s - // `fetchLive` re-derives the canonical account from the API, so - // the empty id/label here are never rendered. Returning a - // snapshot here is what makes `attachStatusCommand` / - // `attachLogoutCommand` work for `OUTLINE_API_TOKEN`-only users. - const envToken = process.env.OUTLINE_API_TOKEN?.trim() - if (envToken) { - const account: OutlineAccount = { - id: '', - label: '', - baseUrl: await getBaseUrl(), - oauthClientId: '', + if (ref === undefined) { + const envToken = process.env[TOKEN_ENV_VAR]?.trim() + if (envToken) { + return { + token: envToken, + account: makeOutlineAccount({ + id: '', + label: '', + baseUrl: await getBaseUrl(), + }), + } } - if (ref !== undefined && !matchesRef(account, ref)) throw refMismatch(ref) - return { token: envToken, account } } - const snapshot = deriveSnapshot(await getConfig()) - if (ref === undefined) return snapshot - if (!snapshot || !matchesRef(snapshot.account, ref)) throw refMismatch(ref) - return snapshot + await ensureMigrated() + const fromStore = await inner.active(ref) + if (fromStore) return fromStore + + const legacy = await readLegacyTokenSnapshot() + if (legacy && (ref === undefined || matchOutlineAccount(legacy.account, ref))) { + return legacy + } + return null }, - async set(account, token) { - await updateConfig({ - api_token: token, - base_url: account.baseUrl, - oauth_client_id: account.oauthClientId, - auth_user_id: account.id, - auth_user_name: account.label, - auth_team_name: account.teamName, - }) + async set(account: OutlineAccount, token: string) { + await ensureMigrated() + await inner.set(account, token) + if (await migrationIsInconclusive()) { + // Best-effort: a lingering `api_token` is dormant because + // `active()` reads v2 first. + await dischargeLegacyState().catch(() => undefined) + } }, async clear(ref?: AccountRef) { - // With `ref`, validate before touching storage so a mismatch is - // an `ACCOUNT_NOT_FOUND` error rather than a silent success — - // `attachLogoutCommand` treats any non-throwing `clear()` as - // success. Load the config once and hand it to `clearConfig` - // so ref-based logout stays at one read + one write. - const config = await getConfig() - if (ref !== undefined) { - const snapshot = deriveSnapshot(config) - if (!snapshot || !matchesRef(snapshot.account, ref)) throw refMismatch(ref) + await ensureMigrated() + await inner.clear(ref) + if (await migrationIsInconclusive()) { + // Must succeed: v2 is now empty, so a surviving legacy + // token would shadow the logout via the fallback. + await dischargeLegacyState() } - await clearConfig(config) }, async list() { - const snapshot = deriveSnapshot(await getConfig()) - return snapshot ? [{ account: snapshot.account, isDefault: true }] : [] + await ensureMigrated() + return inner.list() }, async setDefault(ref: AccountRef) { - const snapshot = deriveSnapshot(await getConfig()) - if (!snapshot || !matchesRef(snapshot.account, ref)) throw refMismatch(ref) - // Single-user store — already the default once `ref` matches. + await ensureMigrated() + return inner.setDefault(ref) }, + getLastStorageResult: () => inner.getLastStorageResult(), + getLastClearResult: () => inner.getLastClearResult(), } } + +/** + * Where the currently-active token lives. Mirrors `active()`'s resolution + * order — env → v2 record → legacy plaintext — so the answer can never + * contradict the token the runtime is actually using. + * + * The precedence cascade is intentionally duplicated with `active()`: + * the only true dedupe options either (a) add an extra config read on + * every `apiRequest` call (regressing the request hot path) or (b) + * augment cli-core's `TokenStore` contract. The drift the duplication + * invites is guarded by the `getActiveTokenSource` regression test that + * asserts v2-record presence wins over a lingering v1 `api_token`. + */ +export async function getActiveTokenSource(): Promise<'env' | 'secure-store' | 'config-file'> { + if (process.env[TOKEN_ENV_VAR]?.trim()) return 'env' + const config = await getConfig() + const record = getDefaultUserRecord(config) + if (record) return record.fallbackToken ? 'config-file' : 'secure-store' + if (config.api_token?.trim()) return 'config-file' + return 'secure-store' +} diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 1077fab..6a40df0 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -1,67 +1,82 @@ -import { type Config, getConfig, setConfig } from './config.js' +import { SecureStoreUnavailableError } from '@doist/cli-core/auth' +import { TOKEN_ENV_VAR } from './auth-constants.js' +import { + createOutlineTokenStore, + getActiveTokenSource, + type OutlineTokenStore, +} from './auth-provider.js' +import { getConfig } from './config.js' +import { CliError } from './errors.js' +import { DEFAULT_BASE_URL } from './outline-account.js' +import { getDefaultUserRecord } from './user-records.js' -const DEFAULT_BASE_URL = 'https://app.getoutline.com' +export { SecureStoreUnavailableError, getActiveTokenSource, TOKEN_ENV_VAR } -export async function getApiToken(): Promise { - const envToken = process.env.OUTLINE_API_TOKEN - if (envToken) return envToken +export class NoTokenError extends CliError { + constructor() { + super( + 'NO_TOKEN', + `No API token found. Set ${TOKEN_ENV_VAR} env var or run: ol auth login`, + [`Set ${TOKEN_ENV_VAR} or run: ol auth login`], + 'info', + ) + this.name = 'NoTokenError' + } +} - const config = await getConfig() - if (config.api_token) return config.api_token +/** + * Module-level token-store singleton. Built lazily on first call; reused + * across every `apiRequest` so the request hot path doesn't reconstruct + * the keyring + user-record adapters per POST. + */ +let storeSingleton: OutlineTokenStore | undefined +function tokenStore(): OutlineTokenStore { + if (!storeSingleton) storeSingleton = createOutlineTokenStore() + return storeSingleton +} - throw new Error('No API token found. Set OUTLINE_API_TOKEN env var or run: ol auth login') +/** + * Read the active token. Hot path: when `OUTLINE_API_TOKEN` is set we + * return it directly without consulting the token store, since + * `apiRequest` already resolves the base URL separately — going through + * `store.active()` here would trigger a redundant `getBaseUrl()` lookup + * per request just to synthesise an account we don't need. + */ +export async function getApiToken(): Promise { + const envToken = process.env[TOKEN_ENV_VAR]?.trim() + if (envToken) return envToken + const snapshot = await tokenStore().active() + if (!snapshot?.token) throw new NoTokenError() + return snapshot.token } +/** + * Base URL cascade: env var → default user record (v2) → legacy + * `base_url` config (v1) → built-in default. The record takes priority + * over the legacy slot so post-migration logins keep defaulting to the + * same Outline instance. + */ export async function getBaseUrl(): Promise { const envUrl = process.env.OUTLINE_URL if (envUrl) return envUrl.replace(/\/$/, '') const config = await getConfig() + const record = getDefaultUserRecord(config) + if (record?.account.baseUrl) return record.account.baseUrl.replace(/\/$/, '') if (config.base_url) return config.base_url.replace(/\/$/, '') - return DEFAULT_BASE_URL } +/** + * OAuth client id cascade: env var → default user record (v2) → legacy + * `oauth_client_id` config (v1) → undefined (caller prompts). + */ export async function getOAuthClientId(): Promise { const envClientId = process.env.OUTLINE_OAUTH_CLIENT_ID if (envClientId) return envClientId const config = await getConfig() + const record = getDefaultUserRecord(config) + if (record?.account.oauthClientId) return record.account.oauthClientId return config.oauth_client_id } - -export async function getTokenSource(): Promise<'env' | 'config' | null> { - if (process.env.OUTLINE_API_TOKEN) return 'env' - const config = await getConfig() - if (config.api_token) return 'config' - return null -} - -/** - * Clear the auth-related keys without deleting the file. The config is now - * shared with non-auth settings (notably `update_channel`); a blanket unlink - * would silently reset the user's update-channel preference too. - * - * Pass `existing` to skip the read when the caller has already loaded the - * config (e.g. ref-validated logout) — keeps that flow at one read + one - * write instead of two reads. - */ -export async function clearConfig(existing?: Partial): Promise { - const config = existing ?? (await getConfig()) - const { - api_token, - base_url, - oauth_client_id, - auth_user_id, - auth_user_name, - auth_team_name, - ...rest - } = config - void api_token - void base_url - void oauth_client_id - void auth_user_id - void auth_user_name - void auth_team_name - await setConfig(rest as Config) -} diff --git a/src/lib/config.ts b/src/lib/config.ts index baf7dda..46ce074 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -8,7 +8,29 @@ import { const APP_NAME = 'outline-cli' +/** + * One row of the `users[]` array. `id` is the Outline user UUID. `token` is + * a plaintext fallback persisted only when the OS keyring is unavailable at + * write time (WSL, headless Linux, missing native binary). + */ +export type StoredUser = { + id: string + name: string + base_url?: string + oauth_client_id?: string + team_name?: string + token?: string +} + export type Config = CoreConfig & { + config_version?: number + users?: StoredUser[] + default_user_id?: string + + /** + * Legacy v1 single-user fields. Read by the migration helper; removed + * from disk by `cleanupLegacyConfig` after the v2 record write succeeds. + */ api_token?: string base_url?: string oauth_client_id?: string diff --git a/src/lib/migrate-auth.ts b/src/lib/migrate-auth.ts new file mode 100644 index 0000000..485c46d --- /dev/null +++ b/src/lib/migrate-auth.ts @@ -0,0 +1,106 @@ +import { type MigrateAuthResult, migrateLegacyAuth } from '@doist/cli-core/auth' +import { fetchWithRetry } from '../transport/fetch-with-retry.js' +import { + LEGACY_CLEAR_PAYLOAD, + LEGACY_KEYRING_ACCOUNT, + SECURE_STORE_SERVICE, +} from './auth-constants.js' +import { getConfig, updateConfig } from './config.js' +import { DEFAULT_BASE_URL, makeOutlineAccount, type OutlineAccount } from './outline-account.js' +import { createOutlineUserRecordStore } from './user-records.js' + +/** + * Pinned to this migration's target schema. Decoupled from any future + * `config_version` bump so this helper doesn't re-run for users already + * on v2 or beyond. + */ +const V2_SCHEMA_VERSION = 2 + +/** + * Cap migration's `auth.info` lookup so a stalled connection can't hang + * the CLI indefinitely on first command. cli-core treats the resulting + * timeout as `identify-failed`, falling back to the legacy snapshot. + */ +const IDENTIFY_TIMEOUT_MS = 10_000 + +type AuthInfoResponse = { + data: { + user: { id: string; name: string } + team: { name: string } + } +} + +/** + * Direct `auth.info` call used only by the migration path. We bypass + * `apiRequest` (and its spinner + `getApiToken`/`getBaseUrl` cascade) so + * this module stays out of the runtime auth/token-store import graph and + * so the silent migration doesn't render a spinner during postinstall-style + * invocations. + */ +async function identifyOutlineAccount(token: string): Promise { + const config = await getConfig() + const baseUrl = (config.base_url ?? DEFAULT_BASE_URL).replace(/\/$/, '') + const res = await fetchWithRetry({ + url: `${baseUrl}/api/auth.info`, + options: { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${token}`, + }, + body: JSON.stringify({}), + timeout: IDENTIFY_TIMEOUT_MS, + }, + }) + if (!res.ok) { + throw new Error(`auth.info failed: ${res.status} ${res.statusText}`) + } + const json: AuthInfoResponse = await res.json() + return makeOutlineAccount({ + id: json.data.user.id, + label: json.data.user.name, + baseUrl, + oauthClientId: config.oauth_client_id ?? '', + teamName: json.data.team.name, + }) +} + +/** + * One-time migration of v1 plaintext auth state into the v2 `users[]` + * shape. Called lazily by `createOutlineTokenStore` on first store access. + * Idempotent via the `config_version` marker — once set to `>= 2`, this + * helper short-circuits with `already-migrated`. + */ +export async function runMigrateLegacyAuth( + options: { silent: boolean } = { silent: true }, +): Promise> { + return migrateLegacyAuth({ + serviceName: SECURE_STORE_SERVICE, + legacyAccount: LEGACY_KEYRING_ACCOUNT, + userRecords: createOutlineUserRecordStore(), + hasMigrated: async () => { + const config = await getConfig() + // Two signals of "already on v2": the explicit marker, or + // the structural presence of `users[]` (a fresh install that + // logged in directly via cli-core writes records without ever + // setting `config_version`, so the marker alone would + // miscount that case as "needs migration"). + if ((config.config_version ?? 0) >= V2_SCHEMA_VERSION) return true + if ((config.users ?? []).length > 0) return true + return false + }, + markMigrated: async () => { + await updateConfig({ config_version: V2_SCHEMA_VERSION }) + }, + loadLegacyPlaintextToken: async () => { + const config = await getConfig() + return config.api_token?.trim() || null + }, + identifyAccount: identifyOutlineAccount, + cleanupLegacyConfig: async () => { + await updateConfig(LEGACY_CLEAR_PAYLOAD) + }, + silent: options.silent, + logPrefix: 'outline-cli', + }) +} diff --git a/src/lib/outline-account.ts b/src/lib/outline-account.ts new file mode 100644 index 0000000..6e46cc9 --- /dev/null +++ b/src/lib/outline-account.ts @@ -0,0 +1,37 @@ +import type { AuthAccount } from '@doist/cli-core/auth' + +/** + * Narrow account shape persisted by the keyring-backed token store. + * + * `id` is the Outline user UUID; `label` is the user's display name. + * `baseUrl` and `oauthClientId` ride along on the account because Outline + * is self-hostable — every persisted token is tied to the instance it was + * issued for, not to a global default. `teamName` is rendered by + * `ol auth status`; it's the only field we cache for display. + */ +export type OutlineAccount = AuthAccount & { + id: string + label: string + baseUrl: string + oauthClientId: string + teamName?: string +} + +export const DEFAULT_BASE_URL = 'https://app.getoutline.com' + +/** Canonical `OutlineAccount` factory. Applies the `baseUrl` default. */ +export function makeOutlineAccount(input: { + id: string + label: string + baseUrl?: string + oauthClientId?: string + teamName?: string +}): OutlineAccount { + return { + id: input.id, + label: input.label, + baseUrl: input.baseUrl ?? DEFAULT_BASE_URL, + oauthClientId: input.oauthClientId ?? '', + teamName: input.teamName, + } +} diff --git a/src/lib/user-records.ts b/src/lib/user-records.ts new file mode 100644 index 0000000..4e7f7c9 --- /dev/null +++ b/src/lib/user-records.ts @@ -0,0 +1,86 @@ +import type { UserRecord, UserRecordStore } from '@doist/cli-core/auth' +import { type Config, getConfig, type StoredUser, updateConfig } from './config.js' +import { makeOutlineAccount, type OutlineAccount } from './outline-account.js' + +/** + * `UserRecordStore` adapter over `config.users[]`. `StoredUser.token` is + * surfaced as `fallbackToken` — cli-core's name for the plaintext copy + * persisted only when the keyring is unreachable. + */ +export function createOutlineUserRecordStore(): UserRecordStore { + return { + async list() { + const config = await getConfig() + return (config.users ?? []).map(toRecord) + }, + async upsert(record) { + const config = await getConfig() + const existing = config.users ?? [] + const next = fromRecord(record) + const index = existing.findIndex((u) => u.id === record.account.id) + const users = + index >= 0 + ? [...existing.slice(0, index), next, ...existing.slice(index + 1)] + : [...existing, next] + await updateConfig({ users }) + }, + async remove(id) { + const config = await getConfig() + const existing = config.users ?? [] + const index = existing.findIndex((u) => u.id === id) + if (index < 0) return + const users = [...existing.slice(0, index), ...existing.slice(index + 1)] + const updates: { users: StoredUser[]; default_user_id?: undefined } = { users } + if (config.default_user_id === id) updates.default_user_id = undefined + await updateConfig(updates) + }, + async getDefaultId() { + const config = await getConfig() + return config.default_user_id ?? null + }, + async setDefaultId(id) { + await updateConfig({ default_user_id: id ?? undefined }) + }, + } +} + +/** + * Resolve the default-or-first `UserRecord` from an already-loaded config. + * Returns `null` when no users are stored. + */ +export function getDefaultUserRecord(config: Partial): UserRecord | null { + const users = config.users ?? [] + if (users.length === 0) return null + const defaultId = config.default_user_id + const user = (defaultId && users.find((u) => u.id === defaultId)) || users[0] + return toRecord(user) +} + +function toRecord(user: StoredUser): UserRecord { + const account = makeOutlineAccount({ + id: user.id, + label: user.name, + baseUrl: user.base_url, + oauthClientId: user.oauth_client_id, + teamName: user.team_name, + }) + const trimmed = user.token?.trim() + const record: UserRecord = { account } + if (trimmed) record.fallbackToken = trimmed + return record +} + +function fromRecord(record: UserRecord): StoredUser { + // Replace, don't merge: an absent `fallbackToken` strips the plaintext + // slot so it can't shadow a fresh keyring-backed write. cli-core contract. + const trimmed = record.fallbackToken?.trim() + const next: StoredUser = { + id: record.account.id, + name: record.account.label, + } + if (record.account.baseUrl) next.base_url = record.account.baseUrl + if (record.account.oauthClientId) next.oauth_client_id = record.account.oauthClientId + if (record.account.teamName) next.team_name = record.account.teamName + if (trimmed && trimmed.length > 0) next.token = trimmed + return next +}