diff --git a/package-lock.json b/package-lock.json index 444c773..e7476aa 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,11 +10,12 @@ "hasInstallScript": true, "license": "MIT", "dependencies": { - "@doist/cli-core": "0.16.1", + "@doist/cli-core": "0.19.0", "chalk": "5.6.2", "commander": "14.0.2", "marked": "18.0.3", "marked-terminal-renderer": "2.2.0", + "oauth4webapi": "3.8.6", "open": "10.2.0", "undici": "7.22.0" }, @@ -132,9 +133,9 @@ } }, "node_modules/@doist/cli-core": { - "version": "0.16.1", - "resolved": "https://registry.npmjs.org/@doist/cli-core/-/cli-core-0.16.1.tgz", - "integrity": "sha512-OoHWYM8Rv+eoPhVR1BUMwFIejZGk5Wyxheva4Cp9x3zTh4RURE+OIJ03G95oBegbXCA2O9LRtZZ9Ww0D6vU1dQ==", + "version": "0.19.0", + "resolved": "https://registry.npmjs.org/@doist/cli-core/-/cli-core-0.19.0.tgz", + "integrity": "sha512-+CswqzGwcFC78v8oH7uC5poS9Ptqxpw7UKIXs3/xLIbgpGMJB+LKYT88lq8LGw2ZJ2bevsRN867lApgnWdJ4vw==", "license": "MIT", "dependencies": { "chalk": "5.6.2", @@ -150,6 +151,7 @@ "commander": ">=14", "marked": ">=18", "marked-terminal-renderer": ">=2", + "oauth4webapi": ">=3", "open": ">=10", "vitest": ">=4.1" }, @@ -163,6 +165,9 @@ "marked-terminal-renderer": { "optional": true }, + "oauth4webapi": { + "optional": true + }, "open": { "optional": true }, @@ -7662,6 +7667,15 @@ "node": ">=18" } }, + "node_modules/oauth4webapi": { + "version": "3.8.6", + "resolved": "https://registry.npmjs.org/oauth4webapi/-/oauth4webapi-3.8.6.tgz", + "integrity": "sha512-iwemM91xz8nryHti2yTmg5fhyEMVOkOXwHNqbvcATjyajb5oQxCQzrNOA6uElRHuMhQQTKUyFKV9y/CNyg25BQ==", + "license": "MIT", + "funding": { + "url": "https://github.com/sponsors/panva" + } + }, "node_modules/object-assign": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/object-assign/-/object-assign-4.1.1.tgz", diff --git a/package.json b/package.json index 4361127..ed6b1b3 100644 --- a/package.json +++ b/package.json @@ -48,11 +48,12 @@ "node": ">=20.18.1" }, "dependencies": { - "@doist/cli-core": "0.16.1", + "@doist/cli-core": "0.19.0", "chalk": "5.6.2", "commander": "14.0.2", "marked": "18.0.3", "marked-terminal-renderer": "2.2.0", + "oauth4webapi": "3.8.6", "open": "10.2.0", "undici": "7.22.0" }, diff --git a/src/__tests__/api.test.ts b/src/__tests__/api.test.ts index 9d33aa9..5dcd8a5 100644 --- a/src/__tests__/api.test.ts +++ b/src/__tests__/api.test.ts @@ -1,24 +1,34 @@ -import { afterEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -vi.mock('../lib/auth.js', () => ({ - getApiToken: async () => 'test-token', - getBaseUrl: async () => 'https://test.outline.com', +const authMocks = vi.hoisted(() => ({ + getApiToken: vi.fn(async () => 'test-token'), + getBaseUrl: vi.fn(async () => 'https://test.outline.com'), + proactiveRefresh: vi.fn(async () => undefined), + reactiveRefresh: vi.fn(async () => false), })) +vi.mock('../lib/auth.js', () => authMocks) + vi.mock('../transport/fetch-with-retry.js', () => ({ fetchWithRetry: vi.fn(), })) describe('apiRequest', () => { + beforeEach(() => { + delete process.env.OUTLINE_API_TOKEN + authMocks.getApiToken.mockReset().mockResolvedValue('test-token') + authMocks.getBaseUrl.mockReset().mockResolvedValue('https://test.outline.com') + authMocks.proactiveRefresh.mockReset().mockResolvedValue(undefined) + authMocks.reactiveRefresh.mockReset().mockResolvedValue(false) + }) + afterEach(() => { vi.clearAllMocks() + vi.unstubAllEnvs() }) it('uses fetchWithRetry for API requests', async () => { - const mockResponse = { - ok: true, - json: async () => ({ data: { id: '123' } }), - } + const mockResponse = { ok: true, json: async () => ({ data: { id: '123' } }) } const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') ;(fetchWithRetry as ReturnType).mockResolvedValue(mockResponse) @@ -41,10 +51,7 @@ describe('apiRequest', () => { it('returns data and pagination', async () => { const mockResponse = { ok: true, - json: async () => ({ - data: [{ id: '1' }], - pagination: { offset: 0, limit: 25 }, - }), + json: async () => ({ data: [{ id: '1' }], pagination: { offset: 0, limit: 25 } }), } const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') ;(fetchWithRetry as ReturnType).mockResolvedValue(mockResponse) @@ -59,18 +66,15 @@ describe('apiRequest', () => { it('throws on non-ok response with API message', async () => { const mockResponse = { ok: false, - status: 401, - statusText: 'Unauthorized', - json: async () => ({ - error: 'auth_required', - message: 'Authentication required', - }), + status: 500, + statusText: 'Internal Server Error', + json: async () => ({ error: 'server_error', message: 'Server exploded' }), } const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') ;(fetchWithRetry as ReturnType).mockResolvedValue(mockResponse) const { apiRequest } = await import('../lib/api.js') - await expect(apiRequest('auth.info')).rejects.toThrow('Authentication required') + await expect(apiRequest('documents.list')).rejects.toThrow('Server exploded') }) it('throws generic message when no API error body', async () => { @@ -90,4 +94,60 @@ describe('apiRequest', () => { 'API error: 500 Internal Server Error', ) }) + + it('force-refreshes and retries once when a managed token gets a 401', async () => { + const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') + const f = fetchWithRetry as ReturnType + f.mockResolvedValueOnce({ + ok: false, + status: 401, + statusText: 'Unauthorized', + json: async () => ({}), + }) + f.mockResolvedValueOnce({ ok: true, json: async () => ({ data: { id: 'ok' } }) }) + authMocks.reactiveRefresh.mockResolvedValueOnce(true) + authMocks.getApiToken + .mockResolvedValueOnce('stale-token') + .mockResolvedValueOnce('rotated-token') + + const { apiRequest } = await import('../lib/api.js') + const result = await apiRequest('documents.info', { id: 'abc' }) + + expect(result.data).toEqual({ id: 'ok' }) + expect(authMocks.reactiveRefresh).toHaveBeenCalledTimes(1) + expect(f).toHaveBeenCalledTimes(2) + const retryHeaders = f.mock.calls[1][0].options.headers as Record + expect(retryHeaders.Authorization).toBe('Bearer rotated-token') + }) + + it('proactively refreshes a managed token and uses the rotated token (no extra store read)', async () => { + const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') + const f = fetchWithRetry as ReturnType + f.mockResolvedValue({ ok: true, json: async () => ({ data: {} }) }) + authMocks.proactiveRefresh.mockResolvedValueOnce('rotated-proactive') + + const { apiRequest } = await import('../lib/api.js') + await apiRequest('documents.list') + + expect(authMocks.proactiveRefresh).toHaveBeenCalledTimes(1) + // The proactive token is reused — no fallback read of getApiToken. + expect(authMocks.getApiToken).not.toHaveBeenCalled() + const headers = f.mock.calls[0][0].options.headers as Record + expect(headers.Authorization).toBe('Bearer rotated-proactive') + }) + + it('does not refresh when OUTLINE_API_TOKEN is set (unmanaged token)', async () => { + vi.stubEnv('OUTLINE_API_TOKEN', 'env-tok') + const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') + ;(fetchWithRetry as ReturnType).mockResolvedValue({ + ok: true, + json: async () => ({ data: {} }), + }) + + const { apiRequest } = await import('../lib/api.js') + await apiRequest('documents.list') + + expect(authMocks.proactiveRefresh).not.toHaveBeenCalled() + expect(authMocks.reactiveRefresh).not.toHaveBeenCalled() + }) }) diff --git a/src/__tests__/auth-command.test.ts b/src/__tests__/auth-command.test.ts index 9c87d11..35b17d7 100644 --- a/src/__tests__/auth-command.test.ts +++ b/src/__tests__/auth-command.test.ts @@ -8,6 +8,9 @@ vi.mock('../lib/auth.js', () => ({ getOAuthClientId: async () => undefined, getActiveTokenSource: async () => process.env.OUTLINE_API_TOKEN ? ('env' as const) : ('secure-store' as const), + // status resolves the live token for the selected account via this; echo + // the snapshot token back (no refresh in these command-surface tests). + refreshedTokenForStatus: async (_account: unknown, fallback: string) => fallback, })) vi.mock('../lib/api.js', () => ({ apiRequest: vi.fn() })) @@ -118,6 +121,8 @@ describe('auth status subcommand', () => { const program = await buildProgram() await program.parseAsync(['node', 'ol', 'auth', 'status']) + // status probes auth.info with the (account-scoped) live token resolved + // for the selected account. expect(apiRequest).toHaveBeenCalledWith( 'auth.info', {}, diff --git a/src/__tests__/auth-provider.test.ts b/src/__tests__/auth-provider.test.ts index 2f666f4..863992c 100644 --- a/src/__tests__/auth-provider.test.ts +++ b/src/__tests__/auth-provider.test.ts @@ -1,5 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { + AUTH_INFO, + errResponse, LEGACY_CLEAR_PAYLOAD, LEGACY_CONFIG, okResponse, @@ -20,7 +22,9 @@ const keyringMocks = vi.hoisted(() => ({ createKeyringTokenStore: vi.fn(), inner: { active: vi.fn(), + activeBundle: vi.fn(), set: vi.fn(), + setBundle: vi.fn(), clear: vi.fn(), list: vi.fn(), setDefault: vi.fn(), @@ -94,15 +98,14 @@ describe('createOutlineAuthProvider', () => { }) expect(url.searchParams.get('code_challenge')).toMatch(/^[A-Za-z0-9_-]+$/) + // createPkceProvider carries the verifier + resolved client id through + // the handshake (base URL rides the provider closure, not the handshake). const handshake = result.handshake as Record - expect(handshake).toMatchObject({ - baseUrl: 'https://wiki.example.com', - clientId: 'cid-xyz', - }) + expect(handshake.clientId).toBe('cid-xyz') expect(handshake.codeVerifier?.length).toBeGreaterThan(40) }) - it('exchangeCode posts via fetchWithRetry and surfaces provider errors', async () => { + it('exchangeCode posts to the token endpoint via outline transport; maps failures to the typed code', async () => { const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') vi.mocked(fetchWithRetry).mockResolvedValueOnce(okResponse({ access_token: 'tok-abc' })) @@ -124,7 +127,7 @@ describe('createOutlineAuthProvider', () => { 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) + const body = new URLSearchParams(args.options?.body as string) expect(Object.fromEntries(body)).toEqual({ grant_type: 'authorization_code', client_id: 'cid-xyz', @@ -133,11 +136,9 @@ describe('createOutlineAuthProvider', () => { code: 'auth-code', }) - vi.mocked(fetchWithRetry).mockResolvedValueOnce({ - ok: false, - statusText: 'Bad Request', - json: async () => ({ error_description: 'Authorization code expired' }), - } as Response) + vi.mocked(fetchWithRetry).mockResolvedValueOnce( + errResponse(400, 'Bad Request', { error_description: 'Authorization code expired' }), + ) await expect( provider.exchangeCode({ code: 'c', @@ -145,22 +146,29 @@ describe('createOutlineAuthProvider', () => { redirectUri: 'http://localhost:54969/callback', handshake, }), - ).rejects.toThrow('OAuth token exchange failed: Authorization code expired') + ).rejects.toMatchObject({ code: 'AUTH_TOKEN_EXCHANGE_FAILED' }) }) - it('validateToken calls auth.info with the unsaved token and builds an OutlineAccount', async () => { + it('validateToken builds an OutlineAccount using the base URL captured at authorize', 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' }, - }, - }) + vi.mocked(apiRequest).mockResolvedValue({ data: AUTH_INFO }) const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') - const account = await createOutlineAuthProvider().validateToken({ + const provider = createOutlineAuthProvider() + // authorize seeds the provider's base-URL closure (from the flag), + // which validate then reuses instead of a handshake field. + await provider.authorize({ + redirectUri: 'http://localhost:54969/callback', + state: 's', + scopes: [], + readOnly: false, + flags: { baseUrl: 'https://wiki.example.com/', clientId: 'cid-xyz' }, + handshake: {}, + }) + + const account = await provider.validateToken({ token: 'tok-abc', - handshake: { baseUrl: 'https://wiki.example.com', clientId: 'cid-xyz' }, + handshake: { clientId: 'cid-xyz' }, }) expect(account).toEqual({ @@ -176,6 +184,52 @@ describe('createOutlineAuthProvider', () => { { token: 'tok-abc', baseUrl: 'https://wiki.example.com' }, ) }) + + it('refreshToken rotates the bundle, resolving base URL + client id from stored config (no prompt)', async () => { + // A fresh provider used only for silent refresh never runs authorize, + // so the resolvers fall back to the logged-in record — never prompting. + configMocks.getConfig.mockResolvedValue({ + users: [ + { + id: 'user-uuid', + name: 'Ada', + base_url: 'https://wiki.example.com', + oauth_client_id: 'cid-xyz', + }, + ], + default_user_id: 'user-uuid', + }) + const { fetchWithRetry } = await import('../transport/fetch-with-retry.js') + vi.mocked(fetchWithRetry).mockClear() // isolate this test's POST from earlier ones + vi.mocked(fetchWithRetry).mockResolvedValue( + Response.json({ + access_token: 'tok-new', + refresh_token: 'r-new', + expires_in: 3600, + token_type: 'bearer', + }), + ) + + const { createOutlineAuthProvider } = await import('../lib/auth-provider.js') + const result = await createOutlineAuthProvider().refreshToken?.({ + refreshToken: 'r-old', + handshake: {}, + }) + + expect(result?.accessToken).toBe('tok-new') + expect(result?.refreshToken).toBe('r-new') + const called = vi.mocked(fetchWithRetry).mock.calls[0][0] + const calledUrl = called.url instanceof Request ? called.url.url : String(called.url) + expect(calledUrl).toBe('https://wiki.example.com/oauth/token') + + // Public-client form body: a regression to confidential-client refresh + // (sending a client_secret) must fail here. + const body = new URLSearchParams(called.options?.body as string) + expect(body.get('grant_type')).toBe('refresh_token') + expect(body.get('refresh_token')).toBe('r-old') + expect(body.get('client_id')).toBe('cid-xyz') + expect(body.has('client_secret')).toBe(false) + }) }) describe('createOutlineTokenStore', () => { @@ -184,7 +238,9 @@ describe('createOutlineTokenStore', () => { delete process.env.OUTLINE_URL keyringMocks.createKeyringTokenStore.mockClear() keyringMocks.inner.active.mockReset().mockResolvedValue(null) + keyringMocks.inner.activeBundle.mockReset().mockResolvedValue(null) keyringMocks.inner.set.mockReset().mockResolvedValue(undefined) + keyringMocks.inner.setBundle.mockReset().mockResolvedValue(undefined) keyringMocks.inner.clear.mockReset().mockResolvedValue(undefined) keyringMocks.inner.list.mockReset().mockResolvedValue([]) keyringMocks.inner.setDefault.mockReset().mockResolvedValue(undefined) @@ -397,6 +453,46 @@ describe('createOutlineTokenStore', () => { expect(callOrder).toEqual(['migrate', 'set']) }) + + it('activeBundle env short-circuit returns an access-only bundle and bypasses inner', async () => { + vi.stubEnv(TOKEN_ENV_VAR, 'env_token_value') + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + + const snapshot = await createOutlineTokenStore().activeBundle() + + expect(snapshot?.bundle).toEqual({ accessToken: 'env_token_value' }) + expect(keyringMocks.inner.activeBundle).not.toHaveBeenCalled() + }) + + it('activeBundle forwards to the v2 store, preserving refresh state', async () => { + keyringMocks.inner.activeBundle.mockResolvedValue({ + account: STORED_ACCOUNT, + bundle: { accessToken: 'a', refreshToken: 'r', accessTokenExpiresAt: 123 }, + }) + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + + const snapshot = await createOutlineTokenStore().activeBundle() + + expect(snapshot?.bundle).toEqual({ + accessToken: 'a', + refreshToken: 'r', + accessTokenExpiresAt: 123, + }) + expect(keyringMocks.inner.activeBundle).toHaveBeenCalledTimes(1) + }) + + it('setBundle forwards to inner and discharges legacy state when migration is inconclusive', async () => { + migrateMocks.runMigrateLegacyAuth.mockResolvedValue(SKIPPED_RESULT) + const bundle = { accessToken: 'a', refreshToken: 'r' } + const createOutlineTokenStore = await loadCreateOutlineTokenStore() + + await createOutlineTokenStore().setBundle(STORED_ACCOUNT, bundle, { promoteDefault: true }) + + expect(keyringMocks.inner.setBundle).toHaveBeenCalledWith(STORED_ACCOUNT, bundle, { + promoteDefault: true, + }) + expect(configMocks.updateConfig).toHaveBeenCalledWith(LEGACY_CLEAR_PAYLOAD) + }) }) describe('matchOutlineAccount', () => { diff --git a/src/__tests__/auth.test.ts b/src/__tests__/auth.test.ts index c4dea1d..43f04a2 100644 --- a/src/__tests__/auth.test.ts +++ b/src/__tests__/auth.test.ts @@ -123,4 +123,35 @@ describe('auth', () => { const { getOAuthClientId } = await import('../lib/auth.js') await expect(getOAuthClientId()).resolves.toBe('cid-record') }) + + it('reactiveRefresh maps an unrefreshable token to NoTokenError (prompts re-login)', async () => { + // A stored access token with no refresh token can't be rotated, so the + // real refreshAccessToken throws AUTH_REFRESH_UNAVAILABLE — which the + // helper surfaces as a re-login prompt rather than a raw refresh error. + writeFileSync( + TEST_CONFIG_PATH, + JSON.stringify({ + config_version: 2, + users: [ + { + id: 'u', + name: 'Ada', + base_url: 'https://wiki.example.com', + oauth_client_id: 'cid', + token: 'plain-access', + }, + ], + default_user_id: 'u', + }), + ) + const { reactiveRefresh } = await import('../lib/auth.js') + + let caught: unknown + try { + await reactiveRefresh() + } catch (e) { + caught = e + } + expect((caught as { code?: string }).code).toBe('NO_TOKEN') + }) }) diff --git a/src/__tests__/user-records.test.ts b/src/__tests__/user-records.test.ts index acce45d..01a61c8 100644 --- a/src/__tests__/user-records.test.ts +++ b/src/__tests__/user-records.test.ts @@ -98,6 +98,43 @@ describe('createOutlineUserRecordStore', () => { }) }) + it('round-trips expiry + refresh flag, but never persists the refresh token in plaintext', async () => { + // The expiry/flag metadata must round-trip (proactive refresh needs the + // expiry), but the refresh token is a long-lived secret and must stay + // in the secure store only — `fallbackRefreshToken` is dropped, never + // written to config, even when cli-core supplies it (keyring offline). + const { createOutlineUserRecordStore } = await import('../lib/user-records.js') + + await createOutlineUserRecordStore().upsert({ + account: STORED_ACCOUNT, + accessTokenExpiresAt: 1_700_000_000_000, + refreshTokenExpiresAt: 1_800_000_000_000, + hasRefreshToken: true, + fallbackRefreshToken: 'plain-refresh', + }) + + const stored = { + ...ADA, + access_token_expires_at: 1_700_000_000_000, + refresh_token_expires_at: 1_800_000_000_000, + has_refresh_token: true, + } + const [[written]] = configMock.updateConfig.mock.calls + expect(written).toEqual({ users: [stored] }) + expect(written.users[0]).not.toHaveProperty('refresh_token') + + // ...and back out through list() — still no refresh material. + configMock.getConfig.mockResolvedValue({ users: [stored] }) + const [record] = await createOutlineUserRecordStore().list() + expect(record).toEqual({ + account: STORED_ACCOUNT, + accessTokenExpiresAt: 1_700_000_000_000, + refreshTokenExpiresAt: 1_800_000_000_000, + hasRefreshToken: true, + }) + expect(record.fallbackRefreshToken).toBeUndefined() + }) + it('remove() drops the matching record', async () => { configMock.getConfig.mockResolvedValue({ users: [ADA, GRACE] }) const { createOutlineUserRecordStore } = await import('../lib/user-records.js') diff --git a/src/commands/auth.ts b/src/commands/auth.ts index a2dc509..9f327a7 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -16,6 +16,7 @@ import { type OutlineAccount, type OutlineTokenStore, } from '../lib/auth-provider.js' +import { refreshedTokenForStatus } from '../lib/auth.js' import { CliError } from '../lib/errors.js' const DEFAULT_OAUTH_CALLBACK_PORT = 54969 @@ -104,13 +105,20 @@ export function registerAuthCommand(program: Command): void { attachStatusCommand(auth, { store, description: 'Show current authentication state', - async fetchLive({ token, account }) { + async fetchLive({ account, token }) { try { + // Refresh the *selected* account before the check (scoped via + // its id + base URL/client id), then probe with the rotated + // token. Outline access tokens last ~an hour; without this + // `auth status` can't self-heal even though normal commands + // do. Passing the resolved token keeps the check tied to the + // requested account rather than the default. + const liveToken = await refreshedTokenForStatus(account, token) const [{ data: info }, source] = await Promise.all([ apiRequest( 'auth.info', {}, - { token, baseUrl: account.baseUrl }, + { token: liveToken, baseUrl: account.baseUrl }, ), getActiveTokenSource(), ]) diff --git a/src/lib/api.ts b/src/lib/api.ts index 833f2a4..675ee50 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -1,5 +1,6 @@ import { fetchWithRetry } from '../transport/fetch-with-retry.js' -import { getApiToken, getBaseUrl } from './auth.js' +import { TOKEN_ENV_VAR } from './auth-constants.js' +import { getApiToken, getBaseUrl, proactiveRefresh, reactiveRefresh } from './auth.js' import { type SpinnerOptions, withSpinner } from './spinner.js' /** @@ -52,6 +53,21 @@ export type ApiRequestOverrides = { baseUrl?: string } +/** + * Resolve the token for a request. A caller-supplied override is used as-is. + * On the managed path, prefer the token `proactiveRefresh` resolved (rotated + * or current) so unrefreshable/access-only accounts stay on a single store + * read; only fall back to `getApiToken` when proactive refresh bows out. + */ +async function resolveRequestToken(managed: boolean, override?: string): Promise { + if (override) return override + if (managed) { + const refreshed = await proactiveRefresh() + if (refreshed) return refreshed + } + return getApiToken() +} + /** * Core API request function without spinner wrapping. */ @@ -60,22 +76,37 @@ async function rawApiRequest( body: object = {}, overrides: ApiRequestOverrides = {}, ): Promise> { + // Only stored credentials can be refreshed: a caller-supplied token + // override or the `OUTLINE_API_TOKEN` env var is taken as-is. + const managed = !overrides.token && !process.env[TOKEN_ENV_VAR]?.trim() + + // Resolve the base URL and the (proactively refreshed) token in parallel. const [resolvedBaseUrl, resolvedToken] = await Promise.all([ overrides.baseUrl ? Promise.resolve(overrides.baseUrl.replace(/\/$/, '')) : getBaseUrl(), - overrides.token ? Promise.resolve(overrides.token) : getApiToken(), + resolveRequestToken(managed, overrides.token), ]) - const res = await fetchWithRetry({ - url: `${resolvedBaseUrl}/api/${path}`, - options: { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - Authorization: `Bearer ${resolvedToken}`, + const performRequest = (token: string) => + fetchWithRetry({ + url: `${resolvedBaseUrl}/api/${path}`, + options: { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: `Bearer ${token}`, + }, + body: JSON.stringify(body), }, - body: JSON.stringify(body), - }, - }) + }) + + let res = await performRequest(resolvedToken) + + // Reactive path: a 401 on a managed token triggers a forced rotation and + // a single retry. `reactiveRefresh` throws `NoTokenError` when the refresh + // token is gone, so an unrecoverable 401 surfaces the re-login hint. + if (res.status === 401 && managed && (await reactiveRefresh())) { + res = await performRequest(await getApiToken()) + } if (!res.ok) { let message = `API error: ${res.status} ${res.statusText}` diff --git a/src/lib/auth-provider.ts b/src/lib/auth-provider.ts index 24045ad..f994119 100644 --- a/src/lib/auth-provider.ts +++ b/src/lib/auth-provider.ts @@ -3,10 +3,10 @@ import { type AccountRef, type AuthProvider, createKeyringTokenStore, - deriveChallenge, - generateVerifier, + createPkceProvider, type KeyringTokenStore, type MigrateAuthResult, + type TokenBundle, } from '@doist/cli-core/auth' import { fetchWithRetry } from '../transport/fetch-with-retry.js' import { apiRequest } from './api.js' @@ -26,16 +26,6 @@ export type AuthInfoResponse = { export type OutlineTokenStore = KeyringTokenStore -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 @@ -75,88 +65,61 @@ async function resolveClientId(flags: Record): Promise 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) +/** + * Routes cli-core's OAuth HTTP through outline's transport so the proxy / + * decompression dispatcher applies — to the token exchange AND the + * oauth4webapi refresh grant (cli-core threads this into oauth4webapi's + * `customFetch`), which would otherwise capture the bare global `fetch`. + */ +const outlineFetch: typeof fetch = (input, init) => + fetchWithRetry({ url: input as RequestInfo | URL, options: init ?? {} }) - const handshake: OutlineHandshake = { baseUrl, clientId, codeVerifier } - return { authorizeUrl: url.toString(), handshake } +export function createOutlineAuthProvider(): AuthProvider { + // Captured at `authorize` (which may prompt for it) and reused by + // `exchangeCode` / `validate` so a single login can't double-prompt. A + // provider built only for silent refresh never runs `authorize`, so this + // stays undefined there and `tokenUrl` falls back to stored config — the + // refresh path must never prompt. + let baseUrl: string | undefined + return createPkceProvider({ + authorizeUrl: async ({ flags }) => { + baseUrl = await resolveBaseUrl(flags) + return `${baseUrl}/oauth/authorize` }, - - 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 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 { - 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 } + tokenUrl: async ({ handshake }) => { + // `handshake.baseUrl` lets a caller scope refresh to a specific + // account's instance (status --user); otherwise the authorize-time + // value, then the default config. + const base = + baseUrl ?? (handshake.baseUrl as string | undefined) ?? (await getBaseUrl()) + return `${base}/oauth/token` }, - - async validateToken({ token, handshake }) { - const hs = asHandshake(handshake) + // Prefer a handshake-scoped client id (account-aware refresh); else + // resolve from flag/config (only prompts when neither is set, so it's + // safe on the refresh path — the logged-in record carries one). + clientId: ({ handshake, flags }) => { + const fromHandshake = handshake.clientId + return typeof fromHandshake === 'string' && fromHandshake + ? fromHandshake + : resolveClientId(flags) + }, + validate: async ({ token, handshake }) => { + const base = baseUrl ?? (await getBaseUrl()) const { data } = await apiRequest( 'auth.info', {}, - { - token, - baseUrl: hs.baseUrl, - }, + { token, baseUrl: base }, ) return makeOutlineAccount({ id: data.user.id, label: data.user.name, - baseUrl: hs.baseUrl, - oauthClientId: hs.clientId, + baseUrl: base, + oauthClientId: handshake.clientId as string, teamName: data.team.name, }) }, - } + fetchImpl: outlineFetch, + }) } /** @@ -273,39 +236,68 @@ export function createOutlineTokenStore(): OutlineTokenStore { const result = await ensureMigrated() // memoised return result === null || !migrationIsConclusive(result) } - return { - async active(ref?: AccountRef) { - if (ref === undefined) { - const envToken = process.env[TOKEN_ENV_VAR]?.trim() - if (envToken) { - return { - token: envToken, - account: makeOutlineAccount({ - id: '', - label: '', - baseUrl: await getBaseUrl(), - }), - } - } + /** + * Shared env → v2 → legacy resolution for `active` / `activeBundle`. + * `fromV2` reads the matching cli-core method; `fromEnvOrLegacy` maps an + * env/legacy token into the caller's shape (env + legacy carry no refresh + * material, so the bundle form surfaces as access-only). + */ + async function resolveAuth( + ref: AccountRef | undefined, + fromV2: () => Promise, + fromEnvOrLegacy: (token: string, account: OutlineAccount) => T, + ): Promise { + if (ref === undefined) { + const envToken = process.env[TOKEN_ENV_VAR]?.trim() + if (envToken) { + return fromEnvOrLegacy( + envToken, + makeOutlineAccount({ id: '', label: '', baseUrl: await getBaseUrl() }), + ) } - await ensureMigrated() - const fromStore = await inner.active(ref) - if (fromStore) return fromStore + } + await ensureMigrated() + const fromStore = await fromV2() + if (fromStore) return fromStore + const legacy = await readLegacyTokenSnapshot() + if (legacy && (ref === undefined || matchOutlineAccount(legacy.account, ref))) { + return fromEnvOrLegacy(legacy.token, legacy.account) + } + return null + } - const legacy = await readLegacyTokenSnapshot() - if (legacy && (ref === undefined || matchOutlineAccount(legacy.account, ref))) { - return legacy - } - return null + // `ensureMigrated` runs before the v2 write so the post-write discharge + // can't race a pending migration into re-grabbing the legacy token. The + // discharge is best-effort: a lingering `api_token` is dormant because + // reads hit v2 first. (`clear` needs the stricter propagating variant.) + async function writeThenDischargeLegacy(write: () => Promise): Promise { + await ensureMigrated() + await write() + if (await migrationIsInconclusive()) { + await dischargeLegacyState().catch(() => undefined) + } + } + + return { + active(ref?: AccountRef) { + return resolveAuth( + ref, + () => inner.active(ref), + (token, account) => ({ token, account }), + ) }, - async set(account: OutlineAccount, token: string) { - await ensureMigrated() - await inner.set(account, token) - if (await migrationIsInconclusive()) { - // Best-effort: a lingering `api_token` is dormant because - // `active()` reads v2 first. - await dischargeLegacyState().catch(() => undefined) - } + activeBundle(ref?: AccountRef) { + return resolveAuth( + ref, + () => inner.activeBundle(ref), + (token, account) => ({ account, bundle: { accessToken: token } }), + ) + }, + set(account: OutlineAccount, token: string) { + return writeThenDischargeLegacy(() => inner.set(account, token)) + }, + setBundle(account: OutlineAccount, bundle: TokenBundle, options) { + return writeThenDischargeLegacy(() => inner.setBundle(account, bundle, options)) }, async clear(ref?: AccountRef) { await ensureMigrated() diff --git a/src/lib/auth.ts b/src/lib/auth.ts index 6a40df0..e5d51bb 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -1,13 +1,14 @@ -import { SecureStoreUnavailableError } from '@doist/cli-core/auth' +import { refreshAccessToken, SecureStoreUnavailableError } from '@doist/cli-core/auth' import { TOKEN_ENV_VAR } from './auth-constants.js' import { + createOutlineAuthProvider, createOutlineTokenStore, getActiveTokenSource, type OutlineTokenStore, } from './auth-provider.js' -import { getConfig } from './config.js' +import { getConfig, getConfigPath } from './config.js' import { CliError } from './errors.js' -import { DEFAULT_BASE_URL } from './outline-account.js' +import { DEFAULT_BASE_URL, type OutlineAccount } from './outline-account.js' import { getDefaultUserRecord } from './user-records.js' export { SecureStoreUnavailableError, getActiveTokenSource, TOKEN_ENV_VAR } @@ -35,6 +36,91 @@ function tokenStore(): OutlineTokenStore { return storeSingleton } +let providerSingleton: ReturnType | undefined +function authProvider(): ReturnType { + if (!providerSingleton) providerSingleton = createOutlineAuthProvider() + return providerSingleton +} + +// Caller-provided O_EXCL lock so concurrent `ol` invocations don't issue +// parallel refresh grants. Resolved per-call (not cached) to honour test +// config-path mocking, mirroring `getConfigPath`. +function refreshLockPath(): string { + return `${getConfigPath()}.refresh.lock` +} + +/** + * Best-effort proactive rotation before a request. Returns the token to use + * (the rotated one, or the current one if no rotation was needed) so the + * caller doesn't re-read the store; `undefined` when the default account + * isn't refreshable (no refresh token / env / failure) — the caller then + * falls back to `getApiToken()` and the 401 path stays authoritative. + */ +export async function proactiveRefresh(): Promise { + try { + const { bundle } = await refreshAccessToken({ + store: tokenStore(), + provider: authProvider(), + lockPath: refreshLockPath(), + }) + return bundle.accessToken + } catch { + return undefined + } +} + +/** + * Refresh the *selected* account (not the default) for `auth status`, returning + * its current-or-rotated token. Scoped via the account's id + base URL/client + * id through the refresh handshake, so `--user ` checks and rotates the + * right account at the right instance. Env / legacy / record-less accounts + * aren't refreshable — the snapshot `fallback` token is returned as-is. + */ +export async function refreshedTokenForStatus( + account: OutlineAccount, + fallback: string, +): Promise { + if (process.env[TOKEN_ENV_VAR]?.trim() || !account.id) return fallback + try { + const { bundle } = await refreshAccessToken({ + store: tokenStore(), + provider: authProvider(), + lockPath: refreshLockPath(), + ref: account.id, + handshake: { baseUrl: account.baseUrl, clientId: account.oauthClientId }, + }) + return bundle.accessToken + } catch { + return fallback + } +} + +/** + * Reactive rotation after a 401. Returns `true` when the token rotated (the + * caller retries once). A rejected/absent refresh token surfaces as + * `NoTokenError` (re-login); a transient failure propagates unchanged. + */ +export async function reactiveRefresh(): Promise { + try { + const result = await refreshAccessToken({ + store: tokenStore(), + provider: authProvider(), + lockPath: refreshLockPath(), + force: true, + }) + return result.rotated + } catch (err) { + // Match on the structural `.code` rather than `instanceof`: cli-core + // throws its own CliError, and class identity isn't reliable across + // package boundaries (duplicate module instances under linking). + const code = (err as { code?: unknown } | null)?.code + if (code === 'AUTH_REFRESH_EXPIRED' || code === 'AUTH_REFRESH_UNAVAILABLE') { + throw new NoTokenError() + } + throw err + } +} + /** * Read the active token. Hot path: when `OUTLINE_API_TOKEN` is set we * return it directly without consulting the token store, since diff --git a/src/lib/config.ts b/src/lib/config.ts index 46ce074..27fa402 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -19,7 +19,14 @@ export type StoredUser = { base_url?: string oauth_client_id?: string team_name?: string + /** Plaintext access-token fallback, present only when the keyring was unavailable at write time. */ token?: string + /** Access-token expiry (unix-epoch ms); drives proactive refresh. */ + access_token_expires_at?: number + /** Refresh-token expiry (unix-epoch ms), when the server returns one. */ + refresh_token_expires_at?: number + /** Whether a refresh token is stored (keyring slot or `refresh_token`). */ + has_refresh_token?: boolean } export type Config = CoreConfig & { diff --git a/src/lib/user-records.ts b/src/lib/user-records.ts index 4e7f7c9..82927ba 100644 --- a/src/lib/user-records.ts +++ b/src/lib/user-records.ts @@ -64,16 +64,22 @@ function toRecord(user: StoredUser): UserRecord { oauthClientId: user.oauth_client_id, teamName: user.team_name, }) - const trimmed = user.token?.trim() const record: UserRecord = { account } - if (trimmed) record.fallbackToken = trimmed + const token = user.token?.trim() + if (token) record.fallbackToken = token + if (user.access_token_expires_at !== undefined) { + record.accessTokenExpiresAt = user.access_token_expires_at + } + if (user.refresh_token_expires_at !== undefined) { + record.refreshTokenExpiresAt = user.refresh_token_expires_at + } + if (user.has_refresh_token !== undefined) record.hasRefreshToken = user.has_refresh_token return record } function fromRecord(record: UserRecord): StoredUser { - // Replace, don't merge: an absent `fallbackToken` strips the plaintext - // slot so it can't shadow a fresh keyring-backed write. cli-core contract. - const trimmed = record.fallbackToken?.trim() + // Replace, don't merge: absent fields strip the corresponding slots so a + // stale value can't shadow a fresh keyring-backed write. cli-core contract. const next: StoredUser = { id: record.account.id, name: record.account.label, @@ -81,6 +87,19 @@ function fromRecord(record: UserRecord): StoredUser { if (record.account.baseUrl) next.base_url = record.account.baseUrl if (record.account.oauthClientId) next.oauth_client_id = record.account.oauthClientId if (record.account.teamName) next.team_name = record.account.teamName - if (trimmed && trimmed.length > 0) next.token = trimmed + const token = record.fallbackToken?.trim() + if (token) next.token = token + // Deliberately NOT persisting `fallbackRefreshToken`: the refresh token is + // a long-lived credential and must stay in the secure store only. If the + // keyring is offline at write time, the refresh token isn't persisted, so + // that account fails closed (re-auth on next expiry) rather than leaving a + // long-lived secret in plaintext config. (Doist secrets-management standard.) + if (record.accessTokenExpiresAt !== undefined) { + next.access_token_expires_at = record.accessTokenExpiresAt + } + if (record.refreshTokenExpiresAt !== undefined) { + next.refresh_token_expires_at = record.refreshTokenExpiresAt + } + if (record.hasRefreshToken !== undefined) next.has_refresh_token = record.hasRefreshToken return next }