refactor: delegate auth flow to @doist/cli-core/auth

skills/outline-cli/SKILL.md

@@ -73,9 +73,14 @@ ol col delete <id> --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 <url>         # Specify Outline base URL for this login (saved for future use)
+ol auth login --client-id <id>         # Specify OAuth client ID for this login (saved for future use)
+ol auth login --callback-port <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
 ```
 
 ### Update & Changelog

src/__tests__/auth-command.test.ts

@@ -0,0 +1,71 @@
+import { Command } from 'commander'
+import { afterEach, 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() }))
+
+// 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<typeof import('@doist/cli-core/auth')>('@doist/cli-core/auth')),
+    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', () => {
+    it('wires --base-url / --client-id, env-driven port, and prints success only in human output mode', async () => {
+        process.env.OUTLINE_OAUTH_CALLBACK_PORT = '7000'
+        const logs: string[] = []
+        vi.spyOn(console, 'log').mockImplementation((...args: unknown[]) => {
+            logs.push(args.join(' '))
+        })
+
+        const { options, login } = await captureAttachOptions()
+
+        expect(options.preferredPort).toBe(7000)
+        const flags = login.options.map((o) => o.flags)
+        expect(flags).toContain('--base-url <url>')
+        expect(flags).toContain('--client-id <clientId>')
+
+        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'
+        const { options } = await captureAttachOptions()
+        expect(options.preferredPort).toBe(54969)
+    })
+})

src/__tests__/auth-pages.test.ts

@@ -0,0 +1,19 @@
+import { describe, expect, it } from 'vitest'
+import { renderError, renderSuccess } from '../lib/auth-pages.js'
+
+describe('auth pages', () => {
+    it('renderSuccess returns the branded post-login page', () => {
+        const html = renderSuccess()
+        expect(html).toContain('<title>Login complete - Outline CLI</title>')
+        expect(html).toContain('Outline CLI is now authenticated.')
+        expect(html).toContain('You can close this tab now.')
+    })
+
+    it('renderError surfaces the failure message and escapes hostile HTML', () => {
+        const html = renderError('<script>alert(1)</script>')
+        expect(html).toContain('<title>Authentication failed - Outline CLI</title>')
+        expect(html).toContain('Outline CLI could not finish OAuth login.')
+        expect(html).not.toContain('<script>alert(1)</script>')
+        expect(html).toContain('&lt;script&gt;alert(1)&lt;/script&gt;')
+    })
+})

src/__tests__/auth-provider.test.ts

@@ -0,0 +1,179 @@
+import { existsSync, mkdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+
+const TEST_XDG = join(tmpdir(), `outline-cli-test-${process.pid}-auth-provider`)
+const TEST_CONFIG_DIR = join(TEST_XDG, 'outline-cli')
+const TEST_CONFIG_PATH = join(TEST_CONFIG_DIR, 'config.json')
+
+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()
+})
+
+afterEach(() => {
+    if (existsSync(TEST_XDG)) rmSync(TEST_XDG, { recursive: true })
+    delete process.env.XDG_CONFIG_HOME
+})
+
+describe('OutlineAuthProvider', () => {
+    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: {},
+        })
+
+        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',
+        })
+        expect(url.searchParams.get('code_challenge')).toMatch(/^[A-Za-z0-9_-]+$/)
+
+        const handshake = result.handshake as Record<string, string>
+        expect(handshake).toMatchObject({
+            baseUrl: 'https://wiki.example.com',
+            clientId: 'cid-xyz',
+        })
+        expect(handshake.codeVerifier?.length).toBeGreaterThan(40)
+    })
+
+    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 handshake = {
+            baseUrl: 'https://wiki.example.com',
+            clientId: 'cid-xyz',
+            codeVerifier: 'verifier-1',
+        }
+
+        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',
+        })
+
+        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('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' },
+            },
+        })
+
+        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 Lovelace',
+            baseUrl: 'https://wiki.example.com',
+            oauthClientId: 'cid-xyz',
+            teamName: 'Analytics',
+        })
+        expect(apiRequest).toHaveBeenCalledWith(
+            'auth.info',
+            {},
+            { token: 'tok-abc', baseUrl: 'https://wiki.example.com' },
+        )
+    })
+})
+
+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 store.set(sampleAccount, 'tok-persisted')
+        const got = await store.active()
+        expect(got).toEqual({ token: 'tok-persisted', account: sampleAccount })
+    })
+
+    it('active returns null when the saved token predates the persisted-identity fields', async () => {
+        writeFileSync(TEST_CONFIG_PATH, JSON.stringify({ api_token: 'legacy-tok' }))
+        const { createOutlineTokenStore } = await import('../lib/auth-provider.js')
+        await expect(createOutlineTokenStore().active()).resolves.toBeNull()
+    })
+
+    it('clear strips every auth field but preserves unrelated config keys', async () => {
+        writeFileSync(
+            TEST_CONFIG_PATH,
+            JSON.stringify({
+                api_token: 'tok',
+                base_url: 'https://x',
+                oauth_client_id: 'c',
+                auth_user_id: 'u',
+                auth_user_name: 'l',
+                auth_team_name: 't',
+                update_channel: 'pre-release',
+            }),
+        )
+        const { createOutlineTokenStore } = await import('../lib/auth-provider.js')
+        await createOutlineTokenStore().clear()
+        const after = JSON.parse(readFileSync(TEST_CONFIG_PATH, 'utf8'))
+        expect(after).toEqual({ update_channel: 'pre-release' })
+    })
+})

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()
     })

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(),
 }))
 

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(),
 }))