From 8cae515daa8397e047456a7ae223fabf6e3bf360 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jan 2021 20:32:41 +0100 Subject: [PATCH 01/10] Use constants from interface rather than class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../lib/Controller/UsersController.php | 4 +- .../tests/Controller/UsersControllerTest.php | 38 +++++++++---------- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index a9b72571de6be..d2f168387172c 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -523,14 +523,14 @@ public function getVerificationCode(string $account, bool $onlyVerificationCode) switch ($account) { case 'verify-twitter': - $accountData[IAccountManager::PROPERTY_TWITTER]['verified'] = AccountManager::VERIFICATION_IN_PROGRESS; + $accountData[IAccountManager::PROPERTY_TWITTER]['verified'] = IAccountManager::VERIFICATION_IN_PROGRESS; $msg = $this->l10n->t('In order to verify your Twitter account, post the following tweet on Twitter (please make sure to post it without any line breaks):'); $code = $codeMd5; $type = IAccountManager::PROPERTY_TWITTER; $accountData[IAccountManager::PROPERTY_TWITTER]['signature'] = $signature; break; case 'verify-website': - $accountData[IAccountManager::PROPERTY_WEBSITE]['verified'] = AccountManager::VERIFICATION_IN_PROGRESS; + $accountData[IAccountManager::PROPERTY_WEBSITE]['verified'] = IAccountManager::VERIFICATION_IN_PROGRESS; $msg = $this->l10n->t('In order to verify your Website, store the following content in your web-root at \'.well-known/CloudIdVerificationCode.txt\' (please make sure that the complete text is in one line):'); $type = IAccountManager::PROPERTY_WEBSITE; $accountData[IAccountManager::PROPERTY_WEBSITE]['signature'] = $signature; diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index f9652053de891..b3e8937821a72 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -209,42 +209,42 @@ public function testSetUserSettings($email, $validEmail, $expectedStatus) { IAccountManager::PROPERTY_DISPLAYNAME => [ 'value' => 'Display name', - 'scope' => AccountManager::SCOPE_FEDERATED, - 'verified' => AccountManager::NOT_VERIFIED, + 'scope' => IAccountManager::SCOPE_FEDERATED, + 'verified' => IAccountManager::NOT_VERIFIED, ], IAccountManager::PROPERTY_ADDRESS => [ 'value' => '', - 'scope' => AccountManager::SCOPE_LOCAL, - 'verified' => AccountManager::NOT_VERIFIED, + 'scope' => IAccountManager::SCOPE_LOCAL, + 'verified' => IAccountManager::NOT_VERIFIED, ], IAccountManager::PROPERTY_WEBSITE => [ 'value' => '', - 'scope' => AccountManager::SCOPE_LOCAL, - 'verified' => AccountManager::NOT_VERIFIED, + 'scope' => IAccountManager::SCOPE_LOCAL, + 'verified' => IAccountManager::NOT_VERIFIED, ], IAccountManager::PROPERTY_EMAIL => [ 'value' => '', - 'scope' => AccountManager::SCOPE_FEDERATED, - 'verified' => AccountManager::NOT_VERIFIED, + 'scope' => IAccountManager::SCOPE_FEDERATED, + 'verified' => IAccountManager::NOT_VERIFIED, ], IAccountManager::PROPERTY_AVATAR => [ - 'scope' => AccountManager::SCOPE_FEDERATED + 'scope' => IAccountManager::SCOPE_FEDERATED ], IAccountManager::PROPERTY_PHONE => [ 'value' => '', - 'scope' => AccountManager::SCOPE_LOCAL, - 'verified' => AccountManager::NOT_VERIFIED, + 'scope' => IAccountManager::SCOPE_LOCAL, + 'verified' => IAccountManager::NOT_VERIFIED, ], IAccountManager::PROPERTY_TWITTER => [ 'value' => '', - 'scope' => AccountManager::SCOPE_LOCAL, - 'verified' => AccountManager::NOT_VERIFIED, + 'scope' => IAccountManager::SCOPE_LOCAL, + 'verified' => IAccountManager::NOT_VERIFIED, ], ]); @@ -504,18 +504,18 @@ public function testGetVerificationCode($account, $type, $dataBefore, $expectedD public function dataTestGetVerificationCode() { $accountDataBefore = [ - IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://nextcloud.com', 'verified' => AccountManager::NOT_VERIFIED], - IAccountManager::PROPERTY_TWITTER => ['value' => '@nextclouders', 'verified' => AccountManager::NOT_VERIFIED, 'signature' => 'theSignature'], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://nextcloud.com', 'verified' => IAccountManager::NOT_VERIFIED], + IAccountManager::PROPERTY_TWITTER => ['value' => '@nextclouders', 'verified' => IAccountManager::NOT_VERIFIED, 'signature' => 'theSignature'], ]; $accountDataAfterWebsite = [ - IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://nextcloud.com', 'verified' => AccountManager::VERIFICATION_IN_PROGRESS, 'signature' => 'theSignature'], - IAccountManager::PROPERTY_TWITTER => ['value' => '@nextclouders', 'verified' => AccountManager::NOT_VERIFIED, 'signature' => 'theSignature'], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://nextcloud.com', 'verified' => IAccountManager::VERIFICATION_IN_PROGRESS, 'signature' => 'theSignature'], + IAccountManager::PROPERTY_TWITTER => ['value' => '@nextclouders', 'verified' => IAccountManager::NOT_VERIFIED, 'signature' => 'theSignature'], ]; $accountDataAfterTwitter = [ - IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://nextcloud.com', 'verified' => AccountManager::NOT_VERIFIED], - IAccountManager::PROPERTY_TWITTER => ['value' => '@nextclouders', 'verified' => AccountManager::VERIFICATION_IN_PROGRESS, 'signature' => 'theSignature'], + IAccountManager::PROPERTY_WEBSITE => ['value' => 'https://nextcloud.com', 'verified' => IAccountManager::NOT_VERIFIED], + IAccountManager::PROPERTY_TWITTER => ['value' => '@nextclouders', 'verified' => IAccountManager::VERIFICATION_IN_PROGRESS, 'signature' => 'theSignature'], ]; return [ From ef5d538448cc2322bff0762df417d6ac110bded2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jan 2021 20:34:10 +0100 Subject: [PATCH 02/10] Extract default test data to a helper getter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../tests/Controller/UsersControllerTest.php | 88 ++++++++++--------- 1 file changed, 46 insertions(+), 42 deletions(-) diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index b3e8937821a72..fd536a399e2b4 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -180,6 +180,51 @@ protected function getController($isAdmin = false, $mockedMethods = []) { } } + protected function getDefaultAccountManagerUserData() { + return [ + IAccountManager::PROPERTY_DISPLAYNAME => + [ + 'value' => 'Display name', + 'scope' => IAccountManager::SCOPE_FEDERATED, + 'verified' => IAccountManager::NOT_VERIFIED, + ], + IAccountManager::PROPERTY_ADDRESS => + [ + 'value' => '', + 'scope' => IAccountManager::SCOPE_LOCAL, + 'verified' => IAccountManager::NOT_VERIFIED, + ], + IAccountManager::PROPERTY_WEBSITE => + [ + 'value' => '', + 'scope' => IAccountManager::SCOPE_LOCAL, + 'verified' => IAccountManager::NOT_VERIFIED, + ], + IAccountManager::PROPERTY_EMAIL => + [ + 'value' => '', + 'scope' => IAccountManager::SCOPE_FEDERATED, + 'verified' => IAccountManager::NOT_VERIFIED, + ], + IAccountManager::PROPERTY_AVATAR => + [ + 'scope' => IAccountManager::SCOPE_FEDERATED + ], + IAccountManager::PROPERTY_PHONE => + [ + 'value' => '', + 'scope' => IAccountManager::SCOPE_LOCAL, + 'verified' => IAccountManager::NOT_VERIFIED, + ], + IAccountManager::PROPERTY_TWITTER => + [ + 'value' => '', + 'scope' => IAccountManager::SCOPE_LOCAL, + 'verified' => IAccountManager::NOT_VERIFIED, + ], + ]; + } + /** * @dataProvider dataTestSetUserSettings * @@ -205,48 +250,7 @@ public function testSetUserSettings($email, $validEmail, $expectedStatus) { $this->accountManager->expects($this->once()) ->method('getUser') ->with($user) - ->willReturn([ - IAccountManager::PROPERTY_DISPLAYNAME => - [ - 'value' => 'Display name', - 'scope' => IAccountManager::SCOPE_FEDERATED, - 'verified' => IAccountManager::NOT_VERIFIED, - ], - IAccountManager::PROPERTY_ADDRESS => - [ - 'value' => '', - 'scope' => IAccountManager::SCOPE_LOCAL, - 'verified' => IAccountManager::NOT_VERIFIED, - ], - IAccountManager::PROPERTY_WEBSITE => - [ - 'value' => '', - 'scope' => IAccountManager::SCOPE_LOCAL, - 'verified' => IAccountManager::NOT_VERIFIED, - ], - IAccountManager::PROPERTY_EMAIL => - [ - 'value' => '', - 'scope' => IAccountManager::SCOPE_FEDERATED, - 'verified' => IAccountManager::NOT_VERIFIED, - ], - IAccountManager::PROPERTY_AVATAR => - [ - 'scope' => IAccountManager::SCOPE_FEDERATED - ], - IAccountManager::PROPERTY_PHONE => - [ - 'value' => '', - 'scope' => IAccountManager::SCOPE_LOCAL, - 'verified' => IAccountManager::NOT_VERIFIED, - ], - IAccountManager::PROPERTY_TWITTER => - [ - 'value' => '', - 'scope' => IAccountManager::SCOPE_LOCAL, - 'verified' => IAccountManager::NOT_VERIFIED, - ], - ]); + ->willReturn($this->getDefaultAccountManagerUserData()); $controller->expects($this->once()) ->method('saveUserSettings') From 190ffd75b7d3e24e8d8b31ba387e3f33c3600bb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jan 2021 20:37:14 +0100 Subject: [PATCH 03/10] Change default test data to values less similar to empty values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Right now it makes no difference, but this should make future tests clearer, specially in case of failure. Signed-off-by: Daniel Calviño Sánchez --- .../tests/Controller/UsersControllerTest.php | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index fd536a399e2b4..5fef8b8ccc2ff 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -184,27 +184,27 @@ protected function getDefaultAccountManagerUserData() { return [ IAccountManager::PROPERTY_DISPLAYNAME => [ - 'value' => 'Display name', + 'value' => 'Default display name', 'scope' => IAccountManager::SCOPE_FEDERATED, - 'verified' => IAccountManager::NOT_VERIFIED, + 'verified' => IAccountManager::VERIFIED, ], IAccountManager::PROPERTY_ADDRESS => [ - 'value' => '', + 'value' => 'Default address', 'scope' => IAccountManager::SCOPE_LOCAL, - 'verified' => IAccountManager::NOT_VERIFIED, + 'verified' => IAccountManager::VERIFIED, ], IAccountManager::PROPERTY_WEBSITE => [ - 'value' => '', + 'value' => 'Default website', 'scope' => IAccountManager::SCOPE_LOCAL, - 'verified' => IAccountManager::NOT_VERIFIED, + 'verified' => IAccountManager::VERIFIED, ], IAccountManager::PROPERTY_EMAIL => [ - 'value' => '', + 'value' => 'Default email', 'scope' => IAccountManager::SCOPE_FEDERATED, - 'verified' => IAccountManager::NOT_VERIFIED, + 'verified' => IAccountManager::VERIFIED, ], IAccountManager::PROPERTY_AVATAR => [ @@ -212,15 +212,15 @@ protected function getDefaultAccountManagerUserData() { ], IAccountManager::PROPERTY_PHONE => [ - 'value' => '', + 'value' => 'Default phone', 'scope' => IAccountManager::SCOPE_LOCAL, - 'verified' => IAccountManager::NOT_VERIFIED, + 'verified' => IAccountManager::VERIFIED, ], IAccountManager::PROPERTY_TWITTER => [ - 'value' => '', + 'value' => 'Default twitter', 'scope' => IAccountManager::SCOPE_LOCAL, - 'verified' => IAccountManager::NOT_VERIFIED, + 'verified' => IAccountManager::VERIFIED, ], ]; } From c5e3d2e5c27067112e45d3da6dcad8c53d5e8cd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jan 2021 21:21:54 +0100 Subject: [PATCH 04/10] Add more unit tests for setting user settings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .../tests/Controller/UsersControllerTest.php | 160 ++++++++++++++++++ 1 file changed, 160 insertions(+) diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index 5fef8b8ccc2ff..81592c28603bc 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -287,6 +287,166 @@ public function dataTestSetUserSettings() { ]; } + public function testSetUserSettingsWhenUserDisplayNameChangeNotAllowed() { + $controller = $this->getController(false, ['saveUserSettings']); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('johndoe'); + + $this->userSession->method('getUser')->willReturn($user); + + $defaultProperties = $this->getDefaultAccountManagerUserData(); + + $this->accountManager->expects($this->once()) + ->method('getUser') + ->with($user) + ->willReturn($defaultProperties); + + $this->config->expects($this->once()) + ->method('getSystemValue') + ->with('allow_user_to_change_display_name') + ->willReturn(false); + + $this->appManager->expects($this->any()) + ->method('isEnabledForUser') + ->with('federatedfilesharing') + ->willReturn(true); + + $avatarScope = IAccountManager::SCOPE_PUBLISHED; + $displayName = 'Display name'; + $displayNameScope = IAccountManager::SCOPE_PUBLISHED; + $phone = '47658468'; + $phoneScope = IAccountManager::SCOPE_PUBLISHED; + $email = 'john@example.com'; + $emailScope = IAccountManager::SCOPE_PUBLISHED; + $website = 'nextcloud.com'; + $websiteScope = IAccountManager::SCOPE_PUBLISHED; + $address = 'street and city'; + $addressScope = IAccountManager::SCOPE_PUBLISHED; + $twitter = '@nextclouders'; + $twitterScope = IAccountManager::SCOPE_PUBLISHED; + + // Display name and email are not changed. + $expectedProperties = $defaultProperties; + $expectedProperties[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; + $expectedProperties[IAccountManager::PROPERTY_PHONE]['value'] = $phone; + $expectedProperties[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope; + unset($expectedProperties[IAccountManager::PROPERTY_PHONE]['verified']); + $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['value'] = $website; + $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope; + unset($expectedProperties[IAccountManager::PROPERTY_WEBSITE]['verified']); + $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['value'] = $address; + $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope; + unset($expectedProperties[IAccountManager::PROPERTY_ADDRESS]['verified']); + $expectedProperties[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter; + $expectedProperties[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope; + unset($expectedProperties[IAccountManager::PROPERTY_TWITTER]['verified']); + + $this->mailer->expects($this->once())->method('validateMailAddress') + ->willReturn(true); + + $controller->expects($this->once()) + ->method('saveUserSettings') + ->with($user, $expectedProperties) + ->willReturnArgument(1); + + $result = $controller->setUserSettings( + $avatarScope, + $displayName, + $displayNameScope, + $phone, + $phoneScope, + $email, + $emailScope, + $website, + $websiteScope, + $address, + $addressScope, + $twitter, + $twitterScope + ); + } + + public function testSetUserSettingsWhenFederatedFilesharingNotEnabled() { + $controller = $this->getController(false, ['saveUserSettings']); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('johndoe'); + + $this->userSession->method('getUser')->willReturn($user); + + $defaultProperties = $this->getDefaultAccountManagerUserData(); + + $this->accountManager->expects($this->once()) + ->method('getUser') + ->with($user) + ->willReturn($defaultProperties); + + $this->appManager->expects($this->any()) + ->method('isEnabledForUser') + ->with('federatedfilesharing') + ->willReturn(false); + + $avatarScope = IAccountManager::SCOPE_PUBLISHED; + $displayName = 'Display name'; + $displayNameScope = IAccountManager::SCOPE_PUBLISHED; + $phone = '47658468'; + $phoneScope = IAccountManager::SCOPE_PUBLISHED; + $email = 'john@example.com'; + $emailScope = IAccountManager::SCOPE_PUBLISHED; + $website = 'nextcloud.com'; + $websiteScope = IAccountManager::SCOPE_PUBLISHED; + $address = 'street and city'; + $addressScope = IAccountManager::SCOPE_PUBLISHED; + $twitter = '@nextclouders'; + $twitterScope = IAccountManager::SCOPE_PUBLISHED; + + // All settings are changed (in the past phone, website, address and + // twitter were not changed). + $expectedProperties = $defaultProperties; + $expectedProperties[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; + $expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $displayName; + $expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displayNameScope; + unset($expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['verified']); + $expectedProperties[IAccountManager::PROPERTY_EMAIL]['value'] = $email; + $expectedProperties[IAccountManager::PROPERTY_EMAIL]['scope'] = $emailScope; + unset($expectedProperties[IAccountManager::PROPERTY_EMAIL]['verified']); + $expectedProperties[IAccountManager::PROPERTY_PHONE]['value'] = $phone; + $expectedProperties[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope; + unset($expectedProperties[IAccountManager::PROPERTY_PHONE]['verified']); + $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['value'] = $website; + $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope; + unset($expectedProperties[IAccountManager::PROPERTY_WEBSITE]['verified']); + $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['value'] = $address; + $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope; + unset($expectedProperties[IAccountManager::PROPERTY_ADDRESS]['verified']); + $expectedProperties[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter; + $expectedProperties[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope; + unset($expectedProperties[IAccountManager::PROPERTY_TWITTER]['verified']); + + $this->mailer->expects($this->once())->method('validateMailAddress') + ->willReturn(true); + + $controller->expects($this->once()) + ->method('saveUserSettings') + ->with($user, $expectedProperties) + ->willReturnArgument(1); + + $result = $controller->setUserSettings( + $avatarScope, + $displayName, + $displayNameScope, + $phone, + $phoneScope, + $email, + $emailScope, + $website, + $websiteScope, + $address, + $addressScope, + $twitter, + $twitterScope + ); + } + /** * @dataProvider dataTestSaveUserSettings * From a7431817b90a8343744ebd31941d49676d108313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Fri, 29 Jan 2021 22:11:16 +0100 Subject: [PATCH 05/10] Respect additional user settings not covered by the controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "AccountManager::updateUser()" wipes previous user data with whichever user data is given (except for some adjustments, like resetting the verified status when needed). As the controller overrode the properties those properties would lose some of their attributes even if they are not affected by the changes made by the controller. Now the controller only modifies the attributes set ("value" and "scope") to prevent that. Note that with this change the controller no longer removes the "verified" status, but this is not a problem because, as mentioned, "AccountManager::updateUser()" resets them when needed (for example, when the value of the website property changes). This change is a previous step to fix overwritting properties with null values, and it will prevent the controller from making unexpected changes if more attributes are added in the future. Signed-off-by: Daniel Calviño Sánchez --- .../lib/Controller/UsersController.php | 20 ++++++++++++------- .../tests/Controller/UsersControllerTest.php | 10 ---------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index d2f168387172c..2ea3ada4f08d4 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -395,15 +395,21 @@ public function setUserSettings(?string $avatarScope = null, $data = $this->accountManager->getUser($user); $beforeData = $data; - $data[IAccountManager::PROPERTY_AVATAR] = ['scope' => $avatarScope]; + $data[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { - $data[IAccountManager::PROPERTY_DISPLAYNAME] = ['value' => $displayname, 'scope' => $displaynameScope]; - $data[IAccountManager::PROPERTY_EMAIL] = ['value' => $email, 'scope' => $emailScope]; + $data[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $displayname; + $data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displaynameScope; + $data[IAccountManager::PROPERTY_EMAIL]['value'] = $email; + $data[IAccountManager::PROPERTY_EMAIL]['scope'] = $emailScope; } - $data[IAccountManager::PROPERTY_WEBSITE] = ['value' => $website, 'scope' => $websiteScope]; - $data[IAccountManager::PROPERTY_ADDRESS] = ['value' => $address, 'scope' => $addressScope]; - $data[IAccountManager::PROPERTY_PHONE] = ['value' => $phone, 'scope' => $phoneScope]; - $data[IAccountManager::PROPERTY_TWITTER] = ['value' => $twitter, 'scope' => $twitterScope]; + $data[IAccountManager::PROPERTY_WEBSITE]['value'] = $website; + $data[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope; + $data[IAccountManager::PROPERTY_ADDRESS]['value'] = $address; + $data[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope; + $data[IAccountManager::PROPERTY_PHONE]['value'] = $phone; + $data[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope; + $data[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter; + $data[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope; try { $data = $this->saveUserSettings($user, $data); diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index 81592c28603bc..48c6fba18946e 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -330,16 +330,12 @@ public function testSetUserSettingsWhenUserDisplayNameChangeNotAllowed() { $expectedProperties[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; $expectedProperties[IAccountManager::PROPERTY_PHONE]['value'] = $phone; $expectedProperties[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope; - unset($expectedProperties[IAccountManager::PROPERTY_PHONE]['verified']); $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['value'] = $website; $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope; - unset($expectedProperties[IAccountManager::PROPERTY_WEBSITE]['verified']); $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['value'] = $address; $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope; - unset($expectedProperties[IAccountManager::PROPERTY_ADDRESS]['verified']); $expectedProperties[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter; $expectedProperties[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope; - unset($expectedProperties[IAccountManager::PROPERTY_TWITTER]['verified']); $this->mailer->expects($this->once())->method('validateMailAddress') ->willReturn(true); @@ -405,22 +401,16 @@ public function testSetUserSettingsWhenFederatedFilesharingNotEnabled() { $expectedProperties[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; $expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $displayName; $expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displayNameScope; - unset($expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['verified']); $expectedProperties[IAccountManager::PROPERTY_EMAIL]['value'] = $email; $expectedProperties[IAccountManager::PROPERTY_EMAIL]['scope'] = $emailScope; - unset($expectedProperties[IAccountManager::PROPERTY_EMAIL]['verified']); $expectedProperties[IAccountManager::PROPERTY_PHONE]['value'] = $phone; $expectedProperties[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope; - unset($expectedProperties[IAccountManager::PROPERTY_PHONE]['verified']); $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['value'] = $website; $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope; - unset($expectedProperties[IAccountManager::PROPERTY_WEBSITE]['verified']); $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['value'] = $address; $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope; - unset($expectedProperties[IAccountManager::PROPERTY_ADDRESS]['verified']); $expectedProperties[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter; $expectedProperties[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope; - unset($expectedProperties[IAccountManager::PROPERTY_TWITTER]['verified']); $this->mailer->expects($this->once())->method('validateMailAddress') ->willReturn(true); From ac0c18327eacd0dc4a26d7478d09cbb8da6b12b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 31 Jan 2021 14:08:24 +0100 Subject: [PATCH 06/10] Fix TypeError when "email" is not given in the controller request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- apps/settings/lib/Controller/UsersController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 2ea3ada4f08d4..451dabde4d18f 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -380,7 +380,7 @@ public function setUserSettings(?string $avatarScope = null, ); } - $email = strtolower($email); + $email = !is_null($email) ? strtolower($email) : $email; if (!empty($email) && !$this->mailer->validateMailAddress($email)) { return new DataResponse( [ From 67dd087ce46794580a29fdfcf4776665349f3a3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 31 Jan 2021 22:53:48 +0100 Subject: [PATCH 07/10] Fix deleting properties of user settings when not given explicitly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The controller can receive an optional subset of the properties of the user settings; values not given are set to "null" by default. However, those null values overwrote the previously existing values, so in practice any value not given was deleted from the user settings. Now only non null values overwrite the previous values. Signed-off-by: Daniel Calviño Sánchez --- .../lib/Controller/UsersController.php | 52 ++++++-- .../tests/Controller/UsersControllerTest.php | 120 ++++++++++++++++++ 2 files changed, 159 insertions(+), 13 deletions(-) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index 451dabde4d18f..a568b350883a4 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -395,21 +395,47 @@ public function setUserSettings(?string $avatarScope = null, $data = $this->accountManager->getUser($user); $beforeData = $data; - $data[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; + if (!is_null($avatarScope)) { + $data[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; + } if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { - $data[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $displayname; - $data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displaynameScope; - $data[IAccountManager::PROPERTY_EMAIL]['value'] = $email; - $data[IAccountManager::PROPERTY_EMAIL]['scope'] = $emailScope; + if (!is_null($displayname)) { + $data[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $displayname; + } + if (!is_null($displaynameScope)) { + $data[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $displaynameScope; + } + if (!is_null($email)) { + $data[IAccountManager::PROPERTY_EMAIL]['value'] = $email; + } + if (!is_null($emailScope)) { + $data[IAccountManager::PROPERTY_EMAIL]['scope'] = $emailScope; + } + } + if (!is_null($website)) { + $data[IAccountManager::PROPERTY_WEBSITE]['value'] = $website; + } + if (!is_null($websiteScope)) { + $data[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope; + } + if (!is_null($address)) { + $data[IAccountManager::PROPERTY_ADDRESS]['value'] = $address; + } + if (!is_null($addressScope)) { + $data[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope; + } + if (!is_null($phone)) { + $data[IAccountManager::PROPERTY_PHONE]['value'] = $phone; + } + if (!is_null($phoneScope)) { + $data[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope; + } + if (!is_null($twitter)) { + $data[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter; + } + if (!is_null($twitterScope)) { + $data[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope; } - $data[IAccountManager::PROPERTY_WEBSITE]['value'] = $website; - $data[IAccountManager::PROPERTY_WEBSITE]['scope'] = $websiteScope; - $data[IAccountManager::PROPERTY_ADDRESS]['value'] = $address; - $data[IAccountManager::PROPERTY_ADDRESS]['scope'] = $addressScope; - $data[IAccountManager::PROPERTY_PHONE]['value'] = $phone; - $data[IAccountManager::PROPERTY_PHONE]['scope'] = $phoneScope; - $data[IAccountManager::PROPERTY_TWITTER]['value'] = $twitter; - $data[IAccountManager::PROPERTY_TWITTER]['scope'] = $twitterScope; try { $data = $this->saveUserSettings($user, $data); diff --git a/apps/settings/tests/Controller/UsersControllerTest.php b/apps/settings/tests/Controller/UsersControllerTest.php index 48c6fba18946e..c0dadd4c47002 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -437,6 +437,126 @@ public function testSetUserSettingsWhenFederatedFilesharingNotEnabled() { ); } + /** + * @dataProvider dataTestSetUserSettingsSubset + * + * @param string $property + * @param string $propertyValue + */ + public function testSetUserSettingsSubset($property, $propertyValue) { + $controller = $this->getController(false, ['saveUserSettings']); + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('johndoe'); + + $this->userSession->method('getUser')->willReturn($user); + + $defaultProperties = $this->getDefaultAccountManagerUserData(); + + $this->accountManager->expects($this->once()) + ->method('getUser') + ->with($user) + ->willReturn($defaultProperties); + + $avatarScope = ($property === 'avatarScope') ? $propertyValue : null; + $displayName = ($property === 'displayName') ? $propertyValue : null; + $displayNameScope = ($property === 'displayNameScope') ? $propertyValue : null; + $phone = ($property === 'phone') ? $propertyValue : null; + $phoneScope = ($property === 'phoneScope') ? $propertyValue : null; + $email = ($property === 'email') ? $propertyValue : null; + $emailScope = ($property === 'emailScope') ? $propertyValue : null; + $website = ($property === 'website') ? $propertyValue : null; + $websiteScope = ($property === 'websiteScope') ? $propertyValue : null; + $address = ($property === 'address') ? $propertyValue : null; + $addressScope = ($property === 'addressScope') ? $propertyValue : null; + $twitter = ($property === 'twitter') ? $propertyValue : null; + $twitterScope = ($property === 'twitterScope') ? $propertyValue : null; + + $expectedProperties = $defaultProperties; + if ($property === 'avatarScope') { + $expectedProperties[IAccountManager::PROPERTY_AVATAR]['scope'] = $propertyValue; + } + if ($property === 'displayName') { + $expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['value'] = $propertyValue; + } + if ($property === 'displayNameScope') { + $expectedProperties[IAccountManager::PROPERTY_DISPLAYNAME]['scope'] = $propertyValue; + } + if ($property === 'phone') { + $expectedProperties[IAccountManager::PROPERTY_PHONE]['value'] = $propertyValue; + } + if ($property === 'phoneScope') { + $expectedProperties[IAccountManager::PROPERTY_PHONE]['scope'] = $propertyValue; + } + if ($property === 'email') { + $expectedProperties[IAccountManager::PROPERTY_EMAIL]['value'] = $propertyValue; + } + if ($property === 'emailScope') { + $expectedProperties[IAccountManager::PROPERTY_EMAIL]['scope'] = $propertyValue; + } + if ($property === 'website') { + $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['value'] = $propertyValue; + } + if ($property === 'websiteScope') { + $expectedProperties[IAccountManager::PROPERTY_WEBSITE]['scope'] = $propertyValue; + } + if ($property === 'address') { + $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['value'] = $propertyValue; + } + if ($property === 'addressScope') { + $expectedProperties[IAccountManager::PROPERTY_ADDRESS]['scope'] = $propertyValue; + } + if ($property === 'twitter') { + $expectedProperties[IAccountManager::PROPERTY_TWITTER]['value'] = $propertyValue; + } + if ($property === 'twitterScope') { + $expectedProperties[IAccountManager::PROPERTY_TWITTER]['scope'] = $propertyValue; + } + + if (!empty($email)) { + $this->mailer->expects($this->once())->method('validateMailAddress') + ->willReturn(true); + } + + $controller->expects($this->once()) + ->method('saveUserSettings') + ->with($user, $expectedProperties) + ->willReturnArgument(1); + + $result = $controller->setUserSettings( + $avatarScope, + $displayName, + $displayNameScope, + $phone, + $phoneScope, + $email, + $emailScope, + $website, + $websiteScope, + $address, + $addressScope, + $twitter, + $twitterScope + ); + } + + public function dataTestSetUserSettingsSubset() { + return [ + ['avatarScope', IAccountManager::SCOPE_PUBLISHED], + ['displayName', 'Display name'], + ['displayNameScope', IAccountManager::SCOPE_PUBLISHED], + ['phone', '47658468'], + ['phoneScope', IAccountManager::SCOPE_PUBLISHED], + ['email', 'john@example.com'], + ['emailScope', IAccountManager::SCOPE_PUBLISHED], + ['website', 'nextcloud.com'], + ['websiteScope', IAccountManager::SCOPE_PUBLISHED], + ['address', 'street and city'], + ['addressScope', IAccountManager::SCOPE_PUBLISHED], + ['twitter', '@nextclouders'], + ['twitterScope', IAccountManager::SCOPE_PUBLISHED], + ]; + } + /** * @dataProvider dataTestSaveUserSettings * From e2c4a174f2f19761a30125bb35b348258d4c118f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 31 Jan 2021 23:10:55 +0100 Subject: [PATCH 08/10] Add integration tests for searching users in contacts menu MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel Calviño Sánchez --- .drone.yml | 25 +++ .../features/bootstrap/ContactsMenu.php | 69 +++++++ .../features/bootstrap/FeatureContext.php | 1 + .../features/contacts-menu.feature | 188 ++++++++++++++++++ 4 files changed, 283 insertions(+) create mode 100644 build/integration/features/bootstrap/ContactsMenu.php create mode 100644 build/integration/features/contacts-menu.feature diff --git a/.drone.yml b/.drone.yml index d2059b766b5d2..4a2df572dac7c 100644 --- a/.drone.yml +++ b/.drone.yml @@ -1164,6 +1164,31 @@ trigger: - pull_request - push +--- +kind: pipeline +name: integration-contacts-menu + +steps: +- name: submodules + image: docker:git + commands: + - git submodule update --init +- name: integration-contacts-menu + image: nextcloudci/integration-php7.3:integration-php7.3-2 + commands: + - bash tests/drone-run-integration-tests.sh || exit 0 + - ./occ maintenance:install --admin-pass=admin --data-dir=/dev/shm/nc_int + - cd build/integration + - ./run.sh features/contacts-menu.feature + +trigger: + branch: + - master + - stable* + event: + - pull_request + - push + --- kind: pipeline name: integration-favorites diff --git a/build/integration/features/bootstrap/ContactsMenu.php b/build/integration/features/bootstrap/ContactsMenu.php new file mode 100644 index 0000000000000..30e1dad625982 --- /dev/null +++ b/build/integration/features/bootstrap/ContactsMenu.php @@ -0,0 +1,69 @@ + + * + * @author Daniel Calviño Sánchez + * + * @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 . + * + */ + +use PHPUnit\Framework\Assert; + +trait ContactsMenu { + + // BasicStructure trait is expected to be used in the class that uses this + // trait. + + /** + * @When /^searching for contacts matching with "([^"]*)"$/ + * + * @param string $filter + */ + public function searchingForContactsMatchingWith(string $filter) { + $url = '/index.php/contactsmenu/contacts'; + + $parameters[] = 'filter=' . $filter; + + $url .= '?' . implode('&', $parameters); + + $this->sendingAToWithRequesttoken('POST', $url); + } + + /** + * @Then /^the list of searched contacts has "(\d+)" contacts$/ + */ + public function theListOfSearchedContactsHasContacts(int $count) { + $this->theHTTPStatusCodeShouldBe(200); + + $searchedContacts = json_decode($this->response->getBody(), $asAssociativeArray = true)['contacts']; + + Assert::assertEquals($count, count($searchedContacts)); + } + + /** + * @Then /^searched contact "(\d+)" is named "([^"]*)"$/ + * + * @param int $index + * @param string $expectedName + */ + public function searchedContactXIsNamed(int $index, string $expectedName) { + $searchedContacts = json_decode($this->response->getBody(), $asAssociativeArray = true)['contacts']; + $searchedContact = $searchedContacts[$index]; + + Assert::assertEquals($expectedName, $searchedContact['fullName']); + } +} diff --git a/build/integration/features/bootstrap/FeatureContext.php b/build/integration/features/bootstrap/FeatureContext.php index e9c486daa4dcf..9437b50cd1c7e 100644 --- a/build/integration/features/bootstrap/FeatureContext.php +++ b/build/integration/features/bootstrap/FeatureContext.php @@ -33,6 +33,7 @@ * Features context. */ class FeatureContext implements Context, SnippetAcceptingContext { + use ContactsMenu; use Search; use WebDav; use Trashbin; diff --git a/build/integration/features/contacts-menu.feature b/build/integration/features/contacts-menu.feature new file mode 100644 index 0000000000000..845d4d35925f5 --- /dev/null +++ b/build/integration/features/contacts-menu.feature @@ -0,0 +1,188 @@ +Feature: contacts-menu + + Scenario: users can be searched by display name + Given user "user0" exists + And user "user1" exists + And As an "admin" + And sending "PUT" to "/cloud/users/user1" with + | key | displayname | + | value | Test name | + When Logging in using web as "user0" + And searching for contacts matching with "test" + Then the list of searched contacts has "1" contacts + And searched contact "0" is named "Test name" + + Scenario: users can be searched by email + Given user "user0" exists + And user "user1" exists + And As an "admin" + And sending "PUT" to "/cloud/users/user1" with + | key | email | + | value | test@example.com | + When Logging in using web as "user0" + And searching for contacts matching with "test" + Then the list of searched contacts has "1" contacts + And searched contact "0" is named "user1" + + Scenario: users can not be searched by id + Given user "user0" exists + And user "user1" exists + And As an "admin" + And sending "PUT" to "/cloud/users/user1" with + | key | displayname | + | value | Test name | + When Logging in using web as "user0" + And searching for contacts matching with "user" + Then the list of searched contacts has "0" contacts + + Scenario: search several users + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And user "user3" exists + And user "user4" exists + And user "user5" exists + And As an "admin" + And sending "PUT" to "/cloud/users/user1" with + | key | displayname | + | value | Test name | + And sending "PUT" to "/cloud/users/user2" with + | key | email | + | value | test@example.com | + And sending "PUT" to "/cloud/users/user3" with + | key | displayname | + | value | Unmatched name | + And sending "PUT" to "/cloud/users/user4" with + | key | email | + | value | unmatched@example.com | + And sending "PUT" to "/cloud/users/user5" with + | key | displayname | + | value | Another test name | + And sending "PUT" to "/cloud/users/user5" with + | key | email | + | value | another_test@example.com | + When Logging in using web as "user0" + And searching for contacts matching with "test" + Then the list of searched contacts has "3" contacts + # Results are sorted alphabetically + And searched contact "0" is named "Another test name" + And searched contact "1" is named "Test name" + And searched contact "2" is named "user2" + + + + Scenario: users can not be found by display name if visibility is private + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And Logging in using web as "user1" + And Sending a "PUT" to "/settings/users/user1/settings" with requesttoken + | displayname | Test name | + | displaynameScope | private | + And Logging in using web as "user2" + And Sending a "PUT" to "/settings/users/user2/settings" with requesttoken + | displayname | Another test name | + | displaynameScope | contacts | + When Logging in using web as "user0" + And searching for contacts matching with "test" + Then the list of searched contacts has "1" contacts + And searched contact "0" is named "Another test name" + + Scenario: users can not be found by email if visibility is private + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And Logging in using web as "user1" + And Sending a "PUT" to "/settings/users/user1/settings" with requesttoken + | email | test@example.com | + | emailScope | private | + And Logging in using web as "user2" + And Sending a "PUT" to "/settings/users/user2/settings" with requesttoken + | email | another_test@example.com | + | emailScope | contacts | + When Logging in using web as "user0" + And searching for contacts matching with "test" + Then the list of searched contacts has "1" contacts + And searched contact "0" is named "user2" + + Scenario: users can be found by other properties if the visibility of one is private + Given user "user0" exists + And user "user1" exists + And user "user2" exists + And Logging in using web as "user1" + And Sending a "PUT" to "/settings/users/user1/settings" with requesttoken + | displayname | Test name | + | displaynameScope | contacts | + | email | test@example.com | + | emailScope | private | + And Logging in using web as "user2" + And Sending a "PUT" to "/settings/users/user2/settings" with requesttoken + | displayname | Another test name | + | displaynameScope | private | + | email | another_test@example.com | + | emailScope | contacts | + When Logging in using web as "user0" + And searching for contacts matching with "test" + Then the list of searched contacts has "2" contacts + And searched contact "0" is named "" + And searched contact "1" is named "Test name" + + + + Scenario: users can be searched by display name if visibility is increased again + Given user "user0" exists + And user "user1" exists + And Logging in using web as "user1" + And Sending a "PUT" to "/settings/users/user1/settings" with requesttoken + | displayname | Test name | + | displaynameScope | private | + And Sending a "PUT" to "/settings/users/user1/settings" with requesttoken + | displaynameScope | contacts | + When Logging in using web as "user0" + And searching for contacts matching with "test" + Then the list of searched contacts has "1" contacts + And searched contact "0" is named "Test name" + + Scenario: users can be searched by email if visibility is increased again + Given user "user0" exists + And user "user1" exists + And Logging in using web as "user1" + And Sending a "PUT" to "/settings/users/user1/settings" with requesttoken + | email | test@example.com | + | emailScope | private | + And Sending a "PUT" to "/settings/users/user1/settings" with requesttoken + | emailScope | contacts | + When Logging in using web as "user0" + And searching for contacts matching with "test" + Then the list of searched contacts has "1" contacts + And searched contact "0" is named "user1" + + + + Scenario: users can not be searched by display name if visibility is private even if updated with provisioning + Given user "user0" exists + And user "user1" exists + And Logging in using web as "user1" + And Sending a "PUT" to "/settings/users/user1/settings" with requesttoken + | displaynameScope | private | + And As an "admin" + And sending "PUT" to "/cloud/users/user1" with + | key | displayname | + | value | Test name | + When Logging in using web as "user0" + And searching for contacts matching with "test" + Then the list of searched contacts has "0" contacts + + Scenario: users can not be searched by email if visibility is private even if updated with provisioning + Given user "user0" exists + And user "user1" exists + And Logging in using web as "user1" + And Sending a "PUT" to "/settings/users/user1/settings" with requesttoken + | emailScope | private | + And As an "admin" + And sending "PUT" to "/cloud/users/user1" with + | key | email | + | value | test@example.com | + When Logging in using web as "user0" + And searching for contacts matching with "test" + Then the list of searched contacts has "0" contacts From 7159a708947e29f963b634aa3af6ab5354f6bed4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Sun, 31 Jan 2021 23:53:45 +0100 Subject: [PATCH 09/10] Guard against null phone number value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "parsePhoneNumber()" expects a string, so a TypeError would be thrown if the phone number value is null. Signed-off-by: Daniel Calviño Sánchez --- lib/private/Accounts/AccountManager.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index ea8f99e021652..d5df6557c8f13 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -134,6 +134,9 @@ public function updateUser(IUser $user, array $data, bool $throwOnData = false): $updated = true; if (isset($data[self::PROPERTY_PHONE]) && $data[self::PROPERTY_PHONE]['value'] !== '') { + // Sanitize null value. + $data[self::PROPERTY_PHONE]['value'] = $data[self::PROPERTY_PHONE]['value'] ?? ''; + try { $data[self::PROPERTY_PHONE]['value'] = $this->parsePhoneNumber($data[self::PROPERTY_PHONE]['value']); } catch (\InvalidArgumentException $e) { From da84ed7d4c2d05b4f9841d345d3a47dfe673ea7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Calvi=C3=B1o=20S=C3=A1nchez?= Date: Mon, 1 Feb 2021 03:34:36 +0100 Subject: [PATCH 10/10] Fix active scope not visible in the menu if excluded MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Depending on some settings (for example, if lookup server upload is disabled) some items can be hidden in the scope menu. However, if the user selected an scope in the past once the settings were changed the scope was no longer visible in the menu. Now the active scope will be always visible in the menu, although if it is an excluded scope it will be disabled. Selecting any other scope will then hide the excluded and no longer active one. When upload to the lookup server is disabled the scope menu was hidden for display name and email in the personal information settings; now the menu will be always shown to enable the above described behaviour. Note that the menu will be shown even if there is a single available scope so the user can read its description. Signed-off-by: Daniel Calviño Sánchez --- apps/settings/css/settings.scss | 10 +++ apps/settings/js/federationscopemenu.js | 16 ++++- apps/settings/js/templates.js | 72 ++++++++++++++----- .../templates/federationscopemenu.handlebars | 10 +++ .../settings/personal/personal.info.php | 6 +- 5 files changed, 90 insertions(+), 24 deletions(-) diff --git a/apps/settings/css/settings.scss b/apps/settings/css/settings.scss index 88c5e4dbcf990..53a9a28c0800b 100644 --- a/apps/settings/css/settings.scss +++ b/apps/settings/css/settings.scss @@ -425,6 +425,16 @@ select { font-weight: bold; } } + + &.disabled { + opacity: .5; + + cursor: default; + + * { + cursor: default; + } + } } } } diff --git a/apps/settings/js/federationscopemenu.js b/apps/settings/js/federationscopemenu.js index d19c9d7d0bf7b..72fd8bc7284e7 100644 --- a/apps/settings/js/federationscopemenu.js +++ b/apps/settings/js/federationscopemenu.js @@ -23,6 +23,7 @@ className: 'federationScopeMenu popovermenu bubble menu menu-center', field: undefined, _scopes: undefined, + _excludedScopes: [], initialize: function(options) { this.field = options.field; @@ -58,9 +59,7 @@ ]; if (options.excludedScopes && options.excludedScopes.length) { - this._scopes = this._scopes.filter(function(scopeEntry) { - return options.excludedScopes.indexOf(scopeEntry.name) === -1; - }) + this._excludedScopes = options.excludedScopes } }, @@ -122,6 +121,17 @@ } else { this._scopes[i].active = false; } + + var isExcludedScope = this._excludedScopes.includes(this._scopes[i].name) + if (isExcludedScope && !this._scopes[i].active) { + this._scopes[i].hidden = true + } else if (isExcludedScope && this._scopes[i].active) { + this._scopes[i].hidden = false + this._scopes[i].disabled = true + } else { + this._scopes[i].hidden = false + this._scopes[i].disabled = false + } } this.render(); diff --git a/apps/settings/js/templates.js b/apps/settings/js/templates.js index d0d623d9ed939..7988a8df6a926 100644 --- a/apps/settings/js/templates.js +++ b/apps/settings/js/templates.js @@ -1,6 +1,15 @@ (function() { var template = Handlebars.template, templates = OC.Settings.Templates = OC.Settings.Templates || {}; templates['federationscopemenu'] = template({"1":function(container,depth0,helpers,partials,data) { + var stack1, lookupProperty = container.lookupProperty || function(parent, propertyName) { + if (Object.prototype.hasOwnProperty.call(parent, propertyName)) { + return parent[propertyName]; + } + return undefined + }; + + return ((stack1 = lookupProperty(helpers,"unless").call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? lookupProperty(depth0,"hidden") : depth0),{"name":"unless","hash":{},"fn":container.program(2, data, 0),"inverse":container.noop,"data":data,"loc":{"start":{"line":3,"column":2},"end":{"line":25,"column":13}}})) != null ? stack1 : ""); +},"2":function(container,depth0,helpers,partials,data) { var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=container.hooks.helperMissing, alias3="function", alias4=container.escapeExpression, lookupProperty = container.lookupProperty || function(parent, propertyName) { if (Object.prototype.hasOwnProperty.call(parent, propertyName)) { return parent[propertyName]; @@ -8,22 +17,49 @@ templates['federationscopemenu'] = template({"1":function(container,depth0,helpe return undefined }; - return "
  • \n \n" - + ((stack1 = lookupProperty(helpers,"if").call(alias1,(depth0 != null ? lookupProperty(depth0,"iconClass") : depth0),{"name":"if","hash":{},"fn":container.program(4, data, 0),"inverse":container.program(6, data, 0),"data":data,"loc":{"start":{"line":5,"column":4},"end":{"line":9,"column":11}}})) != null ? stack1 : "") + return "
  • \n" + + ((stack1 = lookupProperty(helpers,"if").call(alias1,(depth0 != null ? lookupProperty(depth0,"disabled") : depth0),{"name":"if","hash":{},"fn":container.program(3, data, 0),"inverse":container.program(6, data, 0),"data":data,"loc":{"start":{"line":5,"column":3},"end":{"line":9,"column":10}}})) != null ? stack1 : "") + + ((stack1 = lookupProperty(helpers,"if").call(alias1,(depth0 != null ? lookupProperty(depth0,"iconClass") : depth0),{"name":"if","hash":{},"fn":container.program(8, data, 0),"inverse":container.program(10, data, 0),"data":data,"loc":{"start":{"line":10,"column":4},"end":{"line":14,"column":11}}})) != null ? stack1 : "") + "

    \n " - + alias4(((helper = (helper = lookupProperty(helpers,"displayName") || (depth0 != null ? lookupProperty(depth0,"displayName") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"displayName","hash":{},"data":data,"loc":{"start":{"line":11,"column":35},"end":{"line":11,"column":50}}}) : helper))) + + alias4(((helper = (helper = lookupProperty(helpers,"displayName") || (depth0 != null ? lookupProperty(depth0,"displayName") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"displayName","hash":{},"data":data,"loc":{"start":{"line":16,"column":35},"end":{"line":16,"column":50}}}) : helper))) + "
    \n " - + alias4(((helper = (helper = lookupProperty(helpers,"tooltip") || (depth0 != null ? lookupProperty(depth0,"tooltip") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"tooltip","hash":{},"data":data,"loc":{"start":{"line":12,"column":40},"end":{"line":12,"column":51}}}) : helper))) - + "\n

    \n     \n
    • \n"; -},"2":function(container,depth0,helpers,partials,data) { - return "active"; + + alias4(((helper = (helper = lookupProperty(helpers,"tooltip") || (depth0 != null ? lookupProperty(depth0,"tooltip") : depth0)) != null ? helper : alias2),(typeof helper === alias3 ? helper.call(alias1,{"name":"tooltip","hash":{},"data":data,"loc":{"start":{"line":17,"column":40},"end":{"line":17,"column":51}}}) : helper))) + + "\n

    \n" + + ((stack1 = lookupProperty(helpers,"if").call(alias1,(depth0 != null ? lookupProperty(depth0,"disabled") : depth0),{"name":"if","hash":{},"fn":container.program(12, data, 0),"inverse":container.program(14, data, 0),"data":data,"loc":{"start":{"line":19,"column":3},"end":{"line":23,"column":10}}})) != null ? stack1 : "") + + " \n"; +},"3":function(container,depth0,helpers,partials,data) { + var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=container.hooks.helperMissing, alias3="function", alias4=container.escapeExpression, lookupProperty = container.lookupProperty || function(parent, propertyName) { + if (Object.prototype.hasOwnProperty.call(parent, propertyName)) { + return parent[propertyName]; + } + return undefined + }; + + return "
    \n"; },"4":function(container,depth0,helpers,partials,data) { + return "active"; +},"6":function(container,depth0,helpers,partials,data) { + var stack1, helper, alias1=depth0 != null ? depth0 : (container.nullContext || {}), alias2=container.hooks.helperMissing, alias3="function", alias4=container.escapeExpression, lookupProperty = container.lookupProperty || function(parent, propertyName) { + if (Object.prototype.hasOwnProperty.call(parent, propertyName)) { + return parent[propertyName]; + } + return undefined + }; + + return " \n"; +},"8":function(container,depth0,helpers,partials,data) { var helper, lookupProperty = container.lookupProperty || function(parent, propertyName) { if (Object.prototype.hasOwnProperty.call(parent, propertyName)) { return parent[propertyName]; @@ -32,10 +68,14 @@ templates['federationscopemenu'] = template({"1":function(container,depth0,helpe }; return " \n"; -},"6":function(container,depth0,helpers,partials,data) { +},"10":function(container,depth0,helpers,partials,data) { return " \n"; +},"12":function(container,depth0,helpers,partials,data) { + return "
    \n"; +},"14":function(container,depth0,helpers,partials,data) { + return " \n"; },"compiler":[8,">= 4.3.0"],"main":function(container,depth0,helpers,partials,data) { var stack1, lookupProperty = container.lookupProperty || function(parent, propertyName) { if (Object.prototype.hasOwnProperty.call(parent, propertyName)) { @@ -45,7 +85,7 @@ templates['federationscopemenu'] = template({"1":function(container,depth0,helpe }; return "
      \n" - + ((stack1 = lookupProperty(helpers,"each").call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? lookupProperty(depth0,"items") : depth0),{"name":"each","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data,"loc":{"start":{"line":2,"column":1},"end":{"line":16,"column":10}}})) != null ? stack1 : "") + + ((stack1 = lookupProperty(helpers,"each").call(depth0 != null ? depth0 : (container.nullContext || {}),(depth0 != null ? lookupProperty(depth0,"items") : depth0),{"name":"each","hash":{},"fn":container.program(1, data, 0),"inverse":container.noop,"data":data,"loc":{"start":{"line":2,"column":1},"end":{"line":26,"column":10}}})) != null ? stack1 : "") + "
    \n"; },"useData":true}); })(); \ No newline at end of file diff --git a/apps/settings/js/templates/federationscopemenu.handlebars b/apps/settings/js/templates/federationscopemenu.handlebars index e5cfd942f4661..5a2077d4fc3bf 100644 --- a/apps/settings/js/templates/federationscopemenu.handlebars +++ b/apps/settings/js/templates/federationscopemenu.handlebars @@ -1,7 +1,12 @@ diff --git a/apps/settings/templates/settings/personal/personal.info.php b/apps/settings/templates/settings/personal/personal.info.php index 8aa7b195ff5e8..6f8516e6437f9 100644 --- a/apps/settings/templates/settings/personal/personal.info.php +++ b/apps/settings/templates/settings/personal/personal.info.php @@ -122,9 +122,7 @@ - -
    @@ -172,9 +170,7 @@ t('For password reset and notifications')); ?> - - - +