From e54d2fcf2b5f4b06b933b12a2158492b9bb84f3a Mon Sep 17 00:00:00 2001 From: Shawn Jackson Date: Sat, 31 Oct 2026 13:23:07 -0700 Subject: [PATCH 1/2] RD-T49 Store and api fixes --- app.config.ts | 33 +-- src/api/calendar/__tests__/calendar.test.ts | 85 ++++++ src/api/calendar/calendar.ts | 24 +- .../checkIn/__tests__/checkInTimers.test.ts | 25 ++ src/api/checkIn/checkInTimers.ts | 5 +- src/api/shifts/__tests__/shifts.test.ts | 71 +++++ src/api/shifts/shifts.ts | 22 +- src/app/login/index.tsx | 5 +- src/lib/__tests__/logging.test.ts | 158 ----------- src/lib/auth/api.tsx | 7 +- src/lib/logging/__tests__/index.test.ts | 229 ++++++++++++++++ src/lib/logging/index.tsx | 250 +++++++++--------- src/lib/logging/types.tsx | 2 + src/stores/auth/store.tsx | 2 +- src/stores/shifts/__tests__/store.test.ts | 8 +- src/stores/shifts/store.ts | 10 +- 16 files changed, 596 insertions(+), 340 deletions(-) create mode 100644 src/api/calendar/__tests__/calendar.test.ts create mode 100644 src/api/checkIn/__tests__/checkInTimers.test.ts create mode 100644 src/api/shifts/__tests__/shifts.test.ts delete mode 100644 src/lib/__tests__/logging.test.ts create mode 100644 src/lib/logging/__tests__/index.test.ts diff --git a/app.config.ts b/app.config.ts index 83b7be59..85373c9e 100644 --- a/app.config.ts +++ b/app.config.ts @@ -227,36 +227,25 @@ export default ({ config }: ConfigContext): ExpoConfig => ({ [ 'expo-location', { + // Foreground-only: nothing in src/ requests background location or registers a location task, so every + // background flag stays off. isIosBackgroundLocationEnabled would add `location` to UIBackgroundModes and + // isAndroidBackgroundLocationEnabled would declare ACCESS_BACKGROUND_LOCATION / FOREGROUND_SERVICE_LOCATION; + // App Review (Guideline 2.5.4) and Play reject both when no persistent-location feature exists. locationWhenInUsePermission: 'Resgrid Dispatch uses your location while you use the app to center the department map on you and to attach your coordinates when you dispatch or update a call. For example, when you create a call from the field, your location can be used as the incident address.', + // The "Always" keys must exist anyway: expo-location's background requester references + // requestAlwaysAuthorization, and App Store static analysis rejects the binary with + // ITMS-90683 when they are absent. The app never shows these prompts, so they state the + // same foreground-only use. locationAlwaysAndWhenInUsePermission: - 'Resgrid Dispatch uses your location, including in the background, to keep the department map updated with your position. For example, while you are working a call away from the console, your location is periodically sent so other dispatchers and responders can see where you are, even when the app is not on screen.', + 'Resgrid Dispatch uses your location only while the app is open and never tracks it in the background. It centers the department map on you and attaches your coordinates when you dispatch or update a call. For example, when you create a call from the field, your location can be used as the incident address.', locationAlwaysPermission: - 'Resgrid Dispatch uses your location in the background to keep the department map updated with your position. For example, while you are working a call away from the console, your location is periodically sent so other dispatchers and responders can see where you are, even when the app is not on screen.', + 'Resgrid Dispatch uses your location only while the app is open and never tracks it in the background. It centers the department map on you and attaches your coordinates when you dispatch or update a call. For example, when you create a call from the field, your location can be used as the incident address.', // Required even though getMotionActivityAsync() is never called: expo-location links // CoreMotion (MotionActivityPermissionRequester), and App Store static analysis rejects // the binary with ITMS-90683 whenever the framework is referenced and the string is absent. motionUsagePermission: 'Resgrid Dispatch uses motion data to improve the accuracy of the location shown on the department map. For example, while you are driving to an incident, motion data helps distinguish travel from a stop so other dispatchers and responders see an accurate position.', - isIosBackgroundLocationEnabled: true, - isAndroidBackgroundLocationEnabled: true, - isAndroidForegroundServiceEnabled: true, - taskManager: { - locationTaskName: 'location-updates', - locationTaskOptions: { - accuracy: 'balanced', - distanceInterval: 10, - timeInterval: 5000, - }, - }, - }, - ], - [ - 'expo-task-manager', - { - taskManager: { - taskName: 'location-updates', - }, }, ], [ @@ -313,7 +302,7 @@ export default ({ config }: ConfigContext): ExpoConfig => ({ '@sentry/react-native/expo', { organization: 'sentry', - project: 'unit', + project: 'dispatch', url: 'https://sentry.resgrid.net/', }, ], diff --git a/src/api/calendar/__tests__/calendar.test.ts b/src/api/calendar/__tests__/calendar.test.ts new file mode 100644 index 00000000..98315d73 --- /dev/null +++ b/src/api/calendar/__tests__/calendar.test.ts @@ -0,0 +1,85 @@ +interface MockEndpoint { + get: jest.Mock; + post: jest.Mock; +} + +// The module under test creates its endpoints at import time, so the registry lives inside the mock factory. +jest.mock('../../common/client', () => { + const endpoints: Record = {}; + return { + __endpoints: endpoints, + createApiEndpoint: (path: string) => { + const api = { get: jest.fn(), post: jest.fn() }; + endpoints[path] = api; + return api; + }, + }; +}); + +import { getCalendarItem, getCalendarItems, getCalendarItemsForDateRange, getCalendarItemTypes, setCalendarAttending } from '../calendar'; + +const mockEndpoints: Record = jest.requireMock('../../common/client').__endpoints; + +const endpoint = (path: string) => { + const api = mockEndpoints[path]; + if (!api) throw new Error(`endpoint ${path} was not created`); + return api; +}; + +describe('calendar api', () => { + beforeEach(() => { + Object.values(mockEndpoints).forEach((api) => { + api.get.mockReset().mockResolvedValue({ data: { Data: [] } }); + api.post.mockReset().mockResolvedValue({ data: { Id: '' } }); + }); + }); + + it('only creates routes that exist on the v4 Calendar controller', () => { + expect(Object.keys(mockEndpoints).sort()).toEqual([ + '/Calendar/GetCalendarItem', + '/Calendar/GetDepartmentCalendarItemTypes', + '/Calendar/GetDepartmentCalendarItems', + '/Calendar/GetDepartmentCalendarItemsInRange', + '/Calendar/SetCalendarAttendingStatus', + ]); + }); + + it('reads the department calendar items', async () => { + await getCalendarItems(); + + expect(endpoint('/Calendar/GetDepartmentCalendarItems').get).toHaveBeenCalledWith(); + }); + + it('sends the calendar item id as id', async () => { + await getCalendarItem('42'); + + expect(endpoint('/Calendar/GetCalendarItem').get).toHaveBeenCalledWith({ id: '42' }); + }); + + it('reads the department calendar item types', async () => { + await getCalendarItemTypes(); + + expect(endpoint('/Calendar/GetDepartmentCalendarItemTypes').get).toHaveBeenCalledWith(); + }); + + it('sends the range as start and end without pre-encoding', async () => { + await getCalendarItemsForDateRange('2026-10-01T00:00:00.000Z', '2026-10-31T23:59:59.000Z'); + + expect(endpoint('/Calendar/GetDepartmentCalendarItemsInRange').get).toHaveBeenCalledWith({ + start: '2026-10-01T00:00:00.000Z', + end: '2026-10-31T23:59:59.000Z', + }); + }); + + it('posts attending as CalendarItemAttendInput Type 1', async () => { + await setCalendarAttending({ calendarItemId: '42', attending: true, note: 'On my way' }); + + expect(endpoint('/Calendar/SetCalendarAttendingStatus').post).toHaveBeenCalledWith({ CalendarEventId: '42', Type: 1, Note: 'On my way' }); + }); + + it('posts not attending as Type 4 with an empty note', async () => { + await setCalendarAttending({ calendarItemId: '42', attending: false }); + + expect(endpoint('/Calendar/SetCalendarAttendingStatus').post).toHaveBeenCalledWith({ CalendarEventId: '42', Type: 4, Note: '' }); + }); +}); diff --git a/src/api/calendar/calendar.ts b/src/api/calendar/calendar.ts index c8ac14fd..5861f99c 100644 --- a/src/api/calendar/calendar.ts +++ b/src/api/calendar/calendar.ts @@ -4,17 +4,17 @@ */ import { type CalendarItemResult } from '@/models/v4/calendar/calendarItemResult'; -import { type CalendarItemResultData } from '@/models/v4/calendar/calendarItemResultData'; import { type CalendarItemsResult } from '@/models/v4/calendar/calendarItemsResult'; import { type CalendarItemTypesResult } from '@/models/v4/calendar/calendarItemTypesResult'; +import { type SetCalendarAttendingResult } from '@/models/v4/calendar/setCalendarAttendingResult'; import { createApiEndpoint } from '../common/client'; -const calendarApi = createApiEndpoint('/Calendar/GetCalendarItems'); +const calendarApi = createApiEndpoint('/Calendar/GetDepartmentCalendarItems'); const calendarItemApi = createApiEndpoint('/Calendar/GetCalendarItem'); -const calendarItemTypesApi = createApiEndpoint('/Calendar/GetCalendarItemTypes'); -const calendarItemsForDateRangeApi = createApiEndpoint('/Calendar/GetCalendarItemsForDateRange'); -const setCalendarAttendingApi = createApiEndpoint('/Calendar/SetCalendarAttending'); +const calendarItemTypesApi = createApiEndpoint('/Calendar/GetDepartmentCalendarItemTypes'); +const calendarItemsForDateRangeApi = createApiEndpoint('/Calendar/GetDepartmentCalendarItemsInRange'); +const setCalendarAttendingApi = createApiEndpoint('/Calendar/SetCalendarAttendingStatus'); /** * Get all calendar items @@ -28,7 +28,7 @@ export const getCalendarItems = async (): Promise => { * Get a specific calendar item by ID */ export const getCalendarItem = async (itemId: string): Promise => { - const response = await calendarItemApi.get({ itemId }); + const response = await calendarItemApi.get({ id: itemId }); return response.data; }; @@ -45,15 +45,19 @@ export const getCalendarItemTypes = async (): Promise = */ export const getCalendarItemsForDateRange = async (startDate: string, endDate: string): Promise => { const response = await calendarItemsForDateRangeApi.get({ - startDate, - endDate, + start: startDate, + end: endDate, }); return response.data; }; /** - * Set calendar attendance + * Set calendar attendance (Type 1 = attending, 4 = not attending) */ export const setCalendarAttending = async ({ calendarItemId, attending, note }: { calendarItemId: string; attending: boolean; note?: string }): Promise => { - await setCalendarAttendingApi.post({ itemId: calendarItemId, attending, note }); + await setCalendarAttendingApi.post({ + CalendarEventId: calendarItemId, + Type: attending === true ? 1 : 4, + Note: note || '', + }); }; diff --git a/src/api/checkIn/__tests__/checkInTimers.test.ts b/src/api/checkIn/__tests__/checkInTimers.test.ts new file mode 100644 index 00000000..ebf92144 --- /dev/null +++ b/src/api/checkIn/__tests__/checkInTimers.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 '../checkInTimers'; + +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, false); + + expect(mockCreateApiEndpoint).toHaveBeenLastCalledWith('/CheckInTimers/ToggleCallTimers?callId=101&enabled=false'); + expect(endpoint.put).toHaveBeenCalledWith({}); + }); +}); diff --git a/src/api/checkIn/checkInTimers.ts b/src/api/checkIn/checkInTimers.ts index 2d0b1739..ac65bb3d 100644 --- a/src/api/checkIn/checkInTimers.ts +++ b/src/api/checkIn/checkInTimers.ts @@ -10,7 +10,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'); const getCallPersonnelCheckInStatusesApi = createApiEndpoint('/CheckInTimers/GetCallPersonnelCheckInStatuses'); export const getTimerStatuses = async (callId: number) => { @@ -85,8 +84,10 @@ 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, 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/shifts/__tests__/shifts.test.ts b/src/api/shifts/__tests__/shifts.test.ts new file mode 100644 index 00000000..eff0dce2 --- /dev/null +++ b/src/api/shifts/__tests__/shifts.test.ts @@ -0,0 +1,71 @@ +interface MockEndpoint { + get: jest.Mock; + post: jest.Mock; +} + +// The module under test creates its endpoints at import time, so the registry lives inside the mock factory. +jest.mock('../../common/client', () => { + const endpoints: Record = {}; + return { + __endpoints: endpoints, + createApiEndpoint: (path: string) => { + const api = { get: jest.fn(), post: jest.fn() }; + endpoints[path] = api; + return api; + }, + }; +}); + +import { getAllShifts, getShift, getShiftDay, getTodaysShifts, signupForShiftDay } from '../shifts'; + +const mockEndpoints: Record = jest.requireMock('../../common/client').__endpoints; + +const endpoint = (path: string) => { + const api = mockEndpoints[path]; + if (!api) throw new Error(`endpoint ${path} was not created`); + return api; +}; + +describe('shifts api', () => { + beforeEach(() => { + Object.values(mockEndpoints).forEach((api) => { + api.get.mockReset().mockResolvedValue({ data: { Data: [] } }); + api.post.mockReset().mockResolvedValue({ data: { Id: '' } }); + }); + }); + + it('only creates routes that exist on the v4 Shifts controller', () => { + expect(Object.keys(mockEndpoints).sort()).toEqual(['/Shifts/GetShift', '/Shifts/GetShiftDay', '/Shifts/GetShifts', '/Shifts/GetTodaysShifts', '/Shifts/SignupForShiftDay']); + }); + + it('reads all shifts from GetShifts', async () => { + await getAllShifts(); + + expect(endpoint('/Shifts/GetShifts').get).toHaveBeenCalledWith(); + }); + + it('sends the shift id as id', async () => { + await getShift('7'); + + expect(endpoint('/Shifts/GetShift').get).toHaveBeenCalledWith({ id: '7' }); + }); + + it('sends the shift day id as id', async () => { + await getShiftDay('12'); + + expect(endpoint('/Shifts/GetShiftDay').get).toHaveBeenCalledWith({ id: '12' }); + }); + + it("reads today's shift days", async () => { + await getTodaysShifts(); + + expect(endpoint('/Shifts/GetTodaysShifts').get).toHaveBeenCalledWith(); + }); + + it('posts ShiftDaySignupInput with numeric ids', async () => { + endpoint('/Shifts/SignupForShiftDay').post.mockResolvedValue({ data: { Id: '99' } }); + + await expect(signupForShiftDay('12', '3')).resolves.toEqual({ Id: '99' }); + expect(endpoint('/Shifts/SignupForShiftDay').post).toHaveBeenCalledWith({ ShiftDayId: 12, GroupId: 3 }); + }); +}); diff --git a/src/api/shifts/shifts.ts b/src/api/shifts/shifts.ts index d67081b4..f1004981 100644 --- a/src/api/shifts/shifts.ts +++ b/src/api/shifts/shifts.ts @@ -7,15 +7,21 @@ import { type ShiftDayResult } from '@/models/v4/shifts/shiftDayResult'; import { type ShiftDaysResult } from '@/models/v4/shifts/shiftDaysResult'; import { type ShiftResult } from '@/models/v4/shifts/shiftResult'; import { type ShiftsResult } from '@/models/v4/shifts/shiftsResult'; +import { type SignupShiftDayResult } from '@/models/v4/shifts/signupShiftDayResult'; import { createApiEndpoint } from '../common/client'; -const getAllShiftsApi = createApiEndpoint('/Shifts/GetAllShifts'); +const getAllShiftsApi = createApiEndpoint('/Shifts/GetShifts'); const getShiftApi = createApiEndpoint('/Shifts/GetShift'); const getShiftDayApi = createApiEndpoint('/Shifts/GetShiftDay'); const getTodaysShiftsApi = createApiEndpoint('/Shifts/GetTodaysShifts'); const signupForShiftDayApi = createApiEndpoint('/Shifts/SignupForShiftDay'); +/** + * Result ids are strings, but the POST inputs bind numeric ids. + */ +const toNumericId = (id: string | number): number => (typeof id === 'number' ? id : Number.parseInt(id, 10)); + /** * Get all shifts */ @@ -28,7 +34,7 @@ export const getAllShifts = async (): Promise => { * Get a specific shift by ID */ export const getShift = async (shiftId: string): Promise => { - const response = await getShiftApi.get({ shiftId }); + const response = await getShiftApi.get({ id: shiftId }); return response.data; }; @@ -36,7 +42,7 @@ export const getShift = async (shiftId: string): Promise => { * Get shift day information */ export const getShiftDay = async (shiftDayId: string): Promise => { - const response = await getShiftDayApi.get({ shiftDayId }); + const response = await getShiftDayApi.get({ id: shiftDayId }); return response.data; }; @@ -49,8 +55,12 @@ export const getTodaysShifts = async (): Promise => { }; /** - * Sign up for a shift day + * Sign the current user up for a shift day; the server requires the group (team) the signup is under. */ -export const signupForShiftDay = async (shiftDayId: string, userId?: string): Promise => { - await signupForShiftDayApi.post({ shiftDayId, userId }); +export const signupForShiftDay = async (shiftDayId: string, groupId: string): Promise => { + const response = await signupForShiftDayApi.post({ + ShiftDayId: toNumericId(shiftDayId), + GroupId: toNumericId(groupId), + }); + return response.data; }; diff --git a/src/app/login/index.tsx b/src/app/login/index.tsx index 4a8d3f8b..aecdf42f 100644 --- a/src/app/login/index.tsx +++ b/src/app/login/index.tsx @@ -79,10 +79,7 @@ export default function Login() { }, [status, error]); const onSubmit: LoginFormProps['onSubmit'] = async (data) => { - logger.info({ - message: 'Starting Login (button press)', - context: { username: data.username }, - }); + logger.info({ message: 'Starting Login (button press)' }); setPendingCredentials({ username: data.username, password: data.password }); setOtpDismissed(false); try { diff --git a/src/lib/__tests__/logging.test.ts b/src/lib/__tests__/logging.test.ts deleted file mode 100644 index 3fd023b3..00000000 --- a/src/lib/__tests__/logging.test.ts +++ /dev/null @@ -1,158 +0,0 @@ -/** - * Tests for logging service with Sentry integration - */ - -// Mock Sentry before importing logging -jest.mock('@sentry/react-native', () => ({ - addBreadcrumb: jest.fn(), - captureException: jest.fn(), - captureMessage: jest.fn(), - setContext: jest.fn(), - setUser: jest.fn(), - withScope: jest.fn((callback) => { - const mockScope = { - setExtras: jest.fn(), - }; - callback(mockScope); - }), -})); - -// Mock react-native-logs -jest.mock('react-native-logs', () => ({ - consoleTransport: jest.fn(), - logger: { - createLogger: () => ({ - debug: jest.fn(), - info: jest.fn(), - warn: jest.fn(), - error: jest.fn(), - }), - }, -})); - -import * as Sentry from '@sentry/react-native'; -import { logger } from '../logging'; - -describe('LogService', () => { - beforeEach(() => { - jest.clearAllMocks(); - }); - - describe('basic logging', () => { - it('should log debug messages', () => { - logger.debug({ message: 'Debug message', context: { key: 'value' } }); - // Verify breadcrumb was added to Sentry - expect(Sentry.addBreadcrumb).toHaveBeenCalledWith( - expect.objectContaining({ - category: 'log', - message: 'Debug message', - level: 'debug', - }) - ); - }); - - it('should log info messages', () => { - logger.info({ message: 'Info message', context: { key: 'value' } }); - expect(Sentry.addBreadcrumb).toHaveBeenCalledWith( - expect.objectContaining({ - category: 'log', - message: 'Info message', - level: 'info', - }) - ); - }); - - it('should log warn messages', () => { - logger.warn({ message: 'Warning message' }); - expect(Sentry.addBreadcrumb).toHaveBeenCalledWith( - expect.objectContaining({ - category: 'log', - message: 'Warning message', - level: 'warning', - }) - ); - }); - - it('should log error messages and capture in Sentry', () => { - logger.error({ message: 'Error message', context: { error: 'some error' } }); - - // Verify breadcrumb was added - expect(Sentry.addBreadcrumb).toHaveBeenCalledWith( - expect.objectContaining({ - category: 'log', - message: 'Error message', - level: 'error', - }) - ); - - // Verify error was captured in Sentry - expect(Sentry.withScope).toHaveBeenCalled(); - }); - }); - - describe('error capturing', () => { - it('should capture Error objects in Sentry', () => { - const error = new Error('Test error'); - logger.error({ message: 'An error occurred', context: { error } }); - - expect(Sentry.withScope).toHaveBeenCalled(); - expect(Sentry.captureException).toHaveBeenCalledWith(error); - }); - - it('should capture string errors as new Error in Sentry', () => { - logger.error({ message: 'An error occurred', context: { error: 'string error' } }); - - expect(Sentry.withScope).toHaveBeenCalled(); - expect(Sentry.captureException).toHaveBeenCalled(); - }); - - it('should capture errors without error context as message', () => { - logger.error({ message: 'An error without context' }); - - expect(Sentry.withScope).toHaveBeenCalled(); - expect(Sentry.captureMessage).toHaveBeenCalledWith('An error without context', 'error'); - }); - }); - - describe('captureException', () => { - it('should capture exceptions directly', () => { - const error = new Error('Direct exception'); - logger.captureException(error, { userId: '123' }); - - expect(Sentry.captureException).toHaveBeenCalledWith(error); - }); - }); - - describe('context management', () => { - it('should set global context', () => { - logger.setGlobalContext({ userId: '123', environment: 'test' }); - expect(Sentry.setContext).toHaveBeenCalledWith('app_context', expect.objectContaining({ userId: '123' })); - }); - }); - - describe('user management', () => { - it('should set user in Sentry', () => { - logger.setUser({ id: '123', email: 'test@example.com' }); - expect(Sentry.setUser).toHaveBeenCalledWith({ id: '123', email: 'test@example.com' }); - }); - - it('should clear user in Sentry', () => { - logger.setUser(null); - expect(Sentry.setUser).toHaveBeenCalledWith(null); - }); - }); - - describe('Sentry toggle', () => { - it('should allow disabling Sentry integration', () => { - logger.setSentryEnabled(false); - logger.error({ message: 'This should not go to Sentry' }); - - // Should not have called Sentry methods after disabling - // Note: The first call might have happened before disabling - const callsAfterDisable = (Sentry.addBreadcrumb as jest.Mock).mock.calls.filter( - (call) => call[0]?.message === 'This should not go to Sentry' - ); - expect(callsAfterDisable.length).toBe(0); - }); - }); -}); diff --git a/src/lib/auth/api.tsx b/src/lib/auth/api.tsx index 2542e2ef..c21ebbf1 100644 --- a/src/lib/auth/api.tsx +++ b/src/lib/auth/api.tsx @@ -184,7 +184,7 @@ export const loginRequest = async (credentials: LoginCredentials): Promise('/connect/token', data); @@ -197,7 +197,6 @@ export const loginRequest = async (credentials: LoginCredentials): Promise ({ + captureException: (...args: unknown[]) => mockCaptureException(...args), + captureMessage: (...args: unknown[]) => mockCaptureMessage(...args), +})); + +// Replace the console transport so exercising the real (non-Jest) code path +// doesn't print every test's log line. +const mockTransportLog = jest.fn(); +jest.mock('react-native-logs', () => ({ + consoleTransport: jest.fn(), + logger: { + createLogger: () => ({ + debug: mockTransportLog, + info: mockTransportLog, + warn: mockTransportLog, + error: mockTransportLog, + }), + }, +})); + +jest.mock('react-native', () => ({ + Platform: { OS: 'ios' }, +})); + +import type { Logger } from '../types'; + +let logger: Logger; +let sanitizeLogContext: (context: Record | undefined) => Record; + +beforeAll(() => { + const workerId = process.env.JEST_WORKER_ID; + delete process.env.JEST_WORKER_ID; + jest.isolateModules(() => { + const loggingModule = require('../index'); + logger = loggingModule.logger; + sanitizeLogContext = loggingModule.sanitizeLogContext; + }); + process.env.JEST_WORKER_ID = workerId; +}); + +beforeEach(() => { + jest.clearAllMocks(); +}); + +describe('LogService#error Sentry reporting', () => { + it('reports an Error as an exception so Sentry groups by the real stack', () => { + const error = new Error('Config fetch failed'); + error.stack = 'Error: Config fetch failed\n at fetchConfig (core-store.ts:307:31)'; + + logger.error({ message: 'Failed to initialize app', context: { error } }); + + expect(mockCaptureMessage).not.toHaveBeenCalled(); + expect(mockCaptureException).toHaveBeenCalledTimes(1); + + const [captured] = mockCaptureException.mock.calls[0]; + expect(captured).toBeInstanceOf(Error); + expect(captured.name).toBe('Error'); + expect(captured.message).toBe('Config fetch failed'); + expect(captured.stack).toBe(error.stack); + }); + + it('preserves a custom error name so the issue title is the real exception type', () => { + const error = new Error('Network Error'); + error.name = 'AxiosError'; + + logger.error({ message: 'Failed to send location to API', context: { error } }); + + expect(mockCaptureException.mock.calls[0][0].name).toBe('AxiosError'); + }); + + it('does not forward the original error object to Sentry', () => { + // captureException serializes an error's own enumerable properties into the + // event; an AxiosError carries config.data with urlencoded credentials. + const axiosError = Object.assign(new Error('Request failed with status code 400'), { + name: 'AxiosError', + isAxiosError: true, + code: 'ERR_BAD_REQUEST', + config: { method: 'post', url: '/Connect/token', data: 'password=hunter2&grant_type=password' }, + response: { status: 400 }, + }); + + logger.error({ message: 'Failed to send location to API', context: { error: axiosError } }); + + const [captured] = mockCaptureException.mock.calls[0]; + expect(captured).not.toBe(axiosError); + expect(captured).not.toHaveProperty('config'); + expect(captured).not.toHaveProperty('response'); + expect(JSON.stringify(captured)).not.toContain('hunter2'); + }); + + it('still attaches the sanitized context as extra data', () => { + const axiosError = Object.assign(new Error('Request failed with status code 400'), { + name: 'AxiosError', + isAxiosError: true, + code: 'ERR_BAD_REQUEST', + config: { method: 'post', url: '/UnitLocation/SetUnitLocation?key=abc' }, + response: { status: 400 }, + }); + + logger.error({ message: 'Failed to send location to API', context: { error: axiosError, unitId: 'unit-123' } }); + + const [, options] = mockCaptureException.mock.calls[0]; + expect(options.extra).toEqual( + expect.objectContaining({ + message: 'Failed to send location to API', + unitId: 'unit-123', + error: expect.objectContaining({ + name: 'AxiosError', + code: 'ERR_BAD_REQUEST', + status: 400, + url: '/UnitLocation/SetUnitLocation', + isAxiosError: true, + }), + }) + ); + }); + + it('falls back to captureMessage when the context holds no Error', () => { + logger.error({ message: 'Token refresh rejected by server', context: { error: 'invalid_grant' } }); + + expect(mockCaptureException).not.toHaveBeenCalled(); + expect(mockCaptureMessage).toHaveBeenCalledWith('Token refresh rejected by server', { level: 'error', extra: { error: 'invalid_grant' } }); + }); + + it('falls back to captureMessage when there is no context at all', () => { + logger.error({ message: 'Something went wrong' }); + + expect(mockCaptureException).not.toHaveBeenCalled(); + expect(mockCaptureMessage).toHaveBeenCalledWith('Something went wrong', { level: 'error', extra: {} }); + }); + + it('includes operation and trace_id in the reported extra data', () => { + logger.error({ message: 'Request failed', operation: 'token_refresh', trace_id: 'abc123' }); + + expect(mockCaptureMessage).toHaveBeenCalledWith('Request failed', { level: 'error', extra: { operation: 'token_refresh', trace_id: 'abc123' } }); + }); + + it('does not report warn, info or debug to Sentry', () => { + logger.warn({ message: 'Failed to send location to API', context: { error: new Error('Network Error') } }); + logger.info({ message: 'Location successfully sent to API' }); + logger.debug({ message: 'Skipping location API call' }); + + expect(mockCaptureException).not.toHaveBeenCalled(); + expect(mockCaptureMessage).not.toHaveBeenCalled(); + }); +}); + +describe('LogService console transport', () => { + it('redacts the context before it reaches the device console', () => { + const axiosError = Object.assign(new Error('Request failed with status code 400'), { + name: 'AxiosError', + isAxiosError: true, + config: { method: 'post', url: '/Connect/token', data: 'password=hunter2&grant_type=password' }, + response: { status: 400 }, + }); + + logger.warn({ message: 'Sign-in failed', operation: 'login', context: { error: axiosError, accessToken: 'abc', unitId: 'unit-123' } }); + + expect(mockTransportLog).toHaveBeenCalledTimes(1); + const [message, context] = mockTransportLog.mock.calls[0]; + expect(message).toBe('Sign-in failed'); + expect(context).toEqual( + expect.objectContaining({ + accessToken: '[REDACTED]', + unitId: 'unit-123', + operation: 'login', + error: expect.objectContaining({ name: 'AxiosError', status: 400, url: '/Connect/token' }), + }) + ); + expect(JSON.stringify(context)).not.toContain('hunter2'); + }); +}); + +describe('sanitizeLogContext', () => { + it('redacts sensitive keys', () => { + expect(sanitizeLogContext({ accessToken: 'abc', refresh_token: 'def', unitId: 'unit-123' })).toEqual({ + accessToken: '[REDACTED]', + refresh_token: '[REDACTED]', + unitId: 'unit-123', + }); + }); + + it('redacts usernames and a bare auth key, but not words that merely start with "auth"', () => { + expect(sanitizeLogContext({ username: 'pat@example.com', auth: 'secret', authority: 'https://idp.example.com', author: 'Pat' })).toEqual({ + username: '[REDACTED]', + auth: '[REDACTED]', + authority: 'https://idp.example.com', + author: 'Pat', + }); + }); + + it('reduces an axios error to a summary without the request body', () => { + const axiosError = Object.assign(new Error('Request failed with status code 401'), { + name: 'AxiosError', + isAxiosError: true, + code: 'ERR_BAD_REQUEST', + config: { method: 'post', url: '/Connect/token?x=1', baseURL: 'https://api.resgrid.com/api/v4', data: 'password=hunter2' }, + response: { status: 401 }, + }); + + expect(sanitizeLogContext({ error: axiosError })).toEqual({ + error: { + name: 'AxiosError', + message: 'Request failed with status code 401', + code: 'ERR_BAD_REQUEST', + status: 401, + method: 'post', + url: '/Connect/token', + baseURL: 'https://api.resgrid.com/api/v4', + isAxiosError: true, + }, + }); + }); + + it('expands a plain Error, whose properties are non-enumerable', () => { + const error = new Error('boom'); + error.stack = 'Error: boom\n at somewhere'; + + expect(sanitizeLogContext({ error })).toEqual({ + error: { name: 'Error', message: 'boom', stack: 'Error: boom\n at somewhere' }, + }); + }); +}); diff --git a/src/lib/logging/index.tsx b/src/lib/logging/index.tsx index d75a969f..1b6f22f5 100644 --- a/src/lib/logging/index.tsx +++ b/src/lib/logging/index.tsx @@ -2,7 +2,102 @@ import * as Sentry from '@sentry/react-native'; import { Platform } from 'react-native'; import { consoleTransport, logger as rnLogger } from 'react-native-logs'; -import type { LogEntry, Logger, LogLevel } from './types'; +import type { LogContext, LogEntry, Logger, LogLevel } from './types'; + +// Substring match: any context key CONTAINING one of these fragments is redacted. +// This catches camelCase/snake_case variants (accessToken, refresh_token, id_token, ...) +// without having to enumerate every possible spelling. +const SENSITIVE_KEY_PARTS = ['token', 'password', 'passwd', 'secret', 'apikey', 'api_key', 'authorization', 'cred', 'email', 'ssn', 'saml', 'cookie', 'session', 'username', 'grant']; + +// Exact-match keys too short to match as fragments without redacting unrelated words (author, authority). +const SENSITIVE_EXACT_KEYS = new Set(['auth']); + +const isSensitiveKey = (key: string): boolean => { + const lower = key.toLowerCase(); + return SENSITIVE_EXACT_KEYS.has(lower) || SENSITIVE_KEY_PARTS.some((part) => lower.includes(part)); +}; + +// Strip query string and hash — URLs can carry credentials (e.g. access_token params). +const sanitizeUrl = (url: unknown): string | undefined => { + if (typeof url !== 'string' || url.length === 0) return undefined; + return url.split('?')[0].split('#')[0]; +}; + +interface AxiosErrorLike { + isAxiosError?: boolean; + name?: string; + message?: string; + code?: string; + config?: { method?: string; url?: string; baseURL?: string }; + response?: { status?: number }; +} + +// Axios errors carry the full request (incl. urlencoded password/token bodies in +// config.data) and response objects with circular refs. Never forward them raw — +// reduce to a safe summary instead. +const isAxiosErrorLike = (value: unknown): value is AxiosErrorLike => typeof value === 'object' && value !== null && ((value as AxiosErrorLike).isAxiosError === true || ('config' in value && 'message' in value)); + +const summarizeAxiosError = (error: AxiosErrorLike): Record => ({ + name: error.name, + message: error.message, + code: error.code, + status: error.response?.status, + method: error.config?.method, + url: sanitizeUrl(error.config?.url), + baseURL: sanitizeUrl(error.config?.baseURL), + isAxiosError: true, +}); + +const sanitizeValue = (key: string, value: unknown, depth: number): unknown => { + if (isSensitiveKey(key)) return '[REDACTED]'; + if (isAxiosErrorLike(value)) return summarizeAxiosError(value); + // Error instances have non-enumerable props; Object.keys() would yield {}. + 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)) { + return sanitizeObject(value as Record, depth - 1); + } + if (typeof value === 'function' || typeof value === 'symbol') return String(value); + return value; +}; + +const sanitizeObject = (obj: Record, depth: number): Record => { + const result: Record = {}; + for (const key of Object.keys(obj)) { + result[key] = sanitizeValue(key, obj[key], depth); + } + return result; +}; + +export const sanitizeLogContext = (context: LogContext | undefined): LogContext => { + if (!context) return {}; + return sanitizeObject(context as Record, 2); +}; + +/** + * Rebuilds a bare Error carrying only name/message/stack so Sentry groups the + * issue by the real exception and shows the originating frames. + * + * The original is never handed to Sentry directly: `captureException` serializes + * an error's own enumerable properties into the event, which for an AxiosError + * means `config.data` — the urlencoded password/token bodies `summarizeAxiosError` + * exists to strip. Copying three fields keeps the grouping win without the leak. + */ +const toCaptureableError = (value: unknown): Error | null => { + if (!(value instanceof Error)) return null; + const stripped = new Error(value.message); + stripped.name = value.name; + stripped.stack = value.stack; + return stripped; +}; + +// On web, async: true wraps every log call in setTimeout which — combined with +// Sentry's setTimeout instrumentation — creates unbounded memory growth. +// Setting async: false on web prevents this. Severity stays 'debug' in dev +// on all platforms so console output is visible for debugging. +const isWeb = Platform.OS === 'web'; +const isJest = typeof process !== 'undefined' && typeof process.env?.JEST_WORKER_ID !== 'undefined'; const config = { levels: { @@ -21,29 +116,21 @@ const config = { error: 'redBright', }, }, - async: true, + async: !isWeb && !isJest, dateFormat: 'time', printLevel: true, printDate: true, fixedExtLvlLength: false, - enabled: true, + enabled: !isJest, }; -/** - * Maps log levels to Sentry severity levels - */ -const sentryLevelMap: Record = { - debug: 'debug', - info: 'info', - warn: 'warning', - error: 'error', -}; +const LEVEL_VALUES: Record = { debug: 0, info: 1, warn: 2, error: 3 }; +const MIN_SEVERITY: number = LEVEL_VALUES[(config.severity as LogLevel) ?? 'warn'] ?? 2; class LogService { private static instance: LogService; private logger: any; private globalContext: Record = {}; - private sentryEnabled: boolean = true; private constructor() { this.logger = rnLogger.createLogger(config as any); @@ -56,95 +143,22 @@ class LogService { return LogService.instance; } - /** - * Enable or disable Sentry integration for logging - */ - public setSentryEnabled(enabled: boolean): void { - this.sentryEnabled = enabled; - } - - /** - * Add a breadcrumb to Sentry for tracking user actions and app state - */ - private addSentryBreadcrumb(level: LogLevel, message: string, context: Record): void { - if (!this.sentryEnabled) return; - - try { - Sentry.addBreadcrumb({ - category: 'log', - message, - level: sentryLevelMap[level], - data: { - ...this.globalContext, - ...context, - platform: Platform.OS, - }, - }); - } catch (e) { - // Silently fail - don't want Sentry errors to break logging - if (__DEV__) { - console.warn('Failed to add Sentry breadcrumb:', e); - } - } - } - - /** - * Capture an error in Sentry with additional context - */ - private captureErrorInSentry(message: string, context: Record): void { - if (!this.sentryEnabled) return; - - try { - Sentry.withScope((scope) => { - // Set additional context - scope.setExtras({ - ...this.globalContext, - ...context, - platform: Platform.OS, - }); - - // If context contains an error object, capture it - const errorObj = context.error; - if (errorObj instanceof Error) { - Sentry.captureException(errorObj); - } else if (typeof errorObj === 'string') { - // Create an error from the string message - Sentry.captureException(new Error(`${message}: ${errorObj}`)); - } else { - // Capture as a message with error level - Sentry.captureMessage(message, 'error'); - } - }); - } catch (e) { - // Silently fail - don't want Sentry errors to break logging - if (__DEV__) { - console.warn('Failed to capture error in Sentry:', e); - } - } - } - - private log(level: LogLevel, { message, context = {} }: LogEntry): void { + private log(level: LogLevel, { message, operation, trace_id, context = {} }: LogEntry): void { + // Bail before allocating the context object on hot paths (SignalR messages, + // GPS fixes) when the level would be filtered out anyway. + if (isJest || LEVEL_VALUES[level] < MIN_SEVERITY) return; + // Redacted here, before the console transport, not only on the Sentry path: release builds still print warn and + // error to logcat and the Xcode console, and a raw AxiosError serializes its urlencoded password body there. this.logger[level](message, { - ...this.globalContext, - ...context, + ...sanitizeLogContext({ ...this.globalContext, ...context }), + ...(operation ? { operation } : {}), + ...(trace_id ? { trace_id } : {}), timestamp: new Date().toISOString(), }); - - // Add breadcrumb to Sentry for all log levels - this.addSentryBreadcrumb(level, message, context); } public setGlobalContext(context: Record): void { this.globalContext = { ...this.globalContext, ...context }; - - // Also set this context in Sentry - if (this.sentryEnabled) { - try { - Sentry.setContext('app_context', this.globalContext); - } catch (e) { - // Silently fail - } - } } public clearGlobalContext(): void { @@ -165,35 +179,23 @@ class LogService { public error(entry: LogEntry): void { this.log('error', entry); - - // Capture errors in Sentry (in addition to breadcrumb) - this.captureErrorInSentry(entry.message, entry.context || {}); - } - - /** - * Capture an exception directly in Sentry - * Use this when you have an actual Error object to report - */ - public captureException(error: Error, context?: Record): void { - this.error({ - message: error.message, - context: { - ...context, - error, - stack: error.stack, - }, - }); - } - - /** - * Set user information for Sentry tracking - */ - public setUser(user: { id?: string; email?: string; username?: string } | null): void { - if (this.sentryEnabled) { - try { - Sentry.setUser(user); - } catch (e) { - // Silently fail + if (!isJest) { + // Read the error BEFORE sanitizing: sanitizeLogContext replaces every + // Error with a plain object, so testing the sanitized value for + // `instanceof Error` never matched and every logger.error was reported as + // a message fingerprinted on LogService#error — no exception type and no + // originating stack on any issue in the project. + const rawError = entry.context?.error; + const sanitized = sanitizeLogContext({ + ...entry.context, + ...(entry.operation ? { operation: entry.operation } : {}), + ...(entry.trace_id ? { trace_id: entry.trace_id } : {}), + }); + const captureable = toCaptureableError(rawError); + if (captureable) { + Sentry.captureException(captureable, { extra: { message: entry.message, ...sanitized } }); + } else { + Sentry.captureMessage(entry.message, { level: 'error', extra: sanitized }); } } } diff --git a/src/lib/logging/types.tsx b/src/lib/logging/types.tsx index 9066d20d..3a2a8ce3 100644 --- a/src/lib/logging/types.tsx +++ b/src/lib/logging/types.tsx @@ -6,6 +6,8 @@ export interface LogContext { export interface LogEntry { message: string; + operation?: string; + trace_id?: string; context?: LogContext; } diff --git a/src/stores/auth/store.tsx b/src/stores/auth/store.tsx index 78cbaf54..9b722212 100644 --- a/src/stores/auth/store.tsx +++ b/src/stores/auth/store.tsx @@ -128,7 +128,7 @@ const useAuthStore = create()( set({ status: 'loading', error: null, loginFailure: null }); logger.info({ message: 'Login: Calling loginRequest API', - context: { username: credentials.username, platform: Platform.OS }, + context: { platform: Platform.OS }, }); const response = await loginRequest(credentials); diff --git a/src/stores/shifts/__tests__/store.test.ts b/src/stores/shifts/__tests__/store.test.ts index df873396..cbd952a1 100644 --- a/src/stores/shifts/__tests__/store.test.ts +++ b/src/stores/shifts/__tests__/store.test.ts @@ -265,17 +265,17 @@ describe('useShiftsStore', () => { describe('signup functionality', () => { it('should sign up for shift successfully', async () => { - mockedShiftsApi.signupForShiftDay.mockResolvedValue(undefined); + mockedShiftsApi.signupForShiftDay.mockResolvedValue({ Id: 'signup1' } as Awaited>); mockedShiftsApi.getTodaysShifts.mockResolvedValue(mockTodaysShiftsResult); mockedShiftsApi.getShiftDay.mockResolvedValue(mockShiftDayResult); const { result } = renderHook(() => useShiftsStore()); await act(async () => { - await result.current.signupForShift('day1', 'user1'); + await result.current.signupForShift('day1', 'group1'); }); - expect(mockedShiftsApi.signupForShiftDay).toHaveBeenCalledWith('day1', 'user1'); + expect(mockedShiftsApi.signupForShiftDay).toHaveBeenCalledWith('day1', 'group1'); expect(mockedShiftsApi.getTodaysShifts).toHaveBeenCalled(); expect(mockedShiftsApi.getShiftDay).toHaveBeenCalledWith('day1'); expect(result.current.isSignupLoading).toBe(false); @@ -289,7 +289,7 @@ describe('useShiftsStore', () => { const { result } = renderHook(() => useShiftsStore()); await act(async () => { - await result.current.signupForShift('day1', 'user1'); + await result.current.signupForShift('day1', 'group1'); }); expect(result.current.signupError).toBe('Failed to sign up for shift'); diff --git a/src/stores/shifts/store.ts b/src/stores/shifts/store.ts index 2c6d1ef8..909afc57 100644 --- a/src/stores/shifts/store.ts +++ b/src/stores/shifts/store.ts @@ -44,7 +44,7 @@ interface ShiftsState { //fetchShiftDaysForDateRange: (shiftId: string, startDate: string, endDate: string) => Promise; // Actions - User interactions - signupForShift: (shiftDayId: string, userId: string) => Promise; + signupForShift: (shiftDayId: string, groupId: string) => Promise; //withdrawFromShift: (shiftDayId: string, userId: string) => Promise; // Actions - UI state @@ -210,10 +210,10 @@ export const useShiftsStore = create((set, get) => ({ }, // User interaction actions - signupForShift: async (shiftDayId: string, userId: string) => { + signupForShift: async (shiftDayId: string, groupId: string) => { set({ isSignupLoading: true, signupError: null }); try { - await signupForShiftDay(shiftDayId, userId); + await signupForShiftDay(shiftDayId, groupId); // Refresh data after successful signup await Promise.all([get().fetchTodaysShifts(), get().fetchShiftDay(shiftDayId)]); @@ -221,7 +221,7 @@ export const useShiftsStore = create((set, get) => ({ set({ isSignupLoading: false }); logger.info({ message: 'Successfully signed up for shift', - context: { shiftDayId, userId }, + context: { shiftDayId, groupId }, }); } catch (error) { set({ @@ -230,7 +230,7 @@ export const useShiftsStore = create((set, get) => ({ }); logger.error({ message: 'Failed to sign up for shift', - context: { error, shiftDayId, userId }, + context: { error, shiftDayId, groupId }, }); } }, From 25201a1caa5924ca1873aeddd19b18eb7fa1617e Mon Sep 17 00:00:00 2001 From: Shawn Jackson Date: Sat, 31 Oct 2026 19:19:56 -0700 Subject: [PATCH 2/2] RD-T49 PR #156 Fixes --- .../client-refresh.integration.test.ts | 17 +++- src/lib/auth/__tests__/token-refresh.test.ts | 82 +++++++++++++++++-- src/lib/auth/token-refresh.ts | 41 ++++++++-- src/lib/logging/__tests__/index.test.ts | 13 +++ src/lib/logging/index.tsx | 6 +- 5 files changed, 142 insertions(+), 17 deletions(-) diff --git a/src/api/common/__tests__/client-refresh.integration.test.ts b/src/api/common/__tests__/client-refresh.integration.test.ts index d0afe410..cf2a7c7e 100644 --- a/src/api/common/__tests__/client-refresh.integration.test.ts +++ b/src/api/common/__tests__/client-refresh.integration.test.ts @@ -69,7 +69,7 @@ describe('API requests using the shared token refresh engine', () => { it('ends a rejected restored session once and stops all protected requests', async () => { useAuthStore.setState({ refreshTokenExpiresOn: String(Date.now() - 1000) }); - (refreshTokenRequest as jest.Mock).mockRejectedValue(new AxiosError('Rejected', 'ERR_BAD_REQUEST', undefined, undefined, { status: 400 } as AxiosResponse)); + (refreshTokenRequest as jest.Mock).mockRejectedValue(new AxiosError('Rejected', 'ERR_BAD_REQUEST', undefined, undefined, { status: 400, data: { error: 'invalid_grant' } } as AxiosResponse)); const results = await Promise.allSettled([api.get('/Calls/GetActiveCalls'), api.get('/Units/GetAllUnitsInfos')]); @@ -81,4 +81,19 @@ describe('API requests using the shared token refresh engine', () => { } expect(jest.getTimerCount()).toBe(0); }); + + it('keeps a restored session and retries when the token endpoint answers a 400 that is not invalid_grant', async () => { + useAuthStore.setState({ refreshTokenExpiresOn: String(Date.now() - 1000) }); + (refreshTokenRequest as jest.Mock).mockRejectedValue(new AxiosError('Bad Request', 'ERR_BAD_REQUEST', undefined, undefined, { status: 400, data: { error: 'invalid_request' } } as AxiosResponse)); + + const results = await Promise.allSettled([api.get('/Calls/GetActiveCalls'), api.get('/Units/GetAllUnitsInfos')]); + + expect(refreshTokenRequest).toHaveBeenCalledTimes(1); + expect(adapter).not.toHaveBeenCalled(); + expect(useAuthStore.getState()).toMatchObject({ status: 'signedIn', accessToken: 'old-access', refreshToken: 'old-refresh' }); + for (const result of results) { + expect(result).toMatchObject({ status: 'rejected' }); + } + expect(jest.getTimerCount()).toBe(1); + }); }); diff --git a/src/lib/auth/__tests__/token-refresh.test.ts b/src/lib/auth/__tests__/token-refresh.test.ts index 670c3011..10a4ca0e 100644 --- a/src/lib/auth/__tests__/token-refresh.test.ts +++ b/src/lib/auth/__tests__/token-refresh.test.ts @@ -3,7 +3,7 @@ import { AxiosError, type AxiosResponse } from 'axios'; import { logger } from '@/lib/logging'; import { refreshTokenRequest } from '../api'; -import { cancelScheduledTokenRefresh, initTokenRefresh, performTokenRefresh, scheduleTokenRefresh, transientRetryDelayMs } from '../token-refresh'; +import { cancelScheduledTokenRefresh, initTokenRefresh, isRefreshCredentialRejection, performTokenRefresh, scheduleTokenRefresh, transientRetryDelayMs } from '../token-refresh'; jest.mock('@/lib/logging', () => ({ logger: { info: jest.fn(), warn: jest.fn(), error: jest.fn() }, @@ -16,6 +16,43 @@ jest.mock('../api', () => ({ const mockRefreshTokenRequest = refreshTokenRequest as jest.MockedFunction; const mockLogger = logger as jest.Mocked; +const axiosErrorWithStatus = (status: number, data?: unknown): AxiosError => new AxiosError('Request failed with status code ' + status, 'ERR_BAD_REQUEST', undefined, undefined, { status, data } as AxiosResponse); + +// How the token endpoint (OpenIddict) answers a refresh token it no longer honours. +const INVALID_GRANT = { error: 'invalid_grant', error_description: 'The refresh token is no longer valid.' }; + +describe('isRefreshCredentialRejection', () => { + it.each([400, 401])('returns true for a %i invalid_grant from the token endpoint', (status) => { + expect(isRefreshCredentialRejection(axiosErrorWithStatus(status, INVALID_GRANT))).toBe(true); + expect(isRefreshCredentialRejection(axiosErrorWithStatus(status, { 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, {}))).toBe(false); + expect(isRefreshCredentialRejection(axiosErrorWithStatus(status, 'Bad Request'))).toBe(false); + }); + + it.each([403, 404, 408, 429, 500, 502, 503])('returns false for invalid_grant on a %i', (status) => { + expect(isRefreshCredentialRejection(axiosErrorWithStatus(status, { error: 'invalid_grant' }))).toBe(false); + }); + + it('returns false for a network error and for non-Axios errors', () => { + expect(isRefreshCredentialRejection(new AxiosError('Network Error', 'ERR_NETWORK'))).toBe(false); + expect(isRefreshCredentialRejection(new Error('boom'))).toBe(false); + expect(isRefreshCredentialRejection({ response: { status: 400, data: { error: 'invalid_grant' } } })).toBe(false); + expect(isRefreshCredentialRejection(null)).toBe(false); + }); +}); + describe('scheduleTokenRefresh', () => { let setTimeoutSpy: jest.SpyInstance; @@ -66,8 +103,6 @@ describe('performTokenRefresh failure reporting', () => { onRefreshFailed: jest.fn(), }; - const axiosErrorWithStatus = (status: number): AxiosError => new AxiosError('Request failed with status code ' + status, 'ERR_BAD_REQUEST', undefined, undefined, { status } as AxiosResponse); - beforeEach(() => { jest.clearAllMocks(); handlers.getRefreshToken.mockReturnValue('refresh-token'); @@ -81,8 +116,8 @@ describe('performTokenRefresh failure reporting', () => { cancelScheduledTokenRefresh(); }); - it.each([400, 401])('treats a %i from the token endpoint as an expired session, not an error', async (status) => { - mockRefreshTokenRequest.mockRejectedValue(axiosErrorWithStatus(status)); + it.each([400, 401])('treats a %i invalid_grant from the token endpoint as an expired session, not an error', async (status) => { + mockRefreshTokenRequest.mockRejectedValue(axiosErrorWithStatus(status, INVALID_GRANT)); await expect(performTokenRefresh()).resolves.toBe(false); @@ -91,6 +126,41 @@ describe('performTokenRefresh failure reporting', () => { expect(handlers.onRefreshFailed).toHaveBeenCalledTimes(1); }); + it.each([ + ['a 400 invalid_request', 400, { error: 'invalid_request' }], + ['a 401 invalid_client', 401, { error: 'invalid_client' }], + ['a 400 with no body', 400, undefined], + ['a 401 with no body', 401, undefined], + ['a 400 proxy error page', 400, 'Bad Request'], + ])('keeps the session and schedules a retry for %s, which says nothing about the refresh token', async (_label, status, data) => { + jest.useFakeTimers(); + const setTimeoutSpy = jest.spyOn(global, 'setTimeout'); + try { + mockRefreshTokenRequest.mockRejectedValue(axiosErrorWithStatus(status, data)); + + await expect(performTokenRefresh()).resolves.toBe(false); + + expect(mockLogger.warn).not.toHaveBeenCalledWith(expect.objectContaining({ message: 'Token refresh rejected, ending session' })); + expect(mockLogger.error).toHaveBeenCalledWith(expect.objectContaining({ message: 'Token refresh failed; will retry', context: expect.objectContaining({ status }) })); + expect(handlers.onRefreshFailed).not.toHaveBeenCalled(); + expect(jest.getTimerCount()).toBe(1); + expect(setTimeoutSpy.mock.calls[setTimeoutSpy.mock.calls.length - 1][1]).toBe(transientRetryDelayMs(1)); + } finally { + cancelScheduledTokenRefresh(); + jest.useRealTimers(); + setTimeoutSpy.mockRestore(); + } + }); + + it.each([403, 404])('still ends the session for a non-transient %i', async (status) => { + mockRefreshTokenRequest.mockRejectedValue(axiosErrorWithStatus(status)); + + await expect(performTokenRefresh()).resolves.toBe(false); + + expect(mockLogger.error).toHaveBeenCalledWith(expect.objectContaining({ message: 'Token refresh failed' })); + expect(handlers.onRefreshFailed).toHaveBeenCalledTimes(1); + }); + it('keeps the session and schedules a retry when the token endpoint is unreachable', async () => { // A network blip says nothing about the refresh token. Signing the user out for it would end // a dispatcher's shift over a dead spot; the token is kept and the refresh tried again. @@ -139,7 +209,7 @@ describe('performTokenRefresh failure reporting', () => { it('retries on the timer and ends the session once the server actually rejects the token', async () => { jest.useFakeTimers(); try { - mockRefreshTokenRequest.mockRejectedValueOnce(new AxiosError('Network Error', 'ERR_NETWORK')).mockRejectedValueOnce(axiosErrorWithStatus(400)); + mockRefreshTokenRequest.mockRejectedValueOnce(new AxiosError('Network Error', 'ERR_NETWORK')).mockRejectedValueOnce(axiosErrorWithStatus(400, INVALID_GRANT)); await expect(performTokenRefresh()).resolves.toBe(false); expect(handlers.onRefreshFailed).not.toHaveBeenCalled(); diff --git a/src/lib/auth/token-refresh.ts b/src/lib/auth/token-refresh.ts index 93b66f3c..d95b4e1e 100644 --- a/src/lib/auth/token-refresh.ts +++ b/src/lib/auth/token-refresh.ts @@ -13,7 +13,7 @@ const MIN_REFRESH_DELAY_MS = 5000; // than treated as the end of the session: a dispatcher who loses signal for a minute must not be // signed out by it. The delay doubles from the first value up to the cap and stays there; the // refresh token's own lifetime is what bounds the retries, because once it lapses the server -// answers 400 and the session ends the normal way. +// answers 400 invalid_grant and the session ends the normal way. const TRANSIENT_RETRY_BASE_MS = 5000; const TRANSIENT_RETRY_MAX_MS = 60000; @@ -57,11 +57,34 @@ const scheduleTransientRetry = (): void => { }; /** - * Whether a failed token request says anything about the token. The endpoint answering 400/401 is - * the server's verdict on the refresh token itself; a network error, a timeout or a 5xx is the - * endpoint not being reachable, which says nothing about the token. + * Whether the token endpoint rejected the refresh token itself: a 400 or 401 whose OAuth body says + * `invalid_grant`, which is how the Resgrid token endpoint (OpenIddict) answers a refresh token it no + * longer honours (expired, revoked, already rotated). Only this ends the session. + * + * Any other 400/401 says nothing about the token: `invalid_request`, `invalid_client` and the other + * OAuth errors are about the request or the client, and a 400/401 with no OAuth body at all is a + * proxy's or load balancer's error page. Those are retried like an outage; a refresh token that + * really is dead still ends the session once it expires, because then the server answers + * invalid_grant. */ -const isTransientFailure = (status: number | undefined): boolean => status === undefined || status >= 500 || status === 408 || status === 429; +export const isRefreshCredentialRejection = (error: unknown): boolean => { + if (!axios.isAxiosError(error)) { + return false; + } + const status = error.response?.status; + 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'; +}; + +/** + * Whether a failed token request is worth retrying, checked after `isRefreshCredentialRejection`. A + * network error, a timeout, a 5xx or rate limiting is the endpoint not being reachable, which says + * nothing about the token; so does a 400/401 that is not invalid_grant (see above). + */ +const isTransientFailure = (status: number | undefined): boolean => status === undefined || status === 400 || status === 401 || status >= 500 || status === 408 || status === 429; // Only an Axios error qualifies: a throw from our own handlers (applyAuthResponse refusing a // response that raced sign-out) has no status either, but it is a verdict, not an outage. @@ -134,13 +157,13 @@ export function performTokenRefresh(): Promise { return false; } // A refresh token the server no longer honours is how a session ends: the token - // endpoint answers 400/401, the user goes back to the login screen, and nothing is - // broken. Only the shapes that mean something is actually wrong - network loss, - // server faults - are worth reporting as errors. + // endpoint answers 400/401 invalid_grant, the user goes back to the login screen, and + // nothing is broken. Only the shapes that mean something is actually wrong - network + // loss, server faults, a 400/401 that is not about the token - are worth reporting as errors. const status = axios.isAxiosError(error) ? error.response?.status : undefined; const context = { status, error: error instanceof Error ? error.message : String(error) }; - if (status === 400 || status === 401) { + if (isRefreshCredentialRejection(error)) { logger.warn({ message: 'Token refresh rejected, ending session', context }); } else if (axios.isAxiosError(error) && isTransientFailure(status)) { // The token is still good as far as anyone knows; keep it and try again shortly. Callers diff --git a/src/lib/logging/__tests__/index.test.ts b/src/lib/logging/__tests__/index.test.ts index 60f7dfff..28c71bcd 100644 --- a/src/lib/logging/__tests__/index.test.ts +++ b/src/lib/logging/__tests__/index.test.ts @@ -226,4 +226,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 1b6f22f5..7c5b9566 100644 --- a/src/lib/logging/index.tsx +++ b/src/lib/logging/index.tsx @@ -55,7 +55,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);