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

tomneedham · 2018-05-21T15:10:55Z

Description

Sharing is disabled, if a user is a member of any of the excluded groups.

Related Issue

https://github.com/owncloud/enterprise/issues/2508

Motivation and Context

The existing feature is redundant if the user gets added to another group.

How Has This Been Tested?

Manually.

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.

phil-davis · 2018-05-21T15:33:34Z

lib/private/Share20/Manager.php

-					return true;
-				}
+			$matchingGroups = array_intersect($usersGroups, $excludedGroups);
+			if(!empty($matchingGroups)) {


s/if(/if (/
Pretty sure php-cs-fixer wants a space between if and (

Agreed - still need to cache this in my brain-memory

phil-davis · 2018-05-21T15:34:10Z

lib/private/Share20/Manager.php

-				}
+			$matchingGroups = array_intersect($usersGroups, $excludedGroups);
+			if(!empty($matchingGroups)) {
+				// If the user is a member of any of the exlucded groups they cannot use sharing


s/exlucded/excluded/

phil-davis · 2018-05-21T15:36:52Z

@paurakhsharma as you happen to be working on expanding sharing API acceptance tests, it would be good to cover scenarios for this kind of thing.

codecov · 2018-05-22T09:48:03Z

Codecov Report

Merging #31490 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##             master   #31490      +/-   ##
============================================
- Coverage     62.83%   62.83%   -0.01%     
+ Complexity    18371    18370       -1     
============================================
  Files          1151     1151              
  Lines         69058    69057       -1     
  Branches       1260     1260              
============================================
- Hits          43395    43394       -1     
  Misses        25294    25294              
  Partials        369      369

Flag	Coverage Δ	Complexity Δ
#javascript	`52.39% <ø> (ø)`	`0 <ø> (ø)`	⬇️
#phpunit	`64.04% <100%> (-0.01%)`	`18370 <0> (-1)`

Impacted Files	Coverage Δ	Complexity Δ
lib/private/Share20/Manager.php	`97.21% <100%> (-0.01%)`	`226 <0> (-1)`

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 5659036...a1827de. Read the comment docs.

tomneedham · 2018-05-22T15:38:12Z

Fixes #16346

paurakhsharma · 2018-05-23T09:18:55Z

Ok I will look into it.

PVince81 · 2018-05-23T12:57:21Z

needs @pmaier1 approval for this change of behavior

we can still carry on and write acceptance tests as we need them anyway for groups exclusion

PVince81 · 2018-05-23T15:35:14Z

Ok, so this was approved already.

Need to carry on with acceptance tests and code review

tomneedham · 2018-05-23T15:42:18Z

I need to fix the tests

PVince81 · 2018-05-25T09:17:57Z

Estimate to completion: 1md (dev + QA tests) @tomneedham @patrickjahns

phil-davis · 2018-05-25T10:40:20Z

@paurakhsharma will look on Monday and first make some acceptance tests in master that check the the "easy" behavior when the user is in just the "excluded from sharing" group. Then add a commit to this PR with tests for the case when the user is in the "excluded from sharing" group and also in other group(s).

PVince81 · 2018-05-28T11:12:10Z

@tomneedham code change looks good but PR is missing a unit test. Please add it!

PVince81

👍

@phil-davis acceptance tests to be added separately or on this PR ?

phil-davis · 2018-06-01T15:09:22Z

If we can resolve #31580 so I can get these "exclude groups" settings easily in the capabilities (which is the way acceptance tests handle most of these sort of file-sharing options) then I can do the acceptance tests "in no time".

PVince81 · 2018-06-08T09:59:05Z

I'll merge this, please separate PR for acceptance tests

PVince81 · 2018-06-08T09:59:24Z

please backport

phil-davis · 2018-06-08T10:13:51Z

Issue #31696 for acceptance tests, so we do not forget.

phil-davis · 2018-06-12T11:18:49Z

@tomneedham backporting?
I guess it would be nice to get this in stable10 for 10.0.9.

mmattel · 2018-06-12T13:00:54Z

Question, where can you define that a group is an excluded group for sharing?

phil-davis · 2018-06-12T13:03:52Z

Settings, Admin, Sharing. Check box "Exclude groups from sharing". Then you see a box to input group names.

mmattel · 2018-06-12T13:10:10Z

got it, thanks a lot 😄

phil-davis · 2018-06-18T11:13:27Z

See #31819 for another place where this logic lives.

PVince81 · 2018-06-18T11:54:52Z

issue raised here #31820, need to look into steps to confirm this

lock · 2019-07-30T15:02:35Z

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.

tomneedham added the feature:sharing label May 21, 2018

tomneedham added this to the development milestone May 21, 2018

tomneedham self-assigned this May 21, 2018

tomneedham requested review from PVince81, DeepDiver1975 and pmaier1 May 21, 2018 15:10

tomneedham added the 2 - Developing label May 21, 2018

phil-davis reviewed May 21, 2018

View reviewed changes

tomneedham force-pushed the exclude-groups-frompsharing branch from f69e145 to 4353cb7 Compare May 22, 2018 09:47

tomneedham force-pushed the exclude-groups-frompsharing branch from 4353cb7 to 4f0eaf5 Compare May 22, 2018 09:48

phil-davis changed the title ~~Exclude users from sharing if they are a member of any exluded group~~ Exclude users from sharing if they are a member of any excluded group May 22, 2018

tomneedham mentioned this pull request May 22, 2018

Exclude groups from sharing stops working when user is a member of multiple groups #16346

Closed

phil-davis assigned paurakhsharma May 25, 2018

phil-davis mentioned this pull request May 30, 2018

Add exclude_groups_from_sharing to capabilities #31580

Merged

9 tasks

Exclude users from sharing if they are a member of any exluded group

a1827de

tomneedham force-pushed the exclude-groups-frompsharing branch from 4f0eaf5 to a1827de Compare June 1, 2018 10:59

PVince81 approved these changes Jun 1, 2018

View reviewed changes

PVince81 merged commit 63c26f1 into master Jun 8, 2018

PVince81 deleted the exclude-groups-frompsharing branch June 8, 2018 09:59

PVince81 added the backport-request label Jun 8, 2018

phil-davis mentioned this pull request Jun 8, 2018

Acceptance tests for exclude users from sharing changes #31696

Closed

tomneedham mentioned this pull request Jun 12, 2018

[stable10] Exclude from creating shares if member of any excluded groups #31737

Merged

phil-davis removed the backport-request label Jun 12, 2018

PVince81 modified the milestones: development, QA Jun 13, 2018

phil-davis mentioned this pull request Jun 18, 2018

Exclude users from sharing if they are a member of any excluded group - util.php #31819

Merged

9 tasks

PVince81 mentioned this pull request Jun 18, 2018

Exclude group from sharing behavior adjustment doesn't apply everywhere #31820

Closed

1 task

ownclouders mentioned this pull request Jun 20, 2018

Group excluded from sharing - difficult to find comments panel #31840

Closed

This was referenced Dec 3, 2018

when a user that is excluded from sharing tries to share something a success status is returned #33733

Closed

Add test scenarios for trying to reshare when user is in a group excluded from sharing #33824

Closed

ownclouders mentioned this pull request Dec 17, 2018

Add webUI test scenarios for trying to (re)share when user is in a group excluded from sharing #33918

Closed

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

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

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

tomneedham commented May 21, 2018 •

edited

Loading

phil-davis May 21, 2018

tomneedham May 22, 2018

phil-davis May 21, 2018

tomneedham May 22, 2018

phil-davis commented May 21, 2018

codecov bot commented May 22, 2018 •

edited

Loading

tomneedham commented May 22, 2018

paurakhsharma commented May 23, 2018

PVince81 commented May 23, 2018

PVince81 commented May 23, 2018

tomneedham commented May 23, 2018

PVince81 commented May 25, 2018

phil-davis commented May 25, 2018

PVince81 commented May 28, 2018

PVince81 left a comment

phil-davis commented Jun 1, 2018

PVince81 commented Jun 8, 2018

PVince81 commented Jun 8, 2018

phil-davis commented Jun 8, 2018

phil-davis commented Jun 12, 2018

mmattel commented Jun 12, 2018

phil-davis commented Jun 12, 2018

mmattel commented Jun 12, 2018

phil-davis commented Jun 18, 2018

PVince81 commented Jun 18, 2018

lock bot commented Jul 30, 2019

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

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

Conversation

tomneedham commented May 21, 2018 • edited Loading

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Types of changes

Checklist:

phil-davis May 21, 2018

Choose a reason for hiding this comment

tomneedham May 22, 2018

Choose a reason for hiding this comment

phil-davis May 21, 2018

Choose a reason for hiding this comment

tomneedham May 22, 2018

Choose a reason for hiding this comment

phil-davis commented May 21, 2018

codecov bot commented May 22, 2018 • edited Loading

Codecov Report

tomneedham commented May 22, 2018

paurakhsharma commented May 23, 2018

PVince81 commented May 23, 2018

PVince81 commented May 23, 2018

tomneedham commented May 23, 2018

PVince81 commented May 25, 2018

phil-davis commented May 25, 2018

PVince81 commented May 28, 2018

PVince81 left a comment

Choose a reason for hiding this comment

phil-davis commented Jun 1, 2018

PVince81 commented Jun 8, 2018

PVince81 commented Jun 8, 2018

phil-davis commented Jun 8, 2018

phil-davis commented Jun 12, 2018

mmattel commented Jun 12, 2018

phil-davis commented Jun 12, 2018

mmattel commented Jun 12, 2018

phil-davis commented Jun 18, 2018

PVince81 commented Jun 18, 2018

lock bot commented Jul 30, 2019

tomneedham commented May 21, 2018 •

edited

Loading

codecov bot commented May 22, 2018 •

edited

Loading