diff --git a/.changeset/app-automation-token-deprecation.md b/.changeset/app-automation-token-deprecation.md new file mode 100644 index 00000000000..9c7e5697182 --- /dev/null +++ b/.changeset/app-automation-token-deprecation.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': minor +--- + +Warn that `SHOPIFY_APP_AUTOMATION_TOKEN` and `SHOPIFY_CLI_PARTNERS_TOKEN` are deprecated in favor of `SHOPIFY_SERVICE_ACCOUNT_TOKEN`, which takes the same tokens. The warning shows at most once a day, and both variables keep working for now. 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 index 3769e50fba1..2c640b1c66e 100644 --- a/packages/cli-kit/src/private/node/session/automation-token.test.ts +++ b/packages/cli-kit/src/private/node/session/automation-token.test.ts @@ -2,6 +2,7 @@ import { automationTokenAuthMethod, automationTokenVariable, automationTokenVariablesProblem, + deprecatedAutomationTokenVariable, } from './automation-token.js' import {environmentVariables} from '../constants.js' import {describe, expect, test} from 'vitest' @@ -47,6 +48,25 @@ describe('automationTokenAuthMethod', () => { }) }) +describe('deprecatedAutomationTokenVariable', () => { + test.each([ + {name: 'the app variable', env: {[APP]: 'a'}, expected: APP}, + {name: 'the app variable over the Partners variable', env: {[APP]: 'a', [PARTNERS]: 'p'}, expected: APP}, + {name: 'the Partners variable', env: {[PARTNERS]: 'p'}, expected: PARTNERS}, + {name: 'no automation token variable', env: {}, expected: undefined}, + {name: 'the service account variable', env: {[SERVICE_ACCOUNT]: 's'}, expected: undefined}, + {name: 'an empty app variable', env: {[APP]: ''}, expected: undefined}, + {name: 'an empty Partners variable', env: {[PARTNERS]: ''}, expected: undefined}, + { + name: 'the app variable set together with the service account variable', + env: {[SERVICE_ACCOUNT]: 's', [APP]: 'a'}, + expected: undefined, + }, + ])('returns $expected for $name', ({env, expected}) => { + expect(deprecatedAutomationTokenVariable(env)).toBe(expected) + }) +}) + describe('automationTokenVariablesProblem', () => { test.each([ {name: 'no automation token variable', env: {}}, diff --git a/packages/cli-kit/src/private/node/session/automation-token.ts b/packages/cli-kit/src/private/node/session/automation-token.ts index e6ebd58a53d..fbf727d0aa6 100644 --- a/packages/cli-kit/src/private/node/session/automation-token.ts +++ b/packages/cli-kit/src/private/node/session/automation-token.ts @@ -36,6 +36,22 @@ export function automationTokenAuthMethod( return automationTokenVariable(env) === SERVICE_ACCOUNT_TOKEN ? 'service_account_token' : 'partners_token' } +/** + * Returns the deprecated automation token variable the CLI authenticates with: SHOPIFY_APP_AUTOMATION_TOKEN or + * SHOPIFY_CLI_PARTNERS_TOKEN. Both are deprecated in favor of SHOPIFY_SERVICE_ACCOUNT_TOKEN, which takes the same + * tokens. + * + * Returns undefined when the variables can't be used, since the CLI reports that problem instead. + * + * @param env - Environment variables to read. + * @returns The deprecated variable in use, or undefined. + */ +export function deprecatedAutomationTokenVariable(env: NodeJS.ProcessEnv = process.env): string | undefined { + if (automationTokenVariablesProblem(env)) return undefined + const variable = automationTokenVariable(env) + return variable === APP_TOKEN || variable === PARTNERS_TOKEN ? variable : 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. diff --git a/packages/cli-kit/src/public/node/base-command.test.ts b/packages/cli-kit/src/public/node/base-command.test.ts index 21c0b43370b..94d271d9b4d 100644 --- a/packages/cli-kit/src/public/node/base-command.test.ts +++ b/packages/cli-kit/src/public/node/base-command.test.ts @@ -9,10 +9,22 @@ import {mockAndCaptureOutput} from './testing/output.js' import {unstyled} from './output.js' import {defineJsonOutputSchema} from './json-output-schema.js' import {zod} from './schema.js' +import {runAtMinimumInterval} from '../../private/node/conf-store.js' import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest' import {Flags} from '@oclif/core' import {Ajv} from 'ajv' +vi.mock('../../private/node/conf-store.js', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + runAtMinimumInterval: vi.fn(async (_key: string, _interval: unknown, task: () => Promise) => { + await task() + return true + }), + } +}) + let originalStdinIsTTY: boolean | undefined let originalStderrIsTTY: boolean | undefined @@ -260,6 +272,25 @@ describe('command events', () => { }) }) + test('reports the SHOPIFY_APP_AUTOMATION_TOKEN deprecation as a JSON diagnostic for JSON commands', async () => { + const outputMock = mockAndCaptureOutput() + outputMock.clear() + vi.stubEnv('SHOPIFY_APP_AUTOMATION_TOKEN', 'app-token') + + await MockCommandWithEvents.run(['--json']) + + const events = outputMock + .info() + .split('\n') + .map((line) => JSON.parse(line)) + expect(events).toContainEqual({ + type: 'diagnostic', + level: 'warning', + timestamp: expect.any(String), + message: expect.stringContaining('SHOPIFY_APP_AUTOMATION_TOKEN is deprecated.'), + }) + }) + test('does not duplicate events already displayed by the text UI', async () => { const outputMock = mockAndCaptureOutput() outputMock.clear() @@ -856,6 +887,86 @@ describe('applying environments', async () => { `) }) + test('shows a deprecation warning when SHOPIFY_APP_AUTOMATION_TOKEN is set', async () => { + // Given + const outputMock = mockAndCaptureOutput() + outputMock.clear() + vi.stubEnv('SHOPIFY_APP_AUTOMATION_TOKEN', 'app-token') + + // When + await MockCommand.run() + + // Then + expect(outputMock.warn()).toMatchInlineSnapshot(` + "╭─ warning ────────────────────────────────────────────────────────────────────╮ + │ │ + │ SHOPIFY_APP_AUTOMATION_TOKEN is deprecated. │ + │ │ + │ Move the token to SHOPIFY_SERVICE_ACCOUNT_TOKEN. │ + │ SHOPIFY_APP_AUTOMATION_TOKEN still works for now, but a future release │ + │ will stop reading it. │ + │ │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + " + `) + }) + + test('shows a deprecation warning when SHOPIFY_CLI_PARTNERS_TOKEN is set', async () => { + // Given + const outputMock = mockAndCaptureOutput() + outputMock.clear() + vi.stubEnv('SHOPIFY_CLI_PARTNERS_TOKEN', 'partners-token') + + // When + await MockCommand.run() + + // Then + expect(outputMock.warn()).toMatchInlineSnapshot(` + "╭─ warning ────────────────────────────────────────────────────────────────────╮ + │ │ + │ SHOPIFY_CLI_PARTNERS_TOKEN is deprecated. │ + │ │ + │ Move the token to SHOPIFY_SERVICE_ACCOUNT_TOKEN. │ + │ SHOPIFY_CLI_PARTNERS_TOKEN still works for now, but a future release will │ + │ stop reading it. │ + │ │ + ╰──────────────────────────────────────────────────────────────────────────────╯ + " + `) + }) + + test('shows the deprecation warning at most once a day', async () => { + // Given + const outputMock = mockAndCaptureOutput() + outputMock.clear() + vi.stubEnv('SHOPIFY_APP_AUTOMATION_TOKEN', 'app-token') + vi.mocked(runAtMinimumInterval).mockResolvedValueOnce(false) + + // When + await MockCommand.run() + + // Then + expect(runAtMinimumInterval).toHaveBeenCalledWith( + 'warn-on-deprecated-automation-token-variable', + {days: 1}, + expect.any(Function), + ) + expect(outputMock.warn()).toMatchInlineSnapshot('""') + }) + + test('does not show the deprecation warning when SHOPIFY_SERVICE_ACCOUNT_TOKEN is set', async () => { + // Given + const outputMock = mockAndCaptureOutput() + outputMock.clear() + vi.stubEnv('SHOPIFY_SERVICE_ACCOUNT_TOKEN', 'service-account-token') + + // When + await MockCommand.run() + + // Then + expect(outputMock.warn()).toMatchInlineSnapshot('""') + }) + test('does not show a warning about NPM separator when no flag matches a NPM config env variable', async () => { // Given const outputMock = mockAndCaptureOutput() diff --git a/packages/cli-kit/src/public/node/base-command.ts b/packages/cli-kit/src/public/node/base-command.ts index ad894ebb811..26042f982d0 100644 --- a/packages/cli-kit/src/public/node/base-command.ts +++ b/packages/cli-kit/src/public/node/base-command.ts @@ -10,6 +10,9 @@ import {isTruthy} from './context/utilities.js' import {setCurrentCommandId} from './global-context.js' import {type JsonOutputSchema} from './json-output-schema.js' import {JsonMap} from '../../private/common/json.js' +import {environmentVariables} from '../../private/node/constants.js' +import {deprecatedAutomationTokenVariable} from '../../private/node/session/automation-token.js' +import {runAtMinimumInterval} from '../../private/node/conf-store.js' import {underscore} from '../common/string.js' import {Command, Config, Errors, Flags} from '@oclif/core' import {OutputFlags, Input, ParserOutput, FlagInput, OutputArgs} from '@oclif/core/parser' @@ -94,6 +97,7 @@ abstract class BaseCommand extends Command { } await removeDuplicatedPlugins(this.config) this.showNpmFlagWarning() + await this.showDeprecatedAutomationTokenVariableWarning() const {showNotificationsIfNeeded} = await import('./notifications-system.js') await showNotificationsIfNeeded() return super.init() @@ -122,6 +126,22 @@ abstract class BaseCommand extends Command { } } + // SHOPIFY_APP_AUTOMATION_TOKEN and SHOPIFY_CLI_PARTNERS_TOKEN keep working, but SHOPIFY_SERVICE_ACCOUNT_TOKEN replaces + // them and takes the same tokens. This warning is how people still using an old name find out about the new one. + protected async showDeprecatedAutomationTokenVariableWarning(): Promise { + const variable = deprecatedAutomationTokenVariable() + if (!variable) return + + // Show the warning only once per day + await runAtMinimumInterval('warn-on-deprecated-automation-token-variable', {days: 1}, async () => { + const {renderWarning} = await import('./ui.js') + renderWarning({ + headline: `${variable} is deprecated.`, + body: `Move the token to ${environmentVariables.serviceAccountToken}. ${variable} still works for now, but a future release will stop reading it.`, + }) + }) + } + protected exitWithTimestampWhenEnvVariablePresent() { if (isTruthy(process.env.SHOPIFY_CLI_ENV_STARTUP_PERFORMANCE_RUN)) { outputResult(` diff --git a/packages/cli-kit/src/public/node/environment.ts b/packages/cli-kit/src/public/node/environment.ts index 77b7133da8d..e6f92bdfb81 100644 --- a/packages/cli-kit/src/public/node/environment.ts +++ b/packages/cli-kit/src/public/node/environment.ts @@ -20,7 +20,7 @@ export function getEnvironmentVariables(): NodeJS.ProcessEnv { /** * Returns the automation token the CLI authenticates with, from the first of these variables that is set: - * SHOPIFY_SERVICE_ACCOUNT_TOKEN, SHOPIFY_APP_AUTOMATION_TOKEN, or the deprecated SHOPIFY_CLI_PARTNERS_TOKEN. + * SHOPIFY_SERVICE_ACCOUNT_TOKEN, or the deprecated SHOPIFY_APP_AUTOMATION_TOKEN and SHOPIFY_CLI_PARTNERS_TOKEN. * * Returns undefined when the variables can't be used (an empty value, or the service account variable set alongside * another one). Callers then fall back to the login flow, which reports the problem instead of logging in.