Add exclude_groups_from_sharing to capabilities #31580

phil-davis · 2018-05-29T14:50:13Z

Description

In the capabilities API response, files_sharing section, add:

exclude_groups_from_sharing => boolean state of this setting
groups_excluded_from_sharing => array of groups that are excluded from sharing

and only report this if the user is an admin.

In the capabilities API response, files_sharing section, add:

can_share => boolean state of this setting

Set to true if the user requesting the capabilities can do sharing (i.e. the sharing API is enabled and the user is not excluded from sharing). Otherwise set to ``false```.
This capability could be used by a client app to, for example, decide if the client app should bother to display UI elements for adding shares...

Related Issue

#31576

Motivation and Context

The setting of "exclude groups from sharing and the list of groups that are excluded is not in the capabilities API response.

How Has This Been Tested?

Local API test runs, and browsing to the capabilities URL.

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have read the CONTRIBUTING document.
I have added tests to cover my changes.
All new and existing tests passed.

codecov · 2018-05-30T04:17:19Z

Codecov Report

Merging #31580 into master will decrease coverage by <.01%.
The diff coverage is 55.55%.

@@             Coverage Diff              @@
##             master   #31580      +/-   ##
============================================
- Coverage     63.27%   63.26%   -0.01%     
- Complexity    18479    18483       +4     
============================================
  Files          1161     1161              
  Lines         69377    69386       +9     
  Branches       1261     1261              
============================================
+ Hits          43895    43900       +5     
- Misses        25112    25116       +4     
  Partials        370      370

Flag	Coverage Δ	Complexity Δ
#javascript	`52.47% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`64.5% <55.55%> (-0.01%)`	`18483 <2> (+4)`

