Skip to content
Merged
50 changes: 33 additions & 17 deletions README.md

Large diffs are not rendered by default.

15 changes: 15 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
"lefthook": "2.1.6",
"marked": "18.0.3",
"marked-terminal-renderer": "2.2.0",
"oauth4webapi": "3.8.6",
"open": "11.0.0",
"oxfmt": "0.49.0",
"oxlint": "1.64.0",
Expand All @@ -90,6 +91,7 @@
"commander": ">=14",
"marked": ">=18",
"marked-terminal-renderer": ">=2",
"oauth4webapi": ">=3",
"open": ">=10",
"vitest": ">=4.1"
},
Expand All @@ -103,6 +105,9 @@
"marked-terminal-renderer": {
"optional": true
},
"oauth4webapi": {
"optional": true
},
"open": {
"optional": true
},
Expand Down
3 changes: 3 additions & 0 deletions src/auth/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ export type AuthErrorCode =
| 'AUTH_TOKEN_EXCHANGE_FAILED'
| 'AUTH_STORE_WRITE_FAILED'
| 'AUTH_STORE_READ_FAILED'
| 'AUTH_REFRESH_EXPIRED'
| 'AUTH_REFRESH_TRANSIENT'
| 'AUTH_REFRESH_UNAVAILABLE'
| 'NOT_AUTHENTICATED'
| 'TOKEN_FROM_ENV'
| 'NO_ACCOUNT_SELECTED'
Expand Down
83 changes: 82 additions & 1 deletion src/auth/flow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { execFile } from 'node:child_process'
import { readFileSync } from 'node:fs'
import openBrowserModule from 'open'
import { type RunOAuthFlowOptions, runOAuthFlow } from './flow.js'
import type { AuthProvider, TokenStore } from './types.js'
import type { AuthProvider, TokenBundle, TokenStore } from './types.js'

type Account = { id: string; label?: string; email: string }

Expand Down Expand Up @@ -360,6 +360,87 @@ describe('runOAuthFlow', () => {
expect(result.token).toBe('tok-1')
})

it('persists the full bundle via setBundle when the store implements it (promoteDefault: true)', async () => {
const setBundle = vi.fn(
async (
_account: Account,
_bundle: TokenBundle,
_options?: { promoteDefault?: boolean },
) => undefined,
)
const set = vi.fn(async () => undefined)
const store: TokenStore<Account> = {
async active() {
return null
},
set,
setBundle,
async clear() {},
async list() {
return []
},
async setDefault() {},
}
const { provider, getRedirect } = instrument({
exchangeCode: async () => ({
accessToken: 'tok-1',
refreshToken: 'r-1',
expiresAt: 1_700_000_000_000,
refreshTokenExpiresAt: 1_800_000_000_000,
}),
})

await runOAuthFlow<Account>(
flowOptions({
provider,
store,
openBrowser: driveCallback(getRedirect),
onAuthorizeUrl: () => undefined,
}),
)

expect(set).not.toHaveBeenCalled()
expect(setBundle).toHaveBeenCalledTimes(1)
const [, persistedBundle, opts] = setBundle.mock.calls[0]
expect(persistedBundle).toEqual({
accessToken: 'tok-1',
refreshToken: 'r-1',
accessTokenExpiresAt: 1_700_000_000_000,
refreshTokenExpiresAt: 1_800_000_000_000,
})
expect(opts).toEqual({ promoteDefault: true })
})

it('falls back to set(accessToken) when the store does not implement setBundle (refresh state dropped)', async () => {
const set = vi.fn(async () => undefined)
const store: TokenStore<Account> = {
async active() {
return null
},
set,
async clear() {},
async list() {
return []
},
async setDefault() {},
}
const { provider, getRedirect } = instrument({
exchangeCode: async () => ({ accessToken: 'tok-1', refreshToken: 'r-1' }),
})

const result = await runOAuthFlow<Account>(
flowOptions({
provider,
store,
openBrowser: driveCallback(getRedirect),
onAuthorizeUrl: () => undefined,
}),
)

expect(result.token).toBe('tok-1')
expect(set).toHaveBeenCalledWith(expect.objectContaining({ id: '1' }), 'tok-1')
})

