Merge pull request #13636 from owncloud/add-warning-if-restore-of-data-is-not-possible

Add value if restore of data is possible for a user

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) {

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,
+		];
 	}
 
 	/**

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 */

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() {

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->container['OCP\\App\\IAppManager'] = $this->getMockBuilder('OCP\\App\\IAppManager')
-		$this->usersController = $this->container['UsersController'];
+			->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);
+	}
+
 }