Skip to content

Commit 555d227

Browse files
rlinozrlinoz
authored
Merge pull request Expensify#64280 from callstack-internal/pac-guerreiro/fix/missing-credentials-during-reauthentication
fix: missing credentials during re-authentication
2 parents b96adc2 + 800a97e commit 555d227

5 files changed

Lines changed: 104 additions & 66 deletions

File tree

‎src/libs/Authentication.ts‎

Lines changed: 79 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import redirectToSignIn from './actions/SignInRedirect';
99
import {getAuthenticateErrorMessage} from './ErrorUtils';
1010
import Log from './Log';
1111
import {post} from './Network';
12-
import {getCredentials, setAuthToken, setIsAuthenticating} from './Network/NetworkStore';
12+
import {getCredentials, hasReadRequiredDataFromStorage, setAuthToken, setIsAuthenticating} from './Network/NetworkStore';
1313
import requireParameters from './requireParameters';
1414

1515
type Parameters = {
@@ -31,7 +31,7 @@ Onyx.connect({
3131
},
3232
});
3333

34-
function Authenticate(parameters: Parameters): Promise<Response> | undefined {
34+
function Authenticate(parameters: Parameters): Promise<Response | void> {
3535
const commandName = 'Authenticate';
3636

3737
try {
@@ -42,7 +42,7 @@ function Authenticate(parameters: Parameters): Promise<Response> | undefined {
4242
error: errorMessage,
4343
});
4444
redirectToSignIn(errorMessage);
45-
return;
45+
return Promise.resolve();
4646
}
4747

4848
return post(commandName, {
@@ -69,60 +69,93 @@ function Authenticate(parameters: Parameters): Promise<Response> | undefined {
6969
/**
7070
* Reauthenticate using the stored credentials and redirect to the sign in page if unable to do so.
7171
* @param [command] command name for logging purposes
72+
* @return returns true if reauthentication was successful, false otherwise.
7273
*/
73-
function reauthenticate(command = ''): Promise<void> | undefined {
74+
function reauthenticate(command = ''): Promise<boolean> {
75+
Log.hmmm('Reauthenticate - Attempting re-authentication', {
76+
command,
77+
});
78+
7479
// Prevent re-authentication if authentication with shortLiveToken is in progress
7580
if (isAuthenticatingWithShortLivedToken) {
76-
return;
81+
Log.hmmm('Reauthenticate - Authentication with shortLivedToken is in progress. Re-authentication aborted.', {
82+
command,
83+
});
84+
return Promise.resolve(false);
7785
}
7886

7987
// Prevent any more requests from being processed while authentication happens
8088
setIsAuthenticating(true);
8189

82-
const credentials = getCredentials();
83-
return Authenticate({
84-
useExpensifyLogin: false,
85-
partnerName: CONFIG.EXPENSIFY.PARTNER_NAME,
86-
partnerPassword: CONFIG.EXPENSIFY.PARTNER_PASSWORD,
87-
partnerUserID: credentials?.autoGeneratedLogin,
88-
partnerUserSecret: credentials?.autoGeneratedPassword,
89-
})?.then((response) => {
90-
if (response.jsonCode === CONST.JSON_CODE.UNABLE_TO_RETRY) {
91-
// When a fetch() fails due to a network issue and an error is thrown we won't log the user out. Most likely they
92-
// have a spotty connection and will need to retry reauthenticate when they come back online. Error so it can be handled by the retry mechanism.
93-
throw new Error('Unable to retry Authenticate request');
94-
}
95-
96-
// If authentication fails and we are online then log the user out
97-
if (response.jsonCode !== 200) {
98-
const errorMessage = getAuthenticateErrorMessage(response);
90+
Log.hmmm('Reauthenticate - Waiting for credentials', {
91+
command,
92+
});
93+
94+
return hasReadRequiredDataFromStorage().then(() => {
95+
const credentials = getCredentials();
96+
97+
Log.hmmm('Reauthenticate - Starting authentication process', {
98+
command,
99+
});
100+
101+
return Authenticate({
102+
useExpensifyLogin: false,
103+
partnerName: CONFIG.EXPENSIFY.PARTNER_NAME,
104+
partnerPassword: CONFIG.EXPENSIFY.PARTNER_PASSWORD,
105+
partnerUserID: credentials?.autoGeneratedLogin,
106+
partnerUserSecret: credentials?.autoGeneratedPassword,
107+
}).then((response) => {
108+
if (!response) {
109+
return false;
110+
}
111+
112+
Log.hmmm('Reauthenticate - Processing authentication result', {
113+
command,
114+
});
115+
116+
if (response.jsonCode === CONST.JSON_CODE.UNABLE_TO_RETRY) {
117+
// When a fetch() fails due to a network issue and an error is thrown we won't log the user out. Most likely they
118+
// have a spotty connection and will need to retry reauthenticate when they come back online. Error so it can be handled by the retry mechanism.
119+
throw new Error('Unable to retry Authenticate request');
120+
}
121+
122+
// If authentication fails and we are online then log the user out
123+
if (response.jsonCode !== 200) {
124+
const errorMessage = getAuthenticateErrorMessage(response);
125+
setIsAuthenticating(false);
126+
Log.hmmm('Redirecting to Sign In because we failed to reauthenticate', {
127+
command,
128+
error: errorMessage,
129+
});
130+
redirectToSignIn(errorMessage);
131+
return false;
132+
}
133+
134+
// If we reauthenticate due to an expired delegate token, restore the delegate's original account.
135+
// This is because the credentials used to reauthenticate were for the delegate's original account, and not for the account they were connected as.
136+
if (isConnectedAsDelegate()) {
137+
Log.info('Reauthenticate while connected as a delegate. Restoring original account.');
138+
restoreDelegateSession(response);
139+
return true;
140+
}
141+
142+
// Update authToken in Onyx and in our local variables so that API requests will use the new authToken
143+
updateSessionAuthTokens(response.authToken, response.encryptedAuthToken);
144+
145+
// Note: It is important to manually set the authToken that is in the store here since any requests that are hooked into
146+
// reauthenticate .then() will immediate post and use the local authToken. Onyx updates subscribers lately so it is not
147+
// enough to do the updateSessionAuthTokens() call above.
148+
setAuthToken(response.authToken ?? null);
149+
150+
// The authentication process is finished so the network can be unpaused to continue processing requests
99151
setIsAuthenticating(false);
100-
Log.hmmm('Redirecting to Sign In because we failed to reauthenticate', {
152+
153+
Log.hmmm('Reauthenticate - Re-authentication successful', {
101154
command,
102-
error: errorMessage,
103155
});
104-
redirectToSignIn(errorMessage);
105-
return;
106-
}
107-
108-
// If we reauthenticate due to an expired delegate token, restore the delegate's original account.
109-
// This is because the credentials used to reauthenticate were for the delegate's original account, and not for the account they were connected as.
110-
if (isConnectedAsDelegate()) {
111-
Log.info('Reauthenticate while connected as a delegate. Restoring original account.');
112-
restoreDelegateSession(response);
113-
return;
114-
}
115-
116-
// Update authToken in Onyx and in our local variables so that API requests will use the new authToken
117-
updateSessionAuthTokens(response.authToken, response.encryptedAuthToken);
118-
119-
// Note: It is important to manually set the authToken that is in the store here since any requests that are hooked into
120-
// reauthenticate .then() will immediate post and use the local authToken. Onyx updates subscribers lately so it is not
121-
// enough to do the updateSessionAuthTokens() call above.
122-
setAuthToken(response.authToken ?? null);
123-
124-
// The authentication process is finished so the network can be unpaused to continue processing requests
125-
setIsAuthenticating(false);
156+
157+
return true;
158+
});
126159
});
127160
}
128161

‎src/libs/E2E/actions/e2eLogin.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,10 @@ export default function (): Promise<boolean> {
5252
// authenticate with a predefined user
5353
console.debug('[E2E] Signing in…');
5454
Authenticate(e2eUserCredentials)
55-
?.then((response) => {
55+
.then((response) => {
56+
if (!response) {
57+
return;
58+
}
5659
Onyx.merge(ONYXKEYS.SESSION, {
5760
authToken: response.authToken,
5861
creationDate: new Date().getTime(),

‎src/libs/Middleware/Reauthentication.ts‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -11,39 +11,33 @@ import CONST from '@src/CONST';
1111
import type Middleware from './types';
1212

1313
// We store a reference to the active authentication request so that we are only ever making one request to authenticate at a time.
14-
let isAuthenticating: Promise<void> | null = null;
14+
let isAuthenticating: Promise<boolean> | null = null;
1515

1616
const reauthThrottle = new RequestThrottle('Re-authentication');
1717

18-
function reauthenticate(commandName?: string): Promise<void> | null {
18+
function reauthenticate(commandName?: string): Promise<boolean> {
1919
if (isAuthenticating) {
2020
return isAuthenticating;
2121
}
2222

23-
isAuthenticating =
24-
retryReauthenticate(commandName)
25-
?.then((response) => {
26-
return response;
27-
})
28-
.catch((error) => {
29-
throw error;
30-
})
31-
.finally(() => {
32-
isAuthenticating = null;
33-
}) ?? null;
23+
isAuthenticating = retryReauthenticate(commandName).finally(() => {
24+
// Reset the isAuthenticating state to allow new reauthentication flows to start fresh
25+
isAuthenticating = null;
26+
});
3427

3528
return isAuthenticating;
3629
}
3730

38-
function retryReauthenticate(commandName?: string): Promise<void> | undefined {
39-
return reauthenticateLibs(commandName)?.catch((error: RequestError) => {
31+
function retryReauthenticate(commandName?: string): Promise<boolean> {
32+
return reauthenticateLibs(commandName).catch((error: RequestError) => {
4033
return reauthThrottle
4134
.sleep(error, 'Authenticate')
4235
.then(() => retryReauthenticate(commandName))
4336
.catch(() => {
4437
setIsAuthenticating(false);
4538
Log.hmmm('Redirecting to Sign In because we failed to reauthenticate after multiple attempts', {error});
4639
redirectToSignIn('passwordForm.error.fallback');
40+
return false;
4741
});
4842
});
4943
}
@@ -100,7 +94,11 @@ const Reauthentication: Middleware = (response, request, isFromSequentialQueue)
10094
}
10195

10296
return reauthenticate(request?.commandName)
103-
?.then((authenticateResponse) => {
97+
.then((wasSuccessful) => {
98+
if (!wasSuccessful) {
99+
return;
100+
}
101+
104102
if (isFromSequentialQueue || apiRequestType === CONST.API_REQUEST_TYPE.MAKE_REQUEST_WITH_SIDE_EFFECTS) {
105103
return processWithMiddleware(request, isFromSequentialQueue);
106104
}
@@ -111,7 +109,6 @@ const Reauthentication: Middleware = (response, request, isFromSequentialQueue)
111109
}
112110

113111
replayMainQueue(request);
114-
return authenticateResponse;
115112
})
116113
.catch(() => {
117114
if (isFromSequentialQueue || apiRequestType) {

‎src/libs/actions/Session/AttachmentImageReauthenticator.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ function tryReauthenticate() {
6464
if (isOffline || !active) {
6565
return;
6666
}
67-
reauthenticate()?.catch((error) => {
67+
reauthenticate().catch((error) => {
6868
Log.hmmm('Could not reauthenticate attachment image or receipt', {error});
6969
});
7070
}

‎src/libs/actions/Session/index.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -919,7 +919,12 @@ const reauthenticatePusher = throttle(
919919
() => {
920920
Log.info('[Pusher] Re-authenticating and then reconnecting');
921921
Authentication.reauthenticate(SIDE_EFFECT_REQUEST_COMMANDS.AUTHENTICATE_PUSHER)
922-
?.then(Pusher.reconnect)
922+
.then((wasSuccessful) => {
923+
if (!wasSuccessful) {
924+
return;
925+
}
926+
Pusher.reconnect();
927+
})
923928
.catch(() => {
924929
console.debug('[PusherConnectionManager]', 'Unable to re-authenticate Pusher because we are offline.');
925930
});

0 commit comments

Comments
 (0)