From 93a88b21e0ccbafaa0bca4da2af0f1fa2eba3a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?V=C5=A9=20Th=C3=A0nh=20Trung?= Date: Mon, 28 Sep 2026 22:25:48 +0700 Subject: [PATCH] fix(auth): render Remember Me on SSO sign in page when local login is disabled (#2784) (#3117) --- .../src/__tests__/pages/LoginPage.test.tsx | 211 ++++++++++++++++++ frontend/src/pages/LoginPage.tsx | 45 +++- 2 files changed, 245 insertions(+), 11 deletions(-) diff --git a/frontend/src/__tests__/pages/LoginPage.test.tsx b/frontend/src/__tests__/pages/LoginPage.test.tsx index e4003e92b..826eb26e2 100644 --- a/frontend/src/__tests__/pages/LoginPage.test.tsx +++ b/frontend/src/__tests__/pages/LoginPage.test.tsx @@ -19,6 +19,13 @@ vi.mock('react-router-dom', async (importActual) => { return { ...actual, useNavigate: () => mockNavigate }; }); +// The setup file's Location proxy. Several tests below swap window.location +// for a plain object so the OIDC redirect's href write can be read back. +// Everything restored in the afterEach below used to leak into whichever test +// ran next: a stale location object hid the URL it set with pushState, and a +// token left by a successful login signed in a visitor meant to be anonymous. +const realLocation = window.location; + describe('LoginPage', () => { beforeEach(() => { server.use( @@ -28,6 +35,12 @@ describe('LoginPage', () => { ); }); + afterEach(() => { + Object.defineProperty(window, 'location', { configurable: true, writable: true, value: realLocation }); + setAuthToken(null); + sessionStorage.clear(); + }); + describe('rendering', () => { it('renders the login form', async () => { render(); @@ -638,6 +651,204 @@ describe('LoginPage', () => { }); }); + // #2784: Remember Me on SSO sign in page when local login is disabled + describe('SSO login with local login disabled (#2784)', () => { + const mockProviders = [ + { + id: 101, + name: 'Authentik', + issuer_url: 'https://auth.test', + client_id: 'client-101', + is_enabled: true, + icon_url: null, + has_icon: false, + email_claim: 'email', + require_email_verified: true, + auto_create_users: false, + auto_link_existing_accounts: false, + }, + ]; + + beforeEach(() => { + vi.mocked(localStorage.setItem).mockClear(); + sessionStorage.clear(); + server.use( + http.get('/api/v1/auth/advanced-auth/status', () => + HttpResponse.json({ + advanced_auth_enabled: true, + smtp_configured: false, + local_login_enabled: false, + autologin_provider_id: null, + }) + ), + http.get('/api/v1/auth/oidc/providers', () => + HttpResponse.json(mockProviders) + ), + http.get('/api/v1/auth/oidc/authorize/101', () => + HttpResponse.json({ auth_url: 'https://auth.test/authorize?state=xyz' }) + ) + ); + }); + + afterEach(() => { + sessionStorage.clear(); + }); + + it('renders Remember Me checkbox on SSO login page when local login is disabled', async () => { + render(); + + // Notice for disabled local login should be shown + await waitFor(() => { + expect(screen.getByText(/Local sign-in is disabled/i)).toBeInTheDocument(); + }); + + // Local username & password inputs should NOT be shown + expect(screen.queryByLabelText(/Username/i)).not.toBeInTheDocument(); + expect(screen.queryByLabelText(/Password/i)).not.toBeInTheDocument(); + + // SSO provider button should be shown + expect(screen.getByRole('button', { name: /Authentik/i })).toBeInTheDocument(); + + // "or continue with" divider should NOT be shown when local login is disabled + expect(screen.queryByText(/or continue with/i)).not.toBeInTheDocument(); + + // Remember Me checkbox MUST be rendered and unchecked by default + const rememberCheckbox = screen.getByRole('checkbox', { name: /Remember Me/i }); + expect(rememberCheckbox).toBeInTheDocument(); + expect(rememberCheckbox).not.toBeChecked(); + }); + + it('writes auth_remember_me flag to sessionStorage before SSO redirect when Remember Me is checked', async () => { + const user = userEvent.setup(); + render(); + + await waitFor(() => { + expect(screen.getByRole('button', { name: /Authentik/i })).toBeInTheDocument(); + }); + + const rememberCheckbox = screen.getByRole('checkbox', { name: /Remember Me/i }); + await user.click(rememberCheckbox); + expect(rememberCheckbox).toBeChecked(); + + Object.defineProperty(window, 'location', { + writable: true, + value: { ...window.location, href: 'http://localhost:3000/' }, + }); + + await user.click(screen.getByRole('button', { name: /Authentik/i })); + + await waitFor(() => { + expect(sessionStorage.getItem('auth_remember_me')).toBe('1'); + }); + }); + + it('does not write auth_remember_me flag when Remember Me is not checked', async () => { + const user = userEvent.setup(); + render(); + + await waitFor(() => { + expect(screen.getByRole('button', { name: /Authentik/i })).toBeInTheDocument(); + }); + + Object.defineProperty(window, 'location', { + writable: true, + value: { ...window.location, href: 'http://localhost:3000/' }, + }); + + await user.click(screen.getByRole('button', { name: /Authentik/i })); + + // The flag starts out null, so wait for the redirect itself before + // asserting on it -- otherwise this passes before the click is handled. + await waitFor(() => { + expect(window.location.href).toBe('https://auth.test/authorize?state=xyz'); + }); + expect(sessionStorage.getItem('auth_remember_me')).toBeNull(); + }); + + it('cleans up stale auth_remember_me flag if Remember Me is unchecked before SSO redirect', async () => { + sessionStorage.setItem('auth_remember_me', '1'); + const user = userEvent.setup(); + render(); + + await waitFor(() => { + expect(screen.getByRole('button', { name: /Authentik/i })).toBeInTheDocument(); + }); + + Object.defineProperty(window, 'location', { + writable: true, + value: { ...window.location, href: 'http://localhost:3000/' }, + }); + + // Clicking SSO without checking Remember Me should remove the stale flag + await user.click(screen.getByRole('button', { name: /Authentik/i })); + + // The flag starts out null, so wait for the redirect itself before + // asserting on it -- otherwise this passes before the click is handled. + await waitFor(() => { + expect(window.location.href).toBe('https://auth.test/authorize?state=xyz'); + }); + expect(sessionStorage.getItem('auth_remember_me')).toBeNull(); + }); + }); + + // #1589 autologin: the page redirects to the IdP on mount, so the + // destination the user was sent to /login from has to be stashed first, or + // the round-trip lands them on "/" instead (#2784 review). + describe('OIDC autologin keeps the post-login destination', () => { + beforeEach(() => { + sessionStorage.clear(); + server.use( + http.get('/api/v1/auth/advanced-auth/status', () => + HttpResponse.json({ + advanced_auth_enabled: true, + smtp_configured: false, + local_login_enabled: false, + autologin_provider_id: 101, + }) + ), + http.get('/api/v1/auth/oidc/providers', () => HttpResponse.json([])), + http.get('/api/v1/auth/oidc/authorize/101', () => + HttpResponse.json({ auth_url: 'https://auth.test/authorize?state=xyz' }) + ) + ); + }); + + afterEach(() => { + window.history.replaceState(null, '', '/'); + }); + + // BrowserRouter reads router state from history.state.usr, which is where + // ProtectedRoute's navigate('/login', { state: { from } }) puts it. + function arriveAtLogin(from?: { pathname: string; search?: string }) { + window.history.replaceState(from ? { usr: { from }, key: 'test', idx: 0 } : null, '', '/login'); + Object.defineProperty(window, 'location', { + configurable: true, + writable: true, + value: { ...window.location, href: 'http://localhost:3000/login' }, + }); + } + + it('stashes the page the user was sent from before redirecting to the IdP', async () => { + arriveAtLogin({ pathname: '/archives', search: '?id=5' }); + render(); + + await waitFor(() => { + expect(window.location.href).toBe('https://auth.test/authorize?state=xyz'); + }); + expect(sessionStorage.getItem('auth_post_login_redirect')).toBe('/archives?id=5'); + }); + + it('stashes nothing when the user opened /login directly', async () => { + arriveAtLogin(); + render(); + + await waitFor(() => { + expect(window.location.href).toBe('https://auth.test/authorize?state=xyz'); + }); + expect(sessionStorage.getItem('auth_post_login_redirect')).toBeNull(); + }); + }); + // #1333: icon proxy — login page renders from /icon endpoint // rather than the upstream icon_url, so the strict img-src CSP holds. describe('OIDC icon proxy (#1333)', () => { diff --git a/frontend/src/pages/LoginPage.tsx b/frontend/src/pages/LoginPage.tsx index 2f03494ae..e5c70bd18 100644 --- a/frontend/src/pages/LoginPage.tsx +++ b/frontend/src/pages/LoginPage.tsx @@ -207,12 +207,16 @@ export function LoginPage() { ); Promise.race([api.getOIDCAuthorizeUrl(providerId), timeoutPromise]) .then((result) => { + const fromState = (location.state as { from?: { pathname?: string; search?: string } } | null)?.from; + if (fromState?.pathname) { + stashPostLoginRedirect(`${fromState.pathname}${fromState.search ?? ''}`); + } window.location.href = (result as { auth_url: string }).auth_url; }) .catch(() => { setAutologinFailed(true); }); - }, [advancedAuthStatus, searchParams]); + }, [advancedAuthStatus, searchParams, location.state]); const localLoginEnabled = advancedAuthStatus?.local_login_enabled !== false; const showAutologinBanner = autologinFailed && advancedAuthStatus?.autologin_provider_id != null; @@ -391,12 +395,14 @@ export function LoginPage() { const oidcLoginMutation = useMutation({ mutationFn: (providerId: number) => api.getOIDCAuthorizeUrl(providerId), onSuccess: (data) => { - if (rememberMe) { - try { + try { + if (rememberMe) { sessionStorage.setItem(REMEMBER_ME_KEY, '1'); - } catch (err) { - console.warn('setItem auth_remember_me failed, Remember Me will not carry through OIDC redirect', err); + } else { + sessionStorage.removeItem(REMEMBER_ME_KEY); } + } catch (err) { + console.warn('setItem/removeItem auth_remember_me failed, Remember Me will not carry through OIDC redirect', err); } // Stash the post-login destination from router state so it survives the // provider round-trip (window.location.href kills React state). If the @@ -809,14 +815,31 @@ export function LoginPage() { {/* OIDC provider buttons */} {oidcProviders && oidcProviders.length > 0 && (
-
-
-
+ {localLoginEnabled && ( +
+
+
+
+
+ {t('login.twoFA.orContinueWith')} +
-
- {t('login.twoFA.orContinueWith')} + )} + + {!localLoginEnabled && ( +
+ setRememberMe(e.target.checked)} + className="h-4 w-4 rounded border-bambu-dark-tertiary bg-bambu-dark-secondary text-bambu-green focus:ring-bambu-green/50 cursor-pointer" + /> +
-
+ )}
{oidcProviders.map((provider) => (