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

ACL Options to Cloud Validator #6975

Closed
wants to merge 2 commits into from

Conversation

dblythy
Copy link
Member

@dblythy dblythy commented Oct 27, 2020

Extending on the 'how to improve default security' discussion in the community forum, I thought it could be handy for the built-in cloud validator to have some ACL enforcement.

With this PR, you can add the following validation:

Parse.Cloud.beforeSave('BeforeSave', () => {}, {
      setACL: 'request.user',
});

or

Parse.Cloud.beforeSave('BeforeSave', () => {}, {
      setACL: {
        'request.user': 'readWrite',
        'role:Administrator': 'read',
      },
});

Options:
As a string:

  • request.user: request.user can read + write. No public access
  • publicRead: public read + no public write
  • publicWrite: no public read + public write
  • roleRead:roleID: role read for roleID + no write for roleID
  • roleWrite:roleID: no role read for roleID + write for roleID
  • roleReadWrite:roleID:role read for roleID + write for roleID

As an object:

  • override: whether the ACL should override the request.object ACL
  • public: public ACL options. Either read,write, or readWrite
  • request.user: request.user ACL options. Either read,write, or readWrite
  • role:***: role ACL options. Either read,write, or readWrite
  • userID: userID ACL options. Either read,write, or readWrite

Also, could it be worth allowing setting the validation object in schema, or another method (maybe the first parameter can be a validation object? If you want the validator on a class but don't care for the beforeSave logic, you'll just get:

Parse.Cloud.beforeSave('myClass', () => {}, {
      //validation    
});

This is another quick project for Hacktober by me, no stress if it's not appropriate for Parse!

@codecov
Copy link

codecov bot commented Oct 27, 2020

Codecov Report

Merging #6975 into master will decrease coverage by 0.07%.
The diff coverage is 84.78%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6975      +/-   ##
==========================================
- Coverage   93.83%   93.75%   -0.08%     
==========================================
  Files         169      169              
  Lines       12402    12494      +92     
==========================================
+ Hits        11637    11714      +77     
- Misses        765      780      +15     
Impacted Files Coverage Δ
src/triggers.js 92.13% <84.78%> (-1.58%) ⬇️
src/Adapters/Files/GridFSBucketAdapter.js 79.50% <0.00%> (-0.82%) ⬇️

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 6fc3afc...6477a63. Read the comment docs.

@davimacedo
Copy link
Member

I believe it is a good addition but I got confused it were the ACL required to run the function or the default ACL to be applied to the objects. So I suggest changing the name of the option from 'ACL' to 'setACL' or something like this.

@@ -733,6 +733,118 @@ function builtInTriggerValidator(options, request) {
}
}
}
const aclOptions = options.setACL;
if (aclOptions && request.object && functionName === 'beforeSave.BeforeSave') {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe some error message if someone try to setACL for other function that not beforeSave?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can do!

@dblythy
Copy link
Member Author

dblythy commented Dec 14, 2020

Closing as I think #7068 will be a better approach

@dblythy dblythy closed this Dec 14, 2020
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