diff --git a/CHANGELOG.md b/CHANGELOG.md index f6bc5f4a3fae..c334445341a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) @@ -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 diff --git a/apps/files_external/l10n/gl.js b/apps/files_external/l10n/gl.js index 1e1b9af74f5b..8e79514eb0a9 100644 --- a/apps/files_external/l10n/gl.js +++ b/apps/files_external/l10n/gl.js @@ -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", diff --git a/apps/files_external/l10n/gl.json b/apps/files_external/l10n/gl.json index 24ee39de433b..8d2b1af8cb50 100644 --- a/apps/files_external/l10n/gl.json +++ b/apps/files_external/l10n/gl.json @@ -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", diff --git a/apps/files_sharing/lib/Controller/ShareesController.php b/apps/files_sharing/lib/Controller/ShareesController.php index e704277f7d5d..395269673276 100644 --- a/apps/files_sharing/lib/Controller/ShareesController.php +++ b/apps/files_sharing/lib/Controller/ShareesController.php @@ -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 @@ -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(); diff --git a/apps/files_sharing/tests/API/ShareesTest.php b/apps/files_sharing/tests/API/ShareesTest.php index d6da2cdab4ac..4da03264a080 100644 --- a/apps/files_sharing/tests/API/ShareesTest.php +++ b/apps/files_sharing/tests/API/ShareesTest.php @@ -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 + ], ]; } @@ -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, @@ -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') @@ -571,6 +620,9 @@ public function testGetUsers( 'yes' )->willReturn($usersAutoCompletePreference); + $this->userSearch->method('getSearchMinLength') + ->willReturn($searchMinLength); + $this->sharees = new ShareesController( 'files_sharing', $this->request, @@ -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]); @@ -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 + ], ]; } @@ -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, @@ -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'); diff --git a/changelog/unreleased/38489 b/changelog/unreleased/38489 new file mode 100644 index 000000000000..9b878f92f3d1 --- /dev/null +++ b/changelog/unreleased/38489 @@ -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