it('wraps non-CliError store.set failures in AUTH_STORE_WRITE_FAILED', async () => {
const { provider, getRedirect } = instrument()
const store: TokenStore<Account> = {
Expand Down
16 changes: 7 additions & 9 deletions src/auth/flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { type IncomingMessage, type Server, type ServerResponse, createServer }
import { promisify } from 'node:util'
import { CliError, getErrorMessage } from '../errors.js'
import { isStdoutTTY } from '../terminal.js'
import { bundleFromExchange, persistBundle } from './persist.js'
import { generateState } from './pkce.js'
import type { AuthAccount, AuthProvider, TokenStore } from './types.js'

Expand Down Expand Up @@ -187,15 +188,12 @@ export async function runOAuthFlow<TAccount extends AuthAccount>(
}))
checkAborted()

try {
await options.store.set(account, exchange.accessToken)
} catch (error) {
if (error instanceof CliError) throw error
throw new CliError(
'AUTH_STORE_WRITE_FAILED',
`Failed to persist token: ${getErrorMessage(error)}`,
)
}
await persistBundle({
store: options.store,
account,
bundle: bundleFromExchange(exchange),
promoteDefault: true,
})

return { token: exchange.accessToken, account }
} finally {
Expand Down
5 changes: 4 additions & 1 deletion src/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@ export {
generateVerifier,
} from './pkce.js'
export type { GenerateVerifierOptions } from './pkce.js'
export { persistBundle } from './persist.js'
export { bundleFromExchange, persistBundle } from './persist.js'
export type { PersistBundleOptions } from './persist.js'
export { createPkceProvider } from './providers/pkce.js'
export type { PkceLazyString, PkceProviderOptions } from './providers/pkce.js'
export { refreshAccessToken } from './refresh.js'
export type { RefreshAccessTokenOptions, RefreshAccessTokenResult } from './refresh.js'
export type {
AccountRef,
ActiveBundleSnapshot,
AuthAccount,
AuthorizeInput,
AuthorizeResult,
Expand Down
48 changes: 48 additions & 0 deletions src/auth/keyring/internal.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
import { describe, expect, it, vi } from 'vitest'

import type { AuthAccount } from '../types.js'
import { readRefreshTokenForRecord } from './internal.js'
import type { SecureStore } from './secure-store.js'
import type { UserRecord } from './types.js'

type Account = AuthAccount & { id: string }
const account: Account = { id: '42' }

function fakeStore(impl: Partial<SecureStore>): SecureStore {
return {
async getSecret() {
return null
},
async setSecret() {},
async deleteSecret() {
return false
},
...impl,
}
}

describe('readRefreshTokenForRecord', () => {
it('short-circuits to not-present when hasRefreshToken is false', async () => {
const getSpy = vi.fn(async () => 'should-not-be-read')
const store = fakeStore({ getSecret: getSpy })
const record: UserRecord<Account> = { account, hasRefreshToken: false }

const outcome = await readRefreshTokenForRecord(record, store)
expect(outcome).toEqual({ ok: false, reason: 'not-present' })
expect(getSpy).not.toHaveBeenCalled()
})

it('returns fallbackRefreshToken in preference to a (possibly stale) keyring slot', async () => {
const getSpy = vi.fn(async () => 'stale')
const store = fakeStore({ getSecret: getSpy })
const record: UserRecord<Account> = {
account,
hasRefreshToken: true,
fallbackRefreshToken: 'plaintext_fallback',
}

const outcome = await readRefreshTokenForRecord(record, store)
expect(outcome).toEqual({ ok: true, token: 'plaintext_fallback' })
expect(getSpy).not.toHaveBeenCalled()
})
})
50 changes: 43 additions & 7 deletions src/auth/keyring/internal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,18 @@ export type ReadAccessTokenOutcome =
| { ok: true; token: string }
| { ok: false; reason: 'slot-empty' | 'slot-unavailable' | 'slot-error'; detail: string }

/**
* Outcome of resolving the refresh token for a record. Mirrors
* `ReadAccessTokenOutcome`, plus an extra `not-present` variant for records
* the store knows carry no refresh state (`hasRefreshToken: false`) — the
* gate lets `activeBundle` skip the slot IPC entirely on access-only
* records.
*/
export type ReadRefreshTokenOutcome =
| { ok: true; token: string }
| { ok: false; reason: 'not-present' }
| { ok: false; reason: 'slot-empty' | 'slot-unavailable' | 'slot-error'; detail: string }

/**
* `fallbackToken` first (so an offline-keyring write is preferred over a
* stale slot), then the keyring slot. Single-source for "is this record
Expand All @@ -25,20 +37,44 @@ export async function readAccessTokenForRecord<TAccount extends AuthAccount>(
): Promise<ReadAccessTokenOutcome> {
const fallback = record.fallbackToken?.trim()
if (fallback) return { ok: true, token: fallback }
return readSecretSlot(secureStore, 'keyring slot returned no credential')
}

/**
* Read + trim a keyring slot, normalising the empty / unavailable / error
* cases. The per-record concerns (fallback field, the refresh `not-present`
* gate, the empty-slot detail string) stay in the callers.
*/
async function readSecretSlot(
store: SecureStore,
emptyDetail: string,
): Promise<ReadAccessTokenOutcome> {
try {
const raw = await secureStore.getSecret()
const trimmed = raw?.trim()
const trimmed = (await store.getSecret())?.trim()
if (trimmed) return { ok: true, token: trimmed }
return {
ok: false,
reason: 'slot-empty',
detail: 'keyring slot returned no credential',
}
return { ok: false, reason: 'slot-empty', detail: emptyDetail }
} catch (error) {
if (error instanceof SecureStoreUnavailableError) {
return { ok: false, reason: 'slot-unavailable', detail: error.message }
}
return { ok: false, reason: 'slot-error', detail: getErrorMessage(error) }
}
}

/**
* Refresh-side analogue of `readAccessTokenForRecord`. Honours the
* `hasRefreshToken: false` gate — a record that knows it has no refresh
* material short-circuits to `not-present` without touching the keyring.
* Legacy records (`hasRefreshToken === undefined`) probe the slot once.
*/
export async function readRefreshTokenForRecord<TAccount extends AuthAccount>(
record: UserRecord<TAccount>,
refreshStore: SecureStore,
): Promise<ReadRefreshTokenOutcome> {
if (record.hasRefreshToken === false) return { ok: false, reason: 'not-present' }

const fallback = record.fallbackRefreshToken?.trim()
if (fallback) return { ok: true, token: fallback }

return readSecretSlot(refreshStore, 'keyring refresh slot returned no credential')
}
Loading
Loading