Move password masking out of logging clients where possible (#2762)

Move password masking functionality into LoggerController.

The is a more aggresive approach to masking password string in the logs.

Cleaning the url is still in the PromiseRouter because picking it out of the log string
would be fragile.

This will cause more log messages to be scanned for password strings, and may cause a password
string to be obsfucated that is not neccesarily part of parse internals -- but i think that is
still a good thing....

see: #2755 & #2680
This commit is contained in:
Arthur Cinader
2016-09-22 12:05:54 -07:00
committed by Florent Vilmart
parent ad707457be
commit a41cbcbc7f
5 changed files with 50 additions and 53 deletions

View File

@@ -97,20 +97,4 @@ describe('verbose logs', () => {
done(); 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();
});
});
}); });

View File

@@ -2,6 +2,7 @@ import { Parse } from 'parse/node';
import PromiseRouter from '../PromiseRouter'; import PromiseRouter from '../PromiseRouter';
import AdaptableController from './AdaptableController'; import AdaptableController from './AdaptableController';
import { LoggerAdapter } from '../Adapters/Logger/LoggerAdapter'; import { LoggerAdapter } from '../Adapters/Logger/LoggerAdapter';
import url from 'url';
const MILLISECONDS_IN_A_DAY = 24 * 60 * 60 * 1000; const MILLISECONDS_IN_A_DAY = 24 * 60 * 60 * 1000;
const LOG_STRING_TRUNCATE_LENGTH = 1000; const LOG_STRING_TRUNCATE_LENGTH = 1000;
@@ -19,8 +20,48 @@ export const LogOrder = {
export class LoggerController extends AdaptableController { 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) { 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); this.adapter.log.apply(this.adapter, args);
} }
@@ -61,14 +102,6 @@ export class LoggerController extends AdaptableController {
return null; return null;
} }
cleanAndTruncateLogMessage(string) {
return this.truncateLogMessage(this.cleanLogMessage(string));
}
cleanLogMessage(string) {
return string.replace(/password":"[^"]*"/g, 'password":"********"');
}
truncateLogMessage(string) { truncateLogMessage(string) {
if (string && string.length > LOG_STRING_TRUNCATE_LENGTH) { if (string && string.length > LOG_STRING_TRUNCATE_LENGTH) {
const truncated = string.substring(0, LOG_STRING_TRUNCATE_LENGTH) + truncationMarker; const truncated = string.substring(0, LOG_STRING_TRUNCATE_LENGTH) + truncationMarker;

View File

@@ -144,7 +144,7 @@ function makeExpressHandler(appId, promiseHandler) {
return function(req, res, next) { return function(req, res, next) {
try { try {
let url = maskSensitiveUrl(req); let url = maskSensitiveUrl(req);
let body = maskSensitiveBody(req); let body = Object.assign({}, req.body);
let stringifiedBody = JSON.stringify(body, null, 2); let stringifiedBody = JSON.stringify(body, null, 2);
log.verbose(`REQUEST for [${req.method}] ${url}: ${stringifiedBody}`, { log.verbose(`REQUEST for [${req.method}] ${url}: ${stringifiedBody}`, {
method: req.method, method: req.method,
@@ -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) { function maskSensitiveUrl(req) {
let maskUrl = req.originalUrl.toString(); let maskUrl = req.originalUrl.toString();
let shouldMaskUrl = req.method === 'GET' && req.originalUrl.includes('/login') let shouldMaskUrl = req.method === 'GET' && req.originalUrl.includes('/login')
&& !req.originalUrl.includes('classes'); && !req.originalUrl.includes('classes');
if (shouldMaskUrl) { if (shouldMaskUrl) {
let password = url.parse(req.originalUrl, true).query.password; maskUrl = log.maskSensitiveUrl(maskUrl);
if (password) {
maskUrl = maskUrl.replace('password=' + password, 'password=********')
}
} }
return maskUrl; return maskUrl;
} }

View File

@@ -125,10 +125,10 @@ export class FunctionsRouter extends PromiseRouter {
return new Promise(function (resolve, reject) { return new Promise(function (resolve, reject) {
const userString = (req.auth && req.auth.user) ? req.auth.user.id : undefined; 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) => { var response = FunctionsRouter.createResponseObject((result) => {
try { 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} ` logger.info(`Ran cloud function ${functionName} for user ${userString} `
+ `with:\n Input: ${cleanInput }\n Result: ${cleanResult }`, { + `with:\n Input: ${cleanInput }\n Result: ${cleanResult }`, {
functionName, functionName,

View File

@@ -212,7 +212,7 @@ function userIdForLog(auth) {
} }
function logTriggerAfterHook(triggerType, className, input, 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}`, { logger.info(`${triggerType} triggered for ${className} for user ${userIdForLog(auth)}:\n Input: ${cleanInput}`, {
className, className,
triggerType, triggerType,
@@ -221,8 +221,8 @@ function logTriggerAfterHook(triggerType, className, input, auth) {
} }
function logTriggerSuccessBeforeHook(triggerType, className, input, result, auth) { function logTriggerSuccessBeforeHook(triggerType, className, input, result, auth) {
const cleanInput = logger.cleanAndTruncateLogMessage(JSON.stringify(input)); const cleanInput = logger.truncateLogMessage(JSON.stringify(input));
const cleanResult = logger.cleanAndTruncateLogMessage(JSON.stringify(result)); const cleanResult = logger.truncateLogMessage(JSON.stringify(result));
logger.info(`${triggerType} triggered for ${className} for user ${userIdForLog(auth)}:\n Input: ${cleanInput}\n Result: ${cleanResult}`, { logger.info(`${triggerType} triggered for ${className} for user ${userIdForLog(auth)}:\n Input: ${cleanInput}\n Result: ${cleanResult}`, {
className, className,
triggerType, triggerType,
@@ -231,7 +231,7 @@ function logTriggerSuccessBeforeHook(triggerType, className, input, result, auth
} }
function logTriggerErrorBeforeHook(triggerType, className, input, auth, error) { 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)}`, { logger.error(`${triggerType} failed for ${className} for user ${userIdForLog(auth)}:\n Input: ${cleanInput}\n Error: ${JSON.stringify(error)}`, {
className, className,
triggerType, triggerType,