fix can change password check in case of encryption is enabled
Admin should _not_ be able to change password when: - if an encryption module is loaded and it uses per-user keys - if encryption is enabled but no encryption modules are loaded Admin should be able to change the password when: - no encryption module is loaded and encryption is disabled - encryption module is loaded but it doesn't require per user keys Signed-off-by: Bjoern Schiessle <bjoern@schiessle.org>
This commit is contained in:
Bjoern Schiessle
parent
9e9b04737e
commit
4b3308bf3f
2 changed files with 95 additions and 21 deletions
|
|
@ -41,6 +41,7 @@ namespace OC\Settings\Controller;
|
|||
|
||||
use OC\Accounts\AccountManager;
|
||||
use OC\AppFramework\Http;
|
||||
use OC\Encryption\Exceptions\ModuleDoesNotExistsException;
|
||||
use OC\ForbiddenException;
|
||||
use OC\Security\IdentityProof\Manager;
|
||||
use OCA\User_LDAP\User_Proxy;
|
||||
|
|
@ -128,9 +129,9 @@ class UsersController extends Controller {
|
|||
/**
|
||||
* @NoCSRFRequired
|
||||
* @NoAdminRequired
|
||||
*
|
||||
*
|
||||
* Display users list template
|
||||
*
|
||||
*
|
||||
* @return TemplateResponse
|
||||
*/
|
||||
public function usersListByGroup() {
|
||||
|
|
@ -140,9 +141,9 @@ class UsersController extends Controller {
|
|||
/**
|
||||
* @NoCSRFRequired
|
||||
* @NoAdminRequired
|
||||
*
|
||||
*
|
||||
* Display users list template
|
||||
*
|
||||
*
|
||||
* @return TemplateResponse
|
||||
*/
|
||||
public function usersList() {
|
||||
|
|
@ -150,7 +151,7 @@ class UsersController extends Controller {
|
|||
$uid = $user->getUID();
|
||||
|
||||
\OC::$server->getNavigationManager()->setActiveEntry('core_users');
|
||||
|
||||
|
||||
/* SORT OPTION: SORT_USERCOUNT or SORT_GROUPNAME */
|
||||
$sortGroupsBy = \OC\Group\MetaData::SORT_USERCOUNT;
|
||||
$isLDAPUsed = false;
|
||||
|
|
@ -166,22 +167,17 @@ class UsersController extends Controller {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
/* ENCRYPTION CONFIG */
|
||||
$isEncryptionEnabled = $this->encryptionManager->isEnabled();
|
||||
$useMasterKey = $this->config->getAppValue('encryption', 'useMasterKey', true);
|
||||
// If masterKey enabled, then you can change password. This is to avoid data loss!
|
||||
$canChangePassword = ($isEncryptionEnabled && $useMasterKey) || $useMasterKey;
|
||||
|
||||
|
||||
/* GROUPS */
|
||||
|
||||
$canChangePassword = $this->canAdminChangeUserPasswords();
|
||||
|
||||
/* GROUPS */
|
||||
$groupsInfo = new \OC\Group\MetaData(
|
||||
$uid,
|
||||
$this->isAdmin,
|
||||
$this->groupManager,
|
||||
$this->userSession
|
||||
);
|
||||
|
||||
|
||||
$groupsInfo->setSorting($sortGroupsBy);
|
||||
list($adminGroup, $groups) = $groupsInfo->get();
|
||||
|
||||
|
|
@ -190,7 +186,7 @@ class UsersController extends Controller {
|
|||
return $ldapFound || $backend instanceof User_Proxy;
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
if ($this->isAdmin) {
|
||||
$disabledUsers = $isLDAPUsed ? -1 : $this->userManager->countDisabledUsers();
|
||||
$userCount = $isLDAPUsed ? 0 : array_reduce($this->userManager->countUsers(), function($v, $w) {
|
||||
|
|
@ -221,7 +217,7 @@ class UsersController extends Controller {
|
|||
'name' => 'Disabled users',
|
||||
'usercount' => $disabledUsers
|
||||
];
|
||||
|
||||
|
||||
/* QUOTAS PRESETS */
|
||||
$quotaPreset = $this->config->getAppValue('files', 'quota_preset', '1 GB, 5 GB, 10 GB');
|
||||
$quotaPreset = explode(',', $quotaPreset);
|
||||
|
|
@ -230,12 +226,12 @@ class UsersController extends Controller {
|
|||
}
|
||||
$quotaPreset = array_diff($quotaPreset, array('default', 'none'));
|
||||
$defaultQuota = $this->config->getAppValue('files', 'default_quota', 'none');
|
||||
|
||||
|
||||
\OC::$server->getEventDispatcher()->dispatch('OC\Settings\Users::loadAdditionalScripts');
|
||||
|
||||
|
||||
/* LANGUAGES */
|
||||
$languages = $this->l10nFactory->getLanguages();
|
||||
|
||||
|
||||
/* FINAL DATA */
|
||||
$serverData = array();
|
||||
// groups
|
||||
|
|
@ -254,6 +250,38 @@ class UsersController extends Controller {
|
|||
return new TemplateResponse('settings', 'settings-vue', ['serverData' => $serverData]);
|
||||
}
|
||||
|
||||
/**
|
||||
* check if the admin can change the users password
|
||||
*
|
||||
* The admin can change the passwords if:
|
||||
*
|
||||
* - no encryption module is loaded and encryption is disabled
|
||||
* - encryption module is loaded but it doesn't require per user keys
|
||||
*
|
||||
* The admin can not change the passwords if:
|
||||
*
|
||||
* - an encryption module is loaded and it uses per-user keys
|
||||
* - encryption is enabled but no encryption modules are loaded
|
||||
*
|
||||
* @return bool
|
||||
*/
|
||||
protected function canAdminChangeUserPasswords() {
|
||||
$isEncryptionEnabled = $this->encryptionManager->isEnabled();
|
||||
try {
|
||||
$noUserSpecificEncryptionKeys =!$this->encryptionManager->getEncryptionModule()->needDetailedAccessList();
|
||||
$isEncryptionModuleLoaded = true;
|
||||
} catch (ModuleDoesNotExistsException $e) {
|
||||
$noUserSpecificEncryptionKeys = true;
|
||||
$isEncryptionModuleLoaded = false;
|
||||
}
|
||||
|
||||
$canChangePassword = ($isEncryptionEnabled && $isEncryptionModuleLoaded && $noUserSpecificEncryptionKeys)
|
||||
|| (!$isEncryptionEnabled && !$isEncryptionModuleLoaded)
|
||||
|| (!$isEncryptionEnabled && $isEncryptionModuleLoaded && $noUserSpecificEncryptionKeys);
|
||||
|
||||
return $canChangePassword;
|
||||
}
|
||||
|
||||
/**
|
||||
* @NoAdminRequired
|
||||
* @NoSubadminRequired
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@
|
|||
namespace Tests\Settings\Controller;
|
||||
|
||||
use OC\Accounts\AccountManager;
|
||||
use OC\Encryption\Exceptions\ModuleDoesNotExistsException;
|
||||
use OC\Group\Group;
|
||||
use OC\Group\Manager;
|
||||
use OC\Settings\Controller\UsersController;
|
||||
|
|
@ -98,7 +99,7 @@ class UsersControllerTest extends \Test\TestCase {
|
|||
$this->securityManager = $this->getMockBuilder(\OC\Security\IdentityProof\Manager::class)->disableOriginalConstructor()->getMock();
|
||||
$this->jobList = $this->createMock(IJobList::class);
|
||||
$this->encryptionManager = $this->createMock(IManager::class);
|
||||
|
||||
|
||||
$this->l->method('t')
|
||||
->will($this->returnCallback(function ($text, $parameters = []) {
|
||||
return vsprintf($text, $parameters);
|
||||
|
|
@ -513,4 +514,49 @@ class UsersControllerTest extends \Test\TestCase {
|
|||
$this->assertSame(Http::STATUS_BAD_REQUEST, $result->getStatus());
|
||||
}
|
||||
|
||||
/**
|
||||
* @dataProvider dataTestCanAdminChangeUserPasswords
|
||||
*
|
||||
* @param bool $encryptionEnabled
|
||||
* @param bool $encryptionModuleLoaded
|
||||
* @param bool $masterKeyEnabled
|
||||
* @param bool $expected
|
||||
*/
|
||||
public function testCanAdminChangeUserPasswords($encryptionEnabled,
|
||||
$encryptionModuleLoaded,
|
||||
$masterKeyEnabled,
|
||||
$expected) {
|
||||
$controller = $this->getController();
|
||||
|
||||
$this->encryptionManager->expects($this->any())
|
||||
->method('isEnabled')
|
||||
->willReturn($encryptionEnabled);
|
||||
$this->encryptionManager->expects($this->any())
|
||||
->method('getEncryptionModule')
|
||||
->willReturnCallback(function() use ($encryptionModuleLoaded) {
|
||||
if ($encryptionModuleLoaded) return $this->encryptionModule;
|
||||
else throw new ModuleDoesNotExistsException();
|
||||
});
|
||||
$this->encryptionModule->expects($this->any())
|
||||
->method('needDetailedAccessList')
|
||||
->willReturn(!$masterKeyEnabled);
|
||||
|
||||
$result = $this->invokePrivate($controller, 'canAdminChangeUserPasswords', []);
|
||||
$this->assertSame($expected, $result);
|
||||
}
|
||||
|
||||
public function dataTestCanAdminChangeUserPasswords() {
|
||||
return [
|
||||
// encryptionEnabled, encryptionModuleLoaded, masterKeyEnabled, expectedResult
|
||||
[true, true, true, true],
|
||||
[false, true, true, true],
|
||||
[true, false, true, false],
|
||||
[false, false, true, true],
|
||||
[true, true, false, false],
|
||||
[false, true, false, false],
|
||||
[true, false, false, false],
|
||||
[false, false, false, true],
|
||||
];
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue