Skip to content

Commit cb84ccc

Browse files
ArtificialOwlArtificialOwl
committed
feat(preset): share password protection
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
1 parent aaca29b commit cb84ccc

File tree

11 files changed

+67
-14
lines changed

11 files changed

+67
-14
lines changed

‎apps/files_sharing/lib/Capabilities.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77
*/
88
namespace OCA\Files_Sharing;
99

10+
use OC\Core\AppInfo\ConfigLexicon;
1011
use OCP\App\IAppManager;
1112
use OCP\Capabilities\ICapability;
1213
use OCP\Constants;
14+
use OCP\IAppConfig;
1315
use OCP\IConfig;
1416
use OCP\Share\IManager;
1517

@@ -21,6 +23,7 @@
2123
class Capabilities implements ICapability {
2224
public function __construct(
2325
private IConfig $config,
26+
private readonly IAppConfig $appConfig,
2427
private IManager $shareManager,
2528
private IAppManager $appManager,
2629
) {
@@ -111,7 +114,7 @@ public function getCapabilities() {
111114
if ($public['password']['enforced']) {
112115
$public['password']['askForOptionalPassword'] = false;
113116
} else {
114-
$public['password']['askForOptionalPassword'] = ($this->config->getAppValue('core', 'shareapi_enable_link_password_by_default', 'no') === 'yes');
117+
$public['password']['askForOptionalPassword'] = $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_PASSWORD_DEFAULT);
115118
}
116119

117120
$public['expire_date'] = [];

‎apps/files_sharing/tests/CapabilitiesTest.php‎

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,30 @@ private function getFilesSharingPart(array $data) {
5656
* @param (string[])[] $map Map of arguments to return types for the getAppValue function in the mock
5757
* @return string[]
5858
*/
59-
private function getResults(array $map, bool $federationEnabled = true) {
59+
private function getResults(array $map, array $typedMap = [], bool $federationEnabled = true) {
6060
$config = $this->getMockBuilder(IConfig::class)->disableOriginalConstructor()->getMock();
6161
$appManager = $this->getMockBuilder(IAppManager::class)->disableOriginalConstructor()->getMock();
6262
$config->method('getAppValue')->willReturnMap($map);
6363
$appManager->method('isEnabledForAnyone')->with('federation')->willReturn($federationEnabled);
64+
65+
if (empty($typedMap)) {
66+
$appConfig = $this->createMock(IAppConfig::class);
67+
} else {
68+
// hack to help transition from old IConfig to new IAppConfig
69+
$appConfig = $this->getMockBuilder(IAppConfig::class)->disableOriginalConstructor()->getMock();
70+
$appConfig->expects($this->any())->method('getValueBool')->willReturnCallback(function (...$args) use ($typedMap): bool {
71+
foreach ($typedMap as $entry) {
72+
if ($entry[0] !== $args[0] || $entry[1] !== $args[1]) {
73+
continue;
74+
}
75+
76+
return $entry[2];
77+
}
78+
79+
return false;
80+
});
81+
}
82+
6483
$shareManager = new Manager(
6584
$this->createMock(LoggerInterface::class),
6685
$config,
@@ -80,9 +99,10 @@ private function getResults(array $map, bool $federationEnabled = true) {
8099
$this->createMock(KnownUserService::class),
81100
$this->createMock(ShareDisableChecker::class),
82101
$this->createMock(IDateTimeZone::class),
83-
$this->createMock(IAppConfig::class),
102+
$appConfig,
84103
);
85-
$cap = new Capabilities($config, $shareManager, $appManager);
104+
105+
$cap = new Capabilities($config, $appConfig, $shareManager, $appManager);
86106
$result = $this->getFilesSharingPart($cap->getCapabilities());
87107
return $result;
88108
}
@@ -135,9 +155,11 @@ public function testLinkPassword(): void {
135155
['core', 'shareapi_enabled', 'yes', 'yes'],
136156
['core', 'shareapi_allow_links', 'yes', 'yes'],
137157
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
138-
['core', 'shareapi_enforce_links_password', 'no', 'yes'],
139158
];
140-
$result = $this->getResults($map);
159+
$typedMap = [
160+
['core', 'shareapi_enforce_links_password', true],
161+
];
162+
$result = $this->getResults($map, $typedMap);
141163
$this->assertArrayHasKey('password', $result['public']);
142164
$this->assertArrayHasKey('enforced', $result['public']['password']);
143165
$this->assertTrue($result['public']['password']['enforced']);
@@ -328,7 +350,7 @@ public function testFederatedSharingExpirationDate(): void {
328350
}
329351

330352
public function testFederatedSharingDisabled(): void {
331-
$result = $this->getResults([], false);
353+
$result = $this->getResults([], federationEnabled: false);
332354
$this->assertArrayHasKey('federation', $result);
333355
$this->assertFalse($result['federation']['incoming']);
334356
$this->assertFalse($result['federation']['outgoing']);

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public function getForm() {
6868
'excludeGroups' => $this->config->getAppValue('core', 'shareapi_exclude_groups', 'no'),
6969
'excludeGroupsList' => json_decode($excludedGroups, true) ?? [],
7070
'publicShareDisclaimerText' => $this->config->getAppValue('core', 'shareapi_public_link_disclaimertext'),
71-
'enableLinkPasswordByDefault' => $this->getHumanBooleanConfig('core', 'shareapi_enable_link_password_by_default'),
71+
'enableLinkPasswordByDefault' => $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_PASSWORD_DEFAULT),
7272
'defaultPermissions' => (int)$this->config->getAppValue('core', 'shareapi_default_permissions', (string)Constants::PERMISSION_ALL),
7373
'defaultInternalExpireDate' => $this->getHumanBooleanConfig('core', 'shareapi_default_internal_expire_date'),
7474
'internalExpireAfterNDays' => $this->config->getAppValue('core', 'shareapi_internal_expire_after_n_days', '7'),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ public function testGetFormWithoutExcludedGroups(): void {
5757
$this->appConfig
5858
->method('getValueBool')
5959
->willReturnMap([
60-
['core', 'shareapi_allow_federation_on_public_shares', false, false, true],
60+
['core', 'shareapi_allow_federation_on_public_shares', true],
61+
['core', 'shareapi_enable_link_password_by_default', true],
6162
]);
6263

6364
$this->config
@@ -82,7 +83,6 @@ public function testGetFormWithoutExcludedGroups(): void {
8283
['core', 'shareapi_enforce_expire_date', 'no', 'no'],
8384
['core', 'shareapi_exclude_groups', 'no', 'no'],
8485
['core', 'shareapi_public_link_disclaimertext', '', 'Lorem ipsum'],
85-
['core', 'shareapi_enable_link_password_by_default', 'no', 'yes'],
8686
['core', 'shareapi_default_permissions', (string)Constants::PERMISSION_ALL, Constants::PERMISSION_ALL],
8787
['core', 'shareapi_default_internal_expire_date', 'no', 'no'],
8888
['core', 'shareapi_internal_expire_after_n_days', '7', '7'],

‎core/AppInfo/ConfigLexicon.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
class ConfigLexicon implements ILexicon {
2323
public const SHAREAPI_ALLOW_FEDERATION_ON_PUBLIC_SHARES = 'shareapi_allow_federation_on_public_shares';
2424
public const SHARE_CUSTOM_TOKEN = 'shareapi_allow_custom_tokens';
25+
public const SHARE_LINK_PASSWORD_DEFAULT = 'shareapi_enable_link_password_by_default';
26+
public const SHARE_LINK_PASSWORD_ENFORCED = 'shareapi_enforce_links_password';
27+
2528
public const USER_LANGUAGE = 'lang';
2629
public const LASTCRON_TIMESTAMP = 'lastcron';
2730

@@ -49,6 +52,16 @@ public function getAppConfigs(): array {
4952
lazy: true,
5053
note: 'Shares with guessable tokens may be accessed easily. Shares with custom tokens will continue to be accessible after this setting has been disabled.',
5154
),
55+
new Entry(self::SHARE_LINK_PASSWORD_DEFAULT, ValueType::BOOL, false, 'Always ask for a password when sharing document'),
56+
new Entry(
57+
key: self::SHARE_LINK_PASSWORD_ENFORCED,
58+
type: ValueType::BOOL,
59+
defaultRaw: fn (Preset $p): bool => match ($p) {
60+
Preset::SCHOOL, Preset::UNIVERSITY, Preset::SHARED, Preset::SMALL, Preset::MEDIUM, Preset::LARGE => true,
61+
default => false,
62+
},
63+
definition: 'Enforce password protection when sharing document'
64+
),
5265
new Entry(self::LASTCRON_TIMESTAMP, ValueType::INT, 0, 'timestamp of last cron execution'),
5366
];
5467
}

‎core/Controller/OCJSController.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use OCP\AppFramework\Http\Attribute\PublicPage;
2121
use OCP\AppFramework\Http\DataDisplayResponse;
2222
use OCP\Defaults;
23+
use OCP\IAppConfig;
2324
use OCP\IConfig;
2425
use OCP\IGroupManager;
2526
use OCP\IInitialStateService;
@@ -43,6 +44,7 @@ public function __construct(
4344
ISession $session,
4445
IUserSession $userSession,
4546
IConfig $config,
47+
IAppConfig $appConfig,
4648
IGroupManager $groupManager,
4749
IniGetWrapper $iniWrapper,
4850
IURLGenerator $urlGenerator,
@@ -62,6 +64,7 @@ public function __construct(
6264
$session,
6365
$userSession->getUser(),
6466
$config,
67+
$appConfig,
6568
$groupManager,
6669
$iniWrapper,
6770
$urlGenerator,

‎lib/private/Share20/Manager.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1778,7 +1778,7 @@ public function shareApiLinkEnforcePassword(bool $checkGroupMembership = true) {
17781778
}
17791779
}
17801780
}
1781-
return $this->config->getAppValue('core', 'shareapi_enforce_links_password', 'no') === 'yes';
1781+
return $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_PASSWORD_ENFORCED);
17821782
}
17831783

17841784
/**

‎lib/private/Template/JSConfigHelper.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use bantu\IniGetWrapper\IniGetWrapper;
1111
use OC\Authentication\Token\IProvider;
1212
use OC\CapabilitiesManager;
13+
use OC\Core\AppInfo\ConfigLexicon;
1314
use OC\Files\FilenameValidator;
1415
use OC\Share\Share;
1516
use OCA\Provisioning_API\Controller\AUserDataOCSController;
@@ -22,6 +23,7 @@
2223
use OCP\Constants;
2324
use OCP\Defaults;
2425
use OCP\Files\FileInfo;
26+
use OCP\IAppConfig;
2527
use OCP\IConfig;
2628
use OCP\IGroupManager;
2729
use OCP\IInitialStateService;
@@ -50,6 +52,7 @@ public function __construct(
5052
protected ISession $session,
5153
protected ?IUser $currentUser,
5254
protected IConfig $config,
55+
protected readonly IAppConfig $appConfig,
5356
protected IGroupManager $groupManager,
5457
protected IniGetWrapper $iniWrapper,
5558
protected IURLGenerator $urlGenerator,
@@ -94,8 +97,7 @@ public function getConfig(): string {
9497
}
9598
}
9699

97-
$enableLinkPasswordByDefault = $this->config->getAppValue('core', 'shareapi_enable_link_password_by_default', 'no');
98-
$enableLinkPasswordByDefault = $enableLinkPasswordByDefault === 'yes';
100+
$enableLinkPasswordByDefault = $this->appConfig->getValueBool('core', ConfigLexicon::SHARE_LINK_PASSWORD_DEFAULT);
99101
$defaultExpireDateEnabled = $this->config->getAppValue('core', 'shareapi_default_expire_date', 'no') === 'yes';
100102
$defaultExpireDate = $enforceDefaultExpireDate = null;
101103
if ($defaultExpireDateEnabled) {

‎lib/private/TemplateLayout.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
use OCP\App\IAppManager;
2121
use OCP\AppFramework\Http\TemplateResponse;
2222
use OCP\Defaults;
23+
use OCP\IAppConfig;
2324
use OCP\IConfig;
2425
use OCP\IInitialStateService;
2526
use OCP\INavigationManager;
@@ -44,6 +45,7 @@ class TemplateLayout {
4445

4546
public function __construct(
4647
private IConfig $config,
48+
private readonly IAppConfig $appConfig,
4749
private IAppManager $appManager,
4850
private InitialStateService $initialState,
4951
private INavigationManager $navigationManager,
@@ -223,6 +225,7 @@ public function getPageTemplate(string $renderAs, string $appId): ITemplate {
223225
\OC::$server->getSession(),
224226
\OC::$server->getUserSession()->getUser(),
225227
$this->config,
228+
$this->appConfig,
226229
\OC::$server->getGroupManager(),
227230
\OC::$server->get(IniGetWrapper::class),
228231
\OC::$server->getURLGenerator(),

‎tests/lib/Share20/ManagerTest.php‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,10 @@ public function testVerifyPasswordNullButEnforced(): void {
722722

723723
$this->config->method('getAppValue')->willReturnMap([
724724
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
725-
['core', 'shareapi_enforce_links_password', 'no', 'yes'],
725+
]);
726+
727+
$this->appConfig->method('getValueBool')->willReturnMap([
728+
['core', 'shareapi_enforce_links_password', true],
726729
]);
727730

728731
self::invokePrivate($this->manager, 'verifyPassword', [null]);

0 commit comments

Comments
 (0)