diff --git a/package-lock.json b/package-lock.json index 1bef0c0..3def946 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "hasInstallScript": true, "license": "MIT", "dependencies": { - "@doist/cli-core": "0.19.0", + "@doist/cli-core": "0.23.0", "chalk": "5.6.2", "commander": "14.0.2", "marked": "18.0.3", @@ -133,9 +133,9 @@ } }, "node_modules/@doist/cli-core": { - "version": "0.19.0", - "resolved": "https://registry.npmjs.org/@doist/cli-core/-/cli-core-0.19.0.tgz", - "integrity": "sha512-+CswqzGwcFC78v8oH7uC5poS9Ptqxpw7UKIXs3/xLIbgpGMJB+LKYT88lq8LGw2ZJ2bevsRN867lApgnWdJ4vw==", + "version": "0.23.0", + "resolved": "https://registry.npmjs.org/@doist/cli-core/-/cli-core-0.23.0.tgz", + "integrity": "sha512-SzqbFi7m5AGQYsgX2U+1zIcKAGsiORk5IJvtfalBuKVlRXT6xdVoQtL8HgsOHli+GqRtAm6xqQlxMHBJqfwAEg==", "license": "MIT", "dependencies": { "chalk": "5.6.2", diff --git a/package.json b/package.json index 3acbff1..c767f70 100644 --- a/package.json +++ b/package.json @@ -48,7 +48,7 @@ "node": ">=20.18.1" }, "dependencies": { - "@doist/cli-core": "0.19.0", + "@doist/cli-core": "0.23.0", "chalk": "5.6.2", "commander": "14.0.2", "marked": "18.0.3", diff --git a/skills/outline-cli/SKILL.md b/skills/outline-cli/SKILL.md index f8d3f7c..78b7ab2 100644 --- a/skills/outline-cli/SKILL.md +++ b/skills/outline-cli/SKILL.md @@ -23,6 +23,10 @@ All list commands support: - `--ndjson` - Newline-delimited JSON (streaming) - `--full` - Include all fields in JSON +## Global Options + +- `--user ` - Act as a specific stored account, matched by Outline user ID or display name. Each `ol auth login` stores an account (accounts can live on different Outline instances), and `--user` selects which one a command runs as; the token, base URL, and OAuth client ID all resolve from that account. Must be placed **before** the command, e.g. `ol --user scott@example.com doc list`. Omitted, commands use the default account. Overridden by `OUTLINE_API_TOKEN` when set. + ## Document References Documents can be referenced by: diff --git a/src/_fixtures/auth.ts b/src/_fixtures/auth.ts index 526758b..61ac351 100644 --- a/src/_fixtures/auth.ts +++ b/src/_fixtures/auth.ts @@ -1,4 +1,5 @@ import type { MigrateAuthResult } from '@doist/cli-core/auth' +import type { Config } from '../lib/config.js' import type { OutlineAccount } from '../lib/outline-account.js' /** Canonical persisted `OutlineAccount` used across auth tests. */ @@ -36,6 +37,32 @@ export const AUTH_INFO = { team: { name: 'Analytics', subdomain: 'analytics' }, } as const +/** + * Two v2 accounts on different Outline instances — Ada is the default, Bob the + * secondary — for exercising the `--user` selector. Tokens are plaintext + * fallbacks so the store resolves them without a live keyring. + */ +export const TWO_USER_CONFIG: Config = { + config_version: 2, + users: [ + { + id: 'id-ada', + name: 'Ada', + base_url: 'https://ada.example.com', + oauth_client_id: 'cid-ada', + token: 'tok-ada', + }, + { + id: 'id-bob', + name: 'Bob', + base_url: 'https://bob.example.com', + oauth_client_id: 'cid-bob', + token: 'tok-bob', + }, + ], + default_user_id: 'id-ada', +} + /** Stand-in for a cli-core `migrateLegacyAuth` skip result. */ export const SKIPPED_RESULT: MigrateAuthResult = { status: 'skipped', diff --git a/src/commands/auth-command.test.ts b/src/commands/auth-command.test.ts index 94d7bd6..d50cc65 100644 --- a/src/commands/auth-command.test.ts +++ b/src/commands/auth-command.test.ts @@ -1,6 +1,6 @@ import { Command } from 'commander' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { AUTH_INFO } from '../_fixtures/auth.js' +import { AUTH_INFO, TWO_USER_CONFIG } from '../_fixtures/auth.js' vi.mock('../lib/auth.js', () => ({ getApiToken: async () => 'test-token', @@ -71,6 +71,9 @@ afterEach(() => { delete process.env.OUTLINE_OAUTH_CALLBACK_PORT delete process.env.OUTLINE_API_TOKEN delete process.env.OUTLINE_URL + // Reset argv so a `--user` set by one test can't leak into the next via + // the (real) global-args parser. + process.argv = ['node', 'ol'] }) describe('registerAuthCommand', () => { @@ -176,6 +179,36 @@ describe('auth status subcommand', () => { }) }) + it('honors a global --user via the wrapped store (routes to that account instance)', async () => { + // Exercises the real ref-aware store wiring (not a fake): a global + // `--user` before the command must reach `attachStatusCommand`'s store + // and resolve the named account, not the default. Guards against a + // regression where `registerAuthCommand` passes the raw store. + const { getConfig } = await import('../lib/config.js') + vi.mocked(getConfig).mockResolvedValue(TWO_USER_CONFIG) + const { resetGlobalArgs } = await import('../lib/global-args.js') + process.argv = ['node', 'ol', '--user', 'Bob', 'auth', 'status'] + resetGlobalArgs() + + const apiRequest = await importApiMock() + apiRequest.mockResolvedValue({ data: AUTH_INFO }) + + const program = await buildProgram() + await program.parseAsync(['node', 'ol', 'auth', 'status']) + + // The probe used Bob's token + instance — proof the global --user flowed + // through the wrapped store rather than defaulting to Ada. + expect(apiRequest).toHaveBeenCalledWith( + 'auth.info', + {}, + { token: 'tok-bob', baseUrl: 'https://bob.example.com' }, + ) + + // `vi.clearAllMocks()` only clears calls, not implementations, so restore + // the config default to keep this override from leaking into later tests. + vi.mocked(getConfig).mockResolvedValue({}) + }) + it('translates a 401 from auth.info into a NO_TOKEN CliError', async () => { process.env.OUTLINE_API_TOKEN = 'expired-token' const apiRequest = await importApiMock() diff --git a/src/commands/auth.ts b/src/commands/auth.ts index 9f327a7..a56b9ad 100644 --- a/src/commands/auth.ts +++ b/src/commands/auth.ts @@ -18,6 +18,7 @@ import { } from '../lib/auth-provider.js' import { refreshedTokenForStatus } from '../lib/auth.js' import { CliError } from '../lib/errors.js' +import { withUserRefAware } from '../lib/user-ref-store.js' const DEFAULT_OAUTH_CALLBACK_PORT = 54969 @@ -64,6 +65,10 @@ export function registerAuthCommand(program: Command): void { const provider = createOutlineAuthProvider() const store: OutlineTokenStore = createOutlineTokenStore() + // Honours a global `ol --user ` placed before `auth status` / `auth + // logout`; login always targets the freshly authenticated account, so it + // keeps the raw store. + const refAware = withUserRefAware(store) attachLoginCommand(auth, { provider, @@ -103,7 +108,7 @@ export function registerAuthCommand(program: Command): void { let statusData: StatusData | null = null attachStatusCommand(auth, { - store, + store: refAware, description: 'Show current authentication state', async fetchLive({ account, token }) { try { @@ -120,7 +125,11 @@ export function registerAuthCommand(program: Command): void { {}, { token: liveToken, baseUrl: account.baseUrl }, ), - getActiveTokenSource(), + // Scope the source to the selected account so `auth status + // --user ` reports where *that* account's token lives, + // not the default/env source. Empty id (env/legacy snapshot) + // falls back to the default cascade. + getActiveTokenSource(account.id || undefined), ]) statusData = { email: info.user.email, source } return { @@ -164,7 +173,7 @@ export function registerAuthCommand(program: Command): void { }) attachLogoutCommand(auth, { - store, + store: refAware, description: 'Clear saved authentication', onCleared({ view }) { const result = store.getLastClearResult() diff --git a/src/index.ts b/src/index.ts index 03619c2..6a9b365 100644 --- a/src/index.ts +++ b/src/index.ts @@ -9,7 +9,7 @@ import { registerSearchCommand } from './commands/search.js' import { registerSkillCommand } from './commands/skill.js' import { registerUpdateCommand } from './commands/update/index.js' import { BaseCliError } from './lib/errors.js' -import { isJsonMode } from './lib/global-args.js' +import { applyUserSelector, isJsonMode } from './lib/global-args.js' import { formatError, formatErrorJson } from './lib/output.js' const program = new Command() @@ -23,6 +23,10 @@ program .addHelpText( 'after', ` +Global options: + --user Act as a specific stored account (place before the command, + e.g. \`ol --user scott@example.com document list\`). + Note for AI/LLM agents: Use --json or --ndjson flags for unambiguous, parseable output. Default JSON shows essential fields; use --full for all fields.`, @@ -36,11 +40,21 @@ registerSkillCommand(program) registerChangelogCommand(program) registerUpdateCommand(program) -program.parseAsync().catch((err: Error) => { +function reportError(err: unknown): never { if (err instanceof BaseCliError) { console.error(isJsonMode() ? formatErrorJson(err) : formatError(err)) } else { - console.error(err.message) + console.error(err instanceof Error ? err.message : String(err)) } process.exit(1) -}) +} + +// Commander has no root `--user` option, so validate it and strip it from argv +// before parsing (see `applyUserSelector` for the warm-cache-then-strip order). +try { + applyUserSelector(new Set(program.commands.map((c) => c.name()))) +} catch (err) { + reportError(err) +} + +program.parseAsync().catch(reportError) diff --git a/src/lib/api.test.ts b/src/lib/api.test.ts index dec9e81..84dd600 100644 --- a/src/lib/api.test.ts +++ b/src/lib/api.test.ts @@ -2,7 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const authMocks = vi.hoisted(() => ({ getApiToken: vi.fn(async () => 'test-token'), - getBaseUrl: vi.fn(async () => 'https://test.outline.com'), + getRequestContext: vi.fn(async () => ({ baseUrl: 'https://test.outline.com' })), proactiveRefresh: vi.fn(async () => undefined), reactiveRefresh: vi.fn(async () => false), })) @@ -17,7 +17,9 @@ describe('apiRequest', () => { beforeEach(() => { delete process.env.OUTLINE_API_TOKEN authMocks.getApiToken.mockReset().mockResolvedValue('test-token') - authMocks.getBaseUrl.mockReset().mockResolvedValue('https://test.outline.com') + authMocks.getRequestContext + .mockReset() + .mockResolvedValue({ baseUrl: 'https://test.outline.com' }) authMocks.proactiveRefresh.mockReset().mockResolvedValue(undefined) authMocks.reactiveRefresh.mockReset().mockResolvedValue(false) }) diff --git a/src/lib/api.ts b/src/lib/api.ts index 675ee50..c142ff2 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -1,8 +1,10 @@ import { fetchWithRetry } from '../transport/fetch-with-retry.js' import { TOKEN_ENV_VAR } from './auth-constants.js' -import { getApiToken, getBaseUrl, proactiveRefresh, reactiveRefresh } from './auth.js' +import { getApiToken, getRequestContext, proactiveRefresh, reactiveRefresh } from './auth.js' import { type SpinnerOptions, withSpinner } from './spinner.js' +type RefreshHandshake = { baseUrl: string; clientId: string } + /** * Spinner configuration mapping API paths to spinner options. * Blue for read operations, green for creates, yellow for updates/deletes. @@ -58,11 +60,16 @@ export type ApiRequestOverrides = { * 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. + * `handshake` pins the refresh to the `--user` account's instance. */ -async function resolveRequestToken(managed: boolean, override?: string): Promise { +async function resolveRequestToken( + managed: boolean, + override?: string, + handshake?: RefreshHandshake, +): Promise { if (override) return override if (managed) { - const refreshed = await proactiveRefresh() + const refreshed = await proactiveRefresh(handshake) if (refreshed) return refreshed } return getApiToken() @@ -80,11 +87,21 @@ async function rawApiRequest( // 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(), - resolveRequestToken(managed, overrides.token), - ]) + // A caller-supplied base URL (login validate, auth status) skips account + // resolution — those paths pass an explicit token, so no refresh runs. + // Otherwise resolve the request's base URL and, for a `--user` account, the + // refresh handshake that keeps rotation pinned to that account's instance. + let resolvedBaseUrl: string + let handshake: RefreshHandshake | undefined + if (overrides.baseUrl) { + resolvedBaseUrl = overrides.baseUrl.replace(/\/$/, '') + } else { + const ctx = await getRequestContext() + resolvedBaseUrl = ctx.baseUrl + handshake = ctx.handshake + } + + const resolvedToken = await resolveRequestToken(managed, overrides.token, handshake) const performRequest = (token: string) => fetchWithRetry({ @@ -104,7 +121,7 @@ async function rawApiRequest( // 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())) { + if (res.status === 401 && managed && (await reactiveRefresh(handshake))) { res = await performRequest(await getApiToken()) } diff --git a/src/lib/auth-provider.ts b/src/lib/auth-provider.ts index f994119..a53c9a9 100644 --- a/src/lib/auth-provider.ts +++ b/src/lib/auth-provider.ts @@ -14,10 +14,11 @@ import { LEGACY_CLEAR_PAYLOAD, SECURE_STORE_SERVICE, TOKEN_ENV_VAR } from './aut import { getBaseUrl, getOAuthClientId } from './auth.js' import { getConfig, getConfigPath, updateConfig } from './config.js' import { runMigrateLegacyAuth } from './migrate-auth.js' -import { makeOutlineAccount, type OutlineAccount } from './outline-account.js' -import { createOutlineUserRecordStore, getDefaultUserRecord } from './user-records.js' +import { makeOutlineAccount, matchOutlineAccount, type OutlineAccount } from './outline-account.js' +import { createOutlineUserRecordStore, getDefaultUserRecord, recordForRef } from './user-records.js' export type { OutlineAccount } from './outline-account.js' +export { matchOutlineAccount } from './outline-account.js' export type AuthInfoResponse = { user: { id: string; name: string; email: string } @@ -122,16 +123,6 @@ export function createOutlineAuthProvider(): AuthProvider { }) } -/** - * Accepts the Outline user UUID or display name. Id matches are - * case-sensitive (UUIDs are canonical); label matches are - * case-insensitive so users can pass the name they see in `auth status`. - */ -export function matchOutlineAccount(account: OutlineAccount, ref: AccountRef): boolean { - if (account.id === ref) return true - return account.label.toLowerCase() === ref.toLowerCase() -} - /** True when the v2 store is the authoritative source. */ function migrationIsConclusive(result: MigrateAuthResult): boolean { return ( @@ -286,6 +277,13 @@ export function createOutlineTokenStore(): OutlineTokenStore { (token, account) => ({ token, account }), ) }, + activeAccount(ref?: AccountRef) { + return resolveAuth( + ref, + () => inner.activeAccount(ref), + (_token, account) => ({ account, isDefault: true }), + ) + }, activeBundle(ref?: AccountRef) { return resolveAuth( ref, @@ -301,12 +299,13 @@ export function createOutlineTokenStore(): OutlineTokenStore { }, async clear(ref?: AccountRef) { await ensureMigrated() - await inner.clear(ref) + const cleared = await inner.clear(ref) if (await migrationIsInconclusive()) { // Must succeed: v2 is now empty, so a surviving legacy // token would shadow the logout via the fallback. await dischargeLegacyState() } + return cleared }, async list() { await ensureMigrated() @@ -326,6 +325,11 @@ export function createOutlineTokenStore(): OutlineTokenStore { * order — env → v2 record → legacy plaintext — so the answer can never * contradict the token the runtime is actually using. * + * Pass `ref` to report the source of a specific `--user` account (used by + * `auth status --user `): a ref selects the matching record and skips the + * env short-circuit, so the source reflects the account being shown rather + * than an ambient env token. Without a ref the default/env cascade applies. + * * The precedence cascade is intentionally duplicated with `active()`: * the only true dedupe options either (a) add an extra config read on * every `apiRequest` call (regressing the request hot path) or (b) @@ -333,11 +337,13 @@ export function createOutlineTokenStore(): OutlineTokenStore { * invites is guarded by the `getActiveTokenSource` regression test that * asserts v2-record presence wins over a lingering v1 `api_token`. */ -export async function getActiveTokenSource(): Promise<'env' | 'secure-store' | 'config-file'> { - if (process.env[TOKEN_ENV_VAR]?.trim()) return 'env' +export async function getActiveTokenSource( + ref?: AccountRef, +): Promise<'env' | 'secure-store' | 'config-file'> { + if (ref === undefined && process.env[TOKEN_ENV_VAR]?.trim()) return 'env' const config = await getConfig() - const record = getDefaultUserRecord(config) + const record = ref !== undefined ? recordForRef(config, ref) : getDefaultUserRecord(config) if (record) return record.fallbackToken ? 'config-file' : 'secure-store' - if (config.api_token?.trim()) return 'config-file' + if (ref === undefined && config.api_token?.trim()) return 'config-file' return 'secure-store' } diff --git a/src/lib/auth.test.ts b/src/lib/auth.test.ts index afb0777..5a9d75d 100644 --- a/src/lib/auth.test.ts +++ b/src/lib/auth.test.ts @@ -2,7 +2,7 @@ import { existsSync, mkdirSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { SKIPPED_RESULT } from '../_fixtures/auth.js' +import { SKIPPED_RESULT, TWO_USER_CONFIG } from '../_fixtures/auth.js' const TEST_XDG = join(tmpdir(), `outline-cli-test-${process.pid}-auth`) const TEST_CONFIG_DIR = join(TEST_XDG, 'outline-cli') @@ -32,6 +32,7 @@ describe('auth', () => { rmSync(TEST_XDG, { recursive: true }) } delete process.env.XDG_CONFIG_HOME + process.argv = ['node', 'ol'] }) it('getApiToken reads from env var first', async () => { @@ -124,6 +125,61 @@ describe('auth', () => { await expect(getOAuthClientId()).resolves.toBe('cid-record') }) + describe('global --user selection', () => { + const TWO_USERS = JSON.stringify(TWO_USER_CONFIG) + + function withUser(ref: string) { + writeFileSync(TEST_CONFIG_PATH, TWO_USERS) + process.argv = ['node', 'ol', '--user', ref, 'document', 'list'] + } + + it('getRequestContext resolves the --user account instance + handshake', async () => { + withUser('Bob') + const { getRequestContext } = await import('./auth.js') + await expect(getRequestContext()).resolves.toEqual({ + baseUrl: 'https://bob.example.com', + handshake: { baseUrl: 'https://bob.example.com', clientId: 'cid-bob' }, + }) + }) + + it('getRequestContext falls back to the default account without --user', async () => { + writeFileSync(TEST_CONFIG_PATH, TWO_USERS) + const { getRequestContext } = await import('./auth.js') + await expect(getRequestContext()).resolves.toEqual({ + baseUrl: 'https://ada.example.com', + }) + }) + + it('getBaseUrl / getOAuthClientId stay account-agnostic under --user (login unaffected)', async () => { + withUser('Bob') + const { getBaseUrl, getOAuthClientId } = await import('./auth.js') + await expect(getBaseUrl()).resolves.toBe('https://ada.example.com') + await expect(getOAuthClientId()).resolves.toBe('cid-ada') + }) + + it('getApiToken returns the --user account token', async () => { + withUser('Bob') + const { getApiToken } = await import('./auth.js') + await expect(getApiToken()).resolves.toBe('tok-bob') + }) + + it('getApiToken rejects with ACCOUNT_NOT_FOUND for an unknown --user', async () => { + withUser('Nobody') + const { getApiToken } = await import('./auth.js') + await expect(getApiToken()).rejects.toMatchObject({ code: 'ACCOUNT_NOT_FOUND' }) + }) + + it('env token wins: --user is ignored on the request path', async () => { + withUser('Bob') + process.env.OUTLINE_API_TOKEN = 'env-token' + const { getApiToken, getRequestContext } = await import('./auth.js') + await expect(getApiToken()).resolves.toBe('env-token') + await expect(getRequestContext()).resolves.toEqual({ + baseUrl: 'https://ada.example.com', + }) + }) + }) + 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 diff --git a/src/lib/auth.ts b/src/lib/auth.ts index e5d51bb..c92d488 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -8,8 +8,16 @@ import { } from './auth-provider.js' import { getConfig, getConfigPath } from './config.js' import { CliError } from './errors.js' +import { getRequestedUserRef } from './global-args.js' import { DEFAULT_BASE_URL, type OutlineAccount } from './outline-account.js' -import { getDefaultUserRecord } from './user-records.js' +import { getDefaultUserRecord, recordForRef } from './user-records.js' +import { withUserRefAware } from './user-ref-store.js' + +/** Base URL + optional refresh handshake for a single request. */ +type RequestContext = { + baseUrl: string + handshake?: { baseUrl: string; clientId: string } +} export { SecureStoreUnavailableError, getActiveTokenSource, TOKEN_ENV_VAR } @@ -32,10 +40,24 @@ export class NoTokenError extends CliError { */ let storeSingleton: OutlineTokenStore | undefined function tokenStore(): OutlineTokenStore { - if (!storeSingleton) storeSingleton = createOutlineTokenStore() + // Wrapped so a global `--user ` flows into every no-ref store read on + // the request hot path (token lookup + refresh). Without `--user`, + // `getRequestedUserRef()` is undefined and the wrap is a pass-through. + if (!storeSingleton) storeSingleton = withUserRefAware(createOutlineTokenStore()) return storeSingleton } +/** + * The `--user` ref to apply on the request path, or `undefined` when an + * `OUTLINE_API_TOKEN` is set — an env token wins outright, so base URL and + * client id resolve from the default account rather than the named one, + * keeping token / base URL / client id pinned to the same source. + */ +function requestPathUserRef(): string | undefined { + if (process.env[TOKEN_ENV_VAR]?.trim()) return undefined + return getRequestedUserRef() +} + let providerSingleton: ReturnType | undefined function authProvider(): ReturnType { if (!providerSingleton) providerSingleton = createOutlineAuthProvider() @@ -52,16 +74,21 @@ function refreshLockPath(): string { /** * 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. + * caller doesn't re-read the store; `undefined` when the account isn't + * refreshable (no refresh token / env / failure) — the caller then falls back + * to `getApiToken()` and the 401 path stays authoritative. The wrapped store + * scopes the bundle to any global `--user`; `handshake` pins the token + * endpoint to that account's instance + client id. */ -export async function proactiveRefresh(): Promise { +export async function proactiveRefresh( + handshake?: RequestContext['handshake'], +): Promise { try { const { bundle } = await refreshAccessToken({ store: tokenStore(), provider: authProvider(), lockPath: refreshLockPath(), + handshake, }) return bundle.accessToken } catch { @@ -100,13 +127,14 @@ export async function refreshedTokenForStatus( * caller retries once). A rejected/absent refresh token surfaces as * `NoTokenError` (re-login); a transient failure propagates unchanged. */ -export async function reactiveRefresh(): Promise { +export async function reactiveRefresh(handshake?: RequestContext['handshake']): Promise { try { const result = await refreshAccessToken({ store: tokenStore(), provider: authProvider(), lockPath: refreshLockPath(), force: true, + handshake, }) return result.rotated } catch (err) { @@ -137,10 +165,11 @@ export async function getApiToken(): Promise { } /** - * Base URL cascade: env var → default user record (v2) → legacy - * `base_url` config (v1) → built-in default. The record takes priority - * over the legacy slot so post-migration logins keep defaulting to the - * same Outline instance. + * Account-agnostic base URL cascade: env var → default user record (v2) → + * legacy `base_url` config (v1) → built-in default. Deliberately ignores the + * global `--user` selector so the login flow (which resolves its default + * prompt through here) isn't skewed by account selection; the request path + * applies `--user` separately via {@link getRequestContext}. */ export async function getBaseUrl(): Promise { const envUrl = process.env.OUTLINE_URL @@ -154,8 +183,9 @@ export async function getBaseUrl(): Promise { } /** - * OAuth client id cascade: env var → default user record (v2) → legacy - * `oauth_client_id` config (v1) → undefined (caller prompts). + * Account-agnostic OAuth client id cascade: env var → default user record + * (v2) → legacy `oauth_client_id` config (v1) → undefined (caller prompts). + * Like {@link getBaseUrl}, ignores `--user` so login stays default-driven. */ export async function getOAuthClientId(): Promise { const envClientId = process.env.OUTLINE_OAUTH_CLIENT_ID @@ -166,3 +196,23 @@ export async function getOAuthClientId(): Promise { if (record?.account.oauthClientId) return record.account.oauthClientId return config.oauth_client_id } + +/** + * Resolve the base URL — and, when a `--user` account is selected, the refresh + * handshake (its instance + client id) — for a single managed request. Falls + * back to the account-agnostic {@link getBaseUrl} cascade without `--user`. + * An unknown ref resolves no handshake and the default base URL; the wrapped + * token store then raises `ACCOUNT_NOT_FOUND` when the token is read, so the + * request never fires. + */ +export async function getRequestContext(): Promise { + const ref = requestPathUserRef() + if (ref !== undefined) { + const record = recordForRef(await getConfig(), ref) + if (record) { + const baseUrl = record.account.baseUrl.replace(/\/$/, '') + return { baseUrl, handshake: { baseUrl, clientId: record.account.oauthClientId } } + } + } + return { baseUrl: await getBaseUrl() } +} diff --git a/src/lib/errors.ts b/src/lib/errors.ts index e9c21f1..facebbc 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -9,6 +9,7 @@ export type ErrorCode = | 'CONFIRMATION_REQUIRED' | 'CONFLICTING_OPTIONS' | 'INVALID_PARENT' + | 'INVALID_USER_FLAG' | 'MISSING_OPTION' | 'NO_TOKEN' | 'OAUTH_CALLBACK_PORT_INVALID' diff --git a/src/lib/global-args.test.ts b/src/lib/global-args.test.ts new file mode 100644 index 0000000..d26e274 --- /dev/null +++ b/src/lib/global-args.test.ts @@ -0,0 +1,97 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { BaseCliError } from './errors.js' +import { + applyUserSelector, + getRequestedUserRef, + resetGlobalArgs, + validateRootUserFlag, +} from './global-args.js' + +const COMMANDS = new Set([ + 'auth', + 'search', + 'document', + 'collection', + 'skill', + 'changelog', + 'update', +]) + +// The global-args store reads `process.argv` lazily and caches the result, so +// each case sets argv then clears the cache. (Static import is fine: this file +// doesn't use `vi.resetModules()`, so a dynamic import would return the same +// module instance anyway.) +function setArgv(...args: string[]) { + process.argv = ['node', 'ol', ...args] + resetGlobalArgs() +} + +beforeEach(() => setArgv()) +afterEach(() => setArgv()) + +describe('getRequestedUserRef', () => { + it('resolves the ref off argv', () => { + setArgv('--user', 'scott', 'document', 'list') + expect(getRequestedUserRef()).toBe('scott') + }) + // Absent-flag case is covered by the applyUserSelector no-op test below; + // flag-form parsing (`--user=`) belongs to cli-core's parseGlobalArgs. +}) + +describe('validateRootUserFlag', () => { + it('passes a valid ref', () => { + expect(() => + validateRootUserFlag(['--user', 'scott', 'document', 'list'], COMMANDS), + ).not.toThrow() + }) + + it('is silent when --user is absent', () => { + expect(() => validateRootUserFlag(['document', 'list'], COMMANDS)).not.toThrow() + }) + + it('ignores a --user that appears after a command (left for Commander)', () => { + // Late flag: stripUserFlag won't remove it, so Commander reports the + // unknown option — the validator must not pre-empt with its own error. + expect(() => validateRootUserFlag(['document', '--user'], COMMANDS)).not.toThrow() + expect(() => + validateRootUserFlag(['document', 'list', '--user', 'scott'], COMMANDS), + ).not.toThrow() + }) + + it('errors on a bare --user', () => { + expect(() => validateRootUserFlag(['--user'], COMMANDS)).toThrow(BaseCliError) + }) + + it('errors on an empty --user=', () => { + expect(() => validateRootUserFlag(['--user=', 'document'], COMMANDS)).toThrow(BaseCliError) + }) + + it('errors when the value is a command name (forgotten value)', () => { + expect(() => validateRootUserFlag(['--user', 'document', 'list'], COMMANDS)).toThrow( + expect.objectContaining({ code: 'INVALID_USER_FLAG' }), + ) + }) +}) + +describe('applyUserSelector (entrypoint flow)', () => { + it('validates, warms the cache, then strips --user from argv', () => { + setArgv('--user', 'scott', 'document', 'list') + applyUserSelector(COMMANDS) + // Ref survives because the cache was warmed before the strip... + expect(getRequestedUserRef()).toBe('scott') + // ...and Commander sees argv without the root --user. + expect(process.argv.slice(2)).toEqual(['document', 'list']) + }) + + it('throws before stripping when the root flag is malformed', () => { + setArgv('--user') + expect(() => applyUserSelector(COMMANDS)).toThrow(BaseCliError) + }) + + it('is a no-op for argv without --user', () => { + setArgv('document', 'list') + applyUserSelector(COMMANDS) + expect(getRequestedUserRef()).toBeUndefined() + expect(process.argv.slice(2)).toEqual(['document', 'list']) + }) +}) diff --git a/src/lib/global-args.ts b/src/lib/global-args.ts index 66b61ff..98ca474 100644 --- a/src/lib/global-args.ts +++ b/src/lib/global-args.ts @@ -1,4 +1,11 @@ -import { createAccessibleGate, createGlobalArgsStore, createSpinnerGate } from '@doist/cli-core' +import { + createAccessibleGate, + createGlobalArgsStore, + createSpinnerGate, + parseGlobalArgs, + stripUserFlag, +} from '@doist/cli-core' +import { CliError } from './errors.js' const store = createGlobalArgsStore() @@ -19,3 +26,57 @@ export function isJsonMode(): boolean { const args = store.get() return args.json || args.ndjson } + +/** + * Pre-subcommand `ol --user ` selector. cli-core's `parseGlobalArgs` + * already extracts it from argv; `applyUserSelector` strips the flag before + * Commander parses, so this reads the value off the warmed cache. + */ +export function getRequestedUserRef(): string | undefined { + return store.get().user +} + +/** + * Guard a root `--user` against the two common footguns before it's stripped: + * a value-less flag (`--user`, `--user=`) and a forgotten value where the next + * token is actually a command (`ol --user document list`). Pure — pass the + * pre-strip argv and the set of registered command names. Silent when `--user` + * is absent, or when it appears *after* a command (a late flag is left for + * Commander to reject as an unknown option, matching what `stripUserFlag` + * leaves in argv). + */ +export function validateRootUserFlag(argv: string[], knownCommands: ReadonlySet): void { + const userIdx = argv.findIndex((a) => a === '--user' || a.startsWith('--user=')) + if (userIdx === -1) return + const firstCmdIdx = argv.findIndex((a) => knownCommands.has(a)) + if (firstCmdIdx !== -1 && userIdx > firstCmdIdx) return + + const ref = parseGlobalArgs(argv).user + if (!ref) { + throw new CliError('INVALID_USER_FLAG', '--user requires a value: .', [ + 'Example: ol --user scott@example.com document list', + ]) + } + if (knownCommands.has(ref)) { + throw new CliError( + 'INVALID_USER_FLAG', + `--user requires a value: got "${ref}", which looks like a command — did you forget the value?`, + [`Example: ol --user scott@example.com ${ref}`], + ) + } +} + +/** + * Entrypoint wiring for the global `--user` selector, kept here so the + * validate → warm-cache → strip order is exercised by tests rather than living + * untested in `src/index.ts`. Validates the root flag, warms the global-args + * cache off the *original* argv (so the ref survives), then rewrites + * `process.argv` with the flag stripped for Commander. Throws `INVALID_USER_FLAG` + * on a malformed root flag. + */ +export function applyUserSelector(knownCommands: ReadonlySet): void { + const original = process.argv.slice(2) + validateRootUserFlag(original, knownCommands) + getRequestedUserRef() + process.argv = [process.argv[0], process.argv[1], ...stripUserFlag(original)] +} diff --git a/src/lib/outline-account.ts b/src/lib/outline-account.ts index 6e46cc9..eca5bc6 100644 --- a/src/lib/outline-account.ts +++ b/src/lib/outline-account.ts @@ -1,4 +1,4 @@ -import type { AuthAccount } from '@doist/cli-core/auth' +import type { AccountRef, AuthAccount } from '@doist/cli-core/auth' /** * Narrow account shape persisted by the keyring-backed token store. @@ -35,3 +35,13 @@ export function makeOutlineAccount(input: { teamName: input.teamName, } } + +/** + * Accepts the Outline user UUID or display name. Id matches are + * case-sensitive (UUIDs are canonical); label matches are case-insensitive + * so users can pass the name they see in `auth status`. + */ +export function matchOutlineAccount(account: OutlineAccount, ref: AccountRef): boolean { + if (account.id === ref) return true + return account.label.toLowerCase() === ref.toLowerCase() +} diff --git a/src/lib/skills/content.ts b/src/lib/skills/content.ts index 7d3aa2d..f4992a2 100644 --- a/src/lib/skills/content.ts +++ b/src/lib/skills/content.ts @@ -22,6 +22,10 @@ All list commands support: - \`--ndjson\` - Newline-delimited JSON (streaming) - \`--full\` - Include all fields in JSON +## Global Options + +- \`--user \` - Act as a specific stored account, matched by Outline user ID or display name. Each \`ol auth login\` stores an account (accounts can live on different Outline instances), and \`--user\` selects which one a command runs as; the token, base URL, and OAuth client ID all resolve from that account. Must be placed **before** the command, e.g. \`ol --user scott@example.com doc list\`. Omitted, commands use the default account. Overridden by \`OUTLINE_API_TOKEN\` when set. + ## Document References Documents can be referenced by: diff --git a/src/lib/user-records.ts b/src/lib/user-records.ts index 82927ba..30d0e43 100644 --- a/src/lib/user-records.ts +++ b/src/lib/user-records.ts @@ -1,6 +1,6 @@ -import type { UserRecord, UserRecordStore } from '@doist/cli-core/auth' +import type { AccountRef, UserRecord, UserRecordStore } from '@doist/cli-core/auth' import { type Config, getConfig, type StoredUser, updateConfig } from './config.js' -import { makeOutlineAccount, type OutlineAccount } from './outline-account.js' +import { makeOutlineAccount, matchOutlineAccount, type OutlineAccount } from './outline-account.js' /** * `UserRecordStore` adapter over `config.users[]`. `StoredUser.token` is @@ -56,6 +56,24 @@ export function getDefaultUserRecord(config: Partial): UserRecord, + ref?: AccountRef, +): UserRecord | null { + if (ref === undefined) return getDefaultUserRecord(config) + for (const user of config.users ?? []) { + const record = toRecord(user) + if (matchOutlineAccount(record.account, ref)) return record + } + return null +} + function toRecord(user: StoredUser): UserRecord { const account = makeOutlineAccount({ id: user.id, diff --git a/src/lib/user-ref-store.test.ts b/src/lib/user-ref-store.test.ts new file mode 100644 index 0000000..621978b --- /dev/null +++ b/src/lib/user-ref-store.test.ts @@ -0,0 +1,105 @@ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import type { OutlineTokenStore } from './auth-provider.js' +import { BaseCliError } from './errors.js' +import { resetGlobalArgs } from './global-args.js' +import { makeOutlineAccount, type OutlineAccount } from './outline-account.js' +import { withUserRefAware } from './user-ref-store.js' + +const ADA = makeOutlineAccount({ id: 'id-ada', label: 'Ada' }) +const BOB = makeOutlineAccount({ id: 'id-bob', label: 'Bob' }) + +/** + * Minimal store double. `activeAccount` resolves a match (the wrap's existence + * probe); the other methods record the ref they were ultimately called with so + * tests can assert the substituted/forwarded value. + */ +function fakeStore(accounts: OutlineAccount[]) { + const seen: Record = {} + const match = (ref?: string) => + ref === undefined + ? accounts[0] + : accounts.find((a) => a.id === ref || a.label.toLowerCase() === ref.toLowerCase()) + const store = { + activeAccount: async (ref?: string) => { + seen.activeAccount = ref + const account = match(ref) + return account ? { account, isDefault: account === accounts[0] } : null + }, + active: async (ref?: string) => { + seen.active = ref + const account = match(ref) + return account ? { token: `tok:${ref ?? 'default'}`, account } : null + }, + activeBundle: async (ref?: string) => { + seen.activeBundle = ref + const account = match(ref) + return account ? { account, bundle: { accessToken: `tok:${ref ?? 'default'}` } } : null + }, + clear: async (ref?: string) => { + seen.clear = ref + const account = match(ref) + return account ? { account, wasDefault: account === accounts[0] } : null + }, + } + return { store: store as unknown as OutlineTokenStore, seen } +} + +function setUserFlag(ref?: string) { + process.argv = ref + ? ['node', 'ol', '--user', ref, 'auth', 'status'] + : ['node', 'ol', 'auth', 'status'] + resetGlobalArgs() +} + +describe('withUserRefAware', () => { + beforeEach(() => setUserFlag(undefined)) + afterEach(() => { + process.argv = ['node', 'ol'] + resetGlobalArgs() + }) + + it('passes undefined through when no --user is set', async () => { + const { store, seen } = fakeStore([ADA, BOB]) + await withUserRefAware(store).active() + expect(seen.active).toBeUndefined() + // No existence probe when there's nothing to resolve. + expect(seen.activeAccount).toBeUndefined() + }) + + it('substitutes the global --user ref on a no-arg call', async () => { + setUserFlag('Bob') + const { store, seen } = fakeStore([ADA, BOB]) + const snapshot = await withUserRefAware(store).active() + expect(seen.active).toBe('Bob') + expect(snapshot?.token).toBe('tok:Bob') + }) + + it('lets an explicit ref win over the global --user', async () => { + setUserFlag('Bob') + const { store, seen } = fakeStore([ADA, BOB]) + await withUserRefAware(store).active('id-ada') + expect(seen.active).toBe('id-ada') + }) + + it('throws a typed ACCOUNT_NOT_FOUND when the global ref matches nothing', async () => { + setUserFlag('nobody') + const { store } = fakeStore([ADA, BOB]) + const caught = await withUserRefAware(store) + .active() + .catch((e) => e) + expect(caught).toBeInstanceOf(BaseCliError) + expect(caught).toMatchObject({ code: 'ACCOUNT_NOT_FOUND' }) + }) + + it('applies the ref to activeBundle, activeAccount, and clear too', async () => { + setUserFlag('Ada') + const { store, seen } = fakeStore([ADA, BOB]) + const wrapped = withUserRefAware(store) + await wrapped.activeBundle() + await wrapped.activeAccount() + await wrapped.clear() + expect(seen.activeBundle).toBe('Ada') + expect(seen.activeAccount).toBe('Ada') + expect(seen.clear).toBe('Ada') + }) +}) diff --git a/src/lib/user-ref-store.ts b/src/lib/user-ref-store.ts new file mode 100644 index 0000000..dda1912 --- /dev/null +++ b/src/lib/user-ref-store.ts @@ -0,0 +1,39 @@ +import type { AccountRef } from '@doist/cli-core/auth' +import type { OutlineTokenStore } from './auth-provider.js' +import { CliError } from './errors.js' +import { getRequestedUserRef } from './global-args.js' + +/** + * Bridge the global `ol --user ` selector — stripped from argv before + * Commander parses (see `applyUserSelector`) — into store reads that make no + * explicit ref. An explicit `ref` argument always wins; only a missing one + * falls back to the global selector. Used by both the request path + * (`auth.ts` `tokenStore()`) and cli-core's auth attachers (`auth status` / + * `auth logout`), so a `--user` placed before any command is honoured + * everywhere. + * + * Existence is checked via `activeAccount(ref)` rather than `list()` so the + * check is both token-free (a keyring-offline account can still be cleared by + * `auth logout`) and legacy-aware (`list()` only exposes v2 records, but a + * pending v1→v2 migration can still resolve its single legacy account). + */ +export function withUserRefAware(store: OutlineTokenStore): OutlineTokenStore { + async function resolveRef(ref?: AccountRef): Promise { + const target = ref ?? getRequestedUserRef() + if (target === undefined) return undefined + if (!(await store.activeAccount(target))) { + throw new CliError('ACCOUNT_NOT_FOUND', `No stored account matches "${target}".`, [ + 'Check the account id or display name, or run `ol auth login` to add it.', + ]) + } + return target + } + + return { + ...store, + active: async (ref?: AccountRef) => store.active(await resolveRef(ref)), + activeBundle: async (ref?: AccountRef) => store.activeBundle(await resolveRef(ref)), + activeAccount: async (ref?: AccountRef) => store.activeAccount(await resolveRef(ref)), + clear: async (ref?: AccountRef) => store.clear(await resolveRef(ref)), + } +}