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
5 changes: 4 additions & 1 deletion apps/files_sharing/lib/Capabilities.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -21,6 +23,7 @@
class Capabilities implements ICapability {
public function __construct(
private IConfig $config,
private readonly IAppConfig $appConfig,
private IManager $shareManager,
private IAppManager $appManager,
) {
Expand Down Expand Up @@ -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'] = [];
Expand Down
34 changes: 28 additions & 6 deletions apps/files_sharing/tests/CapabilitiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
}
Expand Down Expand Up @@ -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']);
Expand Down Expand Up @@ -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']);
Expand Down
2 changes: 1 addition & 1 deletion apps/settings/lib/Settings/Admin/Sharing.php
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
4 changes: 2 additions & 2 deletions apps/settings/tests/Settings/Admin/SharingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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'],
Expand Down
13 changes: 13 additions & 0 deletions core/AppInfo/ConfigLexicon.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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'),
];
}
Expand Down
3 changes: 3 additions & 0 deletions core/Controller/OCJSController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -43,6 +44,7 @@ public function __construct(
ISession $session,
IUserSession $userSession,
IConfig $config,
IAppConfig $appConfig,
IGroupManager $groupManager,
IniGetWrapper $iniWrapper,
IURLGenerator $urlGenerator,
Expand All @@ -62,6 +64,7 @@ public function __construct(
$session,
$userSession->getUser(),
$config,
$appConfig,
$groupManager,
$iniWrapper,
$urlGenerator,
Expand Down
2 changes: 1 addition & 1 deletion lib/private/Share20/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down
6 changes: 4 additions & 2 deletions lib/private/Template/JSConfigHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 3 additions & 0 deletions lib/private/TemplateLayout.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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(),
Expand Down
5 changes: 4 additions & 1 deletion tests/lib/Share20/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down
4 changes: 4 additions & 0 deletions tests/lib/TemplateLayoutTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
Loading