Skip to content

Commit 189b876

Browse files
authored
Merge pull request Expensify#66093 from LorenzoBloedow/use-onyx-auth-migration
Migrate from withOnyx to useOnyx in AuthScreens
2 parents 332d296 + 6301c24 commit 189b876

6 files changed

Lines changed: 214 additions & 45 deletions

File tree

src/libs/Navigation/AppNavigator/AuthScreens.tsx

Lines changed: 6 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import type {RouteProp} from '@react-navigation/native';
22
import {useNavigation} from '@react-navigation/native';
33
import React, {memo, useContext, useEffect, useMemo, useRef, useState} from 'react';
4-
import type {OnyxEntry} from 'react-native-onyx';
5-
import {withOnyx} from 'react-native-onyx';
64
import ComposeProviders from '@components/ComposeProviders';
75
import DelegateNoAccessModalProvider from '@components/DelegateNoAccessModalProvider';
86
import FullScreenLoadingIndicator from '@components/FullscreenLoadingIndicator';
@@ -79,17 +77,6 @@ import WelcomeVideoModalNavigator from './Navigators/WelcomeVideoModalNavigator'
7977
import TestDriveDemoNavigator from './TestDriveDemoNavigator';
8078
import useRootNavigatorScreenOptions from './useRootNavigatorScreenOptions';
8179

82-
type AuthScreensProps = {
83-
/** Session of currently logged in user */
84-
session: OnyxEntry<OnyxTypes.Session>;
85-
86-
/** The report ID of the last opened public room as anonymous user */
87-
lastOpenedPublicRoomID: OnyxEntry<string>;
88-
89-
/** The last Onyx update ID was applied to the client */
90-
initialLastUpdateIDAppliedToClient: OnyxEntry<number>;
91-
};
92-
9380
const loadAttachmentModalScreen = () => require<ReactComponentModule>('../../../pages/media/AttachmentModalScreen').default;
9481
const loadValidateLoginPage = () => require<ReactComponentModule>('../../../pages/ValidateLoginPage').default;
9582
const loadLogOutPreviousUserPage = () => require<ReactComponentModule>('../../../pages/LogOutPreviousUserPage').default;
@@ -163,7 +150,7 @@ const modalScreenListenersWithCancelSearch = {
163150
},
164151
};
165152

