Skip to content

Commit bebcd96

Browse files
Always fetch the email after login instead of prompting for the alias
1 parent 822cfaf commit bebcd96

File tree

9 files changed

+174
-82
lines changed

9 files changed

+174
-82
lines changed
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// UserEmail query for fetching user email from Business Platform API
2+
export interface UserEmailQuery {
3+
currentUserAccount?: {
4+
email: string
5+
} | null
6+
}
7+
8+
export const UserEmailQueryString = `
9+
query UserEmail {
10+
currentUserAccount {
11+
email
12+
}
13+
}
14+
`

packages/cli-kit/src/private/node/session.test.ts

Lines changed: 118 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ import {getCurrentSessionId} from './conf-store.js'
2323
import * as fqdnModule from '../../public/node/context/fqdn.js'
2424
import {themeToken} from '../../public/node/context/local.js'
2525
import {partnersRequest} from '../../public/node/api/partners.js'
26+
import {businessPlatformRequest} from '../../public/node/api/business-platform.js'
2627
import {getPartnersToken} from '../../public/node/environment.js'
2728
import {nonRandomUUID} from '../../public/node/crypto.js'
28-
import {renderTextPrompt} from '../../public/node/ui.js'
2929
import {terminalSupportsPrompting} from '../../public/node/system.js'
3030
import {vi, describe, expect, test, beforeEach} from 'vitest'
3131

@@ -72,6 +72,11 @@ const appTokens: {[x: string]: ApplicationToken} = {
7272
expiresAt: futureDate,
7373
scopes: ['scope2'],
7474
},
75+
'business-platform': {
76+
accessToken: 'business_platform_token',
77+
expiresAt: futureDate,
78+
scopes: ['scope3'],
79+
},
7580
}
7681

7782
const partnersToken: ApplicationToken = {
@@ -108,11 +113,11 @@ vi.mock('./session/scopes')
108113
vi.mock('./session/store')
109114
vi.mock('./session/validate')
110115
vi.mock('../../public/node/api/partners.js')
116+
vi.mock('../../public/node/api/business-platform.js')
111117
vi.mock('../../store')
112118
vi.mock('../../public/node/environment.js')
113119
vi.mock('./session/device-authorization')
114120
vi.mock('./conf-store')
115-
vi.mock('../../public/node/ui.js')
116121
vi.mock('../../public/node/system.js')
117122

118123
beforeEach(() => {
@@ -138,31 +143,34 @@ beforeEach(() => {
138143
interval: 5,
139144
})
140145
vi.mocked(pollForDeviceAuthorization).mockResolvedValue(validIdentityToken)
141-
vi.mocked(renderTextPrompt).mockResolvedValue(userId)
142146
vi.mocked(terminalSupportsPrompting).mockReturnValue(true)
147+
vi.mocked(businessPlatformRequest).mockResolvedValue({
148+
currentUserAccount: {
149+
150+
},
151+
})
143152
})
144153

145154
describe('ensureAuthenticated when previous session is invalid', () => {
146155
test('executes complete auth flow if there is no session', async () => {
147156
// Given
148157
vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth')
149158
vi.mocked(fetchSessions).mockResolvedValue(undefined)
150-
vi.mocked(renderTextPrompt).mockResolvedValue(userId)
151159

152160
// When
153161
const got = await ensureAuthenticated(defaultApplications)
154162

155163
// Then
156164
expect(exchangeAccessForApplicationTokens).toBeCalled()
157165
expect(refreshAccessToken).not.toBeCalled()
158-
expect(renderTextPrompt).toHaveBeenCalledWith({
159-
message: 'Enter an alias to identify this account',
160-
defaultValue: userId,
161-
allowEmpty: false,
162-
})
166+
expect(businessPlatformRequest).toHaveBeenCalled()
163167
expect(storeSessions).toHaveBeenCalledOnce()
164168
expect(got).toEqual(validTokens)
165169

170+
// Verify the session was stored with email as alias
171+
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
172+
expect(storedSession[fqdn]![userId]!.identity.alias).toBe('[email protected]')
173+
166174
// The userID is cached in memory and the secureStore is not accessed again
167175
await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
168176
await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth')
@@ -194,21 +202,75 @@ The CLI is currently unable to prompt for reauthentication.`,
194202
// Given
195203
vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth')
196204
vi.mocked(fetchSessions).mockResolvedValue(invalidSessions)
197-
const newSession: Sessions = {...invalidSessions, ...validSessions}
205+
const expectedSessions = {
206+
...invalidSessions,
207+
[fqdn]: {
208+
[userId]: {
209+
identity: {
210+
...validIdentityToken,
211+
212+
},
213+
applications: appTokens,
214+
},
215+
},
216+
}
198217

199218
// When
200219
const got = await ensureAuthenticated(defaultApplications)
201220

202221
// Then
203222
expect(exchangeAccessForApplicationTokens).toBeCalled()
204223
expect(refreshAccessToken).not.toBeCalled()
205-
expect(storeSessions).toBeCalledWith(newSession)
224+
expect(storeSessions).toBeCalledWith(expectedSessions)
206225
expect(got).toEqual(validTokens)
207226
await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
208227
await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth')
209228
expect(fetchSessions).toHaveBeenCalledOnce()
210229
})
211230

231+
test('falls back to userId when email fetch fails', async () => {
232+
// Given
233+
vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth')
234+
vi.mocked(fetchSessions).mockResolvedValue(undefined)
235+
vi.mocked(businessPlatformRequest).mockRejectedValueOnce(new Error('API Error'))
236+
237+
// When
238+
const got = await ensureAuthenticated(defaultApplications)
239+
240+
// Then
241+
expect(exchangeAccessForApplicationTokens).toBeCalled()
242+
expect(businessPlatformRequest).toHaveBeenCalled()
243+
expect(storeSessions).toHaveBeenCalledOnce()
244+
245+
// Verify the session was stored with userId as alias (fallback)
246+
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
247+
expect(storedSession[fqdn]![userId]!.identity.alias).toBe(userId)
248+
249+
expect(got).toEqual(validTokens)
250+
})
251+
252+
test('falls back to userId when no business platform token available', async () => {
253+
// Given
254+
vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth')
255+
vi.mocked(fetchSessions).mockResolvedValue(undefined)
256+
const appTokensWithoutBusinessPlatform = {
257+
'mystore.myshopify.com-admin': appTokens['mystore.myshopify.com-admin']!,
258+
'storefront-renderer': appTokens['storefront-renderer']!,
259+
partners: appTokens.partners!,
260+
}
261+
vi.mocked(exchangeAccessForApplicationTokens).mockResolvedValueOnce(appTokensWithoutBusinessPlatform)
262+
263+
// When
264+
const got = await ensureAuthenticated(defaultApplications)
265+
266+
// Then
267+
expect(businessPlatformRequest).not.toHaveBeenCalled()
268+
269+
// Verify the session was stored with userId as alias (fallback)
270+
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
271+
expect(storedSession[fqdn]![userId]!.identity.alias).toBe(userId)
272+
})
273+
212274
test('executes complete auth flow if requesting additional scopes', async () => {
213275
// Given
214276
vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth')
@@ -220,7 +282,13 @@ The CLI is currently unable to prompt for reauthentication.`,
220282
// Then
221283
expect(exchangeAccessForApplicationTokens).toBeCalled()
222284
expect(refreshAccessToken).not.toBeCalled()
223-
expect(storeSessions).toBeCalledWith(validSessions)
285+
expect(businessPlatformRequest).toHaveBeenCalled()
286+
expect(storeSessions).toHaveBeenCalledOnce()
287+
288+
// Verify the session was stored with email as alias
289+
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
290+
expect(storedSession[fqdn]![userId]!.identity.alias).toBe('[email protected]')
291+
224292
expect(got).toEqual(validTokens)
225293
await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
226294
await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth')
@@ -317,7 +385,13 @@ describe('when existing session is expired', () => {
317385
// Then
318386
expect(refreshAccessToken).toBeCalled()
319387
expect(exchangeAccessForApplicationTokens).toBeCalled()
320-
expect(storeSessions).toBeCalledWith(validSessions)
388+
expect(businessPlatformRequest).toHaveBeenCalled()
389+
expect(storeSessions).toHaveBeenCalledOnce()
390+
391+
// Verify the session was stored with email as alias
392+
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
393+
expect(storedSession[fqdn]![userId]!.identity.alias).toBe('[email protected]')
394+
321395
expect(got).toEqual(validTokens)
322396
await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
323397
await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth')
@@ -464,30 +538,23 @@ describe('getLastSeenAuthMethod', () => {
464538
})
465539
})
466540

467-
describe('ensureAuthenticated alias functionality', () => {
468-
test('sets alias when provided during full auth flow', async () => {
541+
describe('ensureAuthenticated email fetch functionality', () => {
542+
test('fetches and sets email as alias during full auth flow', async () => {
469543
// Given
470544
vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth')
471545
vi.mocked(fetchSessions).mockResolvedValue(undefined)
472-
vi.mocked(renderTextPrompt).mockResolvedValue('work-account')
473-
const expectedSessionWithAlias = {
474-
...validSessions,
475-
[fqdn]: {
476-
[userId]: {
477-
...validSessions[fqdn]![userId]!,
478-
identity: {
479-
...validSessions[fqdn]![userId]!.identity,
480-
alias: 'work-account',
481-
},
482-
},
546+
vi.mocked(businessPlatformRequest).mockResolvedValueOnce({
547+
currentUserAccount: {
548+
483549
},
484-
}
550+
})
485551

486552
// When
487-
const got = await ensureAuthenticated(defaultApplications, process.env, {alias: 'work-account'})
553+
const got = await ensureAuthenticated(defaultApplications)
488554

489555
// Then
490-
expect(storeSessions).toBeCalledWith(expectedSessionWithAlias)
556+
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
557+
expect(storedSession[fqdn]![userId]!.identity.alias).toBe('[email protected]')
491558
expect(got).toEqual(validTokens)
492559
})
493560

@@ -510,53 +577,52 @@ describe('ensureAuthenticated alias functionality', () => {
510577
vi.mocked(fetchSessions).mockResolvedValue(validSessions)
511578

512579
// When
513-
const got = await ensureAuthenticated(defaultApplications, process.env, {alias: 'updated-alias'})
580+
const got = await ensureAuthenticated(defaultApplications)
514581

515582
// Then
516-
// The alias parameter is ignored during refresh - the session keeps its existing alias
583+
// The email fetch is not called during refresh - the session keeps its existing alias
584+
expect(businessPlatformRequest).not.toHaveBeenCalled()
517585
expect(storeSessions).toBeCalledWith(validSessions)
518586
expect(got).toEqual(validTokens)
519587
})
520588

521-
test('sets alias during token refresh error fallback', async () => {
589+
test('fetches email during token refresh error fallback', async () => {
522590
// Given
523591
const tokenResponseError = new InvalidGrantError()
524592
vi.mocked(validateSession).mockResolvedValueOnce('needs_refresh')
525593
vi.mocked(fetchSessions).mockResolvedValue(validSessions)
526594
vi.mocked(refreshAccessToken).mockRejectedValueOnce(tokenResponseError)
527-
vi.mocked(renderTextPrompt).mockResolvedValue('fallback-alias')
528-
const expectedSessionWithAlias = {
529-
...validSessions,
530-
[fqdn]: {
531-
[userId]: {
532-
...validSessions[fqdn]![userId]!,
533-
identity: {
534-
...validSessions[fqdn]![userId]!.identity,
535-
alias: 'fallback-alias',
536-
},
537-
},
595+
vi.mocked(businessPlatformRequest).mockResolvedValueOnce({
596+
currentUserAccount: {
597+
538598
},
539-
}
599+
})
540600

541601
// When
542-
const got = await ensureAuthenticated(defaultApplications, process.env, {alias: 'fallback-alias'})
602+
const got = await ensureAuthenticated(defaultApplications)
543603

544604
// Then
545-
expect(storeSessions).toBeCalledWith(expectedSessionWithAlias)
605+
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
606+
expect(storedSession[fqdn]![userId]!.identity.alias).toBe('[email protected]')
546607
expect(got).toEqual(validTokens)
547608
})
548609

549-
test('preserves current session alias when setting new alias to undefined', async () => {
610+
test('uses userId as alias when email is not available', async () => {
550611
// Given
551-
vi.mocked(validateSession).mockResolvedValueOnce('ok')
552-
vi.mocked(fetchSessions).mockResolvedValue(validSessions)
612+
vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth')
613+
vi.mocked(fetchSessions).mockResolvedValue(undefined)
614+
vi.mocked(businessPlatformRequest).mockResolvedValueOnce({
615+
currentUserAccount: {
616+
email: null,
617+
},
618+
})
553619

554620
// When
555-
const got = await ensureAuthenticated(defaultApplications, process.env, {alias: undefined})
621+
const got = await ensureAuthenticated(defaultApplications)
556622

557623
// Then
624+
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
625+
expect(storedSession[fqdn]![userId]!.identity.alias).toBe(userId)
558626
expect(got).toEqual(validTokens)
559-
// Verify the session was not stored (no change)
560-
expect(storeSessions).not.toHaveBeenCalled()
561627
})
562628
})

0 commit comments

Comments
 (0)