From 484c2e81cace608eb28508a0926b17b3ba9e6233 Mon Sep 17 00:00:00 2001 From: dblythy Date: Fri, 8 Oct 2021 14:24:20 +1100 Subject: [PATCH] fix: improve security by deprecating creating users with public access by default (#7319) --- CHANGELOG.md | 1 + DEPRECATIONS.md | 1 + spec/ParseUser.spec.js | 51 +++++++++++++------ src/Config.js | 8 +++ src/Deprecator/Deprecations.js | 9 ++-- src/Options/Definitions.js | 6 +++ src/Options/docs.js | 1 + src/Options/index.js | 3 ++ src/RestWrite.js | 4 +- .../CheckGroups/CheckGroupServerConfig.js | 30 ++++++++--- 10 files changed, 86 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 635777d7f1..0a5f6f8dfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -114,6 +114,7 @@ ___ - Added Deprecation Policy to govern the introduction of breaking changes in a phased pattern that is more predictable for developers (Manuel Trezza) [#7199](https://github.com/parse-community/parse-server/pull/7199) - Add REST API endpoint `/loginAs` to create session of any user with master key; allows to impersonate another user. (GormanFletcher) [#7406](https://github.com/parse-community/parse-server/pull/7406) - Add official support for MongoDB 5.0 (Manuel Trezza) [#7469](https://github.com/parse-community/parse-server/pull/7469) +- Added Parse Server Configuration `enforcePrivateUsers`, which will remove public access by default on new Parse.Users (dblythy) [#7319](https://github.com/parse-community/parse-server/pull/7319) ### Other Changes - Support native mongodb syntax in aggregation pipelines (Raschid JF Rafeally) [#7339](https://github.com/parse-community/parse-server/pull/7339) diff --git a/DEPRECATIONS.md b/DEPRECATIONS.md index 87a4e3b927..05ef29bc75 100644 --- a/DEPRECATIONS.md +++ b/DEPRECATIONS.md @@ -6,6 +6,7 @@ The following is a list of deprecations, according to the [Deprecation Policy](h |--------|-------------------------------------------------|----------------------------------------------------------------------|---------------------------------|---------------------------------|-----------------------|-------| | DEPPS1 | Native MongoDB syntax in aggregation pipeline | [#7338](https://github.com/parse-community/parse-server/issues/7338) | 5.0.0 (2022) | 6.0.0 (2023) | deprecated | - | | DEPPS2 | Config option `directAccess` defaults to `true` | [#6636](https://github.com/parse-community/parse-server/pull/6636) | 5.0.0 (2022) | 6.0.0 (2023) | deprecated | - | +| DEPPS3 | Config option `enforcePrivateUsers` defaults to `true` | [#7319](https://github.com/parse-community/parse-server/pull/7319) | 5.0.0 (2022) | 6.0.0 (2023) | deprecated | - | [i_deprecation]: ## "The version and date of the deprecation." [i_removal]: ## "The version and date of the planned removal." diff --git a/spec/ParseUser.spec.js b/spec/ParseUser.spec.js index ee02f88a50..ebdee6a191 100644 --- a/spec/ParseUser.spec.js +++ b/spec/ParseUser.spec.js @@ -13,20 +13,6 @@ const passwordCrypto = require('../lib/password'); const Config = require('../lib/Config'); const cryptoUtils = require('../lib/cryptoUtils'); -function verifyACL(user) { - const ACL = user.getACL(); - expect(ACL.getReadAccess(user)).toBe(true); - expect(ACL.getWriteAccess(user)).toBe(true); - expect(ACL.getPublicReadAccess()).toBe(true); - expect(ACL.getPublicWriteAccess()).toBe(false); - const perms = ACL.permissionsById; - expect(Object.keys(perms).length).toBe(2); - expect(perms[user.id].read).toBe(true); - expect(perms[user.id].write).toBe(true); - expect(perms['*'].read).toBe(true); - expect(perms['*'].write).not.toBe(true); -} - describe('Parse.User testing', () => { it('user sign up class method', async done => { const user = await Parse.User.signUp('asdf', 'zxcv'); @@ -146,7 +132,17 @@ describe('Parse.User testing', () => { await Parse.User.signUp('asdf', 'zxcv'); const user = await Parse.User.logIn('asdf', 'zxcv'); equal(user.get('username'), 'asdf'); - verifyACL(user); + const ACL = user.getACL(); + expect(ACL.getReadAccess(user)).toBe(true); + expect(ACL.getWriteAccess(user)).toBe(true); + expect(ACL.getPublicReadAccess()).toBe(true); + expect(ACL.getPublicWriteAccess()).toBe(false); + const perms = ACL.permissionsById; + expect(Object.keys(perms).length).toBe(2); + expect(perms[user.id].read).toBe(true); + expect(perms[user.id].write).toBe(true); + expect(perms['*'].read).toBe(true); + expect(perms['*'].write).not.toBe(true); done(); }); @@ -3934,6 +3930,31 @@ describe('Parse.User testing', () => { } }); + it('should throw when enforcePrivateUsers is invalid', async () => { + const options = [[], 'a', 0, {}]; + for (const option of options) { + await expectAsync(reconfigureServer({ enforcePrivateUsers: option })).toBeRejected(); + } + }); + + it('user login with enforcePrivateUsers', async done => { + await reconfigureServer({ enforcePrivateUsers: true }); + await Parse.User.signUp('asdf', 'zxcv'); + const user = await Parse.User.logIn('asdf', 'zxcv'); + equal(user.get('username'), 'asdf'); + const ACL = user.getACL(); + expect(ACL.getReadAccess(user)).toBe(true); + expect(ACL.getWriteAccess(user)).toBe(true); + expect(ACL.getPublicReadAccess()).toBe(false); + expect(ACL.getPublicWriteAccess()).toBe(false); + const perms = ACL.permissionsById; + expect(Object.keys(perms).length).toBe(1); + expect(perms[user.id].read).toBe(true); + expect(perms[user.id].write).toBe(true); + expect(perms['*']).toBeUndefined(); + done(); + }); + describe('issue #4897', () => { it_only_db('mongo')('should be able to login with a legacy user (no ACL)', async () => { // This issue is a side effect of the locked users and legacy users which don't have ACL's diff --git a/src/Config.js b/src/Config.js index 302347c5ed..250880efbc 100644 --- a/src/Config.js +++ b/src/Config.js @@ -75,6 +75,7 @@ export class Config { fileUpload, pages, security, + enforcePrivateUsers, }) { if (masterKey === readOnlyMasterKey) { throw new Error('masterKey and readOnlyMasterKey should be different'); @@ -111,6 +112,13 @@ export class Config { this.validateIdempotencyOptions(idempotencyOptions); this.validatePagesOptions(pages); this.validateSecurityOptions(security); + this.validateEnforcePrivateUsers(enforcePrivateUsers); + } + + static validateEnforcePrivateUsers(enforcePrivateUsers) { + if (typeof enforcePrivateUsers !== 'boolean') { + throw 'Parse Server option enforcePrivateUsers must be a boolean.'; + } } static validateSecurityOptions(security) { diff --git a/src/Deprecator/Deprecations.js b/src/Deprecator/Deprecations.js index e32b305fb0..eb2212bf92 100644 --- a/src/Deprecator/Deprecations.js +++ b/src/Deprecator/Deprecations.js @@ -2,11 +2,11 @@ * The deprecations. * * Add deprecations to the array using the following keys: - * - `optionKey`: The option key incl. its path, e.g. `security.enableCheck`. - * - `envKey`: The environment key, e.g. `PARSE_SERVER_SECURITY`. - * - `changeNewKey`: Set the new key name if the current key will be replaced, + * - `optionKey` {String}: The option key incl. its path, e.g. `security.enableCheck`. + * - `envKey` {String}: The environment key, e.g. `PARSE_SERVER_SECURITY`. + * - `changeNewKey` {String}: Set the new key name if the current key will be replaced, * or set to an empty string if the current key will be removed without replacement. - * - `changeNewDefault`: Set the new default value if the key's default value + * - `changeNewDefault` {String}: Set the new default value if the key's default value * will change in a future version. * - `solution`: The instruction to resolve this deprecation warning. Optional. This * instruction must not include the deprecation warning which is auto-generated. @@ -22,4 +22,5 @@ module.exports = [ solution: "Additionally, the environment variable 'PARSE_SERVER_ENABLE_EXPERIMENTAL_DIRECT_ACCESS' will be deprecated and renamed to 'PARSE_SERVER_DIRECT_ACCESS' in a future version; it is currently possible to use either one.", }, + { optionKey: 'enforcePrivateUsers', changeNewDefault: 'true' }, ]; diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index 53067a4399..88779376c2 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -154,6 +154,12 @@ module.exports.ParseServerOptions = { env: 'PARSE_SERVER_ENCRYPTION_KEY', help: 'Key for encrypting your files', }, + enforcePrivateUsers: { + env: 'PARSE_SERVER_ENFORCE_PRIVATE_USERS', + help: 'Set to true if new users should be created without public read and write access.', + action: parsers.booleanParser, + default: false, + }, expireInactiveSessions: { env: 'PARSE_SERVER_EXPIRE_INACTIVE_SESSIONS', help: diff --git a/src/Options/docs.js b/src/Options/docs.js index c3dc43afc4..c6b2e1abf1 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -28,6 +28,7 @@ * @property {Boolean} enableAnonymousUsers Enable (or disable) anonymous users, defaults to true * @property {Boolean} enableExpressErrorHandler Enables the default express error handler for all errors * @property {String} encryptionKey Key for encrypting your files + * @property {Boolean} enforcePrivateUsers Set to true if new users should be created without public read and write access. * @property {Boolean} expireInactiveSessions Sets whether we should expire the inactive sessions, defaults to true. If false, all new sessions are created with no expiration date. * @property {String} fileKey Key for your files * @property {Adapter} filesAdapter Adapter module for the files sub-system diff --git a/src/Options/index.js b/src/Options/index.js index 552eb2e782..34fa5198c2 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -246,6 +246,9 @@ export interface ParseServerOptions { /* The security options to identify and report weak security settings. :DEFAULT: {} */ security: ?SecurityOptions; + /* Set to true if new users should be created without public read and write access. + :DEFAULT: false */ + enforcePrivateUsers: ?boolean; } export interface SecurityOptions { diff --git a/src/RestWrite.js b/src/RestWrite.js index 360c7e0dc0..c1f7ca2a26 100644 --- a/src/RestWrite.js +++ b/src/RestWrite.js @@ -1408,7 +1408,9 @@ RestWrite.prototype.runDatabaseOperation = function () { // default public r/w ACL if (!ACL) { ACL = {}; - ACL['*'] = { read: true, write: false }; + if (!this.config.enforcePrivateUsers) { + ACL['*'] = { read: true, write: false }; + } } // make sure the user is not locked down ACL[this.data.objectId] = { read: true, write: true }; diff --git a/src/Security/CheckGroups/CheckGroupServerConfig.js b/src/Security/CheckGroups/CheckGroupServerConfig.js index a0dc41ec47..a9c6e671cc 100644 --- a/src/Security/CheckGroups/CheckGroupServerConfig.js +++ b/src/Security/CheckGroups/CheckGroupServerConfig.js @@ -8,9 +8,9 @@ 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. -*/ + * 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'; @@ -21,7 +21,8 @@ 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 longer and/or 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); @@ -40,8 +41,9 @@ class CheckGroupServerConfig extends CheckGroup { }), new Check({ title: 'Security log disabled', - warning: 'Security checks in logs may expose vulnerabilities to anyone access to logs.', - solution: 'Change Parse Server configuration to \'security.enableCheckLog: false\'.', + warning: + 'Security checks in logs may expose vulnerabilities to anyone with access to logs.', + solution: "Change Parse Server configuration to 'security.enableCheckLog: false'.", check: () => { if (config.security && config.security.enableCheckLog) { throw 1; @@ -50,14 +52,26 @@ class CheckGroupServerConfig extends CheckGroup { }), 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\'.', + 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; } }, }), + new Check({ + title: 'Users are created without public access', + warning: + 'Users with public read access are exposed to anyone who knows their object IDs, or to anyone who can query the Parse.User class.', + solution: "Change Parse Server configuration to 'enforcePrivateUsers: true'.", + check: () => { + if (!config.enforcePrivateUsers) { + throw 1; + } + }, + }), ]; } }