From b2f78e1cb636723fe27029cdb42ea659210891cd Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 9 Jun 2022 22:07:46 +0200 Subject: [PATCH] Fix exception handling when profile data is too long Signed-off-by: Joas Schilling --- .../lib/Controller/UsersController.php | 6 ++- .../tests/Controller/UsersControllerTest.php | 54 ++++++++++--------- lib/private/Accounts/AccountManager.php | 2 +- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 38d51857ffc91..a26479ba0a8f2 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -958,7 +958,11 @@ public function editUser(string $userId, string $key, string $value): DataRespon } catch (PropertyDoesNotExistException $e) { $userAccount->setProperty($key, $value, IAccountManager::SCOPE_PRIVATE, IAccountManager::NOT_VERIFIED); } - $this->accountManager->updateAccount($userAccount); + try { + $this->accountManager->updateAccount($userAccount); + } catch (InvalidArgumentException $e) { + throw new OCSException('Invalid ' . $e->getMessage(), 102); + } break; case IAccountManager::PROPERTY_PROFILE_ENABLED: $userAccount = $this->accountManager->getAccount($targetUser); diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index 2ce5cadb57d22..6162be54a041c 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -939,9 +939,10 @@ public function testGetUserTargetDoesNotExist() { } public function testGetUserDataAsAdmin() { - $group = $this->getMockBuilder(IGroup::class) - ->disableOriginalConstructor() - ->getMock(); + $group0 = $this->createMock(IGroup::class); + $group1 = $this->createMock(IGroup::class); + $group2 = $this->createMock(IGroup::class); + $group3 = $this->createMock(IGroup::class); $loggedInUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() ->getMock(); @@ -975,7 +976,7 @@ public function testGetUserDataAsAdmin() { $this->groupManager ->expects($this->any()) ->method('getUserGroups') - ->willReturn([$group, $group, $group]); + ->willReturn([$group0, $group1, $group2]); $this->groupManager ->expects($this->once()) ->method('getSubAdmin') @@ -983,17 +984,17 @@ public function testGetUserDataAsAdmin() { $subAdminManager ->expects($this->once()) ->method('getSubAdminsGroups') - ->willReturn([$group]); - $group->expects($this->at(0)) + ->willReturn([$group3]); + $group0->expects($this->once()) ->method('getGID') ->willReturn('group0'); - $group->expects($this->at(1)) + $group1->expects($this->once()) ->method('getGID') ->willReturn('group1'); - $group->expects($this->at(2)) + $group2->expects($this->once()) ->method('getGID') ->willReturn('group2'); - $group->expects($this->at(3)) + $group3->expects($this->once()) ->method('getGID') ->willReturn('group3'); @@ -1009,10 +1010,10 @@ public function testGetUserDataAsAdmin() { IAccountManager::PROPERTY_PROFILE_ENABLED => ['value' => '1'], ]); $this->config - ->expects($this->at(0)) ->method('getUserValue') - ->with('UID', 'core', 'enabled', 'true') - ->willReturn('true'); + ->willReturnMap([ + ['UID', 'core', 'enabled', 'true', 'true'], + ]); $this->api ->expects($this->once()) ->method('fillStorageInfo') @@ -1136,10 +1137,10 @@ public function testGetUserDataAsSubAdminAndUserIsAccessible() { ->method('getSubAdmin') ->willReturn($subAdminManager); $this->config - ->expects($this->at(0)) ->method('getUserValue') - ->with('UID', 'core', 'enabled', 'true') - ->willReturn('true'); + ->willReturnMap([ + ['UID', 'core', 'enabled', 'true', 'true'], + ]); $this->api ->expects($this->once()) ->method('fillStorageInfo') @@ -3622,11 +3623,12 @@ public function testGetUser() { 'profile_enabled' => '1' ]; - $api->expects($this->at(0))->method('getUserData') - ->with('uid', false) - ->willReturn($expected); - $api->expects($this->at(1))->method('getUserData') - ->with('currentuser', true) + $api->expects($this->exactly(2)) + ->method('getUserData') + ->withConsecutive( + ['uid', false], + ['currentuser', true], + ) ->willReturn($expected); $this->assertSame($expected, $api->getUser('uid')->getData()); @@ -3812,11 +3814,11 @@ public function testResendWelcomeMessageSuccess() { ->willReturn('abc@example.org'); $emailTemplate = $this->createMock(IEMailTemplate::class); $this->newUserMailHelper - ->expects($this->at(0)) + ->expects($this->once()) ->method('generateTemplate') ->willReturn($emailTemplate); $this->newUserMailHelper - ->expects($this->at(1)) + ->expects($this->once()) ->method('sendMail') ->with($targetUser, $emailTemplate); @@ -3863,11 +3865,11 @@ public function testResendWelcomeMessageSuccessWithFallbackLanguage() { ->getMock(); $emailTemplate = $this->createMock(IEMailTemplate::class); $this->newUserMailHelper - ->expects($this->at(0)) + ->expects($this->once()) ->method('generateTemplate') ->willReturn($emailTemplate); $this->newUserMailHelper - ->expects($this->at(1)) + ->expects($this->once()) ->method('sendMail') ->with($targetUser, $emailTemplate); @@ -3916,11 +3918,11 @@ public function testResendWelcomeMessageFailed() { ->willReturn('abc@example.org'); $emailTemplate = $this->createMock(IEMailTemplate::class); $this->newUserMailHelper - ->expects($this->at(0)) + ->expects($this->once()) ->method('generateTemplate') ->willReturn($emailTemplate); $this->newUserMailHelper - ->expects($this->at(1)) + ->expects($this->once()) ->method('sendMail') ->with($targetUser, $emailTemplate) ->willThrowException(new \Exception()); diff --git a/lib/private/Accounts/AccountManager.php b/lib/private/Accounts/AccountManager.php index 127adc9ef383f..5ea5f4d245484 100644 --- a/lib/private/Accounts/AccountManager.php +++ b/lib/private/Accounts/AccountManager.php @@ -199,7 +199,7 @@ protected function testValueLengths(array $properties, bool $throwOnData = false foreach ($properties as $property) { if (strlen($property->getValue()) > 2048) { if ($throwOnData) { - throw new InvalidArgumentException(); + throw new InvalidArgumentException($property->getName()); } else { $property->setValue(''); }