From 546989959c11bc773461cdd8ff0233e3f54255e0 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 23 Nov 2016 13:05:01 +0100 Subject: [PATCH] update email address correctly Signed-off-by: Bjoern Schiessle --- settings/Controller/UsersController.php | 18 +- .../Controller/UsersControllerTest.php | 271 ++++++++++++++++-- 2 files changed, 261 insertions(+), 28 deletions(-) diff --git a/settings/Controller/UsersController.php b/settings/Controller/UsersController.php index 206f1872542..42d464c961a 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,20 +600,25 @@ 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']); + $oldEmailAddress = $user->getEMailAddress(); + if (isset($data[AccountManager::PROPERTY_EMAIL]['value']) + && $oldEmailAddress !== $data[AccountManager::PROPERTY_EMAIL]['value'] + ) { + $result = $user->setEMailAddress($data[AccountManager::PROPERTY_EMAIL]['value']); if ($result === false) { throw new ForbiddenException($this->l10n->t('Unable to change mail address')); } diff --git a/tests/Settings/Controller/UsersControllerTest.php b/tests/Settings/Controller/UsersControllerTest.php index ec92479b40e..9f381e31957 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\ForbiddenException; use OC\Group\Manager; use OC\Settings\Controller\UsersController; use OCP\App\IAppManager; @@ -103,27 +104,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 +2035,210 @@ 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->getMock(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->getMock(IUser::class); + + $user->method('getDisplayName')->willReturn($oldDisplayName); + $user->method('getEMailAddress')->willReturn($oldEmailAddress); + + 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 $setEmailResult + * + * @expectedException \OC\ForbiddenException + */ + public function testSaveUserSettingsException($data, + $oldEmailAddress, + $oldDisplayName, + $setDisplayNameResult, + $setEmailResult + ) { + $controller = $this->getController(); + $user = $this->getMock(IUser::class); + + $user->method('getDisplayName')->willReturn($oldDisplayName); + $user->method('getEMailAddress')->willReturn($oldEmailAddress); + + if ($data[AccountManager::PROPERTY_EMAIL]['value'] !== $oldEmailAddress) { + $user->method('setEMailAddress') + ->with($data[AccountManager::PROPERTY_EMAIL]['value']) + ->willReturn($setEmailResult); + } + + 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 + ], + + ]; + } } -- 2.39.5