Fix exception handling when profile data is too long

Signed-off-by: Joas Schilling <coding@schilljs.com>
nextcloud · Jun 12, 2022 · b2f78e1 · b2f78e1
1 parent f8d0ea2
commit b2f78e1
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 28 deletions.
diff --git 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
@@ -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,25 +976,25 @@ 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')
 			->willReturn($subAdminManager);
 		$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
@@ -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('');
 				}