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: live query role cache does not clear when a user is added to a role #8026

Merged
merged 20 commits into from
Jun 11, 2022

Conversation

dblythy
Copy link
Member

@dblythy dblythy commented Jun 7, 2022

New Pull Request Checklist

Issue Description

Currently, the liveQuery cache stores session tokens and their associated auth for an hour, meaning that liveQuery won't function as expected when roles are updated.

Consider:

  1. a user subscribes to a query
  2. an object is created that they don't have role read access to
  3. They won't receive any updates, as expected.

Then, concurrently, they are added to the role via getUsers().add.

  1. The object is updated, now they do have read access to
  2. Because the cache, LiveQuery thinks they are not part of this role, so the event won't be triggered.

Related issue: #5393

Approach

Clear liveQuery cache for respective user on role class save.

TODOs before merging

  • Add tests
  • A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

@parse-github-assistant
Copy link

parse-github-assistant bot commented Jun 7, 2022

Thanks for opening this pull request!

  • 🎉 We are excited about your hands-on contribution!

@codecov
Copy link

codecov bot commented Jun 7, 2022

Codecov Report

Merging #8026 (f392def) into alpha (0cd902b) will decrease coverage by 0.33%.
The diff coverage is 93.10%.

❗ Current head f392def differs from pull request most recent head a1e5b82. Consider uploading reports for the commit a1e5b82 to get more accurate results

@@            Coverage Diff             @@
##            alpha    #8026      +/-   ##
==========================================
- Coverage   94.13%   93.79%   -0.34%     
==========================================
  Files         182      182              
  Lines       13631    13659      +28     
