[stable31] fix: add missing sharing options to ui and add full-match results

apps/settings/lib/Settings/Admin/Sharing.php

@@ -37,6 +37,8 @@ public function getForm() {
 		$excludedPasswordGroups = $this->config->getAppValue('core', 'shareapi_enforce_links_password_excluded_groups', '');
 		$onlyShareWithGroupMembersExcludeGroupList = $this->config->getAppValue('core', 'shareapi_only_share_with_group_members_exclude_group_list', '');
 
+		/** @var \OC\Share20\Manager */
+		$share20Manager = $this->shareManager;
 		$parameters = [
 			// Built-In Sharing
 			'enabled' => $this->getHumanBooleanConfig('core', 'shareapi_enabled', true),
@@ -48,10 +50,10 @@ public function getForm() {
 			'allowShareDialogUserEnumeration' => $this->getHumanBooleanConfig('core', 'shareapi_allow_share_dialog_user_enumeration', true),
 			'restrictUserEnumerationToGroup' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_to_group'),
 			'restrictUserEnumerationToPhone' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_to_phone'),
-			'restrictUserEnumerationFullMatch' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_full_match', true),
-			'restrictUserEnumerationFullMatchUserId' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_full_match_userid', true),
-			'restrictUserEnumerationFullMatchEmail' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_full_match_email', true),
-			'restrictUserEnumerationFullMatchIgnoreSecondDN' => $this->getHumanBooleanConfig('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_dn'),
+			'restrictUserEnumerationFullMatch' => $this->shareManager->allowEnumerationFullMatch(),
+			'restrictUserEnumerationFullMatchUserId' => $share20Manager->matchUserId(),
+			'restrictUserEnumerationFullMatchEmail' => $this->shareManager->matchEmail(),
+			'restrictUserEnumerationFullMatchIgnoreSecondDN' => $this->shareManager->ignoreSecondDisplayName(),
 			'enforceLinksPassword' => Util::isPublicLinkPasswordRequired(false),
 			'enforceLinksPasswordExcludedGroups' => json_decode($excludedPasswordGroups) ?? [],
 			'enforceLinksPasswordExcludedGroupsEnabled' => $this->config->getSystemValueBool('sharing.allow_disabled_password_enforcement_groups', false),

apps/settings/src/components/AdminSettingsSharingForm.vue

@@ -4,7 +4,7 @@
 -->
 <template>
 	<form class="sharing">
-		<NcCheckboxRadioSwitch aria-controls="settings-sharing-api settings-sharing-api-settings settings-sharing-default-permissions settings-sharing-privary-related"
+		<NcCheckboxRadioSwitch aria-controls="settings-sharing-api settings-sharing-api-settings settings-sharing-default-permissions settings-sharing-privacy-related"
 			type="switch"
 			:checked.sync="settings.enabled">
 			{{ t('settings', 'Allow apps to use the Share API') }}
@@ -161,7 +161,7 @@
 			</fieldset>
 		</div>
 
-		<div v-show="settings.enabled" id="settings-sharing-privary-related" class="sharing__section">
+		<div v-show="settings.enabled" id="settings-sharing-privacy-related" class="sharing__section">
 			<h3>{{ t('settings', 'Privacy settings for sharing') }}</h3>
 
 			<NcCheckboxRadioSwitch type="switch"
@@ -170,33 +170,52 @@
 				{{ t('settings', 'Allow account name autocompletion in share dialog and allow access to the system address book') }}
 			</NcCheckboxRadioSwitch>
 			<fieldset v-show="settings.allowShareDialogUserEnumeration" id="settings-sharing-privacy-user-enumeration" class="sharing__sub-section">
+				<legend class="hidden-visually">
+					{{ t('settings', 'Sharing autocompletion restrictions') }}
+				</legend>
 				<em>
-					{{ t('settings', 'If autocompletion "same group" and "phone number integration" are enabled a match in either is enough to show the user.') }}
+					{{ t('settings', 'If autocompletion restrictions for both "same group" and "phonebook integration" are enabled, a match in either is enough to show the user.') }}
 				</em>
 				<NcCheckboxRadioSwitch :checked.sync="settings.restrictUserEnumerationToGroup">
 					{{ t('settings', 'Restrict account name autocompletion and system address book access to users within the same groups') }}
 				</NcCheckboxRadioSwitch>
 				<NcCheckboxRadioSwitch :checked.sync="settings.restrictUserEnumerationToPhone">
-					{{ t('settings', 'Restrict account name autocompletion to users based on phone number integration') }}
+					{{ t('settings', 'Restrict account name autocompletion to users based on their phonebook') }}
 				</NcCheckboxRadioSwitch>
 			</fieldset>
 
-			<NcCheckboxRadioSwitch type="switch" :checked.sync="settings.restrictUserEnumerationFullMatch">
-				{{ t('settings', 'Allow autocompletion when entering the full name or email address (ignoring missing phonebook match and being in the same group)') }}
+			<NcCheckboxRadioSwitch v-model="settings.restrictUserEnumerationFullMatch"
+				type="switch"
+				aria-controls="settings-sharing-privacy-autocomplete">
+				{{ t('settings', 'Allow autocompletion to full match when entering the full name (ignoring restrictions like group membership or missing phonebook match)') }}
 			</NcCheckboxRadioSwitch>
+			<fieldset v-show="settings.restrictUserEnumerationFullMatch" id="settings-sharing-privacy-autocomplete" class="sharing__sub-section">
+				<legend class="hidden-visually">
+					{{ t('settings', 'Full match autocompletion restrictions') }}
+				</legend>
+				<NcCheckboxRadioSwitch :checked.sync="settings.restrictUserEnumerationFullMatchUserId">
+					{{ t('settings', 'Also allow autocompletion on full match of the user id') }}
+				</NcCheckboxRadioSwitch>
+				<NcCheckboxRadioSwitch :checked.sync="settings.restrictUserEnumerationFullMatchEmail">
+					{{ t('settings', 'Also allow autocompletion on full match of the user email') }}
+				</NcCheckboxRadioSwitch>
+				<NcCheckboxRadioSwitch :checked.sync="settings.restrictUserEnumerationFullMatchIgnoreSecondDN">
+					{{ t('settings', 'Do not use second user displayname for full match') }}
+				</NcCheckboxRadioSwitch>
+			</fieldset>
 
 			<NcCheckboxRadioSwitch type="switch" :checked.sync="publicShareDisclaimerEnabled">
 				{{ t('settings', 'Show disclaimer text on the public link upload page (only shown when the file list is hidden)') }}
 			</NcCheckboxRadioSwitch>
 			<div v-if="publicShareDisclaimerEnabled"
-				aria-describedby="settings-sharing-privary-related-disclaimer-hint"
+				aria-describedby="settings-sharing-privacy-related-disclaimer-hint"
 				class="sharing__sub-section">
 				<NcTextArea class="sharing__input"
 					:label="t('settings', 'Disclaimer text')"
-					aria-describedby="settings-sharing-privary-related-disclaimer-hint"
+					aria-describedby="settings-sharing-privacy-related-disclaimer-hint"
 					:value="settings.publicShareDisclaimerText"
 					@update:value="onUpdateDisclaimer" />
-				<em id="settings-sharing-privary-related-disclaimer-hint" class="sharing__input">
+				<em id="settings-sharing-privacy-related-disclaimer-hint" class="sharing__input">
 					{{ t('settings', 'This text will be shown on the public link upload page when the file list is hidden.') }}
 				</em>
 			</div>

apps/settings/tests/Settings/Admin/SharingTest.php

@@ -5,6 +5,7 @@
  */
 namespace OCA\Settings\Tests\Settings\Admin;
 
+use OC\Share20\Manager;
 use OCA\Settings\Settings\Admin\Sharing;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http\TemplateResponse;
@@ -13,7 +14,6 @@
 use OCP\IConfig;
 use OCP\IL10N;
 use OCP\IURLGenerator;
-use OCP\Share\IManager;
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
@@ -24,7 +24,7 @@ class SharingTest extends TestCase {
 	private $config;
 	/** @var IL10N&MockObject */
 	private $l10n;
-	/** @var IManager|MockObject */
+	/** @var Manager|MockObject */
 	private $shareManager;
 	/** @var IAppManager|MockObject */
 	private $appManager;
@@ -38,8 +38,8 @@ protected function setUp(): void {
 		$this->config = $this->getMockBuilder(IConfig::class)->getMock();
 		$this->l10n = $this->getMockBuilder(IL10N::class)->getMock();
 
-		/** @var IManager|MockObject */
-		$this->shareManager = $this->getMockBuilder(IManager::class)->getMock();
+		/** @var Manager|MockObject */
+		$this->shareManager = $this->createMock(Manager::class);
 		/** @var IAppManager|MockObject */
 		$this->appManager = $this->getMockBuilder(IAppManager::class)->getMock();
 		/** @var IURLGenerator|MockObject */

lib/private/Collaboration/Collaborators/UserPlugin.php

@@ -62,8 +62,10 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
 		$users = [];
 		$hasMoreResults = false;
 
-		$currentUserId = $this->userSession->getUser()->getUID();
-		$currentUserGroups = $this->groupManager->getUserGroupIds($this->userSession->getUser());
+		/** @var IUser */
+		$currentUser = $this->userSession->getUser();
+		$currentUserId = $currentUser->getUID();
+		$currentUserGroups = $this->groupManager->getUserGroupIds($currentUser);
 
 		// ShareWithGroupOnly filtering
 		$currentUserGroups = array_diff($currentUserGroups, $this->shareWithGroupOnlyExcludeGroupsList);
@@ -75,7 +77,7 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
 				foreach ($usersInGroup as $userId => $displayName) {
 					$userId = (string)$userId;
 					$user = $this->userManager->get($userId);
-					if (!$user->isEnabled()) {
+					if (!$user?->isEnabled()) {
 						// Ignore disabled users
 						continue;
 					}
@@ -85,37 +87,43 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
 					$hasMoreResults = true;
 				}
 			}
+		}
 
-			if (!$this->shareWithGroupOnly && $this->shareeEnumerationPhone) {
-				$usersTmp = $this->userManager->searchKnownUsersByDisplayName($currentUserId, $search, $limit, $offset);
-				if (!empty($usersTmp)) {
+		// not limited to group only sharing
+		if (!$this->shareWithGroupOnly) {
+			if (!$this->shareeEnumerationPhone && !$this->shareeEnumerationInGroupOnly) {
+				// no restrictions, add everything
+				$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset);
+				foreach ($usersTmp as $user) {
+					if ($user->isEnabled()) { // Don't keep deactivated users
+						$users[$user->getUID()] = $user;
+					}
+				}
+			} else {
+				// make sure to add phonebook matches if configured
+				if ($this->shareeEnumerationPhone) {
+					$usersTmp = $this->userManager->searchKnownUsersByDisplayName($currentUserId, $search, $limit, $offset);
 					foreach ($usersTmp as $user) {
 						if ($user->isEnabled()) { // Don't keep deactivated users
 							$users[$user->getUID()] = $user;
 						}
 					}
-
-					uasort($users, function ($a, $b) {
-						/**
-						 * @var \OC\User\User $a
-						 * @var \OC\User\User $b
-						 */
-						return strcasecmp($a->getDisplayName(), $b->getDisplayName());
-					});
 				}
-			}
-		} else {
-			// Search in all users
-			if ($this->shareeEnumerationPhone) {
-				$usersTmp = $this->userManager->searchKnownUsersByDisplayName($currentUserId, $search, $limit, $offset);
-			} else {
-				$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset);
-			}
-			foreach ($usersTmp as $user) {
-				if ($user->isEnabled()) { // Don't keep deactivated users
-					$users[$user->getUID()] = $user;
+
+				// additionally we need to add full matches
+				if ($this->shareeEnumerationFullMatch) {
+					$usersTmp = $this->userManager->searchDisplayName($search, $limit, $offset);
+					foreach ($usersTmp as $user) {
+						if ($user->isEnabled() && mb_strtolower($user->getDisplayName()) === mb_strtolower($search)) {
+							$users[$user->getUID()] = $user;
+						}
+					}
 				}
 			}
+
+			uasort($users, function (IUser $a, IUser $b) {
+				return strcasecmp($a->getDisplayName(), $b->getDisplayName());
+			});
 		}
 
 		$this->takeOutCurrentUser($users);
@@ -147,11 +155,14 @@ public function search($search, $limit, $offset, ISearchResult $searchResult): b
 
 
 			if (
-				$this->shareeEnumerationFullMatch &&
-				$lowerSearch !== '' && (strtolower($uid) === $lowerSearch ||
-				strtolower($userDisplayName) === $lowerSearch ||
-				($this->shareeEnumerationFullMatchIgnoreSecondDisplayName && trim(strtolower(preg_replace('/ \(.*\)$/', '', $userDisplayName))) === $lowerSearch) ||
-				($this->shareeEnumerationFullMatchEmail && strtolower($userEmail ?? '') === $lowerSearch))
+				$this->shareeEnumerationFullMatch
+				&& $lowerSearch !== ''
+				&& (
+					strtolower($uid) === $lowerSearch
+					|| strtolower($userDisplayName) === $lowerSearch
+					|| ($this->shareeEnumerationFullMatchIgnoreSecondDisplayName && trim(strtolower(preg_replace('/ \(.*\)$/', '', $userDisplayName))) === $lowerSearch)
+					|| ($this->shareeEnumerationFullMatchEmail && strtolower($userEmail ?? '') === $lowerSearch)
+				)
 			) {
 				if (strtolower($uid) === $lowerSearch) {
 					$foundUserById = true;

lib/private/Share20/Manager.php

@@ -1932,6 +1932,10 @@ public function matchEmail(): bool {
 		return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_email', 'yes') === 'yes';
 	}
 
+	public function matchUserId(): bool {
+		return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_userid', 'yes') === 'yes';
+	}
+
 	public function ignoreSecondDisplayName(): bool {
 		return $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match_ignore_second_dn', 'no') === 'yes';
 	}

lib/public/Share/IManager.php

@@ -441,23 +441,26 @@ public function limitEnumerationToGroups(): bool;
 	public function limitEnumerationToPhone(): bool;
 
 	/**
-	 * Check if user enumeration is allowed to return on full match
+	 * Check if user enumeration is allowed to return also on full match
+	 * and ignore limitations to phonebook or groups.
 	 *
 	 * @return bool
 	 * @since 21.0.1
 	 */
 	public function allowEnumerationFullMatch(): bool;
 
 	/**
-	 * Check if the search should match the email
+	 * When `allowEnumerationFullMatch` is enabled and `matchEmail` is set,
+	 * then also return results for full email matches.
 	 *
 	 * @return bool
 	 * @since 25.0.0
 	 */
 	public function matchEmail(): bool;
 
 	/**
-	 * Check if the search should ignore the second in parentheses display name if there is any
+	 * When `allowEnumerationFullMatch` is enabled and `ignoreSecondDisplayName` is set,
+	 * then the search should ignore matches on the second displayname and only use the first.
 	 *
 	 * @return bool
 	 * @since 25.0.0

tests/lib/Collaboration/Collaborators/UserPluginTest.php

@@ -456,6 +456,10 @@ public function testSearch(
 			->method('getUser')
 			->willReturn($this->user);
 
+		$this->userManager->expects($this->any())
+			->method('searchDisplayName')
+			->willReturn($userResponse);
+
 		if (!$shareWithGroupOnly) {
 			if ($shareeEnumerationPhone) {
 				$this->userManager->expects($this->once())
@@ -766,10 +770,10 @@ public function testSearchEnumerationLimit($search, $userGroups, $matchingUsers,
 			->willReturnCallback(function ($search) use ($matchingUsers) {
 				$users = array_filter(
 					$matchingUsers,
-					fn ($user) => str_contains(strtolower($user['displayName']), strtolower($search))
+					fn ($user) => str_contains(strtolower($user['displayName'] ?? $user['uid']), strtolower($search))
 				);
 				return array_map(
-					fn ($user) => $this->getUserMock($user['uid'], $user['displayName']),
+					fn ($user) => $this->getUserMock($user['uid'], $user['displayName'] ?? $user['uid']),
 					$users);
 			});