Repository navigation
Read automation tokens through one selector #8728
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| }) | ||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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) | ||
| } | ||
|
Contributor
There was a problem hiding this comment. I don't understand, why the two functions? this one doesn't do anything differently 🤔
Contributor
There was a problem hiding this comment. Its scaffolding for the next PRs... getting ahead of itself slightly though lol
Author
There was a problem hiding this comment. True- we only need this split in #8731 once we make one of them throw. Most likely closing this as it isn't required here at all |
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the benefit in this interface design? Do we ever actually care about the
sourcereturned? I don't think so. I think if anything you should be able to specify which exact token you want back (since some flows require certain vars, mainly partners) and then app tokens can be aliased/rolled out together, where automation env var takes precedence, but the old env var will always be able to be used too. It would make ripping the partner token code out easier once we're able to as well.