From bfaa35e8eb9d934182c6da037c311d07d736fcdb Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Mon, 11 May 2026 17:57:30 +0100 Subject: [PATCH 1/4] refactor: delegate auth flow to @doist/cli-core/auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Drops the bespoke OAuth stack (`src/lib/pkce.ts`, `oauth.ts`, `oauth-server.ts`, the entire login action in `commands/auth.ts`) and delegates the flow to `@doist/cli-core/auth`'s `runOAuthFlow` + `attachLoginCommand`. - New `OutlineAuthProvider` (`src/lib/auth-provider.ts`) owns the outline-specific behaviour: `authorize` builds the `/oauth/authorize` URL with cli-core's `generateVerifier` / `deriveChallenge`, `exchangeCode` posts the form-urlencoded token exchange (public client — no `client_secret`), `validateToken` hits `auth.info` with the unsaved token via a new `apiRequest` token/baseUrl override. - New `OutlineTokenStore` adapter maps the cli-core account onto the existing config file (adds `auth_user_id` / `auth_user_name` to round-trip the identity). - Branded success / error HTML preserved byte-for-byte in `src/lib/auth-pages.ts` and passed to `attachLoginCommand` via `renderSuccess` / `renderError`. - `ol auth login` gains `--read-only`, `--callback-port`, `--json`, `--ndjson` for free; `--base-url` / `--client-id` chained onto the returned command. `--token` paste flag dropped — login is OAuth-only. - Skill content + skills/outline-cli/SKILL.md updated to match. - 115 tests pass; type-check + oxlint clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- skills/outline-cli/SKILL.md | 10 +- src/__tests__/auth-provider.test.ts | 262 ++++++++++++++++++++++++++++ src/__tests__/oauth-server.test.ts | 65 ------- src/__tests__/oauth.test.ts | 71 -------- src/commands/auth.ts | 222 +++-------------------- src/lib/api.ts | 28 ++- src/lib/auth-pages.ts | 111 ++++++++++++ src/lib/auth-provider.ts | 189 ++++++++++++++++++++ src/lib/auth.ts | 4 +- src/lib/config.ts | 2 + src/lib/oauth-server.ts | 250 -------------------------- src/lib/oauth.ts | 70 -------- src/lib/pkce.ts | 19 -- src/lib/skills/content.ts | 10 +- 14 files changed, 625 insertions(+), 688 deletions(-) create mode 100644 src/__tests__/auth-provider.test.ts delete mode 100644 src/__tests__/oauth-server.test.ts delete mode 100644 src/__tests__/oauth.test.ts create mode 100644 src/lib/auth-pages.ts create mode 100644 src/lib/auth-provider.ts delete mode 100644 src/lib/oauth-server.ts delete mode 100644 src/lib/oauth.ts delete mode 100644 src/lib/pkce.ts diff --git a/skills/outline-cli/SKILL.md b/skills/outline-cli/SKILL.md index fe716ae..c5ab551 100644 --- a/skills/outline-cli/SKILL.md +++ b/skills/outline-cli/SKILL.md @@ -73,9 +73,13 @@ ol col delete --confirm ### Authentication ```bash -ol auth login # Configure API token and base URL -ol auth status # Show current auth state -ol auth logout # Clear saved credentials +ol auth login # OAuth login (opens browser); prompts for base URL + client ID if needed +ol auth login --base-url # Specify Outline base URL for this login (saved for future use) +ol auth login --client-id # Specify OAuth client ID for this login (saved for future use) +ol auth login --callback-port # Override local OAuth callback port +ol auth login --json | --ndjson # Machine-readable success envelope +ol auth status # Show current auth state +ol auth logout # Clear saved credentials ``` ### Update & Changelog diff --git a/src/__tests__/auth-provider.test.ts b/src/__tests__/auth-provider.test.ts new file mode 100644 index 0000000..dd6c17e --- /dev/null +++ b/src/__tests__/auth-provider.test.ts @@ -0,0 +1,262 @@ +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' + +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') + +vi.mock('../transport/fetch-with-retry.js', () => ({ + fetchWithRetry: vi.fn(), +})) + +describe('OutlineAuthProvider', () => { + 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() + }) + + afterEach(() => { + if (existsSync(TEST_XDG)) { + rmSync(TEST_XDG, { recursive: true }) + } + delete process.env.XDG_CONFIG_HOME + vi.unstubAllGlobals() + }) + + describe('authorize', () => { + it('builds an outline authorize URL with PKCE params from explicit flags', async () => { + const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') + const provider = createOutlineAuthProvider() + + const result = await provider.authorize({ + redirectUri: 'http://localhost:54969/callback', + state: 'state-123', + scopes: [], + readOnly: false, + flags: { baseUrl: 'https://wiki.example.com/', clientId: 'cid-xyz' }, + handshake: {}, + }) + + const url = new URL(result.authorizeUrl) + expect(url.origin + url.pathname).toBe('https://wiki.example.com/oauth/authorize') + expect(url.searchParams.get('client_id')).toBe('cid-xyz') + expect(url.searchParams.get('response_type')).toBe('code') + expect(url.searchParams.get('redirect_uri')).toBe('http://localhost:54969/callback') + expect(url.searchParams.get('state')).toBe('state-123') + expect(url.searchParams.get('code_challenge_method')).toBe('S256') + expect(url.searchParams.get('code_challenge')).toMatch(/^[A-Za-z0-9_-]+$/) + + const handshake = result.handshake as Record + expect(handshake.baseUrl).toBe('https://wiki.example.com') + expect(handshake.clientId).toBe('cid-xyz') + expect(typeof handshake.codeVerifier).toBe('string') + expect(handshake.codeVerifier.length).toBeGreaterThan(40) + }) + + it('reads base URL and client ID from environment when flags absent', async () => { + process.env.OUTLINE_URL = 'https://env.example.com/' + // OUTLINE_OAUTH_CLIENT_ID is read via getOAuthClientId + process.env.OUTLINE_OAUTH_CLIENT_ID = 'env-cid' + + const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') + const provider = createOutlineAuthProvider() + + const result = await provider.authorize({ + redirectUri: 'http://localhost:54969/callback', + state: 's', + scopes: [], + readOnly: false, + flags: {}, + handshake: {}, + }) + const url = new URL(result.authorizeUrl) + expect(url.origin).toBe('https://env.example.com') + expect(url.searchParams.get('client_id')).toBe('env-cid') + }) + }) + + describe('exchangeCode', () => { + it('posts form-encoded token exchange and returns access token', async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ access_token: 'tok-abc' }), + }) + vi.stubGlobal('fetch', fetchMock) + + const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') + const provider = createOutlineAuthProvider() + + const result = await provider.exchangeCode({ + code: 'auth-code', + state: 'state-x', + redirectUri: 'http://localhost:54969/callback', + handshake: { + baseUrl: 'https://wiki.example.com', + clientId: 'cid-xyz', + codeVerifier: 'verifier-1', + }, + }) + + expect(result.accessToken).toBe('tok-abc') + expect(fetchMock).toHaveBeenCalledTimes(1) + const [tokenUrl, init] = fetchMock.mock.calls[0] + expect(tokenUrl).toBe('https://wiki.example.com/oauth/token') + expect(init.method).toBe('POST') + expect(init.headers).toMatchObject({ + 'Content-Type': 'application/x-www-form-urlencoded', + }) + const body = new URLSearchParams(init.body as string) + expect(body.get('grant_type')).toBe('authorization_code') + expect(body.get('client_id')).toBe('cid-xyz') + expect(body.get('redirect_uri')).toBe('http://localhost:54969/callback') + expect(body.get('code_verifier')).toBe('verifier-1') + expect(body.get('code')).toBe('auth-code') + }) + + it('surfaces provider error description on failed exchange', async () => { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: false, + statusText: 'Bad Request', + json: async () => ({ + error: 'invalid_grant', + error_description: 'Authorization code expired', + }), + }), + ) + + const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') + const provider = createOutlineAuthProvider() + + await expect( + provider.exchangeCode({ + code: 'c', + state: 's', + redirectUri: 'http://localhost:54969/callback', + handshake: { + baseUrl: 'https://wiki.example.com', + clientId: 'cid-xyz', + codeVerifier: 'verifier-1', + }, + }), + ).rejects.toThrow('OAuth token exchange failed: Authorization code expired') + }) + }) + + describe('validateToken', () => { + it('calls auth.info with handshake base URL + token and returns an OutlineAccount', async () => { + const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') + ;(fetchWithRetry as ReturnType).mockResolvedValue({ + ok: true, + json: async () => ({ + data: { + user: { id: 'user-uuid', name: 'Ada Lovelace', email: 'ada@example.com' }, + team: { name: 'Analytics', subdomain: 'analytics' }, + }, + }), + }) + + const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') + const provider = createOutlineAuthProvider() + + const account = await provider.validateToken({ + token: 'tok-abc', + handshake: { + baseUrl: 'https://wiki.example.com', + clientId: 'cid-xyz', + }, + }) + + expect(account).toEqual({ + id: 'user-uuid', + label: 'Ada Lovelace', + baseUrl: 'https://wiki.example.com', + oauthClientId: 'cid-xyz', + teamName: 'Analytics', + }) + + expect(fetchWithRetry).toHaveBeenCalledTimes(1) + const args = (fetchWithRetry as ReturnType).mock.calls[0][0] + expect(args.url).toBe('https://wiki.example.com/api/auth.info') + expect(args.options.headers.Authorization).toBe('Bearer tok-abc') + }) + }) +}) + +describe('OutlineTokenStore', () => { + beforeEach(() => { + process.env.XDG_CONFIG_HOME = TEST_XDG + mkdirSync(TEST_CONFIG_DIR, { recursive: true }) + delete process.env.OUTLINE_API_TOKEN + vi.resetModules() + }) + + afterEach(() => { + if (existsSync(TEST_XDG)) { + rmSync(TEST_XDG, { recursive: true }) + } + delete process.env.XDG_CONFIG_HOME + }) + + it('set persists token + account fields and active reads them back', async () => { + const { createOutlineTokenStore } = await import('../lib/auth-provider.js') + const store = createOutlineTokenStore() + + await store.set( + { + id: 'user-uuid', + label: 'Ada', + baseUrl: 'https://wiki.example.com', + oauthClientId: 'cid-xyz', + teamName: 'Analytics', + }, + 'tok-persisted', + ) + + const got = await store.active() + expect(got).not.toBeNull() + expect(got?.token).toBe('tok-persisted') + expect(got?.account.id).toBe('user-uuid') + expect(got?.account.label).toBe('Ada') + expect(got?.account.baseUrl).toBe('https://wiki.example.com') + expect(got?.account.oauthClientId).toBe('cid-xyz') + }) + + it('active returns null when config has a token but no persisted identity', async () => { + writeFileSync(TEST_CONFIG_PATH, JSON.stringify({ api_token: 'legacy-tok' })) + const { createOutlineTokenStore } = await import('../lib/auth-provider.js') + const store = createOutlineTokenStore() + await expect(store.active()).resolves.toBeNull() + }) + + it('active returns null when no token saved', async () => { + const { createOutlineTokenStore } = await import('../lib/auth-provider.js') + const store = createOutlineTokenStore() + await expect(store.active()).resolves.toBeNull() + }) + + it('clear removes auth fields', async () => { + const { createOutlineTokenStore } = await import('../lib/auth-provider.js') + const store = createOutlineTokenStore() + await store.set( + { + id: 'u', + label: 'l', + baseUrl: 'https://x', + oauthClientId: 'c', + teamName: 't', + }, + 'tok', + ) + await store.clear() + await expect(store.active()).resolves.toBeNull() + }) +}) diff --git a/src/__tests__/oauth-server.test.ts b/src/__tests__/oauth-server.test.ts deleted file mode 100644 index bbd7282..0000000 --- a/src/__tests__/oauth-server.test.ts +++ /dev/null @@ -1,65 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { startOAuthCallbackServer } from '../lib/oauth-server.js' - -describe('oauth callback server', () => { - it('returns success page and resolves authorization code', async () => { - const callbackServer = await startOAuthCallbackServer({ - state: 'expected-state', - timeoutMs: 10_000, - port: 0, - }) - - const response = await fetch( - `${callbackServer.redirectUri}?code=test-code&state=expected-state`, - ) - const html = await response.text() - - expect(response.status).toBe(200) - expect(html).toContain('Login complete') - await expect(callbackServer.waitForCode).resolves.toBe('test-code') - }) - - it('returns error page and rejects on state mismatch', async () => { - const callbackServer = await startOAuthCallbackServer({ - state: 'expected-state', - timeoutMs: 10_000, - port: 0, - }) - const rejection = callbackServer.waitForCode.then( - () => new Error('Expected OAuth state mismatch.'), - (error) => error as Error, - ) - - const response = await fetch( - `${callbackServer.redirectUri}?code=test-code&state=wrong-state`, - ) - const html = await response.text() - - expect(response.status).toBe(400) - expect(html).toContain('Authentication failed') - const error = await rejection - expect(error.message).toBe('OAuth state mismatch.') - }) - - it('returns error page and rejects when OAuth provider sends an error', async () => { - const callbackServer = await startOAuthCallbackServer({ - state: 'expected-state', - timeoutMs: 10_000, - port: 0, - }) - const rejection = callbackServer.waitForCode.then( - () => new Error('Expected OAuth provider error.'), - (error) => error as Error, - ) - - const response = await fetch( - `${callbackServer.redirectUri}?error=access_denied&error_description=User%20denied`, - ) - const html = await response.text() - - expect(response.status).toBe(400) - expect(html).toContain('Authentication failed') - const error = await rejection - expect(error.message).toBe('OAuth authorization denied: User denied') - }) -}) diff --git a/src/__tests__/oauth.test.ts b/src/__tests__/oauth.test.ts deleted file mode 100644 index 73b8f09..0000000 --- a/src/__tests__/oauth.test.ts +++ /dev/null @@ -1,71 +0,0 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' - -vi.mock('../transport/fetch-with-retry.js', () => ({ - fetchWithRetry: vi.fn(), -})) - -describe('exchangeCodeForToken', () => { - afterEach(() => { - vi.clearAllMocks() - }) - - it('uses fetchWithRetry for token exchange', async () => { - const mockResponse = { - ok: true, - json: async () => ({ access_token: 'test-access-token' }), - } - const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') - ;(fetchWithRetry as ReturnType).mockResolvedValue(mockResponse) - - const { exchangeCodeForToken } = await import('../lib/oauth.js') - const token = await exchangeCodeForToken({ - baseUrl: 'https://test.outline.com', - clientId: 'client-id', - redirectUri: 'http://localhost:3000/callback', - codeVerifier: 'code-verifier', - code: 'auth-code', - }) - - expect(token).toBe('test-access-token') - expect(fetchWithRetry).toHaveBeenCalledWith({ - url: 'https://test.outline.com/oauth/token', - options: { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - }, - body: new URLSearchParams({ - grant_type: 'authorization_code', - client_id: 'client-id', - redirect_uri: 'http://localhost:3000/callback', - code_verifier: 'code-verifier', - code: 'auth-code', - }).toString(), - }, - }) - }) - - it('throws the provider error message on failed exchange', async () => { - const mockResponse = { - ok: false, - statusText: 'Bad Request', - json: async () => ({ - error: 'invalid_grant', - error_description: 'Authorization code expired', - }), - } - const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') - ;(fetchWithRetry as ReturnType).mockResolvedValue(mockResponse) - - const { exchangeCodeForToken } = await import('../lib/oauth.js') - await expect( - exchangeCodeForToken({ - baseUrl: 'https://test.outline.com', - clientId: 'client-id', - redirectUri: 'http://localhost:3000/callback', - codeVerifier: 'code-verifier', - code: 'auth-code', - }), - ).rejects.toThrow('OAuth token exchange failed: Authorization code expired') - }) -}) diff --git a/src/commands/auth.ts b/src/commands/auth.ts index b1a4cac..fc3bd1c 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -1,53 +1,38 @@ -import { createInterface } from 'node:readline/promises' +import { attachLoginCommand } from '@doist/cli-core/auth' import chalk from 'chalk' import type { Command } from 'commander' -import open from 'open' import { apiRequest } from '../lib/api.js' -import { - clearConfig, - getBaseUrl, - getOAuthClientId, - getTokenSource, - saveConfig, -} from '../lib/auth.js' -import { DEFAULT_OAUTH_CALLBACK_PORT, startOAuthCallbackServer } from '../lib/oauth-server.js' -import { buildAuthorizationUrl, exchangeCodeForToken } from '../lib/oauth.js' +import { renderError, renderSuccess } from '../lib/auth-pages.js' +import { createOutlineAuthProvider, createOutlineTokenStore } from '../lib/auth-provider.js' +import { clearConfig, getBaseUrl, getTokenSource } from '../lib/auth.js' import { formatError } from '../lib/output.js' -import { generateCodeChallenge, generateCodeVerifier, generateState } from '../lib/pkce.js' -interface TeamInfo { - name: string - subdomain: string -} +const DEFAULT_OAUTH_CALLBACK_PORT = 54969 interface AuthInfoResponse { user: { name: string; email: string } - team: TeamInfo -} - -async function prompt(question: string): Promise { - const rl = createInterface({ input: process.stdin, output: process.stdout }) - try { - return await rl.question(question) - } finally { - rl.close() - } -} - -function parseCallbackPort(rawPort: string): number | null { - const parsed = Number(rawPort) - if (!Number.isInteger(parsed) || parsed < 1 || parsed > 65_535) { - return null - } - return parsed + team: { name: string; subdomain: string } } export function registerAuthCommand(program: Command): void { const auth = program.command('auth').description('Manage authentication') - auth.command('login') - .description('Authenticate with an Outline instance') - .option('--token ', 'Authenticate using a personal API token') + const provider = createOutlineAuthProvider() + const store = createOutlineTokenStore() + + attachLoginCommand(auth, { + provider, + store, + preferredPort: DEFAULT_OAUTH_CALLBACK_PORT, + resolveScopes: () => [], + renderSuccess, + renderError, + onSuccess({ view, account }) { + if (view.json || view.ndjson) return + console.log(chalk.green(`Authenticated to ${account.teamName} as ${account.label}`)) + }, + }) + .description('Authenticate with an Outline instance via OAuth') .option( '--base-url ', 'Outline base URL to use for this login (saved for future logins)', @@ -56,169 +41,6 @@ export function registerAuthCommand(program: Command): void { '--client-id ', 'OAuth client ID to use for this login (saved for future logins)', ) - .option( - '--callback-port ', - `Local OAuth callback port (default: ${DEFAULT_OAUTH_CALLBACK_PORT})`, - ) - .action( - async (options: { - token?: string - clientId?: string - baseUrl?: string - callbackPort?: string - }) => { - const configuredBaseUrl = await getBaseUrl() - const optionBaseUrl = options.baseUrl?.trim() - const envBaseUrl = process.env.OUTLINE_URL?.trim() - let url = optionBaseUrl || envBaseUrl - if (!url) { - const baseUrlInput = await prompt(`Base URL (default: ${configuredBaseUrl}): `) - url = baseUrlInput.trim() || configuredBaseUrl - } - url = url.replace(/\/$/, '') - const optionClientId = options.clientId?.trim() - const optionCallbackPort = options.callbackPort?.trim() - const envCallbackPort = process.env.OUTLINE_OAUTH_CALLBACK_PORT?.trim() - const rawCallbackPort = optionCallbackPort || envCallbackPort - let callbackPort = DEFAULT_OAUTH_CALLBACK_PORT - - if (rawCallbackPort) { - const parsedPort = parseCallbackPort(rawCallbackPort) - if (!parsedPort) { - console.error( - formatError( - 'OAUTH_CALLBACK_PORT_INVALID', - `Invalid callback port: ${rawCallbackPort}`, - [ - 'Use an integer between 1 and 65535', - 'Set via --callback-port or OUTLINE_OAUTH_CALLBACK_PORT', - ], - ), - ) - process.exit(1) - } - callbackPort = parsedPort - } - - if (options.token) { - await saveConfig(options.token.trim(), url, optionClientId) - try { - const { data } = await apiRequest('auth.info') - console.log( - chalk.green(`Authenticated to ${data.team.name} as ${data.user.name}`), - ) - } catch (err) { - console.log( - chalk.yellow('Token saved, but could not verify:'), - (err as Error).message, - ) - } - return - } - - const existingClientId = await getOAuthClientId() - let clientId = optionClientId || existingClientId - - if (!clientId) { - const clientIdInput = await prompt('OAuth Client ID: ') - clientId = clientIdInput.trim() - } - - if (!clientId) { - console.error( - formatError('OAUTH_CLIENT_ID_REQUIRED', 'OAuth client ID is required.', [ - 'Create a public OAuth app in Outline settings', - 'Use --client-id for this login', - 'Set OUTLINE_OAUTH_CLIENT_ID or enter it here', - ]), - ) - process.exit(1) - } - - const codeVerifier = generateCodeVerifier() - const codeChallenge = generateCodeChallenge(codeVerifier) - const state = generateState() - - let callbackServer: Awaited> - try { - callbackServer = await startOAuthCallbackServer({ - state, - port: callbackPort, - }) - } catch (err) { - const error = err as NodeJS.ErrnoException - const hints = [ - `Ensure http://localhost:${callbackPort}/callback is reachable from your browser`, - "Re-run with 'ol auth login --token ' for manual auth", - ] - if (error.code === 'EADDRINUSE') { - hints.unshift( - `Port ${callbackPort} is already in use. Close the other process using it.`, - ) - } - - console.error( - formatError( - 'OAUTH_CALLBACK_SERVER_FAILED', - `Could not start local OAuth callback server: ${error.message}`, - hints, - ), - ) - process.exit(1) - } - const authorizationUrl = buildAuthorizationUrl({ - baseUrl: url, - clientId, - redirectUri: callbackServer.redirectUri, - codeChallenge, - state, - }) - - try { - await open(authorizationUrl) - } catch (err) { - console.log( - chalk.yellow('Could not open browser automatically.'), - chalk.dim(authorizationUrl), - ) - console.log(chalk.dim((err as Error).message)) - } - - console.log(chalk.dim('Waiting for OAuth authorization...')) - - try { - const code = await callbackServer.waitForCode - const token = await exchangeCodeForToken({ - baseUrl: url, - clientId, - redirectUri: callbackServer.redirectUri, - codeVerifier, - code, - }) - - await saveConfig(token, url, clientId) - - const { data } = await apiRequest('auth.info') - console.log( - chalk.green(`Authenticated to ${data.team.name} as ${data.user.name}`), - ) - } catch (err) { - callbackServer.close() - console.error( - formatError( - 'OAUTH_LOGIN_FAILED', - `OAuth login failed: ${(err as Error).message}`, - [ - 'Confirm the OAuth app redirect URI matches the CLI callback', - 'Verify --base-url or OUTLINE_URL points to your Outline instance', - "Re-run with 'ol auth login --token' for manual auth", - ], - ), - ) - process.exit(1) - } - }, - ) auth.command('status') .description('Show current authentication state') diff --git a/src/lib/api.ts b/src/lib/api.ts index b89cec1..ef5ca3d 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -47,19 +47,31 @@ export interface PaginatedResult { pagination?: Pagination } +export interface ApiRequestOverrides { + token?: string + baseUrl?: string +} + /** * Core API request function without spinner wrapping. */ -async function rawApiRequest(path: string, body: object = {}): Promise> { - const [baseUrl, token] = await Promise.all([getBaseUrl(), getApiToken()]) +async function rawApiRequest( + path: string, + body: object = {}, + overrides: ApiRequestOverrides = {}, +): Promise> { + const [resolvedBaseUrl, resolvedToken] = await Promise.all([ + overrides.baseUrl ? Promise.resolve(overrides.baseUrl.replace(/\/$/, '')) : getBaseUrl(), + overrides.token ? Promise.resolve(overrides.token) : getApiToken(), + ]) const res = await fetchWithRetry({ - url: `${baseUrl}/api/${path}`, + url: `${resolvedBaseUrl}/api/${path}`, options: { method: 'POST', headers: { 'Content-Type': 'application/json', - Authorization: `Bearer ${token}`, + Authorization: `Bearer ${resolvedToken}`, }, body: JSON.stringify(body), }, @@ -82,11 +94,15 @@ async function rawApiRequest(path: string, body: object = {}): Promise(path: string, body: object = {}): Promise> { +export async function apiRequest( + path: string, + body: object = {}, + overrides: ApiRequestOverrides = {}, +): Promise> { const spinnerConfig = API_SPINNER_CONFIG[path] ?? { text: 'Loading...', color: 'blue' as const, } - return withSpinner(spinnerConfig, () => rawApiRequest(path, body)) + return withSpinner(spinnerConfig, () => rawApiRequest(path, body, overrides)) } diff --git a/src/lib/auth-pages.ts b/src/lib/auth-pages.ts new file mode 100644 index 0000000..584ed49 --- /dev/null +++ b/src/lib/auth-pages.ts @@ -0,0 +1,111 @@ +function escapeHtml(text: string): string { + return text + .replaceAll('&', '&') + .replaceAll('<', '<') + .replaceAll('>', '>') + .replaceAll('"', '"') + .replaceAll("'", ''') +} + +function renderPage(title: string, subtitle: string, body: string): string { + return ` + + + + + ${escapeHtml(title)} - Outline CLI + + + +
+

