From b7984cdb51e5a1fb4299516501a4292fc4b35fe3 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Thu, 4 Mar 2021 04:19:36 +0100 Subject: [PATCH 01/25] added Parse Server security option --- resources/buildConfigDefinitions.js | 3 +- spec/Security.spec.js | 58 +++++++++++++++++++++++++++++ src/Config.js | 19 ++++++++++ src/Options/Definitions.js | 21 +++++++++++ src/Options/docs.js | 7 ++++ src/Options/index.js | 12 ++++++ 6 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 spec/Security.spec.js diff --git a/resources/buildConfigDefinitions.js b/resources/buildConfigDefinitions.js index 4f3ccde245..9a03dbf353 100644 --- a/resources/buildConfigDefinitions.js +++ b/resources/buildConfigDefinitions.js @@ -52,6 +52,7 @@ function getENVPrefix(iface) { 'AccountLockoutOptions' : 'PARSE_SERVER_ACCOUNT_LOCKOUT_', 'PasswordPolicyOptions' : 'PARSE_SERVER_PASSWORD_POLICY_', 'FileUploadOptions' : 'PARSE_SERVER_FILE_UPLOAD_', + 'SecurityOptions': 'PARSE_SERVER_SECURITY_', } if (options[iface.id.name]) { return options[iface.id.name] @@ -167,7 +168,7 @@ function parseDefaultValue(elt, value, t) { if (type == 'NumberOrBoolean') { literalValue = t.numericLiteral(parsers.numberOrBoolParser('')(value)); } - const literalTypes = ['Object', 'PagesRoute', 'IdempotencyOptions','FileUploadOptions','CustomPagesOptions', 'PagesCustomUrlsOptions', 'PagesOptions']; + const literalTypes = ['Object', 'SecurityOptions', 'PagesRoute', 'IdempotencyOptions','FileUploadOptions','CustomPagesOptions', 'PagesCustomUrlsOptions', 'PagesOptions']; if (literalTypes.includes(type)) { const object = parsers.objectParser(value); const props = Object.keys(object).map((key) => { diff --git a/spec/Security.spec.js b/spec/Security.spec.js new file mode 100644 index 0000000000..c88acf15db --- /dev/null +++ b/spec/Security.spec.js @@ -0,0 +1,58 @@ +'use strict'; + +const Config = require('../lib/Config'); +const Definitions = require('../lib/Options/Definitions'); + +describe('Security Checks', () => { + let config; + const publicServerURL = 'http://localhost:8378/1'; + + async function reconfigureServerWithSecurityConfig(security) { + config.security = security; + await reconfigureServer(config); + } + + beforeEach(async () => { + config = { + appId: 'test', + appName: 'ExampleAppName', + publicServerURL, + security: { + enableCheck: true, + enableCheckLog: true, + }, + }; + }); + + describe('server options', () => { + it('uses default configuration when none is set', async () => { + await reconfigureServerWithSecurityConfig({}); + expect(Config.get(Parse.applicationId).security.enableCheck).toBe( + Definitions.SecurityOptions.enableCheck.default + ); + expect(Config.get(Parse.applicationId).security.enableCheckLog).toBe( + Definitions.SecurityOptions.enableCheckLog.default + ); + }); + + it('throws on invalid configuration', async () => { + const options = [ + [], + 'a', + 0, + true, + { enableCheck: 'a' }, + { enableCheck: 0 }, + { enableCheck: {} }, + { enableCheck: [] }, + { enableCheckLog: 'a' }, + { enableCheckLog: 0 }, + { enableCheckLog: {} }, + { enableCheckLog: [] }, + ]; + for (const option of options) { + await expectAsync(reconfigureServerWithSecurityConfig(option)).toBeRejected(); + } + }); + }); +}); diff --git a/src/Config.js b/src/Config.js index 9dea819671..521f354d8e 100644 --- a/src/Config.js +++ b/src/Config.js @@ -11,6 +11,7 @@ import { FileUploadOptions, AccountLockoutOptions, PagesOptions, + SecurityOptions, } from './Options/Definitions'; import { isBoolean, isString } from 'lodash'; @@ -79,6 +80,7 @@ export class Config { emailVerifyTokenReuseIfValid, fileUpload, pages, + security, }) { if (masterKey === readOnlyMasterKey) { throw new Error('masterKey and readOnlyMasterKey should be different'); @@ -114,6 +116,23 @@ export class Config { this.validateAllowHeaders(allowHeaders); this.validateIdempotencyOptions(idempotencyOptions); this.validatePagesOptions(pages); + this.validateSecurityOptions(security); + } + + static validateSecurityOptions(security) { + if (Object.prototype.toString.call(security) !== '[object Object]') { + throw 'Parse Server option security must be an object.'; + } + if (security.enableCheck === undefined) { + security.enableCheck = SecurityOptions.enableCheck.default; + } else if (!isBoolean(security.enableCheck)) { + throw 'Parse Server option security.enableCheck must be a boolean.'; + } + if (security.enableCheckLog === undefined) { + security.enableCheckLog = SecurityOptions.enableCheckLog.default; + } else if (!isBoolean(security.enableCheckLog)) { + throw 'Parse Server option security.enableCheckLog must be a boolean.'; + } } static validatePagesOptions(pages) { diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index e06d1c8fc3..c76423c2fd 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -373,6 +373,12 @@ module.exports.ParseServerOptions = { action: parsers.numberParser('schemaCacheTTL'), default: 5000, }, + security: { + env: 'PARSE_SERVER_SECURITY', + help: 'The security options to identify and report weak security settings.', + action: parsers.objectParser, + default: {}, + }, serverCloseComplete: { env: 'PARSE_SERVER_SERVER_CLOSE_COMPLETE', help: 'Callback when server has closed', @@ -424,6 +430,21 @@ module.exports.ParseServerOptions = { help: 'Key sent with outgoing webhook calls', }, }; +module.exports.SecurityOptions = { + enableCheck: { + env: 'PARSE_SERVER_SECURITY_ENABLE_CHECK', + help: 'Is true if Parse Server should check for weak security settings.', + action: parsers.booleanParser, + default: false, + }, + enableCheckLog: { + env: 'PARSE_SERVER_SECURITY_ENABLE_CHECK_LOG', + help: + 'Is true if the security check report should be written to logs. This should only be enabled temporarily to not expose weak security settings in logs.', + action: parsers.booleanParser, + default: false, + }, +}; module.exports.PagesOptions = { customRoutes: { env: 'PARSE_SERVER_PAGES_CUSTOM_ROUTES', diff --git a/src/Options/docs.js b/src/Options/docs.js index da3013c0b7..3e44cda202 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -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 {SecurityOptions} security The security options to identify and report weak security settings. * @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://. @@ -80,6 +81,12 @@ * @property {String} webhookKey Key sent with outgoing webhook calls */ +/** + * @interface SecurityOptions + * @property {Boolean} enableCheck Is true if Parse Server should check for weak security settings. + * @property {Boolean} enableCheckLog Is true if the security check report should be written to logs. This should only be enabled temporarily to not expose weak security settings in logs. + */ + /** * @interface PagesOptions * @property {PagesRoute[]} customRoutes The custom routes. diff --git a/src/Options/index.js b/src/Options/index.js index 1114e4fe0f..64b60fdfdd 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -227,6 +227,18 @@ export interface ParseServerOptions { serverStartComplete: ?(error: ?Error) => void; /* Callback when server has closed */ serverCloseComplete: ?() => void; + /* The security options to identify and report weak security settings. + :DEFAULT: {} */ + security: ?SecurityOptions; +} + +export interface SecurityOptions { + /* Is true if Parse Server should check for weak security settings. + :DEFAULT: false */ + enableCheck: ?boolean; + /* Is true if the security check report should be written to logs. This should only be enabled temporarily to not expose weak security settings in logs. + :DEFAULT: false */ + enableCheckLog: ?boolean; } export interface PagesOptions { From 5c40279e1c0d8b461e4eae87d3b0264bb27678c8 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Thu, 4 Mar 2021 04:37:22 +0100 Subject: [PATCH 02/25] added SecurityRouter --- spec/Security.spec.js | 30 ++++++++++++++++++++++++++++++ src/ParseServer.js | 2 ++ src/Routers/SecurityRouter.js | 31 +++++++++++++++++++++++++++++++ 3 files changed, 63 insertions(+) create mode 100644 src/Routers/SecurityRouter.js diff --git a/spec/Security.spec.js b/spec/Security.spec.js index c88acf15db..3ad1bba758 100644 --- a/spec/Security.spec.js +++ b/spec/Security.spec.js @@ -1,17 +1,28 @@ 'use strict'; const Config = require('../lib/Config'); +const request = require('../lib/request'); const Definitions = require('../lib/Options/Definitions'); describe('Security Checks', () => { let config; const publicServerURL = 'http://localhost:8378/1'; + const securityUrl = publicServerURL + '/security'; async function reconfigureServerWithSecurityConfig(security) { config.security = security; await reconfigureServer(config); } + const securityRequest = (options) => request(Object.assign({ + url: securityUrl, + headers: { + 'X-Parse-Master-Key': Parse.masterKey, + 'X-Parse-Application-Id': Parse.applicationId, + }, + followRedirects: false, + }, options)).catch(e => e); + beforeEach(async () => { config = { appId: 'test', @@ -22,6 +33,7 @@ describe('Security Checks', () => { enableCheckLog: true, }, }; + await reconfigureServer(config); }); describe('server options', () => { @@ -55,4 +67,22 @@ describe('Security Checks', () => { } }); }); + + describe('security endpoint accessibility', () => { + it('responds with 403 without masterkey', async () => { + const response = await securityRequest({ headers: {} }); + expect(response.status).toBe(403); + }); + + it('responds with 409 with masterkey and security check disabled', async () => { + await reconfigureServerWithSecurityConfig({}); + const response = await securityRequest(); + expect(response.status).toBe(409); + }); + + it('responds with 200 with masterkey and security check enabled', async () => { + const response = await securityRequest(); + expect(response.status).toBe(200); + }); + }); }); diff --git a/src/ParseServer.js b/src/ParseServer.js index a81e97fc60..70f29ab712 100644 --- a/src/ParseServer.js +++ b/src/ParseServer.js @@ -41,6 +41,7 @@ import { AggregateRouter } from './Routers/AggregateRouter'; import { ParseServerRESTController } from './ParseServerRESTController'; import * as controllers from './Controllers'; import { ParseGraphQLServer } from './GraphQL/ParseGraphQLServer'; +import { SecurityRouter } from './Routers/SecurityRouter'; // Mutate the Parse object to add the Cloud Code handlers addParseCloud(); @@ -219,6 +220,7 @@ class ParseServer { new CloudCodeRouter(), new AudiencesRouter(), new AggregateRouter(), + new SecurityRouter(), ]; const routes = routers.reduce((memo, router) => { diff --git a/src/Routers/SecurityRouter.js b/src/Routers/SecurityRouter.js new file mode 100644 index 0000000000..888f0ac9a9 --- /dev/null +++ b/src/Routers/SecurityRouter.js @@ -0,0 +1,31 @@ +import PromiseRouter from '../PromiseRouter'; +import * as middleware from '../middlewares'; +import Config from '../Config'; +import Parse from 'parse/node'; + +export class SecurityRouter extends PromiseRouter { + mountRoutes() { + this.route('GET', '/security', + middleware.promiseEnforceMasterKeyAccess, + this._enforceSecurityCheckEnabled, + async () => { + return { + status: 200, + text: 'OK', + }; + } + ); + } + + async _enforceSecurityCheckEnabled() { + const config = Config.get(Parse.applicationId); + if (!config.security || !config.security.enableCheck) { + const error = new Error(); + error.status = 409; + error.message = 'Enable Parse Server option `security.enableCheck` to run security check.'; + throw error; + } + } +} + +export default SecurityRouter; From 0922b46c395e7022e8a0e6aac65355f5507b4765 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Thu, 4 Mar 2021 23:35:11 +0100 Subject: [PATCH 03/25] added Check class --- spec/Security.spec.js | 69 +++++++++++++++++++++++++++++++++ src/Security/Check.js | 89 +++++++++++++++++++++++++++++++++++++++++++ src/Utils.js | 35 +++++++++++++++++ 3 files changed, 193 insertions(+) create mode 100644 src/Security/Check.js diff --git a/spec/Security.spec.js b/spec/Security.spec.js index 3ad1bba758..28936dad11 100644 --- a/spec/Security.spec.js +++ b/spec/Security.spec.js @@ -1,7 +1,9 @@ 'use strict'; +const Utils = require('../lib/Utils'); const Config = require('../lib/Config'); const request = require('../lib/request'); +const { Check, CheckState } = require('../lib/Security/Check'); const Definitions = require('../lib/Options/Definitions'); describe('Security Checks', () => { @@ -85,4 +87,71 @@ describe('Security Checks', () => { expect(response.status).toBe(200); }); }); + + describe('check', () => { + const initCheck = config => (() => new Check(config)).bind(null); + + it('instantiates check with valid parameters', async () => { + const configs = [ + { + group: 'string', + title: 'string', + warning: 'string', + solution: 'string', + script: () => {} + }, + { + group: 'string', + title: 'string', + warning: 'string', + solution: 'string', + script: async () => {}, + }, + ]; + for (const config of configs) { + expect(initCheck(config)).not.toThrow(); + } + }); + + it('throws instantiating check with invalid parameters', async () => { + const configDefinition = { + group: [false, true, 0, 1, [], {}, () => {}], + title: [false, true, 0, 1, [], {}, () => {}], + warning: [false, true, 0, 1, [], {}, () => {}], + solution: [false, true, 0, 1, [], {}, () => {}], + script: [false, true, 0, 1, [], {}, 'string'], + }; + const configs = Utils.getObjectKeyPermutations(configDefinition); + + for (const config of configs) { + expect(initCheck(config)).toThrow(); + } + }); + + it('sets correct states for check success', async () => { + const check = new Check({ + group: 'string', + title: 'string', + warning: 'string', + solution: 'string', + script: () => {}, + }); + expect(check._checkState == CheckState.none); + check.run(); + expect(check._checkState == CheckState.success); + }); + + it('sets correct states for check fail', async () => { + const check = new Check({ + group: 'string', + title: 'string', + warning: 'string', + solution: 'string', + script: () => { throw 'error' }, + }); + expect(check._checkState == CheckState.none); + check.run(); + expect(check._checkState == CheckState.fail); + }); + }); }); diff --git a/src/Security/Check.js b/src/Security/Check.js new file mode 100644 index 0000000000..97712e8f22 --- /dev/null +++ b/src/Security/Check.js @@ -0,0 +1,89 @@ +/** + * @module SecurityCheck + */ + +import { isFunction, isString } from 'lodash'; + +/** + * A security check. + * @class Check + */ +class Check { + /** + * Constructs a new security check. + * @param {Object} params The parameters. + * @param {String} params.title The title. + * @param {String} params.warning The warning message if the check fails. + * @param {String} params.solution The solution to fix the check. + * @param {Promise} params.script The check script; can be an synchronous or asynchronous function. + */ + constructor(params) { + this._validateParams(params); + const { title, warning, solution, script } = params; + + this.title = title; + this.warning = warning; + this.solution = solution; + this.script = script; + + // Set default properties + this._checkState = CheckState.none; + this.error; + } + + /** + * Returns the current check state. + * @return {CheckState} The check state. + */ + checkState() { + return this._checkState; + } + + async run() { + // Get check script as synchronous or asynchronous function + const script = this.script instanceof Promise ? await this.script : this.script; + + // Run script + try { + script(); + this._checkState = CheckState.success; + } catch (e) { + this.stateFailError = e; + this._checkState = CheckState.fail; + } + } + + /** + * Validates the constructor parameters. + * @param {Object} params The parameters to validate. + */ + _validateParams(params) { + const types = { + group: { t: 'string', c: isString }, + title: { t: 'string', c: isString }, + warning: { t: 'string', c: isString }, + solution: { t: 'string', c: isString }, + script: { t: 'function', c: isFunction }, + }; + for (const key of Object.keys(params)) { + if (!types[key].c(params[key])) { + throw `Invalid check parameter ${key} must be of type ${types[key].t} but is ${typeof params[key]}`; + } + } + } +} + +/** + * The check state. + */ +const CheckState = Object.freeze({ + none: "none", + fail: "fail", + success: "success", +}); + +export default Check; +module.exports = { + Check, + CheckState, +}; diff --git a/src/Utils.js b/src/Utils.js index f4912e3736..ad69ca1ae7 100644 --- a/src/Utils.js +++ b/src/Utils.js @@ -118,6 +118,41 @@ class Utils { } return result; } + + /** + * Determines whether an object is a Promise. + * @param {any} object The object to validate. + * @returns {Boolean} Returns true if the object is a promise. + */ + static isPromise(object) { + return object instanceof Promise; + } + + /** + * Creates an object with all permutations of the original keys. + * @param {Object} object The object to permutate. + * @param {Integer} [index=0] The current key index. + * @param {Object} [current={}] The current result entry being composed. + * @param {Array} [results=[]] The resulting array of permutations. + */ + static getObjectKeyPermutations(object, index = 0, current = {}, results = []) { + const keys = Object.keys(object); + const key = keys[index]; + const values = object[key]; + + for (const value of values) { + current[key] = value; + const nextIndex = index + 1; + + if (nextIndex < keys.length) { + this.getObjectKeyPermutations(object, nextIndex, current, results); + } else { + const result = Object.assign({}, current); + results.push(result); + } + } + return results; + } } module.exports = Utils; From 24105d732fb4cc16d944a94136e195f0e685f199 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Fri, 5 Mar 2021 16:28:25 +0100 Subject: [PATCH 04/25] added CheckGroup class --- spec/Security.spec.js | 66 ++++++++++++++++++++++++++++++++++++++ src/Security/CheckGroup.js | 45 ++++++++++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 src/Security/CheckGroup.js diff --git a/spec/Security.spec.js b/spec/Security.spec.js index 28936dad11..45e4b7d2c7 100644 --- a/spec/Security.spec.js +++ b/spec/Security.spec.js @@ -5,6 +5,7 @@ const Config = require('../lib/Config'); const request = require('../lib/request'); const { Check, CheckState } = require('../lib/Security/Check'); const Definitions = require('../lib/Options/Definitions'); +const CheckGroup = require('../lib/Security/CheckGroup'); describe('Security Checks', () => { let config; @@ -154,4 +155,69 @@ describe('Security Checks', () => { expect(check._checkState == CheckState.fail); }); }); + + describe('check group', () => { + let groupName; + let checkSuccess; + let checkFail; + let Group; + + beforeEach(async () => { + groupName = 'Example Group Name'; + checkSuccess = new Check({ + group: 'TestGroup', + title: 'TestTitleSuccess', + warning: 'TestWarning', + solution: 'TestSolution', + script: () => { + return true; + } + }); + checkFail = new Check({ + group: 'TestGroup', + title: 'TestTitleFail', + warning: 'TestWarning', + solution: 'TestSolution', + script: () => { + throw 'Fail'; + } + }); + Group = class Group extends CheckGroup { + setName() { + return groupName; + } + setChecks() { + return [ checkSuccess, checkFail ]; + } + }; + }); + + it('returns properties if subclassed correctly', async () => { + const group = new Group(); + expect(group.name()).toBe(groupName); + expect(group.checks().length).toBe(2); + expect(group.checks()[0]).toEqual(checkSuccess); + expect(group.checks()[1]).toEqual(checkFail); + }); + + it('throws if subclassed incorrectly', async () => { + class InvalidGroup1 extends CheckGroup {} + expect((() => new InvalidGroup1()).bind()).toThrow('Check group has no name.'); + class InvalidGroup2 extends CheckGroup { + setName() { + return groupName; + } + } + expect((() => new InvalidGroup2()).bind()).toThrow('Check group has no checks.'); + }); + + it('runs checks', async () => { + const group = new Group(); + expect(group.checks()[0].checkState()).toBe(CheckState.none); + expect(group.checks()[1].checkState()).toBe(CheckState.none); + expect((() => group.run()).bind(null)).not.toThrow(); + expect(group.checks()[0].checkState()).toBe(CheckState.success); + expect(group.checks()[1].checkState()).toBe(CheckState.fail); + }); + }); }); diff --git a/src/Security/CheckGroup.js b/src/Security/CheckGroup.js new file mode 100644 index 0000000000..a9fde2905f --- /dev/null +++ b/src/Security/CheckGroup.js @@ -0,0 +1,45 @@ +/** + * @module SecurityCheck + */ + +/** + * A group of security checks. + * @interface CheckGroup + */ +class CheckGroup { + constructor() { + this._name = this.setName(); + this._checks = this.setChecks(); + } + + /** + * The security check group name; to be overridden by child class. + */ + setName() { + throw `Check group has no name.`; + } + name() { + return this._name; + } + + /** + * The security checks; to be overridden by child class. + */ + setChecks() { + throw `Check group has no checks.`; + } + checks() { + return this._checks; + } + + /** + * Runs all checks. + */ + async run() { + for (const check of this._checks) { + check.run(); + } + } +} + +module.exports = CheckGroup; From ad90cf98d7e7acdff1f9072bb52e3bf630018dc9 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Fri, 5 Mar 2021 19:49:40 +0100 Subject: [PATCH 05/25] moved parameter validation to Utils --- src/Security/Check.js | 20 ++++++++------------ src/Utils.js | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/Security/Check.js b/src/Security/Check.js index 97712e8f22..044bc21e0f 100644 --- a/src/Security/Check.js +++ b/src/Security/Check.js @@ -2,6 +2,7 @@ * @module SecurityCheck */ +import Utils from '../Utils'; import { isFunction, isString } from 'lodash'; /** @@ -58,18 +59,13 @@ class Check { * @param {Object} params The parameters to validate. */ _validateParams(params) { - const types = { - group: { t: 'string', c: isString }, - title: { t: 'string', c: isString }, - warning: { t: 'string', c: isString }, - solution: { t: 'string', c: isString }, - script: { t: 'function', c: isFunction }, - }; - for (const key of Object.keys(params)) { - if (!types[key].c(params[key])) { - throw `Invalid check parameter ${key} must be of type ${types[key].t} but is ${typeof params[key]}`; - } - } + Utils.validateParams(params, { + group: { t: 'string', v: isString }, + title: { t: 'string', v: isString }, + warning: { t: 'string', v: isString }, + solution: { t: 'string', v: isString }, + script: { t: 'function', v: isFunction }, + }); } } diff --git a/src/Utils.js b/src/Utils.js index ad69ca1ae7..a7833f5727 100644 --- a/src/Utils.js +++ b/src/Utils.js @@ -153,6 +153,36 @@ class Utils { } return results; } + + /** + * Validates parameters and throws if a parameter is invalid. + * Example parameter types syntax: + * ``` + * { + * parameterName: { + * t: 'boolean', + * v: isBoolean, + * o: true + * }, + * ... + * } + * ``` + * @param {Object} params The parameters to validate. + * @param {Array} types The parameter types used for validation. + * @param {Object} types.t The parameter type; used for error message, not for validation. + * @param {Object} types.v The function to validate the parameter value. + * @param {Boolean} [types.o=false] Is true if the parameter is optional. + */ + static validateParams(params, types) { + for (const key of Object.keys(params)) { + const type = types[key]; + const isOptional = !!type.o; + const param = params[key]; + if (!(isOptional && param == null) && (!type.v(param))) { + throw `Invalid parameter ${key} must be of type ${type.t} but is ${typeof param}`; + } + } + } } module.exports = Utils; From 8db2771ea96aa0b745e136252ae4ac543ca618b1 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Fri, 5 Mar 2021 19:50:53 +0100 Subject: [PATCH 06/25] added CheckRunner class --- spec/Security.spec.js | 160 ++++++++++++++----- src/Routers/SecurityRouter.js | 12 +- src/Security/CheckGroups/CheckGroups.js | 7 + src/Security/CheckRunner.js | 199 ++++++++++++++++++++++++ 4 files changed, 336 insertions(+), 42 deletions(-) create mode 100644 src/Security/CheckGroups/CheckGroups.js create mode 100644 src/Security/CheckRunner.js diff --git a/spec/Security.spec.js b/spec/Security.spec.js index 45e4b7d2c7..f570b563e6 100644 --- a/spec/Security.spec.js +++ b/spec/Security.spec.js @@ -3,11 +3,17 @@ const Utils = require('../lib/Utils'); const Config = require('../lib/Config'); const request = require('../lib/request'); -const { Check, CheckState } = require('../lib/Security/Check'); const Definitions = require('../lib/Options/Definitions'); +const { Check, CheckState } = require('../lib/Security/Check'); const CheckGroup = require('../lib/Security/CheckGroup'); +const CheckRunner = require('../lib/Security/CheckRunner'); +const CheckGroups = require('../lib/Security/CheckGroups/CheckGroups'); describe('Security Checks', () => { + let Group; + let groupName; + let checkSuccess; + let checkFail; let config; const publicServerURL = 'http://localhost:8378/1'; const securityUrl = publicServerURL + '/security'; @@ -27,6 +33,33 @@ describe('Security Checks', () => { }, options)).catch(e => e); beforeEach(async () => { + groupName = 'Example Group Name'; + checkSuccess = new Check({ + group: 'TestGroup', + title: 'TestTitleSuccess', + warning: 'TestWarning', + solution: 'TestSolution', + script: () => { + return true; + } + }); + checkFail = new Check({ + group: 'TestGroup', + title: 'TestTitleFail', + warning: 'TestWarning', + solution: 'TestSolution', + script: () => { + throw 'Fail'; + } + }); + Group = class Group extends CheckGroup { + setName() { + return groupName; + } + setChecks() { + return [ checkSuccess, checkFail ]; + } + }; config = { appId: 'test', appName: 'ExampleAppName', @@ -157,41 +190,6 @@ describe('Security Checks', () => { }); describe('check group', () => { - let groupName; - let checkSuccess; - let checkFail; - let Group; - - beforeEach(async () => { - groupName = 'Example Group Name'; - checkSuccess = new Check({ - group: 'TestGroup', - title: 'TestTitleSuccess', - warning: 'TestWarning', - solution: 'TestSolution', - script: () => { - return true; - } - }); - checkFail = new Check({ - group: 'TestGroup', - title: 'TestTitleFail', - warning: 'TestWarning', - solution: 'TestSolution', - script: () => { - throw 'Fail'; - } - }); - Group = class Group extends CheckGroup { - setName() { - return groupName; - } - setChecks() { - return [ checkSuccess, checkFail ]; - } - }; - }); - it('returns properties if subclassed correctly', async () => { const group = new Group(); expect(group.name()).toBe(groupName); @@ -220,4 +218,94 @@ describe('Security Checks', () => { expect(group.checks()[1].checkState()).toBe(CheckState.fail); }); }); + + describe('check runner', () => { + const initRunner = config => (() => new CheckRunner(config)).bind(null); + + it('instantiates runner with valid parameters', async () => { + const configDefinition = { + enableCheck: [false, true, undefined], + enableCheckLog: [false, true, undefined], + checkGroups: [[], undefined], + }; + const configs = Utils.getObjectKeyPermutations(configDefinition); + for (const config of configs) { + expect(initRunner(config)).not.toThrow(); + } + }); + + it('throws instantiating runner with invalid parameters', async () => { + const configDefinition = { + enableCheck: [0, 1, [], {}, () => {}], + enableCheckLog: [0, 1, [], {}, () => {}], + checkGroups: [false, true, 0, 1, {}, () => {}], + }; + const configs = Utils.getObjectKeyPermutations(configDefinition); + + for (const config of configs) { + expect(initRunner(config)).toThrow(); + } + }); + + it('instantiates runner with default parameters', async () => { + const runner = new CheckRunner(); + expect(runner.enableCheck).toBeFalse(); + expect(runner.enableCheckLog).toBeFalse(); + expect(runner.checkGroups).toBe(CheckGroups); + }); + + it('runs all checks of all groups', async () => { + const checkGroups = [ Group, Group ]; + const runner = new CheckRunner({ checkGroups }); + const report = await runner.run(); + expect(report.report.groups[0].checks[0].state).toBe(CheckState.success); + expect(report.report.groups[0].checks[1].state).toBe(CheckState.fail); + expect(report.report.groups[1].checks[0].state).toBe(CheckState.success); + expect(report.report.groups[1].checks[1].state).toBe(CheckState.fail); + }); + + it('reports correct default syntax version 1.0.0', async () => { + const checkGroups = [ Group ]; + const runner = new CheckRunner({ checkGroups, enableCheckLog: true }); + const report = await runner.run(); + expect(report).toEqual({ + report: { + version: "1.0.0", + state: "fail", + groups: [ + { + name: "Example Group Name", + state: "fail", + checks: [ + { + title: "TestTitleSuccess", + state: "success", + }, + { + title: "TestTitleFail", + state: "fail", + warning: "TestWarning", + solution: "TestSolution", + }, + ], + }, + ], + }, + }); + }); + + it('logs report', async () => { + const logger = require('../lib/logger').logger; + const logSpy = spyOn(logger, 'warn').and.callThrough(); + const checkGroups = [ Group ]; + const runner = new CheckRunner({ checkGroups, enableCheckLog: true }); + const report = await runner.run(); + const titles = report.report.groups.flatMap(group => group.checks.map(check => check.title)); + expect(titles.length).toBe(2); + + for (const title of titles) { + expect(logSpy.calls.all()[0].args[0]).toContain(title); + } + }); + }); }); diff --git a/src/Routers/SecurityRouter.js b/src/Routers/SecurityRouter.js index 888f0ac9a9..a9c50ecb8e 100644 --- a/src/Routers/SecurityRouter.js +++ b/src/Routers/SecurityRouter.js @@ -1,24 +1,24 @@ import PromiseRouter from '../PromiseRouter'; import * as middleware from '../middlewares'; -import Config from '../Config'; -import Parse from 'parse/node'; +import CheckRunner from '../Security/CheckRunner'; export class SecurityRouter extends PromiseRouter { mountRoutes() { this.route('GET', '/security', middleware.promiseEnforceMasterKeyAccess, this._enforceSecurityCheckEnabled, - async () => { + async (req) => { + const report = await new CheckRunner(req.config.security).run(); return { status: 200, - text: 'OK', + response: report, }; } ); } - async _enforceSecurityCheckEnabled() { - const config = Config.get(Parse.applicationId); + async _enforceSecurityCheckEnabled(req) { + const config = req.config; if (!config.security || !config.security.enableCheck) { const error = new Error(); error.status = 409; diff --git a/src/Security/CheckGroups/CheckGroups.js b/src/Security/CheckGroups/CheckGroups.js new file mode 100644 index 0000000000..c177e253c3 --- /dev/null +++ b/src/Security/CheckGroups/CheckGroups.js @@ -0,0 +1,7 @@ +/** + * @module SecurityCheck + */ + +/** + * The list of security check groups. + */ diff --git a/src/Security/CheckRunner.js b/src/Security/CheckRunner.js new file mode 100644 index 0000000000..016956f0f5 --- /dev/null +++ b/src/Security/CheckRunner.js @@ -0,0 +1,199 @@ +/** + * @module SecurityCheck + */ + +import Utils from '../Utils'; +import { CheckState } from './Check'; +import * as CheckGroups from './CheckGroups/CheckGroups'; +import logger from '../logger'; +import { isArray, isBoolean } from 'lodash'; + +/** + * The security check runner. + */ +class CheckRunner { + /** + * The security check runner. + * @param {Object} [config] The configuration options. + * @param {Boolean} [config.enableCheck=false] Is true if Parse Server should report weak security settings. + * @param {Boolean} [config.enableCheckLog=false] Is true if the security check report should be written to logs. + * @param {Object} [config.checkGroups] The check groups to run. Default are the groups defined in `./CheckGroups/CheckGroups.js`. + */ + constructor(config = {}) { + this._validateParams(config); + const { enableCheck = false, enableCheckLog = false, checkGroups = CheckGroups } = config; + this.enableCheck = enableCheck; + this.enableCheckLog = enableCheckLog; + this.checkGroups = checkGroups; + } + + /** + * Runs all security checks and returns the results. + * @params + * @returns {Object} The security check report. + */ + async run({ version = '1.0.0' } = {}) { + // Instantiate check groups + const groups = Object.values(this.checkGroups) + .filter(c => typeof c === 'function') + .map(CheckGroup => new CheckGroup()); + + // Run checks + groups.forEach(group => group.run()); + + // Generate JSON report + const report = this._generateReport({ groups, version }); + + // If report should be written to logs + if (this.enableCheckLog) { + this._logReport(report) + } + return report; + } + + /** + * Generates a security check report in JSON format with schema: + * ``` + * { + * report: { + * version: "1.0.0", // The report version, defines the schema + * state: "fail" // The disjunctive indicator of failed checks in all groups. + * groups: [ // The check groups + * { + * name: "House", // The group name + * state: "fail" // The disjunctive indicator of failed checks in this group. + * checks: [ // The checks + * title: "Door locked", // The check title + * state: "fail" // The check state + * warning: "Anyone can enter your house." // The warning. + * solution: "Lock your door." // The solution. + * ] + * }, + * ... + * ] + * } + * } + * ``` + * @param {Object} params The parameters. + * @param {Array} params.groups The check groups. + * @param {String} params.version: The report schema version. + * @returns {Object} The report. + */ + _generateReport({ groups, version }) { + // Create report template + const report = { + report: { + version, + state: CheckState.success, + groups: [] + } + }; + + // Identify report version + switch (version) { + case '1.0.0': + default: + // For each check group + for (const group of groups) { + + // Create group report + const groupReport = { + name: group.name(), + state: CheckState.success, + checks: [], + } + + // Create check reports + groupReport.checks = group.checks().map(check => { + const checkReport = { + title: check.title, + state: check.checkState(), + }; + if (check.checkState() == CheckState.fail) { + checkReport.warning = check.warning; + checkReport.solution = check.solution; + report.report.state = CheckState.fail; + groupReport.state = CheckState.fail; + } + return checkReport; + }); + + report.report.groups.push(groupReport); + } + } + return report; + } + + /** + * Logs the security check report. + * @param {Object} report The report to log. + */ + _logReport(report) { + + // Determine log level depending on whether any check failed + const log = report.report.state == CheckState.success ? (s) => logger.info(s) : (s) => logger.warn(s); + + // Declare output + const indent = ' '; + let output = ''; + let checksCount = 0; + let failedChecksCount = 0; + let skippedCheckCount = 0; + + // Traverse all groups and checks for compose output + for (const group of report.report.groups) { + output += `${this._getLogIconForState(group.state)} ${group.name}:` + + for (const check of group.checks) { + checksCount++; + output += `\n${indent}${this._getLogIconForState(check.state)} ${check.title}`; + + if (check.state == CheckState.fail) { + failedChecksCount++; + output += `\n${indent}${indent}${check.warning}`; + output += `\n${indent}${indent}${check.solution}`; + } else if (check.state == CheckState.none) { + skippedCheckCount++; + output += `\n${indent}${indent}Test did not execute, this is likely an internal server issue, please report.`; + } + } + } + + output = + `Parse Server Security Check` + + `\n\n${failedChecksCount} weak security settings identified; ${checksCount} checks executed; ${skippedCheckCount} checks not executed.` + + `\n\n` + + output + + ``; + + // Write log + log(output); + } + + /** + * Returns an icon for use in the report log output. + * @param {CheckState} state The check state. + * @returns {String} The icon. + */ + _getLogIconForState(state) { + switch (state) { + case CheckState.success: return '✅'; + case CheckState.fail: return '❌'; + default: return 'â„šī¸'; + } + } + + /** + * Validates the constructor parameters. + * @param {Object} params The parameters to validate. + */ + _validateParams(params) { + Utils.validateParams(params, { + enableCheck: { t: 'boolean', v: isBoolean, o: true }, + enableCheckLog: { t: 'boolean', v: isBoolean, o: true }, + checkGroups: { t: 'array', v: isArray, o: true }, + }); + } +} + +module.exports = CheckRunner; From 475767837dfadbe012b69bb7c73f0cc0e0d2c2c1 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Fri, 5 Mar 2021 20:55:19 +0100 Subject: [PATCH 07/25] added auto-run on server start --- spec/Security.spec.js | 22 ++++++++++++++++++++++ src/ParseServer.js | 6 ++++++ 2 files changed, 28 insertions(+) diff --git a/spec/Security.spec.js b/spec/Security.spec.js index f570b563e6..a59a81ba28 100644 --- a/spec/Security.spec.js +++ b/spec/Security.spec.js @@ -104,6 +104,28 @@ describe('Security Checks', () => { }); }); + describe('auto-run', () => { + it('runs security checks on server start if enabled', async () => { + const runnerSpy = spyOn(CheckRunner.prototype, 'run').and.callThrough(); + await reconfigureServerWithSecurityConfig({ enableCheck: true, enableCheckLog: true }); + expect(runnerSpy).toHaveBeenCalledTimes(1); + }); + + it('does not run security checks on server start if disabled', async () => { + const runnerSpy = spyOn(CheckRunner.prototype, 'run').and.callThrough(); + const configs = [ + { enableCheck: true, enableCheckLog: false }, + { enableCheck: false, enableCheckLog: false }, + { enableCheck: false }, + {}, + ]; + for (const config of configs) { + await reconfigureServerWithSecurityConfig(config); + expect(runnerSpy).not.toHaveBeenCalled(); + } + }); + }); + describe('security endpoint accessibility', () => { it('responds with 403 without masterkey', async () => { const response = await securityRequest({ headers: {} }); diff --git a/src/ParseServer.js b/src/ParseServer.js index 70f29ab712..31041b7412 100644 --- a/src/ParseServer.js +++ b/src/ParseServer.js @@ -42,6 +42,7 @@ import { ParseServerRESTController } from './ParseServerRESTController'; import * as controllers from './Controllers'; import { ParseGraphQLServer } from './GraphQL/ParseGraphQLServer'; import { SecurityRouter } from './Routers/SecurityRouter'; +import CheckRunner from './Security/CheckRunner'; // Mutate the Parse object to add the Cloud Code handlers addParseCloud(); @@ -59,6 +60,7 @@ class ParseServer { appId = requiredParameter('You must provide an appId!'), masterKey = requiredParameter('You must provide a masterKey!'), cloud, + security, javascriptKey, serverURL = requiredParameter('You must provide a serverURL!'), serverStartComplete, @@ -102,6 +104,10 @@ class ParseServer { throw "argument 'cloud' must either be a string or a function"; } } + + if (security && security.enableCheck && security.enableCheckLog) { + new CheckRunner(options.security).run(); + } } get app() { From d994747faf367af8509042888f1ca563cbf0d555 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Fri, 5 Mar 2021 20:55:56 +0100 Subject: [PATCH 08/25] added custom security checks as Parse Server option --- src/Options/Definitions.js | 6 ++++++ src/Options/docs.js | 1 + src/Options/index.js | 3 +++ 3 files changed, 10 insertions(+) diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index c76423c2fd..de92038964 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -431,6 +431,12 @@ module.exports.ParseServerOptions = { }, }; module.exports.SecurityOptions = { + checkGroups: { + env: 'PARSE_SERVER_SECURITY_CHECK_GROUPS', + help: + 'The security check groups to run. This allows to add custom security checks or override existing ones. Default are the groups defined in `CheckGroups.js`.', + action: parsers.arrayParser, + }, enableCheck: { env: 'PARSE_SERVER_SECURITY_ENABLE_CHECK', help: 'Is true if Parse Server should check for weak security settings.', diff --git a/src/Options/docs.js b/src/Options/docs.js index 3e44cda202..86c27b761d 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -83,6 +83,7 @@ /** * @interface SecurityOptions + * @property {CheckGroup[]} checkGroups The security check groups to run. This allows to add custom security checks or override existing ones. Default are the groups defined in `CheckGroups.js`. * @property {Boolean} enableCheck Is true if Parse Server should check for weak security settings. * @property {Boolean} enableCheckLog Is true if the security check report should be written to logs. This should only be enabled temporarily to not expose weak security settings in logs. */ diff --git a/src/Options/index.js b/src/Options/index.js index 64b60fdfdd..6b4a504801 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -6,6 +6,7 @@ import { CacheAdapter } from '../Adapters/Cache/CacheAdapter'; import { MailAdapter } from '../Adapters/Email/MailAdapter'; import { PubSubAdapter } from '../Adapters/PubSub/PubSubAdapter'; import { WSSAdapter } from '../Adapters/WebSocketServer/WSSAdapter'; +import { CheckGroup } from '../Security/CheckGroup'; // @flow type Adapter = string | any | T; @@ -239,6 +240,8 @@ export interface SecurityOptions { /* Is true if the security check report should be written to logs. This should only be enabled temporarily to not expose weak security settings in logs. :DEFAULT: false */ enableCheckLog: ?boolean; + /* The security check groups to run. This allows to add custom security checks or override existing ones. Default are the groups defined in `CheckGroups.js`. */ + checkGroups: ?(CheckGroup[]); } export interface PagesOptions { From 2f5997d05dcd2d3da4ce6c0f45760b5c6c419f4c Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Fri, 5 Mar 2021 21:29:19 +0100 Subject: [PATCH 09/25] renamed script to check --- spec/Security.spec.js | 14 +++++++------- src/Security/Check.js | 16 ++++++++-------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/spec/Security.spec.js b/spec/Security.spec.js index a59a81ba28..1d17fb86f1 100644 --- a/spec/Security.spec.js +++ b/spec/Security.spec.js @@ -39,7 +39,7 @@ describe('Security Checks', () => { title: 'TestTitleSuccess', warning: 'TestWarning', solution: 'TestSolution', - script: () => { + check: () => { return true; } }); @@ -48,7 +48,7 @@ describe('Security Checks', () => { title: 'TestTitleFail', warning: 'TestWarning', solution: 'TestSolution', - script: () => { + check: () => { throw 'Fail'; } }); @@ -154,14 +154,14 @@ describe('Security Checks', () => { title: 'string', warning: 'string', solution: 'string', - script: () => {} + check: () => {} }, { group: 'string', title: 'string', warning: 'string', solution: 'string', - script: async () => {}, + check: async () => {}, }, ]; for (const config of configs) { @@ -175,7 +175,7 @@ describe('Security Checks', () => { title: [false, true, 0, 1, [], {}, () => {}], warning: [false, true, 0, 1, [], {}, () => {}], solution: [false, true, 0, 1, [], {}, () => {}], - script: [false, true, 0, 1, [], {}, 'string'], + check: [false, true, 0, 1, [], {}, 'string'], }; const configs = Utils.getObjectKeyPermutations(configDefinition); @@ -190,7 +190,7 @@ describe('Security Checks', () => { title: 'string', warning: 'string', solution: 'string', - script: () => {}, + check: () => {}, }); expect(check._checkState == CheckState.none); check.run(); @@ -203,7 +203,7 @@ describe('Security Checks', () => { title: 'string', warning: 'string', solution: 'string', - script: () => { throw 'error' }, + check: () => { throw 'error' }, }); expect(check._checkState == CheckState.none); check.run(); diff --git a/src/Security/Check.js b/src/Security/Check.js index 044bc21e0f..7853fe7cce 100644 --- a/src/Security/Check.js +++ b/src/Security/Check.js @@ -16,16 +16,16 @@ class Check { * @param {String} params.title The title. * @param {String} params.warning The warning message if the check fails. * @param {String} params.solution The solution to fix the check. - * @param {Promise} params.script The check script; can be an synchronous or asynchronous function. + * @param {Promise} params.check The check as synchronous or asynchronous function. */ constructor(params) { this._validateParams(params); - const { title, warning, solution, script } = params; + const { title, warning, solution, check } = params; this.title = title; this.warning = warning; this.solution = solution; - this.script = script; + this.check = check; // Set default properties this._checkState = CheckState.none; @@ -41,12 +41,12 @@ class Check { } async run() { - // Get check script as synchronous or asynchronous function - const script = this.script instanceof Promise ? await this.script : this.script; + // Get check as synchronous or asynchronous function + const check = this.check instanceof Promise ? await this.check : this.check; - // Run script + // Run check try { - script(); + check(); this._checkState = CheckState.success; } catch (e) { this.stateFailError = e; @@ -64,7 +64,7 @@ class Check { title: { t: 'string', v: isString }, warning: { t: 'string', v: isString }, solution: { t: 'string', v: isString }, - script: { t: 'function', v: isFunction }, + check: { t: 'function', v: isFunction }, }); } } From 4ceaf82435e1e9700be04af5b72c9101ca9e2cfb Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Fri, 5 Mar 2021 22:01:31 +0100 Subject: [PATCH 10/25] reformat log output --- src/Security/CheckRunner.js | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Security/CheckRunner.js b/src/Security/CheckRunner.js index 016956f0f5..3bf29b5f2f 100644 --- a/src/Security/CheckRunner.js +++ b/src/Security/CheckRunner.js @@ -142,7 +142,7 @@ class CheckRunner { // Traverse all groups and checks for compose output for (const group of report.report.groups) { - output += `${this._getLogIconForState(group.state)} ${group.name}:` + output += `- ${group.name}` for (const check of group.checks) { checksCount++; @@ -160,11 +160,17 @@ class CheckRunner { } output = - `Parse Server Security Check` + - `\n\n${failedChecksCount} weak security settings identified; ${checksCount} checks executed; ${skippedCheckCount} checks not executed.` + - `\n\n` + - output + - ``; + `\n###################################` + + `\n# #` + + `\n# Parse Server Security Check #` + + `\n# #` + + `\n###################################` + + `\n` + + `\n${failedChecksCount > 0 ? 'Warning: ' : ''}${failedChecksCount} weak security setting(s) found${failedChecksCount > 0 ? '!' : ''}` + + `\n${checksCount} check(s) executed` + + `\n${skippedCheckCount} check(s) skipped` + + `\n` + + `\n${output}\n`; // Write log log(output); From a4595ab8ba61d407290c1e96fc9b047a021cbdc0 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Fri, 5 Mar 2021 22:15:55 +0100 Subject: [PATCH 11/25] added server config check --- .../CheckGroups/CheckGroupServerConfig.js | 45 +++++++++++++++++++ src/Security/CheckGroups/CheckGroups.js | 1 + 2 files changed, 46 insertions(+) create mode 100644 src/Security/CheckGroups/CheckGroupServerConfig.js diff --git a/src/Security/CheckGroups/CheckGroupServerConfig.js b/src/Security/CheckGroups/CheckGroupServerConfig.js new file mode 100644 index 0000000000..484f94e589 --- /dev/null +++ b/src/Security/CheckGroups/CheckGroupServerConfig.js @@ -0,0 +1,45 @@ +/** + * @module SecurityCheck + */ + +import { Check } from '../Check'; +import CheckGroup from '../CheckGroup'; +import Config from '../../Config'; +import Parse from 'parse/node'; + +/** +* The security checks group for Parse Server configuration. +* Checks common Parse Server parameters such as access keys. +*/ +class CheckGroupServerConfig extends CheckGroup { + setName() { + return 'Parse Server Configuration'; + } + setChecks() { + const config = Config.get(Parse.applicationId); + return [ + new Check({ + title: 'Secure master key', + warning: 'The Parse Server master key is weak, which makes it vulnerable to a brute force attack.', + solution: 'Make the master key longer and a combination of upper- and lowercase character, numbers and special characters.', + check: () => { + const masterKey = config.masterKey; + const hasUpperCase = /[A-Z]/.test(masterKey); + const hasLowerCase = /[a-z]/.test(masterKey); + const hasNumbers = /\d/.test(masterKey); + const hasNonAlphasNumerics = /\W/.test(masterKey); + // Ensure length + if (masterKey.length < 14) { + throw 1; + } + // Ensure at least 3 out of 4 requirements passed + if (hasUpperCase + hasLowerCase + hasNumbers + hasNonAlphasNumerics < 3) { + throw 1; + } + }, + }) + ]; + } +} + +module.exports = CheckGroupServerConfig; diff --git a/src/Security/CheckGroups/CheckGroups.js b/src/Security/CheckGroups/CheckGroups.js index c177e253c3..598bcb7667 100644 --- a/src/Security/CheckGroups/CheckGroups.js +++ b/src/Security/CheckGroups/CheckGroups.js @@ -5,3 +5,4 @@ /** * The list of security check groups. */ +export { default as CheckGroupServerConfig } from './CheckGroupServerConfig'; From fb821bd952ad1cf6e61c75710875d5987735f882 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 00:23:26 +0100 Subject: [PATCH 12/25] improved contributing guideline --- CONTRIBUTING.md | 50 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8a8a4bd80c..8f60cd6dc2 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -13,6 +13,8 @@ - [Postgres with Docker](#postgres-with-docker) - [Feature Considerations](#feature-considerations) - [Security Checks](#security-checks) + - [Add Security Check](#add-security-check) + - [Wording Guideline](#wording-guideline) - [Parse Error](#parse-error) - [Parse Server Configuration](#parse-server-configuration) - [Code of Conduct](#code-of-conduct) @@ -162,7 +164,53 @@ A security check needs to be added for every new feature or enhancement that all For example, allowing public read and write to a class may be useful to simplify development but should be disallowed in a production environment. -Security checks are added in [SecurityChecks.js](https://github.com/parse-community/parse-server/blob/master/src/SecurityChecks.js). +Security checks are added in [CheckGroups](https://github.com/parse-community/parse-server/tree/master/src/Security/CheckGroups). + +#### Add Security Check +Adding a new security check for your feature is easy and fast: +1. Look into [CheckGroups](https://github.com/parse-community/parse-server/tree/master/src/Security/CheckGroups) whether there is an existing `CheckGroup[Category].js` file for the category of check to add. For example, a check regarding the database connection is added to `CheckGroupDatabase.js`. +2. If you did not find a file, duplicate an existing file and replace the category name in `setName()` and the checks in `setChecks()`: + ```js + class CheckGroupNewCategory extends CheckGroup { + setName() { + return 'House'; + } + setChecks() { + return [ + new Check({ + title: 'Door locked', + warning: 'Anyone can enter your house.', + solution: 'Lock your door.', + check: () => { + return; // Example of a passing check + } + }), + new Check({ + title: 'Camera online', + warning: 'Security camera is offline.', + solution: 'Check the camera.', + check: async () => { + throw 1; // Example of a failing check + } + }), + ]; + } + } + ``` + +3. If you added a new file in the previous step, reference the file in [CheckGroups.js](https://github.com/parse-community/parse-server/blob/master/src/Security/CheckGroups/CheckGroups.js), which is the collector of all security checks: + ``` + export { default as CheckGroupNewCategory } from './CheckGroupNewCategory'; + ``` + +#### Wording Guideline +Consider the following when adding a new security check: +- The wordings should be concise and not contain verbose explanations. Remember that these phrases contribute to data traffic and are therefore a cost factor that can become significant when scaling up. +- Do not use pronouns such as "you" or "your" because log files can have various readers with different roles. Do no use pronouns such as "I" or "me" because although we love it dearly, Parse Server is not a human. +- *Group.name*: The category name; ends without period as this is a headline. +- *Check.title*: Is the positive hypothesis that should be checked; ends without period as this is a title. +- *Check.warning*: The warning if the test fails; ends with period as this is a description. +- *Check.solution*: The recommended solution if the test fails; ends with period as this is an instruction. ### Parse Error From 3e0ff4285b23f829c01f0c46d084b86fe2a6c0cd Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 00:34:59 +0100 Subject: [PATCH 13/25] improved contribution guide --- CONTRIBUTING.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8f60cd6dc2..b981efaec5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -205,12 +205,13 @@ Adding a new security check for your feature is easy and fast: #### Wording Guideline Consider the following when adding a new security check: -- The wordings should be concise and not contain verbose explanations. Remember that these phrases contribute to data traffic and are therefore a cost factor that can become significant when scaling up. -- Do not use pronouns such as "you" or "your" because log files can have various readers with different roles. Do no use pronouns such as "I" or "me" because although we love it dearly, Parse Server is not a human. - *Group.name*: The category name; ends without period as this is a headline. - *Check.title*: Is the positive hypothesis that should be checked; ends without period as this is a title. - *Check.warning*: The warning if the test fails; ends with period as this is a description. - *Check.solution*: The recommended solution if the test fails; ends with period as this is an instruction. +- The wordings must not contain any sensitive information such as keys, as the security report may be exposed in logs. +- The wordings should be concise and not contain verbose explanations. Remember that these phrases contribute to data traffic and are therefore a cost factor that can become significant when scaling up. +- Do not use pronouns such as "you" or "your" because log files can have various readers with different roles. Do no use pronouns such as "I" or "me" because although we love it dearly, Parse Server is not a human. ### Parse Error From ec8a67bb4130693b0004e485fca672f7e340f1d2 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 00:53:02 +0100 Subject: [PATCH 14/25] added check security log --- src/Security/CheckGroups/CheckGroupServerConfig.js | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/Security/CheckGroups/CheckGroupServerConfig.js b/src/Security/CheckGroups/CheckGroupServerConfig.js index 484f94e589..42b5d3a744 100644 --- a/src/Security/CheckGroups/CheckGroupServerConfig.js +++ b/src/Security/CheckGroups/CheckGroupServerConfig.js @@ -20,8 +20,8 @@ class CheckGroupServerConfig extends CheckGroup { return [ new Check({ title: 'Secure master key', - warning: 'The Parse Server master key is weak, which makes it vulnerable to a brute force attack.', - solution: 'Make the master key longer and a combination of upper- and lowercase character, numbers and special characters.', + warning: 'The Parse Server master key is insecure and vulnerable to brute force attacks.', + solution: 'Choose a more complex master key with a combination of upper- and lowercase characters, numbers and special characters.', check: () => { const masterKey = config.masterKey; const hasUpperCase = /[A-Z]/.test(masterKey); @@ -37,6 +37,16 @@ class CheckGroupServerConfig extends CheckGroup { throw 1; } }, + }), + new Check({ + title: 'Security log disabled', + warning: 'Security report in log.', + solution: 'Set Parse Server configuration `security.enableCheckLog` to false.', + check: () => { + if (config.security && config.security.enableCheckLog) { + throw 1; + } + }, }) ]; } From 73b019525991f95f9160cbb4311f5cc175b33c67 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 00:53:10 +0100 Subject: [PATCH 15/25] improved log format --- src/Security/CheckRunner.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Security/CheckRunner.js b/src/Security/CheckRunner.js index 3bf29b5f2f..b569a2078c 100644 --- a/src/Security/CheckRunner.js +++ b/src/Security/CheckRunner.js @@ -150,8 +150,8 @@ class CheckRunner { if (check.state == CheckState.fail) { failedChecksCount++; - output += `\n${indent}${indent}${check.warning}`; - output += `\n${indent}${indent}${check.solution}`; + output += `\n${indent}${indent}Warning: ${check.warning}`; + output += ` ${check.solution}`; } else if (check.state == CheckState.none) { skippedCheckCount++; output += `\n${indent}${indent}Test did not execute, this is likely an internal server issue, please report.`; From a48383bc935eefeeca4660f08651eddd1feab47f Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 02:38:13 +0100 Subject: [PATCH 16/25] added checks --- .../CheckGroups/CheckGroupServerConfig.js | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Security/CheckGroups/CheckGroupServerConfig.js b/src/Security/CheckGroups/CheckGroupServerConfig.js index 42b5d3a744..2ef349af28 100644 --- a/src/Security/CheckGroups/CheckGroupServerConfig.js +++ b/src/Security/CheckGroups/CheckGroupServerConfig.js @@ -40,14 +40,24 @@ class CheckGroupServerConfig extends CheckGroup { }), new Check({ title: 'Security log disabled', - warning: 'Security report in log.', - solution: 'Set Parse Server configuration `security.enableCheckLog` to false.', + warning: 'Security checks in logs may expose vulnerabilities to anyone access to logs.', + solution: 'Change Parse Server configuration to \'security.enableCheckLog: false\'.', check: () => { if (config.security && config.security.enableCheckLog) { throw 1; } }, - }) + }), + new Check({ + title: 'Client class creation disabled', + warning: 'Attackers are allowed to create new classes without restriction and flood the database.', + solution: 'Change Parse Server configuration to \'allowClientClassCreation: false\'.', + check: () => { + if (config.allowClientClassCreation || config.allowClientClassCreation == null) { + throw 1; + } + }, + }), ]; } } From 5c0053fe3cb847bc1f107dabe199bdc0019faf4d Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 04:31:06 +0100 Subject: [PATCH 17/25] fixed log fomat typo --- src/Security/CheckRunner.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Security/CheckRunner.js b/src/Security/CheckRunner.js index b569a2078c..2e522fefcb 100644 --- a/src/Security/CheckRunner.js +++ b/src/Security/CheckRunner.js @@ -142,7 +142,7 @@ class CheckRunner { // Traverse all groups and checks for compose output for (const group of report.report.groups) { - output += `- ${group.name}` + output += `\n- ${group.name}` for (const check of group.checks) { checksCount++; @@ -170,7 +170,7 @@ class CheckRunner { `\n${checksCount} check(s) executed` + `\n${skippedCheckCount} check(s) skipped` + `\n` + - `\n${output}\n`; + `${output}`; // Write log log(output); From 60c6cbf7f2eae98665bc2adaf3e749e243f1b828 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 04:31:22 +0100 Subject: [PATCH 18/25] added database checks --- .../CheckGroups/CheckGroupDatabase.js | 63 +++++++++++++++++++ src/Security/CheckGroups/CheckGroups.js | 1 + 2 files changed, 64 insertions(+) create mode 100644 src/Security/CheckGroups/CheckGroupDatabase.js diff --git a/src/Security/CheckGroups/CheckGroupDatabase.js b/src/Security/CheckGroups/CheckGroupDatabase.js new file mode 100644 index 0000000000..d48cde7ff8 --- /dev/null +++ b/src/Security/CheckGroups/CheckGroupDatabase.js @@ -0,0 +1,63 @@ +/** + * @module SecurityCheck + */ + +import { Check } from '../Check'; +import CheckGroup from '../CheckGroup'; +import Config from '../../Config'; +import Parse from 'parse/node'; + +/** +* The security checks group for Parse Server configuration. +* Checks common Parse Server parameters such as access keys. +*/ +class CheckGroupDatabase extends CheckGroup { + setName() { + return 'Database'; + } + setChecks() { + const config = Config.get(Parse.applicationId); + const databaseAdapter = config.database.adapter; + const databaseUrl = databaseAdapter._uri; + const MongoClient = require('mongodb').MongoClient; + return [ + new Check({ + title: `Database requires authentication`, + warning: 'Database requires no authentication to connect which allows anyone to connect and potentially access data.', + solution: 'Change database access settings.', + check: async () => { + try { + const urlWithoutCredentials = databaseUrl.replace(/\/\/(\S+:\S+)@/, '//'); + const client = await MongoClient.connect(urlWithoutCredentials, { useNewUrlParser: true }); + await client.db("admin").command({ ping: 1 }); + throw 1; + } catch { + return; + } + }, + }), + new Check({ + title: 'Secure database password', + warning: 'The Parse Server master key is insecure and vulnerable to brute force attacks.', + solution: 'Choose a more complex master key with a combination of upper- and lowercase characters, numbers and special characters.', + check: () => { + const masterKey = config.masterKey; + const hasUpperCase = /[A-Z]/.test(masterKey); + const hasLowerCase = /[a-z]/.test(masterKey); + const hasNumbers = /\d/.test(masterKey); + const hasNonAlphasNumerics = /\W/.test(masterKey); + // Ensure length + if (masterKey.length < 14) { + throw 1; + } + // Ensure at least 3 out of 4 requirements passed + if (hasUpperCase + hasLowerCase + hasNumbers + hasNonAlphasNumerics < 3) { + throw 1; + } + }, + }), + ]; + } +} + +module.exports = CheckGroupDatabase; diff --git a/src/Security/CheckGroups/CheckGroups.js b/src/Security/CheckGroups/CheckGroups.js index 598bcb7667..d1c4f03d8c 100644 --- a/src/Security/CheckGroups/CheckGroups.js +++ b/src/Security/CheckGroups/CheckGroups.js @@ -5,4 +5,5 @@ /** * The list of security check groups. */ +export { default as CheckGroupDatabase } from './CheckGroupDatabase'; export { default as CheckGroupServerConfig } from './CheckGroupServerConfig'; From c84eb3b6a19f76957289b5ebd634ed2a58dbfd4d Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 04:38:56 +0100 Subject: [PATCH 19/25] fixed database check --- src/Security/CheckGroups/CheckGroupDatabase.js | 16 ++++++++-------- .../CheckGroups/CheckGroupServerConfig.js | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Security/CheckGroups/CheckGroupDatabase.js b/src/Security/CheckGroups/CheckGroupDatabase.js index d48cde7ff8..9323989a82 100644 --- a/src/Security/CheckGroups/CheckGroupDatabase.js +++ b/src/Security/CheckGroups/CheckGroupDatabase.js @@ -38,16 +38,16 @@ class CheckGroupDatabase extends CheckGroup { }), new Check({ title: 'Secure database password', - warning: 'The Parse Server master key is insecure and vulnerable to brute force attacks.', - solution: 'Choose a more complex master key with a combination of upper- and lowercase characters, numbers and special characters.', + warning: 'The database password is insecure and vulnerable to brute force attacks.', + solution: 'Choose a longer and/or more complex password with a combination of upper- and lowercase characters, numbers and special characters.', check: () => { - const masterKey = config.masterKey; - const hasUpperCase = /[A-Z]/.test(masterKey); - const hasLowerCase = /[a-z]/.test(masterKey); - const hasNumbers = /\d/.test(masterKey); - const hasNonAlphasNumerics = /\W/.test(masterKey); + const password = databaseUrl.match(/\/\/\S+:(\S+)@/)[1]; + const hasUpperCase = /[A-Z]/.test(password); + const hasLowerCase = /[a-z]/.test(password); + const hasNumbers = /\d/.test(password); + const hasNonAlphasNumerics = /\W/.test(password); // Ensure length - if (masterKey.length < 14) { + if (password.length < 14) { throw 1; } // Ensure at least 3 out of 4 requirements passed diff --git a/src/Security/CheckGroups/CheckGroupServerConfig.js b/src/Security/CheckGroups/CheckGroupServerConfig.js index 2ef349af28..a0dc41ec47 100644 --- a/src/Security/CheckGroups/CheckGroupServerConfig.js +++ b/src/Security/CheckGroups/CheckGroupServerConfig.js @@ -21,7 +21,7 @@ class CheckGroupServerConfig extends CheckGroup { new Check({ title: 'Secure master key', warning: 'The Parse Server master key is insecure and vulnerable to brute force attacks.', - solution: 'Choose a more complex master key with a combination of upper- and lowercase characters, numbers and special characters.', + solution: 'Choose a longer and/or more complex master key with a combination of upper- and lowercase characters, numbers and special characters.', check: () => { const masterKey = config.masterKey; const hasUpperCase = /[A-Z]/.test(masterKey); From a0277257fc252a54841a604408dfd2b7f4ec6b47 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 04:51:35 +0100 Subject: [PATCH 20/25] removed database auth check in initial version --- src/Security/CheckGroups/CheckGroupDatabase.js | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/Security/CheckGroups/CheckGroupDatabase.js b/src/Security/CheckGroups/CheckGroupDatabase.js index 9323989a82..d0da79a4ba 100644 --- a/src/Security/CheckGroups/CheckGroupDatabase.js +++ b/src/Security/CheckGroups/CheckGroupDatabase.js @@ -19,23 +19,7 @@ class CheckGroupDatabase extends CheckGroup { const config = Config.get(Parse.applicationId); const databaseAdapter = config.database.adapter; const databaseUrl = databaseAdapter._uri; - const MongoClient = require('mongodb').MongoClient; return [ - new Check({ - title: `Database requires authentication`, - warning: 'Database requires no authentication to connect which allows anyone to connect and potentially access data.', - solution: 'Change database access settings.', - check: async () => { - try { - const urlWithoutCredentials = databaseUrl.replace(/\/\/(\S+:\S+)@/, '//'); - const client = await MongoClient.connect(urlWithoutCredentials, { useNewUrlParser: true }); - await client.db("admin").command({ ping: 1 }); - throw 1; - } catch { - return; - } - }, - }), new Check({ title: 'Secure database password', warning: 'The database password is insecure and vulnerable to brute force attacks.', From d9f1b17b77cd02e27876ecb4efb30690631b13b4 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 05:39:09 +0100 Subject: [PATCH 21/25] improved contribution guide --- CONTRIBUTING.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b981efaec5..1172fd33a6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -202,6 +202,7 @@ Adding a new security check for your feature is easy and fast: ``` export { default as CheckGroupNewCategory } from './CheckGroupNewCategory'; ``` +4. Add a test that covers the new check to [SecurityCheckGroups.js](https://github.com/parse-community/parse-server/blob/master/spec/SecurityCheckGroups.js) for the cases of success and failure. #### Wording Guideline Consider the following when adding a new security check: From fbc099781c96b45291d84eb8ffa82b92831c3b56 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 05:39:40 +0100 Subject: [PATCH 22/25] added security check tests --- ...Security.spec.js => SecurityCheck.spec.js} | 2 +- spec/SecurityCheckGroups.spec.js | 81 +++++++++++++++++++ 2 files changed, 82 insertions(+), 1 deletion(-) rename spec/{Security.spec.js => SecurityCheck.spec.js} (99%) create mode 100644 spec/SecurityCheckGroups.spec.js diff --git a/spec/Security.spec.js b/spec/SecurityCheck.spec.js similarity index 99% rename from spec/Security.spec.js rename to spec/SecurityCheck.spec.js index 1d17fb86f1..5f79ca2bbd 100644 --- a/spec/Security.spec.js +++ b/spec/SecurityCheck.spec.js @@ -9,7 +9,7 @@ const CheckGroup = require('../lib/Security/CheckGroup'); const CheckRunner = require('../lib/Security/CheckRunner'); const CheckGroups = require('../lib/Security/CheckGroups/CheckGroups'); -describe('Security Checks', () => { +describe('Security Check', () => { let Group; let groupName; let checkSuccess; diff --git a/spec/SecurityCheckGroups.spec.js b/spec/SecurityCheckGroups.spec.js new file mode 100644 index 0000000000..43d523c214 --- /dev/null +++ b/spec/SecurityCheckGroups.spec.js @@ -0,0 +1,81 @@ +'use strict'; + +const Config = require('../lib/Config'); +const { CheckState } = require('../lib/Security/Check'); +const CheckGroupServerConfig = require('../lib/Security/CheckGroups/CheckGroupServerConfig'); +const CheckGroupDatabase = require('../lib/Security/CheckGroups/CheckGroupDatabase'); + +describe('Security Check Groups', () => { + let config; + + beforeEach(async () => { + config = { + appId: 'test', + appName: 'ExampleAppName', + publicServerURL: 'http://localhost:8378/1', + security: { + enableCheck: true, + enableCheckLog: false, + }, + }; + await reconfigureServer(config); + }); + + describe('CheckGroupServerConfig', () => { + it('is subclassed correctly', async () => { + const group = new CheckGroupServerConfig(); + expect(group.name()).toBeDefined(); + expect(group.checks().length).toBeGreaterThan(0); + }); + + it('checks succeed correctly', async () => { + config.masterKey = 'aMoreSecur3Passwor7!'; + config.security.enableCheckLog = false; + config.allowClientClassCreation = false; + await reconfigureServer(config); + + const group = new CheckGroupServerConfig(); + await group.run(); + expect(group.checks()[0].checkState()).toBe(CheckState.success); + expect(group.checks()[1].checkState()).toBe(CheckState.success); + expect(group.checks()[2].checkState()).toBe(CheckState.success); + }); + + it('checks fail correctly', async () => { + config.masterKey = 'insecure'; + config.security.enableCheckLog = true; + config.allowClientClassCreation = true; + await reconfigureServer(config); + + const group = new CheckGroupServerConfig(); + await group.run(); + expect(group.checks()[0].checkState()).toBe(CheckState.fail); + expect(group.checks()[1].checkState()).toBe(CheckState.fail); + expect(group.checks()[2].checkState()).toBe(CheckState.fail); + }); + }); + + describe('CheckGroupDatabase', () => { + it('is subclassed correctly', async () => { + const group = new CheckGroupDatabase(); + expect(group.name()).toBeDefined(); + expect(group.checks().length).toBeGreaterThan(0); + }); + + it('checks succeed correctly', async () => { + const config = Config.get(Parse.applicationId); + config.database.adapter._uri = 'protocol://user:aMoreSecur3Passwor7!@example.com'; + const group = new CheckGroupDatabase(); + await group.run(); + expect(group.checks()[0].checkState()).toBe(CheckState.success); + }); + + it('checks fail correctly', async () => { + const config = Config.get(Parse.applicationId); + config.database.adapter._uri = 'protocol://user:insecure@example.com'; + const group = new CheckGroupDatabase(); + await group.run(); + expect(group.checks()[0].checkState()).toBe(CheckState.fail); + }); + }); +}); From 02875dacfd8f8006b16fe063f13e6f1b8ff02ba4 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Sat, 6 Mar 2021 06:01:50 +0100 Subject: [PATCH 23/25] fixed typo --- CONTRIBUTING.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1172fd33a6..67fd22f5b7 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -212,7 +212,7 @@ Consider the following when adding a new security check: - *Check.solution*: The recommended solution if the test fails; ends with period as this is an instruction. - The wordings must not contain any sensitive information such as keys, as the security report may be exposed in logs. - The wordings should be concise and not contain verbose explanations. Remember that these phrases contribute to data traffic and are therefore a cost factor that can become significant when scaling up. -- Do not use pronouns such as "you" or "your" because log files can have various readers with different roles. Do no use pronouns such as "I" or "me" because although we love it dearly, Parse Server is not a human. +- Do not use pronouns such as "you" or "your" because log files can have various readers with different roles. Do not use pronouns such as "I" or "me" because although we love it dearly, Parse Server is not a human. ### Parse Error From 38bdda793308eadb3ef5a81a8e3d94727227731d Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Wed, 10 Mar 2021 17:31:42 +0100 Subject: [PATCH 24/25] improved wording guidelines --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 67fd22f5b7..b38f5586d5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -207,11 +207,11 @@ Adding a new security check for your feature is easy and fast: #### Wording Guideline Consider the following when adding a new security check: - *Group.name*: The category name; ends without period as this is a headline. -- *Check.title*: Is the positive hypothesis that should be checked; ends without period as this is a title. +- *Check.title*: Is the positive hypothesis that should be checked (for example "Door locked" instead of "Door unlocked"); ends without period as this is a title. - *Check.warning*: The warning if the test fails; ends with period as this is a description. - *Check.solution*: The recommended solution if the test fails; ends with period as this is an instruction. - The wordings must not contain any sensitive information such as keys, as the security report may be exposed in logs. -- The wordings should be concise and not contain verbose explanations. Remember that these phrases contribute to data traffic and are therefore a cost factor that can become significant when scaling up. +- The wordings should be concise and not contain verbose explanations. - Do not use pronouns such as "you" or "your" because log files can have various readers with different roles. Do not use pronouns such as "I" or "me" because although we love it dearly, Parse Server is not a human. ### Parse Error From d9aeb73fb11b38f4fe2cf4a5de5ef49a1f58b200 Mon Sep 17 00:00:00 2001 From: Manuel Trezza <5673677+mtrezza@users.noreply.github.com> Date: Wed, 10 Mar 2021 17:39:12 +0100 Subject: [PATCH 25/25] improved wording guidelines --- CONTRIBUTING.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b38f5586d5..bd2955430f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -180,7 +180,7 @@ Adding a new security check for your feature is easy and fast: new Check({ title: 'Door locked', warning: 'Anyone can enter your house.', - solution: 'Lock your door.', + solution: 'Lock the door.', check: () => { return; // Example of a passing check } @@ -207,11 +207,11 @@ Adding a new security check for your feature is easy and fast: #### Wording Guideline Consider the following when adding a new security check: - *Group.name*: The category name; ends without period as this is a headline. -- *Check.title*: Is the positive hypothesis that should be checked (for example "Door locked" instead of "Door unlocked"); ends without period as this is a title. +- *Check.title*: Is the positive hypothesis that should be checked, for example "Door locked" instead of "Door unlocked"; ends without period as this is a title. - *Check.warning*: The warning if the test fails; ends with period as this is a description. - *Check.solution*: The recommended solution if the test fails; ends with period as this is an instruction. - The wordings must not contain any sensitive information such as keys, as the security report may be exposed in logs. -- The wordings should be concise and not contain verbose explanations. +- The wordings should be concise and not contain verbose explanations, for example "Door locked" instead of "Door has been locked securely". - Do not use pronouns such as "you" or "your" because log files can have various readers with different roles. Do not use pronouns such as "I" or "me" because although we love it dearly, Parse Server is not a human. ### Parse Error