From 4985d78fffedfc1cc64e3813ee6f210c485d0216 Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Sun, 24 May 2026 16:51:14 +0100 Subject: [PATCH 1/3] feat(auth): add `token` command group (save + view) `ol auth token [token]` saves a personal Outline API token: it validates the token via `auth.info`, resolves the real account identity (user id, name, team), and persists a full account record so `--user`, `account list`, and `auth status` all work. Reuses the same base-URL cascade as `auth login` (`--base-url` -> $OUTLINE_URL -> saved default -> prompt) and prompts for the token with masked input when no argument is given. `ol auth token view` delegates to cli-core's `attachTokenViewCommand` to print the bare stored token to stdout for scripts, with a newline only on a TTY and a refusal when the token comes from $OUTLINE_API_TOKEN. Any `auth.info` failure on save (bad token, wrong instance, unreachable host) is wrapped as a single AUTH_VERIFICATION_FAILED error with a base-URL hint, rather than leaking the raw API string. Co-Authored-By: Claude Opus 4.7 (1M context) --- skills/outline-cli/SKILL.md | 5 + src/commands/auth-token.test.ts | 184 ++++++++++++++++++++++++++++++++ src/commands/auth.ts | 107 ++++++++++++++++++- src/lib/auth-provider.ts | 2 +- src/lib/skills/content.ts | 5 + 5 files changed, 301 insertions(+), 2 deletions(-) create mode 100644 src/commands/auth-token.test.ts diff --git a/skills/outline-cli/SKILL.md b/skills/outline-cli/SKILL.md index b228966..6acf1fb 100644 --- a/skills/outline-cli/SKILL.md +++ b/skills/outline-cli/SKILL.md @@ -88,6 +88,11 @@ ol auth status # Show current auth state ol auth status --json | --ndjson # Machine-readable status envelope ({id, team, baseUrl, source}) ol auth logout # Clear saved credentials ol auth logout --json | --ndjson # Machine-readable logout envelope ({ok: true}; --ndjson is silent) +ol auth token # Save a personal API token (validates via auth.info, resolves identity) +ol auth token --base-url # Save a token for a specific Outline instance +ol auth token # Prompt for the token (hidden input; errors in non-interactive shells) +ol auth token view # Print the bare stored token to stdout for scripts (no newline when piped) +ol auth token view --user # Print a specific stored account's token; refuses when OUTLINE_API_TOKEN is set ``` ### Accounts diff --git a/src/commands/auth-token.test.ts b/src/commands/auth-token.test.ts new file mode 100644 index 0000000..29efe63 --- /dev/null +++ b/src/commands/auth-token.test.ts @@ -0,0 +1,184 @@ +import { captureConsole, captureStream, createTestProgram } from '@doist/cli-core/testing' +import type { Command } from 'commander' +import { type MockInstance, afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { AUTH_INFO, STORED_ACCOUNT, STORED_ACCOUNT_BOB } from '../_fixtures/auth.js' + +// `auth token` save drives the raw store's `set` + `getLastStorageResult`; +// `auth token view` (real cli-core attacher) reads through the ref-aware store's +// `active` / `activeAccount`. Stub the store so neither path touches a keyring. +const storeMocks = vi.hoisted(() => ({ + set: vi.fn(), + getLastStorageResult: vi.fn(() => undefined), + active: vi.fn(), + activeAccount: vi.fn(async () => ({ account: STORED_ACCOUNT, isDefault: true })), +})) + +vi.mock('../lib/auth-provider.js', async (importOriginal) => { + const actual = await importOriginal() + return { ...actual, createOutlineTokenStore: () => storeMocks } +}) + +vi.mock('../lib/api.js', () => ({ apiRequest: vi.fn() })) + +function lines(spy: MockInstance): string { + return spy.mock.calls.map((args) => args.join(' ')).join('\n') +} + +async function buildProgram(): Promise { + const { registerAuthCommand } = await import('./auth.js') + return createTestProgram(registerAuthCommand) +} + +async function importApiMock() { + const { apiRequest } = await import('../lib/api.js') + return vi.mocked(apiRequest) +} + +beforeEach(() => { + vi.resetModules() + delete process.env.OUTLINE_API_TOKEN + delete process.env.OUTLINE_URL +}) + +afterEach(() => { + vi.clearAllMocks() + delete process.env.OUTLINE_API_TOKEN + delete process.env.OUTLINE_URL + process.argv = ['node', 'ol'] +}) + +describe('auth token (save)', () => { + it('validates via auth.info, stores the resolved account, and confirms', async () => { + const log = captureConsole() + const apiRequest = await importApiMock() + apiRequest.mockResolvedValue({ data: AUTH_INFO }) + + const program = await buildProgram() + await program.parseAsync([ + 'node', + 'ol', + 'auth', + 'token', + 'tok-paste', + '--base-url', + 'https://wiki.test', + ]) + + expect(apiRequest).toHaveBeenCalledWith( + 'auth.info', + {}, + { token: 'tok-paste', baseUrl: 'https://wiki.test' }, + ) + expect(storeMocks.set).toHaveBeenCalledWith( + { + id: 'user-uuid', + label: 'Ada Lovelace', + baseUrl: 'https://wiki.test', + oauthClientId: '', + teamName: 'Analytics', + }, + 'tok-paste', + ) + expect(lines(log)).toContain('Saved token for Ada Lovelace (Analytics)') + }) + + it('translates a rejected token into AUTH_VERIFICATION_FAILED', async () => { + const apiRequest = await importApiMock() + // Outline's real invalid-token error carries no status code (api.ts drops + // it when the body has a message), so the wrapper must not depend on `401`. + apiRequest.mockRejectedValue(new Error('API error: Unable to decode token')) + + const program = await buildProgram() + await expect( + program.parseAsync([ + 'node', + 'ol', + 'auth', + 'token', + 'bad-token', + '--base-url', + 'https://wiki.test', + ]), + ).rejects.toHaveProperty('code', 'AUTH_VERIFICATION_FAILED') + expect(storeMocks.set).not.toHaveBeenCalled() + }) + + it('throws NO_TOKEN when no token is given in a non-interactive shell', async () => { + Object.defineProperty(process.stdin, 'isTTY', { value: false, configurable: true }) + const program = await buildProgram() + await expect(program.parseAsync(['node', 'ol', 'auth', 'token'])).rejects.toHaveProperty( + 'code', + 'NO_TOKEN', + ) + }) + + it('suppresses the human confirmation in machine-output mode', async () => { + const log = captureConsole() + const apiRequest = await importApiMock() + apiRequest.mockResolvedValue({ data: AUTH_INFO }) + + // `--json` is a root selector read off argv by global-args, not a + // commander option on `auth token`, so warm the cache rather than + // passing it through parseAsync. + const { resetGlobalArgs } = await import('../lib/global-args.js') + process.argv = ['node', 'ol', '--json', 'auth', 'token'] + resetGlobalArgs() + + const program = await buildProgram() + await program.parseAsync([ + 'node', + 'ol', + 'auth', + 'token', + 'tok-paste', + '--base-url', + 'https://wiki.test', + ]) + + expect(storeMocks.set).toHaveBeenCalled() + expect(lines(log)).toEqual('') + }) +}) + +describe('auth token view', () => { + it('writes the bare stored token to stdout with no envelope or newline', async () => { + storeMocks.active.mockResolvedValue({ token: 'stored-tok', account: STORED_ACCOUNT }) + const out = captureStream('stdout') + + const program = await buildProgram() + await program.parseAsync(['node', 'ol', 'auth', 'token', 'view']) + + expect(out.mock.calls).toEqual([['stored-tok']]) + }) + + it('refuses to print when OUTLINE_API_TOKEN is set', async () => { + process.env.OUTLINE_API_TOKEN = 'env-token' + const program = await buildProgram() + await expect( + program.parseAsync(['node', 'ol', 'auth', 'token', 'view']), + ).rejects.toHaveProperty('code', 'TOKEN_FROM_ENV') + }) + + it('routes a global --user through the ref-aware store', async () => { + storeMocks.active.mockImplementation(async (ref?: string) => + ref === 'Bob' + ? { token: 'tok-bob', account: STORED_ACCOUNT_BOB } + : { token: 'tok-ada', account: STORED_ACCOUNT }, + ) + storeMocks.activeAccount.mockResolvedValue({ + account: STORED_ACCOUNT_BOB, + isDefault: false, + }) + const out = captureStream('stdout') + + const { resetGlobalArgs } = await import('../lib/global-args.js') + process.argv = ['node', 'ol', '--user', 'Bob', 'auth', 'token', 'view'] + resetGlobalArgs() + + const program = await buildProgram() + await program.parseAsync(['node', 'ol', 'auth', 'token', 'view']) + + expect(storeMocks.active).toHaveBeenCalledWith('Bob') + expect(out.mock.calls).toEqual([['tok-bob']]) + }) +}) diff --git a/src/commands/auth.ts b/src/commands/auth.ts index 93a67b7..40f79a3 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -1,7 +1,14 @@ -import { attachLoginCommand, attachLogoutCommand, attachStatusCommand } from '@doist/cli-core/auth' +import { createInterface } from 'node:readline' +import { + attachLoginCommand, + attachLogoutCommand, + attachStatusCommand, + attachTokenViewCommand, +} from '@doist/cli-core/auth' import chalk from 'chalk' import type { Command } from 'commander' import { apiRequest } from '../lib/api.js' +import { TOKEN_ENV_VAR } from '../lib/auth-constants.js' import { logClearResult, logTokenStorageResult } from '../lib/auth-output.js' import { renderError, renderSuccess } from '../lib/auth-pages.js' import { @@ -11,9 +18,12 @@ import { getActiveTokenSource, type OutlineAccount, type OutlineTokenStore, + resolveBaseUrl, } from '../lib/auth-provider.js' import { refreshedTokenForStatus } from '../lib/auth.js' import { CliError } from '../lib/errors.js' +import { isJsonMode } from '../lib/global-args.js' +import { makeOutlineAccount } from '../lib/outline-account.js' import { withUserRefAware } from '../lib/user-ref-store.js' const DEFAULT_OAUTH_CALLBACK_PORT = 54969 @@ -33,6 +43,85 @@ function resolvePreferredCallbackPort(): number { return parsed } +// Read a secret without echoing it. Node exposes no public masked-prompt API, +// so we override readline's private `_writeToOutput` to suppress keystrokes and +// echo only the prompt label. Output goes to stderr so a `--json` stdout stays clean. +function promptHiddenToken(question: string): Promise { + return new Promise((resolve) => { + const rl = createInterface({ input: process.stdin, output: process.stderr }) + const internal = rl as unknown as { _writeToOutput?: (str: string) => void } + const original = internal._writeToOutput?.bind(rl) + internal._writeToOutput = (str: string) => { + if (original && str.includes(question)) original(question) + } + rl.question(question, (answer) => { + rl.close() + process.stderr.write('\n') + resolve(answer.trim()) + }) + }) +} + +async function saveToken( + store: OutlineTokenStore, + token: string | undefined, + options: { baseUrl?: string }, +): Promise { + if (!token) { + if (!process.stdin.isTTY) { + throw new CliError('NO_TOKEN', 'No token provided', [ + 'Pass it as an argument: ol auth token ', + 'Or set the OUTLINE_API_TOKEN environment variable', + 'Or use OAuth: ol auth login', + ]) + } + token = await promptHiddenToken('API token: ') + } + const trimmed = token.trim() + if (!trimmed) throw new CliError('NO_TOKEN', 'No token provided') + + const baseUrl = await resolveBaseUrl({ baseUrl: options.baseUrl }) + + // A freshly pasted token is verified by probing `auth.info`. Any failure + // (bad token, wrong instance, unreachable host) means we can't trust it, so + // surface a single actionable error rather than leaking the raw API string. + let data: AuthInfoResponse + try { + ;({ data } = await apiRequest( + 'auth.info', + {}, + { token: trimmed, baseUrl }, + )) + } catch (err) { + const detail = err instanceof Error ? err.message : String(err) + throw new CliError('AUTH_VERIFICATION_FAILED', `Could not verify token (${detail})`, [ + 'Check the token value', + `Check --base-url matches the instance the token came from (used: ${baseUrl})`, + ]) + } + + const account = makeOutlineAccount({ + id: data.user.id, + label: data.user.name, + baseUrl, + teamName: data.team.name, + }) + await store.set(account, trimmed) + + const machine = isJsonMode() + if (!machine) { + console.log(chalk.green('✓'), `Saved token for ${account.label} (${account.teamName})`) + } + const result = store.getLastStorageResult() + if (result) { + logTokenStorageResult( + result, + 'Token stored securely in the system credential manager', + machine, + ) + } +} + export function registerAuthCommand(program: Command): void { const auth = program.command('auth').description('Manage authentication') @@ -152,4 +241,20 @@ export function registerAuthCommand(program: Command): void { logClearResult(store, view.json || view.ndjson) }, }) + + const tokenCmd = auth + .command('token [token]') + .description('Save an Outline API token for CLI auth (or use the `view` subcommand)') + .option('--base-url ', 'Outline base URL the token belongs to') + .action((token: string | undefined, options: { baseUrl?: string }) => + saveToken(store, token, options), + ) + + attachTokenViewCommand(tokenCmd, { + name: 'view', + store: refAware, + envVarName: TOKEN_ENV_VAR, + description: + 'Print the stored token for the active user (or --user ) to stdout for scripts', + }) } diff --git a/src/lib/auth-provider.ts b/src/lib/auth-provider.ts index 9ef8d26..1cdf721 100644 --- a/src/lib/auth-provider.ts +++ b/src/lib/auth-provider.ts @@ -42,7 +42,7 @@ async function prompt(question: string): Promise { } } -async function resolveBaseUrl(flags: Record): Promise { +export async function resolveBaseUrl(flags: Record): Promise { const fromFlag = stringFlag(flags, 'baseUrl') if (fromFlag) return fromFlag.replace(/\/$/, '') const fromEnv = process.env.OUTLINE_URL?.trim() diff --git a/src/lib/skills/content.ts b/src/lib/skills/content.ts index 0db8cb8..5d15d43 100644 --- a/src/lib/skills/content.ts +++ b/src/lib/skills/content.ts @@ -87,6 +87,11 @@ ol auth status # Show current auth state ol auth status --json | --ndjson # Machine-readable status envelope ({id, team, baseUrl, source}) ol auth logout # Clear saved credentials ol auth logout --json | --ndjson # Machine-readable logout envelope ({ok: true}; --ndjson is silent) +ol auth token # Save a personal API token (validates via auth.info, resolves identity) +ol auth token --base-url # Save a token for a specific Outline instance +ol auth token # Prompt for the token (hidden input; errors in non-interactive shells) +ol auth token view # Print the bare stored token to stdout for scripts (no newline when piped) +ol auth token view --user # Print a specific stored account's token; refuses when OUTLINE_API_TOKEN is set \`\`\` ### Accounts From 7bd6550d0c5de0541bc085b18d530bf94c08b099 Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Sun, 24 May 2026 17:18:11 +0100 Subject: [PATCH 2/3] fix(auth): address review on `auth token` - Fix masked prompt: it dropped readline's ANSI line-clear codes, so the label repeated on every keystroke. Consolidate into a shared `prompt(question, { hidden })` in auth-provider (mute-after-prompt pattern) and drop the duplicate `promptHiddenToken`. - Collapse any `auth.info` verification failure into a single stable `AUTH_VERIFICATION_FAILED` with base-URL hints, never interpolating the raw API/network string. - Extract `identifyAccount(token, baseUrl, oauthClientId?)` and reuse it from the OAuth `validate` hook and `auth token` so identity resolution lives in one place. - `resolveBaseUrl`: explicit `{ baseUrl?: unknown }` input shape and a non-TTY fast path that returns the configured default instead of blocking a CI shell on a prompt. - Fix the SKILL doc: `--user` is a root flag (`ol --user auth token view`), not a subcommand option. - Tests: assert the wrapped error contract (message hidden, hints present), snapshot/restore `process.stdin.isTTY`, and cover the masked prompt save path. Co-Authored-By: Claude Opus 4.7 (1M context) --- skills/outline-cli/SKILL.md | 4 +- src/commands/auth-token.test.ts | 56 ++++++++++++++++++++++---- src/commands/auth.ts | 49 +++++------------------ src/lib/auth-provider.ts | 70 +++++++++++++++++++++++++-------- src/lib/skills/content.ts | 4 +- 5 files changed, 114 insertions(+), 69 deletions(-) diff --git a/skills/outline-cli/SKILL.md b/skills/outline-cli/SKILL.md index 6acf1fb..4f2d520 100644 --- a/skills/outline-cli/SKILL.md +++ b/skills/outline-cli/SKILL.md @@ -91,8 +91,8 @@ ol auth logout --json | --ndjson # Machine-readable logout envelope ({ok: ol auth token # Save a personal API token (validates via auth.info, resolves identity) ol auth token --base-url # Save a token for a specific Outline instance ol auth token # Prompt for the token (hidden input; errors in non-interactive shells) -ol auth token view # Print the bare stored token to stdout for scripts (no newline when piped) -ol auth token view --user # Print a specific stored account's token; refuses when OUTLINE_API_TOKEN is set +ol auth token view # Print the bare stored token to stdout for scripts (no newline when piped; refuses when OUTLINE_API_TOKEN is set) +ol --user auth token view # Print a specific stored account's token (--user is a root flag, before the command) ``` ### Accounts diff --git a/src/commands/auth-token.test.ts b/src/commands/auth-token.test.ts index 29efe63..d0a85a6 100644 --- a/src/commands/auth-token.test.ts +++ b/src/commands/auth-token.test.ts @@ -2,6 +2,7 @@ import { captureConsole, captureStream, createTestProgram } from '@doist/cli-cor import type { Command } from 'commander' import { type MockInstance, afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { AUTH_INFO, STORED_ACCOUNT, STORED_ACCOUNT_BOB } from '../_fixtures/auth.js' +import type { CliError } from '../lib/errors.js' // `auth token` save drives the raw store's `set` + `getLastStorageResult`; // `auth token view` (real cli-core attacher) reads through the ref-aware store's @@ -13,9 +14,15 @@ const storeMocks = vi.hoisted(() => ({ activeAccount: vi.fn(async () => ({ account: STORED_ACCOUNT, isDefault: true })), })) +// Stub the shared masked prompt so the interactive (no-argument) save path is +// testable without a real TTY. `identifyAccount` / `resolveBaseUrl` stay real. +const promptMock = vi.hoisted(() => + vi.fn<(q: string, o?: { hidden?: boolean }) => Promise>(), +) + vi.mock('../lib/auth-provider.js', async (importOriginal) => { const actual = await importOriginal() - return { ...actual, createOutlineTokenStore: () => storeMocks } + return { ...actual, createOutlineTokenStore: () => storeMocks, prompt: promptMock } }) vi.mock('../lib/api.js', () => ({ apiRequest: vi.fn() })) @@ -34,6 +41,13 @@ async function importApiMock() { return vi.mocked(apiRequest) } +// `process.stdin` is a shared global; mutating `isTTY` would bleed across tests +// (resetModules doesn't isolate it), so snapshot and restore it every test. +const ORIGINAL_STDIN_ISTTY = process.stdin.isTTY +function setStdinIsTTY(value: boolean | undefined): void { + Object.defineProperty(process.stdin, 'isTTY', { value, configurable: true }) +} + beforeEach(() => { vi.resetModules() delete process.env.OUTLINE_API_TOKEN @@ -45,6 +59,7 @@ afterEach(() => { delete process.env.OUTLINE_API_TOKEN delete process.env.OUTLINE_URL process.argv = ['node', 'ol'] + setStdinIsTTY(ORIGINAL_STDIN_ISTTY) }) describe('auth token (save)', () => { @@ -82,15 +97,15 @@ describe('auth token (save)', () => { expect(lines(log)).toContain('Saved token for Ada Lovelace (Analytics)') }) - it('translates a rejected token into AUTH_VERIFICATION_FAILED', async () => { + it('collapses any auth.info failure into a leak-free AUTH_VERIFICATION_FAILED', async () => { const apiRequest = await importApiMock() // Outline's real invalid-token error carries no status code (api.ts drops - // it when the body has a message), so the wrapper must not depend on `401`. + // it when the body has a message); the wrapper must hide it entirely. apiRequest.mockRejectedValue(new Error('API error: Unable to decode token')) const program = await buildProgram() - await expect( - program.parseAsync([ + const err = (await program + .parseAsync([ 'node', 'ol', 'auth', @@ -98,18 +113,43 @@ describe('auth token (save)', () => { 'bad-token', '--base-url', 'https://wiki.test', - ]), - ).rejects.toHaveProperty('code', 'AUTH_VERIFICATION_FAILED') + ]) + .then( + () => null, + (e: unknown) => e, + )) as CliError + + expect(err.code).toBe('AUTH_VERIFICATION_FAILED') + expect(err.message).toBe('Could not verify the token with Outline') + expect(err.message).not.toContain('Unable to decode token') + expect(err.hints).toEqual(expect.arrayContaining([expect.stringContaining('--base-url')])) expect(storeMocks.set).not.toHaveBeenCalled() }) it('throws NO_TOKEN when no token is given in a non-interactive shell', async () => { - Object.defineProperty(process.stdin, 'isTTY', { value: false, configurable: true }) + setStdinIsTTY(false) const program = await buildProgram() await expect(program.parseAsync(['node', 'ol', 'auth', 'token'])).rejects.toHaveProperty( 'code', 'NO_TOKEN', ) + expect(promptMock).not.toHaveBeenCalled() + }) + + it('reads the token from a masked prompt when no argument is given in a TTY', async () => { + setStdinIsTTY(true) + promptMock.mockResolvedValue('tok-prompt') + const apiRequest = await importApiMock() + apiRequest.mockResolvedValue({ data: AUTH_INFO }) + + const program = await buildProgram() + await program.parseAsync(['node', 'ol', 'auth', 'token', '--base-url', 'https://wiki.test']) + + expect(promptMock).toHaveBeenCalledWith('API token: ', { hidden: true }) + expect(storeMocks.set).toHaveBeenCalledWith( + expect.objectContaining({ id: 'user-uuid', label: 'Ada Lovelace' }), + 'tok-prompt', + ) }) it('suppresses the human confirmation in machine-output mode', async () => { diff --git a/src/commands/auth.ts b/src/commands/auth.ts index 40f79a3..6d087ba 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -1,4 +1,3 @@ -import { createInterface } from 'node:readline' import { attachLoginCommand, attachLogoutCommand, @@ -16,14 +15,15 @@ import { createOutlineAuthProvider, createOutlineTokenStore, getActiveTokenSource, + identifyAccount, type OutlineAccount, type OutlineTokenStore, + prompt, resolveBaseUrl, } from '../lib/auth-provider.js' import { refreshedTokenForStatus } from '../lib/auth.js' import { CliError } from '../lib/errors.js' import { isJsonMode } from '../lib/global-args.js' -import { makeOutlineAccount } from '../lib/outline-account.js' import { withUserRefAware } from '../lib/user-ref-store.js' const DEFAULT_OAUTH_CALLBACK_PORT = 54969 @@ -43,25 +43,6 @@ function resolvePreferredCallbackPort(): number { return parsed } -// Read a secret without echoing it. Node exposes no public masked-prompt API, -// so we override readline's private `_writeToOutput` to suppress keystrokes and -// echo only the prompt label. Output goes to stderr so a `--json` stdout stays clean. -function promptHiddenToken(question: string): Promise { - return new Promise((resolve) => { - const rl = createInterface({ input: process.stdin, output: process.stderr }) - const internal = rl as unknown as { _writeToOutput?: (str: string) => void } - const original = internal._writeToOutput?.bind(rl) - internal._writeToOutput = (str: string) => { - if (original && str.includes(question)) original(question) - } - rl.question(question, (answer) => { - rl.close() - process.stderr.write('\n') - resolve(answer.trim()) - }) - }) -} - async function saveToken( store: OutlineTokenStore, token: string | undefined, @@ -75,7 +56,7 @@ async function saveToken( 'Or use OAuth: ol auth login', ]) } - token = await promptHiddenToken('API token: ') + token = await prompt('API token: ', { hidden: true }) } const trimmed = token.trim() if (!trimmed) throw new CliError('NO_TOKEN', 'No token provided') @@ -83,29 +64,17 @@ async function saveToken( const baseUrl = await resolveBaseUrl({ baseUrl: options.baseUrl }) // A freshly pasted token is verified by probing `auth.info`. Any failure - // (bad token, wrong instance, unreachable host) means we can't trust it, so - // surface a single actionable error rather than leaking the raw API string. - let data: AuthInfoResponse + // (bad token, wrong instance, unreachable host) collapses to one stable + // error — never surface the raw API/network string. + let account: OutlineAccount try { - ;({ data } = await apiRequest( - 'auth.info', - {}, - { token: trimmed, baseUrl }, - )) - } catch (err) { - const detail = err instanceof Error ? err.message : String(err) - throw new CliError('AUTH_VERIFICATION_FAILED', `Could not verify token (${detail})`, [ + account = await identifyAccount(trimmed, baseUrl) + } catch { + throw new CliError('AUTH_VERIFICATION_FAILED', 'Could not verify the token with Outline', [ 'Check the token value', `Check --base-url matches the instance the token came from (used: ${baseUrl})`, ]) } - - const account = makeOutlineAccount({ - id: data.user.id, - label: data.user.name, - baseUrl, - teamName: data.team.name, - }) await store.set(account, trimmed) const machine = isJsonMode() diff --git a/src/lib/auth-provider.ts b/src/lib/auth-provider.ts index 1cdf721..3d9ba35 100644 --- a/src/lib/auth-provider.ts +++ b/src/lib/auth-provider.ts @@ -32,26 +32,73 @@ function stringFlag(flags: Record, key: string): string | undef return typeof value === 'string' && value.trim() ? value.trim() : undefined } -async function prompt(question: string): Promise { - // Output to stderr so `--json` / `--ndjson` envelopes on stdout stay clean. +/** + * Read a line from stdin, prompting on stderr so `--json` / `--ndjson` + * envelopes on stdout stay clean. With `hidden: true` the typed characters are + * masked (for secrets): the prompt label is shown once, then readline's echo is + * muted so keystrokes don't leak to the terminal. + */ +export async function prompt( + question: string, + options: { hidden?: boolean } = {}, +): Promise { const rl = createInterface({ input: process.stdin, output: process.stderr }) try { - return (await rl.question(question)).trim() + if (!options.hidden) return (await rl.question(question)).trim() + const internal = rl as unknown as { _writeToOutput?: (str: string) => void } + const original = internal._writeToOutput?.bind(rl) + let muted = false + internal._writeToOutput = (str: string) => { + if (original && !muted) original(str) + } + // `question()` writes the prompt synchronously; mute everything after so + // only the typed characters are suppressed, not the label. + const pending = rl.question(question) + muted = true + const answer = (await pending).trim() + process.stderr.write('\n') + return answer } finally { rl.close() } } -export async function resolveBaseUrl(flags: Record): Promise { - const fromFlag = stringFlag(flags, 'baseUrl') +export async function resolveBaseUrl(options: { baseUrl?: unknown } = {}): Promise { + const fromFlag = + typeof options.baseUrl === 'string' && options.baseUrl.trim() + ? options.baseUrl.trim() + : undefined if (fromFlag) return fromFlag.replace(/\/$/, '') const fromEnv = process.env.OUTLINE_URL?.trim() if (fromEnv) return fromEnv.replace(/\/$/, '') const configured = await getBaseUrl() + // Never block a non-interactive shell (CI, piped input) on a prompt — + // `auth token` is meant to be scriptable, so fall back to the default. + if (!process.stdin.isTTY) return configured.replace(/\/$/, '') const answered = await prompt(`Base URL (default: ${configured}): `) return (answered || configured).replace(/\/$/, '') } +/** + * Verify a token by probing `auth.info` and map the response to the persisted + * account shape. Shared by the OAuth `validate` hook and `auth token` so the + * identity-resolution rules live in one place. + */ +export async function identifyAccount( + token: string, + baseUrl: string, + oauthClientId?: string, +): Promise { + const { data } = await apiRequest('auth.info', {}, { token, baseUrl }) + return makeOutlineAccount({ + id: data.user.id, + label: data.user.name, + baseUrl, + oauthClientId, + teamName: data.team.name, + }) +} + async function resolveClientId(flags: Record): Promise { const fromFlag = stringFlag(flags, 'clientId') if (fromFlag) return fromFlag @@ -106,18 +153,7 @@ export function createOutlineAuthProvider(): AuthProvider { }, validate: async ({ token, handshake }) => { const base = baseUrl ?? (await getBaseUrl()) - const { data } = await apiRequest( - 'auth.info', - {}, - { token, baseUrl: base }, - ) - return makeOutlineAccount({ - id: data.user.id, - label: data.user.name, - baseUrl: base, - oauthClientId: handshake.clientId as string, - teamName: data.team.name, - }) + return identifyAccount(token, base, handshake.clientId as string) }, fetchImpl: outlineFetch, }) diff --git a/src/lib/skills/content.ts b/src/lib/skills/content.ts index 5d15d43..c7cf596 100644 --- a/src/lib/skills/content.ts +++ b/src/lib/skills/content.ts @@ -90,8 +90,8 @@ ol auth logout --json | --ndjson # Machine-readable logout envelope ({ok: ol auth token # Save a personal API token (validates via auth.info, resolves identity) ol auth token --base-url # Save a token for a specific Outline instance ol auth token # Prompt for the token (hidden input; errors in non-interactive shells) -ol auth token view # Print the bare stored token to stdout for scripts (no newline when piped) -ol auth token view --user # Print a specific stored account's token; refuses when OUTLINE_API_TOKEN is set +ol auth token view # Print the bare stored token to stdout for scripts (no newline when piped; refuses when OUTLINE_API_TOKEN is set) +ol --user auth token view # Print a specific stored account's token (--user is a root flag, before the command) \`\`\` ### Accounts From 74957b093ceeeb3025724c4327dcb7d48e4b8e5a Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Sun, 24 May 2026 17:49:17 +0100 Subject: [PATCH 3/3] refactor(auth): address second review on `auth token` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Masked prompt no longer pokes readline's private `_writeToOutput`; it routes echo through a muted `Writable` passed as `output` (public API). - Extract `logSaveResult` in auth-output.ts and reuse it from both the `auth login` success hook and `auth token`, so the two save flows share one machine-output/warning path (mirrors `logClearResult`). - Fix the `NO_TOKEN` hint: it no longer implies `auth token` reads `OUTLINE_API_TOKEN` as input — it's framed as an alternative auth method. - `stringFlag` now takes a value (`stringFlag(options.baseUrl)`); callers updated. - Tests: use `.mock*Once` to stop hoisted-mock bleed, capture the rejection with `.catch`, add an isolated `prompt({ hidden })` masking test (mocked `node:readline/promises`) and a non-TTY `resolveBaseUrl` fallback test. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/commands/auth-token.test.ts | 21 ++++++------- src/commands/auth.ts | 23 +++----------- src/lib/auth-output.ts | 15 +++++++++ src/lib/auth-provider.test.ts | 55 +++++++++++++++++++++++++++++++++ src/lib/auth-provider.ts | 43 ++++++++++++++------------ 5 files changed, 108 insertions(+), 49 deletions(-) diff --git a/src/commands/auth-token.test.ts b/src/commands/auth-token.test.ts index d0a85a6..85be749 100644 --- a/src/commands/auth-token.test.ts +++ b/src/commands/auth-token.test.ts @@ -66,7 +66,7 @@ describe('auth token (save)', () => { it('validates via auth.info, stores the resolved account, and confirms', async () => { const log = captureConsole() const apiRequest = await importApiMock() - apiRequest.mockResolvedValue({ data: AUTH_INFO }) + apiRequest.mockResolvedValueOnce({ data: AUTH_INFO }) const program = await buildProgram() await program.parseAsync([ @@ -101,7 +101,7 @@ describe('auth token (save)', () => { const apiRequest = await importApiMock() // Outline's real invalid-token error carries no status code (api.ts drops // it when the body has a message); the wrapper must hide it entirely. - apiRequest.mockRejectedValue(new Error('API error: Unable to decode token')) + apiRequest.mockRejectedValueOnce(new Error('API error: Unable to decode token')) const program = await buildProgram() const err = (await program @@ -114,10 +114,7 @@ describe('auth token (save)', () => { '--base-url', 'https://wiki.test', ]) - .then( - () => null, - (e: unknown) => e, - )) as CliError + .catch((e: unknown) => e)) as CliError expect(err.code).toBe('AUTH_VERIFICATION_FAILED') expect(err.message).toBe('Could not verify the token with Outline') @@ -138,9 +135,9 @@ describe('auth token (save)', () => { it('reads the token from a masked prompt when no argument is given in a TTY', async () => { setStdinIsTTY(true) - promptMock.mockResolvedValue('tok-prompt') + promptMock.mockResolvedValueOnce('tok-prompt') const apiRequest = await importApiMock() - apiRequest.mockResolvedValue({ data: AUTH_INFO }) + apiRequest.mockResolvedValueOnce({ data: AUTH_INFO }) const program = await buildProgram() await program.parseAsync(['node', 'ol', 'auth', 'token', '--base-url', 'https://wiki.test']) @@ -155,7 +152,7 @@ describe('auth token (save)', () => { it('suppresses the human confirmation in machine-output mode', async () => { const log = captureConsole() const apiRequest = await importApiMock() - apiRequest.mockResolvedValue({ data: AUTH_INFO }) + apiRequest.mockResolvedValueOnce({ data: AUTH_INFO }) // `--json` is a root selector read off argv by global-args, not a // commander option on `auth token`, so warm the cache rather than @@ -182,7 +179,7 @@ describe('auth token (save)', () => { describe('auth token view', () => { it('writes the bare stored token to stdout with no envelope or newline', async () => { - storeMocks.active.mockResolvedValue({ token: 'stored-tok', account: STORED_ACCOUNT }) + storeMocks.active.mockResolvedValueOnce({ token: 'stored-tok', account: STORED_ACCOUNT }) const out = captureStream('stdout') const program = await buildProgram() @@ -200,12 +197,12 @@ describe('auth token view', () => { }) it('routes a global --user through the ref-aware store', async () => { - storeMocks.active.mockImplementation(async (ref?: string) => + storeMocks.active.mockImplementationOnce(async (ref?: string) => ref === 'Bob' ? { token: 'tok-bob', account: STORED_ACCOUNT_BOB } : { token: 'tok-ada', account: STORED_ACCOUNT }, ) - storeMocks.activeAccount.mockResolvedValue({ + storeMocks.activeAccount.mockResolvedValueOnce({ account: STORED_ACCOUNT_BOB, isDefault: false, }) diff --git a/src/commands/auth.ts b/src/commands/auth.ts index 6d087ba..f91fed9 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -8,7 +8,7 @@ import chalk from 'chalk' import type { Command } from 'commander' import { apiRequest } from '../lib/api.js' import { TOKEN_ENV_VAR } from '../lib/auth-constants.js' -import { logClearResult, logTokenStorageResult } from '../lib/auth-output.js' +import { logClearResult, logSaveResult } from '../lib/auth-output.js' import { renderError, renderSuccess } from '../lib/auth-pages.js' import { type AuthInfoResponse, @@ -52,7 +52,8 @@ async function saveToken( if (!process.stdin.isTTY) { throw new CliError('NO_TOKEN', 'No token provided', [ 'Pass it as an argument: ol auth token ', - 'Or set the OUTLINE_API_TOKEN environment variable', + 'Run in an interactive terminal to be prompted for it', + 'Set OUTLINE_API_TOKEN to authenticate without storing a token', 'Or use OAuth: ol auth login', ]) } @@ -81,14 +82,7 @@ async function saveToken( if (!machine) { console.log(chalk.green('✓'), `Saved token for ${account.label} (${account.teamName})`) } - const result = store.getLastStorageResult() - if (result) { - logTokenStorageResult( - result, - 'Token stored securely in the system credential manager', - machine, - ) - } + logSaveResult(store, machine) } export function registerAuthCommand(program: Command): void { @@ -113,14 +107,7 @@ export function registerAuthCommand(program: Command): void { 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, - ) - } + logSaveResult(store, isMachineOutput) }, }) .description('Authenticate with an Outline instance via OAuth') diff --git a/src/lib/auth-output.ts b/src/lib/auth-output.ts index 75cee7f..7958e37 100644 --- a/src/lib/auth-output.ts +++ b/src/lib/auth-output.ts @@ -21,6 +21,21 @@ export function logTokenStorageResult( } } +/** + * Surface the result of a token save (`auth login` / `auth token`): the + * confirmation goes to stdout, any keyring-fallback warning to stderr. Shared so + * both save flows keep identical machine-output and warning behavior. + */ +export function logSaveResult(store: OutlineTokenStore, isMachineOutput: boolean): void { + const result = store.getLastStorageResult() + if (!result) return + logTokenStorageResult( + result, + 'Token stored securely in the system credential manager', + isMachineOutput, + ) +} + /** * Surface the result of a token clear (`auth logout` / `account remove`): the * confirmation goes to stdout, any keyring-fallback warning to stderr. Shared so diff --git a/src/lib/auth-provider.test.ts b/src/lib/auth-provider.test.ts index 24d238e..37d16f3 100644 --- a/src/lib/auth-provider.test.ts +++ b/src/lib/auth-provider.test.ts @@ -1,3 +1,5 @@ +import { Writable } from 'node:stream' +import { captureStream } from '@doist/cli-core/testing' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { AUTH_INFO, @@ -12,6 +14,9 @@ import { vi.mock('../transport/fetch-with-retry.js', () => ({ fetchWithRetry: vi.fn() })) vi.mock('./api.js', () => ({ apiRequest: vi.fn() })) +const readlineMocks = vi.hoisted(() => ({ createInterface: vi.fn() })) +vi.mock('node:readline/promises', () => ({ createInterface: readlineMocks.createInterface })) + const migrateMocks = vi.hoisted(() => ({ runMigrateLegacyAuth: vi.fn(), })) @@ -583,6 +588,56 @@ describe('resolveActiveAccountSource', () => { }) }) +describe('prompt + resolveBaseUrl', () => { + beforeEach(() => { + readlineMocks.createInterface.mockReset() + delete process.env.OUTLINE_URL + configMocks.getConfig.mockReset().mockResolvedValue({}) + }) + + it('prompt({ hidden }) echoes the label but masks the typed characters', async () => { + const stderr = captureStream('stderr') + let output: Writable | undefined + readlineMocks.createInterface.mockImplementation((opts: { output: Writable }) => { + output = opts.output + return { + // Mirror readline: the prompt label is written synchronously + // (before `prompt` mutes), the keystroke echo lands afterwards. + question: (label: string) => { + output?.write(label) + return new Promise((resolve) => { + queueMicrotask(() => { + output?.write('secret') + resolve(' secret ') + }) + }) + }, + close: vi.fn(), + } + }) + + const { prompt } = await import('./auth-provider.js') + const answer = await prompt('API token: ', { hidden: true }) + + expect(answer).toBe('secret') + const written = stderr.mock.calls.map((c) => String(c[0])).join('') + expect(written).toContain('API token: ') + expect(written).not.toContain('secret') + }) + + it('resolveBaseUrl returns the configured default without prompting in a non-TTY shell', async () => { + const original = process.stdin.isTTY + Object.defineProperty(process.stdin, 'isTTY', { value: false, configurable: true }) + try { + const { resolveBaseUrl } = await import('./auth-provider.js') + await expect(resolveBaseUrl()).resolves.toBe('https://app.getoutline.com') + expect(readlineMocks.createInterface).not.toHaveBeenCalled() + } finally { + Object.defineProperty(process.stdin, 'isTTY', { value: original, configurable: true }) + } + }) +}) + describe('matchOutlineAccount', () => { it('matches the UUID exactly and the label case-insensitively', async () => { const { matchOutlineAccount } = await import('./auth-provider.js') diff --git a/src/lib/auth-provider.ts b/src/lib/auth-provider.ts index 3d9ba35..ddac110 100644 --- a/src/lib/auth-provider.ts +++ b/src/lib/auth-provider.ts @@ -1,4 +1,5 @@ import { createInterface } from 'node:readline/promises' +import { Writable } from 'node:stream' import { type AccountRef, type AuthProvider, @@ -27,32 +28,39 @@ export type AuthInfoResponse = { export type OutlineTokenStore = KeyringTokenStore -function stringFlag(flags: Record, key: string): string | undefined { - const value = flags[key] +function stringFlag(value: unknown): string | undefined { return typeof value === 'string' && value.trim() ? value.trim() : undefined } /** * Read a line from stdin, prompting on stderr so `--json` / `--ndjson` * envelopes on stdout stay clean. With `hidden: true` the typed characters are - * masked (for secrets): the prompt label is shown once, then readline's echo is - * muted so keystrokes don't leak to the terminal. + * masked (for secrets): the prompt label is shown, then readline's echo is + * routed through a muted `Writable` so keystrokes don't leak to the terminal. */ export async function prompt( question: string, options: { hidden?: boolean } = {}, ): Promise { - const rl = createInterface({ input: process.stdin, output: process.stderr }) - try { - if (!options.hidden) return (await rl.question(question)).trim() - const internal = rl as unknown as { _writeToOutput?: (str: string) => void } - const original = internal._writeToOutput?.bind(rl) - let muted = false - internal._writeToOutput = (str: string) => { - if (original && !muted) original(str) + if (!options.hidden) { + const rl = createInterface({ input: process.stdin, output: process.stderr }) + try { + return (await rl.question(question)).trim() + } finally { + rl.close() } - // `question()` writes the prompt synchronously; mute everything after so - // only the typed characters are suppressed, not the label. + } + // Echo the prompt label, then mute so typed characters aren't shown. A muted + // `Writable` (public API) avoids poking readline's private `_writeToOutput`. + let muted = false + const output = new Writable({ + write(chunk, _encoding, callback) { + if (!muted) process.stderr.write(chunk) + callback() + }, + }) + const rl = createInterface({ input: process.stdin, output, terminal: true }) + try { const pending = rl.question(question) muted = true const answer = (await pending).trim() @@ -64,10 +72,7 @@ export async function prompt( } export async function resolveBaseUrl(options: { baseUrl?: unknown } = {}): Promise { - const fromFlag = - typeof options.baseUrl === 'string' && options.baseUrl.trim() - ? options.baseUrl.trim() - : undefined + const fromFlag = stringFlag(options.baseUrl) if (fromFlag) return fromFlag.replace(/\/$/, '') const fromEnv = process.env.OUTLINE_URL?.trim() if (fromEnv) return fromEnv.replace(/\/$/, '') @@ -100,7 +105,7 @@ export async function identifyAccount( } async function resolveClientId(flags: Record): Promise { - const fromFlag = stringFlag(flags, 'clientId') + const fromFlag = stringFlag(flags.clientId) if (fromFlag) return fromFlag const existing = await getOAuthClientId() if (existing) return existing