${escapeHtml(title)}

+

${escapeHtml(subtitle)}

+ ${body} +

Return to your terminal and continue with ol commands.

+
+ +` +} + +export function renderSuccess(): string { + return renderPage( + 'Login complete', + 'Outline CLI is now authenticated.', + '
You can close this tab now.
', + ) +} + +export function renderError(message: string): string { + return renderPage( + 'Authentication failed', + 'Outline CLI could not finish OAuth login.', + `
${escapeHtml(message)}
`, + ) +} diff --git a/src/lib/auth-provider.ts b/src/lib/auth-provider.ts new file mode 100644 index 0000000..631e1bd --- /dev/null +++ b/src/lib/auth-provider.ts @@ -0,0 +1,189 @@ +import { createInterface } from 'node:readline/promises' +import { + type AuthAccount, + type AuthProvider, + deriveChallenge, + generateVerifier, + type TokenStore, +} from '@doist/cli-core/auth' +import { apiRequest } from './api.js' +import { clearConfig, getBaseUrl, getOAuthClientId } from './auth.js' +import { getConfig, updateConfig } from './config.js' + +const DEFAULT_BASE_URL = 'https://app.getoutline.com' + +interface 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 +} + +type OutlineHandshake = Record & { + baseUrl: string + clientId: string + codeVerifier?: string +} + +function asHandshake(value: Record): OutlineHandshake { + return value as OutlineHandshake +} + +function stringFlag(flags: Record, key: string): string | undefined { + const value = flags[key] + return typeof value === 'string' && value.trim() ? value.trim() : undefined +} + +async function prompt(question: string): Promise { + const rl = createInterface({ input: process.stdin, output: process.stdout }) + try { + return (await rl.question(question)).trim() + } finally { + rl.close() + } +} + +async function resolveBaseUrl(flags: Record): Promise { + const fromFlag = stringFlag(flags, 'baseUrl') + if (fromFlag) return fromFlag.replace(/\/$/, '') + const fromEnv = process.env.OUTLINE_URL?.trim() + if (fromEnv) return fromEnv.replace(/\/$/, '') + const configured = await getBaseUrl() + const answered = await prompt(`Base URL (default: ${configured}): `) + return (answered || configured).replace(/\/$/, '') +} + +async function resolveClientId(flags: Record): Promise { + const fromFlag = stringFlag(flags, 'clientId') + if (fromFlag) return fromFlag + const existing = await getOAuthClientId() + if (existing) return existing + const answered = await prompt('OAuth Client ID: ') + if (!answered) { + throw new Error( + 'OAuth client ID is required. Create a public OAuth app in Outline settings, then pass --client-id or set OUTLINE_OAUTH_CLIENT_ID.', + ) + } + return answered +} + +export function createOutlineAuthProvider(): AuthProvider { + return { + async authorize({ redirectUri, state, flags }) { + const baseUrl = await resolveBaseUrl(flags) + const clientId = await resolveClientId(flags) + const codeVerifier = generateVerifier() + const codeChallenge = deriveChallenge(codeVerifier) + + const url = new URL(`${baseUrl}/oauth/authorize`) + url.searchParams.set('client_id', clientId) + url.searchParams.set('response_type', 'code') + url.searchParams.set('code_challenge', codeChallenge) + url.searchParams.set('code_challenge_method', 'S256') + url.searchParams.set('redirect_uri', redirectUri) + url.searchParams.set('state', state) + + const handshake: OutlineHandshake = { baseUrl, clientId, codeVerifier } + return { authorizeUrl: url.toString(), handshake } + }, + + async exchangeCode({ code, redirectUri, handshake }) { + const hs = asHandshake(handshake) + if (!hs.codeVerifier) { + throw new Error('Missing PKCE code verifier from authorize step.') + } + + const params = new URLSearchParams({ + grant_type: 'authorization_code', + client_id: hs.clientId, + redirect_uri: redirectUri, + code_verifier: hs.codeVerifier, + code, + }) + + const res = await fetch(`${hs.baseUrl}/oauth/token`, { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: params.toString(), + }) + + const json = (await res.json().catch(() => ({}))) as { + access_token?: string + error?: string + error_description?: string + message?: string + } + + if (!res.ok) { + const message = + json.error_description || json.message || json.error || res.statusText + throw new Error(`OAuth token exchange failed: ${message}`) + } + + if (!json.access_token) { + throw new Error('OAuth token exchange did not return an access token.') + } + + return { accessToken: json.access_token } + }, + + async validateToken({ token, handshake }) { + const hs = asHandshake(handshake) + const { data } = await apiRequest( + 'auth.info', + {}, + { + token, + baseUrl: hs.baseUrl, + }, + ) + return { + id: data.user.id, + label: data.user.name, + baseUrl: hs.baseUrl, + oauthClientId: hs.clientId, + teamName: data.team.name, + } + }, + } +} + +export function createOutlineTokenStore(): TokenStore { + return { + async active() { + const config = await getConfig() + if (!config.api_token) return null + const baseUrl = config.base_url ?? DEFAULT_BASE_URL + const oauthClientId = config.oauth_client_id ?? '' + 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, oauthClientId, teamName: '' }, + } + }, + 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, + }) + }, + async clear() { + await clearConfig() + }, + } +} diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 8c6654c..a58df84 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -55,9 +55,11 @@ export async function saveConfig( */ export async function clearConfig(): Promise { const existing = await getConfig() - const { api_token, base_url, oauth_client_id, ...rest } = existing + const { api_token, base_url, oauth_client_id, auth_user_id, auth_user_name, ...rest } = existing void api_token void base_url void oauth_client_id + void auth_user_id + void auth_user_name await setConfig(rest as Config) } diff --git a/src/lib/config.ts b/src/lib/config.ts index cc6f6f8..00ac592 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -12,6 +12,8 @@ export type Config = CoreConfig & { api_token?: string base_url?: string oauth_client_id?: string + auth_user_id?: string + auth_user_name?: string } /** diff --git a/src/lib/oauth-server.ts b/src/lib/oauth-server.ts deleted file mode 100644 index 52e8425..0000000 --- a/src/lib/oauth-server.ts +++ /dev/null @@ -1,250 +0,0 @@ -import http from 'node:http' -import type { AddressInfo } from 'node:net' - -interface OAuthServerOptions { - state: string - timeoutMs?: number - port?: number -} - -export interface OAuthCallbackServer { - port: number - redirectUri: string - waitForCode: Promise - close: () => void -} - -export const DEFAULT_OAUTH_CALLBACK_PORT = 54969 - -function escapeHtml(text: string): string { - return text - .replaceAll('&', '&') - .replaceAll('<', '<') - .replaceAll('>', '>') - .replaceAll('"', '"') - .replaceAll("'", ''') -} - -function renderPage(title: string, subtitle: string, body: string): string { - return ` - - - - - ${escapeHtml(title)} - Outline CLI - - - -
-

