diff --git a/lib/helpers/redactUrl.js b/lib/helpers/redactUrl.js index 9157caf76..4c83927d4 100644 --- a/lib/helpers/redactUrl.js +++ b/lib/helpers/redactUrl.js @@ -8,6 +8,37 @@ */ const SENSITIVE_QUERY_KEYS = ['inviteToken']; +/** + * Path segments that are immediately followed by a single-use secret carried as + * a PATH parameter (not the query string). Routes such as + * `GET /api/auth/reset/:token`, `POST /api/auth/verify-email/:token` and + * `GET /api/*(auth/)?invitations/verify/:token` embed a still-valid token + * directly in the path, so the segment right after one of these markers must be + * redacted before the URL reaches any log/analytics store. + */ +const SENSITIVE_PATH_MARKERS = ['reset', 'verify', 'verify-email']; + +/** + * @desc Redact single-use secrets embedded as PATH parameters. Any segment that + * immediately follows a sensitive marker (see `SENSITIVE_PATH_MARKERS`) is + * replaced with `REDACTED`; every other segment is preserved. Operates on the + * path portion only — callers strip the query string first. Pure + synchronous, + * tolerant of a missing/empty value (returned unchanged). + * @param {String} path - the request path (no query string) + * @returns {String} the path with secret segments redacted + */ +const redactPathSecrets = (path) => { + if (!path || typeof path !== 'string') return path; + + const segments = path.split('/'); + for (let i = 0; i < segments.length - 1; i += 1) { + if (SENSITIVE_PATH_MARKERS.includes(segments[i]) && segments[i + 1]) { + segments[i + 1] = 'REDACTED'; + } + } + return segments.join('/'); +}; + /** * @desc Redact sensitive query-string parameters from a request URL for logging. * Preserves the path and every other query parameter; only the value of a @@ -39,4 +70,4 @@ const redactUrl = (url) => { }; export default redactUrl; -export { SENSITIVE_QUERY_KEYS }; +export { SENSITIVE_QUERY_KEYS, SENSITIVE_PATH_MARKERS, redactPathSecrets }; diff --git a/lib/helpers/tests/redactUrl.unit.tests.js b/lib/helpers/tests/redactUrl.unit.tests.js index 2c0d9251e..060fe8089 100644 --- a/lib/helpers/tests/redactUrl.unit.tests.js +++ b/lib/helpers/tests/redactUrl.unit.tests.js @@ -1,5 +1,5 @@ import { describe, test, expect } from '@jest/globals'; -import redactUrl, { SENSITIVE_QUERY_KEYS } from '../redactUrl.js'; +import redactUrl, { SENSITIVE_QUERY_KEYS, SENSITIVE_PATH_MARKERS, redactPathSecrets } from '../redactUrl.js'; describe('redactUrl', () => { test('redacts an inviteToken value, preserving the path', () => { @@ -42,3 +42,45 @@ describe('redactUrl', () => { expect(SENSITIVE_QUERY_KEYS).toContain('inviteToken'); }); }); + +describe('redactPathSecrets', () => { + test('redacts the segment following /reset/', () => { + expect(redactPathSecrets('/api/auth/reset/SECRETTOKEN')).toBe('/api/auth/reset/REDACTED'); + }); + + test('redacts the segment following /verify-email/', () => { + expect(redactPathSecrets('/api/auth/verify-email/SECRETTOKEN')).toBe('/api/auth/verify-email/REDACTED'); + }); + + test('redacts the segment following /verify/ (invitation verify routes)', () => { + expect(redactPathSecrets('/api/auth/invitations/verify/SECRETTOKEN')).toBe('/api/auth/invitations/verify/REDACTED'); + expect(redactPathSecrets('/api/invitations/verify/SECRETTOKEN')).toBe('/api/invitations/verify/REDACTED'); + }); + + test('leaves a path with no sensitive marker unchanged', () => { + expect(redactPathSecrets('/api/tasks/123')).toBe('/api/tasks/123'); + }); + + test('does not append REDACTED when the marker is the trailing segment', () => { + expect(redactPathSecrets('/api/auth/reset')).toBe('/api/auth/reset'); + expect(redactPathSecrets('/api/auth/reset/')).toBe('/api/auth/reset/'); + }); + + test('only redacts the single segment immediately after the marker', () => { + expect(redactPathSecrets('/api/auth/reset/SECRETTOKEN/extra')).toBe('/api/auth/reset/REDACTED/extra'); + }); + + test('does not redact a segment that merely contains the marker as a substring', () => { + expect(redactPathSecrets('/api/auth/resetish/keepme')).toBe('/api/auth/resetish/keepme'); + }); + + test('is tolerant of falsy / non-string input', () => { + expect(redactPathSecrets('')).toBe(''); + expect(redactPathSecrets(undefined)).toBeUndefined(); + expect(redactPathSecrets(null)).toBeNull(); + }); + + test('exports the sensitive path markers', () => { + expect(SENSITIVE_PATH_MARKERS).toEqual(expect.arrayContaining(['reset', 'verify', 'verify-email'])); + }); +}); diff --git a/lib/middlewares/analytics.js b/lib/middlewares/analytics.js index 9d397d101..e13d8c366 100644 --- a/lib/middlewares/analytics.js +++ b/lib/middlewares/analytics.js @@ -3,6 +3,7 @@ */ import AnalyticsService from '../services/analytics.js'; import logger from '../services/logger.js'; +import { redactPathSecrets } from '../helpers/redactUrl.js'; /** * Default route prefixes to skip when auto-capturing API request events. @@ -46,8 +47,16 @@ const createAnalyticsMiddleware = (options = {}) => { const distinctId = req.user?._id ? String(req.user._id) : 'anonymous'; const groups = req.organization?._id ? { company: String(req.organization._id) } : undefined; + // Never let a single-use secret carried as a PATH parameter (e.g. + // /api/auth/reset/:token) reach the analytics store. Prefer the matched + // route pattern (resolved by the time `finish` fires) so path params + // collapse to their placeholder; fall back to redacting known secret + // segments from the concrete path (unmatched routes / 404s). + const routePath = typeof req.route?.path === 'string' ? req.route.path : null; + const safeEndpoint = routePath ? `${req.baseUrl || ''}${routePath}` : redactPathSecrets(endpoint); + AnalyticsService.track(distinctId, 'api_request', { - endpoint, + endpoint: safeEndpoint, method: req.method, statusCode: res.statusCode, responseTime: Date.now() - start, diff --git a/lib/middlewares/tests/analytics.middleware.integration.tests.js b/lib/middlewares/tests/analytics.middleware.integration.tests.js index a7bda2c7d..15ba3870a 100644 --- a/lib/middlewares/tests/analytics.middleware.integration.tests.js +++ b/lib/middlewares/tests/analytics.middleware.integration.tests.js @@ -46,6 +46,9 @@ describe('Analytics middleware integration tests:', () => { app.get('/public/logo.png', (_req, res) => res.send('img')); app.get('/favicon.ico', (_req, res) => res.send('ico')); app.get('/api/error', (_req, res) => res.status(500).json({ error: true })); + // Route carrying a single-use secret as a PATH parameter — must never reach + // the analytics store as the raw token. + app.get('/api/auth/reset/:token', (_req, res) => res.json({ ok: true })); }); beforeEach(() => { @@ -159,4 +162,15 @@ describe('Analytics middleware integration tests:', () => { const properties = mockTrack.mock.calls[0][2]; expect(properties.endpoint).toBe('/api/tasks'); }); + + test('should never capture a single-use token embedded in the path', async () => { + await request(app) + .get('/api/auth/reset/SUPERSECRETTOKEN') + .expect(200); + + expect(mockTrack).toHaveBeenCalledTimes(1); + const properties = mockTrack.mock.calls[0][2]; + expect(properties.endpoint).toBe('/api/auth/reset/:token'); + expect(properties.endpoint).not.toContain('SUPERSECRETTOKEN'); + }); }); diff --git a/lib/middlewares/tests/analytics.middleware.unit.tests.js b/lib/middlewares/tests/analytics.middleware.unit.tests.js index a9e50004f..8422d2c3d 100644 --- a/lib/middlewares/tests/analytics.middleware.unit.tests.js +++ b/lib/middlewares/tests/analytics.middleware.unit.tests.js @@ -209,6 +209,73 @@ describe('Analytics middleware unit tests:', () => { ); }); + test('should redact a single-use token embedded in the path (no matched route)', () => { + req.originalUrl = '/api/auth/reset/SECRETTOKEN'; + req.url = '/api/auth/reset/SECRETTOKEN'; + req.route = undefined; // unmatched / 404 — fall back to the path redactor + analyticsMiddleware(req, res, next); + triggerFinish(); + + const { endpoint } = mockTrack.mock.calls[0][2]; + expect(endpoint).toBe('/api/auth/reset/REDACTED'); + expect(endpoint).not.toContain('SECRETTOKEN'); + }); + + test('should redact path tokens for verify-email and invitation verify routes', () => { + const cases = [ + ['/api/auth/verify-email/SECRETTOKEN', '/api/auth/verify-email/REDACTED'], + ['/api/auth/invitations/verify/SECRETTOKEN', '/api/auth/invitations/verify/REDACTED'], + ['/api/invitations/verify/SECRETTOKEN', '/api/invitations/verify/REDACTED'], + ]; + + cases.forEach(([raw, expected]) => { + mockTrack.mockClear(); + // Fresh res per case so a prior iteration's finish handler is not re-fired. + const finishHandlers = []; + const localRes = { + statusCode: 200, + on: jest.fn((event, handler) => { + if (event === 'finish') finishHandlers.push(handler); + }), + }; + req.originalUrl = raw; + req.url = raw; + req.route = undefined; + analyticsMiddleware(req, localRes, next); + finishHandlers.forEach((fn) => fn()); + + const { endpoint } = mockTrack.mock.calls[0][2]; + expect(endpoint).toBe(expected); + expect(endpoint).not.toContain('SECRETTOKEN'); + }); + }); + + test('should prefer the matched route pattern over the concrete path', () => { + req.originalUrl = '/api/auth/reset/SECRETTOKEN'; + req.url = '/api/auth/reset/SECRETTOKEN'; + req.baseUrl = ''; + req.route = { path: '/api/auth/reset/:token' }; + analyticsMiddleware(req, res, next); + triggerFinish(); + + const { endpoint } = mockTrack.mock.calls[0][2]; + expect(endpoint).toBe('/api/auth/reset/:token'); + expect(endpoint).not.toContain('SECRETTOKEN'); + }); + + test('should prefix the route pattern with req.baseUrl when the router is mounted', () => { + req.originalUrl = '/api/auth/reset/SECRETTOKEN'; + req.url = '/api/auth/reset/SECRETTOKEN'; + req.baseUrl = '/api/auth'; + req.route = { path: '/reset/:token' }; + analyticsMiddleware(req, res, next); + triggerFinish(); + + const { endpoint } = mockTrack.mock.calls[0][2]; + expect(endpoint).toBe('/api/auth/reset/:token'); + expect(endpoint).not.toContain('SECRETTOKEN'); + }); + test('should accept custom skipPrefixes via createAnalyticsMiddleware', () => { const customMiddleware = createAnalyticsMiddleware({ skipPrefixes: ['/custom'] }); req.originalUrl = '/custom/path';