Merge pull request #32835 from nextcloud/backport/32799/stable22

[stable22] Fix exception handling when profile data is too long
nextcloud · Jun 13, 2022 · be236ef · be236ef
2 parents 5b4341a + 3189626
commit be236ef
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
@@ -908,7 +908,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_DISPLAYNAME . self::SCOPE_SUFFIX:
 			case IAccountManager::PROPERTY_EMAIL . self::SCOPE_SUFFIX:

diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
@@ -935,9 +935,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();
@@ -971,25 +972,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');
 
@@ -1000,10 +1001,10 @@ public function testGetUserDataAsAdmin() {
 			IAccountManager::PROPERTY_WEBSITE => ['value' => 'website'],
 		]);
 		$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')
@@ -1122,10 +1123,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')
@@ -3563,11 +3564,12 @@ public function testGetUser() {
 			'displayname' => 'Demo User'
 		];
 
-		$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());
@@ -3753,11 +3755,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);
 
@@ -3804,11 +3806,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);
 
@@ -3857,11 +3859,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
@@ -194,7 +194,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('');
 				}