From 4fbb9b212d71d841ee30f4b538ebf693ac1defe4 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 331ea49b8f4..b48971535ea 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -526,14 +526,14 @@ class UsersController extends Controller { 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 f9652053de8..b3e8937821a 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -209,42 +209,42 @@ class UsersControllerTest extends \Test\TestCase { 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 @@ class UsersControllerTest extends \Test\TestCase { 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 2eeac35c2690e3fa778f581262e7f4068c389946 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 b3e8937821a..fd536a399e2 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -180,6 +180,51 @@ class UsersControllerTest extends \Test\TestCase { } } + 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 @@ class UsersControllerTest extends \Test\TestCase { $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 36fa740e629768179c3add32e93ef5a3bac1cd99 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 fd536a399e2..5fef8b8ccc2 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -184,27 +184,27 @@ class UsersControllerTest extends \Test\TestCase { 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 @@ class UsersControllerTest extends \Test\TestCase { ], 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 7ed78d2a31087a23541d4f2a8aef34094736abc3 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 5fef8b8ccc2..81592c28603 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -287,6 +287,166 @@ class UsersControllerTest extends \Test\TestCase { ]; } + 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 6865d518697b59dffe6b1c825f5a082fa9852974 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 b48971535ea..115a94a69df 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -395,15 +395,21 @@ class UsersController extends Controller { $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 81592c28603..48c6fba1894 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -330,16 +330,12 @@ class UsersControllerTest extends \Test\TestCase { $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 @@ class UsersControllerTest extends \Test\TestCase { $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 ae7eca8a36f930f2c332525e55344c6382aeb2fe 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 115a94a69df..f5470edd72e 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -380,7 +380,7 @@ class UsersController extends Controller { ); } - $email = strtolower($email); + $email = !is_null($email) ? strtolower($email) : $email; if (!empty($email) && !$this->mailer->validateMailAddress($email)) { return new DataResponse( [ From 44c870a4701be616047904718ffd788e3fbad5c3 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 | 54 ++++++-- .../tests/Controller/UsersControllerTest.php | 120 ++++++++++++++++++ 2 files changed, 160 insertions(+), 14 deletions(-) diff --git a/apps/settings/lib/Controller/UsersController.php b/apps/settings/lib/Controller/UsersController.php index f5470edd72e..4118b9fe327 100644 --- a/apps/settings/lib/Controller/UsersController.php +++ b/apps/settings/lib/Controller/UsersController.php @@ -395,21 +395,47 @@ class UsersController extends Controller { $data = $this->accountManager->getUser($user); $beforeData = $data; - $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($avatarScope)) { + $data[IAccountManager::PROPERTY_AVATAR]['scope'] = $avatarScope; + } + if ($this->config->getSystemValue('allow_user_to_change_display_name', true) !== false) { + 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 48c6fba1894..c0dadd4c470 100644 --- a/apps/settings/tests/Controller/UsersControllerTest.php +++ b/apps/settings/tests/Controller/UsersControllerTest.php @@ -437,6 +437,126 @@ class UsersControllerTest extends \Test\TestCase { ); } + /** + * @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 491c031c0cebd4e10db70f84a94b1446a24b9780 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 fee6ce6f620..92d44128178 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 00000000000..30e1dad6259 --- /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 e9c486daa4d..9437b50cd1c 100644 --- a/build/integration/features/bootstrap/FeatureContext.php +++ b/build/integration/features/bootstrap/FeatureContext.php @@ -33,6 +33,7 @@ require __DIR__ . '/../../vendor/autoload.php'; * 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 00000000000..845d4d35925 --- /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 3ae1ec4d2a4f10f8cc54c4fc933ef1fe9ea9ff25 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 eff025e511e..53792c70d27 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -153,6 +153,9 @@ class AccountManager implements IAccountManager { $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 43a98794676e4fcb77aebd1447a2ab8af759417f 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 88c5e4dbcf9..53a9a28c080 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 d19c9d7d0bf..72fd8bc7284 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 d0d623d9ed9..7988a8df6a9 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 e5cfd942f46..5a2077d4fc3 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 8aa7b195ff5..6f8516e6437 100644 --- a/apps/settings/templates/settings/personal/personal.info.php +++ b/apps/settings/templates/settings/personal/personal.info.php @@ -122,9 +122,7 @@ script('settings', [ - -
    @@ -172,9 +170,7 @@ script('settings', [ t('For password reset and notifications')); ?> - - - +