diff --git a/packages/cli-kit/src/private/node/session.test.ts b/packages/cli-kit/src/private/node/session.test.ts index d781fff189a..5cd99ce1a33 100644 --- a/packages/cli-kit/src/private/node/session.test.ts +++ b/packages/cli-kit/src/private/node/session.test.ts @@ -20,12 +20,12 @@ import {ApplicationToken, IdentityToken, Sessions} from './session/schema.js' import {validateSession} from './session/validate.js' import {applicationId} from './session/identity.js' import {pollForDeviceAuthorization, requestDeviceAuthorization} from './session/device-authorization.js' +import {getAutomationToken, peekAutomationToken} from './session/automation-token.js' import {getCurrentSessionId, setCurrentSessionId} from './conf-store.js' import * as fqdnModule from '../../public/node/context/fqdn.js' import {themeToken} from '../../public/node/context/local.js' import {partnersRequest} from '../../public/node/api/partners.js' import {businessPlatformRequest} from '../../public/node/api/business-platform.js' -import {getAppAutomationToken} from '../../public/node/environment.js' import {nonRandomUUID} from '../../public/node/crypto.js' import {terminalSupportsPrompting} from '../../public/node/system.js' @@ -113,6 +113,7 @@ vi.mock('./session/exchange') vi.mock('./session/scopes') vi.mock('./session/store') vi.mock('./session/validate') +vi.mock('./session/automation-token') vi.mock('../../public/node/api/partners.js') vi.mock('../../public/node/api/business-platform.js') vi.mock('../../store') @@ -347,7 +348,8 @@ describe('when existing session is valid', () => { // Given vi.mocked(validateSession).mockResolvedValueOnce('ok') vi.mocked(fetchSessions).mockResolvedValue(validSessions) - vi.mocked(getAppAutomationToken).mockReturnValue('custom_cli_token') + vi.mocked(getAutomationToken).mockReturnValue({value: 'custom_cli_token', source: 'app'}) + vi.mocked(peekAutomationToken).mockReturnValue({value: 'custom_cli_token', source: 'app'}) const expected = {...validTokens, partners: 'custom_partners_token'} // When @@ -505,7 +507,7 @@ describe('getLastSeenUserIdAfterAuth', () => { test('returns UUID based on partners token if present in environment', async () => { // Given vi.mocked(getCurrentSessionId).mockReturnValue(undefined) - vi.mocked(getAppAutomationToken).mockReturnValue('partners-token-456') + vi.mocked(peekAutomationToken).mockReturnValue({value: 'partners-token-456', source: 'partners'}) // When const userId = await getLastSeenUserIdAfterAuth() @@ -595,7 +597,7 @@ describe('setLastSeenUserIdAfterAuth', () => { describe('getLastSeenAuthMethod', () => { beforeEach(() => { vi.mocked(getCurrentSessionId).mockReturnValue(undefined) - vi.mocked(getAppAutomationToken).mockReturnValue(undefined) + vi.mocked(peekAutomationToken).mockReturnValue(undefined) vi.mocked(themeToken).mockReturnValue(undefined) setLastSeenAuthMethod('none') }) @@ -626,7 +628,7 @@ describe('getLastSeenAuthMethod', () => { test('returns partners_token if there is a partners token in the environment', async () => { // Given - vi.mocked(getAppAutomationToken).mockReturnValue('partners-token-456') + vi.mocked(peekAutomationToken).mockReturnValue({value: 'partners-token-456', source: 'partners'}) // When const method = await getLastSeenAuthMethod() diff --git a/packages/cli-kit/src/private/node/session.ts b/packages/cli-kit/src/private/node/session.ts index 3eb9f9e5ee6..b6798529f39 100644 --- a/packages/cli-kit/src/private/node/session.ts +++ b/packages/cli-kit/src/private/node/session.ts @@ -12,6 +12,7 @@ import { import {IdentityToken, Session, Sessions} from './session/schema.js' import * as sessionStore from './session/store.js' import {pollForDeviceAuthorization, requestDeviceAuthorization} from './session/device-authorization.js' +import {getAutomationToken, peekAutomationToken} from './session/automation-token.js' import {isThemeAccessSession} from './api/rest.js' import {getCurrentSessionId, setCurrentSessionId} from './conf-store.js' import {UserEmailQueryString, UserEmailQuery} from './api/graphql/business-platform-destinations/user-email.js' @@ -19,7 +20,7 @@ import {outputContent, outputToken, outputDebug, outputCompleted} from '../../pu import {themeToken} from '../../public/node/context/local.js' import {AbortError} from '../../public/node/error.js' import {normalizeStoreFqdn, identityFqdn} from '../../public/node/context/fqdn.js' -import {getIdentityTokenInformation, getAppAutomationToken} from '../../public/node/environment.js' +import {getIdentityTokenInformation} from '../../public/node/environment.js' import {AdminSession, logout} from '../../public/node/session.js' import {nonRandomUUID} from '../../public/node/crypto.js' import {isEmpty} from '../../public/common/object.js' @@ -133,7 +134,7 @@ let commandSessionId: string | undefined * @returns A Promise that resolves to the user ID as a string. */ export async function getLastSeenUserIdAfterAuth(): Promise { - const customToken = getAppAutomationToken() ?? themeToken() + const customToken = peekAutomationToken()?.value ?? themeToken() if (customToken) return nonRandomUUID(customToken) if (userId) return userId @@ -165,8 +166,7 @@ export async function getLastSeenAuthMethod(): Promise { if (getCurrentSessionId()) return 'device_auth' - const appAutomationToken = getAppAutomationToken() - if (appAutomationToken) return 'partners_token' + if (peekAutomationToken()) return 'partners_token' const themePassword = themeToken() if (themePassword) { @@ -270,12 +270,12 @@ ${outputToken.json(applications)} const tokens = await tokensFor(applications, completeSession) - const envToken = getAppAutomationToken() - if (envToken && applications.partnersApi) { - tokens.partners = (await exchangeCustomPartnerToken(envToken)).accessToken + const automationToken = getAutomationToken() + if (automationToken && applications.partnersApi) { + tokens.partners = (await exchangeCustomPartnerToken(automationToken.value)).accessToken } - setLastSeenAuthMethod(envToken ? 'partners_token' : 'device_auth') + setLastSeenAuthMethod(automationToken ? 'partners_token' : 'device_auth') setLastSeenUserIdAfterAuth(tokens.userId) return tokens } diff --git a/packages/cli-kit/src/private/node/session/automation-token.test.ts b/packages/cli-kit/src/private/node/session/automation-token.test.ts new file mode 100644 index 00000000000..52359ccc594 --- /dev/null +++ b/packages/cli-kit/src/private/node/session/automation-token.test.ts @@ -0,0 +1,40 @@ +import {getAutomationToken, peekAutomationToken} from './automation-token.js' +import {environmentVariables} from '../constants.js' +import {describe, expect, test} from 'vitest' + +const APP = environmentVariables.appAutomationToken +const PARTNERS = environmentVariables.partnersToken + +const cases = [ + {name: 'no automation token variable', env: {}, expected: undefined}, + {name: 'only the app variable', env: {[APP]: 'app-token'}, expected: {value: 'app-token', source: 'app'}}, + { + name: 'only the Partners variable', + env: {[PARTNERS]: 'partners-token'}, + expected: {value: 'partners-token', source: 'partners'}, + }, + { + name: 'both variables, preferring the app variable', + env: {[APP]: 'app-token', [PARTNERS]: 'partners-token'}, + expected: {value: 'app-token', source: 'app'}, + }, + {name: 'an empty app variable', env: {[APP]: ''}, expected: undefined}, + { + name: 'an empty app variable, which still hides the Partners variable', + env: {[APP]: '', [PARTNERS]: 'partners-token'}, + expected: undefined, + }, + {name: 'an empty Partners variable', env: {[PARTNERS]: ''}, expected: undefined}, +] + +describe('peekAutomationToken', () => { + test.each(cases)('reads $name', ({env, expected}) => { + expect(peekAutomationToken(env)).toEqual(expected) + }) +}) + +describe('getAutomationToken', () => { + test.each(cases)('selects $name', ({env, expected}) => { + expect(getAutomationToken(env)).toEqual(expected) + }) +}) diff --git a/packages/cli-kit/src/private/node/session/automation-token.ts b/packages/cli-kit/src/private/node/session/automation-token.ts new file mode 100644 index 00000000000..e2abfb24cbd --- /dev/null +++ b/packages/cli-kit/src/private/node/session/automation-token.ts @@ -0,0 +1,41 @@ +import {environmentVariables} from '../constants.js' + +/** + * The environment variable an automation token was read from. + */ +type AutomationTokenSource = 'app' | 'partners' + +interface AutomationToken { + value: string + source: AutomationTokenSource +} + +/** + * Reads the automation token from the environment without validating it, so it never throws. + * + * Telemetry reads the token through this function because reporting must not fail a command. + * Authentication reads it through `getAutomationToken`. + * + * SHOPIFY_APP_AUTOMATION_TOKEN takes precedence over the deprecated SHOPIFY_CLI_PARTNERS_TOKEN whenever it is + * set, even to an empty value. An empty value means no token. + * + * @param env - Environment variables to read the token from. + * @returns The automation token and the variable it came from, or undefined when there is none. + */ +export function peekAutomationToken(env: NodeJS.ProcessEnv = process.env): AutomationToken | undefined { + const appToken = env[environmentVariables.appAutomationToken] + if (appToken !== undefined) return appToken === '' ? undefined : {value: appToken, source: 'app'} + + const partnersToken = env[environmentVariables.partnersToken] + return partnersToken ? {value: partnersToken, source: 'partners'} : undefined +} + +/** + * Selects the automation token to authenticate with. + * + * @param env - Environment variables to read the token from. + * @returns The automation token and the variable it came from, or undefined when there is none. + */ +export function getAutomationToken(env: NodeJS.ProcessEnv = process.env): AutomationToken | undefined { + return peekAutomationToken(env) +} diff --git a/packages/cli-kit/src/public/node/session.test.ts b/packages/cli-kit/src/public/node/session.test.ts index 449992bb4b8..96b717d40bd 100644 --- a/packages/cli-kit/src/public/node/session.test.ts +++ b/packages/cli-kit/src/public/node/session.test.ts @@ -12,7 +12,6 @@ import { } from './session.js' import {nonRandomUUID} from './crypto.js' -import {getAppAutomationToken} from './environment.js' import {shopifyFetch} from './http.js' import { ensureAuthenticated, @@ -21,6 +20,7 @@ import { setLastSeenUserIdAfterAuth, } from '../../private/node/session.js' import * as sessionStore from '../../private/node/session/store.js' +import {getAutomationToken} from '../../private/node/session/automation-token.js' import {ApplicationToken} from '../../private/node/session/schema.js' import { exchangeCustomPartnerToken, @@ -41,6 +41,7 @@ const partnersToken: ApplicationToken = { vi.mock('../../private/node/session.js') vi.mock('../../private/node/session/exchange.js') vi.mock('../../private/node/session/store.js') +vi.mock('../../private/node/session/automation-token.js') vi.mock('./environment.js') vi.mock('./http.js') @@ -198,13 +199,14 @@ describe('ensureAuthenticatedPartners', () => { accessToken: partnersToken.accessToken, userId: '575e2102-cb13-7bea-4631-ce3469eac491cdcba07d', }) - vi.mocked(getAppAutomationToken).mockReturnValue('custom_cli_token') + vi.mocked(getAutomationToken).mockReturnValue({value: 'custom_cli_token', source: 'app'}) // When const got = await ensureAuthenticatedPartners([]) // Then expect(got).toEqual({token: 'custom_partners_token', userId: '575e2102-cb13-7bea-4631-ce3469eac491cdcba07d'}) + expect(exchangeCustomPartnerToken).toHaveBeenCalledWith('custom_cli_token') expect(ensureAuthenticated).not.toHaveBeenCalled() }) }) @@ -343,7 +345,7 @@ describe('ensureAuthenticatedAppManagementAndBusinessPlatform', () => { test('returns app managment and business platform tokens if CLI token envvar is defined', async () => { // Given - vi.mocked(getAppAutomationToken).mockReturnValue('custom_cli_token') + vi.mocked(getAutomationToken).mockReturnValue({value: 'custom_cli_token', source: 'app'}) vi.mocked(exchangeAppAutomationTokenForAppManagementAccessToken).mockResolvedValueOnce({ accessToken: 'app-management-token', userId: '575e2102-cb13-7bea-4631-ce3469eac491cdcba07d', @@ -362,6 +364,8 @@ describe('ensureAuthenticatedAppManagementAndBusinessPlatform', () => { userId: '575e2102-cb13-7bea-4631-ce3469eac491cdcba07d', businessPlatformToken: 'business-platform-token', }) + expect(exchangeAppAutomationTokenForAppManagementAccessToken).toHaveBeenCalledWith('custom_cli_token') + expect(exchangeAppAutomationTokenForBusinessPlatformAccessToken).toHaveBeenCalledWith('custom_cli_token') expect(ensureAuthenticated).not.toHaveBeenCalled() }) }) diff --git a/packages/cli-kit/src/public/node/session.ts b/packages/cli-kit/src/public/node/session.ts index 15be5d39cbf..d68cb4e636f 100644 --- a/packages/cli-kit/src/public/node/session.ts +++ b/packages/cli-kit/src/public/node/session.ts @@ -1,9 +1,9 @@ import {shopifyFetch} from './http.js' import {nonRandomUUID} from './crypto.js' -import {getAppAutomationToken} from './environment.js' import {AbortError, BugError} from './error.js' import {outputContent, outputToken, outputDebug} from './output.js' import * as sessionStore from '../../private/node/session/store.js' +import {getAutomationToken} from '../../private/node/session/automation-token.js' import { exchangeCustomPartnerToken, exchangeAppAutomationTokenForAppManagementAccessToken, @@ -151,9 +151,9 @@ export async function ensureAuthenticatedPartners( outputDebug(outputContent`Ensuring that the user is authenticated with the Partners API with the following scopes: ${outputToken.json(scopes)} `) - const envToken = getAppAutomationToken() - if (envToken) { - const result = await exchangeCustomPartnerToken(envToken) + const automationToken = getAutomationToken() + if (automationToken) { + const result = await exchangeCustomPartnerToken(automationToken.value) return {token: result.accessToken, userId: result.userId} } const tokens = await ensureAuthenticated({partnersApi: {scopes}}, env, options) @@ -182,10 +182,10 @@ export async function ensureAuthenticatedAppManagementAndBusinessPlatform( ${outputToken.json(appManagementScopes)} `) - const envToken = getAppAutomationToken() - if (envToken) { - const appManagmentToken = await exchangeAppAutomationTokenForAppManagementAccessToken(envToken) - const businessPlatformToken = await exchangeAppAutomationTokenForBusinessPlatformAccessToken(envToken) + const automationToken = getAutomationToken() + if (automationToken) { + const appManagmentToken = await exchangeAppAutomationTokenForAppManagementAccessToken(automationToken.value) + const businessPlatformToken = await exchangeAppAutomationTokenForBusinessPlatformAccessToken(automationToken.value) return { appManagementToken: appManagmentToken.accessToken,