Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion lib/helpers/redactUrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -39,4 +70,4 @@ const redactUrl = (url) => {
};

export default redactUrl;
export { SENSITIVE_QUERY_KEYS };
export { SENSITIVE_QUERY_KEYS, SENSITIVE_PATH_MARKERS, redactPathSecrets };
44 changes: 43 additions & 1 deletion lib/helpers/tests/redactUrl.unit.tests.js
Original file line number Diff line number Diff line change
@@ -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', () => {
Expand Down Expand Up @@ -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']));
});
});
11 changes: 10 additions & 1 deletion lib/middlewares/analytics.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 14 additions & 0 deletions lib/middlewares/tests/analytics.middleware.integration.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(() => {
Expand Down Expand Up @@ -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');
});
});
67 changes: 67 additions & 0 deletions lib/middlewares/tests/analytics.middleware.unit.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
Loading