diff --git a/packages/cli-kit/src/private/node/session.test.ts b/packages/cli-kit/src/private/node/session.test.ts index 08eda6acff3..4f092dba64e 100644 --- a/packages/cli-kit/src/private/node/session.test.ts +++ b/packages/cli-kit/src/private/node/session.test.ts @@ -46,7 +46,6 @@ const validIdentityToken: IdentityToken = { expiresAt: futureDate, scopes: ['scope', 'scope2'], userId, - alias: userId, } const validTokens: OAuthSession = { @@ -229,7 +228,7 @@ The CLI is currently unable to prompt for reauthentication.`, expect(fetchSessions).toHaveBeenCalledOnce() }) - test('falls back to userId when email fetch fails', async () => { + test('falls back to userId as alias when email fetch fails', async () => { // Given vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth') vi.mocked(fetchSessions).mockResolvedValue(undefined) @@ -243,7 +242,6 @@ The CLI is currently unable to prompt for reauthentication.`, expect(businessPlatformRequest).toHaveBeenCalled() expect(storeSessions).toHaveBeenCalledOnce() - // Verify the session was stored with userId as alias (fallback) const storedSession = vi.mocked(storeSessions).mock.calls[0]![0] expect(storedSession[fqdn]![userId]!.identity.alias).toBe(userId) @@ -262,7 +260,7 @@ The CLI is currently unable to prompt for reauthentication.`, vi.mocked(exchangeAccessForApplicationTokens).mockResolvedValueOnce(appTokensWithoutBusinessPlatform) // When - const got = await ensureAuthenticated(defaultApplications) + await ensureAuthenticated(defaultApplications) // Then expect(businessPlatformRequest).not.toHaveBeenCalled() @@ -650,16 +648,24 @@ describe('ensureAuthenticated email fetch functionality', () => { test('preserves existing alias during refresh token flow', async () => { // Given + const sessionsWithAlias: Sessions = { + [fqdn]: { + [userId]: { + identity: {...validIdentityToken, alias: 'user@example.com'}, + applications: appTokens, + }, + }, + } vi.mocked(validateSession).mockResolvedValueOnce('needs_refresh') - vi.mocked(fetchSessions).mockResolvedValue(validSessions) + vi.mocked(fetchSessions).mockResolvedValue(sessionsWithAlias) // When const got = await ensureAuthenticated(defaultApplications) // Then - // The email fetch is not called during refresh - the session keeps its existing alias expect(businessPlatformRequest).not.toHaveBeenCalled() - expect(storeSessions).toBeCalledWith(validSessions) + const storedSession = vi.mocked(storeSessions).mock.calls[0]![0] + expect(storedSession[fqdn]![userId]!.identity.alias).toBe('user@example.com') expect(got).toEqual(validTokens) }) @@ -684,7 +690,32 @@ describe('ensureAuthenticated email fetch functionality', () => { expect(got).toEqual(validTokens) }) - test('uses userId as alias when email is not available', async () => { + test('preserves existing alias during token refresh error fallback when email fetch fails', async () => { + // Given + const sessionsWithAlias: Sessions = { + [fqdn]: { + [userId]: { + identity: {...validIdentityToken, alias: 'my-custom-alias'}, + applications: {}, + }, + }, + } + const tokenResponseError = new InvalidGrantError() + vi.mocked(validateSession).mockResolvedValueOnce('needs_refresh') + vi.mocked(fetchSessions).mockResolvedValue(sessionsWithAlias) + vi.mocked(refreshAccessToken).mockRejectedValueOnce(tokenResponseError) + vi.mocked(businessPlatformRequest).mockRejectedValueOnce(new Error('API Error')) + + // When + const got = await ensureAuthenticated(defaultApplications) + + // Then + const storedSession = vi.mocked(storeSessions).mock.calls[0]![0] + expect(storedSession[fqdn]![userId]!.identity.alias).toBe('my-custom-alias') + expect(got).toEqual(validTokens) + }) + + test('falls back to userId as alias when email is not available', async () => { // Given vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth') vi.mocked(fetchSessions).mockResolvedValue(undefined) diff --git a/packages/cli-kit/src/private/node/session.ts b/packages/cli-kit/src/private/node/session.ts index 6f87d00c66d..2468747b851 100644 --- a/packages/cli-kit/src/private/node/session.ts +++ b/packages/cli-kit/src/private/node/session.ts @@ -35,7 +35,7 @@ async function fetchEmail(businessPlatformToken: string | undefined): Promise(UserEmailQueryString, businessPlatformToken) - return userEmailResult.currentUserAccount?.email + return userEmailResult.currentUserAccount?.email ?? undefined // eslint-disable-next-line no-catch-all/no-catch-all } catch (error) { outputDebug(outputContent`Failed to fetch user email: ${(error as Error).message ?? String(error)}`) @@ -230,7 +230,7 @@ ${outputToken.json(applications)} if (validationResult === 'needs_full_auth') { await throwOnNoPrompt(noPrompt) outputDebug(outputContent`Initiating the full authentication flow...`) - newSession = await executeCompleteFlow(applications) + newSession = await executeCompleteFlow(applications, currentSession?.identity.alias) } else if (validationResult === 'needs_refresh' || forceRefresh) { outputDebug(outputContent`The current session is valid but needs refresh. Refreshing...`) try { @@ -238,7 +238,7 @@ ${outputToken.json(applications)} } catch (error) { if (error instanceof InvalidGrantError) { await throwOnNoPrompt(noPrompt) - newSession = await executeCompleteFlow(applications) + newSession = await executeCompleteFlow(applications, currentSession?.identity.alias) } else if (error instanceof InvalidRequestError) { await sessionStore.remove() 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.`, * Execute the full authentication flow. * * @param applications - An object containing the applications we need to be authenticated with. - * @param alias - Optional alias to use for the session. + * @param existingAlias - Optional alias from a previous session to preserve if the email fetch fails. */ -async function executeCompleteFlow(applications: OAuthApplications): Promise { +async function executeCompleteFlow(applications: OAuthApplications, existingAlias?: string): Promise { const scopes = getFlattenScopes(applications) const exchangeScopes = getExchangeScopes(applications) const store = applications.adminApi?.storeFqdn @@ -318,9 +318,9 @@ async function executeCompleteFlow(applications: OAuthApplications): Promise { }) }) + describe('setSessionAlias', () => { + test('updates alias for an existing user', async () => { + // Given + vi.mocked(getSessions).mockReturnValue(JSON.stringify(mockSessions)) + + // When + await setSessionAlias('user1', 'New Alias') + + // Then + const storedSessions = JSON.parse(vi.mocked(setSessions).mock.calls[0]![0]) + expect(storedSessions['identity.fqdn.com'].user1.identity.alias).toBe('New Alias') + }) + + test('does nothing when no sessions exist', async () => { + // Given + vi.mocked(getSessions).mockReturnValue(undefined) + + // When + await setSessionAlias('user1', 'New Alias') + + // Then + expect(setSessions).not.toHaveBeenCalled() + }) + + test('does nothing when user does not exist', async () => { + // Given + vi.mocked(getSessions).mockReturnValue(JSON.stringify(mockSessions)) + + // When + await setSessionAlias('nonexistent', 'New Alias') + + // Then + expect(setSessions).not.toHaveBeenCalled() + }) + + test('does nothing when fqdn does not match', async () => { + // Given + vi.mocked(getSessions).mockReturnValue(JSON.stringify(mockSessions)) + vi.mocked(identityFqdn).mockResolvedValue('different.fqdn.com') + + // When + await setSessionAlias('user1', 'New Alias') + + // Then + expect(setSessions).not.toHaveBeenCalled() + }) + }) + describe('findSessionByAlias', () => { test('returns userId for existing alias', async () => { // Given diff --git a/packages/cli-kit/src/private/node/session/store.ts b/packages/cli-kit/src/private/node/session/store.ts index 17d20b030b9..bba1336a38b 100644 --- a/packages/cli-kit/src/private/node/session/store.ts +++ b/packages/cli-kit/src/private/node/session/store.ts @@ -1,7 +1,7 @@ import {SessionsSchema} from './schema.js' import {getSessions, removeCurrentSessionId, removeSessions, setSessions} from '../conf-store.js' import {identityFqdn} from '../../../public/node/context/fqdn.js' -import type {Sessions} from './schema.js' +import type {Session, Sessions} from './schema.js' /** * Serializes the session as a JSON and stores it in the system. @@ -56,6 +56,24 @@ export async function getSessionAlias(userId: string): Promise { + const sessions = await fetch() + if (!sessions) return + + const fqdn = await identityFqdn() + const session: Session | undefined = sessions[fqdn]?.[userId] + if (!session) return + + session.identity.alias = alias + await store(sessions) +} + /** * Finds a session by its alias. * diff --git a/packages/cli-kit/src/public/node/session-prompt.test.ts b/packages/cli-kit/src/public/node/session-prompt.test.ts index 7b9e983a300..afd1d72aecd 100644 --- a/packages/cli-kit/src/public/node/session-prompt.test.ts +++ b/packages/cli-kit/src/public/node/session-prompt.test.ts @@ -1,5 +1,5 @@ import {promptSessionSelect} from './session-prompt.js' -import {renderSelectPrompt} from './ui.js' +import {renderSelectPrompt, renderTextPrompt} from './ui.js' import {ensureAuthenticatedUser} from './session.js' import {identityFqdn} from './context/fqdn.js' import {setCurrentSessionId} from '../../private/node/conf-store.js' @@ -34,8 +34,6 @@ const mockSessions: Sessions = { expiresAt: new Date(), scopes: ['scope2'], userId: 'user2', - // Default alias is same as userId - alias: 'user2', }, applications: {}, }, @@ -64,6 +62,21 @@ describe('promptSessionSelect', () => { expect(result).toEqual('new-alias') }) + test('prompts for alias when no alias is stored', async () => { + // Given + vi.mocked(sessionStore.fetch).mockResolvedValue(undefined) + vi.mocked(sessionStore.getSessionAlias).mockResolvedValue(undefined) + vi.mocked(renderTextPrompt).mockResolvedValue('typed-alias') + + // When + const result = await promptSessionSelect() + + // Then + expect(renderTextPrompt).toHaveBeenCalled() + expect(sessionStore.setSessionAlias).toHaveBeenCalledWith('new-user-id', 'typed-alias') + expect(result).toEqual('typed-alias') + }) + test('prompts user to create new session with alias when no existing sessions', async () => { // Given vi.mocked(sessionStore.fetch).mockResolvedValue(undefined) @@ -166,6 +179,25 @@ describe('promptSessionSelect', () => { expect(result).toEqual('custom-alias') }) + test('prompts for alias when selecting "Log in with a different account" and no alias is stored', async () => { + // Given + vi.mocked(sessionStore.fetch).mockResolvedValue(mockSessions) + vi.mocked(renderSelectPrompt).mockResolvedValue('NEW_LOGIN') + vi.mocked(sessionStore.getSessionAlias).mockResolvedValue(undefined) + vi.mocked(renderTextPrompt).mockResolvedValue('my-work-account') + + // When + const result = await promptSessionSelect() + + // Then + expect(ensureAuthenticatedUser).toHaveBeenCalledWith({}, {forceNewSession: true}) + expect(renderTextPrompt).toHaveBeenCalledWith({ + message: 'Enter an alias for this account (e.g. your email or a nickname)', + }) + expect(sessionStore.setSessionAlias).toHaveBeenCalledWith('new-user-id', 'my-work-account') + expect(result).toEqual('my-work-account') + }) + test('does not update alias for existing session when not provided', async () => { // Given vi.mocked(sessionStore.fetch).mockResolvedValue(mockSessions) diff --git a/packages/cli-kit/src/public/node/session-prompt.ts b/packages/cli-kit/src/public/node/session-prompt.ts index bb78d17218b..278be669125 100644 --- a/packages/cli-kit/src/public/node/session-prompt.ts +++ b/packages/cli-kit/src/public/node/session-prompt.ts @@ -1,4 +1,4 @@ -import {renderSelectPrompt} from './ui.js' +import {renderSelectPrompt, renderTextPrompt} from './ui.js' import {ensureAuthenticatedUser} from './session.js' import {identityFqdn} from './context/fqdn.js' import * as sessionStore from '../../private/node/session/store.js' @@ -37,13 +37,23 @@ function buildSessionChoices(sessions: Sessions, fqdn: string): SessionChoice[] /** * Handles the new login flow. + * If no alias is stored (email couldn't be fetched), prompts the user for a friendly alias. * * @returns The alias of the authenticated user. */ async function handleNewLogin(): Promise { const result = await ensureAuthenticatedUser({}, {forceNewSession: true}) const alias = await sessionStore.getSessionAlias(result.userId) - return alias ?? result.userId + + if (!alias) { + const userAlias = await renderTextPrompt({ + message: 'Enter an alias for this account (e.g. your email or a nickname)', + }) + await sessionStore.setSessionAlias(result.userId, userAlias) + return userAlias + } + + return alias } /**