Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ Summary
* Bugfix - Show non-generic messages for 403 HTTP status to end user: [#395](https://github.com/owncloud/files_antivirus/issues/395)
* Bugfix - Fix command maintenance:mimetype:update-db --repair-filecache: [#38425](https://github.com/owncloud/core/issues/38425)
* Bugfix - Fix storage lookup in versions when storing a new version: [#38430](https://github.com/owncloud/core/pull/38430)
* Bugfix - Fix behavior for user search at the API level: [#38489](https://github.com/owncloud/core/pull/38489)
* Change - Use OcsController and routes instead of API::register: [#37272](https://github.com/owncloud/core/pull/37272)
* Change - API changes to remove shares pointing to missing files: [#38152](https://github.com/owncloud/core/pull/38152)
* Change - Update laminas/laminas-servicemanager (3.4.1 => 3.5.2): [#38306](https://github.com/owncloud/core/pull/38306)
Expand Down Expand Up @@ -218,6 +219,13 @@ Details

https://github.com/owncloud/core/pull/38430

* Bugfix - Fix behavior for user search at the API level: [#38489](https://github.com/owncloud/core/pull/38489)

The 'user.search_min_length' restriction could be circumvented when accessing the API
directly.

https://github.com/owncloud/core/pull/38489

* Change - Use OcsController and routes instead of API::register: [#37272](https://github.com/owncloud/core/pull/37272)

Implemented OcsController and removed a separate file to register ocs routes. Also some
Expand Down
1 change: 1 addition & 0 deletions apps/files_external/l10n/gl.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ OC.L10N.register(
"All users. Type to select user or group." : "Todos os usuarios. Escriba para seleccionar usuario ou grupo.",
"(group)" : "(grupo)",
"Compatibility with Mac NFD encoding (slow)" : "Compatibilidade coa codificación Mac MFD (lenta)",
"Sharing cannot be enabled due to the chosen authentication method" : "Non é posíbel activar o uso compartido por mor do método de autenticación escollido",
"Unknown auth backend \"{b}\"" : "Infraestrutura de autenticación descoñecida «{b}»",
"Please make sure that the app that provides this backend is installed and enabled" : "Asegúrese de que a aplicación que fornece esta infraestrutura está instalada e activada ",
"Admin defined" : "Definido polo administrador",
Expand Down
1 change: 1 addition & 0 deletions apps/files_external/l10n/gl.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
"All users. Type to select user or group." : "Todos os usuarios. Escriba para seleccionar usuario ou grupo.",
"(group)" : "(grupo)",
"Compatibility with Mac NFD encoding (slow)" : "Compatibilidade coa codificación Mac MFD (lenta)",
"Sharing cannot be enabled due to the chosen authentication method" : "Non é posíbel activar o uso compartido por mor do método de autenticación escollido",
"Unknown auth backend \"{b}\"" : "Infraestrutura de autenticación descoñecida «{b}»",
"Please make sure that the app that provides this backend is installed and enabled" : "Asegúrese de que a aplicación que fornece esta infraestrutura está instalada e activada ",
"Admin defined" : "Definido polo administrador",
Expand Down
10 changes: 10 additions & 0 deletions apps/files_sharing/lib/Controller/ShareesController.php
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ public function __construct($appName,
protected function getUsers($search) {
$this->result['users'] = $this->result['exact']['users'] = $users = [];

if (\strlen(\trim($search)) === 0 && $this->userSearch->getSearchMinLength() > 0) {
$this->result['users'] = [];
return;
}

$userGroups = [];
if ($this->shareWithGroupOnly || $this->shareeEnumerationGroupMembers) {
// Search in all the groups this user is part of
Expand Down Expand Up @@ -296,6 +301,11 @@ protected function getAdditionalUserInfo(IUser $user) {
protected function getGroups($search) {
$this->result['groups'] = $this->result['exact']['groups'] = [];

if (\strlen(\trim($search)) === 0 && $this->userSearch->getSearchMinLength() > 0) {
$this->result['groups'] = [];
return;
}

$groups = $this->groupManager->search($search, $this->limit, $this->offset, 'sharing');
$groupIds = \array_map(function (IGroup $group) {
return $group->getGID();
Expand Down
208 changes: 144 additions & 64 deletions apps/files_sharing/tests/API/ShareesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,53 @@ public function dataGetUsers() {
true,
'email'
],
// Test empty searchTerm should not return users.
['', false, true, [], [], [], [], false, false, false, false, 'id', 1],
['', true, true, [], [], [], [], false, false, false, false, 'id', 1],
[
'',
false,
true,
[],
[
$this->getUserMock('test0', 'Test'),
$this->getUserMock('test1', 'Test One'),
$this->getUserMock('test2', 'Test Two'),
],
[
[
'label' => 'Test',
'value' => [
'shareType' => Share::SHARE_TYPE_USER,
'shareWith' => 'test0',
'userType' => User::USER_TYPE_USER,
]
],
[
'label' => 'Test One',
'value' => [
'shareType' => Share::SHARE_TYPE_USER,
'shareWith' => 'test1',
'userType' => User::USER_TYPE_USER,
]
],
[
'label' => 'Test Two',
'value' => [
'shareType' => Share::SHARE_TYPE_USER,
'shareWith' => 'test2',
'userType' => User::USER_TYPE_USER,
]
],
],
[],
false,
false,
false,
false,
'id',
0
],
];
}

Expand All @@ -544,6 +591,7 @@ public function dataGetUsers() {
* @param mixed $shareeEnumerationGroupMembers restrict enumeration to group members
* @param mixed $additionalUserInfoField
* @param string $usersAutoCompletePreference
* @param int $searchMinLength the configured min length for user search
*/
public function testGetUsers(
$searchTerm,
Expand All @@ -557,7 +605,8 @@ public function testGetUsers(
$singleUser,
$shareeEnumerationGroupMembers = false,
$additionalUserInfoField = null,
$usersAutoCompletePreference = 'yes'
$usersAutoCompletePreference = 'yes',
$searchMinLength = 0
) {
$this->config->expects($this->once())
->method('getAppValue')
Expand All @@ -571,6 +620,9 @@ public function testGetUsers(
'yes'
)->willReturn($usersAutoCompletePreference);

$this->userSearch->method('getSearchMinLength')
->willReturn($searchMinLength);

$this->sharees = new ShareesController(
'files_sharing',
$this->request,
Expand All @@ -596,41 +648,43 @@ public function testGetUsers(
->method('getUser')
->willReturn($user);

if (!$shareWithGroupOnly && !$shareeEnumerationGroupMembers) {
$this->userManager->expects($this->once())
->method('find')
->with($searchTerm, $this->invokePrivate($this->sharees, 'limit'), $this->invokePrivate($this->sharees, 'offset'))
->willReturn($userResponse);
} else {
if ($singleUser !== false && !$shareeEnumerationGroupMembers) {
// first call is for the current user's group memberships
// second call happens later for an exact match to check whether
// that match also is member of the same groups
$this->groupManager->expects($this->exactly(2))
->method('getUserGroupIds')
->withConsecutive(
[$user],
[$singleUser]
)
->willReturn($groupResponse);
if ($searchTerm !== '' || $searchMinLength === 0) {
if (!$shareWithGroupOnly && !$shareeEnumerationGroupMembers) {
$this->userManager->expects($this->once())
->method('find')
->with($searchTerm, $this->invokePrivate($this->sharees, 'limit'), $this->invokePrivate($this->sharees, 'offset'))
->willReturn($userResponse);
} else {
$this->groupManager->expects($this->once())
->method('getUserGroupIds')
->with($user)
->willReturn($groupResponse);
}
if ($singleUser !== false && !$shareeEnumerationGroupMembers) {
// first call is for the current user's group memberships
// second call happens later for an exact match to check whether
// that match also is member of the same groups
$this->groupManager->expects($this->exactly(2))
->method('getUserGroupIds')
->withConsecutive(
[$user],
[$singleUser]
)
->willReturn($groupResponse);
} else {
$this->groupManager->expects($this->once())
->method('getUserGroupIds')
->with($user)
->willReturn($groupResponse);
}

$this->groupManager->expects($this->exactly(\sizeof($groupResponse)))
->method('findUsersInGroup')
->with($this->anything(), $searchTerm, $this->invokePrivate($this->sharees, 'limit'), $this->invokePrivate($this->sharees, 'offset'))
->willReturnMap($userResponse);
}
$this->groupManager->expects($this->exactly(\sizeof($groupResponse)))
->method('findUsersInGroup')
->with($this->anything(), $searchTerm, $this->invokePrivate($this->sharees, 'limit'), $this->invokePrivate($this->sharees, 'offset'))
->willReturnMap($userResponse);
}

if ($singleUser !== false) {
$this->userManager->expects($this->once())
->method('get')
->with($searchTerm)
->willReturn($singleUser);
if ($singleUser !== false) {
$this->userManager->expects($this->once())
->method('get')
->with($searchTerm)
->willReturn($singleUser);
}
}

$this->invokePrivate($this->sharees, 'getUsers', [$searchTerm]);
Expand Down Expand Up @@ -990,6 +1044,26 @@ public function dataGetGroups() {
false,
['test', 'test1']
],
// Test empty $searchTerm should not return any groups.
['', false, true, [], [], [], [], false, false, [], 2],
['', false, true,
[
$this->getGroupMock('test'),
$this->getGroupMock('test0'),
$this->getGroupMock('test1'),
],
[$this->getGroupMock('test'), $this->getGroupMock('test0'), $this->getGroupMock('test1')],
[],
[
['label' => 'test', 'value' => ['shareType' => Share::SHARE_TYPE_GROUP, 'shareWith' => 'test']],
['label' => 'test0', 'value' => ['shareType' => Share::SHARE_TYPE_GROUP, 'shareWith' => 'test0']],
['label' => 'test1', 'value' => ['shareType' => Share::SHARE_TYPE_GROUP, 'shareWith' => 'test1']],
],
false,
false,
[],
0
],
];
}

Expand All @@ -1006,6 +1080,7 @@ public function dataGetGroups() {
* @param bool $reachedEnd
* @param bool $shareeEnumerationGroupMembers
* @param array $blacklistedGroupNames list with the names of the blacklisted groups
* @param int $searchMinLength the min length of the search term
*/
public function testGetGroups(
$searchTerm,
Expand All @@ -1017,45 +1092,50 @@ public function testGetGroups(
$expected,
$reachedEnd,
$shareeEnumerationGroupMembers = false,
$blacklistedGroupNames = []
$blacklistedGroupNames = [],
$searchMinLength = 0
) {
$this->userSearch->method('getSearchMinLength')
->willReturn($searchMinLength);
$this->invokePrivate($this->sharees, 'limit', [2]);
$this->invokePrivate($this->sharees, 'offset', [0]);
$this->invokePrivate($this->sharees, 'shareWithMembershipGroupOnly', [$shareWithMembershipGroupOnly]);
$this->invokePrivate($this->sharees, 'shareeEnumeration', [$shareeEnumeration]);
$this->invokePrivate($this->sharees, 'shareeEnumerationGroupMembers', [$shareeEnumerationGroupMembers]);

$this->groupManager->expects($this->once())
->method('search')
->with($searchTerm, $this->invokePrivate($this->sharees, 'limit'), $this->invokePrivate($this->sharees, 'offset'))
->willReturn($groupResponse);

$getGroupValueMap = \array_map(function ($group) {
return [$group->getGID(), $group];
}, $groupResponse);

$this->groupManager->method('get')
->will($this->returnValueMap($getGroupValueMap));

if ($shareWithMembershipGroupOnly || $shareeEnumerationGroupMembers) {
$user = $this->getUserMock('admin', 'Administrator');
$this->session->expects($this->any())
->method('getUser')
->willReturn($user);

$numGetUserGroupsCalls = empty($groupResponse) ? 0 : 1;
$this->groupManager->expects($this->exactly($numGetUserGroupsCalls))
->method('getUserGroups')
->with($user)
->willReturn($userGroupsResponse);
}
if ($searchTerm !== '' || $searchMinLength === 0) {
$this->groupManager->expects($this->once())
->method('search')
->with($searchTerm, $this->invokePrivate($this->sharees, 'limit'), $this->invokePrivate($this->sharees, 'offset'))
->willReturn($groupResponse);

$getGroupValueMap = \array_map(function ($group) {
return [$group->getGID(), $group];
}, $groupResponse);

$this->groupManager->method('get')
->will($this->returnValueMap($getGroupValueMap));

if ($shareWithMembershipGroupOnly || $shareeEnumerationGroupMembers) {
$user = $this->getUserMock('admin', 'Administrator');
$this->session->expects($this->any())
->method('getUser')
->willReturn($user);

// don't care about the particular implementation of the method
// just mark the group as blacklisted based on the displayname
$this->sharingBlacklist->method('isGroupBlacklisted')
->will($this->returnCallback(function (IGroup $group) use ($blacklistedGroupNames) {
return \in_array($group->getDisplayName(), $blacklistedGroupNames, true);
}));
$numGetUserGroupsCalls = empty($groupResponse) ? 0 : 1;
$this->groupManager->expects($this->exactly($numGetUserGroupsCalls))
->method('getUserGroups')
->with($user)
->willReturn($userGroupsResponse);
}

// don't care about the particular implementation of the method
// just mark the group as blacklisted based on the displayname
$this->sharingBlacklist->method('isGroupBlacklisted')
->will($this->returnCallback(function (IGroup $group) use ($blacklistedGroupNames) {
return \in_array($group->getDisplayName(), $blacklistedGroupNames, true);
}));
}

$this->invokePrivate($this->sharees, 'getGroups', [$searchTerm]);
$result = $this->invokePrivate($this->sharees, 'result');
Expand Down
5 changes: 5 additions & 0 deletions changelog/unreleased/38489
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Bugfix: Fix behavior for user search at the API level

The 'user.search_min_length' restriction could be circumvented when accessing the API directly.

https://github.com/owncloud/core/pull/38489