From 66a8604fa39792ba7ca09e0a705a428f31aea954 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Thu, 18 Dec 2025 18:27:07 +0000 Subject: [PATCH] fix: restore Hide Local Login functionality for OIDC/SSO (#857) When 'Hide local login form' was enabled in Settings -> Authentication, the local login form was still displayed instead of showing only the SSO login. This regression occurred in Pulse 5.x. Root cause: When App.tsx passed hasAuth to Login.tsx, the Login component created a minimal SecurityStatus object with only hasAuthentication set, missing the hideLocalLogin and other OIDC settings. Changes: - App.tsx: Store and pass full securityStatus to Login component - Login.tsx: Accept securityStatus prop and initialize state from it - Login.tsx: Initialize authStatus directly from props to respect hideLocalLogin on first render - Added tests for hideLocalLogin behavior Fixes #857 --- frontend-modern/src/App.tsx | 23 ++- frontend-modern/src/components/Login.tsx | 21 ++- .../src/components/__tests__/Login.test.tsx | 140 ++++++++++++++++++ 3 files changed, 177 insertions(+), 7 deletions(-) create mode 100644 frontend-modern/src/components/__tests__/Login.test.tsx diff --git a/frontend-modern/src/App.tsx b/frontend-modern/src/App.tsx index 1e03d1d..6cc8e96 100644 --- a/frontend-modern/src/App.tsx +++ b/frontend-modern/src/App.tsx @@ -318,6 +318,16 @@ function App() { const [isLoading, setIsLoading] = createSignal(true); const [needsAuth, setNeedsAuth] = createSignal(false); const [hasAuth, setHasAuth] = createSignal(false); + // Store full security status for Login component (hideLocalLogin, oidcEnabled, etc.) + const [securityStatus, setSecurityStatus] = createSignal<{ + hasAuthentication: boolean; + oidcEnabled?: boolean; + oidcIssuer?: string; + oidcClientId?: string; + oidcEnvOverrides?: Record; + hideLocalLogin?: boolean; + deprecatedDisableAuth?: boolean; + } | null>(null); const [proxyAuthInfo, setProxyAuthInfo] = createSignal<{ username?: string; logoutURL?: string; @@ -521,6 +531,17 @@ function App() { const securityData = await securityRes.json(); logger.debug('[App] Security status fetched', securityData); + // Store full security status for Login component + setSecurityStatus({ + hasAuthentication: securityData.hasAuthentication || false, + oidcEnabled: securityData.oidcEnabled, + oidcIssuer: securityData.oidcIssuer, + oidcClientId: securityData.oidcClientId, + oidcEnvOverrides: securityData.oidcEnvOverrides, + hideLocalLogin: securityData.hideLocalLogin, + deprecatedDisableAuth: securityData.deprecatedDisableAuth, + }); + // Detect legacy DISABLE_AUTH flag (now ignored) so we can surface a warning if (securityData.deprecatedDisableAuth === true) { logger.warn( @@ -798,7 +819,7 @@ function App() { } > - }> + }> diff --git a/frontend-modern/src/components/Login.tsx b/frontend-modern/src/components/Login.tsx index e59efe5..a591e8f 100644 --- a/frontend-modern/src/components/Login.tsx +++ b/frontend-modern/src/components/Login.tsx @@ -9,6 +9,7 @@ const SetupWizard = lazy(() => interface LoginProps { onLogin: () => void; hasAuth?: boolean; // If true, auth is configured (passed from App.tsx to skip redundant check) + securityStatus?: SecurityStatus; // Full security status from App.tsx to avoid redundant API call } interface SecurityStatus { @@ -30,9 +31,10 @@ export const Login: Component = (props) => { const [rememberMe, setRememberMe] = createSignal(false); const [error, setError] = createSignal(''); const [loading, setLoading] = createSignal(false); - const [authStatus, setAuthStatus] = createSignal(null); + const [authStatus, setAuthStatus] = createSignal(props.securityStatus ?? null); // If hasAuth is passed from App.tsx, we already know auth status - skip the loading state - const [loadingAuth, setLoadingAuth] = createSignal(props.hasAuth === undefined); + // Also skip if securityStatus is provided + const [loadingAuth, setLoadingAuth] = createSignal(props.hasAuth === undefined && !props.securityStatus); const [oidcLoading, setOidcLoading] = createSignal(false); const [oidcError, setOidcError] = createSignal(''); const [oidcMessage, setOidcMessage] = createSignal(''); @@ -98,15 +100,22 @@ export const Login: Component = (props) => { window.history.replaceState({}, document.title, newUrl); } - // If hasAuth was passed from App.tsx, use it directly without making another API call + // If securityStatus was passed from App.tsx, use it directly without making another API call // This eliminates the flicker between "Checking authentication..." and the login form - if (props.hasAuth !== undefined) { - logger.debug('[Login] Using hasAuth from App.tsx, skipping redundant auth check'); - setAuthStatus({ hasAuthentication: props.hasAuth }); + // AND ensures hideLocalLogin, oidcEnabled, etc. are properly respected + if (props.securityStatus) { + logger.debug('[Login] Using securityStatus from App.tsx, skipping redundant auth check', props.securityStatus); + setAuthStatus(props.securityStatus); setLoadingAuth(false); return; } + // Legacy fallback: if only hasAuth was passed (without full securityStatus) + if (props.hasAuth !== undefined && !props.securityStatus) { + logger.debug('[Login] Using hasAuth from App.tsx (legacy), fetching full security status'); + // Still need to fetch full status to get hideLocalLogin, OIDC settings, etc. + } + logger.debug('[Login] Starting auth check...'); try { const response = await fetch('/api/security/status'); diff --git a/frontend-modern/src/components/__tests__/Login.test.tsx b/frontend-modern/src/components/__tests__/Login.test.tsx new file mode 100644 index 0000000..774efad --- /dev/null +++ b/frontend-modern/src/components/__tests__/Login.test.tsx @@ -0,0 +1,140 @@ +import { afterEach, describe, expect, it, vi, beforeEach } from 'vitest'; +import { cleanup, render, screen } from '@solidjs/testing-library'; +import { Login } from '@/components/Login'; + +// Mock fetch globally +const mockFetch = vi.fn(); +global.fetch = mockFetch; + +// Mock window.matchMedia +Object.defineProperty(window, 'matchMedia', { + writable: true, + value: vi.fn().mockImplementation((query: string) => ({ + matches: false, + media: query, + onchange: null, + addListener: vi.fn(), + removeListener: vi.fn(), + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + dispatchEvent: vi.fn(), + })), +}); + +// Mock localStorage +const mockLocalStorage = { + getItem: vi.fn(), + setItem: vi.fn(), + removeItem: vi.fn(), + clear: vi.fn(), +}; +Object.defineProperty(window, 'localStorage', { value: mockLocalStorage }); + +// Mock history.replaceState +window.history.replaceState = vi.fn(); + +describe('Login', () => { + beforeEach(() => { + mockFetch.mockReset(); + mockLocalStorage.getItem.mockReset(); + // Default mock for security status + mockFetch.mockResolvedValue({ + ok: true, + json: () => Promise.resolve({ hasAuthentication: true }), + }); + }); + + afterEach(() => { + cleanup(); + }); + + it('shows local login form when hideLocalLogin is false', async () => { + const mockOnLogin = vi.fn(); + const securityStatus = { + hasAuthentication: true, + hideLocalLogin: false, + oidcEnabled: false, + }; + + render(() => ( + + )); + + // Username and password fields should be visible + expect(await screen.findByPlaceholderText('Username')).toBeInTheDocument(); + expect(screen.getByPlaceholderText('Password')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /sign in to pulse/i })).toBeInTheDocument(); + }); + + it('hides local login form when hideLocalLogin is true and OIDC is enabled', async () => { + const mockOnLogin = vi.fn(); + const securityStatus = { + hasAuthentication: true, + hideLocalLogin: true, + oidcEnabled: true, + }; + + render(() => ( + + )); + + // Wait for the component to render + expect(await screen.findByText(/Welcome to Pulse/i)).toBeInTheDocument(); + + // SSO button should be visible + expect(screen.getByRole('button', { name: /continue with single sign-on/i })).toBeInTheDocument(); + + // Username and password fields should NOT be visible + expect(screen.queryByPlaceholderText('Username')).toBeNull(); + expect(screen.queryByPlaceholderText('Password')).toBeNull(); + }); + + it('shows local login form when show_local=true is in URL even if hideLocalLogin is true', async () => { + // Set up URL with show_local=true + const originalLocation = window.location; + delete (window as any).location; + window.location = { + ...originalLocation, + search: '?show_local=true', + pathname: '/', + href: 'http://localhost/?show_local=true', + } as any; + + const mockOnLogin = vi.fn(); + const securityStatus = { + hasAuthentication: true, + hideLocalLogin: true, + oidcEnabled: true, + }; + + render(() => ( + + )); + + // Local login form should still be visible due to show_local=true + expect(await screen.findByPlaceholderText('Username')).toBeInTheDocument(); + expect(screen.getByPlaceholderText('Password')).toBeInTheDocument(); + // Restore location + (window as any).location = originalLocation; + }); + + it('uses securityStatus prop instead of making API call when provided', async () => { + const mockOnLogin = vi.fn(); + const securityStatus = { + hasAuthentication: true, + hideLocalLogin: false, + oidcEnabled: false, + }; + + render(() => ( + + )); + + // Wait for component to render + expect(await screen.findByPlaceholderText('Username')).toBeInTheDocument(); + + // The fetch should not have been called for /api/security/status + // because we passed securityStatus directly + expect(mockFetch).not.toHaveBeenCalledWith('/api/security/status'); + }); +});