Skip to content

Add Security Checks Log #6973

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

Closed
wants to merge 22 commits into from
Closed
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
3 changes: 2 additions & 1 deletion resources/buildConfigDefinitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ function getENVPrefix(iface) {
'IdempotencyOptions' : 'PARSE_SERVER_EXPERIMENTAL_IDEMPOTENCY_',
'AccountLockoutOptions' : 'PARSE_SERVER_ACCOUNT_LOCKOUT_',
'PasswordPolicyOptions' : 'PARSE_SERVER_PASSWORD_POLICY_',
'SecurityChecksOptions' : 'PARSE_SERVER_SECURITY_CHECKS_',
'FileUploadOptions' : 'PARSE_SERVER_FILE_UPLOAD_',
}
if (options[iface.id.name]) {
Expand Down Expand Up @@ -166,7 +167,7 @@ function parseDefaultValue(elt, value, t) {
if (type == 'NumberOrBoolean') {
literalValue = t.numericLiteral(parsers.numberOrBoolParser('')(value));
}
const literalTypes = ['Object', 'IdempotencyOptions','FileUploadOptions','CustomPagesOptions', 'PagesCustomUrlsOptions', 'PagesOptions'];
const literalTypes = ['Object', 'IdempotencyOptions','FileUploadOptions','CustomPagesOptions', 'PagesCustomUrlsOptions', 'PagesOptions','SecurityChecksOptions'];
if (literalTypes.includes(type)) {
const object = parsers.objectParser(value);
const props = Object.keys(object).map((key) => {
Expand Down
76 changes: 76 additions & 0 deletions spec/ParseServer.Security.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
'use strict';
const Parse = require('parse/node');
const request = require('../lib/request');

const defaultHeaders = {
'X-Parse-Application-Id': 'test',
'X-Parse-Rest-API-Key': 'rest',
'Content-Type': 'application/json',
};
const masterKeyHeaders = {
'X-Parse-Application-Id': 'test',
'X-Parse-Rest-API-Key': 'rest',
'X-Parse-Master-Key': 'test',
'Content-Type': 'application/json',
};
const defaultOptions = {
headers: defaultHeaders,
json: true,
};
const masterKeyOptions = {
headers: masterKeyHeaders,
json: true,
};

describe('SecurityChecks', () => {
it('should reject access when not using masterKey (/securityChecks)', done => {
request(
Object.assign({ url: Parse.serverURL + '/securityChecks' }, defaultOptions)
).then(done.fail, () => done());
});
it('should reject access by default to /securityChecks, even with masterKey', done => {
request(
Object.assign({ url: Parse.serverURL + '/securityChecks' }, masterKeyOptions)
).then(done.fail, () => done());
});
it('can get security advice', async done => {
await reconfigureServer({
securityChecks: {
enableSecurityChecks: true,
enableLogOutput: true,
},
});
const options = Object.assign({}, masterKeyOptions, {
method: 'GET',
url: Parse.serverURL + '/securityChecks',
});
request(options).then(res => {
expect(res.data.CLP).not.toBeUndefined();
expect(res.data.ServerConfiguration).not.toBeUndefined();
expect(res.data.Database).not.toBeUndefined();
expect(res.data.Total).not.toBeUndefined();
done();
});
});

it('can get security on start', async done => {
await reconfigureServer({
securityChecks: {
enableSecurityChecks: true,
enableLogOutput: true,
},
});
const logger = require('../lib/logger').logger;
spyOn(logger, 'warn').and.callFake(() => {});
await new Promise(resolve => {
setTimeout(resolve, 2000);
});
let messagesCalled = '';
for (const item in logger.warn.calls.all()) {
const call = logger.warn.calls.all()[item];
messagesCalled = messagesCalled + ' ' + (call.args || []).join(' ');
}
expect(messagesCalled).toContain('Clients are currently allowed to create new classes.');
done();
});
});
1 change: 0 additions & 1 deletion src/Adapters/Storage/Mongo/MongoStorageAdapter.js
Original file line number Diff line number Diff line change
Expand Up @@ -1057,5 +1057,4 @@ export class MongoStorageAdapter implements StorageAdapter {
});
}
}

export default MongoStorageAdapter;
20 changes: 20 additions & 0 deletions src/Options/Definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -373,6 +373,11 @@ module.exports.ParseServerOptions = {
action: parsers.numberParser('schemaCacheTTL'),
default: 5000,
},
securityChecks: {
env: 'PARSE_SERVER_SECURITY_CHECKS_OPTIONS',
help: 'View recommendations for server improvements',
action: parsers.objectParser,
},
serverCloseComplete: {
env: 'PARSE_SERVER_SERVER_CLOSE_COMPLETE',
help: 'Callback when server has closed',
Expand Down Expand Up @@ -664,6 +669,21 @@ module.exports.IdempotencyOptions = {
default: 300,
},
};
module.exports.SecurityChecksOptions = {
enableLogOutput: {
env: 'PARSE_SERVER_SECURITY_CHECKS_ENABLE_LOG_OUTPUT',
help:
'Is true if security warnings should be written to logs. This should only be enabled temporarily to not expose weak security settings in logs.',
action: parsers.booleanParser,
default: false,
},
enableSecurityChecks: {
env: 'PARSE_SERVER_SECURITY_CHECKS_ENABLE_SECURITY_CHECKS',
help: 'If true if Parse Server should self-check the security of its current configuration.',
action: parsers.booleanParser,
default: false,
},
};
module.exports.AccountLockoutOptions = {
duration: {
env: 'PARSE_SERVER_ACCOUNT_LOCKOUT_DURATION',
Expand Down
7 changes: 7 additions & 0 deletions src/Options/docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
* @property {Boolean} revokeSessionOnPasswordReset When a user changes their password, either through the reset password email or while logged in, all sessions are revoked if this is true. Set to false if you don't want to revoke sessions.
* @property {Boolean} scheduledPush Configuration for push scheduling, defaults to false.
* @property {Number} schemaCacheTTL The TTL for caching the schema for optimizing read/write operations. You should put a long TTL when your DB is in production. default to 5000; set 0 to disable.
* @property {SecurityChecksOptions} securityChecks View recommendations for server improvements
* @property {Function} serverCloseComplete Callback when server has closed
* @property {Function} serverStartComplete Callback when server has started
* @property {String} serverURL URL to your parse server with http:// or https://.
Expand Down Expand Up @@ -150,6 +151,12 @@
* @property {Number} ttl The duration in seconds after which a request record is discarded from the database, defaults to 300s.
*/

/**
* @interface SecurityChecksOptions
* @property {Boolean} enableLogOutput Is true if security warnings should be written to logs. This should only be enabled temporarily to not expose weak security settings in logs.
* @property {Boolean} enableSecurityChecks If true if Parse Server should self-check the security of its current configuration.
*/

/**
* @interface AccountLockoutOptions
* @property {Number} duration number of minutes that a locked-out account remains locked out before automatically becoming unlocked.
Expand Down
12 changes: 12 additions & 0 deletions src/Options/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,9 @@ export interface ParseServerOptions {
:ENV: PARSE_SERVER_EXPERIMENTAL_IDEMPOTENCY_OPTIONS
:DEFAULT: false */
idempotencyOptions: ?IdempotencyOptions;
/* View recommendations for server improvements
:ENV: PARSE_SERVER_SECURITY_CHECKS_OPTIONS */
securityChecks: ?SecurityChecksOptions;
/* Options for file uploads
:ENV: PARSE_SERVER_FILE_UPLOAD_OPTIONS
:DEFAULT: {} */
Expand Down Expand Up @@ -351,6 +354,15 @@ export interface IdempotencyOptions {
ttl: ?number;
}

export interface SecurityChecksOptions {
/* If true if Parse Server should self-check the security of its current configuration.
:DEFAULT: false */
enableSecurityChecks: ?boolean;
/* Is true if security warnings should be written to logs. This should only be enabled temporarily to not expose weak security settings in logs.
:DEFAULT: false */
enableLogOutput: ?boolean;
}

export interface AccountLockoutOptions {
/* number of minutes that a locked-out account remains locked out before automatically becoming unlocked. */
duration: ?number;
Expand Down
38 changes: 37 additions & 1 deletion src/ParseServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ import { AggregateRouter } from './Routers/AggregateRouter';
import { ParseServerRESTController } from './ParseServerRESTController';
import * as controllers from './Controllers';
import { ParseGraphQLServer } from './GraphQL/ParseGraphQLServer';
import SecurityCheck from './SecurityCheck';
import { registerServerSecurityChecks } from './SecurityChecks';

// Mutate the Parse object to add the Cloud Code handlers
addParseCloud();
Expand Down Expand Up @@ -79,7 +81,19 @@ class ParseServer {
Promise.all([dbInitPromise, hooksLoadPromise])
.then(() => {
if (serverStartComplete) {
serverStartComplete();
return serverStartComplete();
}
return Promise.resolve();
})
.then(() => {
if ((options.securityChecks || {}).enableLogOutput) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should never be undefined, instead set a default value in option definitions.

return registerServerSecurityChecks(this.config);
}
return Promise.resolve();
})
.then(() => {
if ((options.securityChecks || {}).enableLogOutput) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

this.getSecurityChecks();
}
})
.catch(error => {
Expand Down Expand Up @@ -110,6 +124,28 @@ class ParseServer {
return this._app;
}

async getSecurityChecks() {
const checks = await SecurityCheck.getChecks();
const logger = logging.getLogger();
const { Total } = checks;
delete checks.Total;
for (const category in checks) {
const data = checks[category];
for (const check of data) {
const { title, warning, error, result, success } = check;
if (result === 'success') {
logger.warn(`✅ ${success}\n`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All this logic should go into the security checks files, not into the server file.

} else {
const appendString = error && error !== 'Check failed.' ? ` with error: ${error}` : '';
logger.warn(`❌ ${warning}\n❗ Check "${title}" failed${appendString}\n`);
}
}
}
if (Total !== 0) {
logger.warn(`❗ ${Total} security warning(s) for Parse Server`);
}
}

handleShutdown() {
const promises = [];
const { adapter: databaseAdapter } = this.config.databaseController;
Expand Down
17 changes: 17 additions & 0 deletions src/Routers/CloudCodeRouter.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import PromiseRouter from '../PromiseRouter';
import Parse from 'parse/node';
import rest from '../rest';
import SecurityCheck from '../SecurityCheck';
const triggers = require('../triggers');
const middleware = require('../middlewares');

Expand Down Expand Up @@ -53,6 +54,12 @@ export class CloudCodeRouter extends PromiseRouter {
middleware.promiseEnforceMasterKeyAccess,
CloudCodeRouter.deleteJob
);
this.route(
'GET',
'/securityChecks',
middleware.promiseEnforceMasterKeyAccess,
CloudCodeRouter.getSecurityChecks
);
}

static getJobs(req) {
Expand Down Expand Up @@ -120,4 +127,14 @@ export class CloudCodeRouter extends PromiseRouter {
};
});
}

static async getSecurityChecks(req) {
if (!req.config.securityChecks || !req.config.securityChecks.enableSecurityChecks) {
throw new Parse.Error(Parse.Error.OBJECT_NOT_FOUND, 'Unauthorized');
}
const response = await SecurityCheck.getChecks();
return {
response,
};
}
}
102 changes: 102 additions & 0 deletions src/SecurityCheck.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import logger from './logger';
class SecurityCheck {
constructor(data) {
const { group, title, warning, check, failed, success } = data;
try {
if (!group || !title || !warning) {
throw 'Security checks must have a group, title, and a warning.';
}
if (typeof group !== 'string') {
throw '"group" of the security check must be a string, e.g SecurityCheck.Category.Database';
}
if (typeof success !== 'string') {
throw '"success" message of the security check must be a string.';
}
if (typeof title !== 'string') {
throw '"title" of the security check must be a string.';
}
if (typeof warning !== 'string') {
throw '"warning" message of the security check must be a string.';
}
if (check && typeof check !== 'function') {
throw '"check" of the security check must be a function.';
}
this.group = group;
this.title = title;
this.warning = warning;
this.check = check;
this.failed = failed;
this.success = success;
} catch (e) {
logger.error(e);
return;
}
_registerCheck(this);
}
async run() {
try {
if (this.failed) {
throw 'Check failed.';
}
if (!this.check) {
return {
result: 'success',
};
}
const result = await this.check();
if (result != null && result === false) {
throw 'Check failed.';
}
return {
result: 'success',
};
} catch (error) {
return {
result: 'fail',
error,
};
}
}
setFailed() {
this.failed = true;
}
}
SecurityCheck.Category = {
Database: 'Database',
CLP: 'CLP',
ServerConfiguration: 'ServerConfiguration',
};
SecurityCheck.getChecks = async () => {
const resultsByGroup = {};
let total = 0;
const resolveSecurityCheck = async check => {
const { group, title, warning, success } = check;
const { result, error } = await check.run();
const category = resultsByGroup[group] || [];
category.push({
title,
warning,
error,
result,
success,
});
resultsByGroup[group] = category;
if (result !== 'success') {
total++;
}
};
await Promise.all(securityCheckStore.map(check => resolveSecurityCheck(check)));
resultsByGroup.Total = total;
return resultsByGroup;
};
const securityCheckStore = [];
function _registerCheck(securityCheck) {
for (const [i, check] of securityCheckStore.entries()) {
if (check.title == securityCheck.title && check.warning == securityCheck.warning) {
securityCheckStore[i] = securityCheck;
return;
}
}
securityCheckStore.push(securityCheck);
}
module.exports = SecurityCheck;
Loading