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

Check correct permissions when resharing #22740

Merged
merged 1 commit into from
Mar 1, 2016
Merged

Conversation

rullzer
Copy link
Contributor

@rullzer rullzer commented Feb 29, 2016

Fixes #22675

Since we only get a share id we do not know the path for the sharer.
Now if we edit a share we start searching for shares for that user of
that node. And deduce the permissions that way.

  • Intergration test added

@PVince81 @cmonteroluque I really hate to do this but getting this into 9.0 is kind of curcial from my POV

@nasli please verify

The intergration test check the test case. But please try yourself ;)

CC: @schiesbn @MorrisJobke @nickvergessen @PVince81 @LukasReschke @DeepDiver1975

@rullzer
Copy link
Contributor Author

rullzer commented Feb 29, 2016

To give a bit more context. Since we don't know which node to pick we just take the node from the owner. When we verify permissions in the share manager this is enough.

However in the OCS Share API we also have to verify that we have the permissions we want to provide.

Fixes #22675

Since we only get a share id we do not know the path for the sharer.
Now if we edit a share we start searching for shares for that user of
that node. And deduce the permissions that way.

* Intergration test added
* Fix unit tests
@nasli
Copy link

nasli commented Feb 29, 2016

👍 Works for me @rullzer! Fixed #22675
Agree we should include this on 9.0

Just notice that now we receive
<statuscode>404</statuscode> <message>Cannot increase permissions</message>
For server 8 we receive
<statuscode>404</statuscode> <message>Setting permissions for 844 failed, because the permissions exceed permissions granted to user2</message>

@rullzer
Copy link
Contributor Author

rullzer commented Feb 29, 2016

Ok let me fix the error message as well :)

@nasli
Copy link

nasli commented Feb 29, 2016

Ok, but that number does not mean nothing to the user, instead we could show the name of the file

@rullzer
Copy link
Contributor Author

rullzer commented Feb 29, 2016

On second tought... lets keep the message simple like it is. The user knows the action they performed... having the fileid in there is also not really helping :P.

For 9.1 we want to fix more exceptions etc anyway. So then we can take this as well.
Just check the status code ;)

Good that it sovled it.

@rullzer
Copy link
Contributor Author

rullzer commented Feb 29, 2016

Of course you know the file and action in the app. So maybe for now it is best to contstruct an error message yourself.

@nasli
Copy link

nasli commented Feb 29, 2016

Totally agree! Maybe it is better show the shortets

@blizzz
Copy link
Contributor

blizzz commented Feb 29, 2016

Code looks ok, however my test did not succeed: I shared a file from A to B with reduced permissions. With the desktop client, I attempted to share to C, however it failed with master as well as this branch with "Cannot increase permissions of ". Now I am not entirely sure whether this is a valid test case (but it does not look right either).

@MorrisJobke
Copy link
Contributor

I tested this just now and it works. I cannot increase the permissions anymore for the user. 👍

@rullzer
Copy link
Contributor Author

rullzer commented Mar 1, 2016

@blizzz yeah that with the desktop client is a different problem. Basically we need to check in the desktop client what permissions we can provide.

DeepDiver1975 added a commit that referenced this pull request Mar 1, 2016
Check correct permissions when resharing
@DeepDiver1975 DeepDiver1975 merged commit f3050b3 into master Mar 1, 2016
@DeepDiver1975 DeepDiver1975 deleted the fix_resare_updates branch March 1, 2016 07:17
@lock
Copy link

lock bot commented Aug 7, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Share api does not detect exceed permission granted error on server 9
5 participants