diff --git a/README.md b/README.md index 3a9c7d8..00c3769 100644 --- a/README.md +++ b/README.md @@ -111,7 +111,7 @@ The base URL is used for OAuth, the SDK, and search. OAuth login supports `comms ```bash tdc auth status # check if authenticated tdc auth logout # remove saved token -tdc auth token view # print the stored access token +tdc auth token view # print the stored access token (refreshes an expiring OAuth token first) tdc auth refresh-token view # print the stored OAuth refresh token ``` diff --git a/package-lock.json b/package-lock.json index 05fdf0a..f7e4e17 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,7 +10,7 @@ "hasInstallScript": true, "license": "MIT", "dependencies": { - "@doist/cli-core": "1.4.0", + "@doist/cli-core": "1.6.0", "@doist/comms-sdk": "3.0.0", "@pnpm/tabtab": "0.5.4", "chalk": "5.6.2", @@ -139,9 +139,9 @@ } }, "node_modules/@doist/cli-core": { - "version": "1.4.0", - "resolved": "https://registry.npmjs.org/@doist/cli-core/-/cli-core-1.4.0.tgz", - "integrity": "sha512-8noRkEABCsqYFVRl+VviOSipX7O3iORy+OHTMEBtfSVuV3SAInFl8qC96KDl/VHurwoKar1mRLRu4DZ56pwmdw==", + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/@doist/cli-core/-/cli-core-1.6.0.tgz", + "integrity": "sha512-5MF1k08gB0JyY44N93LAqk0vMLXB4PBwKcuvCZ+WhQ60MWqbje7kq17Hbq9IyMWnHEConTmDNL5wY/I1U+cYKw==", "license": "MIT", "dependencies": { "chalk": "5.6.2", @@ -1766,6 +1766,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1782,6 +1783,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1798,6 +1800,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1814,6 +1817,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1830,6 +1834,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1846,6 +1851,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1862,9 +1868,7 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1881,9 +1885,7 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1900,9 +1902,7 @@ "cpu": [ "ppc64" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1919,9 +1919,7 @@ "cpu": [ "s390x" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1938,9 +1936,7 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1957,9 +1953,7 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1976,6 +1970,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -1992,6 +1987,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2008,6 +2004,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MIT", "optional": true, "os": [ @@ -2641,7 +2638,7 @@ "version": "25.8.0", "resolved": "https://registry.npmjs.org/@types/node/-/node-25.8.0.tgz", "integrity": "sha512-TCFSk8IZh+iLX1xtksoBVtdmgL+1IX0fC9BeU4QqFSuNdN/K+HUlhqOzEmSYYpZUVsLYcPqc9KX+60iDuninSQ==", - "devOptional": true, + "dev": true, "license": "MIT", "dependencies": { "undici-types": ">=7.24.0 <7.24.7" @@ -4257,6 +4254,7 @@ "version": "2.3.3", "resolved": "https://registry.npmjs.org/fsevents/-/fsevents-2.3.3.tgz", "integrity": "sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw==", + "dev": true, "hasInstallScript": true, "license": "MIT", "optional": true, @@ -5153,6 +5151,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -5173,6 +5172,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -5193,6 +5193,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -5213,6 +5214,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -5233,6 +5235,7 @@ "cpu": [ "arm" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -5253,9 +5256,7 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -5276,9 +5277,7 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -5299,9 +5298,7 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -5322,9 +5319,7 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -5345,6 +5340,7 @@ "cpu": [ "arm64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -5365,6 +5361,7 @@ "cpu": [ "x64" ], + "dev": true, "license": "MPL-2.0", "optional": true, "os": [ @@ -9719,7 +9716,7 @@ "version": "7.24.6", "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-7.24.6.tgz", "integrity": "sha512-WRNW+sJgj5OBN4/0JpHFqtqzhpbnV0GuB+OozA9gCL7a993SmU+1JBZCzLNxYsbMfIeDL+lTsphD5jN5N+n0zg==", - "devOptional": true, + "dev": true, "license": "MIT" }, "node_modules/unicode-emoji-modifier-base": { diff --git a/package.json b/package.json index 48fcd93..c5f547d 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "CHANGELOG.md" ], "dependencies": { - "@doist/cli-core": "1.4.0", + "@doist/cli-core": "1.6.0", "@doist/comms-sdk": "3.0.0", "@pnpm/tabtab": "0.5.4", "chalk": "5.6.2", diff --git a/skills/comms-cli/SKILL.md b/skills/comms-cli/SKILL.md index afa4b0a..ef4bb90 100644 --- a/skills/comms-cli/SKILL.md +++ b/skills/comms-cli/SKILL.md @@ -31,7 +31,7 @@ tdc --user auth # Equivalen tdc auth logout # Remove saved token and auth metadata tdc auth logout --json # Emits `{"ok": true}` (--ndjson is silent) tdc auth logout --user # Target a specific stored account; mismatched ref errors with ACCOUNT_NOT_FOUND -tdc auth token view # Print the saved token to stdout (pipe-safe; refuses if COMMS_API_TOKEN is set) +tdc auth token view # Print the saved token to stdout (pipe-safe; refreshes an expiring OAuth token first; refuses if COMMS_API_TOKEN is set) tdc auth token view --user # Print the saved token for a specific stored account tdc auth refresh-token view # Print the saved OAuth refresh token to stdout (pipe-safe; OAuth logins only) tdc auth refresh-token view --user # Print the saved OAuth refresh token for a specific stored account diff --git a/src/commands/auth/auth.test.ts b/src/commands/auth/auth.test.ts index 84b1b07..437f90b 100644 --- a/src/commands/auth/auth.test.ts +++ b/src/commands/auth/auth.test.ts @@ -4,12 +4,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' // Mock the auth module (only the read-side shims are stubbed; the // write-side path now goes through `createCommsTokenStore` from -// auth-provider.js, mocked below). +// auth-provider.js, mocked below). `getTokenRefreshOptions` defaults to +// `undefined` so the attachers run without a refresh path; the global +// `--user` suite swaps in a fake provider to drive cli-core's real refresh. vi.mock('../../lib/auth.js', async (importOriginal) => { const actual = await importOriginal() return { ...actual, - getApiTokenSnapshot: vi.fn(), + getTokenRefreshOptions: vi.fn(), probeApiToken: vi.fn(), } }) @@ -31,6 +33,7 @@ const storeMocks = vi.hoisted(() => ({ clear: vi.fn(), active: vi.fn(), activeBundle: vi.fn(), + setBundle: vi.fn(), list: vi.fn(), setDefault: vi.fn(), getLastStorageResult: vi.fn(), @@ -97,12 +100,15 @@ vi.mock('node:readline', () => ({ // Mock chalk to avoid colors in tests vi.mock('chalk') +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { createInterface, type Interface } from 'node:readline' -import { attachLoginCommand } from '@doist/cli-core/auth' +import { attachLoginCommand, type AuthProvider } from '@doist/cli-core/auth' import { CommsRequestError, type User } from '@doist/comms-sdk' import { createWrappedCommsClient } from '../../lib/api.js' import { type CommsAccount, type CommsTokenStore } from '../../lib/auth-provider.js' -import { getApiTokenSnapshot, TOKEN_ENV_VAR } from '../../lib/auth.js' +import { getTokenRefreshOptions, TOKEN_ENV_VAR } from '../../lib/auth.js' import { getConfig, updateConfig } from '../../lib/config.js' import { resetGlobalArgs } from '../../lib/global-args.js' import { registerAuthCommand } from './index.js' @@ -110,7 +116,7 @@ import { attachCommsStatusCommand } from './status.js' const mockCreateInterface = vi.mocked(createInterface) -const mockGetApiTokenSnapshot = vi.mocked(getApiTokenSnapshot) +const mockGetTokenRefreshOptions = vi.mocked(getTokenRefreshOptions) const mockCreateWrappedCommsClient = vi.mocked(createWrappedCommsClient) const mockAttachLoginCommand = vi.mocked(attachLoginCommand) const mockGetConfig = vi.mocked(getConfig) @@ -150,6 +156,46 @@ describe('auth command', () => { const STORED_SNAPSHOT = { token: 'tk_stored_1234567890', account: STORED_ACCOUNT } const STORED_RECORDS = [{ account: STORED_ACCOUNT, isDefault: true }] + + /** + * Point `getTokenRefreshOptions` at a fake provider so the real cli-core + * refresh runs end-to-end (lock file, bundle re-read, persist) without a + * network. Returns the provider spy and a cleanup that also resets the + * mock so later tests go back to the no-refresh path. + */ + async function installFakeRefresh(): Promise<{ + refreshToken: ReturnType + cleanup: () => Promise + }> { + const lockDir = await mkdtemp(join(tmpdir(), 'tdc-auth-')) + const refreshToken = vi.fn(async () => ({ + accessToken: 'tk_refreshed_1234567890', + refreshToken: 'rt_refreshed_1234567890', + expiresAt: Date.now() + 3_600_000, + })) + mockGetTokenRefreshOptions.mockReturnValue({ + provider: { refreshToken } as unknown as AuthProvider, + lockPath: join(lockDir, 'refresh.lock'), + handshake: { clientId: 'tdd_123' }, + }) + storeMocks.list.mockResolvedValue(STORED_RECORDS) + storeMocks.activeBundle.mockResolvedValue({ + account: { ...STORED_ACCOUNT, authResource: 'https://comms.staging.todoist.com' }, + bundle: { + accessToken: 'tk_stored_1234567890', + refreshToken: 'rt_stored_1234567890', + accessTokenExpiresAt: Date.now() - 1_000, + }, + }) + storeMocks.setBundle.mockResolvedValue(undefined) + return { + refreshToken, + cleanup: async () => { + mockGetTokenRefreshOptions.mockReset() + await rm(lockDir, { recursive: true, force: true }) + }, + } + } const COMMS_SCOPE = 'user:read comms:content:read comms:content:write comms:messages:read comms:messages:write' @@ -358,6 +404,29 @@ describe('auth command', () => { vi.unstubAllEnvs() }) + it('prints the rotated token when the stored OAuth token is expiring', async () => { + vi.stubEnv(TOKEN_ENV_VAR, '') + const { refreshToken, cleanup } = await installFakeRefresh() + + try { + await createProgram().parseAsync(['node', 'tdc', 'auth', 'token', 'view']) + } finally { + await cleanup() + } + + expect(refreshToken).toHaveBeenCalledTimes(1) + expect(storeMocks.setBundle).toHaveBeenCalledWith( + expect.objectContaining({ id: '1' }), + expect.objectContaining({ + accessToken: 'tk_refreshed_1234567890', + refreshToken: 'rt_refreshed_1234567890', + }), + ) + expect(stdoutPayload()).toBe('tk_refreshed_1234567890') + // The stored-token read path is bypassed entirely on a refresh. + expect(storeMocks.active).not.toHaveBeenCalled() + }) + it('prints exactly the stored token to stdout with no envelope (pipe-safe)', async () => { vi.stubEnv(TOKEN_ENV_VAR, '') storeMocks.active.mockResolvedValue(STORED_SNAPSHOT) @@ -573,20 +642,9 @@ describe('auth command', () => { ) }) - it('threads `tdc --user auth status` into the snapshot used by fetchLive', async () => { + it('threads `tdc --user auth status` into the refresh and probes with the rotated token', async () => { vi.stubEnv(TOKEN_ENV_VAR, '') - storeMocks.list.mockResolvedValue(STORED_RECORDS) - storeMocks.activeBundle.mockResolvedValue({ - account: STORED_ACCOUNT, - bundle: { accessToken: 'tk_stored_1234567890' }, - }) - mockGetApiTokenSnapshot.mockResolvedValue({ - token: 'tk_refreshed_1234567890', - account: { - ...STORED_ACCOUNT, - authResource: 'https://comms.staging.todoist.com', - }, - }) + const { refreshToken, cleanup } = await installFakeRefresh() mockCreateWrappedCommsClient.mockReturnValue({ users: { getSessionUser: vi.fn().mockResolvedValue(TEST_USER) }, // biome-ignore lint/suspicious/noExplicitAny: only the methods used in this test matter @@ -594,10 +652,29 @@ describe('auth command', () => { process.argv = ['node', 'tdc', '--user', '1', 'auth', 'status'] resetGlobalArgs() - await createProgram().parseAsync(['node', 'tdc', 'auth', 'status']) + try { + await createProgram().parseAsync(['node', 'tdc', 'auth', 'status']) + } finally { + await cleanup() + } + // The global ref reaches the bundle reads cli-core's refresh makes + // via the `withUserRefAware` wrapper, and the whole rotated pair + // (not just the access token) is persisted before `fetchLive` + // probes with the new token. expect(storeMocks.activeBundle).toHaveBeenCalledWith('1') - expect(mockGetApiTokenSnapshot).toHaveBeenCalledWith('1') + expect(refreshToken).toHaveBeenCalledWith({ + refreshToken: 'rt_stored_1234567890', + handshake: { clientId: 'tdd_123' }, + }) + expect(storeMocks.setBundle).toHaveBeenCalledWith( + expect.objectContaining({ id: '1' }), + expect.objectContaining({ + accessToken: 'tk_refreshed_1234567890', + refreshToken: 'rt_refreshed_1234567890', + accessTokenExpiresAt: expect.any(Number), + }), + ) expect(mockCreateWrappedCommsClient).toHaveBeenCalledWith('tk_refreshed_1234567890', { baseUrl: 'https://comms.staging.todoist.com', }) @@ -676,10 +753,6 @@ describe('auth command', () => { } beforeEach(() => { - mockGetApiTokenSnapshot.mockResolvedValue({ - token: 'snapshot_token', - account: SNAPSHOT_ACCOUNT, - }) mockCreateWrappedCommsClient.mockReturnValue({ users: { getSessionUser: vi.fn().mockResolvedValue(TEST_USER) }, // biome-ignore lint/suspicious/noExplicitAny: only the methods used in this test matter diff --git a/src/commands/auth/index.ts b/src/commands/auth/index.ts index 1ad34fe..fb249f2 100644 --- a/src/commands/auth/index.ts +++ b/src/commands/auth/index.ts @@ -5,7 +5,7 @@ import { createCommsTokenStore, parseCredentialStore, } from '../../lib/auth-provider.js' -import { TOKEN_ENV_VAR } from '../../lib/auth.js' +import { getTokenRefreshOptions, TOKEN_ENV_VAR } from '../../lib/auth.js' import { withUnvalidatedChoices } from '../../lib/completion.js' import { getRequestedUserRef } from '../../lib/global-args.js' import { attachCommsLoginCommand } from './login.js' @@ -48,6 +48,7 @@ export function registerAuthCommand(program: Command): void { name: 'view', store: refAware, envVarName: TOKEN_ENV_VAR, + refresh: getTokenRefreshOptions(), description: 'Print the stored API token for the active user (or --user ) to stdout for use in scripts', }) diff --git a/src/commands/auth/status.ts b/src/commands/auth/status.ts index de9c68f..ebc2f1a 100644 --- a/src/commands/auth/status.ts +++ b/src/commands/auth/status.ts @@ -6,7 +6,7 @@ import { createWrappedCommsClient } from '../../lib/api.js' import type { CommsAccount, CommsTokenStore } from '../../lib/auth-provider.js' import { type AuthMetadata, - getApiTokenSnapshot, + getTokenRefreshOptions, NoTokenError, TOKEN_ENV_VAR, } from '../../lib/auth.js' @@ -85,11 +85,11 @@ function buildStatusJson({ user, metadata }: StatusData): Record(auth, { store, description: 'Show current authentication status', + refresh: getTokenRefreshOptions(), fetchLive: async ({ account, token }) => { - const snapshot = account.id ? await getApiTokenSnapshot(account.id) : { account, token } - data = await gatherStatusData(snapshot.token, snapshot.account) + data = await gatherStatusData(token, account) return { id: String(data.user.id), label: data.user.fullName, diff --git a/src/lib/auth.test.ts b/src/lib/auth.test.ts index 30e7de2..801f117 100644 --- a/src/lib/auth.test.ts +++ b/src/lib/auth.test.ts @@ -40,7 +40,14 @@ vi.mock('./config.js', async (importOriginal) => { } }) -import { getApiToken, getAuthMetadata, NoTokenError, probeApiToken, TOKEN_ENV_VAR } from './auth.js' +import { + getApiToken, + getAuthMetadata, + getTokenRefreshOptions, + NoTokenError, + probeApiToken, + TOKEN_ENV_VAR, +} from './auth.js' const STORED_ACCOUNT = { id: '42', @@ -179,6 +186,38 @@ describe('auth shims over the cli-core keyring store', () => { ) }) + it('getTokenRefreshOptions resolves the DCR handshake from the account cli-core hands it', () => { + const options = getTokenRefreshOptions() + + expect(options.lockPath).toBe('/home/user/.config/comms-cli/config.json.refresh.lock') + expect(options.skewMs).toBe(60_000) + expect(typeof options.provider.refreshToken).toBe('function') + expect(typeof options.handshake).toBe('function') + if (typeof options.handshake !== 'function') throw new Error('unreachable') + + expect( + options.handshake({ + account: { + ...STORED_ACCOUNT, + oauthClientId: 'tdd_123', + authBaseUrl: 'https://todoist.com', + authResource: 'https://comms.todoist.com', + }, + }), + ).toMatchObject({ + clientId: 'tdd_123', + accountId: '42', + authBaseUrl: 'https://todoist.com', + resource: 'https://comms.todoist.com', + }) + // Partial metadata surfaces as the same NO_TOKEN the API path raises, + // rather than letting cli-core POST a refresh grant with no client id. + const handshake = options.handshake + expect(() => handshake({ account: STORED_ACCOUNT })).toThrow( + expect.objectContaining({ code: 'NO_TOKEN' }), + ) + }) + it('getApiToken rejects partial OAuth client metadata instead of defaulting refresh target', async () => { mocks.activeBundleMock.mockResolvedValue({ account: { diff --git a/src/lib/auth.ts b/src/lib/auth.ts index d05d46e..5fb0067 100644 --- a/src/lib/auth.ts +++ b/src/lib/auth.ts @@ -1,4 +1,8 @@ -import { refreshAccessToken, SecureStoreUnavailableError } from '@doist/cli-core/auth' +import { + refreshAccessToken, + SecureStoreUnavailableError, + type TokenRefreshOptions, +} from '@doist/cli-core/auth' import type { CommsAccount } from './auth-provider.js' import { createCommsAuthProvider, @@ -57,6 +61,25 @@ export type ActiveAuthSnapshot = { const REFRESH_SKEW_MS = 60_000 +function refreshLockPath(): string { + return `${getConfigPath()}.refresh.lock` +} + +/** + * Refresh wiring for cli-core's `auth status` / `auth token view` attachers, + * so both hand back a token that is usable right now rather than whatever + * was last stored. The handshake is resolved per account because cli-core + * doesn't persist the DCR client metadata the refresh grant needs. + */ +export function getTokenRefreshOptions(): TokenRefreshOptions { + return { + provider: createCommsAuthProvider(), + lockPath: refreshLockPath(), + skewMs: REFRESH_SKEW_MS, + handshake: ({ account }) => getCommsOAuthRefreshHandshake(account), + } +} + export class NoTokenError extends CliError { constructor() { super( @@ -116,11 +139,12 @@ async function getActiveSnapshot({ ['Run: tdc auth login'], ) } + // Same wiring as the attachers; the handshake is resolved up front + // because the missing-metadata check above already ran for it. const refreshed = await refreshAccessToken({ store, - provider: createCommsAuthProvider(), - skewMs: REFRESH_SKEW_MS, - lockPath: `${getConfigPath()}.refresh.lock`, + ref, + ...getTokenRefreshOptions(), handshake: getCommsOAuthRefreshHandshake(account), }) return { token: refreshed.bundle.accessToken, account: refreshed.account } diff --git a/src/lib/skills/content.ts b/src/lib/skills/content.ts index b2e015c..6ced738 100644 --- a/src/lib/skills/content.ts +++ b/src/lib/skills/content.ts @@ -35,7 +35,7 @@ tdc --user auth # Equivalen tdc auth logout # Remove saved token and auth metadata tdc auth logout --json # Emits \`{"ok": true}\` (--ndjson is silent) tdc auth logout --user # Target a specific stored account; mismatched ref errors with ACCOUNT_NOT_FOUND -tdc auth token view # Print the saved token to stdout (pipe-safe; refuses if COMMS_API_TOKEN is set) +tdc auth token view # Print the saved token to stdout (pipe-safe; refreshes an expiring OAuth token first; refuses if COMMS_API_TOKEN is set) tdc auth token view --user # Print the saved token for a specific stored account tdc auth refresh-token view # Print the saved OAuth refresh token to stdout (pipe-safe; OAuth logins only) tdc auth refresh-token view --user # Print the saved OAuth refresh token for a specific stored account