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

Added array support for pointer-permissions #5921

Merged

Conversation

Dobbias
Copy link
Contributor

@Dobbias Dobbias commented Aug 15, 2019

This adds the ability to provide array columns to 'readUserFields' or 'writeUserFields'.
All Pointer<_User> in the array stored in the object field are granted the read and/or write permission.

This extends the pointer permissions without affecting the ability of using single Pointer<_User> columns.

This functionality is also needed for future additions to the protected fields as discussed in pull request #5887

@Dobbias
Copy link
Contributor Author

Dobbias commented Aug 15, 2019

The build for PostgreSQL currently fails since it doesn't support the $all query operator for objects. This is currently used since the pointer-permissions array stores the users as Pointer<_User>. As far as is understand there are two options:

  1. Make this feature MongoDB exclusive
  2. Store the object ids of the users in the pointer-permissions array instead of the actual pointer and use $all with a regex (checking for the userId)

The advantage of 2. is that it should work on both dbs. The advantage of 1. is that it's coherent with the one-user pointer-permissions which uses columns of the type Pointer<_User>.

I would go with number 2. Or is 1 the way to go?

@dplewis
Copy link
Member

dplewis commented Aug 15, 2019

@Dobbias Thank for starting on this PR. I don't know much about permissions but I can look into postgres for you. $all could be transformed to and if I remember correctly.

@codecov
Copy link

codecov bot commented Aug 16, 2019

Codecov Report

Merging #5921 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #5921      +/-   ##
==========================================
- Coverage   93.72%   93.69%   -0.03%     
==========================================
  Files         153      156       +3     
  Lines       10801    10866      +65     
==========================================
+ Hits        10123    10181      +58     
- Misses        678      685       +7
Impacted Files Coverage Δ
src/Controllers/SchemaController.js 96.37% <ø> (ø) ⬆️
src/Controllers/DatabaseController.js 94.8% <100%> (-0.01%) ⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 96.68% <100%> (+0.01%) ⬆️
src/GraphQL/ParseGraphQLServer.js 93.02% <0%> (-4.42%) ⬇️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 92.23% <0%> (-0.71%) ⬇️
src/GraphQL/loaders/parseClassTypes.js 85.26% <0%> (-0.71%) ⬇️
src/GraphQL/loaders/objectsMutations.js 97.5% <0%> (-0.5%) ⬇️
src/GraphQL/loaders/usersQueries.js 96.87% <0%> (ø) ⬆️
src/RestWrite.js 93.72% <0%> (ø) ⬆️
src/GraphQL/transformers/query.js 46.87% <0%> (ø)
... and 9 more

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 cf6e79e...66512ed. Read the comment docs.

@dplewis
Copy link
Member

dplewis commented Aug 16, 2019

use $all with a regex

@Dobbias That code was already there so the Postgres fix was easy.
This fix doesn't use regex.

https://github.com/parse-community/parse-server/blob/master/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js#L478

@dplewis dplewis merged commit 0fa315f into parse-community:master Aug 16, 2019
@Dobbias Dobbias deleted the pointerpermissions-array-support branch August 18, 2019 10:29
UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this pull request Mar 21, 2020
* added array support for pointer permissions

* added tests for array support for pointer permissions

* Postgres fix

* simplify PG, no idea why this works
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants