Skip to content

Commit 072b6e7

Browse files
Ask for an alias on login when the email can't be retrieved
1 parent cb018ff commit 072b6e7

6 files changed

Lines changed: 155 additions & 26 deletions

File tree

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

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ const validIdentityToken: IdentityToken = {
4646
expiresAt: futureDate,
4747
scopes: ['scope', 'scope2'],
4848
userId,
49-
alias: userId,
5049
}
5150

5251
const validTokens: OAuthSession = {
@@ -229,7 +228,7 @@ The CLI is currently unable to prompt for reauthentication.`,
229228
expect(fetchSessions).toHaveBeenCalledOnce()
230229
})
231230

232-
test('falls back to userId when email fetch fails', async () => {
231+
test('leaves alias undefined when email fetch fails', async () => {
233232
// Given
234233
vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth')
235234
vi.mocked(fetchSessions).mockResolvedValue(undefined)
@@ -243,14 +242,13 @@ The CLI is currently unable to prompt for reauthentication.`,
243242
expect(businessPlatformRequest).toHaveBeenCalled()
244243
expect(storeSessions).toHaveBeenCalledOnce()
245244

246-
// Verify the session was stored with userId as alias (fallback)
247245
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
248-
expect(storedSession[fqdn]![userId]!.identity.alias).toBe(userId)
246+
expect(storedSession[fqdn]![userId]!.identity.alias).toBeUndefined()
249247

250248
expect(got).toEqual(validTokens)
251249
})
252250

253-
test('falls back to userId when no business platform token available', async () => {
251+
test('leaves alias undefined when no business platform token available', async () => {
254252
// Given
255253
vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth')
256254
vi.mocked(fetchSessions).mockResolvedValue(undefined)
@@ -267,9 +265,8 @@ The CLI is currently unable to prompt for reauthentication.`,
267265
// Then
268266
expect(businessPlatformRequest).not.toHaveBeenCalled()
269267

270-
// Verify the session was stored with userId as alias (fallback)
271268
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
272-
expect(storedSession[fqdn]![userId]!.identity.alias).toBe(userId)
269+
expect(storedSession[fqdn]![userId]!.identity.alias).toBeUndefined()
273270
})
274271

275272
test('executes complete auth flow if requesting additional scopes', async () => {
@@ -684,7 +681,32 @@ describe('ensureAuthenticated email fetch functionality', () => {
684681
expect(got).toEqual(validTokens)
685682
})
686683

687-
test('uses userId as alias when email is not available', async () => {
684+
test('preserves existing alias during token refresh error fallback when email fetch fails', async () => {
685+
// Given
686+
const sessionsWithAlias: Sessions = {
687+
[fqdn]: {
688+
[userId]: {
689+
identity: {...validIdentityToken, alias: 'my-custom-alias'},
690+
applications: appTokens,
691+
},
692+
},
693+
}
694+
const tokenResponseError = new InvalidGrantError()
695+
vi.mocked(validateSession).mockResolvedValueOnce('needs_refresh')
696+
vi.mocked(fetchSessions).mockResolvedValue(sessionsWithAlias)
697+
vi.mocked(refreshAccessToken).mockRejectedValueOnce(tokenResponseError)
698+
vi.mocked(businessPlatformRequest).mockRejectedValueOnce(new Error('API Error'))
699+
700+
// When
701+
const got = await ensureAuthenticated(defaultApplications)
702+
703+
// Then
704+
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
705+
expect(storedSession[fqdn]![userId]!.identity.alias).toBe('my-custom-alias')
706+
expect(got).toEqual(validTokens)
707+
})
708+
709+
test('leaves alias undefined when email is not available', async () => {
688710
// Given
689711
vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth')
690712
vi.mocked(fetchSessions).mockResolvedValue(undefined)
@@ -699,7 +721,7 @@ describe('ensureAuthenticated email fetch functionality', () => {
699721

700722
// Then
701723
const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
702-
expect(storedSession[fqdn]![userId]!.identity.alias).toBe(userId)
724+
expect(storedSession[fqdn]![userId]!.identity.alias).toBeUndefined()
703725
expect(got).toEqual(validTokens)
704726
})
705727
})

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

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ async function fetchEmail(businessPlatformToken: string | undefined): Promise<st
3535

3636
try {
3737
const userEmailResult = await businessPlatformRequest<UserEmailQuery>(UserEmailQueryString, businessPlatformToken)
38-
return userEmailResult.currentUserAccount?.email
38+
return userEmailResult.currentUserAccount?.email ?? undefined
3939
// eslint-disable-next-line no-catch-all/no-catch-all
4040
} catch (error) {
4141
outputDebug(outputContent`Failed to fetch user email: ${(error as Error).message ?? String(error)}`)
@@ -230,15 +230,15 @@ ${outputToken.json(applications)}
230230
if (validationResult === 'needs_full_auth') {
231231
await throwOnNoPrompt(noPrompt)
232232
outputDebug(outputContent`Initiating the full authentication flow...`)
233-
newSession = await executeCompleteFlow(applications)
233+
newSession = await executeCompleteFlow(applications, currentSession?.identity.alias)
234234
} else if (validationResult === 'needs_refresh' || forceRefresh) {
235235
outputDebug(outputContent`The current session is valid but needs refresh. Refreshing...`)
236236
try {
237237
newSession = await refreshTokens(currentSession!, applications)
238238
} catch (error) {
239239
if (error instanceof InvalidGrantError) {
240240
await throwOnNoPrompt(noPrompt)
241-
newSession = await executeCompleteFlow(applications)
241+
newSession = await executeCompleteFlow(applications, currentSession?.identity.alias)
242242
} else if (error instanceof InvalidRequestError) {
243243
await sessionStore.remove()
244244
throw new AbortError('\nError validating auth session', "We've cleared the current session, please try again")
@@ -289,9 +289,9 @@ The CLI is currently unable to prompt for reauthentication.`,
289289
* Execute the full authentication flow.
290290
*
291291
* @param applications - An object containing the applications we need to be authenticated with.
292-
* @param alias - Optional alias to use for the session.
292+
* @param existingAlias - Optional alias from a previous session to preserve if the email fetch fails.
293293
*/
294-
async function executeCompleteFlow(applications: OAuthApplications): Promise<Session> {
294+
async function executeCompleteFlow(applications: OAuthApplications, existingAlias?: string): Promise<Session> {
295295
const scopes = getFlattenScopes(applications)
296296
const exchangeScopes = getExchangeScopes(applications)
297297
const store = applications.adminApi?.storeFqdn
@@ -318,9 +318,9 @@ async function executeCompleteFlow(applications: OAuthApplications): Promise<Ses
318318
outputDebug(outputContent`CLI token received. Exchanging it for application tokens...`)
319319
const result = await exchangeAccessForApplicationTokens(identityToken, exchangeScopes, store)
320320

321-
// Get the alias for the session (email or userId)
321+
// Preserve existing alias if available, otherwise try fetching email
322322
const businessPlatformToken = result[applicationId('business-platform')]?.accessToken
323-
const alias = (await fetchEmail(businessPlatformToken)) ?? identityToken.userId
323+
const alias = existingAlias ?? (await fetchEmail(businessPlatformToken))
324324

325325
const session: Session = {
326326
identity: {
@@ -447,6 +447,6 @@ function buildIdentityTokenFromEnv(
447447
...identityTokenInformation,
448448
expiresAt: new Date(Date.now() + 30 * 24 * 60 * 60 * 1000),
449449
scopes,
450-
alias: identityTokenInformation.userId,
450+
alias: undefined,
451451
}
452452
}

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

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {Sessions} from './schema.js'
2-
import {store, fetch, remove, getSessionAlias, findSessionByAlias} from './store.js'
2+
import {store, fetch, remove, getSessionAlias, setSessionAlias, findSessionByAlias} from './store.js'
33
import {getSessions, removeSessions, setSessions, removeCurrentSessionId} from '../conf-store.js'
44
import {identityFqdn} from '../../../public/node/context/fqdn.js'
55

@@ -28,7 +28,6 @@ const mockSessions: Sessions = {
2828
expiresAt: new Date(),
2929
scopes: ['scope2'],
3030
userId: 'user2',
31-
alias: 'user2',
3231
},
3332
applications: {},
3433
},
@@ -132,6 +131,54 @@ describe('session store', () => {
132131
})
133132
})
134133

134+
describe('setSessionAlias', () => {
135+
test('updates alias for an existing user', async () => {
136+
// Given
137+
vi.mocked(getSessions).mockReturnValue(JSON.stringify(mockSessions))
138+
139+
// When
140+
await setSessionAlias('user1', 'New Alias')
141+
142+
// Then
143+
const storedSessions = JSON.parse(vi.mocked(setSessions).mock.calls[0]![0])
144+
expect(storedSessions['identity.fqdn.com'].user1.identity.alias).toBe('New Alias')
145+
})
146+
147+
test('does nothing when no sessions exist', async () => {
148+
// Given
149+
vi.mocked(getSessions).mockReturnValue(undefined)
150+
151+
// When
152+
await setSessionAlias('user1', 'New Alias')
153+
154+
// Then
155+
expect(setSessions).not.toHaveBeenCalled()
156+
})
157+
158+
test('does nothing when user does not exist', async () => {
159+
// Given
160+
vi.mocked(getSessions).mockReturnValue(JSON.stringify(mockSessions))
161+
162+
// When
163+
await setSessionAlias('nonexistent', 'New Alias')
164+
165+
// Then
166+
expect(setSessions).not.toHaveBeenCalled()
167+
})
168+
169+
test('does nothing when fqdn does not match', async () => {
170+
// Given
171+
vi.mocked(getSessions).mockReturnValue(JSON.stringify(mockSessions))
172+
vi.mocked(identityFqdn).mockResolvedValue('different.fqdn.com')
173+
174+
// When
175+
await setSessionAlias('user1', 'New Alias')
176+
177+
// Then
178+
expect(setSessions).not.toHaveBeenCalled()
179+
})
180+
})
181+
135182
describe('findSessionByAlias', () => {
136183
test('returns userId for existing alias', async () => {
137184
// Given

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

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {SessionsSchema} from './schema.js'
22
import {getSessions, removeCurrentSessionId, removeSessions, setSessions} from '../conf-store.js'
33
import {identityFqdn} from '../../../public/node/context/fqdn.js'
4-
import type {Sessions} from './schema.js'
4+
import type {Session, Sessions} from './schema.js'
55

66
/**
77
* Serializes the session as a JSON and stores it in the system.
@@ -56,6 +56,24 @@ export async function getSessionAlias(userId: string): Promise<string | undefine
5656
return sessions[fqdn][userId].identity.alias
5757
}
5858

59+
/**
60+
* Sets the alias for a given user's session and persists it.
61+
*
62+
* @param userId - The user ID of the session to update.
63+
* @param alias - The new alias to set.
64+
*/
65+
export async function setSessionAlias(userId: string, alias: string): Promise<void> {
66+
const sessions = await fetch()
67+
if (!sessions) return
68+
69+
const fqdn = await identityFqdn()
70+
const session: Session | undefined = sessions[fqdn]?.[userId]
71+
if (!session) return
72+
73+
session.identity.alias = alias
74+
await store(sessions)
75+
}
76+
5977
/**
6078
* Finds a session by its alias.
6179
*
@@ -71,7 +89,7 @@ export async function findSessionByAlias(alias: string): Promise<string | undefi
7189
if (!fqdnSessions) return undefined
7290

7391
for (const [userId, session] of Object.entries(fqdnSessions)) {
74-
if (session.identity.alias === alias) {
92+
if (session.identity.alias === alias || userId === alias) {
7593
return userId
7694
}
7795
}

packages/cli-kit/src/public/node/session-prompt.test.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {promptSessionSelect} from './session-prompt.js'
2-
import {renderSelectPrompt} from './ui.js'
2+
import {renderSelectPrompt, renderTextPrompt} from './ui.js'
33
import {ensureAuthenticatedUser} from './session.js'
44
import {identityFqdn} from './context/fqdn.js'
55
import {setCurrentSessionId} from '../../private/node/conf-store.js'
@@ -34,8 +34,6 @@ const mockSessions: Sessions = {
3434
expiresAt: new Date(),
3535
scopes: ['scope2'],
3636
userId: 'user2',
37-
// Default alias is same as userId
38-
alias: 'user2',
3937
},
4038
applications: {},
4139
},
@@ -64,6 +62,21 @@ describe('promptSessionSelect', () => {
6462
expect(result).toEqual('new-alias')
6563
})
6664

65+
test('prompts for alias when no alias is stored', async () => {
66+
// Given
67+
vi.mocked(sessionStore.fetch).mockResolvedValue(undefined)
68+
vi.mocked(sessionStore.getSessionAlias).mockResolvedValue(undefined)
69+
vi.mocked(renderTextPrompt).mockResolvedValue('typed-alias')
70+
71+
// When
72+
const result = await promptSessionSelect()
73+
74+
// Then
75+
expect(renderTextPrompt).toHaveBeenCalled()
76+
expect(sessionStore.setSessionAlias).toHaveBeenCalledWith('new-user-id', 'typed-alias')
77+
expect(result).toEqual('typed-alias')
78+
})
79+
6780
test('prompts user to create new session with alias when no existing sessions', async () => {
6881
// Given
6982
vi.mocked(sessionStore.fetch).mockResolvedValue(undefined)
@@ -166,6 +179,25 @@ describe('promptSessionSelect', () => {
166179
expect(result).toEqual('custom-alias')
167180
})
168181

182+
test('prompts for alias when selecting "Log in with a different account" and no alias is stored', async () => {
183+
// Given
184+
vi.mocked(sessionStore.fetch).mockResolvedValue(mockSessions)
185+
vi.mocked(renderSelectPrompt).mockResolvedValue('NEW_LOGIN')
186+
vi.mocked(sessionStore.getSessionAlias).mockResolvedValue(undefined)
187+
vi.mocked(renderTextPrompt).mockResolvedValue('my-work-account')
188+
189+
// When
190+
const result = await promptSessionSelect()
191+
192+
// Then
193+
expect(ensureAuthenticatedUser).toHaveBeenCalledWith({}, {forceNewSession: true})
194+
expect(renderTextPrompt).toHaveBeenCalledWith({
195+
message: 'Enter an alias for this account (e.g. your email or a nickname)',
196+
})
197+
expect(sessionStore.setSessionAlias).toHaveBeenCalledWith('new-user-id', 'my-work-account')
198+
expect(result).toEqual('my-work-account')
199+
})
200+
169201
test('does not update alias for existing session when not provided', async () => {
170202
// Given
171203
vi.mocked(sessionStore.fetch).mockResolvedValue(mockSessions)

packages/cli-kit/src/public/node/session-prompt.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {renderSelectPrompt} from './ui.js'
1+
import {renderSelectPrompt, renderTextPrompt} from './ui.js'
22
import {ensureAuthenticatedUser} from './session.js'
33
import {identityFqdn} from './context/fqdn.js'
44
import * as sessionStore from '../../private/node/session/store.js'
@@ -37,13 +37,23 @@ function buildSessionChoices(sessions: Sessions, fqdn: string): SessionChoice[]
3737

3838
/**
3939
* Handles the new login flow.
40+
* If no alias is stored (email couldn't be fetched), prompts the user for a friendly alias.
4041
*
4142
* @returns The alias of the authenticated user.
4243
*/
4344
async function handleNewLogin(): Promise<string> {
4445
const result = await ensureAuthenticatedUser({}, {forceNewSession: true})
4546
const alias = await sessionStore.getSessionAlias(result.userId)
46-
return alias ?? result.userId
47+
48+
if (!alias) {
49+
const userAlias = await renderTextPrompt({
50+
message: 'Enter an alias for this account (e.g. your email or a nickname)',
51+
})
52+
await sessionStore.setSessionAlias(result.userId, userAlias)
53+
return userAlias
54+
}
55+
56+
return alias
4757
}
4858

4959
/**

0 commit comments

Comments
 (0)