Skip to content
Closed
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
6 changes: 6 additions & 0 deletions .changeset/session-account-details.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/cli-kit': minor
'@shopify/cli': minor
---

Add selected user ID and email to auth login JSON while preserving the alias-only session API.
46 changes: 46 additions & 0 deletions packages/cli-kit/src/private/node/session.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ describe('ensureAuthenticated when previous session is invalid', () => {
// Verify the session was stored with email as alias
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
expect(storedSession[fqdn]![userId]!.identity.alias).toBe('user@example.com')
expect(storedSession[fqdn]![userId]!.identity.email).toBe('user@example.com')

// The userID is cached in memory and the secureStore is not accessed again
await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
Expand Down Expand Up @@ -211,6 +212,7 @@ The CLI is currently unable to prompt for reauthentication.`,
identity: {
...validIdentityToken,
alias: 'user@example.com',
email: 'user@example.com',
},
applications: appTokens,
},
Expand Down Expand Up @@ -687,6 +689,8 @@ describe('ensureAuthenticated email fetch functionality', () => {
// Then
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
expect(storedSession[fqdn]![userId]!.identity.alias).toBe('work@example.com')
expect(storedSession[fqdn]![userId]!.identity.email).toBe('work@example.com')
expect(businessPlatformRequest).toHaveBeenCalledTimes(1)
expect(got).toEqual(validTokens)
})

Expand Down Expand Up @@ -788,6 +792,48 @@ describe('ensureAuthenticated email fetch functionality', () => {
// Then
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
expect(storedSession[fqdn]![userId]!.identity.alias).toBe(userId)
expect(storedSession[fqdn]![userId]!.identity.email).toBeUndefined()
expect(got).toEqual(validTokens)
})
})

describe('stored authentication email', () => {
test('preserves a separately stored email through refresh without another request', async () => {
const sessions: Sessions = {
[fqdn]: {
[userId]: {
identity: {...validIdentityToken, alias: 'Work Account', email: 'work@example.com'},
applications: appTokens,
},
},
}
vi.mocked(fetchSessions).mockResolvedValue(sessions)
vi.mocked(validateSession).mockResolvedValueOnce('needs_refresh')

await ensureAuthenticated(defaultApplications)

const stored = vi.mocked(storeSessions).mock.calls[0]![0]
expect(stored[fqdn]![userId]!.identity).toMatchObject({alias: 'Work Account', email: 'work@example.com'})
expect(businessPlatformRequest).not.toHaveBeenCalled()
})

test('does not copy a cached email when reauthentication selects a different user', async () => {
vi.mocked(fetchSessions).mockResolvedValue({
[fqdn]: {
[userId]: {
identity: {...validIdentityToken, alias: 'Work Account', email: 'previous@example.com'},
applications: appTokens,
},
},
})
vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth')
vi.mocked(pollForDeviceAuthorization).mockResolvedValue({...validIdentityToken, userId: 'different-user'})

await ensureAuthenticated(defaultApplications)

const stored = vi.mocked(storeSessions).mock.calls[0]![0]
expect(stored[fqdn]!['different-user']!.identity.alias).toBe('Work Account')
expect(stored[fqdn]!['different-user']!.identity.email).toBeUndefined()
expect(businessPlatformRequest).not.toHaveBeenCalled()
})
})
37 changes: 28 additions & 9 deletions packages/cli-kit/src/private/node/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,15 +237,15 @@ ${outputToken.json(applications)}
if (validationResult === 'needs_full_auth') {
await throwOnNoPrompt(noPrompt)
outputDebug(outputContent`Initiating the full authentication flow...`)
newSession = await executeCompleteFlow(applications, currentSession?.identity.alias)
newSession = await executeCompleteFlow(applications, currentSession?.identity)
} else if (validationResult === 'needs_refresh' || forceRefresh) {
outputDebug(outputContent`The current session is valid but needs refresh. Refreshing...`)
try {
newSession = await refreshTokens(currentSession!, applications)
} catch (error) {
if (error instanceof InvalidGrantError) {
await throwOnNoPrompt(noPrompt)
newSession = await executeCompleteFlow(applications, currentSession?.identity.alias)
newSession = await executeCompleteFlow(applications, currentSession?.identity)
} else if (error instanceof InvalidRequestError) {
await sessionStore.remove()
throw new AbortError('\nError validating auth session', "We've cleared the current session, please try again")
Expand All @@ -255,8 +255,15 @@ ${outputToken.json(applications)}
}
}

const completeSession = {...currentSession, ...newSession} as Session
const newSessionId = completeSession.identity.userId
const mergedSession = {...currentSession, ...newSession} as Session
const newSessionId = mergedSession.identity.userId
const completeSession: Session = {
...mergedSession,
identity: {
...mergedSession.identity,
email: mergedSession.identity.email ?? sessions[fqdn]?.[newSessionId]?.identity.email,
},
}
const updatedSessions: Sessions = {
...sessions,
[fqdn]: {...sessions[fqdn], [newSessionId]: completeSession},
Expand Down Expand Up @@ -295,9 +302,12 @@ The CLI is currently unable to prompt for reauthentication.`,
* Execute the full authentication flow.
*
* @param applications - An object containing the applications we need to be authenticated with.
* @param existingAlias - Optional alias from a previous session to preserve if the email fetch fails.
* @param existingIdentity - Optional identity from the previous session.
*/
async function executeCompleteFlow(applications: OAuthApplications, existingAlias?: string): Promise<Session> {
async function executeCompleteFlow(
applications: OAuthApplications,
existingIdentity?: IdentityToken,
): Promise<Session> {
const scopes = getFlattenScopes(applications)
const exchangeScopes = getExchangeScopes(applications)
const store = applications.adminApi?.storeFqdn
Expand All @@ -320,14 +330,19 @@ async function executeCompleteFlow(applications: OAuthApplications, existingAlia
outputDebug(outputContent`CLI token received. Exchanging it for application tokens...`)
const result = await exchangeAccessForApplicationTokens(identityToken, exchangeScopes, store)

// Preserve existing alias if available, otherwise try fetching email
// Preserve existing alias if available, otherwise try fetching email.
const businessPlatformToken = result[applicationId('business-platform')]?.accessToken
const alias = existingAlias ?? (await fetchEmail(businessPlatformToken)) ?? identityToken.userId
const existingAlias = existingIdentity?.alias
// A cached email belongs to its user ID, even when reauthentication selects another account.
const existingEmail = existingIdentity?.userId === identityToken.userId ? existingIdentity.email : undefined
const email = existingAlias === undefined ? await fetchEmail(businessPlatformToken) : existingEmail
const alias = existingAlias ?? email ?? identityToken.userId

const session: Session = {
identity: {
...identityToken,
alias,
email,
},
applications: result,
}
Expand All @@ -354,7 +369,11 @@ async function refreshTokens(session: Session, applications: OAuthApplications):
)

return {
identity: {...identityToken, alias: session.identity.alias},
identity: {
...identityToken,
alias: session.identity.alias,
email: identityToken.userId === session.identity.userId ? session.identity.email : undefined,
},
applications: applicationTokens,
}
}
Expand Down
142 changes: 142 additions & 0 deletions packages/cli-kit/src/private/node/session/account-storage.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,142 @@
import {fetch, store, getSessionAccount, findSessionAccountByAlias, setSessionAlias} from './store.js'
import * as exchange from './exchange.js'
import * as deviceAuthorization from './device-authorization.js'
import {allDefaultScopes} from './scopes.js'
import {applicationId} from './identity.js'
import {ensureAuthenticated} from '../session.js'
import * as confStore from '../conf-store.js'
import * as fqdn from '../../../public/node/context/fqdn.js'
import * as environment from '../../../public/node/environment.js'
import * as businessPlatform from '../../../public/node/api/business-platform.js'
import {LocalStorage} from '../../../public/node/local-storage.js'
import {inTemporaryDirectory} from '../../../public/node/fs.js'
import {expect, test, vi} from 'vitest'
import type {Sessions} from './schema.js'

const {getSessions, setSessions, getCurrentSessionId, setCurrentSessionId} = confStore

test.each([undefined, 'verified@example.com'])(
'preserves stored account email %s through file reload and alias changes',
async (email) => {
await inTemporaryDirectory(async (directory) => {
const storage = new LocalStorage<confStore.ConfSchema>({cwd: directory})
vi.spyOn(confStore, 'getSessions').mockImplementation(() => getSessions(storage))
vi.spyOn(confStore, 'setSessions').mockImplementation((sessions) => setSessions(sessions, storage))
vi.spyOn(fqdn, 'identityFqdn').mockResolvedValue('accounts.example.com')
const sessions: Sessions = {
'accounts.example.com': {
'user-123': {
identity: {
userId: 'user-123',
alias: 'nickname@example.com',
email,
accessToken: 'access-token',
refreshToken: 'refresh-token',
expiresAt: new Date('2030-01-01T00:00:00Z'),
scopes: [],
},
applications: {},
},
},
}

await store(sessions)

const reloadedStorage = new LocalStorage<confStore.ConfSchema>({cwd: directory})
vi.mocked(confStore.getSessions).mockImplementation(() => getSessions(reloadedStorage))
vi.mocked(confStore.setSessions).mockImplementation((value) => setSessions(value, reloadedStorage))
const reloaded = JSON.parse(getSessions(reloadedStorage)!)
expect(reloaded['accounts.example.com']['user-123'].identity.email).toBe(email)
expect((await fetch())!['accounts.example.com']!['user-123']!.identity.email).toBe(email)
await expect(getSessionAccount('user-123')).resolves.toEqual({
userId: 'user-123',
alias: 'nickname@example.com',
email,
})

await setSessionAlias('user-123', 'New label')

await expect(findSessionAccountByAlias('New label')).resolves.toEqual({
userId: 'user-123',
alias: 'New label',
email,
})
expect(JSON.parse(getSessions(reloadedStorage)!)['accounts.example.com']['user-123'].identity.email).toBe(email)
})
},
)

test.each(['full-auth', 'invalid-grant'])(
'retains the authenticated account email after %s selects another stored account',
async (flow) => {
await inTemporaryDirectory(async (directory) => {
const storage = new LocalStorage<confStore.ConfSchema>({cwd: directory})
vi.spyOn(confStore, 'getSessions').mockImplementation(() => getSessions(storage))
vi.spyOn(confStore, 'setSessions').mockImplementation((value) => setSessions(value, storage))
vi.spyOn(confStore, 'getCurrentSessionId').mockImplementation(() => getCurrentSessionId(storage))
vi.spyOn(confStore, 'setCurrentSessionId').mockImplementation((value) => setCurrentSessionId(value, storage))
vi.spyOn(fqdn, 'identityFqdn').mockResolvedValue('accounts.example.com')
vi.spyOn(environment, 'getIdentityTokenInformation').mockReturnValue(undefined)
vi.spyOn(environment, 'getAppAutomationToken').mockReturnValue(undefined)
const identity = {
userId: 'second-user',
accessToken: 'access-token',
refreshToken: 'refresh-token',
expiresAt: new Date('2030-01-01T00:00:00Z'),
scopes: allDefaultScopes(),
}
vi.spyOn(deviceAuthorization, 'requestDeviceAuthorization').mockResolvedValue({
deviceCode: 'device-code',
userCode: 'user-code',
verificationUri: 'https://accounts.example.com/activate',
verificationUriComplete: 'https://accounts.example.com/activate?user_code=user-code',
expiresIn: 3600,
interval: 5,
})
vi.spyOn(deviceAuthorization, 'pollForDeviceAuthorization').mockResolvedValue(identity)
vi.spyOn(exchange, 'exchangeAccessForApplicationTokens').mockResolvedValue({
[applicationId('business-platform')]: {
accessToken: 'business-platform-token',
expiresAt: identity.expiresAt,
scopes: [],
},
})
vi.spyOn(exchange, 'refreshAccessToken').mockRejectedValue(new exchange.InvalidGrantError())
const emailRequest = vi
.spyOn(businessPlatform, 'businessPlatformRequest')
.mockResolvedValue({currentUserAccount: {email: 'unexpected@example.com'}})
await store({
'accounts.example.com': {
'first-user': {
identity: {
...identity,
userId: 'first-user',
alias: 'Work',
email: 'first@example.com',
scopes: flow === 'full-auth' ? [] : identity.scopes,
expiresAt: new Date(0),
},
applications: {},
},
'second-user': {
identity: {...identity, alias: 'Personal', email: 'second@example.com'},
applications: {},
},
},
})
setCurrentSessionId('first-user', storage)

await expect(ensureAuthenticated({})).resolves.toEqual({userId: 'second-user'})

const reloadedStorage = new LocalStorage<confStore.ConfSchema>({cwd: directory})
vi.mocked(confStore.getSessions).mockImplementation(() => getSessions(reloadedStorage))
await expect(getSessionAccount('second-user')).resolves.toEqual({
userId: 'second-user',
alias: 'Work',
email: 'second@example.com',
})
expect(emailRequest).not.toHaveBeenCalled()
expect(exchange.refreshAccessToken).toHaveBeenCalledTimes(flow === 'invalid-grant' ? 1 : 0)
})
},
)
1 change: 1 addition & 0 deletions packages/cli-kit/src/private/node/session/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const IdentityTokenSchema = zod.object({
scopes: zod.array(zod.string()),
userId: zod.string(),
alias: zod.string().optional(),
email: zod.string().optional(),
})

/**
Expand Down
14 changes: 7 additions & 7 deletions packages/cli-kit/src/private/node/session/store.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {Sessions} from './schema.js'
import {store, fetch, remove, getSessionAlias, setSessionAlias, findSessionByAlias} from './store.js'
import {store, fetch, remove, getSessionAccount, setSessionAlias, findSessionByAlias} from './store.js'
import {getSessions, removeSessions, setSessions, removeCurrentSessionId} from '../conf-store.js'
import {identityFqdn} from '../../../public/node/context/fqdn.js'

Expand Down Expand Up @@ -129,24 +129,24 @@ describe('session store', () => {
})
})

describe('getSessionAlias', () => {
describe('getSessionAccount', () => {
test('returns alias for existing user', async () => {
// Given
vi.mocked(getSessions).mockReturnValue(JSON.stringify(mockSessions))

// When
const result = await getSessionAlias('user1')
const result = await getSessionAccount('user1')

// Then
expect(result).toBe('Work Account')
expect(result).toEqual({userId: 'user1', alias: 'Work Account', email: undefined})
})

test('returns undefined for non-existent user', async () => {
// Given
vi.mocked(getSessions).mockReturnValue(JSON.stringify(mockSessions))

// When
const result = await getSessionAlias('nonexistent')
const result = await getSessionAccount('nonexistent')

// Then
expect(result).toBeUndefined()
Expand All @@ -157,7 +157,7 @@ describe('session store', () => {
vi.mocked(getSessions).mockReturnValue(undefined)

// When
const result = await getSessionAlias('user1')
const result = await getSessionAccount('user1')

// Then
expect(result).toBeUndefined()
Expand All @@ -169,7 +169,7 @@ describe('session store', () => {
vi.mocked(identityFqdn).mockResolvedValue('different.fqdn.com')

// When
const result = await getSessionAlias('user1')
const result = await getSessionAccount('user1')

// Then
expect(result).toBeUndefined()
Expand Down
Loading
Loading