aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBjoern Schiessle <bjoern@schiessle.org>2018-12-20 11:11:04 +0100
committerBjoern Schiessle <bjoern@schiessle.org>2018-12-20 12:28:40 +0100
commit4b3308bf3fd4c6fe572ee1658a7809bba20c7339 (patch)
treeb7b50eaed48775240a1960a5d85ab3b58acad40a
parent9e9b04737e41ce2a582afdb5decade5293ab115b (diff)
downloadnextcloud-server-4b3308bf3fd4c6fe572ee1658a7809bba20c7339.tar.gz
nextcloud-server-4b3308bf3fd4c6fe572ee1658a7809bba20c7339.zip
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>
-rw-r--r--settings/Controller/UsersController.php68
-rw-r--r--tests/Settings/Controller/UsersControllerTest.php48
2 files changed, 95 insertions, 21 deletions
diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php
index 12c3e47dfea..55ef267d8b7 100644
--- a/settings/Controller/UsersController.php
+++ b/settings/Controller/UsersController.php
@@ -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
@@ -255,6 +251,38 @@ class UsersController extends Controller {
}
/**
+ * 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
* @PasswordConfirmationRequired
diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php
index 74ac990be92..8294514fa50 100644
--- a/tests/Settings/Controller/UsersControllerTest.php
+++ b/tests/Settings/Controller/UsersControllerTest.php
@@ -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],
+ ];
+ }
+
}