Merge pull request #2276 from nextcloud/update-email-address

Update email address

8 files changed, 705 insertions, 50 deletions

diff --git a/lib/base.php b/lib/base.php
index 2f5517f4614..00e984d80c5 100644
--- a/lib/base.php
+++ b/lib/base.php
@@ -766,6 +766,7 @@ class OC {
 		self::registerLogRotate();
 		self::registerEncryptionWrapper();
 		self::registerEncryptionHooks();
+		self::registerAccountHooks();
 		self::registerSettingsHooks();
 
 		//make sure temporary files are cleaned up
@@ -876,6 +877,11 @@ class OC {
 		}
 	}
 
+	private static function registerAccountHooks() {
+		$hookHandler = new \OC\Accounts\Hooks(\OC::$server->getLogger());
+		\OCP\Util::connectHook('OC_User', 'changeUser', $hookHandler, 'changeUserHook');
+	}
+
 	/**
 	 * register hooks for the cache
 	 */
diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php
index 6496a521326..3701421a20f 100644
--- a/lib/private/Accounts/AccountManager.php
+++ b/lib/private/Accounts/AccountManager.php
@@ -81,16 +81,22 @@ class AccountManager {
 	 */
 	public function updateUser(IUser $user, $data) {
 		$userData = $this->getUser($user);
+		$updated = true;
 		if (empty($userData)) {
 			$this->insertNewUser($user, $data);
-		} else {
+		} elseif ($userData !== $data) {
 			$this->updateExistingUser($user, $data);
+		} else {
+			// nothing needs to be done if new and old data set are the same
+			$updated = false;
 		}
 
-		$this->eventDispatcher->dispatch(
-			'OC\AccountManager::userUpdated',
-			new GenericEvent($user)
-		);
+		if ($updated) {
+			$this->eventDispatcher->dispatch(
+				'OC\AccountManager::userUpdated',
+				new GenericEvent($user)
+			);
+		}
 	}
 
 	/**
diff --git a/lib/private/Accounts/Hooks.php b/lib/private/Accounts/Hooks.php
new file mode 100644
index 00000000000..38e7e20485b
--- /dev/null
+++ b/lib/private/Accounts/Hooks.php
@@ -0,0 +1,98 @@
+<?php
+/**
+ * @copyright Copyright (c) 2016 Bjoern Schiessle <bjoern@schiessle.org>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+
+namespace OC\Accounts;
+
+use OCP\ILogger;
+use OCP\IUser;
+
+class Hooks {
+
+	/** @var  AccountManager */
+	private $accountManager = null;
+
+	/** @var ILogger */
+	private $logger;
+
+	/**
+	 * Hooks constructor.
+	 *
+	 * @param ILogger $logger
+	 */
+	public function __construct(ILogger $logger) {
+		$this->logger = $logger;
+	}
+
+	/**
+	 * update accounts table if email address or display name was changed from outside
+	 *
+	 * @param array $params
+	 */
+	public function changeUserHook($params) {
+
+		$accountManager = $this->getAccountManager();
+
+		/** @var IUser $user */
+		$user = isset($params['user']) ? $params['user'] : null;
+		$feature = isset($params['feature']) ? $params['feature'] : null;
+		$newValue = isset($params['value']) ? $params['value'] : null;
+
+		if (is_null($user) || is_null($feature) || is_null($newValue)) {
+			$this->logger->warning('Missing expected parameters in change user hook');
+			return;
+		}
+
+		$accountData = $accountManager->getUser($user);
+
+		switch ($feature) {
+			case 'eMailAddress':
+				if ($accountData[AccountManager::PROPERTY_EMAIL]['value'] !== $newValue) {
+					$accountData[AccountManager::PROPERTY_EMAIL]['value'] = $newValue;
+					$accountManager->updateUser($user, $accountData);
+				}
+				break;
+			case 'displayName':
+				if ($accountData[AccountManager::PROPERTY_DISPLAYNAME]['value'] !== $newValue) {
+					$accountData[AccountManager::PROPERTY_DISPLAYNAME]['value'] = $newValue;
+					$accountManager->updateUser($user, $accountData);
+				}
+				break;
+		}
+
+	}
+
+	/**
+	 * return instance of accountManager
+	 *
+	 * @return AccountManager
+	 */
+	protected function getAccountManager() {
+		if (is_null($this->accountManager)) {
+			$this->accountManager = new AccountManager(
+				\OC::$server->getDatabaseConnection(),
+				\OC::$server->getEventDispatcher()
+			);
+		}
+		return $this->accountManager;
+	}
+
+}
diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php
index 206f1872542..20440e6d395 100644
--- a/settings/Controller/UsersController.php
+++ b/settings/Controller/UsersController.php
@@ -536,7 +536,6 @@ class UsersController extends Controller {
 									$twitterScope
 	) {
 
-
 		if(!empty($email) && !$this->mailer->validateMailAddress($email)) {
 			return new DataResponse(
 				array(
@@ -549,8 +548,6 @@ class UsersController extends Controller {
 			);
 		}
 
-		$user = $this->userSession->getUser();
-
 		$data = [
 			AccountManager::PROPERTY_AVATAR =>  ['scope' => $avatarScope],
 			AccountManager::PROPERTY_DISPLAYNAME => ['value' => $displayname, 'scope' => $displaynameScope],
@@ -561,7 +558,7 @@ class UsersController extends Controller {
 			AccountManager::PROPERTY_TWITTER => ['value' => $twitter, 'scope' => $twitterScope]
 		];
 
-		$this->accountManager->updateUser($user, $data);
+		$user = $this->userSession->getUser();
 
 		try {
 			$this->saveUserSettings($user, $data);
@@ -603,23 +600,30 @@ class UsersController extends Controller {
 	 * @param array $data
 	 * @throws ForbiddenException
 	 */
-	private function saveUserSettings(IUser $user, $data) {
+	protected function saveUserSettings(IUser $user, $data) {
 
 		// keep the user back-end up-to-date with the latest display name and email
 		// address
 		$oldDisplayName = $user->getDisplayName();
-		if (isset($data[AccountManager::PROPERTY_DISPLAYNAME]['value'])  && $oldDisplayName !== $data[AccountManager::PROPERTY_DISPLAYNAME]['value']) {
+		if (isset($data[AccountManager::PROPERTY_DISPLAYNAME]['value'])
+			&& $oldDisplayName !== $data[AccountManager::PROPERTY_DISPLAYNAME]['value']
+		) {
 			$result = $user->setDisplayName($data[AccountManager::PROPERTY_DISPLAYNAME]['value']);
 			if ($result === false) {
 				throw new ForbiddenException($this->l10n->t('Unable to change full name'));
 			}
 		}
 
-		if (isset($data['email'][0]['value']) && $user->getEMailAddress() !== $data['email'][0]['value']) {
-			$result = $user->setEMailAddress($data['email'][0]['value']);
-			if ($result === false) {
-				throw new ForbiddenException($this->l10n->t('Unable to change mail address'));
+		$oldEmailAddress = $user->getEMailAddress();
+		if (isset($data[AccountManager::PROPERTY_EMAIL]['value'])
+			&& $oldEmailAddress !== $data[AccountManager::PROPERTY_EMAIL]['value']
+		) {
+			// this is the only permission a backend provides and is also used
+			// for the permission of setting a email address
+			if (!$user->canChangeDisplayName()) {
+				throw new ForbiddenException($this->l10n->t('Unable to change email address'));
 			}
+			$user->setEMailAddress($data[AccountManager::PROPERTY_EMAIL]['value']);
 		}
 
 		$this->accountManager->updateUser($user, $data);
@@ -720,4 +724,95 @@ class UsersController extends Controller {
 			]);
 		}
 	}
+
+	/**
+	 * Set the mail address of a user
+	 *
+	 * @NoAdminRequired
+	 * @NoSubadminRequired
+	 * @PasswordConfirmationRequired
+	 *
+	 * @param string $id
+	 * @param string $mailAddress
+	 * @return DataResponse
+	 */
+	public function setEMailAddress($id, $mailAddress) {
+		$user = $this->userManager->get($id);
+		if (!$this->isAdmin
+			&& !$this->groupManager->getSubAdmin()->isUserAccessible($this->userSession->getUser(), $user)
+		) {
+			return new DataResponse(
+				array(
+					'status' => 'error',
+					'data' => array(
+						'message' => (string)$this->l10n->t('Forbidden')
+					)
+				),
+				Http::STATUS_FORBIDDEN
+			);
+		}
+
+		if($mailAddress !== '' && !$this->mailer->validateMailAddress($mailAddress)) {
+			return new DataResponse(
+				array(
+					'status' => 'error',
+					'data' => array(
+						'message' => (string)$this->l10n->t('Invalid mail address')
+					)
+				),
+				Http::STATUS_UNPROCESSABLE_ENTITY
+			);
+		}
+
+		if (!$user) {
+			return new DataResponse(
+				array(
+					'status' => 'error',
+					'data' => array(
+						'message' => (string)$this->l10n->t('Invalid user')
+					)
+				),
+				Http::STATUS_UNPROCESSABLE_ENTITY
+			);
+		}
+		// this is the only permission a backend provides and is also used
+		// for the permission of setting a email address
+		if (!$user->canChangeDisplayName()) {
+			return new DataResponse(
+				array(
+					'status' => 'error',
+					'data' => array(
+						'message' => (string)$this->l10n->t('Unable to change mail address')
+					)
+				),
+				Http::STATUS_FORBIDDEN
+			);
+		}
+
+		$userData = $this->accountManager->getUser($user);
+		$userData[AccountManager::PROPERTY_EMAIL]['value'] = $mailAddress;
+
+		try {
+			$this->saveUserSettings($user, $userData);
+			return new DataResponse(
+				array(
+					'status' => 'success',
+					'data' => array(
+						'username' => $id,
+						'mailAddress' => $mailAddress,
+						'message' => (string)$this->l10n->t('Email saved')
+					)
+				),
+				Http::STATUS_OK
+			);
+		} catch (ForbiddenException $e) {
+			return new DataResponse([
+				'status' => 'error',
+				'data' => [
+					'message' => $e->getMessage()
+				],
+			]);
+		}
+	}
+
 }
diff --git a/settings/routes.php b/settings/routes.php
index 62cfc398fdc..3f034d363e2 100644
--- a/settings/routes.php
+++ b/settings/routes.php
@@ -51,6 +51,7 @@ $application->registerRoutes($this, [
 		['name' => 'AppSettings#listApps', 'url' => '/settings/apps/list', 'verb' => 'GET'],
 		['name' => 'SecuritySettings#trustedDomains', 'url' => '/settings/admin/security/trustedDomains', 'verb' => 'POST'],
 		['name' => 'Users#setDisplayName', 'url' => '/settings/users/{username}/displayName', 'verb' => 'POST'],
+		['name' => 'Users#setEMailAddress', 'url' => '/settings/users/{id}/mailAddress', 'verb' => 'PUT'],
 		['name' => 'Users#setUserSettings', 'url' => '/settings/users/{username}/settings', 'verb' => 'PUT'],
 		['name' => 'Users#stats', 'url' => '/settings/users/stats', 'verb' => 'GET'],
 		['name' => 'LogSettings#setLogLevel', 'url' => '/settings/admin/log/level', 'verb' => 'POST'],
diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php
index ec92479b40e..69082a7929c 100644
--- a/tests/Settings/Controller/UsersControllerTest.php
+++ b/tests/Settings/Controller/UsersControllerTest.php
@@ -103,27 +103,51 @@ class UsersControllerTest extends \Test\TestCase {
 
 	/**
 	 * @param bool $isAdmin
-	 * @return UsersController
+	 * @return UsersController | \PHPUnit_Framework_MockObject_MockObject
 	 */
-	protected function getController($isAdmin) {
-		return new UsersController(
-			'settings',
-			$this->createMock(IRequest::class),
-			$this->userManager,
-			$this->groupManager,
-			$this->userSession,
-			$this->config,
-			$isAdmin,
-			$this->l,
-			$this->logger,
-			$this->defaults,
-			$this->mailer,
-			'no-reply@owncloud.com',
-			$this->urlGenerator,
-			$this->appManager,
-			$this->avatarManager,
-			$this->accountManager
-		);
+	protected function getController($isAdmin = false, $mockedMethods = []) {
+		if (empty($mockedMethods)) {
+			return new UsersController(
+				'settings',
+				$this->createMock(IRequest::class),
+				$this->userManager,
+				$this->groupManager,
+				$this->userSession,
+				$this->config,
+				$isAdmin,
+				$this->l,
+				$this->logger,
+				$this->defaults,
+				$this->mailer,
+				'no-reply@owncloud.com',
+				$this->urlGenerator,
+				$this->appManager,
+				$this->avatarManager,
+				$this->accountManager
+			);
+		} else {
+			return $this->getMockBuilder(UsersController::class)
+				->setConstructorArgs(
+					[
+						'settings',
+						$this->createMock(IRequest::class),
+						$this->userManager,
+						$this->groupManager,
+						$this->userSession,
+						$this->config,
+						$isAdmin,
+						$this->l,
+						$this->logger,
+						$this->defaults,
+						$this->mailer,
+						'no-reply@owncloud.com',
+						$this->urlGenerator,
+						$this->appManager,
+						$this->avatarManager,
+						$this->accountManager
+					]
+				)->setMethods($mockedMethods)->getMock();
+		}
 	}
 
 	public function testIndexAdmin() {
@@ -2010,4 +2034,263 @@ class UsersControllerTest extends \Test\TestCase {
 		$response = $controller->setDisplayName($user->getUID(), 'newDisplayName');
 		$this->assertEquals($expectedResponse, $response);
 	}
+
+	/**
+	 * @dataProvider dataTestSetUserSettings
+	 *
+	 * @param string $email
+	 * @param bool $validEmail
+	 * @param $expectedStatus
+	 */
+	public function testSetUserSettings($email, $validEmail, $expectedStatus) {
+		$controller = $this->getController(false, ['saveUserSettings']);
+		$user = $this->createMock(IUser::class);
+
+		$this->userSession->method('getUser')->willReturn($user);
+
+		if (!empty($email) && $validEmail) {
+			$this->mailer->expects($this->once())->method('validateMailAddress')
+				->willReturn($validEmail);
+		}
+
+		$saveData = (!empty($email) && $validEmail) || empty($email);
+
+		if ($saveData) {
+			$controller->expects($this->once())->method('saveUserSettings');
+		} else {
+			$controller->expects($this->never())->method('saveUserSettings');
+		}
+
+		$result = $controller->setUserSettings(
+			AccountManager::VISIBILITY_CONTACTS_ONLY,
+			'displayName',
+			AccountManager::VISIBILITY_CONTACTS_ONLY,
+			'47658468',
+			AccountManager::VISIBILITY_CONTACTS_ONLY,
+			$email,
+			AccountManager::VISIBILITY_CONTACTS_ONLY,
+			'nextcloud.com',
+			AccountManager::VISIBILITY_CONTACTS_ONLY,
+			'street and city',
+			AccountManager::VISIBILITY_CONTACTS_ONLY,
+			'@nextclouders',
+			AccountManager::VISIBILITY_CONTACTS_ONLY
+		);
+
+		$this->assertSame($expectedStatus, $result->getStatus());
+	}
+
+	public function dataTestSetUserSettings() {
+		return [
+			['', true, Http::STATUS_OK],
+			['', false, Http::STATUS_OK],
+			['example.com', false, Http::STATUS_UNPROCESSABLE_ENTITY],
+			['john@example.com', true, Http::STATUS_OK],
+		];
+	}
+
+	/**
+	 * @dataProvider dataTestSaveUserSettings
+	 *
+	 * @param array $data
+	 * @param string $oldEmailAddress
+	 * @param string $oldDisplayName
+	 */
+	public function testSaveUserSettings($data,
+										 $oldEmailAddress,
+										 $oldDisplayName
+	) {
+		$controller = $this->getController();
+		$user = $this->createMock(IUser::class);
+
+		$user->method('getDisplayName')->willReturn($oldDisplayName);
+		$user->method('getEMailAddress')->willReturn($oldEmailAddress);
+		$user->method('canChangeDisplayName')->willReturn(true);
+
+		if ($data[AccountManager::PROPERTY_EMAIL]['value'] !== $oldEmailAddress) {
+			$user->expects($this->once())->method('setEMailAddress')
+				->with($data[AccountManager::PROPERTY_EMAIL]['value'])
+				->willReturn(true);
+		} else {
+			$user->expects($this->never())->method('setEMailAddress');
+		}
+
+		if ($data[AccountManager::PROPERTY_DISPLAYNAME]['value'] !== $oldDisplayName) {
+			$user->expects($this->once())->method('setDisplayName')
+				->with($data[AccountManager::PROPERTY_DISPLAYNAME]['value'])
+				->willReturn(true);
+		} else {
+			$user->expects($this->never())->method('setDisplayName');
+		}
+
+		$this->accountManager->expects($this->once())->method('updateUser')
+			->with($user, $data);
+
+		$this->invokePrivate($controller, 'saveUserSettings', [$user, $data]);
+	}
+
+	public function dataTestSaveUserSettings() {
+		return [
+			[
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => 'john@example.com'],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'john doe'],
+				],
+				'john@example.com',
+				'john doe'
+			],
+			[
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => 'john@example.com'],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'john doe'],
+				],
+				'johnNew@example.com',
+				'john New doe'
+			],
+			[
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => 'john@example.com'],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'john doe'],
+				],
+				'johnNew@example.com',
+				'john doe'
+			],
+			[
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => 'john@example.com'],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'john doe'],
+				],
+				'john@example.com',
+				'john New doe'
+			]
+
+		];
+	}
+
+	/**
+	 * @dataProvider dataTestSaveUserSettingsException
+	 *
+	 * @param array $data
+	 * @param string $oldEmailAddress
+	 * @param string $oldDisplayName
+	 * @param bool $setDisplayNameResult
+	 * @param bool $canChangeEmail
+	 *
+	 * @expectedException \OC\ForbiddenException
+	 */
+	public function testSaveUserSettingsException($data,
+												  $oldEmailAddress,
+												  $oldDisplayName,
+												  $setDisplayNameResult,
+												  $canChangeEmail
+	) {
+		$controller = $this->getController();
+		$user = $this->createMock(IUser::class);
+
+		$user->method('getDisplayName')->willReturn($oldDisplayName);
+		$user->method('getEMailAddress')->willReturn($oldEmailAddress);
+
+		if ($data[AccountManager::PROPERTY_EMAIL]['value'] !== $oldEmailAddress) {
+			$user->method('canChangeDisplayName')
+				->willReturn($canChangeEmail);
+		}
+
+		if ($data[AccountManager::PROPERTY_DISPLAYNAME]['value'] !== $oldDisplayName) {
+			$user->method('setDisplayName')
+				->with($data[AccountManager::PROPERTY_DISPLAYNAME]['value'])
+				->willReturn($setDisplayNameResult);
+		}
+
+		$this->invokePrivate($controller, 'saveUserSettings', [$user, $data]);
+	}
+
+
+	public function dataTestSaveUserSettingsException() {
+		return [
+			[
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => 'john@example.com'],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'john doe'],
+				],
+				'johnNew@example.com',
+				'john New doe',
+				true,
+				false
+			],
+			[
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => 'john@example.com'],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'john doe'],
+				],
+				'johnNew@example.com',
+				'john New doe',
+				false,
+				true
+			],
+			[
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => 'john@example.com'],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'john doe'],
+				],
+				'johnNew@example.com',
+				'john New doe',
+				false,
+				false
+			],
+
+		];
+	}
+
+	/**
+	 * @return array
+	 */
+	public function setEmailAddressData() {
+		return [
+			/* mailAddress,    isValid, expectsUpdate, canChangeDisplayName, responseCode */
+			[ '',              true,    true,          true,                 Http::STATUS_OK ],
+			[ 'foo@local',     true,    true,          true,                 Http::STATUS_OK],
+			[ 'foo@bar@local', false,   false,         true,                 Http::STATUS_UNPROCESSABLE_ENTITY],
+			[ 'foo@local',     true,    false,         false,                Http::STATUS_FORBIDDEN],
+		];
+	}
+	/**
+	 * @dataProvider setEmailAddressData
+	 *
+	 */
+	public function testSetEMailAddress($mailAddress, $isValid, $expectsUpdate, $canChangeDisplayName, $responseCode) {
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user
+			->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('foo'));
+		$user
+			->expects($this->any())
+			->method('canChangeDisplayName')
+			->will($this->returnValue($canChangeDisplayName));
+		$user
+			->expects($expectsUpdate ? $this->once() : $this->never())
+			->method('setEMailAddress')
+			->with(
+				$this->equalTo($mailAddress)
+			);
+		$this->mailer
+			->expects($this->any())
+			->method('validateMailAddress')
+			->with($mailAddress)
+			->willReturn($isValid);
+		if ($isValid) {
+			$user->expects($this->atLeastOnce())
+				->method('canChangeDisplayName')
+				->willReturn(true);
+			$this->userManager
+				->expects($this->atLeastOnce())
+				->method('get')
+				->with('foo')
+				->will($this->returnValue($user));
+		}
+		$controller = $this->getController(true);
+		$response = $controller->setEMailAddress($user->getUID(), $mailAddress);
+		$this->assertSame($responseCode, $response->getStatus());
+	}
 }
