From c9e104cc549f227f342eb46dd3eeb3c77c8b1293 Mon Sep 17 00:00:00 2001 From: Shawn Jackson Date: Sat, 31 Oct 2026 13:22:20 -0700 Subject: [PATCH 1/2] RU-T58 Fixing store and api issues --- .../__tests__/check-in-timers.test.ts | 25 ++++++ src/api/check-in-timers/check-in-timers.ts | 8 +- .../__tests__/mapping-path-routes.test.ts | 87 +++++++++++++++++++ src/api/mapping/mapping.ts | 68 ++++++--------- .../__tests__/background-geolocation.test.ts | 45 ++++++++++ src/lib/storage/background-geolocation.ts | 9 +- 6 files changed, 190 insertions(+), 52 deletions(-) create mode 100644 src/api/check-in-timers/__tests__/check-in-timers.test.ts create mode 100644 src/api/mapping/__tests__/mapping-path-routes.test.ts create mode 100644 src/lib/storage/__tests__/background-geolocation.test.ts diff --git a/src/api/check-in-timers/__tests__/check-in-timers.test.ts b/src/api/check-in-timers/__tests__/check-in-timers.test.ts new file mode 100644 index 00000000..f3e5cf22 --- /dev/null +++ b/src/api/check-in-timers/__tests__/check-in-timers.test.ts @@ -0,0 +1,25 @@ +jest.mock('../../common/client', () => ({ + createApiEndpoint: jest.fn(() => ({ + get: jest.fn(), + post: jest.fn(), + put: jest.fn(), + delete: jest.fn(), + })), +})); + +import { createApiEndpoint } from '../../common/client'; +import { toggleCallTimers } from '../check-in-timers'; + +const mockCreateApiEndpoint = createApiEndpoint as jest.MockedFunction; + +describe('toggleCallTimers', () => { + it('sends callId and enabled in the query string with an empty body', async () => { + const endpoint = { get: jest.fn(), post: jest.fn(), put: jest.fn().mockResolvedValue({ data: { Id: '' } }), delete: jest.fn() }; + mockCreateApiEndpoint.mockReturnValueOnce(endpoint as unknown as ReturnType); + + await toggleCallTimers(101, true); + + expect(mockCreateApiEndpoint).toHaveBeenLastCalledWith('/CheckInTimers/ToggleCallTimers?callId=101&enabled=true'); + expect(endpoint.put).toHaveBeenCalledWith({}); + }); +}); diff --git a/src/api/check-in-timers/check-in-timers.ts b/src/api/check-in-timers/check-in-timers.ts index 6d95c2e9..63e58119 100644 --- a/src/api/check-in-timers/check-in-timers.ts +++ b/src/api/check-in-timers/check-in-timers.ts @@ -9,7 +9,6 @@ const getTimerStatusesApi = createApiEndpoint('/CheckInTimers/GetTimerStatuses') const getTimersForCallApi = createApiEndpoint('/CheckInTimers/GetTimersForCall'); const performCheckInApi = createApiEndpoint('/CheckInTimers/PerformCheckIn'); const getCheckInHistoryApi = createApiEndpoint('/CheckInTimers/GetCheckInHistory'); -const toggleCallTimersApi = createApiEndpoint('/CheckInTimers/ToggleCallTimers'); export interface PerformCheckInInput { CallId: number; @@ -53,10 +52,9 @@ export const getCheckInHistory = async (callId: number) => { return response.data; }; +// The server binds callId and enabled from the query string, not the body. export const toggleCallTimers = async (callId: number, enabled: boolean) => { - const response = await toggleCallTimersApi.put({ - CallId: callId, - Enabled: enabled, - }); + const endpoint = createApiEndpoint(`/CheckInTimers/ToggleCallTimers?callId=${encodeURIComponent(callId)}&enabled=${encodeURIComponent(enabled)}`); + const response = await endpoint.put({}); return response.data; }; diff --git a/src/api/mapping/__tests__/mapping-path-routes.test.ts b/src/api/mapping/__tests__/mapping-path-routes.test.ts new file mode 100644 index 00000000..9e8adf5f --- /dev/null +++ b/src/api/mapping/__tests__/mapping-path-routes.test.ts @@ -0,0 +1,87 @@ +// Id-scoped endpoints are created per call, so responses are looked up by path when each endpoint is made. +jest.mock('../../common/client', () => { + const endpoints: Record = {}; + const responses: Record = {}; + return { + __endpoints: endpoints, + __responses: responses, + createApiEndpoint: (path: string) => { + const api = { get: jest.fn().mockResolvedValue({ data: path in responses ? responses[path] : { Data: {} } }) }; + endpoints[path] = api; + return api; + }, + }; +}); + +jest.mock('../../common/cached-client', () => { + const endpoints: Record = {}; + return { + __endpoints: endpoints, + createCachedApiEndpoint: (path: string) => { + const api = { get: jest.fn().mockResolvedValue({ data: { Data: {} } }) }; + endpoints[path] = api; + return api; + }, + }; +}); + +jest.mock('@/lib/storage/app', () => ({ + getBaseApiUrl: () => 'https://api.example.test/api/v4', +})); + +import { getCustomMap, getCustomMapLayer, getCustomMapRegionsGeoJSON, getIndoorMap, getIndoorMapFloor, getIndoorMapZonesGeoJSON, getMapLayerGeoJSON, getPoi } from '../mapping'; + +const mockClient = jest.requireMock('../../common/client') as { __endpoints: Record; __responses: Record }; +const mockEndpoints = mockClient.__endpoints; +const mockResponses = mockClient.__responses; +const mockCachedEndpoints: Record = jest.requireMock('../../common/cached-client').__endpoints; + +const featureCollection = { type: 'FeatureCollection', features: [{ type: 'Feature', geometry: null, properties: { name: 'Zone A' } }] }; + +describe('mapping id-scoped routes', () => { + it('never creates the id-less query-string routes', () => { + const created = [...Object.keys(mockEndpoints), ...Object.keys(mockCachedEndpoints)]; + ['GetPoi', 'GetIndoorMap', 'GetIndoorMapFloor', 'GetCustomMap', 'GetCustomMapLayer', 'GetMapLayerGeoJSON', 'GetIndoorMapZonesGeoJSON', 'GetCustomMapRegionsGeoJSON'].forEach((action) => { + expect(created).not.toContain(`/Mapping/${action}`); + }); + }); + + it('puts the POI id in the GetPoi path', async () => { + await getPoi(15); + + expect(mockCachedEndpoints['/Mapping/GetPoi/15'].get).toHaveBeenCalledWith(); + }); + + it.each([ + ['GetIndoorMap', getIndoorMap], + ['GetIndoorMapFloor', getIndoorMapFloor], + ['GetCustomMap', getCustomMap], + ['GetCustomMapLayer', getCustomMapLayer], + ])('puts the id in the %s path', async (action, call) => { + await call('a/b c'); + + expect(mockEndpoints[`/Mapping/${action}/a%2Fb%20c`].get).toHaveBeenCalledWith(); + }); + + it.each([ + ['GetMapLayerGeoJSON', getMapLayerGeoJSON], + ['GetIndoorMapZonesGeoJSON', getIndoorMapZonesGeoJSON], + ['GetCustomMapRegionsGeoJSON', getCustomMapRegionsGeoJSON], + ])('puts the id in the %s path and wraps the raw FeatureCollection in Data', async (action, call) => { + const path = `/Mapping/${action}/layer-1`; + mockResponses[path] = featureCollection; + + const result = await call('layer-1'); + + expect(mockEndpoints[path].get).toHaveBeenCalledWith(); + expect(result.Data).toEqual(featureCollection); + }); + + it('falls back to an empty FeatureCollection when the body has no features', async () => { + mockResponses['/Mapping/GetMapLayerGeoJSON/empty-layer'] = ''; + + const result = await getMapLayerGeoJSON('empty-layer'); + + expect(result.Data).toEqual({ type: 'FeatureCollection', features: [] }); + }); +}); diff --git a/src/api/mapping/mapping.ts b/src/api/mapping/mapping.ts index c6b3eaca..06d7936a 100644 --- a/src/api/mapping/mapping.ts +++ b/src/api/mapping/mapping.ts @@ -26,10 +26,6 @@ const getMayLayersApi = createApiEndpoint('/Mapping/GetMayLayers'); const getMapDataAndMarkersApi = createApiEndpoint('/Mapping/GetMapDataAndMarkers'); // Not cached: the legend follows the department's types, statuses, priorities and uploads, so every open asks again. const getMapIconLegendApi = createApiEndpoint('/Mapping/GetMapIconLegend'); -const getPoiApi = createCachedApiEndpoint('/Mapping/GetPoi', { - ttl: 5 * 60 * 1000, - enabled: true, -}); const getPoisApi = createCachedApiEndpoint('/Mapping/GetPois', { ttl: 5 * 60 * 1000, enabled: true, @@ -44,9 +40,6 @@ const getIndoorMapsApi = createCachedApiEndpoint('/Mapping/GetIndoorMaps', { ttl: 5 * 60 * 1000, enabled: true, }); -const getIndoorMapApi = createApiEndpoint('/Mapping/GetIndoorMap'); -const getIndoorMapFloorApi = createApiEndpoint('/Mapping/GetIndoorMapFloor'); -const getIndoorMapZonesGeoJSONApi = createApiEndpoint('/Mapping/GetIndoorMapZonesGeoJSON'); const searchIndoorLocationsApi = createApiEndpoint('/Mapping/SearchIndoorLocations'); const getNearbyIndoorMapsApi = createApiEndpoint('/Mapping/GetNearbyIndoorMaps'); @@ -55,12 +48,20 @@ const getCustomMapsApi = createCachedApiEndpoint('/Mapping/GetCustomMaps', { ttl: 5 * 60 * 1000, enabled: true, }); -const getCustomMapApi = createApiEndpoint('/Mapping/GetCustomMap'); -const getCustomMapLayerApi = createApiEndpoint('/Mapping/GetCustomMapLayer'); -const getMapLayerGeoJSONApi = createApiEndpoint('/Mapping/GetMapLayerGeoJSON'); -const getCustomMapRegionsGeoJSONApi = createApiEndpoint('/Mapping/GetCustomMapRegionsGeoJSON'); const searchCustomMapRegionsApi = createApiEndpoint('/Mapping/SearchCustomMapRegions'); +/** + * The GeoJSON endpoints return a raw `application/geo+json` FeatureCollection body + * (not the standard v4 `{ Data: ... }` envelope). Accept either shape. + */ +const unwrapGeoJSON = (body: unknown): FeatureCollection => { + const asAny = body as { type?: string; Data?: FeatureCollection }; + if (asAny?.type === 'FeatureCollection') { + return body as FeatureCollection; + } + return asAny?.Data ?? { type: 'FeatureCollection', features: [] }; +}; + // Discovery endpoints const getAllActiveLayersApi = createCachedApiEndpoint('/Mapping/GetAllActiveLayers', { ttl: 5 * 60 * 1000, @@ -99,10 +100,9 @@ export const getPois = async (poiTypeId?: number, destinationOnly?: boolean) => return response.data; }; +// The id-scoped endpoints below are path routes on the server (`/{id}`), so each call builds its own endpoint. export const getPoi = async (poiId: number | string) => { - const response = await getPoiApi.get({ - poiId: encodeURIComponent(String(poiId)), - }); + const response = await createCachedApiEndpoint(`/Mapping/GetPoi/${encodeURIComponent(String(poiId))}`, { ttl: 5 * 60 * 1000, enabled: true }).get(); return response.data; }; @@ -124,24 +124,18 @@ export const getIndoorMaps = async () => { }; export const getIndoorMap = async (mapId: string) => { - const response = await getIndoorMapApi.get({ - id: encodeURIComponent(mapId), - }); + const response = await createApiEndpoint(`/Mapping/GetIndoorMap/${encodeURIComponent(mapId)}`).get(); return response.data; }; export const getIndoorMapFloor = async (floorId: string) => { - const response = await getIndoorMapFloorApi.get({ - floorId: encodeURIComponent(floorId), - }); + const response = await createApiEndpoint(`/Mapping/GetIndoorMapFloor/${encodeURIComponent(floorId)}`).get(); return response.data; }; -export const getIndoorMapZonesGeoJSON = async (floorId: string) => { - const response = await getIndoorMapZonesGeoJSONApi.get({ - floorId: encodeURIComponent(floorId), - }); - return response.data; +export const getIndoorMapZonesGeoJSON = async (floorId: string): Promise => { + const response = await createApiEndpoint(`/Mapping/GetIndoorMapZonesGeoJSON/${encodeURIComponent(floorId)}`).get(); + return { Data: unwrapGeoJSON(response.data) } as GetGeoJSONResult; }; export const searchIndoorLocations = async (term: string, mapId?: string) => { @@ -174,31 +168,23 @@ export const getCustomMaps = async (type?: number) => { }; export const getCustomMap = async (mapId: string) => { - const response = await getCustomMapApi.get({ - id: encodeURIComponent(mapId), - }); + const response = await createApiEndpoint(`/Mapping/GetCustomMap/${encodeURIComponent(mapId)}`).get(); return response.data; }; export const getCustomMapLayer = async (layerId: string) => { - const response = await getCustomMapLayerApi.get({ - layerId: encodeURIComponent(layerId), - }); + const response = await createApiEndpoint(`/Mapping/GetCustomMapLayer/${encodeURIComponent(layerId)}`).get(); return response.data; }; -export const getMapLayerGeoJSON = async (layerId: string) => { - const response = await getMapLayerGeoJSONApi.get({ - layerId: encodeURIComponent(layerId), - }); - return response.data; +export const getMapLayerGeoJSON = async (layerId: string): Promise => { + const response = await createApiEndpoint(`/Mapping/GetMapLayerGeoJSON/${encodeURIComponent(layerId)}`).get(); + return { Data: unwrapGeoJSON(response.data) } as GetGeoJSONResult; }; -export const getCustomMapRegionsGeoJSON = async (layerId: string) => { - const response = await getCustomMapRegionsGeoJSONApi.get({ - layerId: encodeURIComponent(layerId), - }); - return response.data; +export const getCustomMapRegionsGeoJSON = async (layerId: string): Promise => { + const response = await createApiEndpoint(`/Mapping/GetCustomMapRegionsGeoJSON/${encodeURIComponent(layerId)}`).get(); + return { Data: unwrapGeoJSON(response.data) } as GetGeoJSONResult; }; export const searchCustomMapRegions = async (term: string, layerId?: string) => { diff --git a/src/lib/storage/__tests__/background-geolocation.test.ts b/src/lib/storage/__tests__/background-geolocation.test.ts new file mode 100644 index 00000000..1d770644 --- /dev/null +++ b/src/lib/storage/__tests__/background-geolocation.test.ts @@ -0,0 +1,45 @@ +const mockValues = new Map(); +const mockMMKVConstructor = jest.fn(); + +jest.mock('react-native-mmkv', () => ({ + MMKV: jest.fn().mockImplementation((...args: unknown[]) => mockMMKVConstructor(...args)), +})); + +jest.mock('../index', () => ({ + storage: { + getBoolean: jest.fn((key: string) => mockValues.get(key)), + set: jest.fn((key: string, value: boolean) => { + mockValues.set(key, value); + }), + }, +})); + +jest.mock('../../logging', () => ({ + logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn(), debug: jest.fn() }, +})); + +import { storage } from '../index'; +import { getBackgroundGeolocationStorageKey, loadBackgroundGeolocationState, saveBackgroundGeolocationState } from '../background-geolocation'; + +describe('background geolocation storage', () => { + beforeEach(() => { + mockValues.clear(); + }); + + // A second MMKV instance on the main store id with a different encryption key could open (and encrypt) the whole + // store with the wrong key, so the setting lives in the app's one shared instance. + it('never opens its own MMKV instance', () => { + expect(mockMMKVConstructor).not.toHaveBeenCalled(); + }); + + it('saves to and loads from the shared storage instance', async () => { + saveBackgroundGeolocationState(true); + + expect(storage.set).toHaveBeenCalledWith(getBackgroundGeolocationStorageKey(), true); + expect(await loadBackgroundGeolocationState()).toBe(true); + }); + + it('defaults to disabled when nothing was saved', async () => { + expect(await loadBackgroundGeolocationState()).toBe(false); + }); +}); diff --git a/src/lib/storage/background-geolocation.ts b/src/lib/storage/background-geolocation.ts index 4ac26d71..66f2ffb0 100644 --- a/src/lib/storage/background-geolocation.ts +++ b/src/lib/storage/background-geolocation.ts @@ -1,13 +1,10 @@ -import { Platform } from 'react-native'; -import { MMKV } from 'react-native-mmkv'; - import { logger } from '../logging'; +// Use the app's single MMKV instance. Opening the 'ResgridUnit' id a second time with a different +// encryption key risks the whole store being opened (and encrypted) with the wrong key. +import { storage } from './index'; const BACKGROUND_GEOLOCATION_ENABLED = 'BACKGROUND_GEOLOCATION_ENABLED'; -// Create storage instance to avoid circular dependency -const storage = Platform.OS === 'web' ? new MMKV({ id: 'ResgridUnit' }) : new MMKV({ id: 'ResgridUnit', encryptionKey: 'hunter2' }); - /** * Load background geolocation state from MMKV storage * This function is used in the location service to avoid circular dependencies From 8d98cde64c1e2d3934a0ea91f0cc4f1d4624cc24 Mon Sep 17 00:00:00 2001 From: Shawn Jackson Date: Sat, 31 Oct 2026 19:18:49 -0700 Subject: [PATCH 2/2] RU-T58 PR #301 Fixes --- src/api/common/client.tsx | 2 +- src/lib/auth/__tests__/token-refresh.test.ts | 27 +++++++++++++++---- src/lib/auth/token-refresh.ts | 21 +++++++++++---- src/lib/logging/__tests__/index.test.ts | 13 +++++++++ src/lib/logging/index.tsx | 6 ++++- .../auth/__tests__/store-cold-start.test.ts | 10 +++---- src/stores/auth/store.tsx | 12 ++++----- 7 files changed, 68 insertions(+), 23 deletions(-) diff --git a/src/api/common/client.tsx b/src/api/common/client.tsx index 869096bd..27f4eb0b 100644 --- a/src/api/common/client.tsx +++ b/src/api/common/client.tsx @@ -152,7 +152,7 @@ axiosInstance.interceptors.response.use( processQueue(refreshError as Error); // The store has already classified the failure: a credential rejection - // (400/401 from the token endpoint) triggered logout there; anything + // (a 400/401 invalid_grant from the token endpoint) triggered logout there; anything // else is transient and the session is preserved for a later retry. logger.warn({ message: 'Request failed after token refresh attempt', diff --git a/src/lib/auth/__tests__/token-refresh.test.ts b/src/lib/auth/__tests__/token-refresh.test.ts index edf8c0bd..c3337bd9 100644 --- a/src/lib/auth/__tests__/token-refresh.test.ts +++ b/src/lib/auth/__tests__/token-refresh.test.ts @@ -3,18 +3,35 @@ import { AxiosError, type AxiosResponse } from 'axios'; import { isRefreshCredentialRejection } from '../token-refresh'; -const axiosErrorWithStatus = (status: number): AxiosError => { - const response = { status, statusText: '', data: {}, headers: {}, config: {} as never } as AxiosResponse; +const axiosErrorWithStatus = (status: number, data: unknown = {}): AxiosError => { + const response = { status, statusText: '', data, headers: {}, config: {} as never } as AxiosResponse; return new AxiosError('Request failed', 'ERR_BAD_RESPONSE', undefined, undefined, response); }; describe('isRefreshCredentialRejection', () => { it('returns true when the token endpoint rejects the refresh token (400 invalid_grant)', () => { - expect(isRefreshCredentialRejection(axiosErrorWithStatus(400))).toBe(true); + expect(isRefreshCredentialRejection(axiosErrorWithStatus(400, { error: 'invalid_grant', error_description: 'The refresh token is no longer valid.' }))).toBe(true); }); - it('returns true for a 401 from the token endpoint', () => { - expect(isRefreshCredentialRejection(axiosErrorWithStatus(401))).toBe(true); + it('returns true for a 401 invalid_grant from the token endpoint', () => { + expect(isRefreshCredentialRejection(axiosErrorWithStatus(401, { error: 'invalid_grant' }))).toBe(true); + }); + + it.each(['invalid_request', 'invalid_client', 'unauthorized_client', 'unsupported_grant_type', 'invalid_scope'])('returns false for a 400 %s (says nothing about the refresh token)', (oauthError) => { + expect(isRefreshCredentialRejection(axiosErrorWithStatus(400, { error: oauthError }))).toBe(false); + }); + + it('returns false for a 401 invalid_client (client authentication, not the refresh token)', () => { + expect(isRefreshCredentialRejection(axiosErrorWithStatus(401, { error: 'invalid_client' }))).toBe(false); + }); + + it.each([400, 401])('returns false for a %i with no OAuth error body (e.g. a proxy error page)', (status) => { + expect(isRefreshCredentialRejection(axiosErrorWithStatus(status))).toBe(false); + expect(isRefreshCredentialRejection(axiosErrorWithStatus(status, 'Bad Request'))).toBe(false); + }); + + it('returns false for invalid_grant on a non-rejection status', () => { + expect(isRefreshCredentialRejection(axiosErrorWithStatus(500, { error: 'invalid_grant' }))).toBe(false); }); it.each([500, 502, 503, 504, 429])('returns false for transient server status %i (preserve session)', (status) => { diff --git a/src/lib/auth/token-refresh.ts b/src/lib/auth/token-refresh.ts index 09c09773..c6b5c07c 100644 --- a/src/lib/auth/token-refresh.ts +++ b/src/lib/auth/token-refresh.ts @@ -5,23 +5,33 @@ import { isAxiosError } from 'axios'; * known-bad and the user must be logged out. * * Returns true ONLY when the OAuth2 token endpoint explicitly rejected the refresh - * token — HTTP 400 (`invalid_grant`) or 401. Every other failure is treated as - * transient and recoverable: + * token — a 400 or 401 whose body says `invalid_grant`. Every other failure is treated + * as transient and recoverable: * - no response at all (offline, DNS/TLS failure, request timeout) * - 5xx server errors (the backend is down/degraded, e.g. 502 Bad Gateway) * - 429 rate limiting + * - a 400/401 with any other OAuth error (`invalid_client`, `invalid_request`, ...) or + * no OAuth body at all (a proxy's error page): those say nothing about this token * - any non-Axios error (e.g. "no refresh token available") * * Treating those as recoverable (reject + retry on the next request) instead of * forcing logout prevents a backend incident from logging out every active user at - * once — critical for an emergency-response app. + * once — critical for an emergency-response app. A refresh token that really is dead + * still ends the session once it expires. + * + * A locked shared session is also refused with a 400 `invalid_grant`, so callers check + * isSharedSessionLockedRefresh first. */ export const isRefreshCredentialRejection = (error: unknown): boolean => { if (!isAxiosError(error)) { return false; } const status = error.response?.status; - return status === 400 || status === 401; + if (status !== 400 && status !== 401) { + return false; + } + const data: unknown = error.response?.data; + return typeof data === 'object' && data !== null && (data as { error?: unknown }).error === 'invalid_grant'; }; /** The HTTP response an error carries, read by shape so any HTTP error (not only an AxiosError instance) is understood. */ @@ -33,7 +43,8 @@ const responseOf = (error: unknown): { status?: number; data?: unknown } | undef /** * A refresh refused because the shared session is locked (passkey workbook section 1.3): `invalid_grant` with * `shared_session_locked: true`. Not a sign-out: the tokens stay, the lock screen unlocks the same session, and the - * refresh runs again afterwards. Checked before isRefreshCredentialRejection, which would otherwise read it as a 400. + * refresh runs again afterwards. Checked before isRefreshCredentialRejection, which would otherwise read its + * 400 invalid_grant as a dead refresh token. */ export const isSharedSessionLockedRefresh = (error: unknown): boolean => { const response = responseOf(error); diff --git a/src/lib/logging/__tests__/index.test.ts b/src/lib/logging/__tests__/index.test.ts index 8e26642f..86da8463 100644 --- a/src/lib/logging/__tests__/index.test.ts +++ b/src/lib/logging/__tests__/index.test.ts @@ -191,4 +191,17 @@ describe('sanitizeLogContext', () => { error: { name: 'Error', message: 'boom', stack: 'Error: boom\n at somewhere' }, }); }); + + it('redacts sensitive keys inside array elements', () => { + expect(sanitizeLogContext({ items: [{ accessToken: 'a', unitId: 'u1' }, 'plain'] })).toEqual({ items: [{ accessToken: '[REDACTED]', unitId: 'u1' }, 'plain'] }); + }); + + it('replaces objects and arrays past the depth limit instead of passing their keys through unchecked', () => { + expect(sanitizeLogContext({ a: { b: { c: { password: 'x' }, list: [{ password: 'x' }] } } })).toEqual({ a: { b: { c: '[Object]', list: '[Array]' } } }); + }); + + it('keeps Date values intact', () => { + const at = new Date('2026-10-31T12:00:00Z'); + expect(sanitizeLogContext({ at, nested: { at } })).toEqual({ at, nested: { at } }); + }); }); diff --git a/src/lib/logging/index.tsx b/src/lib/logging/index.tsx index 60e0459e..59abfbf2 100644 --- a/src/lib/logging/index.tsx +++ b/src/lib/logging/index.tsx @@ -52,7 +52,11 @@ const sanitizeValue = (key: string, value: unknown, depth: number): unknown => { if (value instanceof Error) { return { name: value.name, message: value.message, stack: value.stack }; } - if (depth > 0 && value !== null && typeof value === 'object' && !Array.isArray(value)) { + if (value !== null && typeof value === 'object' && !(value instanceof Date)) { + // Past the depth limit the keys go unchecked, so a nested credential would pass through: drop the value instead. + if (depth <= 0) return Array.isArray(value) ? '[Array]' : '[Object]'; + // Array elements are checked like object values, so { items: [{ accessToken }] } is redacted too. + if (Array.isArray(value)) return value.map((item) => sanitizeValue('', item, depth - 1)); return sanitizeObject(value as Record, depth - 1); } if (typeof value === 'function' || typeof value === 'symbol') return String(value); diff --git a/src/stores/auth/__tests__/store-cold-start.test.ts b/src/stores/auth/__tests__/store-cold-start.test.ts index bb4c8fc5..ed2a71eb 100644 --- a/src/stores/auth/__tests__/store-cold-start.test.ts +++ b/src/stores/auth/__tests__/store-cold-start.test.ts @@ -60,8 +60,8 @@ const makeAccessToken = (expiresAtMs: number): string => { return `header.${encoded}.signature`; }; -const axiosErrorWithStatus = (status: number): AxiosError => { - const response = { status, statusText: '', data: {}, headers: {}, config: {} as never } as AxiosResponse; +const axiosErrorWithStatus = (status: number, data: unknown = {}): AxiosError => { + const response = { status, statusText: '', data, headers: {}, config: {} as never } as AxiosResponse; return new AxiosError('Request failed', 'ERR_BAD_RESPONSE', undefined, undefined, response); }; @@ -281,8 +281,8 @@ describe('auth store cold start', () => { }); describe('credential rejection', () => { - it.each([400, 401])('logs out when the token endpoint rejects the refresh token (%i)', async (status) => { - mockRefreshTokenRequest.mockRejectedValue(axiosErrorWithStatus(status)); + it.each([400, 401])('logs out when the token endpoint rejects the refresh token (%i invalid_grant)', async (status) => { + mockRefreshTokenRequest.mockRejectedValue(axiosErrorWithStatus(status, { error: 'invalid_grant' })); useAuthStore.setState({ refreshToken: 'stored-refresh-token', accessToken: 'stale', status: 'signedIn', userId: 'user-1' }); await useAuthStore.getState().refreshAccessToken(); @@ -296,7 +296,7 @@ describe('auth store cold start', () => { describe('logout idempotence', () => { it('runs the session cleanup once when concurrent refreshes are all rejected', async () => { - mockRefreshTokenRequest.mockRejectedValue(axiosErrorWithStatus(401)); + mockRefreshTokenRequest.mockRejectedValue(axiosErrorWithStatus(401, { error: 'invalid_grant' })); useAuthStore.setState({ refreshToken: 'stored-refresh-token', status: 'signedIn' }); // Three independent callers (proactive timer + two queued 401s) race. diff --git a/src/stores/auth/store.tsx b/src/stores/auth/store.tsx index f6df32bb..2dd78368 100644 --- a/src/stores/auth/store.tsx +++ b/src/stores/auth/store.tsx @@ -435,7 +435,7 @@ const useAuthStore = create()( } if (isRefreshCredentialRejection(error)) { // The token endpoint explicitly rejected the refresh token - // (400 invalid_grant / 401) — credentials are known-bad, log out. + // (a 400/401 invalid_grant) — credentials are known-bad, log out. logger.error({ message: 'Token refresh rejected by server, logging out user', context: { error }, @@ -444,9 +444,9 @@ const useAuthStore = create()( return false; } - // Everything else is transient (offline, timeout, 5xx, 429) — keep the - // session and retry, otherwise a backend incident logs out every - // active responder at once. + // Everything else is transient (offline, timeout, 5xx, 429, other + // OAuth errors) — keep the session and retry, otherwise a backend + // incident logs out every active responder at once. logger.warn({ message: 'Token refresh failed transiently, will retry', context: { error }, @@ -565,8 +565,8 @@ type PersistedSession = Pick { if (!state || !state.refreshToken) {