wip 21

frontend/src/auth/AuthContext.tsx

@@ -8,12 +8,13 @@ import {
   useGetMe,
   useRegister,
 } from '../generated/anthoLumeAPIV1';
-
-interface AuthState {
-  isAuthenticated: boolean;
-  user: { username: string; is_admin: boolean } | null;
-  isCheckingAuth: boolean;
-}
+import {
+  type AuthState,
+  getAuthenticatedAuthState,
+  getUnauthenticatedAuthState,
+  resolveAuthStateFromMe,
+  validateAuthMutationResponse,
+} from './authHelpers';
 
 interface AuthContextType extends AuthState {
   login: (_username: string, _password: string) => Promise<void>;
@@ -23,12 +24,14 @@ interface AuthContextType extends AuthState {
 
 const AuthContext = createContext<AuthContextType | undefined>(undefined);
 
+const initialAuthState: AuthState = {
+  isAuthenticated: false,
+  user: null,
+  isCheckingAuth: true,
+};
+
 export function AuthProvider({ children }: { children: ReactNode }) {
-  const [authState, setAuthState] = useState<AuthState>({
-    isAuthenticated: false,
-    user: null,
-    isCheckingAuth: true,
-  });
+  const [authState, setAuthState] = useState<AuthState>(initialAuthState);
 
   const loginMutation = useLogin();
   const registerMutation = useRegister();
@@ -40,26 +43,14 @@ export function AuthProvider({ children }: { children: ReactNode }) {
   const navigate = useNavigate();
 
   useEffect(() => {
-    setAuthState(prev => {
-      if (meLoading) {
-        return { ...prev, isCheckingAuth: true };
-      } else if (meData?.data && meData.status === 200) {
-        const userData = 'username' in meData.data ? meData.data : null;
-        return {
-          isAuthenticated: true,
-          user: userData as { username: string; is_admin: boolean } | null,
-          isCheckingAuth: false,
-        };
-      } else if (meError || (meData && meData.status === 401)) {
-        return {
-          isAuthenticated: false,
-          user: null,
-          isCheckingAuth: false,
-        };
-      }
-
-      return { ...prev, isCheckingAuth: false };
-    });
+    setAuthState(prev =>
+      resolveAuthStateFromMe({
+        meData,
+        meError,
+        meLoading,
+        previousState: prev,
+      })
+    );
   }, [meData, meError, meLoading]);
 
   const login = useCallback(
@@ -72,29 +63,18 @@ export function AuthProvider({ children }: { children: ReactNode }) {
           },
         });
 
-        if (response.status !== 200 || !('username' in response.data)) {
-          setAuthState({
-            isAuthenticated: false,
-            user: null,
-            isCheckingAuth: false,
-          });
+        const user = validateAuthMutationResponse(response, 200);
+        if (!user) {
+          setAuthState(getUnauthenticatedAuthState());
           throw new Error('Login failed');
         }
 
-        setAuthState({
-          isAuthenticated: true,
-          user: response.data as { username: string; is_admin: boolean },
-          isCheckingAuth: false,
-        });
+        setAuthState(getAuthenticatedAuthState(user));
 
         await queryClient.invalidateQueries({ queryKey: getGetMeQueryKey() });
         navigate('/');
       } catch (_error) {
-        setAuthState({
-          isAuthenticated: false,
-          user: null,
-          isCheckingAuth: false,
-        });
+        setAuthState(getUnauthenticatedAuthState());
         throw new Error('Login failed');
       }
     },
@@ -111,29 +91,18 @@ export function AuthProvider({ children }: { children: ReactNode }) {
           },
         });
 
-        if (response.status !== 201 || !('username' in response.data)) {
-          setAuthState({
-            isAuthenticated: false,
-            user: null,
-            isCheckingAuth: false,
-          });
+        const user = validateAuthMutationResponse(response, 201);
+        if (!user) {
+          setAuthState(getUnauthenticatedAuthState());
           throw new Error('Registration failed');
         }
 
-        setAuthState({
-          isAuthenticated: true,
-          user: response.data as { username: string; is_admin: boolean },
-          isCheckingAuth: false,
-        });
+        setAuthState(getAuthenticatedAuthState(user));
 
         await queryClient.invalidateQueries({ queryKey: getGetMeQueryKey() });
         navigate('/');
       } catch (_error) {
-        setAuthState({
-          isAuthenticated: false,
-          user: null,
-          isCheckingAuth: false,
-        });
+        setAuthState(getUnauthenticatedAuthState());
         throw new Error('Registration failed');
       }
     },
