Ignore Resharing instead of Erroring #4816
Conversation
| // NOTE: clients tend to send "31" as permissions but they mean "15". "31" is no longer allowed as we have no resharing any more. | ||
| if reqRole == "" && reqPermissions == "31" { | ||
| reqPermissions = "15" | ||
| } | ||
|
|
||
| // user collaborations default to Manager (=all permissions). This will result in a BadRequest. Sane Default. |
There was a problem hiding this comment.
then we need to adapt the test. https://github.com/owncloud/ocis/blob/master/tests/acceptance/features/apiReshare/resharing.feature#L108-L156
I see that you have done a workaround for only one case (31 permissions)
If the client sends another permission containing 16 (resharing) -> we get the same result as before
There was a problem hiding this comment.
as suggestion
if reqRole == "" && permissions > 16 {
permissions -= 16
}
There was a problem hiding this comment.
I forwarded the question. I'd go for the simplest fix possible. This is a hack on a deprecated API anyways.
There was a problem hiding this comment.
as suggestion
if reqRole == "" && permissions > 16 { permissions -= 16 }
That doesn't work unfortunately. permissions is a string. It would need to be an int to support substraction.
kobergj
force-pushed
the
IgnoreResharingRequests
branch
from
0feafae
to
cefe7bf
Compare
| // Or we could change the role later and hope everything works out. | ||
| // Or: | ||
| if reqRole == "" { | ||
| switch reqPermissions { |
There was a problem hiding this comment.
The permission bits are:
https://doc.owncloud.com/server/next/admin_manual/configuration/files/file_sharing_configuration.html#permissions-masks
1 - read
2 - update
4 - create
8 - delete
16 - share (no longer used on ocis)
I suppose that someone could be granted 1+8+16=25 (read, delete and share) and that should become 9.
And for "file drop" there can be combinations like update+create=6 (the person doing the "file drop" cannot read back the file after "dropping" it), and maybe some client might send 22 as the permissions?
Why not just "cast to int" then take the remainder mod16, and cast back to string. That avoids thinking about which permission bit combinations will actually happen.
There was a problem hiding this comment.
This is a deprecated endpoint. Additionally I hope that his gets fixed in clients so we can remove it here. It doesn't need to be very resilient, only needs to fix the actual problem. In fact, it would already be enough to only handle "17", "25" and "31". I just added the other values so it matches with the tests. I don't think we need any more values as there will not be any feature updates to this endpoint
There was a problem hiding this comment.
FYI, @TheOneRing, ok for you?
Signed-off-by: jkoberg <jkoberg@owncloud.com>
kobergj
force-pushed
the
IgnoreResharingRequests
branch
from
cefe7bf
to
a66ed3a
Compare
Ignores resharing. See owncloud/ocis#9681
Also changes the default from
ManagertoViewerasManagerwould result in a bad request.