From 7deafc36bf01f38f7b9a8f31aa4fa002533c4bce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A7alo=20Silva?= Date: Tue, 16 Jun 2026 13:04:17 +0000 Subject: [PATCH] fix(auth): refresh token before token view --- src/commands/auth/auth.test.ts | 77 +++++++++++++++++++++++++++------ src/commands/auth/index.ts | 14 ++---- src/commands/auth/token-view.ts | 49 +++++++++++++++++++++ 3 files changed, 117 insertions(+), 23 deletions(-) create mode 100644 src/commands/auth/token-view.ts diff --git a/src/commands/auth/auth.test.ts b/src/commands/auth/auth.test.ts index 62aa54e..7ca6358 100644 --- a/src/commands/auth/auth.test.ts +++ b/src/commands/auth/auth.test.ts @@ -88,7 +88,7 @@ import { attachLoginCommand } 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 { getApiTokenSnapshot, NoTokenError, 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' @@ -284,16 +284,67 @@ describe('auth command', () => { vi.unstubAllEnvs() }) - it('prints exactly the stored token to stdout with no envelope (pipe-safe)', async () => { + it('prints exactly the current token snapshot to stdout with no envelope (pipe-safe)', async () => { vi.stubEnv(TOKEN_ENV_VAR, '') - storeMocks.active.mockResolvedValue(STORED_SNAPSHOT) + mockGetApiTokenSnapshot.mockResolvedValue(STORED_SNAPSHOT) await createProgram().parseAsync(['node', 'tdc', 'auth', 'token', 'view']) + expect(mockGetApiTokenSnapshot).toHaveBeenCalledWith(undefined) expect(stdoutPayload()).toBe('tk_stored_1234567890') expect(consoleSpy).not.toHaveBeenCalled() }) + it('adds a trailing newline only when stdout is a TTY', async () => { + vi.stubEnv(TOKEN_ENV_VAR, '') + mockGetApiTokenSnapshot.mockResolvedValue(STORED_SNAPSHOT) + const originalIsTTY = process.stdout.isTTY + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }) + + try { + await createProgram().parseAsync(['node', 'tdc', 'auth', 'token', 'view']) + } finally { + Object.defineProperty(process.stdout, 'isTTY', { + value: originalIsTTY, + configurable: true, + }) + } + + expect(stdoutPayload()).toBe('tk_stored_1234567890\n') + }) + + it('refreshes an expired OAuth token before printing', async () => { + vi.stubEnv(TOKEN_ENV_VAR, '') + const oauthAccount: CommsAccount = { + ...STORED_ACCOUNT, + oauthClientId: 'tdd_123', + authBaseUrl: 'https://todoist.com', + authResource: 'https://comms.todoist.com', + } + mockGetApiTokenSnapshot.mockResolvedValue({ + token: 'tk_refreshed_1234567890', + account: oauthAccount, + }) + + await createProgram().parseAsync(['node', 'tdc', 'auth', 'token', 'view']) + + expect(mockGetApiTokenSnapshot).toHaveBeenCalledWith(undefined) + expect(stdoutPayload()).toBe('tk_refreshed_1234567890') + }) + + it('prints manual tokens returned by the token snapshot path', async () => { + vi.stubEnv(TOKEN_ENV_VAR, '') + mockGetApiTokenSnapshot.mockResolvedValue({ + token: 'manual_token_1234567890', + account: { id: '', label: '', authMode: 'unknown', authScope: '' }, + }) + + await createProgram().parseAsync(['node', 'tdc', 'auth', 'token', 'view']) + + expect(mockGetApiTokenSnapshot).toHaveBeenCalledWith(undefined) + expect(stdoutPayload()).toBe('manual_token_1234567890') + }) + it('refuses to print when the env var is set so the CLI does not disclose an unmanaged token', async () => { vi.stubEnv(TOKEN_ENV_VAR, 'env_token_supplied_externally') @@ -301,13 +352,13 @@ describe('auth command', () => { createProgram().parseAsync(['node', 'tdc', 'auth', 'token', 'view']), ).rejects.toHaveProperty('code', 'TOKEN_FROM_ENV') - expect(storeMocks.active).not.toHaveBeenCalled() + expect(mockGetApiTokenSnapshot).not.toHaveBeenCalled() expect(stdoutPayload()).toBe('') }) it('throws NOT_AUTHENTICATED when no token is stored', async () => { vi.stubEnv(TOKEN_ENV_VAR, '') - storeMocks.active.mockResolvedValue(null) + mockGetApiTokenSnapshot.mockRejectedValue(new NoTokenError()) await expect( createProgram().parseAsync(['node', 'tdc', 'auth', 'token', 'view']), @@ -316,9 +367,9 @@ describe('auth command', () => { expect(stdoutPayload()).toBe('') }) - it('matches per-command --user against the stored account by id', async () => { + it('passes per-command --user through to the token snapshot path', async () => { vi.stubEnv(TOKEN_ENV_VAR, '') - storeMocks.active.mockResolvedValue(STORED_SNAPSHOT) + mockGetApiTokenSnapshot.mockResolvedValue(STORED_SNAPSHOT) await createProgram().parseAsync([ 'node', @@ -330,13 +381,13 @@ describe('auth command', () => { '1', ]) - expect(storeMocks.active).toHaveBeenCalledWith('1') + expect(mockGetApiTokenSnapshot).toHaveBeenCalledWith('1') expect(stdoutPayload()).toBe('tk_stored_1234567890') }) it('rejects per-command --user with ACCOUNT_NOT_FOUND when the ref does not match', async () => { vi.stubEnv(TOKEN_ENV_VAR, '') - storeMocks.active.mockResolvedValue(null) + mockGetApiTokenSnapshot.mockRejectedValue(new NoTokenError()) await expect( createProgram().parseAsync([ @@ -350,6 +401,7 @@ describe('auth command', () => { ]), ).rejects.toHaveProperty('code', 'ACCOUNT_NOT_FOUND') + expect(mockGetApiTokenSnapshot).toHaveBeenCalledWith('999') expect(stdoutPayload()).toBe('') }) }) @@ -373,16 +425,15 @@ describe('auth command', () => { vi.unstubAllEnvs() }) - it('threads `tdc --user auth token view` into store.active', async () => { + it('threads `tdc --user auth token view` into token refresh', async () => { vi.stubEnv(TOKEN_ENV_VAR, '') - storeMocks.list.mockResolvedValue(STORED_RECORDS) - storeMocks.active.mockResolvedValue(STORED_SNAPSHOT) + mockGetApiTokenSnapshot.mockResolvedValue(STORED_SNAPSHOT) process.argv = ['node', 'tdc', '--user', '1', 'auth', 'token', 'view'] resetGlobalArgs() await createProgram().parseAsync(['node', 'tdc', 'auth', 'token', 'view']) - expect(storeMocks.active).toHaveBeenCalledWith('1') + expect(mockGetApiTokenSnapshot).toHaveBeenCalledWith('1') expect(writeSpy.mock.calls.map((c: unknown[]) => String(c[0])).join('')).toBe( 'tk_stored_1234567890', ) diff --git a/src/commands/auth/index.ts b/src/commands/auth/index.ts index e971d99..d316819 100644 --- a/src/commands/auth/index.ts +++ b/src/commands/auth/index.ts @@ -1,19 +1,19 @@ -import { attachTokenViewCommand } from '@doist/cli-core/auth' import { Command } from 'commander' import { createCommsTokenStore } from '../../lib/auth-provider.js' -import { TOKEN_ENV_VAR } from '../../lib/auth.js' import { getRequestedUserRef } from '../../lib/global-args.js' import { attachCommsLoginCommand } from './login.js' import { attachCommsLogoutCommand } from './logout.js' import { attachCommsStatusCommand } from './status.js' import { withUserRefAware } from './store-wrap.js' +import { attachCommsTokenViewCommand } from './token-view.js' import { loginWithToken } from './token.js' export function registerAuthCommand(program: Command): void { const auth = program.command('auth').description('Manage authentication') const store = createCommsTokenStore() - const refAware = withUserRefAware(store, getRequestedUserRef()) + const requestedRef = getRequestedUserRef() + const refAware = withUserRefAware(store, requestedRef) attachCommsLoginCommand(auth, store) attachCommsLogoutCommand(auth, refAware) @@ -28,11 +28,5 @@ export function registerAuthCommand(program: Command): void { .description('Save API token for CLI authentication (or use a subcommand: `view`)') .action(() => loginWithToken()) - attachTokenViewCommand(tokenCmd, { - name: 'view', - store: refAware, - envVarName: TOKEN_ENV_VAR, - description: - 'Print the stored API token for the active user (or --user ) to stdout for use in scripts', - }) + attachCommsTokenViewCommand(tokenCmd, requestedRef) } diff --git a/src/commands/auth/token-view.ts b/src/commands/auth/token-view.ts new file mode 100644 index 0000000..964f4a6 --- /dev/null +++ b/src/commands/auth/token-view.ts @@ -0,0 +1,49 @@ +import type { Command } from 'commander' +import { getApiTokenSnapshot, NoTokenError, TOKEN_ENV_VAR } from '../../lib/auth.js' +import { CliError } from '../../lib/errors.js' + +type TokenViewOptions = { + user?: string +} + +const USER_FLAG_DESCRIPTION = 'Target a specific stored account' + +export function attachCommsTokenViewCommand( + parent: Command, + requestedRef: string | undefined, +): Command { + return parent + .command('view') + .description( + 'Print the stored API token for the active user (or --user ) to stdout for use in scripts', + ) + .option('--user ', USER_FLAG_DESCRIPTION) + .action((options: TokenViewOptions) => viewToken(options.user ?? requestedRef)) +} + +async function viewToken(ref: string | undefined): Promise { + if (process.env[TOKEN_ENV_VAR]) { + throw new CliError( + 'TOKEN_FROM_ENV', + `Refusing to print: token is being read from $${TOKEN_ENV_VAR}, not the saved store.`, + [ + `Unset ${TOKEN_ENV_VAR} to view the stored token.`, + 'The env var takes precedence over saved tokens; printing it would disclose a secret the CLI did not manage.', + ], + ) + } + + try { + const snapshot = await getApiTokenSnapshot(ref) + process.stdout.write(snapshot.token) + if (process.stdout.isTTY) process.stdout.write('\n') + } catch (error) { + if (error instanceof NoTokenError) { + if (ref !== undefined) { + throw new CliError('ACCOUNT_NOT_FOUND', `No stored account matches "${ref}".`) + } + throw new CliError('NOT_AUTHENTICATED', 'Not signed in.') + } + throw error + } +}