-
Notifications
You must be signed in to change notification settings - Fork 39
Prevent users from deleting their root ACLs #96
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
Conversation
This checks whether pathname indicates that the target is root, or whether the root ACL itself states that the target is root.
|
This fixes https://github.com/solid/solid-ui/issues/94 |
Based on some feedback from Vincent
|
it solves |
|
@bourgeoa thanks for finding this. So this seems to be a bit more complex issue than we initially thought - I will do a bit more research to see how this can be mitigated. I'll update this thread once I know more. |
timbl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be a clean function of targetDoc. Bringing in window.location assumes that the targetDoc os also is the window location, which is non-optimal. Suggest something like targetDoc.uri = targeDoc.site().uri or targetDoc.equals(taregetDoc.site())
window.location && window.location.pathname === '/'
|
@bourgeoa We don't need to protect the folders under root, but https://github.com/solid/solid-ui/issues/99 shows us that we have a bug, namely that we currently cannot create new .acl files for folders directly under root. I will for now only protect root ACL, and work on https://github.com/solid/solid-ui/issues/99 to find a solution that allows for creating new .acl files for folders directly under root. |
Using suggested solution from Tim. Also made code a bit more verbose, to make it easier to fix hacks when we have a standard way of handling this.
|
@timbl I've added the changes that you proposed. Please review again, and merge if everything looks good. |
timbl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK to me thoughI have not tested it
This checks whether pathname indicates that the target is root, or whether the root ACL itself states that the target is root.
Going to create a PR for NSS as well that adds
<.> a sp:Storage.as default for all new root ACLs.