Add value if restore of data is possible for a user

* reason: nice to know before password change in user management
* restore is possible:
	* encryption is disabled
	* encryption is enabled, admin and user has checked the
	  restore option
* if not possible:
	* highlight users row in red once the admin wants to change the password
	* show also a little tipsy

diff --git a/settings/application.php b/settings/application.php
index f7ba72f3bfc6181a4f42fdbb652314f695522c9b..d5516a1eefd8331750b10d81103d19acc7bc90d5 100644 (file)
--- a/settings/application.php
+++ b/settings/application.php
@@ -90,7 +90,8 @@ class Application extends App {
                                $c->query('Defaults'),
                                $c->query('Mail'),
                                $c->query('DefaultMailAddress'),
-                               $c->query('URLGenerator')
+                               $c->query('URLGenerator'),
+                               $c->query('OCP\\App\\IAppManager')
                        );
                });
                $container->registerService('LogSettingsController', function(IContainer $c) {

diff --git a/settings/controller/userscontroller.php b/settings/controller/userscontroller.php
index 1be2f4db9b9af90e25f2dec67cbbd14538e55378..be1b26f86adafc8e176c379a30ed0738d5f2fb3f 100644 (file)
--- a/settings/controller/userscontroller.php
+++ b/settings/controller/userscontroller.php
@@ -11,9 +11,9 @@
 namespace OC\Settings\Controller;
 
 use OC\AppFramework\Http;
-use OC\User\Manager;
 use OC\User\User;
-use \OCP\AppFramework\Controller;
+use OCP\App\IAppManager;
+use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\DataResponse;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\IConfig;
@@ -52,6 +52,10 @@ class UsersController extends Controller {
        private $fromMailAddress;
        /** @var IURLGenerator */
        private $urlGenerator;
+       /** @var bool contains the state of the encryption app */
+       private $isEncryptionAppEnabled;
+       /** @var bool contains the state of the admin recovery setting */
+       private $isRestoreEnabled = false;
 
        /**
         * @param string $appName
@@ -66,6 +70,7 @@ class UsersController extends Controller {
         * @param \OC_Defaults $defaults
         * @param \OC_Mail $mail
         * @param string $fromMailAddress
+        * @param IAppManager $appManager
         */
        public function __construct($appName,
                                                                IRequest $request,
@@ -79,7 +84,8 @@ class UsersController extends Controller {
                                                                \OC_Defaults $defaults,
                                                                \OC_Mail $mail,
                                                                $fromMailAddress,
-                                                               IURLGenerator $urlGenerator) {
+                                                               IURLGenerator $urlGenerator,
+                                                               IAppManager $appManager) {
                parent::__construct($appName, $request);
                $this->userManager = $userManager;
                $this->groupManager = $groupManager;
@@ -92,6 +98,14 @@ class UsersController extends Controller {
                $this->mail = $mail;
                $this->fromMailAddress = $fromMailAddress;
                $this->urlGenerator = $urlGenerator;
+
+               // check for encryption state - TODO see formatUserForIndex
+               $this->isEncryptionAppEnabled = $appManager->isEnabledForUser('files_encryption');
+               if($this->isEncryptionAppEnabled) {
+                       // putting this directly in empty is possible in PHP 5.5+
+                       $result = $config->getAppValue('files_encryption', 'recoveryAdminEnabled', 0);
+                       $this->isRestoreEnabled = !empty($result);
+               }
        }
 
        /**
@@ -100,7 +114,33 @@ class UsersController extends Controller {
         * @return array
         */
        private function formatUserForIndex(IUser $user, array $userGroups = null) {
-               return array(
+
+               // TODO: eliminate this encryption specific code below and somehow
+               // hook in additional user info from other apps
+
+               // recovery isn't possible if admin or user has it disabled and encryption
+               // is enabled - so we eliminate the else paths in the conditional tree
+               // below
+               $restorePossible = false;
+
+               if ($this->isEncryptionAppEnabled) {
+                       if ($this->isRestoreEnabled) {
+                               // check for the users recovery setting
+                               $recoveryMode = $this->config->getUserValue($user->getUID(), 'files_encryption', 'recovery_enabled', '0');
+                               // method call inside empty is possible with PHP 5.5+
+                               $recoveryModeEnabled = !empty($recoveryMode);
+                               if ($recoveryModeEnabled) {
+                                       // user also has recovery mode enabled
+                                       $restorePossible = true;
+                               }
+                       }
+               } else {
+                       // recovery is possible if encryption is disabled (plain files are
+                       // available)
+                       $restorePossible = true;
+               }
+
+               return [
                        'name' => $user->getUID(),
                        'displayname' => $user->getDisplayName(),
                        'groups' => (empty($userGroups)) ? $this->groupManager->getUserGroupIds($user) : $userGroups,
@@ -109,8 +149,9 @@ class UsersController extends Controller {
                        'storageLocation' => $user->getHome(),
                        'lastLogin' => $user->getLastLogin(),
                        'backend' => $user->getBackendClassName(),
-                       'email' => $this->config->getUserValue($user->getUID(), 'settings', 'email', '')
-               );
+                       'email' => $this->config->getUserValue($user->getUID(), 'settings', 'email', ''),
+                       'isRestoreDisabled' => !$restorePossible,
+               ];
        }
 
        /**

diff --git a/settings/css/settings.css b/settings/css/settings.css
index 55367e716c268392e260a8277578349cd2786b46..57edc18bd9a4affb00d220d2d0ead7aabcf37f4c 100644 (file)
--- a/settings/css/settings.css
+++ b/settings/css/settings.css
@@ -132,7 +132,10 @@ input.userFilter {width: 200px;}
 .ie8 table.hascontrols{border-collapse:collapse;width: 100%;}
 .ie8 table.hascontrols tbody tr{border-collapse:collapse;border: 1px solid #ddd !important;}
 
-
+/* used to highlight a user row in red */
+#userlist tr.row-warning {
+       background-color: #FDD;
+}
 
 
 /* APPS */

diff --git a/settings/js/users/users.js b/settings/js/users/users.js
index f21c660b41f4ee53c4de18f298a587a43644b8cf..1a755ab7b25e104d1dcbd26770ef48d27a4dfc00 100644 (file)
--- a/settings/js/users/users.js
+++ b/settings/js/users/users.js
@@ -42,6 +42,7 @@ var UserList = {
         *                              'lastLogin':            '1418632333'
         *                              'backend':                      'LDAP',
         *                              'email':                        'username@example.org'
+        *                              'isRestoreDisabled':false
         *                      }
         * @param sort
         * @returns table row created for this user
@@ -63,11 +64,12 @@ var UserList = {
                }
 
                /**
-                * add username and displayname to row (in data and visible markup
+                * add username and displayname to row (in data and visible markup)
                 */
                $tr.data('uid', user.name);
                $tr.data('displayname', user.displayname);
                $tr.data('mailAddress', user.email);
+               $tr.data('restoreDisabled', user.isRestoreDisabled);
                $tr.find('td.name').text(user.name);
                $tr.find('td.displayName > span').text(user.displayname);
                $tr.find('td.mailAddress > span').text(user.email);
@@ -352,6 +354,9 @@ var UserList = {
        getMailAddress: function(element) {
                return ($(element).closest('tr').data('mailAddress') || '').toString();
        },
+       getRestoreDisabled: function(element) {
+               return ($(element).closest('tr').data('restoreDisabled') || '');
+       },
        initDeleteHandling: function() {
                //set up handler
                UserDeleteHandler = new DeleteHandler('/settings/users/users', 'username',
@@ -627,8 +632,16 @@ $(document).ready(function () {
                event.stopPropagation();
 
                var $td = $(this).closest('td');
+               var $tr = $(this).closest('tr');
                var uid = UserList.getUID($td);
                var $input = $('<input type="password">');
+               var isRestoreDisabled = UserList.getRestoreDisabled($td) === true;
+               if(isRestoreDisabled) {
+                       $tr.addClass('row-warning');
+                       // add tipsy if the password change could cause data loss - no recovery enabled
+                       $input.tipsy({gravity:'s', fade:false});
+                       $input.attr('title', t('settings', 'Changing the password will result in data loss, because data recovery is not available for this user'));
+               }
                $td.find('img').hide();
                $td.children('span').replaceWith($input);
                $input
@@ -655,6 +668,8 @@ $(document).ready(function () {
                        .blur(function () {
                                $(this).replaceWith($('<span>●●●●●●●</span>'));
                                $td.find('img').show();
+                               // remove highlight class from users without recovery ability
+                               $tr.removeClass('row-warning');
                        });
        });
        $('input:password[id="recoveryPassword"]').keyup(function() {

diff --git a/tests/settings/controller/userscontrollertest.php b/tests/settings/controller/userscontrollertest.php
index c6506ee440bf9ab800c1a39948dd3c33bfaba9c8..7dc2d066a5c09831a0d5f9ef686e62845ae5f016 100644 (file)
--- a/tests/settings/controller/userscontrollertest.php
+++ b/tests/settings/controller/userscontrollertest.php
@@ -21,9 +21,6 @@ class UsersControllerTest extends \Test\TestCase {
        /** @var \OCP\AppFramework\IAppContainer */
        private $container;
 
-       /** @var UsersController */
-       private $usersController;
-
        protected function setUp() {
                $app = new Application();
                $this->container = $app->getContainer();
@@ -54,9 +51,8 @@ class UsersControllerTest extends \Test\TestCase {
                        ->disableOriginalConstructor()->getMock();
                $this->container['URLGenerator'] = $this->getMockBuilder('\OCP\IURLGenerator')
                        ->disableOriginalConstructor()->getMock();
-
-               $this->usersController = $this->container['UsersController'];
-
+               $this->container['OCP\\App\\IAppManager'] = $this->getMockBuilder('OCP\\App\\IAppManager')
+                       ->disableOriginalConstructor()->getMock();
        }
 
        /**
@@ -169,7 +165,8 @@ class UsersControllerTest extends \Test\TestCase {
                                        'storageLocation' => '/home/foo',
                                        'lastLogin' => 500,
                                        'backend' => 'OC_User_Database',
-                                       'email' => 'foo@bar.com'
+                                       'email' => 'foo@bar.com',
+                                       'isRestoreDisabled' => false,
                                ),
                                1 => array(
                                        'name' => 'admin',
@@ -180,7 +177,8 @@ class UsersControllerTest extends \Test\TestCase {
                                        'storageLocation' => '/home/admin',
                                        'lastLogin' => 12,
                                        'backend' => 'OC_User_Dummy',
-                                       'email' => 'admin@bar.com'
+                                       'email' => 'admin@bar.com',
+                                       'isRestoreDisabled' => false,
                                ),
                                2 => array(
                                        'name' => 'bar',
@@ -191,11 +189,12 @@ class UsersControllerTest extends \Test\TestCase {
                                        'storageLocation' => '/home/bar',
                                        'lastLogin' => 3999,
                                        'backend' => 'OC_User_Dummy',
-                                       'email' => 'bar@dummy.com'
+                                       'email' => 'bar@dummy.com',
+                                       'isRestoreDisabled' => false,
                                ),
                        )
                );
-               $response = $this->usersController->index(0, 10, 'gid', 'pattern');
+               $response = $this->container['UsersController']->index(0, 10, 'gid', 'pattern');
                $this->assertEquals($expectedResponse, $response);
        }
 
@@ -294,7 +293,8 @@ class UsersControllerTest extends \Test\TestCase {
                                        'storageLocation' => '/home/foo',
                                        'lastLogin' => 500,
                                        'backend' => 'OC_User_Database',
-                                       'email' => 'foo@bar.com'
+                                       'email' => 'foo@bar.com',
+                                       'isRestoreDisabled' => false,
                                ),
                                1 => array(
                                        'name' => 'admin',
@@ -305,7 +305,8 @@ class UsersControllerTest extends \Test\TestCase {
                                        'storageLocation' => '/home/admin',
                                        'lastLogin' => 12,
                                        'backend' => 'OC_User_Dummy',
-                                       'email' => 'admin@bar.com'
+                                       'email' => 'admin@bar.com',
+                                       'isRestoreDisabled' => false,
                                ),
                                2 => array(
                                        'name' => 'bar',
@@ -316,11 +317,12 @@ class UsersControllerTest extends \Test\TestCase {
                                        'storageLocation' => '/home/bar',
                                        'lastLogin' => 3999,
                                        'backend' => 'OC_User_Dummy',
-                                       'email' => 'bar@dummy.com'
+                                       'email' => 'bar@dummy.com',
+                                       'isRestoreDisabled' => false,
                                ),
                        )
                );
-               $response = $this->usersController->index(0, 10, '', 'pattern');
+               $response = $this->container['UsersController']->index(0, 10, '', 'pattern');
                $this->assertEquals($expectedResponse, $response);
        }
 
@@ -374,11 +376,12 @@ class UsersControllerTest extends \Test\TestCase {
                                        'storageLocation' => '/home/foo',
                                        'lastLogin' => 500,
                                        'backend' => 'OC_User_Database',
-                                       'email' => null
+                                       'email' => null,
+                                       'isRestoreDisabled' => false,
                                )
                        )
                );
-               $response = $this->usersController->index(0, 10, '','', 'OC_User_Dummy');
+               $response = $this->container['UsersController']->index(0, 10, '','', 'OC_User_Dummy');
                $this->assertEquals($expectedResponse, $response);
        }
 
@@ -394,7 +397,7 @@ class UsersControllerTest extends \Test\TestCase {
                        ->will($this->returnValue([]));
 
                $expectedResponse = new DataResponse([]);
-               $response = $this->usersController->index(0, 10, '','', 'OC_User_Dummy');
+               $response = $this->container['UsersController']->index(0, 10, '','', 'OC_User_Dummy');
                $this->assertEquals($expectedResponse, $response);
        }
 
@@ -432,11 +435,12 @@ class UsersControllerTest extends \Test\TestCase {
                                'displayname' => null,
                                'quota' => null,
                                'subadmin' => array(),
-                               'email' => null
+                               'email' => null,
+                               'isRestoreDisabled' => false,
                        ),
                        Http::STATUS_CREATED
                );
-               $response = $this->usersController->create('foo', 'password', array());
+               $response = $this->container['UsersController']->create('foo', 'password', array());
                $this->assertEquals($expectedResponse, $response);
        }
 
@@ -502,11 +506,12 @@ class UsersControllerTest extends \Test\TestCase {
                                'displayname' => null,
                                'quota' => null,
                                'subadmin' => array(),
-                               'email' => null
+                               'email' => null,
+                               'isRestoreDisabled' => false,
                        ),
                        Http::STATUS_CREATED
                );
-               $response = $this->usersController->create('foo', 'password', array('NewGroup', 'ExistingGroup'));
+               $response = $this->container['UsersController']->create('foo', 'password', array('NewGroup', 'ExistingGroup'));
                $this->assertEquals($expectedResponse, $response);
        }
 
@@ -525,7 +530,7 @@ class UsersControllerTest extends \Test\TestCase {
                        ),
                        Http::STATUS_FORBIDDEN
                );
-               $response = $this->usersController->create('foo', 'password', array());
+               $response = $this->container['UsersController']->create('foo', 'password', array());
                $this->assertEquals($expectedResponse, $response);
        }
 
@@ -553,7 +558,7 @@ class UsersControllerTest extends \Test\TestCase {
                        ),
                        Http::STATUS_FORBIDDEN
                );
-               $response = $this->usersController->destroy('myself');
+               $response = $this->container['UsersController']->destroy('myself');
                $this->assertEquals($expectedResponse, $response);
        }
 
@@ -591,7 +596,7 @@ class UsersControllerTest extends \Test\TestCase {
                        ),
                        Http::STATUS_NO_CONTENT
                );
-               $response = $this->usersController->destroy('UserToDelete');
+               $response = $this->container['UsersController']->destroy('UserToDelete');
                $this->assertEquals($expectedResponse, $response);
        }
        /**
@@ -628,7 +633,7 @@ class UsersControllerTest extends \Test\TestCase {
                        ),
                        Http::STATUS_FORBIDDEN
                );
-               $response = $this->usersController->destroy('UserToDelete');
+               $response = $this->container['UsersController']->destroy('UserToDelete');
                $this->assertEquals($expectedResponse, $response);
        }
 
@@ -653,7 +658,7 @@ class UsersControllerTest extends \Test\TestCase {
                        ),
                        Http::STATUS_UNPROCESSABLE_ENTITY
                );
-               $response = $this->usersController->create('foo', 'password', array(), 'invalidMailAdress');
+               $response = $this->container['UsersController']->create('foo', 'password', array(), 'invalidMailAdress');
                $this->assertEquals($expectedResponse, $response);
        }
 
@@ -688,8 +693,142 @@ class UsersControllerTest extends \Test\TestCase {
                        ->expects($this->never())
                        ->method('error');
 
-               $response = $this->usersController->create('foo', 'password', array(), 'validMail@Adre.ss');
+               $response = $this->container['UsersController']->create('foo', 'password', array(), 'validMail@Adre.ss');
                $this->assertEquals(Http::STATUS_CREATED, $response->getStatus());
        }
 
+       private function mockUser($userId = 'foo', $displayName = 'M. Foo',
+                                                         $lastLogin = 500, $home = '/home/foo', $backend = 'OC_User_Database') {
+               $user = $this->getMockBuilder('\OC\User\User')
+                       ->disableOriginalConstructor()->getMock();
+               $user
+                       ->expects($this->any())
+                       ->method('getUID')
+                       ->will($this->returnValue($userId));
+               $user
+                       ->expects($this->once())
+                       ->method('getDisplayName')
+                       ->will($this->returnValue($displayName));
+               $user
+                       ->method('getLastLogin')
+                       ->will($this->returnValue($lastLogin));
+               $user
+                       ->method('getHome')
+                       ->will($this->returnValue($home));
+               $user
+                       ->expects($this->once())
+                       ->method('getBackendClassName')
+                       ->will($this->returnValue($backend));
+
+               $result = [
+                       'name' => $userId,
+                       'displayname' => $displayName,
+                       'groups' => null,
+                       'subadmin' => array(),
+                       'quota' => null,
+                       'storageLocation' => $home,
+                       'lastLogin' => $lastLogin,
+                       'backend' => $backend,
+                       'email' => null,
+                       'isRestoreDisabled' => false,
+               ];
+
+               return [$user, $result];
+       }
+
+       public function testRestorePossibleWithoutEncryption() {
+               list($user, $expectedResult) = $this->mockUser();
+
+               $result = \Test_Helper::invokePrivate($this->container['UsersController'], 'formatUserForIndex', [$user]);
+               $this->assertEquals($expectedResult, $result);
+       }
+
+       public function testRestorePossibleWithAdminAndUserRestore() {
+               list($user, $expectedResult) = $this->mockUser();
+
+               $this->container['OCP\\App\\IAppManager']
+                       ->expects($this->once())
+                       ->method('isEnabledForUser')
+                       ->with(
+                               $this->equalTo('files_encryption')
+                       )
+                       ->will($this->returnValue(true));
+               $this->container['Config']
+                       ->expects($this->once())
+                       ->method('getAppValue')
+                       ->with(
+                               $this->equalTo('files_encryption'),
+                               $this->equalTo('recoveryAdminEnabled'),
+                               $this->anything()
+                       )
+                       ->will($this->returnValue('1'));
+
+               $this->container['Config']
+                       ->expects($this->at(1))
+                       ->method('getUserValue')
+                       ->with(
+                               $this->anything(),
+                               $this->equalTo('files_encryption'),
+                               $this->equalTo('recovery_enabled'),
+                               $this->anything()
+                       )
+                       ->will($this->returnValue('1'));
+
+               $result = \Test_Helper::invokePrivate($this->container['UsersController'], 'formatUserForIndex', [$user]);
+               $this->assertEquals($expectedResult, $result);
+       }
+
+       public function testRestoreNotPossibleWithoutAdminRestore() {
+               list($user, $expectedResult) = $this->mockUser();
+
+               $this->container['OCP\\App\\IAppManager']
+                       ->method('isEnabledForUser')
+                       ->with(
+                               $this->equalTo('files_encryption')
+                       )
+                       ->will($this->returnValue(true));
+
+               $expectedResult['isRestoreDisabled'] = true;
+
+               $result = \Test_Helper::invokePrivate($this->container['UsersController'], 'formatUserForIndex', [$user]);
+               $this->assertEquals($expectedResult, $result);
+       }
+
+       public function testRestoreNotPossibleWithoutUserRestore() {
+               list($user, $expectedResult) = $this->mockUser();
+
+               $this->container['OCP\\App\\IAppManager']
+                       ->expects($this->once())
+                       ->method('isEnabledForUser')
+                       ->with(
+                               $this->equalTo('files_encryption')
+                       )
+                       ->will($this->returnValue(true));
+               $this->container['Config']
+                       ->expects($this->once())
+                       ->method('getAppValue')
+                       ->with(
+                               $this->equalTo('files_encryption'),
+                               $this->equalTo('recoveryAdminEnabled'),
+                               $this->anything()
+                       )
+                       ->will($this->returnValue('1'));
+
+               $this->container['Config']
+                       ->expects($this->at(1))
+                       ->method('getUserValue')
+                       ->with(
+                               $this->anything(),
+                               $this->equalTo('files_encryption'),
+                               $this->equalTo('recovery_enabled'),
+                               $this->anything()
+                       )
+                       ->will($this->returnValue('0'));
+
+               $expectedResult['isRestoreDisabled'] = true;
+
+               $result = \Test_Helper::invokePrivate($this->container['UsersController'], 'formatUserForIndex', [$user]);
+               $this->assertEquals($expectedResult, $result);
+       }
+
 }