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
17 changes: 9 additions & 8 deletions lib/helpers/redactUrl.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,19 @@ const redactPathSecrets = (path) => {
};

/**
* @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
* sensitive key is replaced with `REDACTED`. Tolerant of a missing/empty URL and
* URLs with no query string (returned unchanged). Pure + synchronous so it is
* safe to call on every logged request.
* @desc Redact single-use secrets from a request URL for logging: sensitive
* PATH segments (see `redactPathSecrets`) AND sensitive query-string
* parameters. Preserves every other path segment and query parameter; only
* matching path segments / query values are replaced with `REDACTED`.
* Tolerant of a missing/empty URL and URLs with no query string. Pure +
* synchronous so it is safe to call on every logged request.
* @param {String} url - the raw request URL (path + optional query string)
* @returns {String} the URL with sensitive query values redacted
* @returns {String} the URL with sensitive path segments and query values redacted
*/
const redactUrl = (url) => {
if (!url || typeof url !== 'string') return url;
const queryStart = url.indexOf('?');
if (queryStart === -1) return url;
if (queryStart === -1) return redactPathSecrets(url);

const pathPart = url.slice(0, queryStart);
const queryPart = url.slice(queryStart + 1);
Expand All @@ -66,7 +67,7 @@ const redactUrl = (url) => {
})
.join('&');

return `${pathPart}?${redactedQuery}`;
return `${redactPathSecrets(pathPart)}?${redactedQuery}`;
};

export default redactUrl;
Expand Down
31 changes: 30 additions & 1 deletion lib/helpers/tests/redactUrl.unit.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,43 @@ describe('redactUrl', () => {
expect(redactUrl('/x?inviteToken=secret&bar=2')).toBe('/x?inviteToken=REDACTED&bar=2');
});

test('leaves a URL without a query string unchanged', () => {
test('leaves a non-sensitive URL without a query string unchanged', () => {
expect(redactUrl('/api/auth/signup')).toBe('/api/auth/signup');
});

test('leaves a URL whose query has no sensitive key unchanged', () => {
expect(redactUrl('/api/users?page=2&perPage=10')).toBe('/api/users?page=2&perPage=10');
});

test('redacts a reset-token path with no query string', () => {
expect(redactUrl('/api/auth/reset/SECRETTOKEN')).toBe('/api/auth/reset/REDACTED');
});

test('redacts a verify-email-token path with no query string', () => {
expect(redactUrl('/api/auth/verify-email/SECRETTOKEN')).toBe('/api/auth/verify-email/REDACTED');
});

test('redacts an invitation verify-token path with no query string', () => {
expect(redactUrl('/api/auth/invitations/verify/SECRETTOKEN')).toBe('/api/auth/invitations/verify/REDACTED');
expect(redactUrl('/api/invitations/verify/SECRETTOKEN')).toBe('/api/invitations/verify/REDACTED');
});

test('redacts a reset-token path when a query string is also present', () => {
expect(redactUrl('/api/auth/reset/SECRETTOKEN?foo=1')).toBe('/api/auth/reset/REDACTED?foo=1');
});

test('redacts a verify-email-token path when a query string is also present', () => {
expect(redactUrl('/api/auth/verify-email/SECRETTOKEN?foo=1')).toBe('/api/auth/verify-email/REDACTED?foo=1');
});

test('redacts an invitation verify-token path when a query string is also present', () => {
expect(redactUrl('/api/auth/invitations/verify/SECRETTOKEN?foo=1')).toBe('/api/auth/invitations/verify/REDACTED?foo=1');
});

test('redacts both a path secret and a query secret in the same URL', () => {
expect(redactUrl('/api/auth/reset/SECRETTOKEN?inviteToken=abc123')).toBe('/api/auth/reset/REDACTED?inviteToken=REDACTED');
});

test('handles a bare sensitive key with no value (no = sign)', () => {
// `?inviteToken` with no value: still scrubbed (becomes inviteToken=REDACTED) so a
// malformed/edge query can never leak a partial token form downstream.
Expand Down
7 changes: 4 additions & 3 deletions lib/services/express.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,9 +198,10 @@ const initMiddleware = (app) => {
morgan.token('email', (req) => _.get(req, 'user.email') || 'Unknown email');
morgan.token('requestId', (req) => req.id || '-');
morgan.token('orgId', (req) => _.get(req, 'organization.id') || _.get(req, 'organization._id', '-'));
// Override the built-in :url token to scrub single-use secrets (e.g. inviteToken)
// from the query string before they hit the log stream. Both the dev and prod
// log patterns interpolate :url, so the override covers every logged request.
// Override the built-in :url token to scrub single-use secrets — both path
// tokens (e.g. /auth/reset/:token) and query params (e.g. inviteToken) —
// before they hit the log stream. Both the dev and prod log patterns
// interpolate :url, so the override covers every logged request.
morgan.token('url', (req) => redactUrl(req.originalUrl || req.url));
app.use(morgan(logger.getLogFormat(), logger.getMorganOptions()));
}
Expand Down
Loading