From 00363b41c692a122961855b228e1808b7c540b07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kartal=20Kaan=20Bozdo=C4=9Fan?= Date: Tue, 24 Aug 2021 14:38:52 +0200 Subject: [PATCH] Added a new server option, nonMasterExplain Manages whether non-master users are allowed to use the "explain" parameter on queries Currently defaults to true, for backwards compatibility. However, this behaviour is deprecated, and at some point will default to false. Added tests for it --- DEPRECATIONS.md | 1 + spec/ParseQuery.spec.js | 52 ++++++++++++++++++++++++++++++++++ src/Deprecator/Deprecations.js | 5 ++++ src/Options/Definitions.js | 6 ++++ src/Options/docs.js | 1 + src/Options/index.js | 4 +++ src/rest.js | 6 ++++ 7 files changed, 75 insertions(+) diff --git a/DEPRECATIONS.md b/DEPRECATIONS.md index cf220a0054..c7d0c128ca 100644 --- a/DEPRECATIONS.md +++ b/DEPRECATIONS.md @@ -6,6 +6,7 @@ The following is a list of deprecations, according to the [Deprecation Policy](h |-------------------------------------------------|----------------------------------------------------------------------|---------------------------------|---------------------------------|-----------------------|-------| | 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 | - | | 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 | - | +| Config option `nonMasterExplain` defaults to `false` | [#7519](https://github.com/parse-community/parse-server/issues/7519) | XXX | XXX | deprecated | - | [i_deprecation]: ## "The version and date of the deprecation." [i_removal]: ## "The version and date of the planned removal." diff --git a/spec/ParseQuery.spec.js b/spec/ParseQuery.spec.js index e196280a5c..5035a23fe0 100644 --- a/spec/ParseQuery.spec.js +++ b/spec/ParseQuery.spec.js @@ -5218,4 +5218,56 @@ describe('Parse.Query testing', () => { // Validate expect(result.executionStats).not.toBeUndefined(); }); + + it('users cannot user explain unless nonMasterExplain is set', async () => { + // Create an object + const obj = new TestObject({ foo: 'baz', hello: 'world' }); + await obj.save(); + // Explicitly allow non-master explain + await reconfigureServer({ nonMasterExplain: true }); + // Query TestObject with explain. + let query = new Parse.Query('TestObject'); + query.equalTo('objectId', obj.id); + query.explain(); + let result = await query.find(); // Must not throw + // Explicitly disallow non-master explain + await reconfigureServer({ nonMasterExplain: false }); + try { + await query.find(); + fail('users can use explain even if nonMasterExplain is set to false'); + } catch (e) { + equal(e.code, Parse.Error.OPERATION_FORBIDDEN); + equal(e.message, 'Cannot explain'); + } + try { + await new Parse.Query('TestObject').explain().get(obj.id); + fail('users can use explain even if nonMasterExplain is set to false'); + } catch (e) { + equal(e.code, Parse.Error.OPERATION_FORBIDDEN); + equal(e.message, 'Cannot explain'); + } + // Non-explain queries should still work, of course + query = new Parse.Query('TestObject'); + query.equalTo('objectId', obj.id); + result = await query.find(); + equal(result.length, 1); + equal(result[0].id, obj.id); + }); + it('the master key can use explain no matter nonMasterExplain', async () => { + const obj = new TestObject({ foo: 'baz', hello: 'world' }); + await obj.save(); + const queryWithExplain = new Parse.Query('TestObject'); + queryWithExplain.equalTo('objectId', obj.id); + queryWithExplain.explain(); + const queryWoExplain = new Parse.Query('TestObject'); + queryWoExplain.equalTo('objectId', obj.id); + // Explicitly disallow non-master explain + await reconfigureServer({ nonMasterExplain: false }); + await queryWithExplain.find({ useMasterKey: true }); // Must not throw + await queryWoExplain.find({ useMasterKey: true }); // Must not throw + // Explicitly allow non-master explain + await reconfigureServer({ nonMasterExplain: true }); + await queryWithExplain.find({ useMasterKey: true }); // Must not throw + await queryWoExplain.find({ useMasterKey: true }); // Must not throw + }); }); diff --git a/src/Deprecator/Deprecations.js b/src/Deprecator/Deprecations.js index e32b305fb0..4620f0aa05 100644 --- a/src/Deprecator/Deprecations.js +++ b/src/Deprecator/Deprecations.js @@ -22,4 +22,9 @@ 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: 'nonMasterExplain', + envKey: 'PARSE_SERVER_NON_MASTER_EXPLAIN', + changeNewDefault: 'false', + }, ]; diff --git a/src/Options/Definitions.js b/src/Options/Definitions.js index 65799f8191..8a988bc761 100644 --- a/src/Options/Definitions.js +++ b/src/Options/Definitions.js @@ -277,6 +277,12 @@ module.exports.ParseServerOptions = { action: parsers.booleanParser, default: false, }, + nonMasterExplain: { + env: 'PARSE_SERVER_NON_MASTER_EXPLAIN', + help: 'Allow non-master users to use the `explain` query parameter.', + action: parsers.booleanParser, + default: true, + }, objectIdSize: { env: 'PARSE_SERVER_OBJECT_ID_SIZE', help: "Sets the number of characters in generated object id's, default 10", diff --git a/src/Options/docs.js b/src/Options/docs.js index 30b3aba1a0..6ec3dafe71 100644 --- a/src/Options/docs.js +++ b/src/Options/docs.js @@ -52,6 +52,7 @@ * @property {Boolean} mountGraphQL Mounts the GraphQL endpoint * @property {String} mountPath Mount path for the server, defaults to /parse * @property {Boolean} mountPlayground Mounts the GraphQL Playground - never use this option in production + * @property {Boolean} nonMasterExplain Allow non-master users to use the `explain` query parameter, defaults to true * @property {Number} objectIdSize Sets the number of characters in generated object id's, default 10 * @property {PagesOptions} pages The options for pages such as password reset and email verification. Caution, this is an experimental feature that may not be appropriate for production. * @property {PasswordPolicyOptions} passwordPolicy The password policy for enforcing password related rules. diff --git a/src/Options/index.js b/src/Options/index.js index 5f3d9afa47..aa44f76907 100644 --- a/src/Options/index.js +++ b/src/Options/index.js @@ -209,6 +209,7 @@ export interface ParseServerOptions { cluster: ?NumberOrBoolean; /* middleware for express server, can be string or function */ middleware: ?((() => void) | string); + /* Starts the liveQuery server */ startLiveQueryServer: ?boolean; /* Live query server configuration options (will start the liveQuery server) */ @@ -239,6 +240,9 @@ export interface ParseServerOptions { :ENV: PARSE_SERVER_PLAYGROUND_PATH :DEFAULT: /playground */ playgroundPath: ?string; + /* Allow non-master users to use the `explain` query parameter. + :DEFAULT: true */ + nonMasterExplain: ?boolean; /* Callback when server has started */ serverStartComplete: ?(error: ?Error) => void; /* Callback when server has closed */ diff --git a/src/rest.js b/src/rest.js index fca3497a5d..273c108690 100644 --- a/src/rest.js +++ b/src/rest.js @@ -26,6 +26,9 @@ function checkLiveQuery(className, config) { // Returns a promise for an object with optional keys 'results' and 'count'. function find(config, auth, className, restWhere, restOptions, clientSDK, context) { enforceRoleSecurity('find', className, auth); + if (!config.nonMasterExplain && restOptions.explain && !auth.isMaster) { + throw new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Cannot explain'); + } return triggers .maybeRunQueryTrigger( triggers.Types.beforeFind, @@ -57,6 +60,9 @@ function find(config, auth, className, restWhere, restOptions, clientSDK, contex const get = (config, auth, className, objectId, restOptions, clientSDK, context) => { var restWhere = { objectId }; enforceRoleSecurity('get', className, auth); + if (!config.nonMasterExplain && restOptions.explain && !auth.isMaster) { + throw new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Cannot explain'); + } return triggers .maybeRunQueryTrigger( triggers.Types.beforeFind,