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 #7521

Open
wants to merge 5 commits into
base: alpha
Choose a base branch
from
Open
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 @@ -108,6 +108,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)
- Deprecated ``explain` queries run by non-master users (Kartal Kaan Bozdogan) [#7519](https://github.com/parse-community/parse-server/issues/7519)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rephrase this as:

Deprecated executing explain queries without masterkey

- Add issue bot (Manuel Trezza) [#7523](https://github.com/parse-community/parse-server/pull/7523)

### Other Changes
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
|-------------------------------------------------|----------------------------------------------------------------------|---------------------------------|---------------------------------|-----------------------|-------|
| 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 | - |
| `explain` queries used by non-master users | [#7519](https://github.com/parse-community/parse-server/issues/7519) | 5.0.0 (2022) | 6.0.0 (2023) | deprecated | - |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rephrase that as

Execute explain queries without masterkey

And can you add an ID for this deprecation, after you merge master into this PR


[i_deprecation]: ## "The version and date of the deprecation."
[i_removal]: ## "The version and date of the planned removal."
Expand Down
32 changes: 32 additions & 0 deletions spec/ParseQuery.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5218,4 +5218,36 @@ describe('Parse.Query testing', () => {
// Validate
expect(result.executionStats).not.toBeUndefined();
});

xit('users cannot use explain queries', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this is in preparation for after the deprecation?

Could you add a short comment above?

Deprecation DEPPS3: enable this test after deprecation

Assuming that DEPPS3 is the ID of this deprecation

// Create an object
const obj = new TestObject({ foo: 'baz', hello: 'world' });
await obj.save();
// Query TestObject with explain.
const query = new Parse.Query('TestObject');
query.equalTo('objectId', obj.id);
query.explain();
try {
await query.find();
fail('even non-master users can use explain');
} 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('even non-master users can use explain');
} catch (e) {
equal(e.code, Parse.Error.OPERATION_FORBIDDEN);
equal(e.message, 'Cannot explain');
}
}).pend('Disabled until non-master explains are disabled');
it('the master key can use explain queries', async () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

newline between tests

const obj = new TestObject({ foo: 'baz', hello: 'world' });
await obj.save();
const query = new Parse.Query('TestObject');
query.equalTo('objectId', obj.id);
query.explain();
await query.find({ useMasterKey: true }); // Must not throw
});
});
13 changes: 13 additions & 0 deletions src/rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ var Parse = require('parse/node').Parse;
var RestQuery = require('./RestQuery');
var RestWrite = require('./RestWrite');
var triggers = require('./triggers');
import Deprecator from './Deprecator/Deprecator';

function checkTriggers(className, config, types) {
return types.some(triggerType => {
Expand All @@ -26,6 +27,12 @@ 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 (restOptions && restOptions.explain && !auth.isMaster) {
//throw new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Cannot explain');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a comment here please, similar to the one above Deprecation DEPPS3: ...

Deprecator.logRuntimeDeprecation({
usage: 'The use of explain queries by non-master users',
});
}
return triggers
.maybeRunQueryTrigger(
triggers.Types.beforeFind,
Expand Down Expand Up @@ -57,6 +64,12 @@ 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 (restOptions && restOptions.explain && !auth.isMaster) {
//throw new Parse.Error(Parse.Error.OPERATION_FORBIDDEN, 'Cannot explain');
Deprecator.logRuntimeDeprecation({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you also add a comment here please, similar to the one above Deprecation DEPPS3: ...

usage: 'The use of explain queries by non-master users',
});
}
return triggers
.maybeRunQueryTrigger(
triggers.Types.beforeFind,
Expand Down