diff --git a/tests/lib/Accounts/AccountsManagerTest.php b/tests/lib/Accounts/AccountsManagerTest.php
index de88fbcab97..60811140e72 100644
--- a/tests/lib/Accounts/AccountsManagerTest.php
+++ b/tests/lib/Accounts/AccountsManagerTest.php
@@ -78,36 +78,45 @@ class AccountsManagerTest extends TestCase {
 	 *
 	 * @param bool $userAlreadyExists
 	 */
-	public function testUpdateUser($userAlreadyExists) {
+	public function testUpdateUser($newData, $oldData, $insertNew, $updateExisitng) {
 		$accountManager = $this->getInstance(['getUser', 'insertNewUser', 'updateExistingUser']);
 		$user = $this->getMockBuilder('OCP\IUser')->getMock();
 
-		$accountManager->expects($this->once())->method('getUser')->with($user)->willReturn($userAlreadyExists);
+		$accountManager->expects($this->once())->method('getUser')->with($user)->willReturn($oldData);
 
-		if ($userAlreadyExists) {
+		if ($updateExisitng) {
 			$accountManager->expects($this->once())->method('updateExistingUser')
-				->with($user, 'data');
+				->with($user, $newData);
 			$accountManager->expects($this->never())->method('insertNewUser');
-		} else {
+		}
+		if ($insertNew) {
 			$accountManager->expects($this->once())->method('insertNewUser')
-				->with($user, 'data');
+				->with($user, $newData);
 			$accountManager->expects($this->never())->method('updateExistingUser');
 		}
 
-		$this->eventDispatcher->expects($this->once())->method('dispatch')
-			->willReturnCallback(function($eventName, $event) use ($user) {
-				$this->assertSame('OC\AccountManager::userUpdated', $eventName);
-				$this->assertInstanceOf('Symfony\Component\EventDispatcher\GenericEvent', $event);
-			}
-			);
+		if (!$insertNew && !$updateExisitng) {
+			$accountManager->expects($this->never())->method('updateExistingUser');
+			$accountManager->expects($this->never())->method('insertNewUser');
+			$this->eventDispatcher->expects($this->never())->method('dispatch');
+		} else {
+			$this->eventDispatcher->expects($this->once())->method('dispatch')
+				->willReturnCallback(
+					function ($eventName, $event) use ($user) {
+						$this->assertSame('OC\AccountManager::userUpdated', $eventName);
+						$this->assertInstanceOf('Symfony\Component\EventDispatcher\GenericEvent', $event);
+					}
+				);
+		}
 
-		$accountManager->updateUser($user, 'data');
+		$accountManager->updateUser($user, $newData);
 	}
 
 	public function dataTrueFalse() {
 		return [
-			[true],
-			[false]
+			[['newData'], ['oldData'], false, true],
+			[['newData'], [], true, false],
+			[['oldData'], ['oldData'], false, false]
 		];
 	}
 
diff --git a/tests/lib/Accounts/HooksTest.php b/tests/lib/Accounts/HooksTest.php
new file mode 100644
index 00000000000..071e78146ea
--- /dev/null
+++ b/tests/lib/Accounts/HooksTest.php
@@ -0,0 +1,157 @@
+<?php
+/**
+ * @copyright Copyright (c) 2016 Bjoern Schiessle <bjoern@schiessle.org>
+ *
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+
+namespace Test\Accounts;
+
+
+use OC\Accounts\AccountManager;
+use OC\Accounts\Hooks;
+use OCP\ILogger;
+use OCP\IUser;
+use Test\TestCase;
+
+/**
+ * Class HooksTest
+ *
+ * @package Test\Accounts
+ * @group DB
+ */
+class HooksTest extends TestCase  {
+
+	/** @var  ILogger | \PHPUnit_Framework_MockObject_MockObject */
+	private $logger;
+
+	/** @var  AccountManager | \PHPUnit_Framework_MockObject_MockObject */
+	private $accountManager;
+
+	/** @var  Hooks | \PHPUnit_Framework_MockObject_MockObject */
+	private $hooks;
+
+	public function setUp() {
+		parent::setUp();
+
+		$this->logger = $this->createMock(ILogger::class);
+		$this->accountManager = $this->getMockBuilder(AccountManager::class)
+			->disableOriginalConstructor()->getMock();
+
+		$this->hooks = $this->getMockBuilder(Hooks::class)
+			->setConstructorArgs([$this->logger])
+			->setMethods(['getAccountManager'])
+			->getMock();
+
+		$this->hooks->method('getAccountManager')->willReturn($this->accountManager);
+	}
+
+	/**
+	 * @dataProvider dataTestChangeUserHook
+	 *
+	 * @param $params
+	 * @param $data
+	 * @param $setEmail
+	 * @param $setDisplayName
+	 * @param $error
+	 */
+	public function testChangeUserHook($params, $data, $setEmail, $setDisplayName, $error) {
+		if ($error) {
+			$this->accountManager->expects($this->never())->method('getUser');
+			$this->accountManager->expects($this->never())->method('updateUser');
+		} else {
+			$this->accountManager->expects($this->once())->method('getUser')->willReturn($data);
+			$newData = $data;
+			if ($setEmail) {
+				$newData[AccountManager::PROPERTY_EMAIL]['value'] = $params['value'];
+				$this->accountManager->expects($this->once())->method('updateUser')
+					->with($params['user'], $newData);
+			} elseif ($setDisplayName) {
+				$newData[AccountManager::PROPERTY_DISPLAYNAME]['value'] = $params['value'];
+				$this->accountManager->expects($this->once())->method('updateUser')
+					->with($params['user'], $newData);
+			} else {
+				$this->accountManager->expects($this->never())->method('updateUser');
+			}
+		}
+
+		$this->hooks->changeUserHook($params);
+
+	}
+
+	public function dataTestChangeUserHook() {
+		$user = $this->createMock(IUser::class);
+		return [
+			[
+				['feature' => '', 'value' => ''],
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => ''],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => '']
+				],
+				false, false, true
+			],
+			[
+				['user' => $user, 'value' => ''],
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => ''],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => '']
+				],
+				false, false, true
+			],
+			[
+				['user' => $user, 'feature' => ''],
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => ''],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => '']
+				],
+				false, false, true
+			],
+			[
+				['user' => $user, 'feature' => 'foo', 'value' => 'bar'],
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => 'oldMail@example.com'],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'oldDisplayName']
+				],
+				false, false, false
+			],
+			[
+				['user' => $user, 'feature' => 'eMailAddress', 'value' => 'newMail@example.com'],
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => 'oldMail@example.com'],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'oldDisplayName']
+				],
+				true, false, false
+			],
+			[
+				['user' => $user, 'feature' => 'displayName', 'value' => 'newDisplayName'],
+				[
+					AccountManager::PROPERTY_EMAIL => ['value' => 'oldMail@example.com'],
+					AccountManager::PROPERTY_DISPLAYNAME => ['value' => 'oldDisplayName']
+				],
+				false, true, false
+			],
+		];
+	}
+
+	public function testGetAccountManager() {
+		$hooks = new Hooks($this->logger);
+		$result = $this->invokePrivate($hooks, 'getAccountManager');
+		$this->assertInstanceOf(AccountManager::class, $result);
+	}
+
+}