Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable22] Allow to disable password policy enforcement for selected groups #33152

Merged
merged 1 commit into from
Jul 7, 2022
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
12 changes: 12 additions & 0 deletions apps/files_sharing/tests/CapabilitiesTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ public function testOnlyLinkSharing() {
$map = [
['core', 'shareapi_enabled', 'yes', 'yes'],
['core', 'shareapi_allow_links', 'yes', 'yes'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
];
$result = $this->getResults($map);
$this->assertIsArray($result['public']);
Expand All @@ -147,6 +148,7 @@ public function testLinkPassword() {
$map = [
['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);
Expand All @@ -159,6 +161,7 @@ public function testLinkNoPassword() {
$map = [
['core', 'shareapi_enabled', 'yes', 'yes'],
['core', 'shareapi_allow_links', 'yes', 'yes'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
['core', 'shareapi_enforce_links_password', 'no', 'no'],
];
$result = $this->getResults($map);
Expand All @@ -172,6 +175,7 @@ public function testLinkNoExpireDate() {
['core', 'shareapi_enabled', 'yes', 'yes'],
['core', 'shareapi_allow_links', 'yes', 'yes'],
['core', 'shareapi_default_expire_date', 'no', 'no'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
];
$result = $this->getResults($map);
$this->assertArrayHasKey('expire_date', $result['public']);
Expand All @@ -186,6 +190,7 @@ public function testLinkExpireDate() {
['core', 'shareapi_default_expire_date', 'no', 'yes'],
['core', 'shareapi_expire_after_n_days', '7', '7'],
['core', 'shareapi_enforce_expire_date', 'no', 'no'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
];
$result = $this->getResults($map);
$this->assertArrayHasKey('expire_date', $result['public']);
Expand All @@ -201,6 +206,7 @@ public function testLinkExpireDateEnforced() {
['core', 'shareapi_allow_links', 'yes', 'yes'],
['core', 'shareapi_default_expire_date', 'no', 'yes'],
['core', 'shareapi_enforce_expire_date', 'no', 'yes'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
];
$result = $this->getResults($map);
$this->assertArrayHasKey('expire_date', $result['public']);
Expand All @@ -213,6 +219,7 @@ public function testLinkSendMail() {
['core', 'shareapi_enabled', 'yes', 'yes'],
['core', 'shareapi_allow_links', 'yes', 'yes'],
['core', 'shareapi_allow_public_notification', 'no', 'yes'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
];
$result = $this->getResults($map);
$this->assertTrue($result['public']['send_mail']);
Expand All @@ -223,6 +230,7 @@ public function testLinkNoSendMail() {
['core', 'shareapi_enabled', 'yes', 'yes'],
['core', 'shareapi_allow_links', 'yes', 'yes'],
['core', 'shareapi_allow_public_notification', 'no', 'no'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
];
$result = $this->getResults($map);
$this->assertFalse($result['public']['send_mail']);
Expand All @@ -232,6 +240,7 @@ public function testResharing() {
$map = [
['core', 'shareapi_enabled', 'yes', 'yes'],
['core', 'shareapi_allow_resharing', 'yes', 'yes'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
];
$result = $this->getResults($map);
$this->assertTrue($result['resharing']);
Expand All @@ -241,6 +250,7 @@ public function testNoResharing() {
$map = [
['core', 'shareapi_enabled', 'yes', 'yes'],
['core', 'shareapi_allow_resharing', 'yes', 'no'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
];
$result = $this->getResults($map);
$this->assertFalse($result['resharing']);
Expand All @@ -251,6 +261,7 @@ public function testLinkPublicUpload() {
['core', 'shareapi_enabled', 'yes', 'yes'],
['core', 'shareapi_allow_links', 'yes', 'yes'],
['core', 'shareapi_allow_public_upload', 'yes', 'yes'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
];
$result = $this->getResults($map);
$this->assertTrue($result['public']['upload']);
Expand All @@ -262,6 +273,7 @@ public function testLinkNoPublicUpload() {
['core', 'shareapi_enabled', 'yes', 'yes'],
['core', 'shareapi_allow_links', 'yes', 'yes'],
['core', 'shareapi_allow_public_upload', 'yes', 'no'],
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
];
$result = $this->getResults($map);
$this->assertFalse($result['public']['upload']);
Expand Down
6 changes: 5 additions & 1 deletion apps/settings/js/admin.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
window.addEventListener('DOMContentLoaded', function(){
$('#excludedGroups,#linksExcludedGroups').each(function (index, element) {
$('#excludedGroups,#linksExcludedGroups,#passwordsExcludedGroups').each(function(index, element) {
OC.Settings.setupGroupsSelect($(element));
$(element).change(function(ev) {
var groups = ev.val || [];
Expand Down Expand Up @@ -94,6 +94,10 @@ window.addEventListener('DOMContentLoaded', function(){
$("#setDefaultRemoteExpireDate").toggleClass('hidden', !this.checked);
});

$('#enforceLinkPassword').change(function() {
$('#selectPasswordsExcludedGroups').toggleClass('hidden', !this.checked)
})

$('#publicShareDisclaimer').change(function() {
$("#publicShareDisclaimerText").toggleClass('hidden', !this.checked);
if(!this.checked) {
Expand Down
9 changes: 8 additions & 1 deletion apps/settings/lib/Settings/Admin/Sharing.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ public function getForm() {
$linksExcludeGroupsList = !is_null(json_decode($linksExcludedGroups))
? implode('|', json_decode($linksExcludedGroups, true)) : '';

$excludedPasswordGroups = $this->config->getAppValue('core', 'shareapi_enforce_links_password_excluded_groups', '');
$excludedPasswordGroupsList = !is_null(json_decode($excludedPasswordGroups))
? implode('|', json_decode($excludedPasswordGroups, true)) : '';


$parameters = [
// Built-In Sharing
'allowGroupSharing' => $this->config->getAppValue('core', 'shareapi_allow_group_sharing', 'yes'),
Expand All @@ -81,7 +86,9 @@ public function getForm() {
'restrictUserEnumerationToGroup' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_group', 'no'),
'restrictUserEnumerationToPhone' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_to_phone', 'no'),
'restrictUserEnumerationFullMatch' => $this->config->getAppValue('core', 'shareapi_restrict_user_enumeration_full_match', 'yes'),
'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(),
'enforceLinkPassword' => Util::isPublicLinkPasswordRequired(false),
'passwordExcludedGroups' => $excludedPasswordGroupsList,
'passwordExcludedGroupsFeatureEnabled' => $this->config->getSystemValueBool('sharing.allow_disabled_password_enforcement_groups', false),
'onlyShareWithGroupMembers' => $this->shareManager->shareWithGroupMembersOnly(),
'shareAPIEnabled' => $this->config->getAppValue('core', 'shareapi_enabled', 'yes'),
'shareDefaultExpireDateSet' => $this->config->getAppValue('core', 'shareapi_default_expire_date', 'no'),
Expand Down
10 changes: 10 additions & 0 deletions apps/settings/templates/settings/admin/sharing.php
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,16 @@
} ?> />
<label for="enforceLinkPassword"><?php p($l->t('Enforce password protection'));?></label><br/>

<?php if ($_['passwordExcludedGroupsFeatureEnabled']) { ?>
<div id="selectPasswordsExcludedGroups" class="indent <?php if (!$_['enforceLinkPassword']) { p('hidden'); } ?>">
<div class="indent">
<label for="shareapi_enforce_links_password_excluded_groups"><?php p($l->t('Exclude groups from password requirements:'));?>
<br />
<input name="shareapi_enforce_links_password_excluded_groups" id="passwordsExcludedGroups" value="<?php p($_['passwordExcludedGroups']) ?>" style="width: 400px" class="noJSAutoUpdate"/>
</div>
</div>
<?php } ?>

<input type="checkbox" name="shareapi_default_expire_date" id="shareapiDefaultExpireDate" class="checkbox"
value="1" <?php if ($_['shareDefaultExpireDateSet'] === 'yes') {
print_unescaped('checked="checked"');
Expand Down
4 changes: 4 additions & 0 deletions apps/settings/tests/Settings/Admin/SharingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ public function testGetFormWithoutExcludedGroups() {
'shareRemoteExpireAfterNDays' => '7',
'shareRemoteEnforceExpireDate' => 'no',
'allowLinksExcludeGroups' => '',
'passwordExcludedGroups' => '',
'passwordExcludedGroupsFeatureEnabled' => false,
],
''
);
Expand Down Expand Up @@ -194,6 +196,8 @@ public function testGetFormWithExcludedGroups() {
'shareRemoteExpireAfterNDays' => '7',
'shareRemoteEnforceExpireDate' => 'no',
'allowLinksExcludeGroups' => '',
'passwordExcludedGroups' => '',
'passwordExcludedGroupsFeatureEnabled' => false,
],
''
);
Expand Down
5 changes: 5 additions & 0 deletions config/config.sample.php
Original file line number Diff line number Diff line change
Expand Up @@ -1553,6 +1553,11 @@
*/
'sharing.enable_share_mail' => true,

/**
* Set to true to enable the feature to add exceptions for share password enforcement
*/
'sharing.allow_disabled_password_enforcement_groups' => false,

/**
* Set to true to always transfer incoming shares by default
* when running "occ files:transfer-ownership".
Expand Down
14 changes: 13 additions & 1 deletion lib/private/Share20/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -1763,9 +1763,21 @@ public function shareApiAllowLinks() {
/**
* Is password on public link requires
*
* @param bool Check group membership exclusion
* @return bool
*/
public function shareApiLinkEnforcePassword() {
public function shareApiLinkEnforcePassword(bool $checkGroupMembership = true) {
$excludedGroups = $this->config->getAppValue('core', 'shareapi_enforce_links_password_excluded_groups', '');
if ($excludedGroups !== '' && $checkGroupMembership) {
$excludedGroups = json_decode($excludedGroups);
$user = $this->userSession->getUser();
if ($user) {
$userGroups = $this->groupManager->getUserGroupIds($user);
if ((bool)array_intersect($excludedGroups, $userGroups)) {
return false;
}
}
}
return $this->config->getAppValue('core', 'shareapi_enforce_links_password', 'no') === 'yes';
}

Expand Down
7 changes: 4 additions & 3 deletions lib/private/legacy/OC_Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -347,15 +347,16 @@ public static function setupFS(?string $user = '') {
}

/**
* check if a password is required for each public link
* Check if a password is required for each public link
*
* @param bool $checkGroupMembership Check group membership exclusion
* @return boolean
* @suppress PhanDeprecatedFunction
*/
public static function isPublicLinkPasswordRequired() {
public static function isPublicLinkPasswordRequired(bool $checkGroupMembership = true) {
/** @var IManager $shareManager */
$shareManager = \OC::$server->get(IManager::class);
return $shareManager->shareApiLinkEnforcePassword();
return $shareManager->shareApiLinkEnforcePassword($checkGroupMembership);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion lib/public/Share/IManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -316,10 +316,12 @@ public function shareApiAllowLinks();
/**
* Is password on public link requires
*
* @param bool $checkGroupMembership Check group membership exclusion
* @return bool
* @since 9.0.0
* @since 24.0.0 Added optional $checkGroupMembership parameter
*/
public function shareApiLinkEnforcePassword();
public function shareApiLinkEnforcePassword(bool $checkGroupMembership = true);

/**
* Is default expire date enabled
Expand Down
8 changes: 5 additions & 3 deletions lib/public/Util.php
Original file line number Diff line number Diff line change
Expand Up @@ -482,12 +482,14 @@ public static function naturalSortCompare($a, $b) {
}

/**
* check if a password is required for each public link
* Check if a password is required for each public link
*
* @param bool $checkGroupMembership Check group membership exclusion
* @return boolean
* @since 7.0.0
*/
public static function isPublicLinkPasswordRequired() {
return \OC_Util::isPublicLinkPasswordRequired();
public static function isPublicLinkPasswordRequired(bool $checkGroupMembership = true) {
return \OC_Util::isPublicLinkPasswordRequired($checkGroupMembership);
}

/**
Expand Down
34 changes: 34 additions & 0 deletions tests/lib/Share20/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -497,14 +497,46 @@ public function testVerifyPasswordNullButEnforced() {
$this->expectExceptionMessage('Passwords are enforced for link and mail shares');

$this->config->method('getAppValue')->willReturnMap([
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
['core', 'shareapi_enforce_links_password', 'no', 'yes'],
]);

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

public function testVerifyPasswordNotEnforcedGroup() {
$this->config->method('getAppValue')->willReturnMap([
['core', 'shareapi_enforce_links_password_excluded_groups', '', '["admin"]'],
['core', 'shareapi_enforce_links_password', 'no', 'yes'],
]);

// Create admin user
$user = $this->createMock(IUser::class);
$this->userSession->method('getUser')->willReturn($user);
$this->groupManager->method('getUserGroupIds')->with($user)->willReturn(['admin']);

$result = self::invokePrivate($this->manager, 'verifyPassword', [null]);
$this->assertNull($result);
}

public function testVerifyPasswordNotEnforcedMultipleGroups() {
$this->config->method('getAppValue')->willReturnMap([
['core', 'shareapi_enforce_links_password_excluded_groups', '', '["admin", "special"]'],
['core', 'shareapi_enforce_links_password', 'no', 'yes'],
]);

// Create admin user
$user = $this->createMock(IUser::class);
$this->userSession->method('getUser')->willReturn($user);
$this->groupManager->method('getUserGroupIds')->with($user)->willReturn(['special']);

$result = self::invokePrivate($this->manager, 'verifyPassword', [null]);
$this->assertNull($result);
}

public function testVerifyPasswordNull() {
$this->config->method('getAppValue')->willReturnMap([
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
['core', 'shareapi_enforce_links_password', 'no', 'no'],
]);

Expand All @@ -514,6 +546,7 @@ public function testVerifyPasswordNull() {

public function testVerifyPasswordHook() {
$this->config->method('getAppValue')->willReturnMap([
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
['core', 'shareapi_enforce_links_password', 'no', 'no'],
]);

Expand All @@ -535,6 +568,7 @@ public function testVerifyPasswordHookFails() {
$this->expectExceptionMessage('password not accepted');

$this->config->method('getAppValue')->willReturnMap([
['core', 'shareapi_enforce_links_password_excluded_groups', '', ''],
['core', 'shareapi_enforce_links_password', 'no', 'no'],
]);

Expand Down