diff --git a/apps/files_sharing/lib/Capabilities.php b/apps/files_sharing/lib/Capabilities.php index cbb9b5cd2f263..06aa1271c8f8d 100644 --- a/apps/files_sharing/lib/Capabilities.php +++ b/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 @@ class Capabilities implements ICapability { public function __construct( private IConfig $config, + private readonly IAppConfig $appConfig, private IManager $shareManager, private IAppManager $appManager, ) { @@ -111,7 +114,7 @@ public function getCapabilities() { 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'] = []; diff --git a/apps/files_sharing/tests/CapabilitiesTest.php b/apps/files_sharing/tests/CapabilitiesTest.php index 2fe221703a591..9a076d7a171a1 100644 --- a/apps/files_sharing/tests/CapabilitiesTest.php +++ b/apps/files_sharing/tests/CapabilitiesTest.php @@ -56,11 +56,30 @@ private function getFilesSharingPart(array $data) { * @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 @@ private function getResults(array $map, bool $federationEnabled = true) { $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 @@ public function testLinkPassword(): void { ['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 @@ public function testFederatedSharingExpirationDate(): void { } 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']); diff --git a/apps/settings/lib/Settings/Admin/Sharing.php b/apps/settings/lib/Settings/Admin/Sharing.php index e038b2a623148..ec5dcdf624d2f 100644 --- a/apps/settings/lib/Settings/Admin/Sharing.php +++ b/apps/settings/lib/Settings/Admin/Sharing.php @@ -68,7 +68,7 @@ public function getForm() { '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'), diff --git a/apps/settings/tests/Settings/Admin/SharingTest.php b/apps/settings/tests/Settings/Admin/SharingTest.php index 12ab5c3cadac2..f37ade2171f39 100644 --- a/apps/settings/tests/Settings/Admin/SharingTest.php +++ b/apps/settings/tests/Settings/Admin/SharingTest.php @@ -57,7 +57,8 @@ public function testGetFormWithoutExcludedGroups(): void { $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 @@ public function testGetFormWithoutExcludedGroups(): void { ['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'], diff --git a/core/AppInfo/ConfigLexicon.php b/core/AppInfo/ConfigLexicon.php index df8243019ad3b..8f1d89eaace4b 100644 --- a/core/AppInfo/ConfigLexicon.php +++ b/core/AppInfo/ConfigLexicon.php @@ -22,6 +22,9 @@ 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 @@ public function getAppConfigs(): array { 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, 'Ask for a password when sharing document by default'), + 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'), ]; } diff --git a/core/Controller/OCJSController.php b/core/Controller/OCJSController.php index ea372b43b2e37..083ad4b209f4e 100644 --- a/core/Controller/OCJSController.php +++ b/core/Controller/OCJSController.php @@ -20,6 +20,7 @@ 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 @@ public function __construct( ISession $session, IUserSession $userSession, IConfig $config, + IAppConfig $appConfig, IGroupManager $groupManager, IniGetWrapper $iniWrapper, IURLGenerator $urlGenerator, @@ -62,6 +64,7 @@ public function __construct( $session, $userSession->getUser(), $config, + $appConfig, $groupManager, $iniWrapper, $urlGenerator, diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 855bb173d5663..28f29d6b20f29 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -1778,7 +1778,7 @@ public function shareApiLinkEnforcePassword(bool $checkGroupMembership = true) { } } } - return $this->config->getAppValue('core', 'shareapi_enforce_links_password', 'no') === 'yes'; + return $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_PASSWORD_ENFORCED); } /** diff --git a/lib/private/Template/JSConfigHelper.php b/lib/private/Template/JSConfigHelper.php index 07e557d070678..044fa8147a093 100644 --- a/lib/private/Template/JSConfigHelper.php +++ b/lib/private/Template/JSConfigHelper.php @@ -10,6 +10,7 @@ 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\Constants; use OCP\Defaults; use OCP\Files\FileInfo; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IGroupManager; use OCP\IInitialStateService; @@ -50,6 +52,7 @@ public function __construct( 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 @@ public function getConfig(): string { } } - $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) { diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php index cfc387d216479..42861eddc0dde 100644 --- a/lib/private/TemplateLayout.php +++ b/lib/private/TemplateLayout.php @@ -20,6 +20,7 @@ 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 @@ public function getPageTemplate(string $renderAs, string $appId): ITemplate { \OC::$server->getSession(), \OC::$server->getUserSession()->getUser(), $this->config, + $this->appConfig, \OC::$server->getGroupManager(), \OC::$server->get(IniGetWrapper::class), \OC::$server->getURLGenerator(), diff --git a/tests/lib/Share20/ManagerTest.php b/tests/lib/Share20/ManagerTest.php index 7859407651fc6..0e37934786ae9 100644 --- a/tests/lib/Share20/ManagerTest.php +++ b/tests/lib/Share20/ManagerTest.php @@ -722,7 +722,10 @@ public function testVerifyPasswordNullButEnforced(): void { $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]); diff --git a/tests/lib/TemplateLayoutTest.php b/tests/lib/TemplateLayoutTest.php index c1cafcd6b931b..ce5d2f6dd0bd2 100644 --- a/tests/lib/TemplateLayoutTest.php +++ b/tests/lib/TemplateLayoutTest.php @@ -13,6 +13,7 @@ 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 @@ 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 @@ protected function setUp(): void { 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 @@ public function testVersionHash($path, $file, $installed, $debug, $expected): vo ->onlyMethods(['getAppNamefromPath']) ->setConstructorArgs([ $this->config, + $this->appConfig, $this->appManager, $this->initialState, $this->navigationManager,