${escapeHtml(title)}

-

${escapeHtml(subtitle)}

- ${body} -

Return to your terminal and continue with ol commands.

-
- -` -} - -function renderSuccessPage(): string { - return renderPage( - 'Login complete', - 'Outline CLI is now authenticated.', - '
You can close this tab now.
', - ) -} - -function renderErrorPage(message: string): string { - return renderPage( - 'Authentication failed', - 'Outline CLI could not finish OAuth login.', - `
${escapeHtml(message)}
`, - ) -} - -export async function startOAuthCallbackServer( - options: OAuthServerOptions, -): Promise { - const { state, timeoutMs = 3 * 60 * 1000, port = DEFAULT_OAUTH_CALLBACK_PORT } = options - let origin = 'http://localhost' - let resolved = false - let resolveCode: (code: string) => void - let rejectCode: (error: Error) => void - - const waitForCode = new Promise((resolve, reject) => { - resolveCode = resolve - rejectCode = reject - }) - - const server = http.createServer((req, res) => { - if (resolved) { - res.writeHead(409, { 'Content-Type': 'text/html; charset=utf-8' }) - res.end(renderErrorPage('Authorization was already received.')) - return - } - - if (req.method !== 'GET' || !req.url) { - res.writeHead(405, { 'Content-Type': 'text/html; charset=utf-8' }) - res.end(renderErrorPage('Invalid callback request method.')) - return - } - - const url = new URL(req.url, origin) - if (url.pathname !== '/callback') { - res.writeHead(404, { 'Content-Type': 'text/html; charset=utf-8' }) - res.end(renderErrorPage('Unknown callback path.')) - return - } - - // Check for OAuth error response first - const error = url.searchParams.get('error') - if (error) { - const errorDescription = url.searchParams.get('error_description') || error - res.writeHead(400, { 'Content-Type': 'text/html; charset=utf-8' }) - res.end(renderErrorPage(`Authorization failed: ${errorDescription}`)) - rejectCode(new Error(`OAuth authorization denied: ${errorDescription}`)) - resolved = true - return - } - - const code = url.searchParams.get('code') - const returnedState = url.searchParams.get('state') - - if (!code || !returnedState) { - res.writeHead(400, { 'Content-Type': 'text/html; charset=utf-8' }) - res.end(renderErrorPage('Missing OAuth code or state parameter.')) - rejectCode(new Error('Missing OAuth authorization code.')) - resolved = true - return - } - - if (returnedState !== state) { - res.writeHead(400, { 'Content-Type': 'text/html; charset=utf-8' }) - res.end(renderErrorPage('Invalid OAuth state parameter.')) - rejectCode(new Error('OAuth state mismatch.')) - resolved = true - return - } - - res.writeHead(200, { 'Content-Type': 'text/html; charset=utf-8' }) - res.end(renderSuccessPage()) - resolved = true - resolveCode(code) - }) - - server.on('error', (err) => { - if (resolved) return - resolved = true - rejectCode(err as Error) - }) - - await new Promise((resolve, reject) => { - const onListening = () => { - server.off('error', onError) - resolve() - } - const onError = (err: Error) => { - server.off('listening', onListening) - reject(err) - } - - server.once('listening', onListening) - server.once('error', onError) - server.listen(port, '127.0.0.1') - }) - - const { port: listeningPort } = server.address() as AddressInfo - origin = `http://localhost:${listeningPort}` - const redirectUri = `${origin}/callback` - - const timeout = setTimeout(() => { - if (resolved) return - resolved = true - rejectCode(new Error('Timed out waiting for OAuth callback.')) - server.close() - }, timeoutMs) - - const close = () => { - clearTimeout(timeout) - server.close() - } - - waitForCode.then( - () => { - clearTimeout(timeout) - server.close() - }, - () => { - clearTimeout(timeout) - server.close() - }, - ) - - return { port: listeningPort, redirectUri, waitForCode, close } -} diff --git a/src/lib/oauth.ts b/src/lib/oauth.ts deleted file mode 100644 index 817c1b4..0000000 --- a/src/lib/oauth.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { fetchWithRetry } from '../transport/fetch-with-retry.js' - -interface AuthorizationUrlOptions { - baseUrl: string - clientId: string - redirectUri: string - codeChallenge: string - state: string -} - -interface TokenExchangeOptions { - baseUrl: string - clientId: string - redirectUri: string - codeVerifier: string - code: string -} - -interface TokenResponse { - access_token?: string - error?: string - error_description?: string - message?: string -} - -export function buildAuthorizationUrl(options: AuthorizationUrlOptions): string { - const { baseUrl, clientId, redirectUri, codeChallenge, state } = options - const url = new URL(`${baseUrl}/oauth/authorize`) - url.searchParams.set('client_id', clientId) - url.searchParams.set('response_type', 'code') - url.searchParams.set('code_challenge', codeChallenge) - url.searchParams.set('code_challenge_method', 'S256') - url.searchParams.set('redirect_uri', redirectUri) - url.searchParams.set('state', state) - return url.toString() -} - -export async function exchangeCodeForToken(options: TokenExchangeOptions): Promise { - const { baseUrl, clientId, redirectUri, codeVerifier, code } = options - const params = new URLSearchParams({ - grant_type: 'authorization_code', - client_id: clientId, - redirect_uri: redirectUri, - code_verifier: codeVerifier, - code, - }) - - const res = await fetchWithRetry({ - url: `${baseUrl}/oauth/token`, - options: { - method: 'POST', - headers: { - 'Content-Type': 'application/x-www-form-urlencoded', - }, - body: params.toString(), - }, - }) - - const json = (await res.json()) as TokenResponse - if (!res.ok) { - const message = json.error_description || json.message || json.error || res.statusText - throw new Error(`OAuth token exchange failed: ${message}`) - } - - if (!json.access_token) { - throw new Error('OAuth token exchange did not return an access token.') - } - - return json.access_token -} diff --git a/src/lib/pkce.ts b/src/lib/pkce.ts deleted file mode 100644 index 4a9aa37..0000000 --- a/src/lib/pkce.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { createHash, randomBytes } from 'node:crypto' - -function base64UrlEncode(buffer: Buffer): string { - return buffer.toString('base64').replace(/\+/g, '-').replace(/\//g, '_').replace(/=+$/, '') -} - -export function generateCodeVerifier(): string { - // 64 bytes => 86 chars, within 43-128 requirement. - return base64UrlEncode(randomBytes(64)) -} - -export function generateCodeChallenge(codeVerifier: string): string { - const hash = createHash('sha256').update(codeVerifier).digest() - return base64UrlEncode(hash) -} - -export function generateState(): string { - return base64UrlEncode(randomBytes(32)) -} diff --git a/src/lib/skills/content.ts b/src/lib/skills/content.ts index 772c967..41c3292 100644 --- a/src/lib/skills/content.ts +++ b/src/lib/skills/content.ts @@ -72,9 +72,13 @@ ol col delete --confirm ### Authentication \`\`\`bash -ol auth login # Configure API token and base URL -ol auth status # Show current auth state -ol auth logout # Clear saved credentials +ol auth login # OAuth login (opens browser); prompts for base URL + client ID if needed +ol auth login --base-url # Specify Outline base URL for this login (saved for future use) +ol auth login --client-id # Specify OAuth client ID for this login (saved for future use) +ol auth login --callback-port # Override local OAuth callback port +ol auth login --json | --ndjson # Machine-readable success envelope +ol auth status # Show current auth state +ol auth logout # Clear saved credentials \`\`\` ### Update & Changelog From dde82cc23f2202a415e00809edc0bd27dad536ab Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Mon, 11 May 2026 18:17:48 +0100 Subject: [PATCH 2/4] refactor: prefer type aliases over interfaces for new shapes Co-Authored-By: Claude Opus 4.7 (1M context) --- src/commands/auth.ts | 2 +- src/lib/api.ts | 2 +- src/lib/auth-provider.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/commands/auth.ts b/src/commands/auth.ts index fc3bd1c..bb945eb 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -9,7 +9,7 @@ import { formatError } from '../lib/output.js' const DEFAULT_OAUTH_CALLBACK_PORT = 54969 -interface AuthInfoResponse { +type AuthInfoResponse = { user: { name: string; email: string } team: { name: string; subdomain: string } } diff --git a/src/lib/api.ts b/src/lib/api.ts index ef5ca3d..833f2a4 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -47,7 +47,7 @@ export interface PaginatedResult { pagination?: Pagination } -export interface ApiRequestOverrides { +export type ApiRequestOverrides = { token?: string baseUrl?: string } diff --git a/src/lib/auth-provider.ts b/src/lib/auth-provider.ts index 631e1bd..5656bd4 100644 --- a/src/lib/auth-provider.ts +++ b/src/lib/auth-provider.ts @@ -12,7 +12,7 @@ import { getConfig, updateConfig } from './config.js' const DEFAULT_BASE_URL = 'https://app.getoutline.com' -interface AuthInfoResponse { +type AuthInfoResponse = { user: { id: string; name: string; email: string } team: { name: string; subdomain: string } } From 4b5cfe12cf2fa1e84ce41e8443f74de817e4e6c8 Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Mon, 11 May 2026 18:22:03 +0100 Subject: [PATCH 3/4] refactor: address PR review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - [P1] Route OAuth token exchange through `fetchWithRetry` so it picks up the shared Undici proxy/retry dispatcher used by the rest of the network stack. - [P2] Preserve `OUTLINE_OAUTH_CALLBACK_PORT` env override by reading it in `commands/auth.ts` and passing it to `attachLoginCommand`'s `preferredPort` (falls back to the default for unparseable values). - [P2] Send the interactive base-URL / client-ID prompts to stderr so `--json` / `--ndjson` envelopes on stdout stay clean. - [P2] Persist `auth_team_name` alongside the rest of the auth identity so `OutlineTokenStore.active()` round-trips the full account shape instead of fabricating an empty string. `teamName` is now optional. - [P2] Document `ol auth login --read-only` in `SKILL_CONTENT` and regenerate the synced skill file. - [P2] Tests now mock `apiRequest` (not `fetchWithRetry`) when exercising `validateToken`, per `CLAUDE.md`. - [P2] New `auth-command.test.ts` covers the Commander-level surface: asserts the `--base-url` / `--client-id` chained options, the `OUTLINE_OAUTH_CALLBACK_PORT` override, and the `onSuccess` hook in both default and JSON output modes. - [P2] New `auth-pages.test.ts` covers the renderSuccess / renderError output so future edits can't silently change the branded UX or break the html-escaping on the error message. - [P2] `OutlineTokenStore.clear` test now inspects the on-disk config and asserts every auth-related key is removed (and non-auth keys like `update_channel` are preserved). - [P3] Delete `saveConfig` from `lib/auth.ts` — superseded by the token store; remove its lingering mocks in `commands.test.ts` and `empty-output.test.ts`. - [P3] Switch test mocks to `vi.mocked(fn)` instead of forced `as ReturnType` casts. Co-Authored-By: Claude Opus 4.7 (1M context) --- skills/outline-cli/SKILL.md | 1 + src/__tests__/auth-command.test.ts | 161 ++++++++++++++++++++++++++++ src/__tests__/auth-pages.test.ts | 31 ++++++ src/__tests__/auth-provider.test.ts | 121 +++++++++++++-------- src/__tests__/auth.test.ts | 16 +-- src/__tests__/commands.test.ts | 1 - src/__tests__/empty-output.test.ts | 1 - src/commands/auth.ts | 12 ++- src/lib/auth-provider.ts | 26 +++-- src/lib/auth.ts | 24 ++--- src/lib/config.ts | 1 + src/lib/skills/content.ts | 1 + 12 files changed, 323 insertions(+), 73 deletions(-) create mode 100644 src/__tests__/auth-command.test.ts create mode 100644 src/__tests__/auth-pages.test.ts diff --git a/skills/outline-cli/SKILL.md b/skills/outline-cli/SKILL.md index c5ab551..499b739 100644 --- a/skills/outline-cli/SKILL.md +++ b/skills/outline-cli/SKILL.md @@ -77,6 +77,7 @@ ol auth login # OAuth login (opens browser); prompts fo ol auth login --base-url # Specify Outline base URL for this login (saved for future use) ol auth login --client-id # Specify OAuth client ID for this login (saved for future use) ol auth login --callback-port # Override local OAuth callback port +ol auth login --read-only # Request read-only scopes (where supported by the Outline instance) ol auth login --json | --ndjson # Machine-readable success envelope ol auth status # Show current auth state ol auth logout # Clear saved credentials diff --git a/src/__tests__/auth-command.test.ts b/src/__tests__/auth-command.test.ts new file mode 100644 index 0000000..5c64954 --- /dev/null +++ b/src/__tests__/auth-command.test.ts @@ -0,0 +1,161 @@ +import { Command } from 'commander' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +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(), +})) + +vi.mock('../lib/api.js', () => ({ + apiRequest: vi.fn(), +})) + +// The cli-core dependency drives the OAuth flow end-to-end; stub it so we can +// assert the command-surface contract (flag wiring, success envelope) without +// reaching out to a real Outline instance. +vi.mock('@doist/cli-core/auth', async () => { + const actual = + await vi.importActual('@doist/cli-core/auth') + return { + ...actual, + attachLoginCommand: vi.fn(), + } +}) + +describe('registerAuthCommand', () => { + afterEach(() => { + vi.clearAllMocks() + }) + + it('attaches the OAuth login subcommand via cli-core with the expected wiring', async () => { + const { attachLoginCommand } = await import('@doist/cli-core/auth') + const fakeLogin = new Command('login') + vi.mocked(attachLoginCommand).mockReturnValue(fakeLogin) + + const { registerAuthCommand } = await import('../commands/auth.js') + const program = new Command() + program.exitOverride() + registerAuthCommand(program) + + expect(attachLoginCommand).toHaveBeenCalledTimes(1) + const [parent, options] = vi.mocked(attachLoginCommand).mock.calls[0] + expect(parent.name()).toBe('auth') + expect(options.preferredPort).toBe(54969) + expect(options.resolveScopes({ readOnly: false, flags: {} })).toEqual([]) + expect(typeof options.renderSuccess).toBe('function') + expect(typeof options.renderError).toBe('function') + + // The returned Command is the consumer's hook for chaining extra + // flags. Our wrapper must register --base-url and --client-id on it. + const optionFlags = fakeLogin.options.map((o) => o.flags) + expect(optionFlags).toContain('--base-url ') + expect(optionFlags).toContain('--client-id ') + }) + + it('honours OUTLINE_OAUTH_CALLBACK_PORT for the preferred callback port', async () => { + process.env.OUTLINE_OAUTH_CALLBACK_PORT = '7000' + try { + vi.resetModules() + const { attachLoginCommand } = await import('@doist/cli-core/auth') + vi.mocked(attachLoginCommand).mockReturnValue(new Command('login')) + + const { registerAuthCommand } = await import('../commands/auth.js') + const program = new Command() + program.exitOverride() + registerAuthCommand(program) + + const [, options] = vi.mocked(attachLoginCommand).mock.calls[0] + expect(options.preferredPort).toBe(7000) + } finally { + delete process.env.OUTLINE_OAUTH_CALLBACK_PORT + } + }) + + it('falls back to the default callback port when the env var is unparseable', async () => { + process.env.OUTLINE_OAUTH_CALLBACK_PORT = 'not-a-number' + try { + vi.resetModules() + const { attachLoginCommand } = await import('@doist/cli-core/auth') + vi.mocked(attachLoginCommand).mockReturnValue(new Command('login')) + + const { registerAuthCommand } = await import('../commands/auth.js') + const program = new Command() + program.exitOverride() + registerAuthCommand(program) + + const [, options] = vi.mocked(attachLoginCommand).mock.calls[0] + expect(options.preferredPort).toBe(54969) + } finally { + delete process.env.OUTLINE_OAUTH_CALLBACK_PORT + } + }) + + describe('onSuccess hook', () => { + beforeEach(() => { + vi.resetModules() + }) + + it('writes the human success line to stdout in default output mode', async () => { + const logs: string[] = [] + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + logs.push(args.join(' ')) + }) + + const { attachLoginCommand } = await import('@doist/cli-core/auth') + vi.mocked(attachLoginCommand).mockReturnValue(new Command('login')) + + const { registerAuthCommand } = await import('../commands/auth.js') + const program = new Command() + program.exitOverride() + registerAuthCommand(program) + + const [, options] = vi.mocked(attachLoginCommand).mock.calls[0] + options.onSuccess({ + view: { json: false, ndjson: false }, + flags: {}, + account: { + id: 'u', + label: 'Ada', + baseUrl: 'https://x', + oauthClientId: 'c', + teamName: 'Analytics', + }, + }) + + expect(logs.join('\n')).toContain('Authenticated to Analytics as Ada') + }) + + it('stays silent in --json mode so the machine envelope is not polluted', async () => { + const logs: string[] = [] + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + logs.push(args.join(' ')) + }) + + const { attachLoginCommand } = await import('@doist/cli-core/auth') + vi.mocked(attachLoginCommand).mockReturnValue(new Command('login')) + + const { registerAuthCommand } = await import('../commands/auth.js') + const program = new Command() + program.exitOverride() + registerAuthCommand(program) + + const [, options] = vi.mocked(attachLoginCommand).mock.calls[0] + options.onSuccess({ + view: { json: true, ndjson: false }, + flags: {}, + account: { + id: 'u', + label: 'Ada', + baseUrl: 'https://x', + oauthClientId: 'c', + teamName: 'Analytics', + }, + }) + + expect(logs).toHaveLength(0) + }) + }) +}) diff --git a/src/__tests__/auth-pages.test.ts b/src/__tests__/auth-pages.test.ts new file mode 100644 index 0000000..9c6145c --- /dev/null +++ b/src/__tests__/auth-pages.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from 'vitest' +import { renderError, renderSuccess } from '../lib/auth-pages.js' + +describe('renderSuccess', () => { + it('returns a branded HTML page with the post-login title and message', () => { + const html = renderSuccess() + expect(html).toContain('') + expect(html).toContain('Login complete - Outline CLI') + expect(html).toContain('Login complete') + expect(html).toContain('Outline CLI is now authenticated.') + expect(html).toContain('You can close this tab now.') + expect(html).toContain('message success') + }) +}) + +describe('renderError', () => { + it('returns a branded HTML page that includes the failure message', () => { + const html = renderError('Authorization code expired') + expect(html).toContain('Authentication failed - Outline CLI') + expect(html).toContain('Authentication failed') + expect(html).toContain('Outline CLI could not finish OAuth login.') + expect(html).toContain('Authorization code expired') + expect(html).toContain('message error') + }) + + it('html-escapes hostile characters in the failure message', () => { + const html = renderError('') + expect(html).not.toContain('') + expect(html).toContain('<script>alert(1)</script>') + }) +}) diff --git a/src/__tests__/auth-provider.test.ts b/src/__tests__/auth-provider.test.ts index dd6c17e..23c6d18 100644 --- a/src/__tests__/auth-provider.test.ts +++ b/src/__tests__/auth-provider.test.ts @@ -1,4 +1,4 @@ -import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' +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' @@ -11,6 +11,10 @@ vi.mock('../transport/fetch-with-retry.js', () => ({ fetchWithRetry: vi.fn(), })) +vi.mock('../lib/api.js', () => ({ + apiRequest: vi.fn(), +})) + describe('OutlineAuthProvider', () => { beforeEach(() => { process.env.XDG_CONFIG_HOME = TEST_XDG @@ -27,7 +31,6 @@ describe('OutlineAuthProvider', () => { rmSync(TEST_XDG, { recursive: true }) } delete process.env.XDG_CONFIG_HOME - vi.unstubAllGlobals() }) describe('authorize', () => { @@ -62,7 +65,6 @@ describe('OutlineAuthProvider', () => { it('reads base URL and client ID from environment when flags absent', async () => { process.env.OUTLINE_URL = 'https://env.example.com/' - // OUTLINE_OAUTH_CLIENT_ID is read via getOAuthClientId process.env.OUTLINE_OAUTH_CLIENT_ID = 'env-cid' const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') @@ -83,12 +85,12 @@ describe('OutlineAuthProvider', () => { }) describe('exchangeCode', () => { - it('posts form-encoded token exchange and returns access token', async () => { - const fetchMock = vi.fn().mockResolvedValue({ + it('posts form-encoded token exchange via fetchWithRetry and returns access token', async () => { + const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') + vi.mocked(fetchWithRetry).mockResolvedValue({ ok: true, json: async () => ({ access_token: 'tok-abc' }), - }) - vi.stubGlobal('fetch', fetchMock) + } as Response) const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') const provider = createOutlineAuthProvider() @@ -105,14 +107,14 @@ describe('OutlineAuthProvider', () => { }) expect(result.accessToken).toBe('tok-abc') - expect(fetchMock).toHaveBeenCalledTimes(1) - const [tokenUrl, init] = fetchMock.mock.calls[0] - expect(tokenUrl).toBe('https://wiki.example.com/oauth/token') - expect(init.method).toBe('POST') - expect(init.headers).toMatchObject({ + expect(fetchWithRetry).toHaveBeenCalledTimes(1) + const args = vi.mocked(fetchWithRetry).mock.calls[0][0] + expect(args.url).toBe('https://wiki.example.com/oauth/token') + expect(args.options.method).toBe('POST') + expect(args.options.headers).toMatchObject({ 'Content-Type': 'application/x-www-form-urlencoded', }) - const body = new URLSearchParams(init.body as string) + const body = new URLSearchParams(args.options.body as string) expect(body.get('grant_type')).toBe('authorization_code') expect(body.get('client_id')).toBe('cid-xyz') expect(body.get('redirect_uri')).toBe('http://localhost:54969/callback') @@ -121,17 +123,15 @@ describe('OutlineAuthProvider', () => { }) it('surfaces provider error description on failed exchange', async () => { - vi.stubGlobal( - 'fetch', - vi.fn().mockResolvedValue({ - ok: false, - statusText: 'Bad Request', - json: async () => ({ - error: 'invalid_grant', - error_description: 'Authorization code expired', - }), + const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') + vi.mocked(fetchWithRetry).mockResolvedValue({ + ok: false, + statusText: 'Bad Request', + json: async () => ({ + error: 'invalid_grant', + error_description: 'Authorization code expired', }), - ) + } as Response) const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') const provider = createOutlineAuthProvider() @@ -152,16 +152,13 @@ describe('OutlineAuthProvider', () => { }) describe('validateToken', () => { - it('calls auth.info with handshake base URL + token and returns an OutlineAccount', async () => { - const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') - ;(fetchWithRetry as ReturnType).mockResolvedValue({ - ok: true, - json: async () => ({ - data: { - user: { id: 'user-uuid', name: 'Ada Lovelace', email: 'ada@example.com' }, - team: { name: 'Analytics', subdomain: 'analytics' }, - }, - }), + it('calls auth.info with the unsaved token + handshake base URL and returns an OutlineAccount', async () => { + const { apiRequest } = await import('../lib/api.js') + vi.mocked(apiRequest).mockResolvedValue({ + data: { + user: { id: 'user-uuid', name: 'Ada Lovelace', email: 'ada@example.com' }, + team: { name: 'Analytics', subdomain: 'analytics' }, + }, }) const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') @@ -183,10 +180,12 @@ describe('OutlineAuthProvider', () => { teamName: 'Analytics', }) - expect(fetchWithRetry).toHaveBeenCalledTimes(1) - const args = (fetchWithRetry as ReturnType).mock.calls[0][0] - expect(args.url).toBe('https://wiki.example.com/api/auth.info') - expect(args.options.headers.Authorization).toBe('Bearer tok-abc') + expect(apiRequest).toHaveBeenCalledTimes(1) + expect(apiRequest).toHaveBeenCalledWith( + 'auth.info', + {}, + { token: 'tok-abc', baseUrl: 'https://wiki.example.com' }, + ) }) }) }) @@ -222,12 +221,14 @@ describe('OutlineTokenStore', () => { ) const got = await store.active() - expect(got).not.toBeNull() expect(got?.token).toBe('tok-persisted') - expect(got?.account.id).toBe('user-uuid') - expect(got?.account.label).toBe('Ada') - expect(got?.account.baseUrl).toBe('https://wiki.example.com') - expect(got?.account.oauthClientId).toBe('cid-xyz') + expect(got?.account).toEqual({ + id: 'user-uuid', + label: 'Ada', + baseUrl: 'https://wiki.example.com', + oauthClientId: 'cid-xyz', + teamName: 'Analytics', + }) }) it('active returns null when config has a token but no persisted identity', async () => { @@ -243,7 +244,7 @@ describe('OutlineTokenStore', () => { await expect(store.active()).resolves.toBeNull() }) - it('clear removes auth fields', async () => { + it('clear removes all auth fields from the saved config', async () => { const { createOutlineTokenStore } = await import('../lib/auth-provider.js') const store = createOutlineTokenStore() await store.set( @@ -256,7 +257,39 @@ describe('OutlineTokenStore', () => { }, 'tok', ) + + // Sanity: the file is populated with every auth key before clear. + const before = JSON.parse(readFileSync(TEST_CONFIG_PATH, 'utf8')) + expect(before).toMatchObject({ + api_token: 'tok', + base_url: 'https://x', + oauth_client_id: 'c', + auth_user_id: 'u', + auth_user_name: 'l', + auth_team_name: 't', + }) + await store.clear() - await expect(store.active()).resolves.toBeNull() + + // Every auth-related key must be gone from the on-disk config. + const after = JSON.parse(readFileSync(TEST_CONFIG_PATH, 'utf8')) + expect(after).not.toHaveProperty('api_token') + expect(after).not.toHaveProperty('base_url') + expect(after).not.toHaveProperty('oauth_client_id') + expect(after).not.toHaveProperty('auth_user_id') + expect(after).not.toHaveProperty('auth_user_name') + expect(after).not.toHaveProperty('auth_team_name') + }) + + it('clear preserves non-auth config keys', async () => { + writeFileSync( + TEST_CONFIG_PATH, + JSON.stringify({ api_token: 'tok', auth_user_id: 'u', update_channel: 'pre-release' }), + ) + const { createOutlineTokenStore } = await import('../lib/auth-provider.js') + const store = createOutlineTokenStore() + await store.clear() + const after = JSON.parse(readFileSync(TEST_CONFIG_PATH, 'utf8')) + expect(after).toEqual({ update_channel: 'pre-release' }) }) }) diff --git a/src/__tests__/auth.test.ts b/src/__tests__/auth.test.ts index 6f30250..a87beb0 100644 --- a/src/__tests__/auth.test.ts +++ b/src/__tests__/auth.test.ts @@ -58,12 +58,16 @@ describe('auth', () => { await expect(getBaseUrl()).resolves.toBe('https://app.getoutline.com') }) - it('saveConfig and clearConfig work', async () => { - const { saveConfig, clearConfig, getApiToken, getOAuthClientId } = - await import('../lib/auth.js') - await saveConfig('test-token', 'https://wiki.test.com', 'client-id') - await expect(getApiToken()).resolves.toBe('test-token') - await expect(getOAuthClientId()).resolves.toBe('client-id') + it('clearConfig removes the saved token', async () => { + writeFileSync( + TEST_CONFIG_PATH, + JSON.stringify({ + api_token: 'test-token', + base_url: 'https://wiki.test.com', + oauth_client_id: 'client-id', + }), + ) + const { clearConfig, getApiToken } = await import('../lib/auth.js') await clearConfig() await expect(getApiToken()).rejects.toThrow() }) diff --git a/src/__tests__/commands.test.ts b/src/__tests__/commands.test.ts index 21cd7d3..a51d8ff 100644 --- a/src/__tests__/commands.test.ts +++ b/src/__tests__/commands.test.ts @@ -6,7 +6,6 @@ vi.mock('../lib/auth.js', () => ({ getBaseUrl: async () => 'https://test.outline.com', getOAuthClientId: async () => undefined, getTokenSource: async () => 'config' as const, - saveConfig: vi.fn(), clearConfig: vi.fn(), })) diff --git a/src/__tests__/empty-output.test.ts b/src/__tests__/empty-output.test.ts index feb01bc..614fd55 100644 --- a/src/__tests__/empty-output.test.ts +++ b/src/__tests__/empty-output.test.ts @@ -7,7 +7,6 @@ vi.mock('../lib/auth.js', () => ({ getBaseUrl: async () => 'https://test.outline.com', getOAuthClientId: async () => undefined, getTokenSource: async () => 'config' as const, - saveConfig: vi.fn(), clearConfig: vi.fn(), })) diff --git a/src/commands/auth.ts b/src/commands/auth.ts index bb945eb..2a66206 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -14,6 +14,16 @@ type AuthInfoResponse = { team: { name: string; subdomain: string } } +function resolvePreferredCallbackPort(): number { + const raw = process.env.OUTLINE_OAUTH_CALLBACK_PORT?.trim() + if (!raw) return DEFAULT_OAUTH_CALLBACK_PORT + const parsed = Number(raw) + if (!Number.isInteger(parsed) || parsed < 1 || parsed > 65_535) { + return DEFAULT_OAUTH_CALLBACK_PORT + } + return parsed +} + export function registerAuthCommand(program: Command): void { const auth = program.command('auth').description('Manage authentication') @@ -23,7 +33,7 @@ export function registerAuthCommand(program: Command): void { attachLoginCommand(auth, { provider, store, - preferredPort: DEFAULT_OAUTH_CALLBACK_PORT, + preferredPort: resolvePreferredCallbackPort(), resolveScopes: () => [], renderSuccess, renderError, diff --git a/src/lib/auth-provider.ts b/src/lib/auth-provider.ts index 5656bd4..25b9272 100644 --- a/src/lib/auth-provider.ts +++ b/src/lib/auth-provider.ts @@ -6,6 +6,7 @@ import { generateVerifier, type TokenStore, } 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 { getConfig, updateConfig } from './config.js' @@ -22,7 +23,7 @@ export type OutlineAccount = AuthAccount & { label: string baseUrl: string oauthClientId: string - teamName: string + teamName?: string } type OutlineHandshake = Record & { @@ -41,7 +42,8 @@ function stringFlag(flags: Record, key: string): string | undef } async function prompt(question: string): Promise { - const rl = createInterface({ input: process.stdin, output: process.stdout }) + // Output to stderr so `--json` / `--ndjson` envelopes on stdout stay clean. + const rl = createInterface({ input: process.stdin, output: process.stderr }) try { return (await rl.question(question)).trim() } finally { @@ -107,10 +109,13 @@ export function createOutlineAuthProvider(): AuthProvider { code, }) - const res = await fetch(`${hs.baseUrl}/oauth/token`, { - method: 'POST', - headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, - body: params.toString(), + const res = await fetchWithRetry({ + url: `${hs.baseUrl}/oauth/token`, + options: { + method: 'POST', + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + body: params.toString(), + }, }) const json = (await res.json().catch(() => ({}))) as { @@ -170,7 +175,13 @@ export function createOutlineTokenStore(): TokenStore { } return { token: config.api_token, - account: { id, label, baseUrl, oauthClientId, teamName: '' }, + account: { + id, + label, + baseUrl, + oauthClientId, + teamName: config.auth_team_name, + }, } }, async set(account, token) { @@ -180,6 +191,7 @@ export function createOutlineTokenStore(): TokenStore { oauth_client_id: account.oauthClientId, auth_user_id: account.id, auth_user_name: account.label, + auth_team_name: account.teamName, }) }, async clear() { diff --git a/src/lib/auth.ts b/src/lib/auth.ts index a58df84..21fc7a7 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -1,4 +1,4 @@ -import { type Config, getConfig, setConfig, updateConfig } from './config.js' +import { type Config, getConfig, setConfig } from './config.js' const DEFAULT_BASE_URL = 'https://app.getoutline.com' @@ -37,17 +37,6 @@ export async function getTokenSource(): Promise<'env' | 'config' | null> { return null } -export async function saveConfig( - token: string, - baseUrl?: string, - oauthClientId?: string, -): Promise { - const updates: Partial = { api_token: token } - if (baseUrl) updates.base_url = baseUrl.replace(/\/$/, '') - if (oauthClientId) updates.oauth_client_id = oauthClientId - await updateConfig(updates) -} - /** * Clear the auth-related keys without deleting the file. The config is now * shared with non-auth settings (notably `update_channel`); a blanket unlink @@ -55,11 +44,20 @@ export async function saveConfig( */ export async function clearConfig(): Promise { const existing = await getConfig() - const { api_token, base_url, oauth_client_id, auth_user_id, auth_user_name, ...rest } = existing + const { + api_token, + base_url, + oauth_client_id, + auth_user_id, + auth_user_name, + auth_team_name, + ...rest + } = existing 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 00ac592..baf7dda 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -14,6 +14,7 @@ export type Config = CoreConfig & { oauth_client_id?: string auth_user_id?: string auth_user_name?: string + auth_team_name?: string } /** diff --git a/src/lib/skills/content.ts b/src/lib/skills/content.ts index 41c3292..38895ff 100644 --- a/src/lib/skills/content.ts +++ b/src/lib/skills/content.ts @@ -76,6 +76,7 @@ ol auth login # OAuth login (opens browser); prompts fo ol auth login --base-url # Specify Outline base URL for this login (saved for future use) ol auth login --client-id # Specify OAuth client ID for this login (saved for future use) ol auth login --callback-port # Override local OAuth callback port +ol auth login --read-only # Request read-only scopes (where supported by the Outline instance) ol auth login --json | --ndjson # Machine-readable success envelope ol auth status # Show current auth state ol auth logout # Clear saved credentials From 744799aa748728e2d4ec2939ee8034143867165d Mon Sep 17 00:00:00 2001 From: Scott Lovegrove Date: Mon, 11 May 2026 18:33:09 +0100 Subject: [PATCH 4/4] test: consolidate auth-flow tests Squashed redundant cases in auth-provider / auth-pages / auth-command without losing coverage of the substance: provider authorize / exchange / validate; store round-trip + legacy + clear; chained command flags + env-driven callback port + success-mode silencing in JSON output; HTML rendering + escape. Net 487 -> 266 lines across the three new test files. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/__tests__/auth-command.test.ts | 182 ++++---------- src/__tests__/auth-pages.test.ts | 20 +- src/__tests__/auth-provider.test.ts | 376 ++++++++++------------------ 3 files changed, 180 insertions(+), 398 deletions(-) diff --git a/src/__tests__/auth-command.test.ts b/src/__tests__/auth-command.test.ts index 5c64954..e76476d 100644 --- a/src/__tests__/auth-command.test.ts +++ b/src/__tests__/auth-command.test.ts @@ -1,5 +1,5 @@ import { Command } from 'commander' -import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' vi.mock('../lib/auth.js', () => ({ getApiToken: async () => 'test-token', @@ -9,153 +9,63 @@ vi.mock('../lib/auth.js', () => ({ clearConfig: vi.fn(), })) -vi.mock('../lib/api.js', () => ({ - apiRequest: vi.fn(), +vi.mock('../lib/api.js', () => ({ apiRequest: vi.fn() })) + +// Stub cli-core's `attachLoginCommand` so we can inspect the surface contract +// (chained flags, env-driven port, success hook) without running the flow. +vi.mock('@doist/cli-core/auth', async () => ({ + ...(await vi.importActual('@doist/cli-core/auth')), + attachLoginCommand: vi.fn(), })) -// The cli-core dependency drives the OAuth flow end-to-end; stub it so we can -// assert the command-surface contract (flag wiring, success envelope) without -// reaching out to a real Outline instance. -vi.mock('@doist/cli-core/auth', async () => { - const actual = - await vi.importActual('@doist/cli-core/auth') - return { - ...actual, - attachLoginCommand: vi.fn(), - } +async function captureAttachOptions() { + const { attachLoginCommand } = await import('@doist/cli-core/auth') + const login = new Command('login') + vi.mocked(attachLoginCommand).mockReturnValue(login) + const { registerAuthCommand } = await import('../commands/auth.js') + const program = new Command() + program.exitOverride() + registerAuthCommand(program) + return { options: vi.mocked(attachLoginCommand).mock.calls[0][1], login } +} + +afterEach(() => { + vi.clearAllMocks() + delete process.env.OUTLINE_OAUTH_CALLBACK_PORT }) describe('registerAuthCommand', () => { - afterEach(() => { - vi.clearAllMocks() - }) - - it('attaches the OAuth login subcommand via cli-core with the expected wiring', async () => { - const { attachLoginCommand } = await import('@doist/cli-core/auth') - const fakeLogin = new Command('login') - vi.mocked(attachLoginCommand).mockReturnValue(fakeLogin) - - const { registerAuthCommand } = await import('../commands/auth.js') - const program = new Command() - program.exitOverride() - registerAuthCommand(program) - - expect(attachLoginCommand).toHaveBeenCalledTimes(1) - const [parent, options] = vi.mocked(attachLoginCommand).mock.calls[0] - expect(parent.name()).toBe('auth') - expect(options.preferredPort).toBe(54969) - expect(options.resolveScopes({ readOnly: false, flags: {} })).toEqual([]) - expect(typeof options.renderSuccess).toBe('function') - expect(typeof options.renderError).toBe('function') - - // The returned Command is the consumer's hook for chaining extra - // flags. Our wrapper must register --base-url and --client-id on it. - const optionFlags = fakeLogin.options.map((o) => o.flags) - expect(optionFlags).toContain('--base-url ') - expect(optionFlags).toContain('--client-id ') - }) - - it('honours OUTLINE_OAUTH_CALLBACK_PORT for the preferred callback port', async () => { + 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' - try { - vi.resetModules() - const { attachLoginCommand } = await import('@doist/cli-core/auth') - vi.mocked(attachLoginCommand).mockReturnValue(new Command('login')) + const logs: string[] = [] + vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { + logs.push(args.join(' ')) + }) - const { registerAuthCommand } = await import('../commands/auth.js') - const program = new Command() - program.exitOverride() - registerAuthCommand(program) + const { options, login } = await captureAttachOptions() - const [, options] = vi.mocked(attachLoginCommand).mock.calls[0] - expect(options.preferredPort).toBe(7000) - } finally { - delete process.env.OUTLINE_OAUTH_CALLBACK_PORT + expect(options.preferredPort).toBe(7000) + const flags = login.options.map((o) => o.flags) + expect(flags).toContain('--base-url ') + expect(flags).toContain('--client-id ') + + const account = { + id: 'u', + label: 'Ada', + baseUrl: 'https://x', + oauthClientId: 'c', + teamName: 'Analytics', } + await options.onSuccess({ view: { json: false, ndjson: false }, flags: {}, account }) + await options.onSuccess({ view: { json: true, ndjson: false }, flags: {}, account }) + + expect(logs.length).toBe(1) + expect(logs[0]).toContain('Authenticated to Analytics as Ada') }) it('falls back to the default callback port when the env var is unparseable', async () => { process.env.OUTLINE_OAUTH_CALLBACK_PORT = 'not-a-number' - try { - vi.resetModules() - const { attachLoginCommand } = await import('@doist/cli-core/auth') - vi.mocked(attachLoginCommand).mockReturnValue(new Command('login')) - - const { registerAuthCommand } = await import('../commands/auth.js') - const program = new Command() - program.exitOverride() - registerAuthCommand(program) - - const [, options] = vi.mocked(attachLoginCommand).mock.calls[0] - expect(options.preferredPort).toBe(54969) - } finally { - delete process.env.OUTLINE_OAUTH_CALLBACK_PORT - } - }) - - describe('onSuccess hook', () => { - beforeEach(() => { - vi.resetModules() - }) - - it('writes the human success line to stdout in default output mode', async () => { - const logs: string[] = [] - vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { - logs.push(args.join(' ')) - }) - - const { attachLoginCommand } = await import('@doist/cli-core/auth') - vi.mocked(attachLoginCommand).mockReturnValue(new Command('login')) - - const { registerAuthCommand } = await import('../commands/auth.js') - const program = new Command() - program.exitOverride() - registerAuthCommand(program) - - const [, options] = vi.mocked(attachLoginCommand).mock.calls[0] - options.onSuccess({ - view: { json: false, ndjson: false }, - flags: {}, - account: { - id: 'u', - label: 'Ada', - baseUrl: 'https://x', - oauthClientId: 'c', - teamName: 'Analytics', - }, - }) - - expect(logs.join('\n')).toContain('Authenticated to Analytics as Ada') - }) - - it('stays silent in --json mode so the machine envelope is not polluted', async () => { - const logs: string[] = [] - vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => { - logs.push(args.join(' ')) - }) - - const { attachLoginCommand } = await import('@doist/cli-core/auth') - vi.mocked(attachLoginCommand).mockReturnValue(new Command('login')) - - const { registerAuthCommand } = await import('../commands/auth.js') - const program = new Command() - program.exitOverride() - registerAuthCommand(program) - - const [, options] = vi.mocked(attachLoginCommand).mock.calls[0] - options.onSuccess({ - view: { json: true, ndjson: false }, - flags: {}, - account: { - id: 'u', - label: 'Ada', - baseUrl: 'https://x', - oauthClientId: 'c', - teamName: 'Analytics', - }, - }) - - expect(logs).toHaveLength(0) - }) + const { options } = await captureAttachOptions() + expect(options.preferredPort).toBe(54969) }) }) diff --git a/src/__tests__/auth-pages.test.ts b/src/__tests__/auth-pages.test.ts index 9c6145c..b517d32 100644 --- a/src/__tests__/auth-pages.test.ts +++ b/src/__tests__/auth-pages.test.ts @@ -1,30 +1,18 @@ import { describe, expect, it } from 'vitest' import { renderError, renderSuccess } from '../lib/auth-pages.js' -describe('renderSuccess', () => { - it('returns a branded HTML page with the post-login title and message', () => { +describe('auth pages', () => { + it('renderSuccess returns the branded post-login page', () => { const html = renderSuccess() - expect(html).toContain('') expect(html).toContain('Login complete - Outline CLI') - expect(html).toContain('Login complete') expect(html).toContain('Outline CLI is now authenticated.') expect(html).toContain('You can close this tab now.') - expect(html).toContain('message success') }) -}) -describe('renderError', () => { - it('returns a branded HTML page that includes the failure message', () => { - const html = renderError('Authorization code expired') + it('renderError surfaces the failure message and escapes hostile HTML', () => { + const html = renderError('') expect(html).toContain('Authentication failed - Outline CLI') - expect(html).toContain('Authentication failed') expect(html).toContain('Outline CLI could not finish OAuth login.') - expect(html).toContain('Authorization code expired') - expect(html).toContain('message error') - }) - - it('html-escapes hostile characters in the failure message', () => { - const html = renderError('') expect(html).not.toContain('') expect(html).toContain('<script>alert(1)</script>') }) diff --git a/src/__tests__/auth-provider.test.ts b/src/__tests__/auth-provider.test.ts index 23c6d18..8c96615 100644 --- a/src/__tests__/auth-provider.test.ts +++ b/src/__tests__/auth-provider.test.ts @@ -7,288 +7,172 @@ 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') -vi.mock('../transport/fetch-with-retry.js', () => ({ - fetchWithRetry: vi.fn(), -})) +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() +}) -vi.mock('../lib/api.js', () => ({ - apiRequest: vi.fn(), -})) +afterEach(() => { + if (existsSync(TEST_XDG)) rmSync(TEST_XDG, { recursive: true }) + delete process.env.XDG_CONFIG_HOME +}) describe('OutlineAuthProvider', () => { - 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() - }) - - afterEach(() => { - if (existsSync(TEST_XDG)) { - rmSync(TEST_XDG, { recursive: true }) - } - delete process.env.XDG_CONFIG_HOME - }) - - describe('authorize', () => { - it('builds an outline authorize URL with PKCE params from explicit flags', async () => { - const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') - const provider = createOutlineAuthProvider() - - const result = await provider.authorize({ - redirectUri: 'http://localhost:54969/callback', - state: 'state-123', - scopes: [], - readOnly: false, - flags: { baseUrl: 'https://wiki.example.com/', clientId: 'cid-xyz' }, - handshake: {}, - }) - - const url = new URL(result.authorizeUrl) - expect(url.origin + url.pathname).toBe('https://wiki.example.com/oauth/authorize') - expect(url.searchParams.get('client_id')).toBe('cid-xyz') - expect(url.searchParams.get('response_type')).toBe('code') - expect(url.searchParams.get('redirect_uri')).toBe('http://localhost:54969/callback') - expect(url.searchParams.get('state')).toBe('state-123') - expect(url.searchParams.get('code_challenge_method')).toBe('S256') - expect(url.searchParams.get('code_challenge')).toMatch(/^[A-Za-z0-9_-]+$/) - - const handshake = result.handshake as Record - expect(handshake.baseUrl).toBe('https://wiki.example.com') - expect(handshake.clientId).toBe('cid-xyz') - expect(typeof handshake.codeVerifier).toBe('string') - expect(handshake.codeVerifier.length).toBeGreaterThan(40) + 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({ + redirectUri: 'http://localhost:54969/callback', + state: 'state-123', + scopes: [], + readOnly: false, + flags: { baseUrl: 'https://wiki.example.com/', clientId: 'cid-xyz' }, + handshake: {}, }) - it('reads base URL and client ID from environment when flags absent', async () => { - process.env.OUTLINE_URL = 'https://env.example.com/' - process.env.OUTLINE_OAUTH_CLIENT_ID = 'env-cid' - - const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') - const provider = createOutlineAuthProvider() - - const result = await provider.authorize({ - redirectUri: 'http://localhost:54969/callback', - state: 's', - scopes: [], - readOnly: false, - flags: {}, - handshake: {}, - }) - const url = new URL(result.authorizeUrl) - expect(url.origin).toBe('https://env.example.com') - expect(url.searchParams.get('client_id')).toBe('env-cid') + const url = new URL(result.authorizeUrl) + expect(url.origin + url.pathname).toBe('https://wiki.example.com/oauth/authorize') + expect(Object.fromEntries(url.searchParams)).toMatchObject({ + client_id: 'cid-xyz', + response_type: 'code', + redirect_uri: 'http://localhost:54969/callback', + state: 'state-123', + code_challenge_method: 'S256', }) - }) - - describe('exchangeCode', () => { - it('posts form-encoded token exchange via fetchWithRetry and returns access token', async () => { - const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') - vi.mocked(fetchWithRetry).mockResolvedValue({ - ok: true, - json: async () => ({ access_token: 'tok-abc' }), - } as Response) + expect(url.searchParams.get('code_challenge')).toMatch(/^[A-Za-z0-9_-]+$/) - const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') - const provider = createOutlineAuthProvider() - - const result = await provider.exchangeCode({ - code: 'auth-code', - state: 'state-x', - redirectUri: 'http://localhost:54969/callback', - handshake: { - baseUrl: 'https://wiki.example.com', - clientId: 'cid-xyz', - codeVerifier: 'verifier-1', - }, - }) - - expect(result.accessToken).toBe('tok-abc') - expect(fetchWithRetry).toHaveBeenCalledTimes(1) - const args = vi.mocked(fetchWithRetry).mock.calls[0][0] - expect(args.url).toBe('https://wiki.example.com/oauth/token') - expect(args.options.method).toBe('POST') - expect(args.options.headers).toMatchObject({ - 'Content-Type': 'application/x-www-form-urlencoded', - }) - const body = new URLSearchParams(args.options.body as string) - expect(body.get('grant_type')).toBe('authorization_code') - expect(body.get('client_id')).toBe('cid-xyz') - expect(body.get('redirect_uri')).toBe('http://localhost:54969/callback') - expect(body.get('code_verifier')).toBe('verifier-1') - expect(body.get('code')).toBe('auth-code') - }) - - it('surfaces provider error description on failed exchange', async () => { - const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') - vi.mocked(fetchWithRetry).mockResolvedValue({ - ok: false, - statusText: 'Bad Request', - json: async () => ({ - error: 'invalid_grant', - error_description: 'Authorization code expired', - }), - } as Response) - - const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') - const provider = createOutlineAuthProvider() - - await expect( - provider.exchangeCode({ - code: 'c', - state: 's', - redirectUri: 'http://localhost:54969/callback', - handshake: { - baseUrl: 'https://wiki.example.com', - clientId: 'cid-xyz', - codeVerifier: 'verifier-1', - }, - }), - ).rejects.toThrow('OAuth token exchange failed: Authorization code expired') + const handshake = result.handshake as Record + expect(handshake).toMatchObject({ + baseUrl: 'https://wiki.example.com', + clientId: 'cid-xyz', }) + expect(handshake.codeVerifier?.length).toBeGreaterThan(40) }) - describe('validateToken', () => { - it('calls auth.info with the unsaved token + handshake base URL and returns an OutlineAccount', async () => { - const { apiRequest } = await import('../lib/api.js') - vi.mocked(apiRequest).mockResolvedValue({ - data: { - user: { id: 'user-uuid', name: 'Ada Lovelace', email: 'ada@example.com' }, - team: { name: 'Analytics', subdomain: 'analytics' }, - }, - }) + 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) - const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') - const provider = createOutlineAuthProvider() - - const account = await provider.validateToken({ - token: 'tok-abc', - handshake: { - baseUrl: 'https://wiki.example.com', - clientId: 'cid-xyz', - }, - }) - - expect(account).toEqual({ - id: 'user-uuid', - label: 'Ada Lovelace', - baseUrl: 'https://wiki.example.com', - oauthClientId: 'cid-xyz', - teamName: 'Analytics', - }) + const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') + const provider = createOutlineAuthProvider() + const handshake = { + baseUrl: 'https://wiki.example.com', + clientId: 'cid-xyz', + codeVerifier: 'verifier-1', + } - expect(apiRequest).toHaveBeenCalledTimes(1) - expect(apiRequest).toHaveBeenCalledWith( - 'auth.info', - {}, - { token: 'tok-abc', baseUrl: 'https://wiki.example.com' }, - ) + const result = await provider.exchangeCode({ + code: 'auth-code', + state: 's', + redirectUri: 'http://localhost:54969/callback', + handshake, + }) + expect(result.accessToken).toBe('tok-abc') + + const args = vi.mocked(fetchWithRetry).mock.calls[0][0] + expect(args.url).toBe('https://wiki.example.com/oauth/token') + const body = new URLSearchParams(args.options.body as string) + expect(Object.fromEntries(body)).toEqual({ + grant_type: 'authorization_code', + client_id: 'cid-xyz', + redirect_uri: 'http://localhost:54969/callback', + code_verifier: 'verifier-1', + code: 'auth-code', }) - }) -}) - -describe('OutlineTokenStore', () => { - beforeEach(() => { - process.env.XDG_CONFIG_HOME = TEST_XDG - mkdirSync(TEST_CONFIG_DIR, { recursive: true }) - delete process.env.OUTLINE_API_TOKEN - vi.resetModules() - }) - afterEach(() => { - if (existsSync(TEST_XDG)) { - rmSync(TEST_XDG, { recursive: true }) - } - delete process.env.XDG_CONFIG_HOME + vi.mocked(fetchWithRetry).mockResolvedValueOnce({ + ok: false, + statusText: 'Bad Request', + json: async () => ({ error_description: 'Authorization code expired' }), + } as Response) + await expect( + provider.exchangeCode({ + code: 'c', + state: 's', + redirectUri: 'http://localhost:54969/callback', + handshake, + }), + ).rejects.toThrow('OAuth token exchange failed: Authorization code expired') }) - it('set persists token + account fields and active reads them back', async () => { - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - const store = createOutlineTokenStore() - - await store.set( - { - id: 'user-uuid', - label: 'Ada', - baseUrl: 'https://wiki.example.com', - oauthClientId: 'cid-xyz', - teamName: 'Analytics', + it('validateToken calls auth.info with the unsaved token and builds an OutlineAccount', async () => { + const { apiRequest } = await import('../lib/api.js') + vi.mocked(apiRequest).mockResolvedValue({ + data: { + user: { id: 'user-uuid', name: 'Ada Lovelace', email: 'ada@example.com' }, + team: { name: 'Analytics', subdomain: 'analytics' }, }, - 'tok-persisted', - ) + }) - const got = await store.active() - expect(got?.token).toBe('tok-persisted') - expect(got?.account).toEqual({ + const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') + const account = await createOutlineAuthProvider().validateToken({ + token: 'tok-abc', + handshake: { baseUrl: 'https://wiki.example.com', clientId: 'cid-xyz' }, + }) + + expect(account).toEqual({ id: 'user-uuid', - label: 'Ada', + label: 'Ada Lovelace', baseUrl: 'https://wiki.example.com', oauthClientId: 'cid-xyz', teamName: 'Analytics', }) + expect(apiRequest).toHaveBeenCalledWith( + 'auth.info', + {}, + { token: 'tok-abc', baseUrl: 'https://wiki.example.com' }, + ) }) +}) - it('active returns null when config has a token but no persisted identity', async () => { - writeFileSync(TEST_CONFIG_PATH, JSON.stringify({ api_token: 'legacy-tok' })) - const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - const store = createOutlineTokenStore() - await expect(store.active()).resolves.toBeNull() - }) - - it('active returns null when no token saved', async () => { +describe('OutlineTokenStore', () => { + const sampleAccount = { + id: 'user-uuid', + label: 'Ada', + baseUrl: 'https://wiki.example.com', + oauthClientId: 'cid-xyz', + teamName: 'Analytics', + } + + it('round-trips token + account through the config file', async () => { const { createOutlineTokenStore } = await import('../lib/auth-provider.js') const store = createOutlineTokenStore() - await expect(store.active()).resolves.toBeNull() + await store.set(sampleAccount, 'tok-persisted') + const got = await store.active() + expect(got).toEqual({ token: 'tok-persisted', account: sampleAccount }) }) - it('clear removes all auth fields from the saved config', async () => { + 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') - const store = createOutlineTokenStore() - await store.set( - { - id: 'u', - label: 'l', - baseUrl: 'https://x', - oauthClientId: 'c', - teamName: 't', - }, - 'tok', - ) - - // Sanity: the file is populated with every auth key before clear. - const before = JSON.parse(readFileSync(TEST_CONFIG_PATH, 'utf8')) - expect(before).toMatchObject({ - api_token: 'tok', - base_url: 'https://x', - oauth_client_id: 'c', - auth_user_id: 'u', - auth_user_name: 'l', - auth_team_name: 't', - }) - - await store.clear() - - // Every auth-related key must be gone from the on-disk config. - const after = JSON.parse(readFileSync(TEST_CONFIG_PATH, 'utf8')) - expect(after).not.toHaveProperty('api_token') - expect(after).not.toHaveProperty('base_url') - expect(after).not.toHaveProperty('oauth_client_id') - expect(after).not.toHaveProperty('auth_user_id') - expect(after).not.toHaveProperty('auth_user_name') - expect(after).not.toHaveProperty('auth_team_name') + await expect(createOutlineTokenStore().active()).resolves.toBeNull() }) - it('clear preserves non-auth config keys', async () => { + it('clear strips every auth field but preserves unrelated config keys', async () => { writeFileSync( TEST_CONFIG_PATH, - JSON.stringify({ api_token: 'tok', auth_user_id: 'u', update_channel: 'pre-release' }), + 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', + }), ) const { createOutlineTokenStore } = await import('../lib/auth-provider.js') - const store = createOutlineTokenStore() - await store.clear() + await createOutlineTokenStore().clear() const after = JSON.parse(readFileSync(TEST_CONFIG_PATH, 'utf8')) expect(after).toEqual({ update_channel: 'pre-release' }) })