mirror of
https://github.com/maziggy/bambuddy.git
synced 2026-10-17 19:36:28 +02:00
Redirect authenticated visitors off /login (#1889)
LoginPage rendered the credentials form for an already-authenticated session, so a direct visit to /login (browsers autocomplete the origin to it) looked like "Remember Me" never worked despite a live token. Read user/loading from the auth context and redirect to / once the auth check settles, gated on the credentials step so the 2FA and OIDC-callback branches keep their own navigation.
This commit is contained in:
File diff suppressed because one or more lines are too long
@@ -7,9 +7,18 @@ import { fireEvent, screen, waitFor } from '@testing-library/react';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { render } from '../utils';
|
||||
import { LoginPage } from '../../pages/LoginPage';
|
||||
import { setAuthToken } from '../../api/client';
|
||||
import { http, HttpResponse } from 'msw';
|
||||
import { server } from '../mocks/server';
|
||||
|
||||
// Spy on navigation so we can assert the #1889 redirect-away-if-authenticated
|
||||
// guard. importActual keeps BrowserRouter / useLocation / useSearchParams real.
|
||||
const mockNavigate = vi.fn();
|
||||
vi.mock('react-router-dom', async (importActual) => {
|
||||
const actual = await importActual<typeof import('react-router-dom')>();
|
||||
return { ...actual, useNavigate: () => mockNavigate };
|
||||
});
|
||||
|
||||
describe('LoginPage', () => {
|
||||
beforeEach(() => {
|
||||
server.use(
|
||||
@@ -831,4 +840,48 @@ describe('LoginPage', () => {
|
||||
expect(screen.getByRole('button', { name: /BetaIdP/i }).querySelector('img')).not.toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
// #1889: an already-authenticated visit to /login must redirect to the app,
|
||||
// not render the credentials form. Browsers autocomplete the origin to its
|
||||
// most-visited path (/login), so live sessions kept landing on the form and
|
||||
// it looked like Bambuddy "never stays logged in".
|
||||
describe('authenticated redirect (#1889)', () => {
|
||||
const mockUser = {
|
||||
id: 1,
|
||||
username: 'testuser',
|
||||
role: 'admin' as const,
|
||||
is_active: true,
|
||||
created_at: new Date().toISOString(),
|
||||
};
|
||||
|
||||
afterEach(() => {
|
||||
setAuthToken(null);
|
||||
});
|
||||
|
||||
it('redirects an already-authenticated visitor away from /login', async () => {
|
||||
// A live session: token present, /api/v1/auth/me answers 200.
|
||||
setAuthToken('valid-token', 'session');
|
||||
server.use(http.get('/api/v1/auth/me', () => HttpResponse.json(mockUser)));
|
||||
mockNavigate.mockClear();
|
||||
|
||||
render(<LoginPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(mockNavigate).toHaveBeenCalledWith('/', { replace: true });
|
||||
});
|
||||
});
|
||||
|
||||
it('does not redirect an unauthenticated visitor', async () => {
|
||||
// No token → checkAuthStatus leaves user null; the form must stay put.
|
||||
server.use(http.get('/api/v1/auth/me', () => HttpResponse.json(mockUser)));
|
||||
mockNavigate.mockClear();
|
||||
|
||||
render(<LoginPage />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByRole('button', { name: /Sign in/i })).toBeInTheDocument();
|
||||
});
|
||||
expect(mockNavigate).not.toHaveBeenCalledWith('/', { replace: true });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -113,7 +113,7 @@ export function LoginPage() {
|
||||
const location = useLocation();
|
||||
const [searchParams] = useSearchParams();
|
||||
const { t } = useTranslation();
|
||||
const { login, loginWithToken } = useAuth();
|
||||
const { login, loginWithToken, user, loading } = useAuth();
|
||||
const { showToast } = useToast();
|
||||
const { mode } = useTheme();
|
||||
|
||||
@@ -174,6 +174,22 @@ export function LoginPage() {
|
||||
// we skip the redirect and render the normal page, surfacing a banner
|
||||
// so the user understands why autologin didn't kick in.
|
||||
const [autologinFailed, setAutologinFailed] = useState(false);
|
||||
|
||||
// #1889: redirect already-authenticated visitors away from /login. Without
|
||||
// this, a valid session that lands directly on /login (e.g. the browser
|
||||
// address bar autocompletes the origin to its most-visited path) renders the
|
||||
// credentials form even though the token is live and every request succeeds —
|
||||
// making Bambuddy look like it "never stays logged in". Gate on the
|
||||
// credentials step so we don't interrupt the 2FA / OIDC-callback branches,
|
||||
// which navigate themselves after loginWithToken. Send to '/' rather than
|
||||
// resolvePostLoginRedirect() to avoid consuming the OIDC redirect stash: an
|
||||
// already-authed direct visit has no pending redirect to honour.
|
||||
useEffect(() => {
|
||||
if (!loading && user && step === 'credentials') {
|
||||
navigate('/', { replace: true });
|
||||
}
|
||||
}, [loading, user, step, navigate]);
|
||||
|
||||
const autologinAttemptedRef = useRef(false);
|
||||
useEffect(() => {
|
||||
if (autologinAttemptedRef.current) return;
|
||||
|
||||
File diff suppressed because one or more lines are too long
+1
-1
@@ -26,7 +26,7 @@
|
||||
|
||||
<!-- Splash screens for iOS -->
|
||||
<link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
|
||||
<script type="module" crossorigin src="/assets/index-D_ORAGtA.js"></script>
|
||||
<script type="module" crossorigin src="/assets/index-nQKiiuS3.js"></script>
|
||||
<link rel="stylesheet" crossorigin href="/assets/index-DvjR9OL3.css">
|
||||
</head>
|
||||
<body>
|
||||
|
||||
Reference in New Issue
Block a user