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

refactor(query): deprecate explain without master key #7520

Closed
wants to merge 1 commit into from
Closed
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 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
|-------------------------------------------------|----------------------------------------------------------------------|---------------------------------|---------------------------------|-----------------------|-------|
| 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."
Expand Down
52 changes: 52 additions & 0 deletions spec/ParseQuery.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
});
5 changes: 5 additions & 0 deletions src/Deprecator/Deprecations.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
];
6 changes: 6 additions & 0 deletions src/Options/Definitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/Options/docs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions src/Options/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) */
Expand Down Expand Up @@ -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 */
Expand Down
6 changes: 6 additions & 0 deletions src/rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down