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
This commit is contained in:
parent
685c17bc07
commit
66a8604fa3
3 changed files with 177 additions and 7 deletions
|
|
@ -318,6 +318,16 @@ function App() {
|
||||||
const [isLoading, setIsLoading] = createSignal(true);
|
const [isLoading, setIsLoading] = createSignal(true);
|
||||||
const [needsAuth, setNeedsAuth] = createSignal(false);
|
const [needsAuth, setNeedsAuth] = createSignal(false);
|
||||||
const [hasAuth, setHasAuth] = 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<string, boolean>;
|
||||||
|
hideLocalLogin?: boolean;
|
||||||
|
deprecatedDisableAuth?: boolean;
|
||||||
|
} | null>(null);
|
||||||
const [proxyAuthInfo, setProxyAuthInfo] = createSignal<{
|
const [proxyAuthInfo, setProxyAuthInfo] = createSignal<{
|
||||||
username?: string;
|
username?: string;
|
||||||
logoutURL?: string;
|
logoutURL?: string;
|
||||||
|
|
@ -521,6 +531,17 @@ function App() {
|
||||||
const securityData = await securityRes.json();
|
const securityData = await securityRes.json();
|
||||||
logger.debug('[App] Security status fetched', securityData);
|
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
|
// Detect legacy DISABLE_AUTH flag (now ignored) so we can surface a warning
|
||||||
if (securityData.deprecatedDisableAuth === true) {
|
if (securityData.deprecatedDisableAuth === true) {
|
||||||
logger.warn(
|
logger.warn(
|
||||||
|
|
@ -798,7 +819,7 @@ function App() {
|
||||||
</div>
|
</div>
|
||||||
}
|
}
|
||||||
>
|
>
|
||||||
<Show when={!needsAuth()} fallback={<Login onLogin={handleLogin} hasAuth={hasAuth()} />}>
|
<Show when={!needsAuth()} fallback={<Login onLogin={handleLogin} hasAuth={hasAuth()} securityStatus={securityStatus() ?? undefined} />}>
|
||||||
<ErrorBoundary>
|
<ErrorBoundary>
|
||||||
<Show when={enhancedStore()} fallback={
|
<Show when={enhancedStore()} fallback={
|
||||||
<div class="min-h-screen flex items-center justify-center bg-gray-100 dark:bg-gray-900">
|
<div class="min-h-screen flex items-center justify-center bg-gray-100 dark:bg-gray-900">
|
||||||
|
|
|
||||||
|
|
@ -9,6 +9,7 @@ const SetupWizard = lazy(() =>
|
||||||
interface LoginProps {
|
interface LoginProps {
|
||||||
onLogin: () => void;
|
onLogin: () => void;
|
||||||
hasAuth?: boolean; // If true, auth is configured (passed from App.tsx to skip redundant check)
|
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 {
|
interface SecurityStatus {
|
||||||
|
|
@ -30,9 +31,10 @@ export const Login: Component<LoginProps> = (props) => {
|
||||||
const [rememberMe, setRememberMe] = createSignal(false);
|
const [rememberMe, setRememberMe] = createSignal(false);
|
||||||
const [error, setError] = createSignal('');
|
const [error, setError] = createSignal('');
|
||||||
const [loading, setLoading] = createSignal(false);
|
const [loading, setLoading] = createSignal(false);
|
||||||
const [authStatus, setAuthStatus] = createSignal<SecurityStatus | null>(null);
|
const [authStatus, setAuthStatus] = createSignal<SecurityStatus | null>(props.securityStatus ?? null);
|
||||||
// If hasAuth is passed from App.tsx, we already know auth status - skip the loading state
|
// 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 [oidcLoading, setOidcLoading] = createSignal(false);
|
||||||
const [oidcError, setOidcError] = createSignal('');
|
const [oidcError, setOidcError] = createSignal('');
|
||||||
const [oidcMessage, setOidcMessage] = createSignal('');
|
const [oidcMessage, setOidcMessage] = createSignal('');
|
||||||
|
|
@ -98,15 +100,22 @@ export const Login: Component<LoginProps> = (props) => {
|
||||||
window.history.replaceState({}, document.title, newUrl);
|
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
|
// This eliminates the flicker between "Checking authentication..." and the login form
|
||||||
if (props.hasAuth !== undefined) {
|
// AND ensures hideLocalLogin, oidcEnabled, etc. are properly respected
|
||||||
logger.debug('[Login] Using hasAuth from App.tsx, skipping redundant auth check');
|
if (props.securityStatus) {
|
||||||
setAuthStatus({ hasAuthentication: props.hasAuth });
|
logger.debug('[Login] Using securityStatus from App.tsx, skipping redundant auth check', props.securityStatus);
|
||||||
|
setAuthStatus(props.securityStatus);
|
||||||
setLoadingAuth(false);
|
setLoadingAuth(false);
|
||||||
return;
|
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...');
|
logger.debug('[Login] Starting auth check...');
|
||||||
try {
|
try {
|
||||||
const response = await fetch('/api/security/status');
|
const response = await fetch('/api/security/status');
|
||||||
|
|
|
||||||
140
frontend-modern/src/components/__tests__/Login.test.tsx
Normal file
140
frontend-modern/src/components/__tests__/Login.test.tsx
Normal file
|
|
@ -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(() => (
|
||||||
|
<Login onLogin={mockOnLogin} hasAuth={true} securityStatus={securityStatus} />
|
||||||
|
));
|
||||||
|
|
||||||
|
// 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(() => (
|
||||||
|
<Login onLogin={mockOnLogin} hasAuth={true} securityStatus={securityStatus} />
|
||||||
|
));
|
||||||
|
|
||||||
|
// 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(() => (
|
||||||
|
<Login onLogin={mockOnLogin} hasAuth={true} securityStatus={securityStatus} />
|
||||||
|
));
|
||||||
|
|
||||||
|
// 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(() => (
|
||||||
|
<Login onLogin={mockOnLogin} hasAuth={true} securityStatus={securityStatus} />
|
||||||
|
));
|
||||||
|
|
||||||
|
// 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');
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue