mirror of
https://github.com/Sea-Haven-Industries/proposal-system.git
synced 2026-10-07 11:38:56 +00:00
fix(web): harden auth session teardown per /sh-security-review findings
- AUTH-L1 (confirmed medium): logout() now clears the react-query cache — the singleton cache survived SPA logout, serving the previous principal's cached GETs to the next login in the same tab for up to staleTime with no server round-trip. - AUTH-L3 (confirmed low): isTokenValid decodes base64url before atob — valid Cognito JWTs containing '-'/'_' in the payload segment were misclassified as expired (login lockout/loop; inherited from the old authSlice). - AUTH-L2 (unverified, hardened anyway): 401 interceptor broadcasts AUTH_SESSION_CLEARED_EVENT so AuthProvider drops in-memory state synchronously, restoring the old Redux atomic-clear semantics. - INJ-1 (unverified, hardened anyway): Authorization header only set when the stored token is a string. Each fix pinned by a test; 63 vitest green, tsc clean.
This commit is contained in:
parent
8fbca7f780
commit
5441836274
6 changed files with 79 additions and 8 deletions
|
|
@ -120,6 +120,18 @@ describe('API client interceptors', () => {
|
||||||
expect(result.headers.Authorization).toBeUndefined();
|
expect(result.headers.Authorization).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('does not attach header when the token field is not a string', () => {
|
||||||
|
sessionStorageData['proposal_system_token'] = JSON.stringify({ token: { a: 1 } });
|
||||||
|
|
||||||
|
const config = {
|
||||||
|
headers: {} as Record<string, string>,
|
||||||
|
} as unknown as InternalAxiosRequestConfig;
|
||||||
|
|
||||||
|
const result = interceptors.requestFulfilled(config);
|
||||||
|
|
||||||
|
expect(result.headers.Authorization).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
it('WEB-C1: does not attach header when token field is missing in parsed data', () => {
|
it('WEB-C1: does not attach header when token field is missing in parsed data', () => {
|
||||||
sessionStorageData['proposal_system_token'] = JSON.stringify({ email: 'user@test.com' });
|
sessionStorageData['proposal_system_token'] = JSON.stringify({ email: 'user@test.com' });
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,6 @@
|
||||||
import axios from 'axios';
|
import axios from 'axios';
|
||||||
import { API_URL, STORAGE_KEY_TOKEN } from '../../constants';
|
import { API_URL, STORAGE_KEY_TOKEN } from '../../constants';
|
||||||
import { clearAuth } from '../auth/authStorage';
|
import { AUTH_SESSION_CLEARED_EVENT, clearAuth } from '../auth/authStorage';
|
||||||
|
|
||||||
const apiClient = axios.create({
|
const apiClient = axios.create({
|
||||||
baseURL: API_URL,
|
baseURL: API_URL,
|
||||||
|
|
@ -16,7 +16,7 @@ apiClient.interceptors.request.use(
|
||||||
if (tokenData) {
|
if (tokenData) {
|
||||||
try {
|
try {
|
||||||
const parsed = JSON.parse(tokenData);
|
const parsed = JSON.parse(tokenData);
|
||||||
if (parsed?.token) {
|
if (typeof parsed?.token === 'string') {
|
||||||
config.headers.Authorization = `Bearer ${parsed.token}`;
|
config.headers.Authorization = `Bearer ${parsed.token}`;
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
|
|
@ -35,9 +35,10 @@ apiClient.interceptors.response.use(
|
||||||
const { status, data } = error.response;
|
const { status, data } = error.response;
|
||||||
|
|
||||||
if (status === 401) {
|
if (status === 401) {
|
||||||
// Fix: WEB-M2 — clear the stored session before redirecting; the full-page
|
// Fix: WEB-M2 — terminate the session atomically: clear storage, tell the
|
||||||
// navigation resets the AuthProvider, so context state re-derives as logged out.
|
// AuthProvider to drop in-memory state, then hard-redirect as belt-and-braces.
|
||||||
clearAuth();
|
clearAuth();
|
||||||
|
window.dispatchEvent(new Event(AUTH_SESSION_CLEARED_EVENT));
|
||||||
window.location.href = '/login';
|
window.location.href = '/login';
|
||||||
return Promise.reject(new Error('Session expired. Please log in again.'));
|
return Promise.reject(new Error('Session expired. Please log in again.'));
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -67,6 +67,17 @@ describe('authStorage', () => {
|
||||||
expect(isTokenValid(createToken(-3600))).toBe(false);
|
expect(isTokenValid(createToken(-3600))).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('isTokenValid accepts a base64url payload (JWT segments are base64url, not base64)', () => {
|
||||||
|
// '>>>' encodes to 'Pj4-' in base64url — the '-' made the old atob() call throw.
|
||||||
|
const payload = btoa(JSON.stringify({ sub: '>>>>>>', exp: Math.floor(Date.now() / 1000) + 3600 }))
|
||||||
|
.replace(/\+/g, '-')
|
||||||
|
.replace(/\//g, '_')
|
||||||
|
.replace(/=+$/, '');
|
||||||
|
expect(payload).toMatch(/[-_]/);
|
||||||
|
|
||||||
|
expect(isTokenValid(`${btoa(JSON.stringify({ alg: 'HS256' }))}.${payload}.sig`)).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
it('QA-C5: isTokenValid rejects missing or malformed tokens', () => {
|
it('QA-C5: isTokenValid rejects missing or malformed tokens', () => {
|
||||||
expect(isTokenValid(null)).toBe(false);
|
expect(isTokenValid(null)).toBe(false);
|
||||||
expect(isTokenValid(undefined)).toBe(false);
|
expect(isTokenValid(undefined)).toBe(false);
|
||||||
|
|
|
||||||
|
|
@ -22,12 +22,17 @@ export function clearAuth(): void {
|
||||||
window.sessionStorage.removeItem(STORAGE_KEY_TOKEN); // WEB-C1
|
window.sessionStorage.removeItem(STORAGE_KEY_TOKEN); // WEB-C1
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Dispatched by non-React code (the 401 interceptor) so AuthProvider can drop
|
||||||
|
// in-memory session state synchronously, keeping storage and context atomic.
|
||||||
|
export const AUTH_SESSION_CLEARED_EVENT = 'auth:session-cleared';
|
||||||
|
|
||||||
export function isTokenValid(token: string | null | undefined): boolean {
|
export function isTokenValid(token: string | null | undefined): boolean {
|
||||||
if (!token) return false;
|
if (!token) return false;
|
||||||
try {
|
try {
|
||||||
const part = token.split('.')[1];
|
const part = token.split('.')[1];
|
||||||
if (!part) return false;
|
if (!part) return false;
|
||||||
const payload = JSON.parse(atob(part));
|
// JWT segments are base64url; atob only accepts standard base64.
|
||||||
|
const payload = JSON.parse(atob(part.replace(/-/g, '+').replace(/_/g, '/')));
|
||||||
return new Date(payload.exp * 1000) > new Date();
|
return new Date(payload.exp * 1000) > new Date();
|
||||||
} catch {
|
} catch {
|
||||||
return false;
|
return false;
|
||||||
|
|
|
||||||
|
|
@ -3,9 +3,16 @@
|
||||||
// this provider only owns the in-memory state. Token acquisition (Cognito code
|
// this provider only owns the in-memory state. Token acquisition (Cognito code
|
||||||
// exchange, dev-login) stays in the auth pages — they hand the resolved AuthUser
|
// exchange, dev-login) stays in the auth pages — they hand the resolved AuthUser
|
||||||
// to login().
|
// to login().
|
||||||
import { useCallback, useMemo, useState, type ReactNode } from 'react';
|
import { useCallback, useEffect, useMemo, useState, type ReactNode } from 'react';
|
||||||
import type { AuthUser } from '@proposal-system/api-contracts';
|
import type { AuthUser } from '@proposal-system/api-contracts';
|
||||||
import { clearAuth, getAuthUser, isTokenValid, setAuthUser } from '../lib/auth/authStorage';
|
import {
|
||||||
|
AUTH_SESSION_CLEARED_EVENT,
|
||||||
|
clearAuth,
|
||||||
|
getAuthUser,
|
||||||
|
isTokenValid,
|
||||||
|
setAuthUser,
|
||||||
|
} from '../lib/auth/authStorage';
|
||||||
|
import { queryClient } from '../lib/queryClient';
|
||||||
import { AuthContext, type AuthContextValue } from './authContext';
|
import { AuthContext, type AuthContextValue } from './authContext';
|
||||||
|
|
||||||
export default function AuthProvider({ children }: { children: ReactNode }) {
|
export default function AuthProvider({ children }: { children: ReactNode }) {
|
||||||
|
|
@ -22,10 +29,22 @@ export default function AuthProvider({ children }: { children: ReactNode }) {
|
||||||
|
|
||||||
const logout = useCallback(() => {
|
const logout = useCallback(() => {
|
||||||
clearAuth();
|
clearAuth();
|
||||||
|
// Drop all principal-scoped state: cached queries must not survive into the
|
||||||
|
// next login in the same tab, or query keys shared across users would serve
|
||||||
|
// the previous principal's data without hitting the server.
|
||||||
|
queryClient.clear();
|
||||||
setUser(null);
|
setUser(null);
|
||||||
setErrorState(null);
|
setErrorState(null);
|
||||||
}, []);
|
}, []);
|
||||||
|
|
||||||
|
// The 401 interceptor clears storage outside React; mirror it here so context
|
||||||
|
// state never outlives the stored session while the redirect commits.
|
||||||
|
useEffect(() => {
|
||||||
|
const onSessionCleared = () => setUser(null);
|
||||||
|
window.addEventListener(AUTH_SESSION_CLEARED_EVENT, onSessionCleared);
|
||||||
|
return () => window.removeEventListener(AUTH_SESSION_CLEARED_EVENT, onSessionCleared);
|
||||||
|
}, []);
|
||||||
|
|
||||||
const setError = useCallback((message: string | null) => {
|
const setError = useCallback((message: string | null) => {
|
||||||
setErrorState(message);
|
setErrorState(message);
|
||||||
setIsLoading(false);
|
setIsLoading(false);
|
||||||
|
|
|
||||||
|
|
@ -13,7 +13,8 @@ import { describe, it, expect, beforeEach } from 'vitest';
|
||||||
import type { AuthUser } from '@proposal-system/api-contracts';
|
import type { AuthUser } from '@proposal-system/api-contracts';
|
||||||
import AuthProvider from '../AuthProvider';
|
import AuthProvider from '../AuthProvider';
|
||||||
import { useAuthContext } from '../authContext';
|
import { useAuthContext } from '../authContext';
|
||||||
import { setAuthUser } from '../../lib/auth/authStorage';
|
import { AUTH_SESSION_CLEARED_EVENT, setAuthUser } from '../../lib/auth/authStorage';
|
||||||
|
import { queryClient } from '../../lib/queryClient';
|
||||||
import { STORAGE_KEY_TOKEN } from '../../constants';
|
import { STORAGE_KEY_TOKEN } from '../../constants';
|
||||||
|
|
||||||
function createToken(expOffsetSeconds: number): string {
|
function createToken(expOffsetSeconds: number): string {
|
||||||
|
|
@ -78,6 +79,28 @@ describe('AuthProvider', () => {
|
||||||
expect(window.sessionStorage.getItem(STORAGE_KEY_TOKEN)).toBeNull();
|
expect(window.sessionStorage.getItem(STORAGE_KEY_TOKEN)).toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('logout clears the react-query cache so the next principal cannot read cached data', () => {
|
||||||
|
queryClient.setQueryData(['proposals', 'list'], [{ id: 'p1' }]);
|
||||||
|
const { result } = renderAuth();
|
||||||
|
|
||||||
|
act(() => result.current.login(createUser()));
|
||||||
|
act(() => result.current.logout());
|
||||||
|
|
||||||
|
expect(queryClient.getQueryData(['proposals', 'list'])).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('drops in-memory session state when the 401 interceptor broadcasts session-cleared', () => {
|
||||||
|
const { result } = renderAuth();
|
||||||
|
|
||||||
|
act(() => result.current.login(createUser()));
|
||||||
|
act(() => {
|
||||||
|
window.dispatchEvent(new Event(AUTH_SESSION_CLEARED_EVENT));
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(result.current.user).toBeNull();
|
||||||
|
expect(result.current.isAuthenticated).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
it('QA-C5: initializes from an existing stored session', () => {
|
it('QA-C5: initializes from an existing stored session', () => {
|
||||||
const user = createUser();
|
const user = createUser();
|
||||||
setAuthUser(user);
|
setAuthUser(user);
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue