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

Granular CLP pointer permissions #6352

Merged
merged 4 commits into from
Jan 28, 2020

Conversation

BufferUnderflower
Copy link
Contributor

Tracking issue #6351
Previously requested with #4251, #4045

// PUT http://localhost:1337/schemas/:className
{
  "classLevelPermissions":
  {
    "get"/"find"/"count"/"update"/"delete"/"addField": { 
      "*": true, // Public access (set by default) 
      [objectId]: true, // by id of `_User`
      "role:admin": true, // by role (`role:${role_name}`)
      "requiresAuthentication": true, // only authenticated users
      "pointerFields": ["field_name", ...] // fields of type Pointer<_User> or Pointer<_User>[]
    },
    "readUserFields":["field_name",...], // same as 'pointerFields' for get+find+count
    "writeUserFields":["field_name",...] // same as 'pointerFields' for update+delete+addField
    ...
  }
}

Pointer permissions for:

  • create has no effect
  • addField accounted only on updating object, ignored when creating

When set for an operation:

{
  "classLevelPermissions":{
    [operation]: {
      pointerFields: [‘field’]
      // default * Public removed
      // no other rules set
    }
  }
}
Operation user not pointed by field user is pointed by field
get 101: Object not found ok
find Limited results ok
count Limited count ok
create Permission denied Permission denied
update 101: Object not found ok
delete 101: Object not found ok
addField Permission denied ok

@codecov
Copy link

codecov bot commented Jan 17, 2020

Codecov Report

Merging #6352 into master will increase coverage by <.01%.
The diff coverage is 94.79%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6352      +/-   ##
==========================================
+ Coverage   93.83%   93.83%   +<.01%     
==========================================
  Files         169      169              
  Lines       11588    11654      +66     
==========================================
+ Hits        10874    10936      +62     
- Misses        714      718       +4
Impacted Files Coverage Δ
src/RestQuery.js 95.47% <ø> (ø) ⬆️
src/rest.js 98.86% <ø> (ø) ⬆️
src/Routers/AggregateRouter.js 100% <100%> (ø) ⬆️
src/Controllers/DatabaseController.js 95.12% <100%> (+0.1%) ⬆️
src/Adapters/Storage/Mongo/MongoCollection.js 97.43% <100%> (ø) ⬆️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 92.77% <100%> (-0.67%) ⬇️
src/Routers/ClassesRouter.js 96.84% <100%> (+0.13%) ⬆️
src/RestWrite.js 93.81% <100%> (+0.18%) ⬆️
src/triggers.js 92.94% <33.33%> (-1.53%) ⬇️
src/Controllers/SchemaController.js 96.85% <98.14%> (+0.42%) ⬆️
... and 4 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 9c43a93...74d49b5. Read the comment docs.

@davimacedo
Copy link
Member

Pointer permissions for:

  • create has no effect
  • addField accounted only on updating object, ignored when creating

Could you explain why? It is related to this recent issue.

@BufferUnderflower
Copy link
Contributor Author

Could you explain why? It is related to this recent issue.

I've replied in that issue, in brief:

create has no effect

There is no object on server before it is created. And obviously no field of that object with _Users to be allowed.

If we allow create by field (pointer permission) examining just the incoming object from POST body - I believe this adds zero security, but more importantly it would then allow anyone to create an object, just by putting himself into a field.

If you want to enforce a rule that some field should contain only the person who creates it - this has nothing to do with class level permissions. Create operation can be allowed using CLP of any other type (role, user pointer, requiresAuthentication) but not using a Pointer Permission (column name).

addField accounted only on updating object, ignored when creating

As of now (without this PR) there can't be a separate addField pointer permission ( now it is part of writeUserField group in contrary to non-pointer perms that already allow granularity).

My PR adds ability to set pointer permission per operation.

Since addField is not a separate REST operation, but rather a side effect of create/update request
I thought it would be logical to also enforce the same for addField. No object => no column yet (to decide whether a field can be added during the create operation).

But when updating - there is already an object so it's possible to permit/deny based on pointer permission column.

Copy link
Member

@davimacedo davimacedo left a comment

Choose a reason for hiding this comment

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

LGTM!

@davimacedo davimacedo merged commit 3c46117 into parse-community:master Jan 28, 2020
@sunshineo
Copy link
Contributor

This is great work! Thank you! Is there a change in the parse-dashboard project needed?

@difelice
Copy link

This is great work! Thank you! Is there a change in the parse-dashboard project needed?

Yes, it's needed. I left a comment here. Thanks.

@difelice
Copy link

From this comment:

[objectId]: true, // by id of `_User`

How does that work? Thanks.

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.

4 participants