diff --git a/src/libs/Navigation/AppNavigator/AuthScreensInitHandler.tsx b/src/libs/Navigation/AppNavigator/AuthScreensInitHandler.tsx index 506e26f40121..c93504e70275 100644 --- a/src/libs/Navigation/AppNavigator/AuthScreensInitHandler.tsx +++ b/src/libs/Navigation/AppNavigator/AuthScreensInitHandler.tsx @@ -166,7 +166,7 @@ function AuthScreensInitHandler() { }, [session?.accountID, session?.email]); useEffect(() => { - const isLoggingInAsNewUser = !!session?.email && SessionUtils.isLoggingInAsNewUser(currentUrl, session.email); + const isLoggingInAsNewUser = !!session?.email && SessionUtils.isLoggingInAsNewUser(currentUrl, session.email, Session.isDelegateSession(session)); // Sign out the current user if we're transitioning with a different user const isTransitioning = currentUrl.includes(ROUTES.TRANSITION_BETWEEN_APPS); const isSupportalTransition = currentUrl.includes('authTokenType=support'); diff --git a/src/libs/SessionUtils.ts b/src/libs/SessionUtils.ts index b90b19acbbd1..7a8e0ceabc2e 100644 --- a/src/libs/SessionUtils.ts +++ b/src/libs/SessionUtils.ts @@ -8,7 +8,13 @@ const NEW_PARTNER_USER_ID_PREFIX = 'expensify.cash-'; /** * Determine if the transitioning user is logging in as a new user. */ -function isLoggingInAsNewUser(transitionURL?: string, sessionEmail?: string): boolean { +function isLoggingInAsNewUser(transitionURL?: string, sessionEmail?: string, isCopilotSession?: boolean): boolean { + // A copilot session is keyed to the account being worked on, so it never matches the address the transition link + // was generated for. The person behind both addresses is the copilot, so this is not a different user signing in. + if (isCopilotSession) { + return false; + } + // The OldDot mobile app does not URL encode the parameters, but OldDot web // does. We don't want to deploy OldDot mobile again, so as a work around we // compare the session email to both the decoded and raw email from the transition link. diff --git a/src/pages/LogOutPreviousUserPage.tsx b/src/pages/LogOutPreviousUserPage.tsx index 770e76fc4c18..8e8aebe273c9 100644 --- a/src/pages/LogOutPreviousUserPage.tsx +++ b/src/pages/LogOutPreviousUserPage.tsx @@ -10,7 +10,7 @@ import {isLoggingInAsDelegate as isLoggingInAsDelegateSessionUtils, isLoggingInA import Navigation from '@navigation/Navigation'; import type {AuthScreensParamList} from '@navigation/types'; -import {signInWithShortLivedAuthToken, signInWithSupportAuthToken, signOutAndRedirectToSignIn} from '@userActions/Session'; +import {signInWithShortLivedAuthToken, signInWithSupportAuthToken, isDelegateSession, signOutAndRedirectToSignIn} from '@userActions/Session'; import CONFIG from '@src/CONFIG'; import CONST from '@src/CONST'; @@ -37,7 +37,7 @@ function LogOutPreviousUserPage({route}: LogOutPreviousUserPageProps) { useEffect(() => { const sessionEmail = session?.email; const transitionURL = CONFIG.IS_HYBRID_APP ? `${CONST.DEEPLINK_BASE_URL}${initialURL ?? ''}` : initialURL; - const isLoggingInAsNewUser = isLoggingInAsNewUserSessionUtils(transitionURL ?? undefined, sessionEmail); + const isLoggingInAsNewUser = isLoggingInAsNewUserSessionUtils(transitionURL ?? undefined, sessionEmail, isDelegateSession(session)); const isSupportalLogin = authTokenType === CONST.AUTH_TOKEN_TYPES.SUPPORT; if (isLoggingInAsNewUser) { @@ -77,7 +77,7 @@ function LogOutPreviousUserPage({route}: LogOutPreviousUserPageProps) { useEffect(() => { const sessionEmail = session?.email; const transitionURL = CONFIG.IS_HYBRID_APP ? `${CONST.DEEPLINK_BASE_URL}${initialURL ?? ''}` : initialURL; - const isLoggingInAsNewUser = isLoggingInAsNewUserSessionUtils(transitionURL ?? undefined, sessionEmail); + const isLoggingInAsNewUser = isLoggingInAsNewUserSessionUtils(transitionURL ?? undefined, sessionEmail, isDelegateSession(session)); // We don't want to navigate to the exitTo route when creating a new workspace from a deep link, // because we already handle creating the optimistic policy and navigating to it in App.setUpPoliciesAndNavigate, // which is already called when AuthScreens mounts. diff --git a/tests/ui/AuthScreensInitHandlerTest.tsx b/tests/ui/AuthScreensInitHandlerTest.tsx index a216dccd35e2..a2f8afc1fa2c 100644 --- a/tests/ui/AuthScreensInitHandlerTest.tsx +++ b/tests/ui/AuthScreensInitHandlerTest.tsx @@ -9,6 +9,7 @@ import getCurrentUrl from '@libs/Navigation/currentUrl'; import Navigation from '@libs/Navigation/Navigation'; import Pusher from '@libs/Pusher'; import {didUserLogInDuringSession, isLoggingInAsNewUser} from '@libs/SessionUtils'; +import type * as RealSessionUtils from '@libs/SessionUtils'; import {openApp} from '@userActions/App'; import {signOutAndRedirectToSignIn} from '@userActions/Session'; @@ -32,6 +33,11 @@ import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatch const TEST_ACCOUNT_ID = 1; const QA_APP_KEY = 'qa-app-key'; +const ACTOR = 'actor@example.com'; +const DELEGATOR = 'delegator@example.com'; + +// The transition sign-out is only worth testing against the real predicate, so the mock can be pointed at the real one. +const realIsLoggingInAsNewUser = jest.requireActual('@libs/SessionUtils').isLoggingInAsNewUser; jest.mock('@libs/ApiUtils', () => ({ ...jest.requireActual>('@libs/ApiUtils'), @@ -96,6 +102,7 @@ jest.mock('@userActions/Report', () => ({ })); jest.mock('@userActions/Session', () => ({ + ...jest.requireActual>('@userActions/Session'), signOutAndRedirectToSignIn: jest.fn(), cleanupSession: jest.fn(), })); @@ -274,6 +281,26 @@ describe('AuthScreensInitHandler', () => { expect(signOutAndRedirectToSignIn).toHaveBeenCalledWith(false, false, true, undefined, CONST.SIGN_OUT_REASON.LOGIN_AS_NEW_USER); }); + it.each([ + ['a copilot working inside the delegator account', CONST.AUTH_TOKEN_TYPES.DELEGATE, DELEGATOR, false], + ['an ordinary account the link does not name', undefined, 'test@test.com', true], + ])('decides the transition sign-out for %s with the real predicate', async (_description, authTokenType, sessionEmail, expectSignOut) => { + mockedGetCurrentUrl.mockReturnValue(`https://new.expensify.com/transition?email=${ACTOR}&shortLivedAuthToken=token`); + mockedIsLoggingInAsNewUser.mockImplementation(realIsLoggingInAsNewUser); + + await Onyx.merge(ONYXKEYS.SESSION, {accountID: TEST_ACCOUNT_ID, email: sessionEmail, authTokenType}); + await waitForBatchedUpdates(); + + renderAuthScreensInitHandler(); + await waitForBatchedUpdatesWithAct(); + + if (expectSignOut) { + expect(signOutAndRedirectToSignIn).toHaveBeenCalledWith(false, false, true, undefined, CONST.SIGN_OUT_REASON.LOGIN_AS_NEW_USER); + return; + } + expect(signOutAndRedirectToSignIn).not.toHaveBeenCalled(); + }); + it('calls openApp when didUserLogInDuringSession returns true', async () => { mockedDidUserLogInDuringSession.mockReturnValue(true); diff --git a/tests/unit/LogOutPreviousUserPageTest.tsx b/tests/unit/LogOutPreviousUserPageTest.tsx new file mode 100644 index 000000000000..bdaa77dbc561 --- /dev/null +++ b/tests/unit/LogOutPreviousUserPageTest.tsx @@ -0,0 +1,96 @@ +import {render} from '@testing-library/react-native'; + +import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types'; + +import type {AuthScreensParamList} from '@navigation/types'; + +import LogOutPreviousUserPage from '@pages/LogOutPreviousUserPage'; + +import {signOutAndRedirectToSignIn} from '@userActions/Session'; + +import CONST from '@src/CONST'; +import ONYXKEYS from '@src/ONYXKEYS'; +import type SCREENS from '@src/SCREENS'; + +import React from 'react'; +import Onyx from 'react-native-onyx'; + +import createMock from '../utils/createMock'; +import waitForBatchedUpdates from '../utils/waitForBatchedUpdates'; +import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct'; +import wrapOnyxWithWaitForBatchedUpdates from '../utils/wrapOnyxWithWaitForBatchedUpdates'; + +const ACTOR = 'actor@example.com'; +const DELEGATOR = 'delegator@example.com'; +const UNRELATED = 'unrelated@example.com'; + +jest.mock('@components/InitialURLContextProvider', () => ({ + useInitialURLState: () => ({initialURL: `https://new.expensify.com/transition?email=${ACTOR}&shortLivedAuthToken=token`, isAuthenticatedAtStartup: false}), + useInitialURLActions: () => ({setIsAuthenticatedAtStartup: jest.fn()}), +})); + +jest.mock('@libs/Navigation/Navigation', () => ({ + __esModule: true, + default: { + isNavigationReady: jest.fn(() => Promise.resolve()), + goBack: jest.fn(), + navigate: jest.fn(), + }, +})); + +jest.mock('@userActions/Session', () => ({ + ...jest.requireActual>('@userActions/Session'), + signOutAndRedirectToSignIn: jest.fn(), + signInWithShortLivedAuthToken: jest.fn(), + signInWithSupportAuthToken: jest.fn(), +})); + +type TransitionScreenProps = PlatformStackScreenProps; + +function renderLogOutPreviousUserPage() { + return render( + ({params: {shortLivedAuthToken: 'token'}})} + navigation={createMock({})} + />, + ); +} + +describe('LogOutPreviousUserPage', () => { + beforeAll(() => { + Onyx.init({keys: ONYXKEYS}); + }); + + beforeEach(async () => { + jest.clearAllMocks(); + wrapOnyxWithWaitForBatchedUpdates(Onyx); + await Onyx.clear(); + await waitForBatchedUpdates(); + }); + + it('keeps a copilot signed in when the transition link was generated for the copilot themselves', async () => { + // Given: a live copilot session on the delegator account, and a transition link whose email is the copilot's own address + await Onyx.merge(ONYXKEYS.SESSION, {accountID: 1, email: DELEGATOR, authTokenType: CONST.AUTH_TOKEN_TYPES.DELEGATE}); + await waitForBatchedUpdates(); + + // When: the transition page mounts + renderLogOutPreviousUserPage(); + await waitForBatchedUpdatesWithAct(); + + // Then: nobody is signed out, so the copilot session survives the transition + expect(signOutAndRedirectToSignIn).not.toHaveBeenCalled(); + }); + + it('still signs the previous user out when a different person arrives over the same transition link', async () => { + // Given: an ordinary session belonging to someone the link does not name + await Onyx.merge(ONYXKEYS.SESSION, {accountID: 2, email: UNRELATED}); + await waitForBatchedUpdates(); + + // When: the transition page mounts + renderLogOutPreviousUserPage(); + await waitForBatchedUpdatesWithAct(); + + // Then: the previous user is signed out for the new one + expect(signOutAndRedirectToSignIn).toHaveBeenCalledWith(false, false, true, undefined, CONST.SIGN_OUT_REASON.LOGIN_AS_NEW_USER); + }); +}); diff --git a/tests/unit/SessionUtilsTest.ts b/tests/unit/SessionUtilsTest.ts index 392dd016dc60..0e9086d2c48b 100644 --- a/tests/unit/SessionUtilsTest.ts +++ b/tests/unit/SessionUtilsTest.ts @@ -1,4 +1,8 @@ -import {checkIfShouldUseNewPartnerName, getPartnerCredentials, isAgentEmail, isLoggingInAsDelegate} from '@src/libs/SessionUtils'; +import {checkIfShouldUseNewPartnerName, getPartnerCredentials, isAgentEmail, isLoggingInAsDelegate, isLoggingInAsNewUser} from '@src/libs/SessionUtils'; + +// The address of the person who clicked the link, and the address of the account that person is a copilot of. +const ACTOR = 'actor@example.com'; +const DELEGATOR = 'delegator@example.com'; function mockHybridAppConfig(isHybridApp: boolean): () => void { // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment @@ -94,6 +98,39 @@ describe('SessionUtils', () => { }); }); + describe('isLoggingInAsNewUser', () => { + // The three shapes a copilot transition link arrives in, recorded against the session email each caller holds. + describe('transition link shapes', () => { + test.each([ + ['session holds the copilot account, link names the actor, no delegator address', `?email=${ACTOR}`, DELEGATOR, false, true], + ['session holds the copilot account, link names the actor and the copilot account', `?email=${ACTOR}&delegatorEmail=${DELEGATOR}`, DELEGATOR, false, false], + ['session holds the copilot account, link names the actor twice', `?email=${ACTOR}&delegatorEmail=${ACTOR}`, DELEGATOR, false, true], + ['session holds the copilot account, link names the copilot account', `?email=${DELEGATOR}`, DELEGATOR, false, false], + ['session holds the actor account, link names the actor', `?email=${ACTOR}`, ACTOR, false, false], + ['session holds the actor account, link names the copilot account', `?email=${DELEGATOR}`, ACTOR, false, true], + [ + 'full link with the actor as the first param, session holds the copilot account', + `https://new.expensify.com/transition?email=${ACTOR}&delegatorEmail=${DELEGATOR}`, + DELEGATOR, + false, + false, + ], + ['full link with the actor as the first param, no delegator address', `https://new.expensify.com/transition?email=${ACTOR}`, DELEGATOR, false, true], + ])('%s', (_description, transitionURL, sessionEmail, isCopilotSession, expectedResult) => { + expect(isLoggingInAsNewUser(transitionURL, sessionEmail, isCopilotSession)).toBe(expectedResult); + }); + }); + + it('reads a copilot transition link as the same person rather than a new user', () => { + // Given: a copilot working inside the delegator account, and a link generated for the copilot's own address + const transitionURL = `?email=${ACTOR}&shortLivedAuthToken=token`; + + // When: the transition path asks whether a different user is signing in + // Then: the live copilot session answers for the actor, so nobody is signed out + expect(isLoggingInAsNewUser(transitionURL, DELEGATOR, true)).toBe(false); + }); + }); + describe('isLoggingInAsDelegate', () => { test.each([ ['should return false when transitionURL is undefined', undefined, false],