Skip to content

Commit 83a6b32

Browse files
committed
fix: honor OAuth resource in auth probes
Use refreshed account snapshots and stored Comms resources when auth status and doctor validate OAuth tokens.
1 parent 5ddca70 commit 83a6b32

5 files changed

Lines changed: 79 additions & 43 deletions

File tree

src/commands/auth/auth.test.ts

Lines changed: 22 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ vi.mock('../../lib/auth.js', async (importOriginal) => {
99
const actual = await importOriginal<typeof import('../../lib/auth.js')>()
1010
return {
1111
...actual,
12-
getAuthMetadata: vi.fn(),
12+
getApiTokenSnapshot: vi.fn(),
1313
probeApiToken: vi.fn(),
1414
}
1515
})
@@ -88,15 +88,15 @@ import { attachLoginCommand } from '@doist/cli-core/auth'
8888
import { CommsRequestError, type User } from '@doist/comms-sdk'
8989
import { createWrappedCommsClient } from '../../lib/api.js'
9090
import { type CommsAccount, type CommsTokenStore } from '../../lib/auth-provider.js'
91-
import { getAuthMetadata, TOKEN_ENV_VAR } from '../../lib/auth.js'
91+
import { getApiTokenSnapshot, TOKEN_ENV_VAR } from '../../lib/auth.js'
9292
import { getConfig, updateConfig } from '../../lib/config.js'
9393
import { resetGlobalArgs } from '../../lib/global-args.js'
9494
import { registerAuthCommand } from './index.js'
9595
import { attachCommsStatusCommand } from './status.js'
9696

9797
const mockCreateInterface = vi.mocked(createInterface)
9898

99-
const mockGetAuthMetadata = vi.mocked(getAuthMetadata)
99+
const mockGetApiTokenSnapshot = vi.mocked(getApiTokenSnapshot)
100100
const mockCreateWrappedCommsClient = vi.mocked(createWrappedCommsClient)
101101
const mockAttachLoginCommand = vi.mocked(attachLoginCommand)
102102
const mockGetConfig = vi.mocked(getConfig)
@@ -392,22 +392,27 @@ describe('auth command', () => {
392392
vi.stubEnv(TOKEN_ENV_VAR, '')
393393
storeMocks.list.mockResolvedValue(STORED_RECORDS)
394394
storeMocks.active.mockResolvedValue(STORED_SNAPSHOT)
395+
mockGetApiTokenSnapshot.mockResolvedValue({
396+
token: 'tk_refreshed_1234567890',
397+
account: {
398+
...STORED_ACCOUNT,
399+
authResource: 'https://comms.staging.todoist.com',
400+
},
401+
})
395402
mockCreateWrappedCommsClient.mockReturnValue({
396403
users: { getSessionUser: vi.fn().mockResolvedValue(TEST_USER) },
397404
// biome-ignore lint/suspicious/noExplicitAny: only the methods used in this test matter
398405
} as any)
399-
mockGetAuthMetadata.mockResolvedValue({
400-
authMode: 'read-write',
401-
authScope: 'user:read',
402-
source: 'config',
403-
})
404406
process.argv = ['node', 'tdc', '--user', '1', 'auth', 'status']
405407
resetGlobalArgs()
406408

407409
await createProgram().parseAsync(['node', 'tdc', 'auth', 'status'])
408410

409411
expect(storeMocks.active).toHaveBeenCalledWith('1')
410-
expect(mockCreateWrappedCommsClient).toHaveBeenCalledWith('tk_stored_1234567890')
412+
expect(mockGetApiTokenSnapshot).toHaveBeenCalledWith('1')
413+
expect(mockCreateWrappedCommsClient).toHaveBeenCalledWith('tk_refreshed_1234567890', {
414+
baseUrl: 'https://comms.staging.todoist.com',
415+
})
411416
expect(consoleSpy).toHaveBeenCalledWith('✓ Authenticated')
412417
})
413418