Impacted Files	Coverage Δ	Complexity Δ
apps/files_sharing/lib/Capabilities.php	`92.42% <55.55%> (-5.83%)`	`13 <2> (+4)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7549002...13dac01. Read the comment docs.

phil-davis · 2018-05-30T07:58:26Z

tests/acceptance/features/apiCapabilities/capabilities.feature

+ | files_sharing | groups_excluded_from_sharing@@@element[1] |
+ | files_sharing | groups_excluded_from_sharing@@@element[2] |
+
+ @skip @issue-enterprise-2508


This scenario should be fixed by #31490 - so when/if this PR is merged, this skip line can be removed in that PR.

phil-davis · 2018-05-30T12:52:22Z

@PVince81 please have a look and see if I have done reasonable stuff here. I am not familiar with exactly how the DI should be done to get hold of the user and group classes so as to work out if the user is an admin. But this code does work.

PVince81 · 2018-06-01T13:58:14Z

apps/files_sharing/lib/Capabilities.php

@@ -103,6 +131,20 @@ public function getCapabilities() {
 $res['share_with_group_members_only'] = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members', 'yes') === 'yes';
 $res['share_with_membership_groups_only'] = $this->config->getAppValue('core', 'shareapi_only_share_with_membership_groups', 'yes') === 'yes';

+ if ($this->isAdmin()) {
+ $res['exclude_groups_from_sharing'] = $this->config->getAppValue('core', 'shareapi_exclude_groups', 'yes') === 'yes';


in general admins already have access to the config APIs and would be able to retrieve the value from there (which the settings page does), so not sure we really need that in the capabilities for admins

what to do?
do I look somewhere in "config APIs" to get these settings during acceptance test scenarios?
do I remove the code here?
(which will leave just the calculation of can_share which was just a bonus when I started this)

hmm, the "config APIs" are likely a private endpoint... see https://github.com/owncloud/core/blob/v10.0.8/core/js/config.js#L11

We'll likely need to expose those private APIs using public API format (OCS or Webdav)

@phil-davis any reason you need to read any detailed config values during tests ? you could simply set the value to something known so you know it's set during test setup as part of the prerequisite ?

No, we do not have to read back the values. It was just that for other "system/app settings" (at least the ones we used so far), the current setting could be found in capabilities. So I was just maintaining consistency.
We can let the tests make the settings and then the next test steps will observe the changed behavior. If the behavior does not change then either the setting change failed or theere is a bug in the behavior.

PVince81 · 2018-06-01T13:58:41Z

The DI looks ok.

See my comment about the admin part.

PVince81 · 2018-06-15T14:00:50Z

I'd rather not expose this setting if there is no need for clients to know this information, considering that it's conditional. So it's not really a capability any more as it depends on the logged in user.

I see you added some utility functions for tests, maybe we could keep those though?

phil-davis · 2018-06-16T14:58:55Z

rather not expose this setting

which setting do you mean:
can_share - it is conditional, value depends on the logged in user (a capability with-respect-to the current user)
or
exclude_groups_from_sharing - reporting of the "capability" is conditional, depends on the logged in user having admin rights.
?

phil-davis · 2018-06-16T15:00:31Z

I will look through and separate out useful acceptance test stuff related to processing lists in returned xml - that will be good for other stuff in capabilities and... that can have more/better tests.

PVince81 · 2018-06-18T07:55:36Z

I meant not exposing "exclude_groups_from_sharing". we can keep "can_share"

phil-davis · 2018-06-22T03:41:14Z

I rebased this to sit on top of the can_share changes that are now in master - just so that the exclude_groups_from_sharing code is still here. Probably will not be used.

PVince81

👍

PVince81 · 2018-06-28T10:13:30Z

backport?

phil-davis · 2018-06-28T11:05:23Z

@PVince81 the can_share capability was separated into a different PR #31816 which is merged and good.

This PR here is the remaining code, which reports the exclude_groups_from_sharing setting in the capabilities, only if the caller has admin privs. If the Boolean setting is on, then it also reports groups_excluded_from_sharing

I didn't think that you wanted to add this stuff to the capabilities report?

But I am very happy to have it available - it will make tests flow easily.

PVince81 · 2018-06-28T11:31:33Z

I didn't. When I saw the PR I thought it was the updated version where this one was removed already without looking more closely.

Reverting...

PVince81 · 2018-06-28T11:32:28Z

revert PR here #31937

phil-davis · 2018-07-21T12:35:59Z

This part is not being implemented. Backport request removed.

lock · 2019-07-30T11:01:47Z

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.

phil-davis added the 3 - To Review label May 29, 2018

phil-davis added this to the development milestone May 29, 2018

phil-davis self-assigned this May 29, 2018

phil-davis requested review from PVince81, DeepDiver1975 and individual-it May 29, 2018 14:50

phil-davis mentioned this pull request May 29, 2018

Capabilites API response does not have shareapi_exclude_groups #31576

Closed

phil-davis force-pushed the exclude_groups_from_sharing-capability branch from d5299d0 to 9fb33f0 Compare May 30, 2018 07:47

phil-davis commented May 30, 2018

View reviewed changes

phil-davis force-pushed the exclude_groups_from_sharing-capability branch from 9fb33f0 to 3fd24df Compare May 30, 2018 12:49

PVince81 reviewed Jun 1, 2018

View reviewed changes

phil-davis force-pushed the exclude_groups_from_sharing-capability branch from 3fd24df to 9770be2 Compare June 1, 2018 15:01

phil-davis mentioned this pull request Jun 1, 2018

Exclude users from sharing if they are a member of any excluded group #31490

Merged

9 tasks

phil-davis force-pushed the exclude_groups_from_sharing-capability branch from 9770be2 to fcb267e Compare June 5, 2018 07:08

phil-davis mentioned this pull request Jun 8, 2018

Acceptance tests for exclude users from sharing changes #31696

Closed

ownclouders added the status/STALE label Jun 13, 2018

ownclouders removed the status/STALE label Jun 15, 2018

This was referenced Jun 18, 2018

Can share capability #31816

Merged

Add can_share to capabilities report #31817

Closed

phil-davis force-pushed the exclude_groups_from_sharing-capability branch from fcb267e to e39b0cd Compare June 22, 2018 03:25

Add exclude_groups_from_sharing to capabilities

13dac01

phil-davis force-pushed the exclude_groups_from_sharing-capability branch from e39b0cd to 13dac01 Compare June 22, 2018 03:38

PVince81 approved these changes Jun 28, 2018

View reviewed changes

PVince81 merged commit 24411c1 into master Jun 28, 2018

PVince81 deleted the exclude_groups_from_sharing-capability branch June 28, 2018 10:13

PVince81 added the backport-request label Jun 28, 2018

PVince81 mentioned this pull request Jun 28, 2018

Revert "Add exclude_groups_from_sharing to capabilities" #31937

Merged

PVince81 mentioned this pull request Jun 28, 2018

Revert "Revert "Remove old unused private utility method isSharingDisabledForUser"" #31938

Merged

2 tasks

phil-davis removed the backport-request label Jul 21, 2018

lock bot locked as resolved and limited conversation to collaborators Jul 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add exclude_groups_from_sharing to capabilities #31580

Add exclude_groups_from_sharing to capabilities #31580

phil-davis commented May 29, 2018 •

edited

Loading

codecov bot commented May 30, 2018 •

edited

Loading

phil-davis May 30, 2018

phil-davis commented May 30, 2018 •

edited

Loading

PVince81 Jun 1, 2018

phil-davis Jun 1, 2018

PVince81 Jun 4, 2018

PVince81 Jun 4, 2018

phil-davis Jun 5, 2018

PVince81 commented Jun 1, 2018

PVince81 commented Jun 15, 2018

phil-davis commented Jun 16, 2018

phil-davis commented Jun 16, 2018

PVince81 commented Jun 18, 2018

phil-davis commented Jun 22, 2018 •

edited

Loading

PVince81 left a comment

PVince81 commented Jun 28, 2018

phil-davis commented Jun 28, 2018 •

edited

Loading

PVince81 commented Jun 28, 2018

PVince81 commented Jun 28, 2018

phil-davis commented Jul 21, 2018

lock bot commented Jul 30, 2019

Add exclude_groups_from_sharing to capabilities #31580

Add exclude_groups_from_sharing to capabilities #31580

Conversation

phil-davis commented May 29, 2018 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

codecov bot commented May 30, 2018 • edited Loading

Codecov Report

phil-davis May 30, 2018

Choose a reason for hiding this comment

phil-davis commented May 30, 2018 • edited Loading

PVince81 Jun 1, 2018

Choose a reason for hiding this comment

phil-davis Jun 1, 2018

Choose a reason for hiding this comment

PVince81 Jun 4, 2018

Choose a reason for hiding this comment

PVince81 Jun 4, 2018

Choose a reason for hiding this comment

phil-davis Jun 5, 2018

Choose a reason for hiding this comment

PVince81 commented Jun 1, 2018

PVince81 commented Jun 15, 2018

phil-davis commented Jun 16, 2018

phil-davis commented Jun 16, 2018

PVince81 commented Jun 18, 2018

phil-davis commented Jun 22, 2018 • edited Loading

PVince81 left a comment

Choose a reason for hiding this comment

PVince81 commented Jun 28, 2018

phil-davis commented Jun 28, 2018 • edited Loading

PVince81 commented Jun 28, 2018

PVince81 commented Jun 28, 2018

phil-davis commented Jul 21, 2018

lock bot commented Jul 30, 2019

phil-davis commented May 29, 2018 •

edited

Loading

codecov bot commented May 30, 2018 •

edited

Loading

phil-davis commented May 30, 2018 •

edited

Loading

phil-davis commented Jun 22, 2018 •

edited

Loading

phil-davis commented Jun 28, 2018 •

edited

Loading