Skip to content

Commit

Permalink
fix: improve security by deprecating creating users with public acces…
Browse files Browse the repository at this point in the history
…s by default (#7319)
  • Loading branch information
dblythy committed Oct 8, 2021
1 parent 2b5bf22 commit 484c2e8
Show file tree
Hide file tree
Showing 10 changed files with 86 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions DEPRECATIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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."
Expand Down
51 changes: 36 additions & 15 deletions spec/ParseUser.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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();
});

Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions src/Config.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export class Config {
fileUpload,
pages,
security,
enforcePrivateUsers,
}) {
if (masterKey === readOnlyMasterKey) {
throw new Error('masterKey and readOnlyMasterKey should be different');
Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 5 additions & 4 deletions src/Deprecator/Deprecations.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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' },
];
6 changes: 6 additions & 0 deletions src/Options/Definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions src/Options/docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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>} filesAdapter Adapter module for the files sub-system
Expand Down
3 changes: 3 additions & 0 deletions src/Options/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 3 additions & 1 deletion src/RestWrite.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
30 changes: 22 additions & 8 deletions src/Security/CheckGroups/CheckGroupServerConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);
Expand All @@ -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;
Expand All @@ -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;
}
},
}),
];
}
}
Expand Down

0 comments on commit 484c2e8

Please sign in to comment.