@@ -449,6 +454,7 @@ describe('auth command', () => {
449454
label: TEST_USER.fullName,
450455
authMode: 'read-write',
451456
authScope: COMMS_SCOPE,
457+
authResource: 'https://comms.staging.todoist.com',
452458
}
453459

454460
function programWithSnapshot(): Command {
@@ -482,21 +488,22 @@ describe('auth command', () => {
482488
}
483489

484490
beforeEach(() => {
491+
mockGetApiTokenSnapshot.mockResolvedValue({
492+
token: 'snapshot_token',
493+
account: SNAPSHOT_ACCOUNT,
494+
})
485495
mockCreateWrappedCommsClient.mockReturnValue({
486496
users: { getSessionUser: vi.fn().mockResolvedValue(TEST_USER) },
487497
// biome-ignore lint/suspicious/noExplicitAny: only the methods used in this test matter
488498
} as any)
489-
mockGetAuthMetadata.mockResolvedValue({
490-
authMode: 'read-write',
491-
authScope: COMMS_SCOPE,
492-
source: 'config',
493-
})
494499
})
495500

496501
it('renders text status from the snapshot', async () => {
497502
await programWithSnapshot().parseAsync(['node', 'tdc', 'auth', 'status'])
498503

499-
expect(mockCreateWrappedCommsClient).toHaveBeenCalledWith('snapshot_token')
504+
expect(mockCreateWrappedCommsClient).toHaveBeenCalledWith('snapshot_token', {
505+
baseUrl: 'https://comms.staging.todoist.com',
506+
})
500507
expect(consoleSpy).toHaveBeenCalledWith('✓ Authenticated')
501508
expect(consoleSpy).toHaveBeenCalledWith(' Email: test@example.com')
502509
expect(consoleSpy).toHaveBeenCalledWith(' Name: Test User')

src/commands/auth/status.ts

Lines changed: 34 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,12 @@ import chalk from 'chalk'
44
import type { Command } from 'commander'
55
import { createWrappedCommsClient } from '../../lib/api.js'
66
import type { CommsAccount, CommsTokenStore } from '../../lib/auth-provider.js'
7-
import { type AuthMetadata, getAuthMetadata, NoTokenError } from '../../lib/auth.js'
7+
import {
8+
type AuthMetadata,
9+
getApiTokenSnapshot,
10+
NoTokenError,
11+
TOKEN_ENV_VAR,
12+
} from '../../lib/auth.js'
813
import type { AuthMode } from '../../lib/config.js'
914
import { CliError } from '../../lib/errors.js'
1015

@@ -24,19 +29,28 @@ function formatAuthMode(authMode: AuthMode, authScope?: string): string {
2429
}
2530

2631
/**
27-
* Fetch the live session user (via the snapshot token) and the local auth
28-
* metadata concurrently — the API call dominates and the metadata read is
29-
* independent. 401-translation lives here so both the snapshot path and any
30-
* future callers emit the same `NO_TOKEN` envelope when the token is
31-
* rejected by the API.
32+
* Fetch the live session user via the selected account's resource. 401
33+
* translation lives here so both refreshed OAuth tokens and manual tokens emit
34+
* the same `NO_TOKEN` envelope when Comms rejects them.
3235
*/
33-
async function gatherStatusData(token: string): Promise<StatusData> {
36+
function metadataFromAccount(account: CommsAccount): AuthMetadata {
37+
const authUserId = Number(account.id)
38+
return {
39+
authMode: account.authMode,
40+
...(account.authResource ? { authResource: account.authResource } : {}),
41+
authScope: account.authScope || undefined,
42+
authUserId: Number.isFinite(authUserId) && authUserId > 0 ? authUserId : undefined,
43+
authUserName: account.label || undefined,
44+
source: account.id || !process.env[TOKEN_ENV_VAR] ? 'config' : 'env',
45+
}
46+
}
47+
48+
async function gatherStatusData(token: string, account: CommsAccount): Promise<StatusData> {
3449
try {
35-
const [user, metadata] = await Promise.all([
36-
createWrappedCommsClient(token).users.getSessionUser(),
37-
getAuthMetadata(),
38-
])
39-
return { user, metadata }
50+
const user = await createWrappedCommsClient(token, {
51+
baseUrl: account.authResource,
52+
}).users.getSessionUser()
53+
return { user, metadata: metadataFromAccount(account) }
4054
} catch (error) {
4155
if (error instanceof CommsRequestError && error.httpStatusCode === 401) {
4256
throw new CliError('NO_TOKEN', 'Not authenticated (token expired or invalid)', [
@@ -71,23 +85,21 @@ function buildStatusJson({ user, metadata }: StatusData): Record<string, unknown
7185
/**
7286
* Attach `tdc auth status` via cli-core's generic `attachStatusCommand`.
7387
*
74-
* `CommsTokenStore.active()` returns a snapshot whenever a token resolves
75-
* (per the adapter's documented contract — see `auth-provider.ts`), so
76-
* `fetchLive` covers every token-present path: secure-store, plaintext
77-
* config fallback, env-token mode, and manual `tdc auth token`. The
78-
* snapshot's token is reused inside `gatherStatusData` so credentials are
79-
* read once per invocation. `onNotAuthenticated` only fires when nothing
80-
* is stored — it throws `NoTokenError` so the standard CliError envelope
81-
* reaches the operator unchanged.
88+
* cli-core reads the selected account first. `fetchLive` then refreshes OAuth
89+
* accounts through the same auth shim as normal API calls before validating
90+
* the token against Comms. `onNotAuthenticated` only fires when nothing is
91+
* stored — it throws `NoTokenError` so the standard CliError envelope reaches
92+
* the operator unchanged.
8293
*/
8394
export function attachCommsStatusCommand(auth: Command, store: CommsTokenStore): Command {
8495
let data: StatusData | null = null
8596

8697
return attachStatusCommand<CommsAccount>(auth, {
8798
store,
8899
description: 'Show current authentication status',
89-
fetchLive: async ({ token }) => {
90-
data = await gatherStatusData(token)
100+
fetchLive: async ({ account, token }) => {
101+
const snapshot = account.id ? await getApiTokenSnapshot(account.id) : { account, token }
102+
data = await gatherStatusData(snapshot.token, snapshot.account)
91103
return {
92104
id: String(data.user.id),
93105
label: data.user.fullName,

src/commands/doctor.test.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,11 @@ describe('doctor command', () => {
7474
mockGetConfig.mockResolvedValue({})
7575
mockProbeApiToken.mockResolvedValue({
7676
token: 'test_token_123456789',
77-
metadata: { authMode: 'read-write', source: 'secure-store' },
77+
metadata: {
78+
authMode: 'read-write',
79+
authResource: 'https://comms.staging.todoist.com',
80+
source: 'secure-store',
81+
},
7882
})
7983
mockCreateWrappedCommsClient.mockReturnValue({
8084
users: {
@@ -110,6 +114,9 @@ describe('doctor command', () => {
110114
expect(consoleSpy).toHaveBeenCalledWith(
111115
expect.stringContaining('PASS Authenticated as person@example.com via secure-store'),
112116
)
117+
expect(mockCreateWrappedCommsClient).toHaveBeenCalledWith('test_token_123456789', {
118+
baseUrl: 'https://comms.staging.todoist.com',
119+
})
113120
expect(consoleSpy).toHaveBeenCalledWith(
114121
expect.stringContaining('PASS CLI is up to date on stable (v1.0.0)'),
115122
)

src/commands/doctor.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ async function checkAuthentication(offline: boolean): Promise<DoctorCheck> {
284284
}
285285

286286
try {
287-
const api = createWrappedCommsClient(token)
287+
const api = createWrappedCommsClient(token, { baseUrl: metadata.authResource })
288288
const user = await api.users.getSessionUser()
289289
details.email = user.email
290290
details.name = user.fullName

src/lib/auth.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export type TokenStorageResult = {
2525

2626
export type AuthMetadata = {
2727
authMode: AuthMode
28+
authResource?: string
2829
authScope?: string
2930
authUserId?: number
3031
authUserName?: string
@@ -33,6 +34,7 @@ export type AuthMetadata = {
3334

3435
export type AuthProbeMetadata = {
3536
authMode: AuthMode
37+
authResource?: string
3638
authScope?: string
3739
authUserId?: number
3840
authUserName?: string
@@ -69,8 +71,8 @@ export async function getApiToken(): Promise<string> {
6971
return snapshot.token
7072
}
7173

72-
export async function getApiTokenSnapshot(): Promise<ActiveAuthSnapshot> {
73-
return getActiveSnapshot({ refresh: true })
74+
export async function getApiTokenSnapshot(ref?: string): Promise<ActiveAuthSnapshot> {
75+
return getActiveSnapshot({ refresh: true, ref })
7476
}
7577

7678
/** Token + metadata in one round-trip for `tdc config view` / `tdc doctor`. */
@@ -86,9 +88,15 @@ export async function probeApiToken(): Promise<AuthProbeResult> {
8688
}
8789
}
8890

89-
async function getActiveSnapshot({ refresh }: { refresh: boolean }): Promise<ActiveAuthSnapshot> {
91+
async function getActiveSnapshot({
92+
refresh,
93+
ref,
94+
}: {
95+
refresh: boolean
96+
ref?: string
97+
}): Promise<ActiveAuthSnapshot> {
9098
const store = createCommsTokenStore()
91-
const snapshot = await store.activeBundle()
99+
const snapshot = await store.activeBundle(ref)
92100
if (!snapshot) throw new NoTokenError()
93101

94102
const { account, bundle } = snapshot
@@ -135,12 +143,14 @@ export async function getAuthMetadata(): Promise<AuthMetadata> {
135143

136144
function toAccountFields(account: CommsAccount): {
137145
authMode: AuthMode
146+
authResource?: string
138147
authScope?: string
139148
authUserId?: number
140149
authUserName?: string
141150
} {
142151
return {
143152
authMode: account.authMode,
153+
...(account.authResource ? { authResource: account.authResource } : {}),
144154
authScope: account.authScope || undefined,
145155
authUserId: account.id ? toAuthUserId(account.id) : undefined,
146156
authUserName: account.label || undefined,

0 commit comments

Comments
 (0)