==========================================
- Hits        12831    12811      -20     
- Misses        800      848      +48     
Impacted Files Coverage Δ
src/Auth.js 99.30% <83.33%> (-0.70%) ⬇️
src/LiveQuery/ParseLiveQueryServer.js 95.65% <94.11%> (+0.16%) ⬆️
src/Controllers/LiveQueryController.js 97.05% <100.00%> (+0.28%) ⬆️
src/LiveQuery/ParseCloudCodePublisher.js 100.00% <100.00%> (ø)
src/RestWrite.js 94.12% <100.00%> (-0.29%) ⬇️
src/Adapters/Cache/RedisCacheAdapter.js 12.28% <0.00%> (-75.44%) ⬇️
src/Adapters/Files/GridFSBucketAdapter.js 79.50% <0.00%> (-0.82%) ⬇️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 92.94% <0.00%> (-0.22%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0cd902b...a1e5b82. Read the comment docs.

@dblythy
Copy link
Member Author

dblythy commented Jun 7, 2022

Just trying to get tests to pass

@Moumouls
Copy link
Member

Moumouls commented Jun 8, 2022

Just trying to get tests to pass

@dblythy tests are okay on your computer ?

@dblythy
Copy link
Member Author

dblythy commented Jun 8, 2022

Yep, it's got something to do with the live query cache not clearing between tests I think. It's very flaky here and only breaking sometimes. Struggling to work it out

@mtrezza
Copy link
Member

mtrezza commented Jun 8, 2022

Please rename PR to describe what was broken instead of how it was solved

@dblythy dblythy changed the title fix: clear live query role cache when a user is added to a role fix: live query role cache does not clear when a user is added to a role Jun 8, 2022
src/Auth.js Outdated Show resolved Hide resolved
src/Auth.js Outdated Show resolved Hide resolved
src/LiveQuery/ParseLiveQueryServer.js Outdated Show resolved Hide resolved
src/LiveQuery/ParseLiveQueryServer.js Outdated Show resolved Hide resolved
src/LiveQuery/ParseLiveQueryServer.js Outdated Show resolved Hide resolved
src/LiveQuery/ParseLiveQueryServer.js Outdated Show resolved Hide resolved
src/LiveQuery/ParseLiveQueryServer.js Outdated Show resolved Hide resolved
src/RestWrite.js Outdated Show resolved Hide resolved
src/LiveQuery/ParseLiveQueryServer.js Show resolved Hide resolved
dblythy and others added 7 commits June 9, 2022 00:18
Co-authored-by: Antoine Cormouls <contact.antoine.cormouls@gmail.com>
Co-authored-by: Antoine Cormouls <contact.antoine.cormouls@gmail.com>
Co-authored-by: Antoine Cormouls <contact.antoine.cormouls@gmail.com>
Co-authored-by: Antoine Cormouls <contact.antoine.cormouls@gmail.com>
@dblythy dblythy requested a review from a team June 10, 2022 02:49
Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

A review has been requested but there are still unresolved discussions from a previous feedback.

@dblythy
Copy link
Member Author

dblythy commented Jun 11, 2022

@Moumouls feedback regarding non awaited promises is out of the scope of this PR, the function this._clearCachedRoles follows the same style as the existing live query events, this._onAfterSave, this._onAfterDelete. The live query controller essentially functions already by using non awaited promises, which is why I suggested opening another issue to discuss and address this existing functionality

@mtrezza
Copy link
Member

mtrezza commented Jun 11, 2022

In that case you can just resolve the discussion. It's a merge condition to have all open discussions resolved. I'll go ahead and do that for you.

Copy link
Member

@mtrezza mtrezza left a comment

Choose a reason for hiding this comment

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

Looks good, great work everyone!

@mtrezza mtrezza merged commit 199dfc1 into parse-community:alpha Jun 11, 2022
@dblythy dblythy deleted the live-query-roles branch June 11, 2022 08:22
@dblythy
Copy link
Member Author

dblythy commented Jun 11, 2022

Thanks @Moumouls and @mtrezza 😊

parseplatformorg pushed a commit that referenced this pull request Jun 11, 2022
# [5.3.0-alpha.16](5.3.0-alpha.15...5.3.0-alpha.16) (2022-06-11)

### Bug Fixes

* live query role cache does not clear when a user is added to a role ([#8026](#8026)) ([199dfc1](199dfc1))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.3.0-alpha.16

@parseplatformorg parseplatformorg added the state:released-alpha Released as alpha version label Jun 11, 2022
parseplatformorg pushed a commit that referenced this pull request Jun 17, 2022
# [5.3.0-beta.1](5.2.1...5.3.0-beta.1) (2022-06-17)

### Bug Fixes

* afterSave trigger removes pointer in Parse object ([#7913](#7913)) ([47d796e](47d796e))
* auto-release process may fail if optional back-merging task fails ([#8051](#8051)) ([cf925e7](cf925e7))
* custom database options are not passed to MongoDB GridFS ([#7911](#7911)) ([b1e5565](b1e5565))
* depreciate allowClientClassCreation defaulting to true ([#7925](#7925)) ([38ed96a](38ed96a))
* errors in GraphQL do not show the original error but a general `Unexpected Error` ([#8045](#8045)) ([0d81887](0d81887))
* interrupted WebSocket connection not closed by LiveQuery server ([#8012](#8012)) ([2d5221e](2d5221e))
* live query role cache does not clear when a user is added to a role ([#8026](#8026)) ([199dfc1](199dfc1))
* peer dependency mismatch for GraphQL dependencies ([#7934](#7934)) ([0a6faa8](0a6faa8))
* return correct response when revert is used in beforeSave ([#7839](#7839)) ([19900fc](19900fc))
* security upgrade @parse/fs-files-adapter from 1.2.1 to 1.2.2 ([#7948](#7948)) ([3a70fda](3a70fda))
* security upgrade moment from 2.29.1 to 2.29.2 ([#7931](#7931)) ([731c550](731c550))
* security upgrade parse push adapter from 4.1.0 to 4.1.2 ([#7893](#7893)) ([93667b4](93667b4))
* websocket connection of LiveQuery interrupts frequently ([#8048](#8048)) ([03caae1](03caae1))

### Features

* add MongoDB 5.1 compatibility ([#7682](#7682)) ([022a856](022a856))
* add MongoDB 5.2 support ([#7894](#7894)) ([5bfa716](5bfa716))
* add support for Node 17 and 18 ([#7896](#7896)) ([3e9f292](3e9f292))
* align file trigger syntax with class trigger; use the new syntax `Parse.Cloud.beforeSave(Parse.File, (request) => {})`, the old syntax `Parse.Cloud.beforeSaveFile((request) => {})` has been deprecated ([#7966](#7966)) ([c6dcad8](c6dcad8))
* replace GraphQL Apollo with GraphQL Yoga ([#7967](#7967)) ([1aa2204](1aa2204))
* selectively enable / disable default authentication adapters ([#7953](#7953)) ([c1e808f](c1e808f))
* upgrade mongodb from 4.4.1 to 4.5.0 ([#7991](#7991)) ([e692b5d](e692b5d))

### Performance Improvements

* reduce database operations when using the constant parameter in Cloud Function validation ([#7892](#7892)) ([041197f](041197f))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.3.0-beta.1

@parseplatformorg parseplatformorg added the state:released-beta Released as beta version label Jun 17, 2022
parseplatformorg pushed a commit that referenced this pull request Oct 29, 2022
# [5.3.0](5.2.8...5.3.0) (2022-10-29)

### Bug Fixes

* afterSave trigger removes pointer in Parse object ([#7913](#7913)) ([47d796e](47d796e))
* authentication adapter app ID validation may be circumvented; this fixes a vulnerability that affects configurations which allow users to authenticate using the Parse Server authentication adapter for *Facebook* or *Spotify* and where the server-side authentication adapter configuration `appIds` is set as a string (e.g. `abc`) instead of an array of strings (e.g. `["abc"]`) ([GHSA-r657-33vp-gp22](GHSA-r657-33vp-gp22)) [skip release] ([#8188](#8188)) ([1a2b1b9](1a2b1b9))
* auto-release process may fail if optional back-merging task fails ([#8051](#8051)) ([cf925e7](cf925e7))
* brute force guessing of user sensitive data via search patterns (GHSA-2m6g-crv8-p3c6) ([#8145](#8145)) [skip release] ([f0db4ca](f0db4ca))
* certificate in Apple Game Center auth adapter not validated [skip release] ([#8055](#8055)) ([4c2aa63](4c2aa63))
* custom database options are not passed to MongoDB GridFS ([#7911](#7911)) ([b1e5565](b1e5565))
* depreciate allowClientClassCreation defaulting to true ([#7925](#7925)) ([38ed96a](38ed96a))
* errors in GraphQL do not show the original error but a general `Unexpected Error` ([#8045](#8045)) ([0d81887](0d81887))
* interrupted WebSocket connection not closed by LiveQuery server ([#8012](#8012)) ([2d5221e](2d5221e))
* invalid file request not properly handled [skip release] ([#8061](#8061)) ([1a04a34](1a04a34))
* live query role cache does not clear when a user is added to a role ([#8026](#8026)) ([199dfc1](199dfc1))
* peer dependency mismatch for GraphQL dependencies ([#7934](#7934)) ([0a6faa8](0a6faa8))
* protected fields exposed via LiveQuery (GHSA-crrq-vr9j-fxxh) [skip release] ([#8075](#8075)) ([636d16e](636d16e))
* return correct response when revert is used in beforeSave ([#7839](#7839)) ([19900fc](19900fc))
* security upgrade @parse/fs-files-adapter from 1.2.1 to 1.2.2 ([#7948](#7948)) ([3a70fda](3a70fda))
* security upgrade moment from 2.29.1 to 2.29.2 ([#7931](#7931)) ([731c550](731c550))
* security upgrade parse push adapter from 4.1.0 to 4.1.2 ([#7893](#7893)) ([93667b4](93667b4))
* server crashes when receiving file download request with invalid byte range; this fixes a security vulnerability that allows an attacker to impact the availability of the server instance; the fix improves parsing of the range parameter to properly handle invalid range requests ([GHSA-h423-w6qv-2wj3](GHSA-h423-w6qv-2wj3)) [skip release] ([#8237](#8237)) ([4c1befa](4c1befa))
* session object properties can be updated by foreign user; this fixes a security vulnerability in which a foreign user can write to the session object of another user if the session object ID is known; the fix prevents writing to foreign session objects ([GHSA-6w4q-23cf-j9jp](GHSA-6w4q-23cf-j9jp)) [skip release] ([#8181](#8181)) ([83cdc89](83cdc89))
* websocket connection of LiveQuery interrupts frequently ([#8048](#8048)) ([03caae1](03caae1))

### Features

* add MongoDB 5.1 compatibility ([#7682](#7682)) ([022a856](022a856))
* add MongoDB 5.2 support ([#7894](#7894)) ([5bfa716](5bfa716))
* add support for Node 17 and 18 ([#7896](#7896)) ([3e9f292](3e9f292))
* align file trigger syntax with class trigger; use the new syntax `Parse.Cloud.beforeSave(Parse.File, (request) => {})`, the old syntax `Parse.Cloud.beforeSaveFile((request) => {})` has been deprecated ([#7966](#7966)) ([c6dcad8](c6dcad8))
* replace GraphQL Apollo with GraphQL Yoga ([#7967](#7967)) ([1aa2204](1aa2204))
* selectively enable / disable default authentication adapters ([#7953](#7953)) ([c1e808f](c1e808f))
* upgrade mongodb from 4.4.1 to 4.5.0 ([#7991](#7991)) ([e692b5d](e692b5d))

### Performance Improvements

* reduce database operations when using the constant parameter in Cloud Function validation ([#7892](#7892)) ([041197f](041197f))
@parseplatformorg
Copy link
Contributor

🎉 This change has been released in version 5.3.0

@parseplatformorg parseplatformorg added the state:released Released as stable version label Oct 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:released Released as stable version state:released-alpha Released as alpha version state:released-beta Released as beta version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants