diff --git a/src/pages/account/Account.tsx b/src/pages/account/Account.tsx
index 2168141..cde9795 100644
--- a/src/pages/account/Account.tsx
+++ b/src/pages/account/Account.tsx
@@ -13,6 +13,7 @@ import {
} from '../../services/account/accountService';
import { formatEventDate, formatPrice } from '../../services/events/eventsService';
import StravaSection from './StravaSection';
+import { getLocationReturnPath } from '../login/loginReturnPath';
function roleLabel(role: string) {
if (role === 'admin') return 'Admin';
@@ -347,7 +348,13 @@ function Account() {
if (isLoading) return
Loading...
;
if (!isAuthenticated || !user) {
- return ;
+ return (
+
+ );
}
return ;
}
diff --git a/src/pages/admin/AdminGuard.tsx b/src/pages/admin/AdminGuard.tsx
index 1581ea5..68115ea 100644
--- a/src/pages/admin/AdminGuard.tsx
+++ b/src/pages/admin/AdminGuard.tsx
@@ -1,6 +1,7 @@
import React from 'react';
import { Navigate, useLocation } from 'react-router-dom';
import { useAuth } from '../../services/hooks/useAuth';
+import { getLocationReturnPath } from '../login/loginReturnPath';
interface AdminGuardProps {
children: React.ReactNode;
@@ -14,7 +15,13 @@ function AdminGuard({ children }: AdminGuardProps) {
return Loading...
;
}
if (!isAuthenticated) {
- return ;
+ return (
+
+ );
}
if (!isAdmin) {
return (
diff --git a/src/pages/login/LoginForm.jsx b/src/pages/login/LoginForm.jsx
index 3cd00da..c19758b 100644
--- a/src/pages/login/LoginForm.jsx
+++ b/src/pages/login/LoginForm.jsx
@@ -1,9 +1,12 @@
import React, { useState } from 'react';
-import { Link } from 'react-router-dom';
+import {
+ Link, useLocation, useNavigate,
+} from 'react-router-dom';
import HeaderImage from '../../images/activities/header_bg_1.jpg';
import Header from '../../components/Header';
import { useServiceLocator } from '../../services/ServiceLocatorContext';
import SEO from '../../components/SEO';
+import { getSafeLoginReturnPath } from './loginReturnPath';
function LoginForm() {
const [email, setEmail] = useState('');
@@ -14,6 +17,8 @@ function LoginForm() {
const [currentUser, setCurrentUser] = useState(null);
const [isRegistering, setIsRegistering] = useState(false);
const [resetSent, setResetSent] = useState(false);
+ const location = useLocation();
+ const navigate = useNavigate();
const handleSubmit = async (e) => {
e.preventDefault();
@@ -34,6 +39,7 @@ function LoginForm() {
} else {
const credential = await identityService.signIn(email, password);
setCurrentUser(credential.user);
+ navigate(getSafeLoginReturnPath(location.state?.from), { replace: true });
}
} catch (loginError) {
setError('Failed to authenticate. Please check your credentials and try again.');
diff --git a/src/pages/login/LoginForm.test.jsx b/src/pages/login/LoginForm.test.jsx
new file mode 100644
index 0000000..56d0abf
--- /dev/null
+++ b/src/pages/login/LoginForm.test.jsx
@@ -0,0 +1,191 @@
+/* eslint-env jest */
+import React from 'react';
+import PropTypes from 'prop-types';
+import {
+ MemoryRouter, Route, Routes, useLocation,
+} from 'react-router-dom';
+import { fireEvent, render, screen } from '@testing-library/react';
+import LoginForm from './LoginForm';
+import AdminGuard from '../admin/AdminGuard';
+import { useServiceLocator } from '../../services/ServiceLocatorContext';
+import { useAuth } from '../../services/hooks/useAuth';
+
+jest.mock('../../services/ServiceLocatorContext', () => ({
+ useServiceLocator: jest.fn(),
+}));
+
+jest.mock('../../services/hooks/useAuth', () => ({
+ useAuth: jest.fn(),
+}));
+
+jest.mock('../../components/Header', () => function Header() {
+ return null;
+});
+
+jest.mock('../../components/SEO', () => function SEO() {
+ return null;
+});
+
+function LocationProbe({ label }) {
+ const location = useLocation();
+ return (
+
+ {label}
+ {' '}
+
+ {location.pathname}
+ {location.search}
+ {location.hash}
+
+
+ );
+}
+
+LocationProbe.propTypes = {
+ label: PropTypes.string.isRequired,
+};
+
+function renderLogin(initialEntry = '/login') {
+ return render(
+
+
+ } />
+ } />
+ } />
+
+
+
+ )}
+ />
+
+ ,
+ );
+}
+
+function submitCredentials() {
+ fireEvent.change(screen.getByLabelText('Email address'), {
+ target: { value: 'member@example.com' },
+ });
+ fireEvent.change(screen.getByLabelText('Password'), {
+ target: { value: 'correct horse battery staple' },
+ });
+ fireEvent.click(screen.getByRole('button', { name: 'Sign in' }));
+}
+
+describe('LoginForm return navigation', () => {
+ const signIn = jest.fn();
+ const register = jest.fn();
+ const sendPasswordReset = jest.fn();
+
+ beforeEach(() => {
+ jest.clearAllMocks();
+ signIn.mockResolvedValue({ user: { email: 'member@example.com' } });
+ register.mockResolvedValue({ user: { email: 'member@example.com' } });
+ useServiceLocator.mockReturnValue({
+ isReady: true,
+ services: {
+ identityService: { signIn, register, sendPasswordReset },
+ },
+ });
+ useAuth.mockReturnValue({
+ isLoading: false,
+ isAuthenticated: false,
+ isAdmin: false,
+ });
+ });
+
+ test('round trips query and hash through an actual protected-route redirect', async () => {
+ signIn.mockImplementation(async () => {
+ useAuth.mockReturnValue({
+ isLoading: false,
+ isAuthenticated: true,
+ isAdmin: true,
+ });
+ return { user: { email: 'member@example.com' } };
+ });
+ renderLogin('/admin/events?view=drafts#pending');
+
+ expect(await screen.findByRole('heading', { name: 'Sign in' })).toBeInTheDocument();
+ submitCredentials();
+
+ expect(await screen.findByText('Admin destination')).toBeInTheDocument();
+ expect(screen.getByTestId('location'))
+ .toHaveTextContent('/admin/events?view=drafts#pending');
+ });
+
+ test('returns to the requested safe internal route after sign-in', async () => {
+ renderLogin({
+ pathname: '/login',
+ state: { from: '/discounts?kind=race#active' },
+ });
+
+ submitCredentials();
+
+ expect(await screen.findByText('Discounts destination')).toBeInTheDocument();
+ expect(screen.getByTestId('location')).toHaveTextContent('/discounts?kind=race#active');
+ });
+
+ test('defaults to the account page after sign-in', async () => {
+ renderLogin();
+
+ submitCredentials();
+
+ expect(await screen.findByText('Account destination')).toBeInTheDocument();
+ expect(screen.getByTestId('location')).toHaveTextContent('/account');
+ });
+
+ test.each([
+ '//evil.example/path',
+ 'https://evil.example/path',
+ '/\\evil.example/path',
+ '/%2F%2Fevil.example/path',
+ '/.//evil.example/path',
+ '/a/..//evil.example/path',
+ '/%2e%2e//evil.example/path',
+ '/safe/%2e%2e/%2F%2Fevil.example/path',
+ '/discounts%00',
+ '/discounts%',
+ ])('rejects unsafe return target %s', async (from) => {
+ renderLogin({ pathname: '/login', state: { from } });
+
+ submitCredentials();
+
+ expect(await screen.findByText('Account destination')).toBeInTheDocument();
+ expect(screen.getByTestId('location')).toHaveTextContent('/account');
+ });
+
+ test('stays on login and shows a generic error when sign-in fails', async () => {
+ signIn.mockRejectedValue(new Error('provider detail'));
+ renderLogin({ pathname: '/login', state: { from: '/discounts' } });
+
+ submitCredentials();
+
+ expect(await screen.findByText(
+ 'Failed to authenticate. Please check your credentials and try again.',
+ )).toBeInTheDocument();
+ expect(screen.queryByText('Discounts destination')).not.toBeInTheDocument();
+ });
+
+ test('keeps the registration confirmation on the login page', async () => {
+ renderLogin({ pathname: '/login', state: { from: '/discounts' } });
+ fireEvent.click(screen.getByRole('button', { name: 'New here? Register' }));
+ fireEvent.change(screen.getByLabelText('Email address'), {
+ target: { value: 'member@example.com' },
+ });
+ fireEvent.change(screen.getByLabelText('Password'), {
+ target: { value: 'correct horse battery staple' },
+ });
+ fireEvent.click(screen.getByRole('button', { name: 'Create account' }));
+
+ expect(await screen.findByText('Check your inbox for a verification email.'))
+ .toBeInTheDocument();
+ expect(screen.queryByText('Discounts destination')).not.toBeInTheDocument();
+ expect(register).toHaveBeenCalledWith(
+ 'member@example.com',
+ 'correct horse battery staple',
+ );
+ });
+});
diff --git a/src/pages/login/loginReturnPath.ts b/src/pages/login/loginReturnPath.ts
new file mode 100644
index 0000000..5d61cff
--- /dev/null
+++ b/src/pages/login/loginReturnPath.ts
@@ -0,0 +1,55 @@
+const DEFAULT_RETURN_PATH = '/account';
+
+interface RouterLocation {
+ pathname: string;
+ search: string;
+ hash: string;
+}
+
+export function getLocationReturnPath(location: RouterLocation): string {
+ return `${location.pathname}${location.search}${location.hash}`;
+}
+
+function hasControlCharacter(value: string): boolean {
+ return Array.from(value).some((character) => {
+ const codePoint = character.codePointAt(0) as number;
+ return codePoint < 0x20 || codePoint === 0x7F;
+ });
+}
+
+function hasSingleLeadingSlash(pathname: string): boolean {
+ return pathname.startsWith('/') && !pathname.startsWith('//');
+}
+
+export function getSafeLoginReturnPath(value: unknown): string {
+ if (typeof value !== 'string' || !hasSingleLeadingSlash(value)) {
+ return DEFAULT_RETURN_PATH;
+ }
+
+ try {
+ const decodedValue = decodeURIComponent(value);
+ if (
+ !hasSingleLeadingSlash(decodedValue)
+ || decodedValue.includes('\\')
+ || hasControlCharacter(decodedValue)
+ ) {
+ return DEFAULT_RETURN_PATH;
+ }
+
+ const { origin } = window.location;
+ const returnUrl = new URL(value, origin);
+ const decodedUrl = new URL(decodedValue, origin);
+ if (
+ returnUrl.origin !== origin
+ || decodedUrl.origin !== origin
+ || !hasSingleLeadingSlash(returnUrl.pathname)
+ || !hasSingleLeadingSlash(decodedUrl.pathname)
+ ) {
+ return DEFAULT_RETURN_PATH;
+ }
+
+ return `${returnUrl.pathname}${returnUrl.search}${returnUrl.hash}`;
+ } catch {
+ return DEFAULT_RETURN_PATH;
+ }
+}