@@ -143,11 +112,7 @@ export function AuthProvider({ children }: { children: ReactNode }) {
   const logout = useCallback(() => {
     logoutMutation.mutate(undefined, {
       onSuccess: async () => {
-        setAuthState({
-          isAuthenticated: false,
-          user: null,
-          isCheckingAuth: false,
-        });
+        setAuthState(getUnauthenticatedAuthState());
         await queryClient.removeQueries({ queryKey: getGetMeQueryKey() });
         navigate('/login');
       },

frontend/src/auth/ProtectedRoute.test.tsx

@@ -0,0 +1,90 @@
+import { describe, expect, it, vi, beforeEach } from 'vitest';
+import { render, screen } from '@testing-library/react';
+import { MemoryRouter, Route, Routes } from 'react-router-dom';
+import { ProtectedRoute } from './ProtectedRoute';
+import { useAuth } from './AuthContext';
+
+vi.mock('./AuthContext', () => ({
+  useAuth: vi.fn(),
+}));
+
+const mockedUseAuth = vi.mocked(useAuth);
+
+describe('ProtectedRoute', () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+  });
+
+  it('shows a loading state while auth is being checked', () => {
+    mockedUseAuth.mockReturnValue({
+      isAuthenticated: false,
+      isCheckingAuth: true,
+      user: null,
+      login: vi.fn(),
+      register: vi.fn(),
+      logout: vi.fn(),
+    });
+
+    render(
+      <MemoryRouter initialEntries={['/private']}>
+        <ProtectedRoute>
+          <div>Secret</div>
+        </ProtectedRoute>
+      </MemoryRouter>
+    );
+
+    expect(screen.getByText('Loading...')).toBeInTheDocument();
+    expect(screen.queryByText('Secret')).not.toBeInTheDocument();
+  });
+
+  it('redirects unauthenticated users to the login page', () => {
+    mockedUseAuth.mockReturnValue({
+      isAuthenticated: false,
+      isCheckingAuth: false,
+      user: null,
+      login: vi.fn(),
+      register: vi.fn(),
+      logout: vi.fn(),
+    });
+
+    render(
+      <MemoryRouter initialEntries={['/private']}>
+        <Routes>
+          <Route
+            path="/private"
+            element={
+              <ProtectedRoute>
+                <div>Secret</div>
+              </ProtectedRoute>
+            }
+          />
+          <Route path="/login" element={<div>Login Page</div>} />
+        </Routes>
+      </MemoryRouter>
+    );
+
+    expect(screen.getByText('Login Page')).toBeInTheDocument();
+    expect(screen.queryByText('Secret')).not.toBeInTheDocument();
+  });
+
+  it('renders children for authenticated users', () => {
+    mockedUseAuth.mockReturnValue({
+      isAuthenticated: true,
+      isCheckingAuth: false,
+      user: { username: 'evan', is_admin: false },
+      login: vi.fn(),
+      register: vi.fn(),
+      logout: vi.fn(),
+    });
+
+    render(
+      <MemoryRouter>
+        <ProtectedRoute>
+          <div>Secret</div>
+        </ProtectedRoute>
+      </MemoryRouter>
+    );
+
+    expect(screen.getByText('Secret')).toBeInTheDocument();
+  });
+});

frontend/src/auth/authHelpers.test.ts

@@ -0,0 +1,157 @@
+import { describe, expect, it } from 'vitest';
+import {
+  getCheckingAuthState,
+  getUnauthenticatedAuthState,
+  normalizeAuthenticatedUser,
+  resolveAuthStateFromMe,
+  validateAuthMutationResponse,
+  type AuthState,
+} from './authHelpers';
+
+const previousState: AuthState = {
+  isAuthenticated: false,
+  user: null,
+  isCheckingAuth: true,
+};
+
+describe('authHelpers', () => {
+  it('normalizes a valid authenticated user payload', () => {
+    expect(normalizeAuthenticatedUser({ username: 'evan', is_admin: true })).toEqual({
+      username: 'evan',
+      is_admin: true,
+    });
+  });
+
+  it('rejects invalid authenticated user payloads', () => {
+    expect(normalizeAuthenticatedUser(null)).toBeNull();
+    expect(normalizeAuthenticatedUser({ username: 'evan' })).toBeNull();
+    expect(normalizeAuthenticatedUser({ username: 123, is_admin: true })).toBeNull();
+    expect(normalizeAuthenticatedUser({ username: 'evan', is_admin: 'yes' })).toBeNull();
+  });
+
+  it('returns a checking state while preserving previous auth information', () => {
+    expect(
+      getCheckingAuthState({
+        isAuthenticated: true,
+        user: { username: 'evan', is_admin: false },
+        isCheckingAuth: false,
+      })
+    ).toEqual({
+      isAuthenticated: true,
+      user: { username: 'evan', is_admin: false },
+      isCheckingAuth: true,
+    });
+  });
+
+  it('resolves auth state from a successful /auth/me response', () => {
+    expect(
+      resolveAuthStateFromMe({
+        meData: {
+          status: 200,
+          data: { username: 'evan', is_admin: false },
+        },
+        meError: undefined,
+        meLoading: false,
+        previousState,
+      })
+    ).toEqual({
+      isAuthenticated: true,
+      user: { username: 'evan', is_admin: false },
+      isCheckingAuth: false,
+    });
+  });
+
+  it('resolves auth state to unauthenticated on 401 or query error', () => {
+    expect(
+      resolveAuthStateFromMe({
+        meData: {
+          status: 401,
+        },
+        meError: undefined,
+        meLoading: false,
+        previousState,
+      })
+    ).toEqual(getUnauthenticatedAuthState());
+
+    expect(
+      resolveAuthStateFromMe({
+        meData: undefined,
+        meError: new Error('failed'),
+        meLoading: false,
+        previousState,
+      })
+    ).toEqual(getUnauthenticatedAuthState());
+  });
+
+  it('keeps checking state while /auth/me is still loading', () => {
+    expect(
+      resolveAuthStateFromMe({
+        meData: undefined,
+        meError: undefined,
+        meLoading: true,
+        previousState: {
+          isAuthenticated: true,
+          user: { username: 'evan', is_admin: true },
+          isCheckingAuth: false,
+        },
+      })
+    ).toEqual({
+      isAuthenticated: true,
+      user: { username: 'evan', is_admin: true },
+      isCheckingAuth: true,
+    });
+  });
+
+  it('returns the previous state with checking disabled when there is no decisive me result', () => {
+    expect(
+      resolveAuthStateFromMe({
+        meData: {
+          status: 204,
+        },
+        meError: undefined,
+        meLoading: false,
+        previousState: {
+          isAuthenticated: false,
+          user: null,
+          isCheckingAuth: true,
+        },
+      })
+    ).toEqual({
+      isAuthenticated: false,
+      user: null,
+      isCheckingAuth: false,
+    });
+  });
+
+  it('validates auth mutation responses by expected status and payload shape', () => {
+    expect(
+      validateAuthMutationResponse(
+        {
+          status: 200,
+          data: { username: 'evan', is_admin: false },
+        },
+        200
+      )
+    ).toEqual({ username: 'evan', is_admin: false });
+
+    expect(
+      validateAuthMutationResponse(
+        {
+          status: 201,
+          data: { username: 'evan', is_admin: false },
+        },
+        200
+      )
+    ).toBeNull();
+
+    expect(
+      validateAuthMutationResponse(
+        {
+          status: 200,
+          data: { username: 'evan' },
+        },
+        200
+      )
+    ).toBeNull();
+  });
+});

frontend/src/auth/authHelpers.ts

@@ -0,0 +1,98 @@
+export interface AuthUser {
+  username: string;
+  is_admin: boolean;
+}
+
+export interface AuthState {
+  isAuthenticated: boolean;
+  user: AuthUser | null;
+  isCheckingAuth: boolean;
+}
+
+interface ResponseLike {
+  status?: number;
+  data?: unknown;
+}
+
+export function getUnauthenticatedAuthState(): AuthState {
+  return {
+    isAuthenticated: false,
+    user: null,
+    isCheckingAuth: false,
+  };
+}
+
+export function getCheckingAuthState(previousState?: AuthState): AuthState {
+  return {
+    isAuthenticated: previousState?.isAuthenticated ?? false,
+    user: previousState?.user ?? null,
+    isCheckingAuth: true,
+  };
+}
+
+export function getAuthenticatedAuthState(user: AuthUser): AuthState {
+  return {
+    isAuthenticated: true,
+    user,
+    isCheckingAuth: false,
+  };
+}
+
+export function normalizeAuthenticatedUser(value: unknown): AuthUser | null {
+  if (!value || typeof value !== 'object') {
+    return null;
+  }
+
+  if (!('username' in value) || typeof value.username !== 'string') {
+    return null;
+  }
+
+  if (!('is_admin' in value) || typeof value.is_admin !== 'boolean') {
+    return null;
+  }
+
+  return {
+    username: value.username,
+    is_admin: value.is_admin,
+  };
+}
+
+export function resolveAuthStateFromMe(params: {
+  meData?: ResponseLike;
+  meError?: unknown;
+  meLoading: boolean;
+  previousState: AuthState;
+}): AuthState {
+  const { meData, meError, meLoading, previousState } = params;
+
+  if (meLoading) {
+    return getCheckingAuthState(previousState);
+  }
+
+  if (meData?.status === 200) {
+    const user = normalizeAuthenticatedUser(meData.data);
+    if (user) {
+      return getAuthenticatedAuthState(user);
+    }
+  }
+
+  if (meError || meData?.status === 401) {
+    return getUnauthenticatedAuthState();
+  }
+
+  return {
+    ...previousState,
+    isCheckingAuth: false,
+  };
+}
+
+export function validateAuthMutationResponse(
+  response: ResponseLike,
+  expectedStatus: number
+): AuthUser | null {
+  if (response.status !== expectedStatus) {
+    return null;
+  }
+
+  return normalizeAuthenticatedUser(response.data);
+}

frontend/src/auth/authInterceptor.test.ts

@@ -0,0 +1,115 @@
+import { beforeEach, describe, expect, it, vi } from 'vitest';
+import { setupAuthInterceptors, TOKEN_KEY } from './authInterceptor';
+
+type RequestConfig = {
+  headers?: Record<string, string>;
+};
+
+type ResponseValue = {
+  status?: number;
+  data?: unknown;
+};
+
+type ResponseError = {
+  response?: {
+    status?: number;
+  };
+};
+
+function createMockAxiosInstance() {
+  let nextRequestId = 1;
+  let nextResponseId = 1;
+
+  const requestHandlers = new Map<
+    number,
+    [(config: RequestConfig) => RequestConfig, (error: unknown) => Promise<never>]
+  >();
+  const responseHandlers = new Map<
+    number,
+    [(response: ResponseValue) => ResponseValue, (error: ResponseError) => Promise<never>]
+  >();
+
+  return {
+    interceptors: {
+      request: {
+        use: vi.fn((fulfilled, rejected) => {
+          const id = nextRequestId++;
+          requestHandlers.set(id, [fulfilled, rejected]);
+          return id;
+        }),
+        eject: vi.fn((id: number) => {
+          requestHandlers.delete(id);
+        }),
+      },
+      response: {
+        use: vi.fn((fulfilled, rejected) => {
+          const id = nextResponseId++;
+          responseHandlers.set(id, [fulfilled, rejected]);
+          return id;
+        }),
+        eject: vi.fn((id: number) => {
+          responseHandlers.delete(id);
+        }),
+      },
+    },
+    getRequestHandler(id = 1) {
+      return requestHandlers.get(id);
+    },
+    getResponseHandler(id = 1) {
+      return responseHandlers.get(id);
+    },
+  };
+}
+
+describe('setupAuthInterceptors', () => {
+  beforeEach(() => {
+    localStorage.clear();
+  });
+
+  it('registers request and response interceptors and adds the auth header when a token exists', () => {
+    const axiosInstance = createMockAxiosInstance();
+
+    setupAuthInterceptors(axiosInstance as never);
+
+    expect(axiosInstance.interceptors.request.use).toHaveBeenCalledTimes(1);
+    expect(axiosInstance.interceptors.response.use).toHaveBeenCalledTimes(1);
+
+    localStorage.setItem(TOKEN_KEY, 'token-123');
+
+    const requestHandler = axiosInstance.getRequestHandler()?.[0];
+    const config: { headers: Record<string, string> } = { headers: {} };
+    const nextConfig = requestHandler?.(config);
+
+    expect(nextConfig).toBe(config);
+    expect(config.headers.Authorization).toBe('Bearer token-123');
+  });
+
+  it('clears the auth token on 401 responses', async () => {
+    const axiosInstance = createMockAxiosInstance();
+    setupAuthInterceptors(axiosInstance as never);
+
+    localStorage.setItem(TOKEN_KEY, 'token-123');
+
+    const responseErrorHandler = axiosInstance.getResponseHandler()?.[1];
+
+    await expect(responseErrorHandler?.({ response: { status: 401 } })).rejects.toEqual({
+      response: { status: 401 },
+    });
+    expect(localStorage.getItem(TOKEN_KEY)).toBeNull();
+  });
+
+  it('ejects previous interceptors before installing a new set', () => {
+    const firstInstance = createMockAxiosInstance();
+    const secondInstance = createMockAxiosInstance();
+
+    const cleanup = setupAuthInterceptors(firstInstance as never);
+    setupAuthInterceptors(secondInstance as never);
+
+    expect(firstInstance.interceptors.request.eject).toHaveBeenCalledWith(1);
+    expect(firstInstance.interceptors.response.eject).toHaveBeenCalledWith(1);
+
+    cleanup();
+    expect(firstInstance.interceptors.request.eject).toHaveBeenCalledWith(1);
+    expect(firstInstance.interceptors.response.eject).toHaveBeenCalledWith(1);
+  });
+});

frontend/src/auth/authInterceptor.ts

@@ -1,35 +1,46 @@
-import axios from 'axios';
+import axios, { type AxiosInstance } from 'axios';
 
 const TOKEN_KEY = 'antholume_token';
 
-// Request interceptor to add auth token to requests
-axios.interceptors.request.use(
-  config => {
-    const token = localStorage.getItem(TOKEN_KEY);
-    if (token && config.headers) {
-      config.headers.Authorization = `Bearer ${token}`;
-    }
-    return config;
-  },
-  error => {
-    return Promise.reject(error);
-  }
-);
+let interceptorCleanup: (() => void) | null = null;
 
-// Response interceptor to handle auth errors
-axios.interceptors.response.use(
-  response => {
-    return response;
-  },
-  error => {
-    if (error.response?.status === 401) {
-      // Clear token on auth failure
-      localStorage.removeItem(TOKEN_KEY);
-      // Optionally redirect to login
-      // window.location.href = '/login';
-    }
-    return Promise.reject(error);
+export function setupAuthInterceptors(axiosInstance: AxiosInstance = axios) {
+  if (interceptorCleanup) {
+    interceptorCleanup();
   }
-);
 
+  const requestInterceptorId = axiosInstance.interceptors.request.use(
+    config => {
+      const token = localStorage.getItem(TOKEN_KEY);
+      if (token && config.headers) {
+        config.headers.Authorization = `Bearer ${token}`;
+      }
+      return config;
+    },
+    error => {
+      return Promise.reject(error);
+    }
+  );
+
+  const responseInterceptorId = axiosInstance.interceptors.response.use(
+    response => {
+      return response;
+    },
+    error => {
+      if (error.response?.status === 401) {
+        localStorage.removeItem(TOKEN_KEY);
+      }
+      return Promise.reject(error);
+    }
+  );
+
+  interceptorCleanup = () => {
+    axiosInstance.interceptors.request.eject(requestInterceptorId);
+    axiosInstance.interceptors.response.eject(responseInterceptorId);
+  };
+
+  return interceptorCleanup;
+}
+
+export { TOKEN_KEY };
 export default axios;