mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
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.
This commit is contained in:
committed by
GitHub
parent
b4f9e96cc9
commit
073e55cbe4
@@ -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', () => {
|
||||
|
||||
+22
-5
@@ -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 <Navigate to={START_DESTINATION_ROUTE} replace />
|
||||
}
|
||||
return (
|
||||
<ErrorBoundary key={location.pathname} boundaryId="public-route" level="route">
|
||||
{children}
|
||||
@@ -372,10 +389,10 @@ export default function App() {
|
||||
<Suspense fallback={<RouteFallback />}>
|
||||
<Routes>
|
||||
<Route path="/" element={<RootRedirect />} />
|
||||
<Route path="/login" element={<PublicRoute><LoginPage /></PublicRoute>} />
|
||||
<Route path="/login" element={<PublicRoute redirectAuthed><LoginPage /></PublicRoute>} />
|
||||
<Route path="/shared/:token" element={<PublicRoute><SharedTripPage /></PublicRoute>} />
|
||||
<Route path="/public/journey/:token" element={<PublicRoute><JourneyPublicPage /></PublicRoute>} />
|
||||
<Route path="/register" element={<PublicRoute><LoginPage /></PublicRoute>} />
|
||||
<Route path="/register" element={<PublicRoute redirectAuthed><LoginPage /></PublicRoute>} />
|
||||
<Route path="/forgot-password" element={<PublicRoute><ForgotPasswordPage /></PublicRoute>} />
|
||||
<Route path="/reset-password" element={<PublicRoute><ResetPasswordPage /></PublicRoute>} />
|
||||
{/* OAuth 2.1 consent page — intentionally outside ProtectedRoute */}
|
||||
|
||||
Reference in New Issue
Block a user