diff --git a/README.md b/README.md index 64d261b..ff48088 100644 --- a/README.md +++ b/README.md @@ -138,6 +138,13 @@ Every command that calls the Xero API supports: Environment variables `XERO_PROFILE` and `XERO_CLIENT_ID` are also supported. Token storage can be tuned with `XERO_KEY_STORAGE`, `XERO_KEYRING_FILE_BACKUP`, and `XERO_TOKEN_PASSPHRASE` (see [Token storage](#token-storage)). The `xero login` command additionally accepts `XERO_SCOPES` (see [OAuth scopes](#oauth-scopes) above). +When using an inline client ID rather than a named profile, use the same client ID for login and logout. Inline sessions are kept separate for each client ID: + +```bash +xero login --client-id YOUR_CLIENT_ID +xero logout --client-id YOUR_CLIENT_ID +``` + ## Finding IDs Most commands that reference contacts, invoices, or accounts require a Xero GUID (e.g., `edc74793-8d7e-4bf2-9e63-146dc4c675a2`). Use the list commands to find IDs: diff --git a/src/base-command.ts b/src/base-command.ts index c81d8cb..3623a7d 100644 --- a/src/base-command.ts +++ b/src/base-command.ts @@ -5,6 +5,7 @@ import {XeroClient} from 'xero-node' import {getProfileClientId, getDefaultProfile} from './lib/profiles.js' import {withRetry} from './lib/xero-client.js' import {formatOutput, type OutputFormat} from './lib/formatters.js' +import {resolveCredentialCacheKey} from './lib/credential-cache-key.js' export abstract class BaseCommand extends Command { static baseFlags = { @@ -35,21 +36,12 @@ export abstract class BaseCommand extends Command { profile?: string 'client-id'?: string }): {profileName: string; clientId: string} { - // Priority 1: Explicit client-id flag - if (flags['client-id']) { - return { - profileName: flags.profile ?? '_inline', - clientId: flags['client-id'], - } - } - - // Priority 2: Named profile or default - const profileName = flags.profile ?? getDefaultProfile() + const profileName = resolveCredentialCacheKey(flags) ?? getDefaultProfile() if (!profileName) { this.error('No profile configured. Run "xero profile add " to set up a profile.') } - const clientId = getProfileClientId(profileName) + const clientId = flags['client-id'] ?? getProfileClientId(profileName) return {profileName, clientId} } diff --git a/src/commands/logout.ts b/src/commands/logout.ts index cfe89e3..61189d4 100644 --- a/src/commands/logout.ts +++ b/src/commands/logout.ts @@ -2,6 +2,11 @@ import {Flags} from '@oclif/core' import {BaseCommand} from '../base-command.js' import {clearCachedToken} from '../lib/auth.js' import {getDefaultProfile} from '../lib/profiles.js' +import { + getCredentialCacheKeysToClear, + isInlineCredentialSelector, + resolveCredentialCacheKey, +} from '../lib/credential-cache-key.js' export default class Logout extends BaseCommand { static override description = 'Log out from Xero (clear cached tokens)' @@ -9,6 +14,7 @@ export default class Logout extends BaseCommand { static override examples = [ '<%= config.bin %> logout', '<%= config.bin %> logout -p acme-corp', + '<%= config.bin %> logout --client-id YOUR_CLIENT_ID', ] static override flags = { @@ -17,17 +23,27 @@ export default class Logout extends BaseCommand { description: 'Xero profile name', env: 'XERO_PROFILE', }), + 'client-id': Flags.string({ + description: 'Xero client ID for an inline login', + env: 'XERO_CLIENT_ID', + }), } async run(): Promise { const {flags} = await this.parse(Logout) - const profileName = flags.profile ?? getDefaultProfile() + const profileName = resolveCredentialCacheKey(flags) ?? getDefaultProfile() if (!profileName) { this.error('No profile configured. Nothing to log out from.') } - clearCachedToken(profileName) - this.log(`Logged out from profile "${profileName}". Run "xero login" to re-authenticate.`) + for (const cacheKey of getCredentialCacheKeysToClear(flags, profileName)) { + clearCachedToken(cacheKey) + } + + const sessionDescription = isInlineCredentialSelector(flags) + ? 'inline client credentials' + : `profile "${profileName}"` + this.log(`Logged out from ${sessionDescription}. Run "xero login" to re-authenticate.`) } } diff --git a/src/commands/profile/add.ts b/src/commands/profile/add.ts index e6bb419..d178bf5 100644 --- a/src/commands/profile/add.ts +++ b/src/commands/profile/add.ts @@ -2,6 +2,7 @@ import {Args, Flags} from '@oclif/core' import {input} from '@inquirer/prompts' import {BaseCommand} from '../../base-command.js' import {addProfile, profileExists} from '../../lib/profiles.js' +import {clearCachedToken} from '../../lib/auth.js' export default class ProfileAdd extends BaseCommand { static override args = { @@ -24,7 +25,8 @@ export default class ProfileAdd extends BaseCommand { const {args, flags} = await this.parse(ProfileAdd) const {name} = args - if (profileExists(name) && !flags.force) { + const replacingProfile = profileExists(name) + if (replacingProfile && !flags.force) { this.error(`Profile "${name}" already exists. Use --force to overwrite.`) } @@ -33,6 +35,10 @@ export default class ProfileAdd extends BaseCommand { validate: (v) => v.length > 0 || 'Client ID is required', }) + if (replacingProfile) { + clearCachedToken(name) + } + addProfile(name, clientId) this.log(`Profile "${name}" added successfully.`) diff --git a/src/commands/profile/remove.ts b/src/commands/profile/remove.ts index 839b910..6e03c17 100644 --- a/src/commands/profile/remove.ts +++ b/src/commands/profile/remove.ts @@ -1,6 +1,7 @@ import {Args} from '@oclif/core' import {BaseCommand} from '../../base-command.js' import {profileExists, removeProfile} from '../../lib/profiles.js' +import {clearCachedToken} from '../../lib/auth.js' export default class ProfileRemove extends BaseCommand { static override args = { @@ -19,6 +20,7 @@ export default class ProfileRemove extends BaseCommand { this.error(`Profile "${name}" not found.`) } + clearCachedToken(name) removeProfile(name) this.log(`Profile "${name}" removed.`) } diff --git a/src/lib/credential-cache-key.ts b/src/lib/credential-cache-key.ts new file mode 100644 index 0000000..cc96631 --- /dev/null +++ b/src/lib/credential-cache-key.ts @@ -0,0 +1,49 @@ +export interface CredentialSelector { + profile?: string + 'client-id'?: string +} + +/** + * The cache key used by releases before inline credentials were scoped to a + * client ID. It is deliberately never used for reads because it cannot be + * safely associated with a particular client ID. + */ +export const LEGACY_INLINE_CACHE_KEY = '_inline' + +const INLINE_CACHE_KEY_PREFIX = '_inline:' + +export function getInlineCredentialCacheKey(clientId: string): string { + return `${INLINE_CACHE_KEY_PREFIX}${clientId}` +} + +/** + * Resolve an explicitly supplied profile/client ID to the token-cache key. + * Callers that support a default profile should fall back to it only when + * this function returns undefined. + */ +export function resolveCredentialCacheKey(flags: CredentialSelector): string | undefined { + if (flags.profile) return flags.profile + if (flags['client-id']) return getInlineCredentialCacheKey(flags['client-id']) + return undefined +} + +export function isInlineCredentialSelector(flags: CredentialSelector): boolean { + return !flags.profile && Boolean(flags['client-id']) +} + +/** + * A logout of an inline session also removes the ambiguous legacy key. A + * previous `_inline` token has no client ID recorded with it, so it must not + * be reused by a client-ID-scoped session. + */ +export function getCredentialCacheKeysToClear( + flags: CredentialSelector, + defaultProfile?: string, +): string[] { + const cacheKey = resolveCredentialCacheKey(flags) ?? defaultProfile + if (!cacheKey) return [] + + return isInlineCredentialSelector(flags) + ? [cacheKey, LEGACY_INLINE_CACHE_KEY] + : [cacheKey] +} diff --git a/test/commands/oauth-cache-lifecycle.test.ts b/test/commands/oauth-cache-lifecycle.test.ts new file mode 100644 index 0000000..2cc5edf --- /dev/null +++ b/test/commands/oauth-cache-lifecycle.test.ts @@ -0,0 +1,203 @@ +import {beforeEach, describe, expect, it, vi} from 'vitest' +import {getInlineCredentialCacheKey, LEGACY_INLINE_CACHE_KEY} from '../../src/lib/credential-cache-key.js' + +const mocks = vi.hoisted(() => ({ + addProfile: vi.fn(), + cacheTokenSet: vi.fn(), + clearCachedToken: vi.fn(), + getDefaultProfile: vi.fn(), + getProfileClientId: vi.fn(), + performLogin: vi.fn(), + profileExists: vi.fn(), + removeProfile: vi.fn(), +})) + +vi.mock('../../src/lib/auth.js', () => ({ + cacheTokenSet: mocks.cacheTokenSet, + clearCachedToken: mocks.clearCachedToken, +})) + +vi.mock('../../src/lib/profiles.js', () => ({ + addProfile: mocks.addProfile, + getDefaultProfile: mocks.getDefaultProfile, + getProfileClientId: mocks.getProfileClientId, + profileExists: mocks.profileExists, + removeProfile: mocks.removeProfile, +})) + +vi.mock('@inquirer/prompts', () => ({ + input: vi.fn(), +})) + +vi.mock('../../src/lib/oauth.js', () => ({ + performLogin: mocks.performLogin, +})) + +const {BaseCommand} = await import('../../src/base-command.js') +const {default: Login} = await import('../../src/commands/login.js') +const {default: Logout} = await import('../../src/commands/logout.js') +const {default: ProfileAdd} = await import('../../src/commands/profile/add.js') +const {default: ProfileRemove} = await import('../../src/commands/profile/remove.js') + +function createCommand( + CommandClass: T, + parsed: {args?: Record; flags?: Record}, +): {run: () => Promise; log: ReturnType; error: ReturnType} { + const command = Object.create(CommandClass.prototype) as { + parse: ReturnType + run: () => Promise + log: ReturnType + error: ReturnType + } + command.parse = vi.fn().mockResolvedValue(parsed) + command.log = vi.fn() + command.error = vi.fn((message: string) => { + throw new Error(message) + }) + return command +} + +describe('OAuth cache lifecycle commands', () => { + beforeEach(() => { + vi.clearAllMocks() + mocks.getDefaultProfile.mockReturnValue(undefined) + }) + + it('uses a client-ID-scoped cache key for inline commands', () => { + const command = Object.create(BaseCommand.prototype) as { + resolveCredentials: (flags: {profile?: string; 'client-id'?: string}) => {profileName: string; clientId: string} + error: (message: string) => never + } + command.error = (message: string): never => { + throw new Error(message) + } + + expect(command.resolveCredentials({'client-id': 'client-a'})).toEqual({ + profileName: getInlineCredentialCacheKey('client-a'), + clientId: 'client-a', + }) + expect(command.resolveCredentials({'client-id': 'client-b'})).toEqual({ + profileName: getInlineCredentialCacheKey('client-b'), + clientId: 'client-b', + }) + }) + + it('keeps a named profile as the cache key when its client ID is overridden', () => { + const command = Object.create(BaseCommand.prototype) as { + resolveCredentials: (flags: {profile?: string; 'client-id'?: string}) => {profileName: string; clientId: string} + error: (message: string) => never + } + command.error = (message: string): never => { + throw new Error(message) + } + + expect(command.resolveCredentials({profile: 'acme', 'client-id': 'alternate-client'})).toEqual({ + profileName: 'acme', + clientId: 'alternate-client', + }) + }) + + it('keeps default-profile resolution unchanged when no client ID is supplied', () => { + mocks.getDefaultProfile.mockReturnValue('default-profile') + mocks.getProfileClientId.mockReturnValue('default-client') + const command = Object.create(BaseCommand.prototype) as { + resolveCredentials: (flags: {profile?: string; 'client-id'?: string}) => {profileName: string; clientId: string} + error: (message: string) => never + } + command.error = (message: string): never => { + throw new Error(message) + } + + expect(command.resolveCredentials({})).toEqual({ + profileName: 'default-profile', + clientId: 'default-client', + }) + }) + + it('stores each inline login under its client-ID-scoped key', async () => { + mocks.performLogin + .mockResolvedValueOnce({ + tokenSet: {access_token: 'token-a', refresh_token: 'refresh-a'}, + tenantId: 'tenant-a', + tenantName: 'Tenant A', + }) + .mockResolvedValueOnce({ + tokenSet: {access_token: 'token-b', refresh_token: 'refresh-b'}, + tenantId: 'tenant-b', + tenantName: 'Tenant B', + }) + + await createCommand(Login, {flags: {'client-id': 'client-a'}}).run() + await createCommand(Login, {flags: {'client-id': 'client-b'}}).run() + + expect(mocks.cacheTokenSet).toHaveBeenNthCalledWith( + 1, + getInlineCredentialCacheKey('client-a'), + expect.objectContaining({access_token: 'token-a'}), + 'tenant-a', + 'Tenant A', + ) + expect(mocks.cacheTokenSet).toHaveBeenNthCalledWith( + 2, + getInlineCredentialCacheKey('client-b'), + expect.objectContaining({access_token: 'token-b'}), + 'tenant-b', + 'Tenant B', + ) + }) + + it('logs out a scoped inline session and purges the ambiguous legacy key', async () => { + expect(Logout.flags['client-id']).toBeDefined() + const command = createCommand(Logout, {flags: {'client-id': 'client-a'}}) + + await command.run() + + expect(mocks.clearCachedToken).toHaveBeenNthCalledWith(1, getInlineCredentialCacheKey('client-a')) + expect(mocks.clearCachedToken).toHaveBeenNthCalledWith(2, LEGACY_INLINE_CACHE_KEY) + expect(command.log).toHaveBeenCalledWith( + 'Logged out from inline client credentials. Run "xero login" to re-authenticate.', + ) + }) + + it('continues to log out the default profile when no selector is supplied', async () => { + mocks.getDefaultProfile.mockReturnValue('default-profile') + const command = createCommand(Logout, {flags: {}}) + + await command.run() + + expect(mocks.clearCachedToken).toHaveBeenCalledTimes(1) + expect(mocks.clearCachedToken).toHaveBeenCalledWith('default-profile') + expect(command.log).toHaveBeenCalledWith( + 'Logged out from profile "default-profile". Run "xero login" to re-authenticate.', + ) + }) + + it('clears cached credentials before removing a named profile', async () => { + mocks.profileExists.mockReturnValue(true) + const command = createCommand(ProfileRemove, {args: {name: 'acme'}}) + + await command.run() + + expect(mocks.clearCachedToken).toHaveBeenCalledWith('acme') + expect(mocks.removeProfile).toHaveBeenCalledWith('acme') + expect(mocks.clearCachedToken.mock.invocationCallOrder[0]).toBeLessThan( + mocks.removeProfile.mock.invocationCallOrder[0], + ) + }) + + it('clears stale credentials when --force replaces a profile', async () => { + mocks.profileExists.mockReturnValue(true) + const command = createCommand(ProfileAdd, { + args: {name: 'acme'}, + flags: {force: true, 'client-id': 'replacement-client'}, + }) + + await command.run() + + expect(mocks.clearCachedToken).toHaveBeenCalledWith('acme') + expect(mocks.addProfile).toHaveBeenCalledWith('acme', 'replacement-client') + expect(mocks.clearCachedToken.mock.invocationCallOrder[0]).toBeLessThan( + mocks.addProfile.mock.invocationCallOrder[0], + ) + }) +}) diff --git a/test/lib/credential-cache-key.test.ts b/test/lib/credential-cache-key.test.ts new file mode 100644 index 0000000..1339556 --- /dev/null +++ b/test/lib/credential-cache-key.test.ts @@ -0,0 +1,39 @@ +import {describe, expect, it} from 'vitest' +import { + LEGACY_INLINE_CACHE_KEY, + getCredentialCacheKeysToClear, + getInlineCredentialCacheKey, + resolveCredentialCacheKey, +} from '../../src/lib/credential-cache-key.js' + +describe('credential cache keys', () => { + it('scopes inline credentials to their client ID', () => { + const firstKey = resolveCredentialCacheKey({'client-id': 'client-a'}) + const secondKey = resolveCredentialCacheKey({'client-id': 'client-b'}) + + expect(firstKey).toBe(getInlineCredentialCacheKey('client-a')) + expect(secondKey).toBe(getInlineCredentialCacheKey('client-b')) + expect(firstKey).not.toBe(secondKey) + expect(firstKey).not.toBe(LEGACY_INLINE_CACHE_KEY) + }) + + it('preserves named-profile cache keys when a client ID overrides the profile configuration', () => { + expect(resolveCredentialCacheKey({profile: 'acme', 'client-id': 'alternate-client'})).toBe('acme') + }) + + it('leaves default-profile selection to the caller when no selector is supplied', () => { + expect(resolveCredentialCacheKey({})).toBeUndefined() + }) + + it('clears only the named/default profile cache key', () => { + expect(getCredentialCacheKeysToClear({profile: 'acme'}, 'default')).toEqual(['acme']) + expect(getCredentialCacheKeysToClear({}, 'default')).toEqual(['default']) + }) + + it('clears the scoped inline key and the ambiguous legacy inline key on logout', () => { + expect(getCredentialCacheKeysToClear({'client-id': 'client-a'})).toEqual([ + getInlineCredentialCacheKey('client-a'), + LEGACY_INLINE_CACHE_KEY, + ]) + }) +})