feat(preset): share password protection

Signed-off-by: Maxence Lange <maxence@artificial-owl.com>

apps/files_sharing/lib/Capabilities.php

@@ -7,9 +7,11 @@
  */
 namespace OCA\Files_Sharing;
 
+use OC\Core\AppInfo\ConfigLexicon;
 use OCP\App\IAppManager;
 use OCP\Capabilities\ICapability;
 use OCP\Constants;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\Share\IManager;
 
@@ -21,6 +23,7 @@ use OCP\Share\IManager;
 class Capabilities implements ICapability {
 	public function __construct(
 		private IConfig $config,
+		private readonly IAppConfig $appConfig,
 		private IManager $shareManager,
 		private IAppManager $appManager,
 	) {
@@ -111,7 +114,7 @@ class Capabilities implements ICapability {
 				if ($public['password']['enforced']) {
 					$public['password']['askForOptionalPassword'] = false;
 				} else {
-					$public['password']['askForOptionalPassword'] = ($this->config->getAppValue('core', 'shareapi_enable_link_password_by_default', 'no') === 'yes');
+					$public['password']['askForOptionalPassword'] = $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_PASSWORD_DEFAULT);
 				}
 
 				$public['expire_date'] = [];

apps/files_sharing/tests/CapabilitiesTest.php

@@ -56,11 +56,30 @@ class CapabilitiesTest extends \Test\TestCase {
 	 * @param (string[])[] $map Map of arguments to return types for the getAppValue function in the mock
 	 * @return string[]
 	 */
-	private function getResults(array $map, bool $federationEnabled = true) {
+	private function getResults(array $map, array $typedMap = [], bool $federationEnabled = true) {
 		$config = $this->getMockBuilder(IConfig::class)->disableOriginalConstructor()->getMock();
 		$appManager = $this->getMockBuilder(IAppManager::class)->disableOriginalConstructor()->getMock();
 		$config->method('getAppValue')->willReturnMap($map);
 		$appManager->method('isEnabledForAnyone')->with('federation')->willReturn($federationEnabled);
+
+		if (empty($typedMap)) {
+			$appConfig = $this->createMock(IAppConfig::class);
+		} else {
+			// hack to help transition from old IConfig to new IAppConfig
+			$appConfig = $this->getMockBuilder(IAppConfig::class)->disableOriginalConstructor()->getMock();
+			$appConfig->expects($this->any())->method('getValueBool')->willReturnCallback(function (...$args) use ($typedMap): bool {
+				foreach ($typedMap as $entry) {
+					if ($entry[0] !== $args[0] || $entry[1] !== $args[1]) {
+						continue;
+					}
+
+					return $entry[2];
+				}
+
+				return false;
+			});
+		}
+
 		$shareManager = new Manager(
 			$this->createMock(LoggerInterface::class),
 			$config,
@@ -80,9 +99,10 @@ class CapabilitiesTest extends \Test\TestCase {
 			$this->createMock(KnownUserService::class),
 			$this->createMock(ShareDisableChecker::class),
 			$this->createMock(IDateTimeZone::class),
-			$this->createMock(IAppConfig::class),
+			$appConfig,
 		);
-		$cap = new Capabilities($config, $shareManager, $appManager);
+
+		$cap = new Capabilities($config, $appConfig, $shareManager, $appManager);
 		$result = $this->getFilesSharingPart($cap->getCapabilities());
 		return $result;
 	}
@@ -135,9 +155,11 @@ class CapabilitiesTest extends \Test\TestCase {
 			['core', 'shareapi_enabled', 'yes', 'yes'],
 			['core', 'shareapi_allow_links', 'yes', 'yes'],
 			['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
-			['core', 'shareapi_enforce_links_password', 'no', 'yes'],
 		];
-		$result = $this->getResults($map);
+		$typedMap = [
+			['core', 'shareapi_enforce_links_password', true],
+		];
+		$result = $this->getResults($map, $typedMap);
 		$this->assertArrayHasKey('password', $result['public']);
 		$this->assertArrayHasKey('enforced', $result['public']['password']);
 		$this->assertTrue($result['public']['password']['enforced']);
@@ -328,7 +350,7 @@ class CapabilitiesTest extends \Test\TestCase {
 	}
 
 	public function testFederatedSharingDisabled(): void {
-		$result = $this->getResults([], false);
+		$result = $this->getResults([], federationEnabled: false);
 		$this->assertArrayHasKey('federation', $result);
 		$this->assertFalse($result['federation']['incoming']);
 		$this->assertFalse($result['federation']['outgoing']);

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

@@ -68,7 +68,7 @@ class Sharing implements IDelegatedSettings {
 			'excludeGroups' => $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no'),
 			'excludeGroupsList' => json_decode($excludedGroups, true) ?? [],
 			'publicShareDisclaimerText' => $this->config->getAppValue('core', 'shareapi_public_link_disclaimertext'),
-			'enableLinkPasswordByDefault' => $this->getHumanBooleanConfig('core', 'shareapi_enable_link_password_by_default'),
+			'enableLinkPasswordByDefault' => $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_PASSWORD_DEFAULT),
 			'defaultPermissions' => (int)$this->config->getAppValue('core', 'shareapi_default_permissions', (string)Constants::PERMISSION_ALL),
 			'defaultInternalExpireDate' => $this->getHumanBooleanConfig('core', 'shareapi_default_internal_expire_date'),
 			'internalExpireAfterNDays' => $this->config->getAppValue('core', 'shareapi_internal_expire_after_n_days', '7'),

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

@@ -57,7 +57,8 @@ class SharingTest extends TestCase {
 		$this->appConfig
 			->method('getValueBool')
 			->willReturnMap([
-				['core', 'shareapi_allow_federation_on_public_shares', false, false, true],
+				['core', 'shareapi_allow_federation_on_public_shares', true],
+				['core', 'shareapi_enable_link_password_by_default', true],
 			]);
 
 		$this->config
@@ -82,7 +83,6 @@ class SharingTest extends TestCase {
 				['core', 'shareapi_enforce_expire_date', 'no', 'no'],
 				['core', 'shareapi_exclude_groups', 'no', 'no'],
 				['core', 'shareapi_public_link_disclaimertext', '', 'Lorem ipsum'],
-				['core', 'shareapi_enable_link_password_by_default', 'no', 'yes'],
 				['core', 'shareapi_default_permissions', (string)Constants::PERMISSION_ALL, Constants::PERMISSION_ALL],
 				['core', 'shareapi_default_internal_expire_date', 'no', 'no'],
 				['core', 'shareapi_internal_expire_after_n_days', '7', '7'],

core/AppInfo/ConfigLexicon.php

@@ -22,6 +22,9 @@ use OCP\Config\ValueType;
 class ConfigLexicon implements ILexicon {
 	public const SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES = 'shareapi_allow_federation_on_public_shares';
 	public const SHARE_CUSTOM_TOKEN = 'shareapi_allow_custom_tokens';
+	public const SHARE_LINK_PASSWORD_DEFAULT = 'shareapi_enable_link_password_by_default';
+	public const SHARE_LINK_PASSWORD_ENFORCED = 'shareapi_enforce_links_password';
+
 	public const USER_LANGUAGE = 'lang';
 	public const LASTCRON_TIMESTAMP = 'lastcron';
 
@@ -49,6 +52,16 @@ class ConfigLexicon implements ILexicon {
 				lazy: true,
 				note: 'Shares with guessable tokens may be accessed easily. Shares with custom tokens will continue to be accessible after this setting has been disabled.',
 			),
+			new Entry(self::SHARE_LINK_PASSWORD_DEFAULT, ValueType::BOOL, false, 'Always ask for a password when sharing document'),
+			new Entry(
+				key: self::SHARE_LINK_PASSWORD_ENFORCED,
+				type: ValueType::BOOL,
+				defaultRaw: fn (Preset $p): bool => match ($p) {
+					Preset::SCHOOL, Preset::UNIVERSITY, Preset::SHARED, Preset::SMALL, Preset::MEDIUM, Preset::LARGE => true,
+					default => false,
+				},
+				definition: 'Enforce password protection when sharing document'
+			),
 			new Entry(self::LASTCRON_TIMESTAMP, ValueType::INT, 0, 'timestamp of last cron execution'),
 		];
 	}

core/Controller/OCJSController.php

@@ -20,6 +20,7 @@ use OCP\AppFramework\Http\Attribute\OpenAPI;
 use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\DataDisplayResponse;
 use OCP\Defaults;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IInitialStateService;
@@ -43,6 +44,7 @@ class OCJSController extends Controller {
 		ISession $session,
 		IUserSession $userSession,
 		IConfig $config,
+		IAppConfig $appConfig,
 		IGroupManager $groupManager,
 		IniGetWrapper $iniWrapper,
 		IURLGenerator $urlGenerator,
@@ -62,6 +64,7 @@ class OCJSController extends Controller {
 			$session,
 			$userSession->getUser(),
 			$config,
+			$appConfig,
 			$groupManager,
 			$iniWrapper,
 			$urlGenerator,

lib/private/Share20/Manager.php

@@ -1778,7 +1778,7 @@ class Manager implements IManager {
 				}
 			}
 		}
-		return $this->config->getAppValue('core', 'shareapi_enforce_links_password', 'no') === 'yes';
+		return $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_PASSWORD_ENFORCED);
 	}
 
 	/**

lib/private/Template/JSConfigHelper.php

@@ -10,6 +10,7 @@ namespace OC\Template;
 use bantu\IniGetWrapper\IniGetWrapper;
 use OC\Authentication\Token\IProvider;
 use OC\CapabilitiesManager;
+use OC\Core\AppInfo\ConfigLexicon;
 use OC\Files\FilenameValidator;
 use OC\Share\Share;
 use OCA\Provisioning_API\Controller\AUserDataOCSController;
@@ -22,6 +23,7 @@ use OCP\Authentication\Token\IToken;
 use OCP\Constants;
 use OCP\Defaults;
 use OCP\Files\FileInfo;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IGroupManager;
 use OCP\IInitialStateService;
@@ -50,6 +52,7 @@ class JSConfigHelper {
 		protected ISession $session,
 		protected ?IUser $currentUser,
 		protected IConfig $config,
+		protected readonly IAppConfig $appConfig,
 		protected IGroupManager $groupManager,
 		protected IniGetWrapper $iniWrapper,
 		protected IURLGenerator $urlGenerator,
@@ -94,8 +97,7 @@ class JSConfigHelper {
 			}
 		}
 
-		$enableLinkPasswordByDefault = $this->config->getAppValue('core', 'shareapi_enable_link_password_by_default', 'no');
-		$enableLinkPasswordByDefault = $enableLinkPasswordByDefault === 'yes';
+		$enableLinkPasswordByDefault = $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_PASSWORD_DEFAULT);
 		$defaultExpireDateEnabled = $this->config->getAppValue('core', 'shareapi_default_expire_date', 'no') === 'yes';
 		$defaultExpireDate = $enforceDefaultExpireDate = null;
 		if ($defaultExpireDateEnabled) {

lib/private/TemplateLayout.php

@@ -20,6 +20,7 @@ use OC\Template\JSResourceLocator;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\Defaults;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\IInitialStateService;
 use OCP\INavigationManager;
@@ -44,6 +45,7 @@ class TemplateLayout {
 
 	public function __construct(
 		private IConfig $config,
+		private readonly IAppConfig $appConfig,
 		private IAppManager $appManager,
 		private InitialStateService $initialState,
 		private INavigationManager $navigationManager,
@@ -223,6 +225,7 @@ class TemplateLayout {
 				\OC::$server->getSession(),
 				\OC::$server->getUserSession()->getUser(),
 				$this->config,
+				$this->appConfig,
 				\OC::$server->getGroupManager(),
 				\OC::$server->get(IniGetWrapper::class),
 				\OC::$server->getURLGenerator(),

tests/lib/Share20/ManagerTest.php

@@ -722,7 +722,10 @@ class ManagerTest extends \Test\TestCase {
 
 		$this->config->method('getAppValue')->willReturnMap([
 			['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
-			['core', 'shareapi_enforce_links_password', 'no', 'yes'],
+		]);
+
+		$this->appConfig->method('getValueBool')->willReturnMap([
+			['core', 'shareapi_enforce_links_password', true],
 		]);
 
 		self::invokePrivate($this->manager, 'verifyPassword', [null]);

tests/lib/TemplateLayoutTest.php

@@ -13,6 +13,7 @@ use OC\InitialStateService;
 use OC\TemplateLayout;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Http\TemplateResponse;
+use OCP\IAppConfig;
 use OCP\IConfig;
 use OCP\INavigationManager;
 use OCP\ServerVersion;
@@ -21,6 +22,7 @@ use PHPUnit\Framework\MockObject\MockObject;
 
 class TemplateLayoutTest extends \Test\TestCase {
 	private IConfig&MockObject $config;
+	private IAppConfig&MockObject $appConfig;
 	private IAppManager&MockObject $appManager;
 	private InitialStateService&MockObject $initialState;
 	private INavigationManager&MockObject $navigationManager;
@@ -33,6 +35,7 @@ class TemplateLayoutTest extends \Test\TestCase {
 		parent::setUp();
 
 		$this->config = $this->createMock(IConfig::class);
+		$this->appConfig = $this->createMock(IAppConfig::class);
 		$this->appManager = $this->createMock(IAppManager::class);
 		$this->initialState = $this->createMock(InitialStateService::class);
 		$this->navigationManager = $this->createMock(INavigationManager::class);
@@ -68,6 +71,7 @@ class TemplateLayoutTest extends \Test\TestCase {
 			->onlyMethods(['getAppNamefromPath'])
 			->setConstructorArgs([
 				$this->config,
+				$this->appConfig,
 				$this->appManager,
 				$this->initialState,
 				$this->navigationManager,