Skip to content

Commit 78551d4

Browse files
fix: [M3-8711] - Authentication Race Condition (#12399)
* prevent multiple calls to `redirectToLogin` * fix initial app load redirect * simplify things to prevent bugs --------- Co-authored-by: Banks Nussman <[email protected]>
1 parent 20106b0 commit 78551d4

File tree

3 files changed

+36
-56
lines changed

3 files changed

+36
-56
lines changed

packages/manager/src/OAuth/utils.ts

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
/* eslint-disable no-console */
2-
import { useLocation } from 'react-router-dom';
3-
42
import { CLIENT_ID, LOGIN_ROOT } from 'src/constants';
5-
import { redirectToLogin, revokeToken } from 'src/session';
3+
import { revokeToken } from 'src/session';
64
import {
75
clearUserInput,
86
getEnvLocalStorageOverrides,
@@ -63,24 +61,6 @@ export function getIsLoggedInAsCustomer() {
6361
return token.toLowerCase().includes('admin');
6462
}
6563

66-
export function useOAuth() {
67-
const location = useLocation();
68-
const token = storage.authentication.token.get();
69-
70-
const isPendingAuthentication =
71-
location.pathname.includes('/oauth/callback') ||
72-
location.pathname.includes('/admin/callback') ||
73-
window.location.pathname.includes('/oauth/callback') ||
74-
window.location.pathname.includes('/admin/callback');
75-
76-
// If no token is stored and we are not in the process of authentication, redirect to login.
77-
if (!token && !isPendingAuthentication) {
78-
redirectToLogin(window.location.pathname, window.location.search);
79-
}
80-
81-
return { isPendingAuthentication };
82-
}
83-
8464
function getSafeLoginURL() {
8565
const localStorageOverrides = getEnvLocalStorageOverrides();
8666

packages/manager/src/index.tsx

Lines changed: 23 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import './index.css';
1818
import { ENABLE_DEV_TOOLS } from './constants';
1919
import { Logout } from './layouts/Logout';
2020
import { LinodeThemeWrapper } from './LinodeThemeWrapper';
21-
import { useOAuth } from './OAuth/utils';
2221

2322
const Lish = React.lazy(() => import('src/features/Lish'));
2423

@@ -40,38 +39,6 @@ const store = storeFactory();
4039

4140
setupInterceptors(store);
4241

43-
const Routes = () => {
44-
const { isPendingAuthentication } = useOAuth();
45-
46-
if (isPendingAuthentication) {
47-
return (
48-
<React.Suspense fallback={<SplashScreen />}>
49-
<Switch>
50-
<Route component={OAuthCallbackPage} exact path="/oauth/callback" />
51-
<Route
52-
component={LoginAsCustomerCallback}
53-
exact
54-
path="/admin/callback"
55-
/>
56-
<Route component={Logout} exact path="/logout" />
57-
<Route component={CancelLanding} exact path="/cancel" />
58-
</Switch>
59-
</React.Suspense>
60-
);
61-
}
62-
63-
return (
64-
<React.Suspense fallback={<SplashScreen />}>
65-
<Switch>
66-
<Route component={Logout} exact path="/logout" />
67-
<Route component={CancelLanding} exact path="/cancel" />
68-
<Route component={Lish} path="/linodes/:linodeId/lish/:type" />
69-
<Route component={App} />
70-
</Switch>
71-
</React.Suspense>
72-
);
73-
};
74-
7542
const Main = () => {
7643
if (!navigator.cookieEnabled) {
7744
return <CookieWarning />;
@@ -90,7 +57,29 @@ const Main = () => {
9057
hideIconVariant={false}
9158
maxSnack={3}
9259
>
93-
<Routes />
60+
<React.Suspense fallback={<SplashScreen />}>
61+
<Switch>
62+
<Route
63+
component={OAuthCallbackPage}
64+
exact
65+
path="/oauth/callback"
66+
/>
67+
<Route
68+
component={LoginAsCustomerCallback}
69+
exact
70+
path="/admin/callback"
71+
/>
72+
<Route component={Logout} exact path="/logout" />
73+
<Route component={CancelLanding} exact path="/cancel" />
74+
<Route component={Logout} exact path="/logout" />
75+
<Route component={CancelLanding} exact path="/cancel" />
76+
<Route
77+
component={Lish}
78+
path="/linodes/:linodeId/lish/:type"
79+
/>
80+
<Route component={App} />
81+
</Switch>
82+
</React.Suspense>
9483
</Snackbar>
9584
</Router>
9685
</React.Suspense>

packages/manager/src/request.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,15 +26,26 @@ const handleSuccess: <T extends AxiosResponse<any>>(response: T) => T | T = (
2626
// All errors returned by the actual Linode API are in this shape.
2727
export type LinodeError = { errors: APIError[] };
2828

29+
/**
30+
* Exists to prevent the async `redirectToLogin` function from being called many times
31+
* when many 401 API errors are handled at the same time.
32+
*
33+
* Without this, `redirectToLogin` may be invoked many times before navigation to login actually happens,
34+
* which results in the nonce and code verifier being re-generated, leading to authentication race conditions.
35+
*/
36+
let isRedirectingToLogin = false;
37+
2938
export const handleError = (
3039
error: AxiosError<LinodeError>,
3140
store: ApplicationStore
3241
) => {
3342
if (
3443
error.response &&
3544
error.response.status === 401 &&
36-
!store.getState().pendingUpload
45+
!store.getState().pendingUpload &&
46+
!isRedirectingToLogin
3747
) {
48+
isRedirectingToLogin = true;
3849
clearAuthDataFromLocalStorage();
3950
redirectToLogin(window.location.pathname, window.location.search);
4051
}

0 commit comments

Comments
 (0)