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

Allow option to set Default ACL #7068

Open
dblythy opened this issue Dec 13, 2020 · 5 comments · May be fixed by #8701
Open

Allow option to set Default ACL #7068

dblythy opened this issue Dec 13, 2020 · 5 comments · May be fixed by #8701
Labels
type:feature New feature or improvement of existing feature

Comments

@dblythy
Copy link
Member

dblythy commented Dec 13, 2020

Is your feature request related to a problem? Please describe.
Considering the evolution of data security and privacy, new developers might assume that Parse Object ACL defaults to private, or requested user R/W only.

I think it could be beneficial to have some server config such as:

defaultACL: 'request.user', // default to give requested user R + W
// or
defaultACL: '', // default to give no one access 

and / or

defaultACL : {
  'classname': {
    "*": "r"
  }
}

I think that when a developer new to parse looks at a Parse Object and sees no ACL set, it might be assumed that no one can access the object, when the inverse is true.

A solution could also be ACL options to cloud validator.

Discussed in community forum

@mtrezza
Copy link
Member

mtrezza commented Dec 13, 2020

Thanks for suggesting.

I think this would be a useful addition to Parse Server. It would also fall in line with our strategy to improve Parse Server's security guidance.

when a developer new to parse looks at a Parse Object and sees no ACL set, it might be assumed that no one can access the object, when the inverse is true.

I agree that this assumption will increasingly become true in the context of how security is approached by large cloud service providers.

Some thoughts:

  • The setting could be more granular per class, e.g. with a regex definition for which classes which settings apply by default. If this is not part of this PR, this should be kept in mind as a possible future enhancement when designing the solution.
  • The default could be set to private (only the user who created the object has access to it). This would be a breaking change, as developers would have to manually set the default to public. It would be part of the current strategy to make Parse Server's default security more restrictive as guidance for developers.
  • A public default setting should cause a security check log entry.

@dblythy
Copy link
Member Author

dblythy commented Dec 13, 2020

The setting could be more granular per class, e.g. with a regex definition for which classes which settings apply by default. If this is not part of this PR, this should be kept in mind as a possible future enhancement when designing the solution.

I might need some assistance on the regex definition stuff. My regex skills are to be desired.

The default could be set to private (only the user who created the object has access to it). This would be a breaking change, as developers would have to manually set the default to public. It would be part of the current strategy to make Parse Server's default security more restrictive as guidance for developers.

This could also apply to default CLPs for Parse Server, perhaps we assume requiresAuthentication:true if no CLP is set? Or maybe we could have a defaultCLP option.

Would you be open to adding that if an object has no ACL set, interpret that as public R/W false?

I think the only challenge with this is for users getting started and playing with a development server, if they write some code such as:

const note = new Parse.Object('Note');
await note.save();

And then later, note.find() will effectively return 0 objects, which for a new developer, it might be confusing as to why nothing is showing.

Alternatively, we could add to the documentation for Parse Objects a bold message that states "Objects can be publicly accessed by anyone, unless ACLs and/or CLPs are restrictive. Consider how your data can be publicly accessed before deploying to production".

@mtrezza
Copy link
Member

mtrezza commented Dec 13, 2020

I might need some assistance on the regex definition stuff. My regex skills are to be desired.

You can take a look at the Idempotency regex logic that parses for class names. It's basically the same logic needed here.

This could also apply to default CLPs for Parse Server

Yes, we can think about that, maybe leave that for a separate PR to keep this one simple. But it's something to keep in mind when designing this PR.

Would you be open to adding that if an object has no ACL set, interpret that as public R/W false?

We can discuss that but at first glance it seems to be a fundamental change that could require complex migration efforts for developers, and within Parse Server itself. It basically means to migrate existing data, which is something that requires a very convincing cost/benefit ratio to be considered.

And then later, note.find() will effectively return 0 objects, which for a new developer, it might be confusing as to why nothing is showing

Security awareness is something we want to established for new developers of Parse Server. The guides and example codes would need to be adapted to reflect that. For a simple playground environment, the developer can always set the default object ACL public read/write which would cause a security check log entry as a reminder to disable it later on.

@dblythy
Copy link
Member Author

dblythy commented Dec 14, 2020

We can discuss that but at first glance it seems to be a fundamental change that could require complex migration efforts for developers, and within Parse Server itself. It basically means to migrate existing data, which is something that requires a very convincing cost/benefit ratio to be considered.

That is a good point. So, if I understand correctly, the PR shouldn't modify any existing find logic, just purely save logic? And if no ACL is set on the object, set the acl in accordance with config.defaultACL (defaults to private)?

@mtrezza mtrezza added type:feature New feature or improvement of existing feature and removed type:improvement labels Dec 6, 2021
@parse-github-assistant
Copy link

The label type:feature cannot be used in combination with type:improvement.

@parse-github-assistant parse-github-assistant bot removed the type:feature New feature or improvement of existing feature label Dec 6, 2021
@mtrezza mtrezza added the type:feature New feature or improvement of existing feature label Mar 1, 2023
@dblythy dblythy linked a pull request Jul 24, 2023 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or improvement of existing feature
Projects
None yet
2 participants