Skip to content

Commit 8175e1b

Browse files
committed
fix: Prevent login page redirect loop and race conditions
1 parent 8f5cae0 commit 8175e1b

File tree

3 files changed

+31
-8
lines changed

3 files changed

+31
-8
lines changed

app/src/app/login/LoginClientBoundary.tsx

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export default function LoginClientBoundary() {
3030
const nextPath = searchParams.get('next');
3131
const authStatus = useAtomValue(authStatusAtom);
3232
const [pendingRedirect, setPendingRedirect] = useAtom(pendingRedirectAtom);
33+
const loginActionIsActive = useAtomValue(loginActionInProgressAtom);
3334
const [lastPathBeforeAuthChange, setLastPathBeforeAuthChange] = useAtom(lastKnownPathBeforeAuthChangeAtom);
3435
const pathname = usePathname();
3536
const [state, send] = useAtom(loginPageMachineAtom);
@@ -69,8 +70,9 @@ export default function LoginClientBoundary() {
6970
return;
7071
}
7172

72-
// If the machine wants to redirect AND no redirect is currently pending, set one.
73-
if (state.matches('redirecting') && pendingRedirect === null) {
73+
// If the machine wants to redirect AND no redirect is currently pending,
74+
// AND a login action isn't already handling the redirect, set one.
75+
if (state.matches('redirecting') && pendingRedirect === null && !loginActionIsActive) {
7476
let targetRedirectPath: string;
7577

7678
// Prioritize the path stored before a cross-tab auth change.
@@ -85,7 +87,7 @@ export default function LoginClientBoundary() {
8587
// Clear the last path atom after using it for a redirect.
8688
setLastPathBeforeAuthChange(null);
8789
}
88-
}, [clientMounted, state, nextPath, lastPathBeforeAuthChange, setLastPathBeforeAuthChange, pendingRedirect, setPendingRedirect]);
90+
}, [clientMounted, state, nextPath, lastPathBeforeAuthChange, setLastPathBeforeAuthChange, pendingRedirect, setPendingRedirect, loginActionIsActive]);
8991

9092
// Render content based on the machine's state.
9193
if (state.matches('showingForm')) {

app/src/atoms/JotaiAppProvider.tsx

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -595,6 +595,10 @@ export const StateInspector = () => {
595595

596596
// Atoms for redirect logic debugging
597597
const pathname = usePathname(); // Get current pathname
598+
const pendingRedirectValue = useAtomValue(pendingRedirectAtom);
599+
const requiredSetupRedirectValue = useAtomValue(requiredSetupRedirectAtom);
600+
const loginActionInProgressValue = useAtomValue(loginActionInProgressAtom);
601+
const lastKnownPathValue = useAtomValue(lastKnownPathBeforeAuthChangeAtom);
598602
const restClientFromAtom = useAtomValue(importedRestClientAtom);
599603
const activityStandardFromAtom = useAtomValue(activityCategoryStandardSettingAtomAsync);
600604
const numberOfRegionsFromAtom = useAtomValue(numberOfRegionsAtomAsync);
@@ -660,6 +664,12 @@ export const StateInspector = () => {
660664
loading: workerStatusValue.loading,
661665
error: workerStatusValue.error,
662666
},
667+
navigationState: {
668+
pendingRedirect: pendingRedirectValue,
669+
requiredSetupRedirect: requiredSetupRedirectValue,
670+
loginActionInProgress: loginActionInProgressValue,
671+
lastKnownPathBeforeAuthChange: lastKnownPathValue,
672+
},
663673
redirectRelevantState: {
664674
authCheckDone: authLoadableValue.state !== 'loading',
665675
isRestClientReady: !!restClientFromAtom,
@@ -765,9 +775,14 @@ export const StateInspector = () => {
765775
</div>
766776

767777
<div>
768-
<strong>Redirect Relevant State:</strong>
778+
<strong>Navigation & Redirect Debugging:</strong>
769779
<div className="pl-4 mt-1 space-y-1">
770780
<div><strong>Pathname:</strong> {pathname}</div>
781+
<div><strong>Pending Redirect:</strong> {pendingRedirectValue || 'None'}</div>
782+
<div><strong>Required Setup Redirect:</strong> {requiredSetupRedirectValue || 'None'}</div>
783+
<div><strong>Login Action in Progress:</strong> {loginActionInProgressValue ? 'Yes' : 'No'}</div>
784+
<div><strong>Last Known Path (pre-auth):</strong> {lastKnownPathValue || 'None'}</div>
785+
<hr className="my-1 border-gray-500" />
771786
<div><strong>Auth Check Done:</strong> {authLoadableValue.state !== 'loading' ? 'Yes' : 'No'}</div>
772787
<div><strong>REST Client Ready:</strong> {restClientFromAtom ? 'Yes' : 'No'}</div>
773788
<div><strong>Activity Standard:</strong> {activityStandardFromAtom === null ? 'Null' : JSON.stringify(activityStandardFromAtom)}</div>

app/src/atoms/auth.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,19 +294,25 @@ export const loginAtom = atom(
294294

295295
// Successfully authenticated (200 OK and is_authenticated: true implied or explicit from responseData)
296296
// After successful login, the backend sets cookies.
297-
// We need to refresh our authStatusCoreAtom to read the new state.
298-
await set(fetchAuthStatusAtom);
299297

300298
// Signal that a login action is now managing the redirect.
301-
// This must be set BEFORE pendingRedirectAtom to allow RedirectHandler to identify the source.
299+
// This must be set BEFORE awaiting the auth status fetch to prevent race conditions
300+
// with other components that might react to the auth state change.
302301
set(loginActionInProgressAtom, true);
303302

303+
// We need to refresh our authStatusCoreAtom to read the new state.
304+
await set(fetchAuthStatusAtom);
305+
304306
// The call to `await set(fetchAuthStatusAtom)` will now handle triggering the cross-tab sync
305307
// if the authentication state has changed.
306308

307309
// Set pendingRedirectAtom to trigger navigation.
308310
// Prioritize nextPath if available and valid, otherwise default to '/'.
309-
const redirectTarget = nextPath && nextPath.startsWith('/') ? nextPath : '/';
311+
let redirectTarget = nextPath && nextPath.startsWith('/') ? nextPath : '/';
312+
// Ensure we don't redirect back to the login page after a successful login.
313+
if (redirectTarget === '/login') {
314+
redirectTarget = '/';
315+
}
310316
set(pendingRedirectAtom, redirectTarget);
311317

312318
} catch (error) {

0 commit comments

Comments
 (0)