Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/empty-automation-token-variable.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/cli-kit': patch
---

Fail with a clear error, instead of logging in with your Shopify account, when `SHOPIFY_APP_AUTOMATION_TOKEN` or `SHOPIFY_CLI_PARTNERS_TOKEN` is set but empty
1 change: 1 addition & 0 deletions packages/cli-kit/src/private/node/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ export const environmentVariables = {
env: 'SHOPIFY_CLI_ENV',
noAnalytics: 'SHOPIFY_CLI_NO_ANALYTICS',
optOutInstrumentation: 'OPT_OUT_INSTRUMENTATION',
organizationAutomationToken: 'SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN',
appAutomationToken: 'SHOPIFY_APP_AUTOMATION_TOKEN',
partnersToken: 'SHOPIFY_CLI_PARTNERS_TOKEN',
runAsUser: 'SHOPIFY_RUN_AS_USER',
Expand Down
33 changes: 33 additions & 0 deletions packages/cli-kit/src/private/node/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ 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 {automationTokenVariable, automationTokenVariablesProblem} 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'
Expand Down Expand Up @@ -113,6 +114,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')
Expand Down Expand Up @@ -153,6 +155,37 @@ beforeEach(() => {
})
})

describe('ensureAuthenticated with automation token variables', () => {
test('fails before logging in when the automation token variables are invalid', async () => {
// Given
vi.mocked(automationTokenVariablesProblem).mockReturnValue({
message: 'SHOPIFY_APP_AUTOMATION_TOKEN is set but empty.',
tryMessage: 'Set it to an automation token, or unset it to log in with your Shopify account.',
})

// When
const got = ensureAuthenticated(defaultApplications)

// Then
await expect(got).rejects.toThrow('SHOPIFY_APP_AUTOMATION_TOKEN is set but empty.')
expect(fetchSessions).not.toHaveBeenCalled()
expect(requestDeviceAuthorization).not.toHaveBeenCalled()
})

test('refuses to log in while the organization automation token is set', async () => {
// Given
vi.mocked(automationTokenVariable).mockReturnValue('SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN')

// When
const got = ensureAuthenticated(defaultApplications)

// Then
await expect(got).rejects.toThrow("This command can't use SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN.")
expect(fetchSessions).not.toHaveBeenCalled()
expect(requestDeviceAuthorization).not.toHaveBeenCalled()
})
})

