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

Capabilites API response does not have shareapi_exclude_groups #31576

Closed
phil-davis opened this issue May 29, 2018 · 9 comments
Closed

Capabilites API response does not have shareapi_exclude_groups #31576

phil-davis opened this issue May 29, 2018 · 9 comments

Comments

@phil-davis
Copy link
Contributor

While starting to make acceptance tests related to "exclude groups from sharing", I noticed that the setting of this does not appear in the capabilities API response.
It would be good to have it, so tests can easily see what the current setting is and so (in future) clients can easily know about the setting and adjust their sharing behavior (if they care).

  • add the setting of shareapi_exclude_groups to the capabilities API response
  • add the list of excluded groups to the capabilities API response
@ownclouders
Copy link
Contributor

GitMate.io thinks possibly related issues are #3061 (API - failed response BUG), #9304 ([webdav] Etag does not change when disabling share api), #22666 (No WWW-authenticate response header for HTTP PUT API requests), #3215 (OCS API does not allow function closures as actions), and #5724 ([OC6Beta2] Share API Get all shares doesn't work with CURL).

@phil-davis
Copy link
Contributor Author

PR #31580 should do it

@PVince81
Copy link
Contributor

I'm not sure whether this is a good idea as it could be revealing private information. Some users might not be allowed to know about the existence of other groups but having this on capabilities would expose just that.

In general I'd expect all API calls to already comply with this rule so clients wouldn't need to know about the exclusion, so the acceptance tests would be the only case ?

cc @patrickjahns @SamuAlfageme as we talked about capabilities this morning

@PVince81 PVince81 added this to the backlog milestone May 29, 2018
@phil-davis
Copy link
Contributor Author

If a client wants to be nice, it can query the capabilities when it connects. Then it knows the various sharing things that can/cannot be done (e.g. if public link shares are not enabled, or the whole of sharing is not enabled, or...). The client can then pro-actively switch off bits of its UI, rather than having elements on the UI that are going to result in error messages if the end-user tries to use them.

But yes, for keeping group names as private as possible, it would be better if the client provides their authentication to some end-point which responds with client-behavior capabilities - e.g. can_share boolean. The client does not need to know exactly why they cannot share (e.g. because sharing is off, or because they are in an excluded group or...)

@PVince81
Copy link
Contributor

Yes, the can_share boolean approach is better and its value depends on the logged in user.
If more info is needed then its value could be more detailed like "exclusion", "sharing_off" or something. But I'm not sure whether there is any value for clients knowing that.

@phil-davis
Copy link
Contributor Author

For this change here, adding to the general capabilities report, would it be easy for the Capabilities class to know the answer to isAdmin() (be passed a GroupManager, User object, whatever...)?

Then a Capabilities class instance can control the response based on who is asking.

The files_sharing capabilities can decide the setting for can_share based on what groups the user is in, and if the user is an admin it can also provide the "exclude groups from sharing" details.

@PVince81
Copy link
Contributor

PVince81 commented Jul 5, 2018

close as we have can_share now ?

@phil-davis
Copy link
Contributor Author

Yes, closing.
I will have to work around not having the actual excluded groups information in the capabilities. (We decided not to expose that in capabilities) We will have to be bit trickier with acceptance tests for that.

@lock
Copy link

lock bot commented Jul 30, 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 Jul 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants