Skip to content

Commit 8e5ae53

Browse files
danxuliudanxuliu
authored
Merge pull request #56938 from nextcloud/unify-handling-of-exclude-groups-in-contacts-menu-and-sharing
fix: Unify handling of exclude groups in contacts menu and sharing
2 parents 15b4597 + fbe2023 commit 8e5ae53

File tree

6 files changed

+304
-4
lines changed

6 files changed

+304
-4
lines changed

‎build/integration/features/bootstrap/FeatureContext.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,7 @@ protected function resetAppConfigs(): void {
2525
$this->deleteServerConfig('bruteForce', 'whitelist_0');
2626
$this->deleteServerConfig('bruteForce', 'whitelist_1');
2727
$this->deleteServerConfig('bruteforcesettings', 'apply_allowlist_to_ratelimit');
28+
$this->deleteServerConfig('core', 'shareapi_exclude_groups');
29+
$this->deleteServerConfig('core', 'shareapi_exclude_groups_list');
2830
}
2931
}

‎build/integration/features/bootstrap/ShareesContext.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,7 @@ protected function resetAppConfigs() {
2222
$this->deleteServerConfig('core', 'shareapi_only_share_with_group_members');
2323
$this->deleteServerConfig('core', 'shareapi_allow_share_dialog_user_enumeration');
2424
$this->deleteServerConfig('core', 'shareapi_allow_group_sharing');
25+
$this->deleteServerConfig('core', 'shareapi_exclude_groups');
26+
$this->deleteServerConfig('core', 'shareapi_exclude_groups_list');
2527
}
2628
}

‎build/integration/features/contacts-menu.feature‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,140 @@ Feature: contacts-menu
7171
And searched contact "1" is named "Test name"
7272
And searched contact "2" is named "user2"
7373

74+
75+
76+
Scenario: users can not be searched by display name when searcher belongs to a group excluded from sharing
77+
Given user "user0" exists
78+
And group "ExcludedGroup" exists
79+
And user "user0" belongs to group "ExcludedGroup"
80+
And parameter "shareapi_exclude_groups" of app "core" is set to "yes"
81+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "ExcludedGroup"
82+
And user "user1" exists
83+
And As an "admin"
84+
And sending "PUT" to "/cloud/users/user1" with
85+
| key | displayname |
86+
| value | Test name |
87+
When Logging in using web as "user0"
88+
And searching for contacts matching with "test"
89+
Then the list of searched contacts has "0" contacts
90+
91+
Scenario: users can not be searched by email when searcher belongs to a group excluded from sharing
92+
Given user "user0" exists
93+
And group "ExcludedGroup" exists
94+
And user "user0" belongs to group "ExcludedGroup"
95+
And parameter "shareapi_exclude_groups" of app "core" is set to "yes"
96+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "ExcludedGroup"
97+
And user "user1" exists
98+
And As an "admin"
99+
And sending "PUT" to "/cloud/users/user1" with
100+
| key | email |
101+
| value | test@example.com |
102+
When Logging in using web as "user0"
103+
And searching for contacts matching with "test"
104+
Then the list of searched contacts has "0" contacts
105+
106+
Scenario: users can be searched by display name when searcher belongs to both a group excluded from sharing and another group
107+
Given user "user0" exists
108+
And group "ExcludedGroup" exists
109+
And user "user0" belongs to group "ExcludedGroup"
110+
And group "AnotherGroup" exists
111+
And user "user0" belongs to group "AnotherGroup"
112+
And parameter "shareapi_exclude_groups" of app "core" is set to "yes"
113+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "ExcludedGroup"
114+
And user "user1" exists
115+
And As an "admin"
116+
And sending "PUT" to "/cloud/users/user1" with
117+
| key | displayname |
118+
| value | Test name |
119+
When Logging in using web as "user0"
120+
And searching for contacts matching with "test"
121+
Then the list of searched contacts has "1" contacts
122+
And searched contact "0" is named "Test name"
123+
124+
Scenario: users can be searched by email when searcher belongs to both a group excluded from sharing and another group
125+
Given user "user0" exists
126+
And group "ExcludedGroup" exists
127+
And user "user0" belongs to group "ExcludedGroup"
128+
And group "AnotherGroup" exists
129+
And user "user0" belongs to group "AnotherGroup"
130+
And parameter "shareapi_exclude_groups" of app "core" is set to "yes"
131+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "ExcludedGroup"
132+
And user "user1" exists
133+
And As an "admin"
134+
And sending "PUT" to "/cloud/users/user1" with
135+
| key | email |
136+
| value | test@example.com |
137+
When Logging in using web as "user0"
138+
And searching for contacts matching with "test"
139+
Then the list of searched contacts has "1" contacts
140+
And searched contact "0" is named "user1"
141+
142+
Scenario: users can not be searched by display name when searcher does not belong to a group allowed to share
143+
Given user "user0" exists
144+
And group "AllowedGroup" exists
145+
And parameter "shareapi_exclude_groups" of app "core" is set to "allow"
146+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "AllowedGroup"
147+
And user "user1" exists
148+
And As an "admin"
149+
And sending "PUT" to "/cloud/users/user1" with
150+
| key | displayname |
151+
| value | Test name |
152+
When Logging in using web as "user0"
153+
And searching for contacts matching with "test"
154+
Then the list of searched contacts has "0" contacts
155+
156+
Scenario: users can not be searched by email when searcher does not belong to a group allowed to share
157+
Given user "user0" exists
158+
And group "AllowedGroup" exists
159+
And parameter "shareapi_exclude_groups" of app "core" is set to "allow"
160+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "AllowedGroup"
161+
And user "user1" exists
162+
And As an "admin"
163+
And sending "PUT" to "/cloud/users/user1" with
164+
| key | email |
165+
| value | test@example.com |
166+
When Logging in using web as "user0"
167+
And searching for contacts matching with "test"
168+
Then the list of searched contacts has "0" contacts
169+
170+
Scenario: users can be searched by display name when searcher belongs to both a group allowed to share and another group
171+
Given user "user0" exists
172+
And group "AllowedGroup" exists
173+
And user "user0" belongs to group "AllowedGroup"
174+
And group "AnotherGroup" exists
175+
And user "user0" belongs to group "AnotherGroup"
176+
And parameter "shareapi_exclude_groups" of app "core" is set to "allow"
177+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "AllowedGroup"
178+
And user "user1" exists
179+
And As an "admin"
180+
And sending "PUT" to "/cloud/users/user1" with
181+
| key | displayname |
182+
| value | Test name |
183+
When Logging in using web as "user0"
184+
And searching for contacts matching with "test"
185+
Then the list of searched contacts has "1" contacts
186+
And searched contact "0" is named "Test name"
187+
188+
Scenario: users can be searched by email when searcher belongs to both a group allowed to share and another group
189+
Given user "user0" exists
190+
And group "AllowedGroup" exists
191+
And user "user0" belongs to group "AllowedGroup"
192+
And group "AnotherGroup" exists
193+
And user "user0" belongs to group "AnotherGroup"
194+
And parameter "shareapi_exclude_groups" of app "core" is set to "allow"
195+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "AllowedGroup"
196+
And user "user1" exists
197+
And As an "admin"
198+
And sending "PUT" to "/cloud/users/user1" with
199+
| key | email |
200+
| value | test@example.com |
201+
When Logging in using web as "user0"
202+
And searching for contacts matching with "test"
203+
Then the list of searched contacts has "1" contacts
204+
And searched contact "0" is named "user1"
205+
206+
207+
74208
Scenario: users can not be found by display name if visibility is private
75209
Given user "user0" exists
76210
And user "user1" exists

‎build/integration/sharees_features/sharees.feature‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,81 @@ Feature: sharees
117117
And "exact remotes" sharees returned is empty
118118
And "remotes" sharees returned is empty
119119

120+
Scenario: Search when belonging to a group excluded from sharing
121+
Given As an "test"
122+
And parameter "shareapi_exclude_groups" of app "core" is set to "yes"
123+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "ShareeGroup"
124+
When getting sharees for
125+
| search | sharee |
126+
| itemType | file |
127+
Then the OCS status code should be "100"
128+
And the HTTP status code should be "200"
129+
And "exact users" sharees returned is empty
130+
And "users" sharees returned is empty
131+
And "exact groups" sharees returned is empty
132+
And "groups" sharees returned is empty
133+
And "exact remotes" sharees returned is empty
134+
And "remotes" sharees returned is empty
135+
136+
Scenario: Search when belonging to both a group excluded from sharing and another group
137+
Given As an "test"
138+
And group "AnotherGroup" exists
139+
And user "test" belongs to group "AnotherGroup"
140+
And parameter "shareapi_exclude_groups" of app "core" is set to "yes"
141+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "ShareeGroup"
142+
When getting sharees for
143+
| search | sharee |
144+
| itemType | file |
145+
Then the OCS status code should be "100"
146+
And the HTTP status code should be "200"
147+
And "exact users" sharees returned is empty
148+
And "users" sharees returned are
149+
| Sharee1 | 0 | Sharee1 | Sharee1 |
150+
| Sharee2 | 0 | Sharee2 | sharee2@system.com |
151+
And "exact groups" sharees returned is empty
152+
And "groups" sharees returned are
153+
| ShareeGroup | 1 | ShareeGroup |
154+
And "exact remotes" sharees returned is empty
155+
And "remotes" sharees returned is empty
156+
157+
Scenario: Search when not belonging to a group allowed to share
158+
Given As an "test"
159+
And group "AnotherGroup" exists
160+
And parameter "shareapi_exclude_groups" of app "core" is set to "allow"
161+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "AnotherGroup"
162+
When getting sharees for
163+
| search | sharee |
164+
| itemType | file |
165+
Then the OCS status code should be "100"
166+
And the HTTP status code should be "200"
167+
And "exact users" sharees returned is empty
168+
And "users" sharees returned is empty
169+
And "exact groups" sharees returned is empty
170+
And "groups" sharees returned is empty
171+
And "exact remotes" sharees returned is empty
172+
And "remotes" sharees returned is empty
173+
174+
Scenario: Search when belonging to both a group allowed to share and another group
175+
Given As an "test"
176+
And group "AnotherGroup" exists
177+
And user "test" belongs to group "AnotherGroup"
178+
And parameter "shareapi_exclude_groups" of app "core" is set to "allow"
179+
And parameter "shareapi_exclude_groups_list" of app "core" is set to "AnotherGroup"
180+
When getting sharees for
181+
| search | sharee |
182+
| itemType | file |
183+
Then the OCS status code should be "100"
184+
And the HTTP status code should be "200"
185+
And "exact users" sharees returned is empty
186+
And "users" sharees returned are
187+
| Sharee1 | 0 | Sharee1 | Sharee1 |
188+
| Sharee2 | 0 | Sharee2 | sharee2@system.com |
189+
And "exact groups" sharees returned is empty
190+
And "groups" sharees returned are
191+
| ShareeGroup | 1 | ShareeGroup |
192+
And "exact remotes" sharees returned is empty
193+
And "remotes" sharees returned is empty
194+
120195
Scenario: Search without exact match no iteration allowed
121196
Given As an "test"
122197
And parameter "shareapi_allow_share_dialog_user_enumeration" of app "core" is set to "no"

‎lib/private/Contacts/ContactsMenu/ContactsStore.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ public function getContacts(IUser $user, ?string $filter, ?int $limit = null, ?i
149149
* 1. if the `shareapi_allow_share_dialog_user_enumeration` config option is
150150
* enabled it will filter all local users
151151
* 2. if the `shareapi_exclude_groups` config option is enabled and the
152-
* current user is in an excluded group it will filter all local users.
152+
* current user is only in excluded groups it will filter all local users.
153153
* 3. if the `shareapi_only_share_with_group_members` config option is
154154
* enabled it will filter all users which doesn't have a common group
155155
* with the current user.
@@ -184,8 +184,8 @@ private function filterContacts(
184184
$excludeGroupsList = $decodedExcludeGroups ?? [];
185185

186186
if ($excludeGroups != 'allow') {
187-
if (count(array_intersect($excludeGroupsList, $selfGroups)) !== 0) {
188-
// a group of the current user is excluded -> filter all local users
187+
if (count($selfGroups) > 0 && count(array_diff($selfGroups, $excludeGroupsList)) === 0) {
188+
// all the groups of the current user are excluded -> filter all local users
189189
$skipLocal = true;
190190
}
191191
} else {

‎tests/lib/Contacts/ContactsMenu/ContactsStoreTest.php‎

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,94 @@ public function testGetContactsWithoutAvatarURI(): void {
188188
$this->assertEquals('https://photo', $entries[1]->getAvatar());
189189
}
190190

191-
public function testGetContactsWhenUserIsInExcludeGroups(): void {
191+
public static function dataGetContactsWhenUserIsInExcludeGroups(): array {
192+
return [
193+
['yes', '[]', [], ['user123', 'user12345']],
194+
['yes', '["excludedGroup1"]', [], ['user123', 'user12345']],
195+
['yes', '["excludedGroup1"]', ['anotherGroup1'], ['user123', 'user12345']],
196+
['yes', '["excludedGroup1"]', ['anotherGroup1', 'anotherGroup2', 'anotherGroup3'], ['user123', 'user12345']],
197+
['yes', '["excludedGroup1"]', ['excludedGroup1'], []],
198+
['yes', '["excludedGroup1"]', ['anotherGroup1', 'excludedGroup1'], ['user123', 'user12345']],
199+
['yes', '["excludedGroup1"]', ['excludedGroup1', 'anotherGroup1', 'anotherGroup2', 'anotherGroup3'], ['user123', 'user12345']],
200+
['yes', '["excludedGroup1", "excludedGroup2", "excludedGroup3"]', [], ['user123', 'user12345']],
201+
['yes', '["excludedGroup1", "excludedGroup2", "excludedGroup3"]', ['anotherGroup1'], ['user123', 'user12345']],
202+
['yes', '["excludedGroup1", "excludedGroup2", "excludedGroup3"]', ['anotherGroup1', 'anotherGroup2', 'anotherGroup3'], ['user123', 'user12345']],
203+
['yes', '["excludedGroup1", "excludedGroup2", "excludedGroup3"]', ['excludedGroup1'], []],
204+
['yes', '["excludedGroup1", "excludedGroup2", "excludedGroup3"]', ['excludedGroup2'], []],
205+
['yes', '["excludedGroup1", "excludedGroup2", "excludedGroup3"]', ['excludedGroup3'], []],
206+
['yes', '["excludedGroup1", "excludedGroup2", "excludedGroup3"]', ['excludedGroup1', 'excludedGroup2', 'excludedGroup3'], []],
207+
['yes', '["excludedGroup1", "excludedGroup2", "excludedGroup3"]', ['anotherGroup1', 'excludedGroup1'], ['user123', 'user12345']],
208+
['yes', '["excludedGroup1", "excludedGroup2", "excludedGroup3"]', ['anotherGroup1', 'excludedGroup2', 'anotherGroup2', 'anotherGroup3'], ['user123', 'user12345']],
209+
['yes', '["excludedGroup1", "excludedGroup2", "excludedGroup3"]', ['excludedGroup3', 'anotherGroup1', 'anotherGroup2', 'anotherGroup3'], ['user123', 'user12345']],
210+
['allow', '[]', [], []],
211+
['allow', '["allowedGroup1"]', [], []],
212+
['allow', '["allowedGroup1"]', ['anotherGroup1'], []],
213+
['allow', '["allowedGroup1"]', ['anotherGroup1', 'anotherGroup2', 'anotherGroup3'], []],
214+
['allow', '["allowedGroup1"]', ['allowedGroup1'], ['user123', 'user12345']],
215+
['allow', '["allowedGroup1"]', ['anotherGroup1', 'allowedGroup1'], ['user123', 'user12345']],
216+
['allow', '["allowedGroup1"]', ['allowedGroup1', 'anotherGroup1', 'anotherGroup2', 'anotherGroup3'], ['user123', 'user12345']],
217+
['allow', '["allowedGroup1", "allowedGroup2", "allowedGroup3"]', [], []],
218+
['allow', '["allowedGroup1", "allowedGroup2", "allowedGroup3"]', ['anotherGroup1'], []],
219+
['allow', '["allowedGroup1", "allowedGroup2", "allowedGroup3"]', ['anotherGroup1', 'anotherGroup2', 'anotherGroup3'], []],
220+
['allow', '["allowedGroup1", "allowedGroup2", "allowedGroup3"]', ['allowedGroup1'], ['user123', 'user12345']],
221+
['allow', '["allowedGroup1", "allowedGroup2", "allowedGroup3"]', ['allowedGroup2'], ['user123', 'user12345']],
222+
['allow', '["allowedGroup1", "allowedGroup2", "allowedGroup3"]', ['allowedGroup3'], ['user123', 'user12345']],
223+
['allow', '["allowedGroup1", "allowedGroup2", "allowedGroup3"]', ['allowedGroup1', 'allowedGroup2', 'allowedGroup3'], ['user123', 'user12345']],
224+
['allow', '["allowedGroup1", "allowedGroup2", "allowedGroup3"]', ['anotherGroup1', 'allowedGroup1'], ['user123', 'user12345']],
225+
['allow', '["allowedGroup1", "allowedGroup2", "allowedGroup3"]', ['anotherGroup1', 'allowedGroup2', 'anotherGroup2', 'anotherGroup3'], ['user123', 'user12345']],
226+
['allow', '["allowedGroup1", "allowedGroup2", "allowedGroup3"]', ['allowedGroup3', 'anotherGroup1', 'anotherGroup2', 'anotherGroup3'], ['user123', 'user12345']],
227+
];
228+
}
229+
230+
#[\PHPUnit\Framework\Attributes\DataProvider('dataGetContactsWhenUserIsInExcludeGroups')]
231+
public function testGetContactsWhenUserIsInExcludeGroups(string $excludeGroups, string $excludeGroupsList, array $currentUserGroupIds, array $expectedUids): void {
232+
$this->config
233+
->method('getAppValue')
234+
->willReturnMap([
235+
['core', 'shareapi_allow_share_dialog_user_enumeration', 'yes', 'yes'],
236+
['core', 'shareapi_restrict_user_enumeration_to_group', 'no', 'no'],
237+
['core', 'shareapi_restrict_user_enumeration_to_phone', 'no', 'no'],
238+
['core', 'shareapi_exclude_groups', 'no', $excludeGroups],
239+
['core', 'shareapi_only_share_with_group_members', 'no', 'no'],
240+
['core', 'shareapi_exclude_groups_list', '', $excludeGroupsList],
241+
['core', 'shareapi_only_share_with_group_members_exclude_group_list', '', '[]'],
242+
]);
243+
244+
/** @var IUser|MockObject $currentUser */
245+
$currentUser = $this->createMock(IUser::class);
246+
$currentUser->expects($this->exactly(2))
247+
->method('getUID')
248+
->willReturn('user001');
249+
250+
$this->groupManager->expects($this->once())
251+
->method('getUserGroupIds')
252+
->with($this->equalTo($currentUser))
253+
->willReturn($currentUserGroupIds);
254+
255+
$this->contactsManager->expects($this->once())
256+
->method('search')
257+
->with($this->equalTo(''), $this->equalTo(['FN', 'EMAIL']))
258+
->willReturn([
259+
[
260+
'UID' => 'user123',
261+
'isLocalSystemBook' => true
262+
],
263+
[
264+
'UID' => 'user12345',
265+
'isLocalSystemBook' => true
266+
],
267+
]);
268+
269+
270+
$entries = $this->contactsStore->getContacts($currentUser, '');
271+
272+
$this->assertCount(count($expectedUids), $entries);
273+
for ($i = 0; $i < count($expectedUids); $i++) {
274+
$this->assertEquals($expectedUids[$i], $entries[$i]->getProperty('UID'));
275+
}
276+
}
277+
278+
public function testGetContactsOnlyShareIfInTheSameGroupWhenUserIsInExcludeGroups(): void {
192279
$this->config
193280
->method('getAppValue')
194281
->willReturnMap([

0 commit comments

Comments
 (0)