describe('ensureAuthenticated when previous session is invalid', () => {
test('executes complete auth flow if there is no session', async () => {
// Given
Expand Down
10 changes: 9 additions & 1 deletion packages/cli-kit/src/private/node/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {automationTokenVariablesProblem} 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'
Expand All @@ -20,7 +21,7 @@ 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 {AdminSession, logout} from '../../public/node/session.js'
import {AdminSession, ensureNoOrganizationAutomationToken, logout} from '../../public/node/session.js'
import {nonRandomUUID} from '../../public/node/crypto.js'
import {isEmpty} from '../../public/common/object.js'
import {businessPlatformRequest} from '../../public/node/api/business-platform.js'
Expand Down Expand Up @@ -203,6 +204,13 @@ export async function ensureAuthenticated(
_env?: NodeJS.ProcessEnv,
{forceRefresh = false, noPrompt = false, forceNewSession = false}: EnsureAuthenticatedAdditionalOptions = {},
): Promise<OAuthSession> {
// Commands that support automation tokens exchange them before calling this function, and getAppAutomationToken
// returns no token when the variables are invalid, so an invalid environment always reaches this check before any
// login starts.
const variablesProblem = automationTokenVariablesProblem()
if (variablesProblem) throw new AbortError(variablesProblem.message, variablesProblem.tryMessage)
ensureNoOrganizationAutomationToken()

const fqdn = await identityFqdn()

const previousStoreFqdn = applications.adminApi?.storeFqdn
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
import {automationTokenVariable, automationTokenVariablesProblem} from './automation-token.js'
import {environmentVariables} from '../constants.js'
import {describe, expect, test} from 'vitest'

const ORGANIZATION = environmentVariables.organizationAutomationToken
const APP = environmentVariables.appAutomationToken
const PARTNERS = environmentVariables.partnersToken

describe('automationTokenVariable', () => {
test.each([
{name: 'no automation token variable', env: {}, expected: undefined},
{name: 'the organization variable', env: {[ORGANIZATION]: 'org-token'}, expected: ORGANIZATION},
{name: 'the app variable', env: {[APP]: 'app-token'}, expected: APP},
{name: 'the Partners variable', env: {[PARTNERS]: 'partners-token'}, expected: PARTNERS},
{name: 'the app variable over the Partners variable', env: {[APP]: 'a', [PARTNERS]: 'p'}, expected: APP},
{name: 'an empty app variable over the Partners variable', env: {[APP]: '', [PARTNERS]: 'p'}, expected: APP},
{name: 'the organization variable over the others', env: {[ORGANIZATION]: 'o', [APP]: 'a'}, expected: ORGANIZATION},
])('selects $name', ({env, expected}) => {
expect(automationTokenVariable(env)).toBe(expected)
})
})

describe('automationTokenVariablesProblem', () => {
test.each([
{name: 'no automation token variable', env: {}},
{name: 'only the organization variable', env: {[ORGANIZATION]: 'org-token'}},
{name: 'only the app variable', env: {[APP]: 'app-token'}},
{name: 'only the Partners variable', env: {[PARTNERS]: 'partners-token'}},
{name: 'both legacy variables', env: {[APP]: 'app-token', [PARTNERS]: 'partners-token'}},
{name: 'an empty Partners variable behind the app variable', env: {[APP]: 'app-token', [PARTNERS]: ''}},
])('accepts $name', ({env}) => {
expect(automationTokenVariablesProblem(env)).toBeUndefined()
})

test.each([
{
name: 'the organization variable with the app variable',
env: {[ORGANIZATION]: 'org-token', [APP]: 'app-token'},
message: `${ORGANIZATION} can't be set together with ${APP}.`,
},
{
name: 'the organization variable with the Partners variable',
env: {[ORGANIZATION]: 'org-token', [PARTNERS]: 'partners-token'},
message: `${ORGANIZATION} can't be set together with ${PARTNERS}.`,
},
{
name: 'the organization variable with both legacy variables',
env: {[ORGANIZATION]: 'org-token', [APP]: 'app-token', [PARTNERS]: 'partners-token'},
message: `${ORGANIZATION} can't be set together with ${APP} or ${PARTNERS}.`,
},
{
name: 'the organization variable with an empty app variable',
env: {[ORGANIZATION]: 'org-token', [APP]: ''},
message: `${ORGANIZATION} can't be set together with ${APP}.`,
},
{name: 'an empty organization variable', env: {[ORGANIZATION]: ''}, message: `${ORGANIZATION} is set but empty.`},
{name: 'an empty app variable', env: {[APP]: ''}, message: `${APP} is set but empty.`},
{
name: 'an empty app variable, even when the Partners variable has a value',
env: {[APP]: '', [PARTNERS]: 'partners-token'},
message: `${APP} is set but empty.`,
},
{name: 'an empty Partners variable', env: {[PARTNERS]: ''}, message: `${PARTNERS} is set but empty.`},
])('rejects $name', ({env, message}) => {
expect(automationTokenVariablesProblem(env)?.message).toBe(message)
})
})
57 changes: 57 additions & 0 deletions packages/cli-kit/src/private/node/session/automation-token.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import {environmentVariables} from '../constants.js'

const ORGANIZATION_TOKEN = environmentVariables.organizationAutomationToken
const APP_TOKEN = environmentVariables.appAutomationToken
const PARTNERS_TOKEN = environmentVariables.partnersToken

interface AutomationTokenVariablesProblem {
message: string
tryMessage: string
}

/**
* Returns the name of the automation token variable the CLI authenticates with: the first one that is set, in
* the order SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN, SHOPIFY_APP_AUTOMATION_TOKEN, SHOPIFY_CLI_PARTNERS_TOKEN.
*
* A variable set to an empty string still counts as set, so `automationTokenVariablesProblem` can report it.
*
* @param env - Environment variables to read.
* @returns The variable name, or undefined when none of them is set.
*/
export function automationTokenVariable(env: NodeJS.ProcessEnv = process.env): string | undefined {
return [ORGANIZATION_TOKEN, APP_TOKEN, PARTNERS_TOKEN].find((name) => env[name] !== undefined)
}

/**
* Explains why the automation token variables can't be used, so a misconfigured environment fails instead of
* falling back to the logged-in user.
*
* - SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN can't be set together with SHOPIFY_APP_AUTOMATION_TOKEN or
* SHOPIFY_CLI_PARTNERS_TOKEN.
* - The selected variable can't be empty.
*
* @param env - Environment variables to check.
* @returns The problem to report, or undefined when the variables can be used.
*/
export function automationTokenVariablesProblem(
env: NodeJS.ProcessEnv = process.env,
): AutomationTokenVariablesProblem | undefined {
if (env[ORGANIZATION_TOKEN] !== undefined) {
const conflictingVariables = [APP_TOKEN, PARTNERS_TOKEN].filter((name) => env[name] !== undefined)
if (conflictingVariables.length > 0) {
return {
message: `${ORGANIZATION_TOKEN} can't be set together with ${conflictingVariables.join(' or ')}.`,
tryMessage: `Unset ${conflictingVariables.join(' and ')}, or unset ${ORGANIZATION_TOKEN}.`,
}
}
}

const variable = automationTokenVariable(env)
if (variable && env[variable] === '') {
return {
message: `${variable} is set but empty.`,
tryMessage: 'Set it to an automation token, or unset it to log in with your Shopify account.',
}
}
return undefined
}
21 changes: 21 additions & 0 deletions packages/cli-kit/src/public/node/environment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {environmentVariables, systemEnvironmentVariables} from '../../private/no
import {describe, expect, test, beforeEach} from 'vitest'

beforeEach(() => {
delete process.env[environmentVariables.organizationAutomationToken]
delete process.env[environmentVariables.appAutomationToken]
delete process.env[environmentVariables.partnersToken]
delete process.env[systemEnvironmentVariables.backendPort]
Expand Down Expand Up @@ -32,6 +33,26 @@ describe('getAppAutomationToken', () => {
test('returns undefined when neither env var is set', () => {
expect(getAppAutomationToken()).toBeUndefined()
})

test('returns SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN when set', () => {
process.env[environmentVariables.organizationAutomationToken] = 'org-token'

expect(getAppAutomationToken()).toBe('org-token')
})

test('returns undefined when SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN is set together with another variable', () => {
process.env[environmentVariables.organizationAutomationToken] = 'org-token'
process.env[environmentVariables.appAutomationToken] = 'new-token'

expect(getAppAutomationToken()).toBeUndefined()
})

test('returns undefined when the selected variable is empty, even if a later one has a value', () => {
process.env[environmentVariables.appAutomationToken] = ''
process.env[environmentVariables.partnersToken] = 'old-token'

expect(getAppAutomationToken()).toBeUndefined()
})
})

describe('getBackendPort', () => {
Expand Down
14 changes: 10 additions & 4 deletions packages/cli-kit/src/public/node/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {nonRandomUUID} from './crypto.js'
import {isTruthy} from './context/utilities.js'
import {sniffForJson} from './path.js'
import {environmentVariables, systemEnvironmentVariables} from '../../private/node/constants.js'
import {automationTokenVariable, automationTokenVariablesProblem} from '../../private/node/session/automation-token.js'

/**
* It returns the environment variables of the environment
Expand All @@ -18,14 +19,19 @@ export function getEnvironmentVariables(): NodeJS.ProcessEnv {
}

/**
* Returns the value of the SHOPIFY_APP_AUTOMATION_TOKEN environment variable,
* falling back to the deprecated SHOPIFY_CLI_PARTNERS_TOKEN.
* Returns the automation token the CLI authenticates with, from the first of these variables that is set:
* SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN, SHOPIFY_APP_AUTOMATION_TOKEN, or the deprecated SHOPIFY_CLI_PARTNERS_TOKEN.
*
* @returns The app automation token value, or undefined if neither env var is set.
* Returns undefined when the variables can't be used (an empty value, or the organization variable set alongside
* another one). Callers then fall back to the login flow, which reports the problem instead of logging in.
*
* @returns The automation token, or undefined if there is no usable one.
*/
export function getAppAutomationToken(): string | undefined {
const env = getEnvironmentVariables()
return env[environmentVariables.appAutomationToken] ?? env[environmentVariables.partnersToken]
if (automationTokenVariablesProblem(env)) return undefined
const variable = automationTokenVariable(env)
return variable ? env[variable] : undefined
}

/**
Expand Down
22 changes: 22 additions & 0 deletions packages/cli-kit/src/public/node/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
ensureAuthenticatedPartners,
ensureAuthenticatedStorefront,
ensureAuthenticatedThemes,
ensureNoOrganizationAutomationToken,
findSessionIdByAlias,
setCurrentSessionAlias,
setLastSeenUserId,
Expand All @@ -21,6 +22,7 @@ import {
setLastSeenUserIdAfterAuth,
} from '../../private/node/session.js'
import * as sessionStore from '../../private/node/session/store.js'
import {automationTokenVariable} from '../../private/node/session/automation-token.js'
import {ApplicationToken} from '../../private/node/session/schema.js'
import {
exchangeCustomPartnerToken,
Expand All @@ -41,6 +43,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')

Expand All @@ -52,6 +55,25 @@ describe('store command analytics session helpers', () => {
})
})

describe('ensureNoOrganizationAutomationToken', () => {
test('fails while SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN is set', () => {
vi.mocked(automationTokenVariable).mockReturnValue('SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN')

expect(() => ensureNoOrganizationAutomationToken()).toThrow(
"This command can't use SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN.",
)
})

test.each([undefined, 'SHOPIFY_APP_AUTOMATION_TOKEN', 'SHOPIFY_CLI_PARTNERS_TOKEN'])(
'does nothing when the automation token variable is %s',
(variable) => {
vi.mocked(automationTokenVariable).mockReturnValue(variable)

expect(() => ensureNoOrganizationAutomationToken()).not.toThrow()
},
)
})

describe('findSessionIdByAlias', () => {
test('returns the matching session ID without selecting it', async () => {
// Given
Expand Down
18 changes: 18 additions & 0 deletions packages/cli-kit/src/public/node/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ 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 {automationTokenVariable} from '../../private/node/session/automation-token.js'
import {environmentVariables} from '../../private/node/constants.js'
import {
exchangeCustomPartnerToken,
exchangeAppAutomationTokenForAppManagementAccessToken,
Expand Down Expand Up @@ -341,6 +343,22 @@ ${outputToken.json(scopes)}
return tokens.businessPlatform
}

/**
* Fails when SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN is set, for commands that can't run with that token.
*
* Organization automation tokens can't log in as a user or call a store's Admin API, so commands that need either
* refuse to run instead of quietly using the person's own login. `ensureAuthenticated` runs this check before any
* login. Commands that run on a login saved by `shopify store auth` call it before loading that login.
*
* @throws AbortError when SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN is set.
*/
export function ensureNoOrganizationAutomationToken(): void {
const variable = environmentVariables.organizationAutomationToken
if (automationTokenVariable() !== variable) return

throw new AbortError(`This command can't use ${variable}.`, `Unset ${variable} to run it with your Shopify account.`)
}

/**
* Logout from Shopify.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {loadAdminSessionFromStoreAuth} from './admin-session.js'
import {loadStoredStoreSession} from './session-lifecycle.js'
import {recordStoreFqdnMetadata} from '../attribution.js'
import {setLastSeenUserId} from '@shopify/cli-kit/node/session'
import {ensureNoOrganizationAutomationToken, setLastSeenUserId} from '@shopify/cli-kit/node/session'
import {AbortError} from '@shopify/cli-kit/node/error'
import {describe, expect, test, vi} from 'vitest'

vi.mock('./session-lifecycle.js')
Expand Down Expand Up @@ -44,4 +45,15 @@ describe('loadAdminSessionFromStoreAuth', () => {
expect(recordStoreFqdnMetadata).not.toHaveBeenCalled()
expect(setLastSeenUserId).not.toHaveBeenCalled()
})

test('refuses to use stored store auth while the organization automation token is set', async () => {
vi.mocked(ensureNoOrganizationAutomationToken).mockImplementation(() => {
throw new AbortError("This command can't use SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN.")
})

await expect(loadAdminSessionFromStoreAuth('shop.myshopify.com')).rejects.toThrow(
"This command can't use SHOPIFY_ORGANIZATION_AUTOMATION_TOKEN.",
)
expect(loadStoredStoreSession).not.toHaveBeenCalled()
})
})
Loading
Loading