From 073e55cbe4ec39373eff03a9bf9766d7790b77e9 Mon Sep 17 00:00:00 2001 From: Konstantinos Thermos Date: Tue, 11 Aug 2026 11:21:58 +0300 Subject: [PATCH] fix(auth): redirect authenticated users away from /login and /register (#1822) Landing on /login while already signed in, by typing the URL or going back, showed the login form again (#1810). PublicRoute takes a redirectAuthed flag on those two routes and sends the visitor to the start destination instead, capturing the auth state at mount so a fresh login is left alone during the takeoff animation. Only from a bare URL: ?redirect=, ?invite=, ?oidc_code= and ?oidc_error= all have to reach useLogin intact. --- client/src/App.test.tsx | 51 +++++++++++++++++++++++++++++++++++++++++ client/src/App.tsx | 27 ++++++++++++++++++---- 2 files changed, 73 insertions(+), 5 deletions(-) diff --git a/client/src/App.test.tsx b/client/src/App.test.tsx index 788dd1f03..ecee3d3a6 100644 --- a/client/src/App.test.tsx +++ b/client/src/App.test.tsx @@ -291,6 +291,57 @@ describe('Public routes', () => { }) }) +// ── PublicRoute — redirect already-authenticated visitors ───────────────────── + +describe('PublicRoute — already authenticated', () => { + /** + * The bounce goes to START_DESTINATION_ROUTE, so RootRedirect resolves where + * it actually lands — which needs settings, exactly like the cases above. + */ + function seedAuthedOnDashboard() { + seedAuth({ isAuthenticated: true, user: buildUser() }) + useSettingsStore.setState({ + isLoaded: true, + settings: buildSettings({ start_page: 'dashboard' }), + }) + } + + it('FE-COMP-APP-012a: /login sends an authenticated visitor to the start destination', async () => { + seedAuthedOnDashboard() + renderApp('/login') + await waitFor(() => expect(currentPath()).toBe('/dashboard')) + expect(screen.queryByText('Login')).not.toBeInTheDocument() + }) + + it('FE-COMP-APP-012b: /register does the same', async () => { + seedAuthedOnDashboard() + renderApp('/register') + await waitFor(() => expect(currentPath()).toBe('/dashboard')) + expect(screen.queryByText('Login')).not.toBeInTheDocument() + }) + + it('FE-COMP-APP-012c: /login with a ?redirect= target is left to useLogin (OAuth consent handoff), not bounced', async () => { + // The consent page parks its URL in ?redirect= when it needs a login; that + // flow must reach the form even for an authenticated visitor, so the guard + // stays out of the way whenever a redirect target is present. + seedAuthedOnDashboard() + const target = '/login?redirect=' + encodeURIComponent('/oauth/consent?client_id=x') + renderApp(target) + await waitFor(() => expect(screen.getByText('Login')).toBeInTheDocument()) + expect(currentPath()).toBe(target) + }) + + it('FE-COMP-APP-012d: /register?invite= reaches the form so the token can be validated', async () => { + // An invite link is the one way someone joins a trip they were invited to + // from an account they are already signed into; bouncing it would swallow + // the token silently. + seedAuthedOnDashboard() + renderApp('/register?invite=abc123') + await waitFor(() => expect(screen.getByText('Login')).toBeInTheDocument()) + expect(currentPath()).toBe('/register?invite=abc123') + }) +}) + // ── App — on-mount effects ───────────────────────────────────────────────────── describe('App — on-mount effects', () => { diff --git a/client/src/App.tsx b/client/src/App.tsx index b98133640..18a057e8a 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -1,4 +1,4 @@ -import React, { useEffect, useState, ReactNode, Suspense } from 'react' +import React, { useEffect, useState, useRef, ReactNode, Suspense } from 'react' import { Routes, Route, Navigate, useLocation } from 'react-router' import { useAuthStore } from './store/authStore' import { useSettingsStore } from './store/settingsStore' @@ -20,7 +20,7 @@ import { lazyWithRetry } from './utils/lazyWithRetry' import { useIsPhone } from './mobile/useIsPhone' import { TranslationProvider, useTranslation } from './i18n' import { authApi, tripsApi } from './api/client' -import { readStartDestination, tripStartPath, DEFAULT_START_PAGE, DEFAULT_START_TRIP_TAB, SETTINGS_WAIT_MS } from './utils/startDestination' +import { readStartDestination, tripStartPath, DEFAULT_START_PAGE, DEFAULT_START_TRIP_TAB, SETTINGS_WAIT_MS, START_DESTINATION_ROUTE } from './utils/startDestination' import { usePermissionsStore, PermissionLevel } from './store/permissionsStore' import { useInAppNotificationListener } from './hooks/useInAppNotificationListener.ts' import { registerSyncTriggers, unregisterSyncTriggers } from './sync/syncTriggers' @@ -155,8 +155,25 @@ function ProtectedRoute({ children, adminRequired = false, addonId }: ProtectedR * never sees them — including /login, where someone lands when everything else * failed, and the two anonymous share pages. */ -function PublicRoute({ children }: { children: React.ReactNode }) { +function PublicRoute({ children, redirectAuthed = false }: { children: React.ReactNode; redirectAuthed?: boolean }) { const location = useLocation() + // redirectAuthed (only /login and /register) bounces a visitor who is already + // authenticated when they land here — manual URL, browser back button (#1810). + // Only on a bare URL, because every parameter this page takes is a flow that + // has to reach useLogin intact: ?redirect= carries the OAuth consent handoff + // (bouncing drops it, or loops if the server still reports login_required), + // ?invite= puts the form into register mode against a validated token, and + // ?oidc_code= / ?oidc_error= complete an external login. A visitor with any of + // those in the URL asked for this page on purpose. + // Capture the flag at mount so a fresh login is left alone: it flips + // isAuthenticated to true while the takeoff animation still plays here, before + // useLogin navigates away. + // The target is START_DESTINATION_ROUTE, not /dashboard, so the bounce honours + // the start-page preference the same way useLogin does. + const wasAuthenticated = useRef(useAuthStore.getState().isAuthenticated) + if (redirectAuthed && wasAuthenticated.current && !location.search) { + return + } return ( {children} @@ -372,10 +389,10 @@ export default function App() { }> } /> - } /> + } /> } /> } /> - } /> + } /> } /> } /> {/* OAuth 2.1 consent page — intentionally outside ProtectedRoute */}