166-
function AuthScreens({session, lastOpenedPublicRoomID, initialLastUpdateIDAppliedToClient}: AuthScreensProps) {
153+
function AuthScreens() {
167154
const theme = useTheme();
168155
const StyleUtils = useStyleUtils();
169156
const {shouldUseNarrowLayout} = useResponsiveLayout();
@@ -190,6 +177,10 @@ function AuthScreens({session, lastOpenedPublicRoomID, initialLastUpdateIDApplie
190177
// State to track whether the delegator's authentication is completed before displaying data
191178
const [isDelegatorFromOldDotIsReady, setIsDelegatorFromOldDotIsReady] = useState(false);
192179

180+
const [session] = useOnyx(ONYXKEYS.SESSION, {canBeMissing: true});
181+
const [lastOpenedPublicRoomID] = useOnyx(ONYXKEYS.LAST_OPENED_PUBLIC_ROOM_ID, {canBeMissing: true});
182+
const [initialLastUpdateIDAppliedToClient] = useOnyx(ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT, {canBeMissing: true});
183+
193184
// On HybridApp we need to prevent flickering during transition to OldDot
194185
const shouldRenderOnboardingExclusivelyOnHybridApp = useMemo(() => {
195186
return CONFIG.IS_HYBRID_APP && Navigation.getActiveRoute().includes(ROUTES.ONBOARDING_INTERESTED_FEATURES.route) && isOnboardingCompleted === true;
@@ -744,18 +735,4 @@ AuthScreens.displayName = 'AuthScreens';
744735

745736
const AuthScreensMemoized = memo(AuthScreens, () => true);
746737

747-
// Migration to useOnyx cause re-login if logout from deeplinked report in desktop app
748-
// Further analysis required and more details can be seen here:
749-
// https://github.com/Expensify/App/issues/50560
750-
// eslint-disable-next-line
751-
export default withOnyx<AuthScreensProps, AuthScreensProps>({
752-
session: {
753-
key: ONYXKEYS.SESSION,
754-
},
755-
lastOpenedPublicRoomID: {
756-
key: ONYXKEYS.LAST_OPENED_PUBLIC_ROOM_ID,
757-
},
758-
initialLastUpdateIDAppliedToClient: {
759-
key: ONYXKEYS.ONYX_UPDATES_LAST_UPDATE_ID_APPLIED_TO_CLIENT,
760-
},
761-
})(AuthScreensMemoized);
738+
export default AuthScreensMemoized;

src/libs/Navigation/AppNavigator/PublicScreens.tsx

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import React from 'react';
22
import useStyleUtils from '@hooks/useStyleUtils';
33
import useTheme from '@hooks/useTheme';
44
import createPlatformStackNavigator from '@libs/Navigation/PlatformStackNavigation/createPlatformStackNavigator';
5-
import {InternalPlatformAnimations} from '@libs/Navigation/PlatformStackNavigation/navigationOptions/animation';
5+
import Animations, {InternalPlatformAnimations} from '@libs/Navigation/PlatformStackNavigation/navigationOptions/animation';
66
import type {PublicScreensParamList} from '@navigation/types';
77
import ConnectionCompletePage from '@pages/ConnectionCompletePage';
88
import LogInWithShortLivedAuthTokenPage from '@pages/LogInWithShortLivedAuthTokenPage';
@@ -30,7 +30,12 @@ function PublicScreens() {
3030
{/* The structure for the HOME route has to be the same in public and auth screens. That's why the name for SignInPage is REPORTS_SPLIT_NAVIGATOR. */}
3131
<RootStack.Screen
3232
name={NAVIGATORS.REPORTS_SPLIT_NAVIGATOR}
33-
options={defaultScreenOptions}
33+
options={{
34+
...defaultScreenOptions,
35+
// If you want to change this, make sure there aren't any animation bugs when signing out.
36+
// This was put here to prevent excessive animations when resetting the navigation state in `resetNavigationState`
37+
animation: Animations.NONE,
38+
}}
3439
component={SignInPage}
3540
/>
3641
<RootStack.Screen

src/libs/Network/NetworkStore.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import ONYXKEYS from '@src/ONYXKEYS';
88
import type Credentials from '@src/types/onyx/Credentials';
99

1010
let credentials: Credentials | null | undefined;
11+
let lastShortAuthToken: string | null | undefined;
1112
let authToken: string | null | undefined;
1213
let authTokenType: ValueOf<typeof CONST.AUTH_TOKEN_TYPES> | null;
1314
let currentUserEmail: string | null = null;
@@ -124,6 +125,14 @@ function getAuthToken(): string | null | undefined {
124125
return authToken;
125126
}
126127

128+
function getLastShortAuthToken(): string | null | undefined {
129+
return lastShortAuthToken;
130+
}
131+
132+
function setLastShortAuthToken(newLastAuthToken: string | null) {
133+
lastShortAuthToken = newLastAuthToken;
134+
}
135+
127136
function isSupportRequest(command: string): boolean {
128137
return [
129138
WRITE_COMMANDS.OPEN_APP,
@@ -208,4 +217,6 @@ export {
208217
checkRequiredData,
209218
isSupportAuthToken,
210219
isSupportRequest,
220+
getLastShortAuthToken,
221+
setLastShortAuthToken,
211222
};

src/libs/actions/Session/index.ts

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -56,10 +56,10 @@ import Timing from '@userActions/Timing';
5656
import * as Welcome from '@userActions/Welcome';
5757
import CONFIG from '@src/CONFIG';
5858
import CONST from '@src/CONST';
59+
import NAVIGATORS from '@src/NAVIGATORS';
5960
import ONYXKEYS from '@src/ONYXKEYS';
6061
import type {Route} from '@src/ROUTES';
6162
import ROUTES from '@src/ROUTES';
62-
import SCREENS from '@src/SCREENS';
6363
import type {TryNewDot} from '@src/types/onyx';
6464
import type Credentials from '@src/types/onyx/Credentials';
6565
import type Locale from '@src/types/onyx/Locale';
@@ -651,6 +651,7 @@ function beginGoogleSignIn(token: string | null) {
651651
function signInWithShortLivedAuthToken(authToken: string) {
652652
const {optimisticData, finallyData} = getShortLivedLoginParams();
653653
API.read(READ_COMMANDS.SIGN_IN_WITH_SHORT_LIVED_AUTH_TOKEN, {authToken, skipReauthentication: true}, {optimisticData, finallyData});
654+
NetworkStore.setLastShortAuthToken(authToken);
654655
}
655656

656657
/**
@@ -845,20 +846,11 @@ function clearSignInData() {
845846
}
846847

847848
/**
848-
* Reset all current params of the Home route
849+
* Reset navigation to a brand new state with Home as the initial screen.
849850
*/
850-
function resetHomeRouteParams() {
851+
function resetNavigationState() {
851852
Navigation.isNavigationReady().then(() => {
852-
const routes = navigationRef.current?.getState()?.routes;
853-
const homeRoute = routes?.find((route) => route.name === SCREENS.HOME);
854-
855-
const emptyParams: Record<string, undefined> = {};
856-
Object.keys(homeRoute?.params ?? {}).forEach((paramKey) => {
857-
emptyParams[paramKey] = undefined;
858-
});
859-
860-
Navigation.setParams(emptyParams, homeRoute?.key ?? '');
861-
Onyx.set(ONYXKEYS.IS_CHECKING_PUBLIC_ROOM, false);
853+
navigationRef.resetRoot({index: 0, routes: [{name: NAVIGATORS.REPORTS_SPLIT_NAVIGATOR}]});
862854
});
863855
}
864856

@@ -876,7 +868,7 @@ function cleanupSession() {
876868
PersistedRequests.clear();
877869
NetworkConnection.clearReconnectionCallbacks();
878870
SessionUtils.resetDidUserLogInDuringSession();
879-
resetHomeRouteParams();
871+
resetNavigationState();
880872
clearCache().then(() => {
881873
Log.info('Cleared all cache data', true, {}, true);
882874
});
@@ -1463,4 +1455,5 @@ export {
14631455
MergeIntoAccountAndLogin,
14641456
resetSMSDeliveryFailureStatus,
14651457
clearDisableTwoFactorAuthErrors,
1458+
getShortLivedLoginParams,
14661459
};

src/pages/LogInWithShortLivedAuthTokenPage.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import Log from '@libs/Log';
55
import Navigation from '@libs/Navigation/Navigation';
66
import type {PlatformStackScreenProps} from '@libs/Navigation/PlatformStackNavigation/types';
77
import type {PublicScreensParamList} from '@libs/Navigation/types';
8+
import {getLastShortAuthToken} from '@libs/Network/NetworkStore';
89
import {setAccountError, signInWithShortLivedAuthToken, signInWithSupportAuthToken} from '@userActions/Session';
910
import CONST from '@src/CONST';
1011
import ONYXKEYS from '@src/ONYXKEYS';
@@ -23,6 +24,11 @@ function LogInWithShortLivedAuthTokenPage({route}: LogInWithShortLivedAuthTokenP
2324
// We have to check for both shortLivedAuthToken and shortLivedToken, as the old mobile app uses shortLivedToken, and is not being actively updated.
2425
const token = shortLivedAuthToken || shortLivedToken;
2526

27+
// This is to prevent re-authenticating when logging out if the initial URL (deep link) hasn't changed.
28+
if (token === getLastShortAuthToken()) {
29+
return;
30+
}
31+
2632
if (!account?.isLoading && authTokenType === CONST.AUTH_TOKEN_TYPES.SUPPORT) {
2733
signInWithSupportAuthToken(shortLivedAuthToken);
2834
Navigation.isNavigationReady().then(() => {

tests/ui/SessionTest.tsx

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
import {render} from '@testing-library/react-native';
2+
import {Str} from 'expensify-common';
3+
import {Linking} from 'react-native';
4+
import type {OnyxEntry} from 'react-native-onyx';
5+
import Onyx from 'react-native-onyx';
6+
// eslint-disable-next-line no-restricted-imports, no-restricted-syntax
7+
import * as AppActions from '@libs/actions/App';
8+
import {hasAuthToken, signOutAndRedirectToSignIn} from '@libs/actions/Session';
9+
// eslint-disable-next-line no-restricted-imports, no-restricted-syntax
10+
import * as Session from '@libs/actions/Session';
11+
import {getCurrentUserEmail, setLastShortAuthToken} from '@libs/Network/NetworkStore';
12+
import App from '@src/App';
13+
import CONST from '@src/CONST';
14+
import ONYXKEYS from '@src/ONYXKEYS';
15+
import ROUTES from '@src/ROUTES';
16+
import {createRandomReport} from '../utils/collections/reports';
17+
import * as TestHelper from '../utils/TestHelper';
18+
import waitForBatchedUpdates from '../utils/waitForBatchedUpdates';
19+
import waitForBatchedUpdatesWithAct from '../utils/waitForBatchedUpdatesWithAct';
20+
import waitForNetworkPromises from '../utils/waitForNetworkPromises';
21+
22+
const TEST_USER_ACCOUNT_ID_1 = 123;
23+
const TEST_USER_LOGIN_1 = 'test@test.com';
24+
// cspell:disable-next-line
25+
const TEST_AUTH_TOKEN_1 = 'asdfghjkl';
26+
27+
const TEST_USER_ACCOUNT_ID_2 = 456;
28+
const TEST_USER_LOGIN_2 = 'test2@test.com';
29+
// cspell:disable-next-line
30+
const TEST_AUTH_TOKEN_2 = 'zxcvbnm';
31+
32+
jest.setTimeout(60000);
33+
TestHelper.setupApp();
34+
TestHelper.setupGlobalFetchMock();
35+
36+
const report = createRandomReport(7);
37+
38+
function getInitialURL() {
39+
const params = new URLSearchParams();
40+
41+
params.set('exitTo', `${ROUTES.REPORT}/${report.reportID}`);
42+
params.set('email', TEST_USER_LOGIN_1);
43+
params.set('shortLivedAuthToken', TEST_AUTH_TOKEN_1);
44+
45+
const deeplinkUrl = `${CONST.DEEPLINK_BASE_URL}/transition?${params.toString()}`;
46+
return deeplinkUrl;
47+
}
48+
49+
describe('Deep linking', () => {
50+
let lastVisitedPath: string | undefined;
51+
let originalSignInWithShortLivedAuthToken: typeof Session.signInWithShortLivedAuthToken;
52+
let originalOpenApp: typeof AppActions.openApp;
53+
54+
beforeAll(() => {
55+
originalSignInWithShortLivedAuthToken = Session.signInWithShortLivedAuthToken;
56+
originalOpenApp = AppActions.openApp;
57+
});
58+
59+
beforeEach(() => {
60+
Onyx.connect({
61+
key: ONYXKEYS.LAST_VISITED_PATH,
62+
callback: (val: OnyxEntry<string>) => (lastVisitedPath = val),
63+
});
64+
65+
jest.spyOn(Session, 'signInWithShortLivedAuthToken').mockImplementation(() => {
66+
Onyx.merge(ONYXKEYS.CREDENTIALS, {
67+
login: TEST_USER_LOGIN_1,
68+
autoGeneratedLogin: Str.guid('expensify.cash-'),
69+
autoGeneratedPassword: Str.guid(),
70+
});
71+
Onyx.merge(ONYXKEYS.ACCOUNT, {
72+
validated: true,
73+
isUsingExpensifyCard: false,
74+
});
75+
Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {
76+
[TEST_USER_ACCOUNT_ID_1]: TestHelper.buildPersonalDetails(TEST_USER_LOGIN_1, TEST_USER_ACCOUNT_ID_1, 'Test'),
77+
});
78+
Onyx.merge(ONYXKEYS.SESSION, {
79+
authToken: TEST_AUTH_TOKEN_1,
80+
accountID: TEST_USER_ACCOUNT_ID_1,
81+
email: TEST_USER_LOGIN_1,
82+
encryptedAuthToken: TEST_AUTH_TOKEN_1,
83+
});
84+
Onyx.merge(ONYXKEYS.NVP_PRIVATE_PUSH_NOTIFICATION_ID, 'randomID');
85+
86+
return originalSignInWithShortLivedAuthToken(TEST_AUTH_TOKEN_1);
87+
});
88+
89+
jest.spyOn(AppActions, 'openApp').mockImplementation(async () => {
90+
const originalResult = await originalOpenApp();
91+
await Onyx.set(`${ONYXKEYS.COLLECTION.REPORT}${report.reportID}`, report);
92+
return originalResult;
93+
});
94+
});
95+
96+
afterEach(async () => {
97+
await Onyx.clear();
98+
await waitForNetworkPromises();
99+
jest.clearAllMocks();
100+
lastVisitedPath = undefined;
101+
Linking.setInitialURL('');
102+
setLastShortAuthToken(null);
103+
});
104+
105+
it('should not remember the report path of the last deep link login after signing out and in again', async () => {
106+
expect(hasAuthToken()).toBe(false);
107+
108+
const url = getInitialURL();
109+
// User signs in automatically when the app is rendered because of the deep link
110+
Linking.setInitialURL(url);
111+
const {unmount} = render(<App />);
112+
113+
await waitForBatchedUpdates();
114+
115+
expect(lastVisitedPath).toBe(`/${ROUTES.REPORT}/${report.reportID}`);
116+
117+
expect(hasAuthToken()).toBe(true);
118+
119+
signOutAndRedirectToSignIn();
120+
121+
await waitForBatchedUpdatesWithAct();
122+
123+
expect(hasAuthToken()).toBe(false);
124+
125+
await TestHelper.signInWithTestUser(TEST_USER_ACCOUNT_ID_2, TEST_USER_LOGIN_2, undefined, TEST_AUTH_TOKEN_2);
126+
127+
await waitForBatchedUpdatesWithAct();
128+
129+
expect(lastVisitedPath).toBeDefined();
130+
expect(lastVisitedPath).not.toBe(`/${ROUTES.REPORT}/${report.reportID}`);
131+
132+
unmount();
133+
await waitForBatchedUpdatesWithAct();
134+
});
135+
136+
it('should not reuse the last deep link and log in again when signing out', async () => {
137+
expect(hasAuthToken()).toBe(false);
138+
139+
const {unmount: unmount1} = render(<App />);
140+
await TestHelper.signInWithTestUser(TEST_USER_ACCOUNT_ID_2, TEST_USER_LOGIN_2, undefined, TEST_AUTH_TOKEN_2);
141+
142+
await waitForBatchedUpdatesWithAct();
143+
144+
expect(hasAuthToken()).toBe(true);
145+
expect(getCurrentUserEmail()).toBe(TEST_USER_LOGIN_2);
146+
// Unmount so we can prepare the deep link login
147+
unmount1();
148+
149+
await waitForBatchedUpdatesWithAct();
150+
151+
const url = getInitialURL();
152+
// User signs in automatically when the app is remounted because of the deep link.
153+
// This overrides the previous sign-in.
154+
Linking.setInitialURL(url);
155+
const {unmount: unmount2} = render(<App />);
156+
157+
await waitForBatchedUpdatesWithAct();
158+
159+
expect(getCurrentUserEmail()).toBe(TEST_USER_LOGIN_1);
160+
161+
signOutAndRedirectToSignIn();
162+
163+
await waitForBatchedUpdatesWithAct();
164+
165+
// In a failing scenario, remounting triggers the sign-in with the deep link again because it still remembers it.
166+
// However, we've implemented a fix so that it does not reuse the last deep link.
167+
unmount2();
168+
const {unmount: unmount3} = render(<App />);
169+
170+
await waitForBatchedUpdatesWithAct();
171+
172+
expect(hasAuthToken()).toBe(false);
173+
174+
unmount3();
175+
await waitForBatchedUpdatesWithAct();
176+
});
177+
});

0 commit comments

Comments
 (0)