Skip to content

Move password masking out of logging clients where possible #2762

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
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
16 changes: 0 additions & 16 deletions spec/WinstonLoggerAdapter.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,20 +97,4 @@ describe('verbose logs', () => {
done();
})
});

it("should not mask information in non _User class", (done) => {
let obj = new Parse.Object('users');
obj.set('password', 'pw');
obj.save().then(() => {
let winstonLoggerAdapter = new WinstonLoggerAdapter();
return winstonLoggerAdapter.query({
from: new Date(Date.now() - 500),
size: 100,
level: 'verbose'
});
}).then((results) => {
expect(results[1].body.password).toEqual("pw");
done();
});
});
});
51 changes: 42 additions & 9 deletions src/Controllers/LoggerController.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { Parse } from 'parse/node';
import PromiseRouter from '../PromiseRouter';
import AdaptableController from './AdaptableController';
import { LoggerAdapter } from '../Adapters/Logger/LoggerAdapter';
import url from 'url';

const MILLISECONDS_IN_A_DAY = 24 * 60 * 60 * 1000;
const LOG_STRING_TRUNCATE_LENGTH = 1000;
Expand All @@ -19,8 +20,48 @@ export const LogOrder = {

export class LoggerController extends AdaptableController {

maskSensitiveUrl(urlString) {
const password = url.parse(urlString, true).query.password;

if (password) {
urlString = urlString.replace('password=' + password, 'password=********');
}
return urlString;
}

maskSensitive(argArray) {
return argArray.map(e => {
if (!e) {
return e;
}

if (typeof e === 'string') {
return e.replace(/(password".?:.?")[^"]*"/g, '$1********"');
}
// else it is an object...

// check the url
if (e.url) {
e.url = this.maskSensitiveUrl(e.url);
}

if (e.body) {
for (let key of Object.keys(e.body)) {
if (key === 'password') {
e.body[key] = '********';
break;
}
}
}

return e;
});
}

log(level, args) {
args = [].concat(level, [...args]);
// make the passed in arguments object an array with the spread operator
args = this.maskSensitive([...args]);
args = [].concat(level, args);
this.adapter.log.apply(this.adapter, args);
}

Expand Down Expand Up @@ -61,14 +102,6 @@ export class LoggerController extends AdaptableController {
return null;
}

cleanAndTruncateLogMessage(string) {
return this.truncateLogMessage(this.cleanLogMessage(string));
}

cleanLogMessage(string) {
return string.replace(/password":"[^"]*"/g, 'password":"********"');
}

truncateLogMessage(string) {
if (string && string.length > LOG_STRING_TRUNCATE_LENGTH) {
const truncated = string.substring(0, LOG_STRING_TRUNCATE_LENGTH) + truncationMarker;
Expand Down
24 changes: 2 additions & 22 deletions src/PromiseRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ function makeExpressHandler(appId, promiseHandler) {
return function(req, res, next) {
try {
let url = maskSensitiveUrl(req);
let body = maskSensitiveBody(req);
let body = Object.assign({}, req.body);
let stringifiedBody = JSON.stringify(body, null, 2);
log.verbose(`REQUEST for [${req.method}] ${url}: ${stringifiedBody}`, {
method: req.method,
Expand Down Expand Up @@ -198,33 +198,13 @@ function makeExpressHandler(appId, promiseHandler) {
}
}

function maskSensitiveBody(req) {
let maskBody = Object.assign({}, req.body);
let shouldMaskBody = (req.method === 'POST' && req.originalUrl.endsWith('/users')
&& !req.originalUrl.includes('classes')) ||
(req.method === 'PUT' && /users\/\w+$/.test(req.originalUrl)
&& !req.originalUrl.includes('classes')) ||
(req.originalUrl.includes('classes/_User'));
if (shouldMaskBody) {
for (let key of Object.keys(maskBody)) {
if (key == 'password') {
maskBody[key] = '********';
break;
}
}
}
return maskBody;
}

function maskSensitiveUrl(req) {
let maskUrl = req.originalUrl.toString();
let shouldMaskUrl = req.method === 'GET' && req.originalUrl.includes('/login')
&& !req.originalUrl.includes('classes');
if (shouldMaskUrl) {
let password = url.parse(req.originalUrl, true).query.password;
if (password) {
maskUrl = maskUrl.replace('password=' + password, 'password=********')
}
maskUrl = log.maskSensitiveUrl(maskUrl);
}
return maskUrl;
}
4 changes: 2 additions & 2 deletions src/Routers/FunctionsRouter.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@ export class FunctionsRouter extends PromiseRouter {

return new Promise(function (resolve, reject) {
const userString = (req.auth && req.auth.user) ? req.auth.user.id : undefined;
const cleanInput = logger.cleanAndTruncateLogMessage(JSON.stringify(params));
const cleanInput = logger.truncateLogMessage(JSON.stringify(params));
var response = FunctionsRouter.createResponseObject((result) => {
try {
const cleanResult = logger.cleanAndTruncateLogMessage(JSON.stringify(result.response.result));
const cleanResult = logger.truncateLogMessage(JSON.stringify(result.response.result));
logger.info(`Ran cloud function ${functionName} for user ${userString} `
+ `with:\n Input: ${cleanInput }\n Result: ${cleanResult }`, {
functionName,
Expand Down
8 changes: 4 additions & 4 deletions src/triggers.js
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ function userIdForLog(auth) {
}

function logTriggerAfterHook(triggerType, className, input, auth) {
const cleanInput = logger.cleanAndTruncateLogMessage(JSON.stringify(input));
const cleanInput = logger.truncateLogMessage(JSON.stringify(input));
logger.info(`${triggerType} triggered for ${className} for user ${userIdForLog(auth)}:\n Input: ${cleanInput}`, {
className,
triggerType,
Expand All @@ -221,8 +221,8 @@ function logTriggerAfterHook(triggerType, className, input, auth) {
}

function logTriggerSuccessBeforeHook(triggerType, className, input, result, auth) {
const cleanInput = logger.cleanAndTruncateLogMessage(JSON.stringify(input));
const cleanResult = logger.cleanAndTruncateLogMessage(JSON.stringify(result));
const cleanInput = logger.truncateLogMessage(JSON.stringify(input));
const cleanResult = logger.truncateLogMessage(JSON.stringify(result));
logger.info(`${triggerType} triggered for ${className} for user ${userIdForLog(auth)}:\n Input: ${cleanInput}\n Result: ${cleanResult}`, {
className,
triggerType,
Expand All @@ -231,7 +231,7 @@ function logTriggerSuccessBeforeHook(triggerType, className, input, result, auth
}

function logTriggerErrorBeforeHook(triggerType, className, input, auth, error) {
const cleanInput = logger.cleanAndTruncateLogMessage(JSON.stringify(input));
const cleanInput = logger.truncateLogMessage(JSON.stringify(input));
logger.error(`${triggerType} failed for ${className} for user ${userIdForLog(auth)}:\n Input: ${cleanInput}\n Error: ${JSON.stringify(error)}`, {
className,
triggerType,
Expand Down