Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve security by deprecating creating users with public access by default #7319

Merged
merged 17 commits into from
Oct 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
}

dblythy marked this conversation as resolved.
Show resolved Hide resolved
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