-
Notifications
You must be signed in to change notification settings - Fork 112
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
fix: permission check in public share update #4622
Conversation
Found one more corner case 🙈 |
484e469
to
e9dd5f1
Compare
ready for review. |
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.
LGTM
(it really sucks that the publicshareprovider doesn't have any unit tests)
I am on it. We can also wait for them. |
Hm, @butonic The publicsharemanager is currently not testable, it creates its own gatewaySelector. We would need to refactor that to make it testable. |
Cool 🎉 . I am fine to merge this as is and have another PR for the unit test. |
Fine. I need to refactor the New() Method which creates a lot of noise. So i would merge that now here, backport to stable and then refactor it on |
Description
We fixed the permission check for updating public shares. When updating the permissions of a public share while not providing a password, the check must be against the new permissions to take into account that users can opt out only for view permissions.
relates to https://github.com/owncloud/enterprise/issues/6558