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.

Sign up for GitHub

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

Allow to disable password policy enforcement for selected groups #31194

Merged
merged 1 commit into from
Apr 6, 2022
Merged

Allow to disable password policy enforcement for selected groups #31194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,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 @@ -149,6 +150,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 @@ -161,6 +163,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 @@ -174,6 +177,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 @@ -188,6 +192,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 @@ -203,6 +208,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 @@ -215,6 +221,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 @@ -225,6 +232,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 @@ -234,6 +242,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 @@ -243,6 +252,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 @@ -253,6 +263,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 @@ -264,6 +275,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
9 changes: 8 additions & 1 deletion apps/settings/lib/Settings/Admin/Sharing.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,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)) : '';
CarlSchwan marked this conversation as resolved.
Show resolved Hide resolved


$parameters = [
// Built-In Sharing
'sharingAppEnabled' => $this->appManager->isEnabledForUser('files_sharing'),
Expand All @@ -84,7 +89,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
6 changes: 5 additions & 1 deletion apps/settings/src/admin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
window.addEventListener('DOMContentLoaded', () => {
$('#excludedGroups,#linksExcludedGroups').each((index, element) => {
$('#excludedGroups,#linksExcludedGroups,#passwordsExcludedGroups').each(function(index, element) {
OC.Settings.setupGroupsSelect($(element))
$(element).change((ev) => {
let groups = ev.val || []
Expand Down Expand Up @@ -93,6 +93,10 @@ window.addEventListener('DOMContentLoaded', () => {
$('#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
12 changes: 12 additions & 0 deletions apps/settings/templates/settings/admin/sharing.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,18 @@
} ?> />
<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"'); } ?> />

Comment on lines +133 to +134
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate checkbox breaks the UI

<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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,8 @@ public function testGetFormWithoutExcludedGroups(): void {
'shareRemoteExpireAfterNDays' => '7',
'shareRemoteEnforceExpireDate' => 'no',
'allowLinksExcludeGroups' => '',
'passwordExcludedGroups' => '',
'passwordExcludedGroupsFeatureEnabled' => false,
],
''
);
Expand Down Expand Up @@ -208,6 +210,8 @@ public function testGetFormWithExcludedGroups(): void {
'shareRemoteExpireAfterNDays' => '7',
'shareRemoteEnforceExpireDate' => 'no',
'allowLinksExcludeGroups' => '',
'passwordExcludedGroups' => '',
'passwordExcludedGroupsFeatureEnabled' => false,
],
''
);
Expand Down
5 changes: 5 additions & 0 deletions config/config.sample.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1572,6 +1572,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
4 changes: 2 additions & 2 deletions dist/settings-legacy-admin.js
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/settings-legacy-admin.js.map
View file
Open in desktop

Large diffs are not rendered by default.

14 changes: 13 additions & 1 deletion lib/private/Share20/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1784,9 +1784,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) {
CarlSchwan marked this conversation as resolved.
Show resolved Hide resolved
$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;
}
}
}
CarlSchwan marked this conversation as resolved.
Show resolved Hide resolved
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,15 +116,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);
CarlSchwan marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand Down
4 changes: 3 additions & 1 deletion lib/public/Share/IManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,10 +321,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -547,12 +547,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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,14 +505,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"]'],
CarlSchwan marked this conversation as resolved.
Show resolved Hide resolved
['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 @@ -522,6 +554,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 @@ -543,6 +576,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