Merge pull request #43234 from nextcloud/fix/provisioning-exception-msgs

fix(provisioning_api): Translate exceptions shown in the frontend
2026-02-20 00:12:30 -05:00 · 2024-02-01 16:55:04 +01:00 · 2024-02-01 16:55:04 +01:00 · 937a6a84fe
commit 937a6a84fe
parent 1b2a96679c a025611420
2 changed files with 60 additions and 82 deletions
--- a/apps/provisioning_api/lib/Controller/UsersController.php
+++ b/apps/provisioning_api/lib/Controller/UsersController.php
@ -63,6 +63,7 @@ use OCP\HintException;
 use OCP\IConfig;
 use OCP\IGroup;
 use OCP\IGroupManager;
+use OCP\IL10N;
 use OCP\IPhoneNumberUtil;
 use OCP\IRequest;
 use OCP\IURLGenerator;
@ -79,22 +80,8 @@ use Psr\Log\LoggerInterface;
 * @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
 */
 class UsersController extends AUserData {
-	/** @var IURLGenerator */
-	protected $urlGenerator;
-	/** @var LoggerInterface */
-	private $logger;
-	/** @var IFactory */
-	protected $l10nFactory;
-	/** @var NewUserMailHelper */
-	private $newUserMailHelper;
-	/** @var ISecureRandom */
-	private $secureRandom;
-	/** @var RemoteWipe */
-	private $remoteWipe;
-	/** @var KnownUserService */
-	private $knownUserService;
-	/** @var IEventDispatcher */
-	private $eventDispatcher;
+
+	private IL10N $l10n;

 	public function __construct(
 		string $appName,
@ -104,14 +91,14 @@ class UsersController extends AUserData {
 		IGroupManager $groupManager,
 		IUserSession $userSession,
 		IAccountManager $accountManager,
-		IURLGenerator $urlGenerator,
-		LoggerInterface $logger,
 		IFactory $l10nFactory,
-		NewUserMailHelper $newUserMailHelper,
-		ISecureRandom $secureRandom,
-		RemoteWipe $remoteWipe,
-		KnownUserService $knownUserService,
-		IEventDispatcher $eventDispatcher,
+		private IURLGenerator $urlGenerator,
+		private LoggerInterface $logger,
+		private NewUserMailHelper $newUserMailHelper,
+		private ISecureRandom $secureRandom,
+		private RemoteWipe $remoteWipe,
+		private KnownUserService $knownUserService,
+		private IEventDispatcher $eventDispatcher,
 		private IPhoneNumberUtil $phoneNumberUtil,
 	) {
 		parent::__construct(
@ -125,14 +112,7 @@ class UsersController extends AUserData {
 			$l10nFactory
 		);

-		$this->urlGenerator = $urlGenerator;
-		$this->logger = $logger;
-		$this->l10nFactory = $l10nFactory;
-		$this->newUserMailHelper = $newUserMailHelper;
-		$this->secureRandom = $secureRandom;
-		$this->remoteWipe = $remoteWipe;
-		$this->knownUserService = $knownUserService;
-		$this->eventDispatcher = $eventDispatcher;
+		$this->l10n = $l10nFactory->get($appName);
 	}

 	/**
@ -392,7 +372,7 @@ class UsersController extends AUserData {
 			}
 			$attempts++;
 		} while ($attempts < 10);
-		throw new OCSException('Could not create non-existing user id', 111);
+		throw new OCSException($this->l10n->t('Could not create non-existing user id'), 111);
 	}

 	/**
@ -437,21 +417,21 @@ class UsersController extends AUserData {

 		if ($this->userManager->userExists($userid)) {
 			$this->logger->error('Failed addUser attempt: User already exists.', ['app' => 'ocs_api']);
-			throw new OCSException($this->l10nFactory->get('provisioning_api')->t('User already exists'), 102);
+			throw new OCSException($this->l10n->t('User already exists'), 102);
 		}

 		if ($groups !== []) {
 			foreach ($groups as $group) {
 				if (!$this->groupManager->groupExists($group)) {
-					throw new OCSException('group ' . $group . ' does not exist', 104);
+					throw new OCSException($this->l10n->t('Group %1$s does not exist', [$group]), 104);
 				}
 				if (!$isAdmin && !$subAdminManager->isSubAdminOfGroup($user, $this->groupManager->get($group))) {
-					throw new OCSException('insufficient privileges for group ' . $group, 105);
+					throw new OCSException($this->l10n->t('Insufficient privileges for group %1$s', [$group]), 105);
 				}
 			}
 		} else {
 			if (!$isAdmin) {
-				throw new OCSException('no group specified (required for subadmins)', 106);
+				throw new OCSException($this->l10n->t('No group specified (required for sub-admins)'), 106);
 			}
 		}

@ -461,15 +441,15 @@ class UsersController extends AUserData {
 				$group = $this->groupManager->get($groupid);
 				// Check if group exists
 				if ($group === null) {
-					throw new OCSException('Subadmin group does not exist', 102);
+					throw new OCSException($this->l10n->t('Sub-admin group does not exist'), 102);
 				}
 				// Check if trying to make subadmin of admin group
 				if ($group->getGID() === 'admin') {
-					throw new OCSException('Cannot create subadmins for admin group', 103);
+					throw new OCSException($this->l10n->t('Cannot create sub-admins for admin group'), 103);
 				}
 				// Check if has permission to promote subadmins
 				if (!$subAdminManager->isSubAdminOfGroup($user, $group) && !$isAdmin) {
-					throw new OCSForbiddenException('No permissions to promote subadmins');
+					throw new OCSForbiddenException($this->l10n->t('No permissions to promote sub-admins'));
 				}
 				$subadminGroups[] = $group;
 			}
@ -477,11 +457,11 @@ class UsersController extends AUserData {

 		$generatePasswordResetToken = false;
 		if (strlen($password) > IUserManager::MAX_PASSWORD_LENGTH) {
-			throw new OCSException('Invalid password value', 101);
+			throw new OCSException($this->l10n->t('Invalid password value'), 101);
 		}
 		if ($password === '') {
 			if ($email === '') {
-				throw new OCSException('To send a password link to the user an email address is required.', 108);
+				throw new OCSException($this->l10n->t('To send a password link to the user an email address is required.'), 108);
 			}

 			$passwordEvent = new GenerateSecurePasswordEvent();
@ -500,7 +480,7 @@ class UsersController extends AUserData {
 		}

 		if ($email === '' && $this->config->getAppValue('core', 'newUser.requireEmail', 'no') === 'yes') {
-			throw new OCSException('Required email address was not provided', 110);
+			throw new OCSException($this->l10n->t('Required email address was not provided'), 110);
 		}

 		try {
@ -986,14 +966,14 @@ class UsersController extends AUserData {
 						$quota = \OCP\Util::computerFileSize($quota);
 					}
 					if ($quota === false) {
-						throw new OCSException('Invalid quota value ' . $value, 102);
+						throw new OCSException($this->l10n->t('Invalid quota value: %1$s', [$value]), 102);
 					}
 					if ($quota === -1) {
 						$quota = 'none';
 					} else {
 						$maxQuota = (int) $this->config->getAppValue('files', 'max_quota', '-1');
 						if ($maxQuota !== -1 && $quota > $maxQuota) {
-							throw new OCSException('Invalid quota value. ' . $value . ' is exceeding the maximum quota', 102);
+							throw new OCSException($this->l10n->t('Invalid quota value. %1$s is exceeding the maximum quota', [$value]), 102);
 						}
 						$quota = \OCP\Util::humanFileSize($quota);
 					}
@ -1002,7 +982,7 @@ class UsersController extends AUserData {
 				if ($quota === 'none') {
 					$allowUnlimitedQuota = $this->config->getAppValue('files', 'allow_unlimited_quota', '1') === '1';
 					if (!$allowUnlimitedQuota) {
-						throw new OCSException('Unlimited quota is forbidden on this instance', 102);
+						throw new OCSException($this->l10n->t('Unlimited quota is forbidden on this instance'), 102);
 					}
 				}
 				$targetUser->setQuota($quota);
@ -1013,10 +993,10 @@ class UsersController extends AUserData {
 			case self::USER_FIELD_PASSWORD:
 				try {
 					if (strlen($value) > IUserManager::MAX_PASSWORD_LENGTH) {
-						throw new OCSException('Invalid password value', 102);
+						throw new OCSException($this->l10n->t('Invalid password value'), 102);
 					}
 					if (!$targetUser->canChangePassword()) {
-						throw new OCSException('Setting the password is not supported by the users backend', 103);
+						throw new OCSException($this->l10n->t('Setting the password is not supported by the users backend'), 103);
 					}
 					$targetUser->setPassword($value);
 				} catch (HintException $e) { // password policy error
@ -1026,13 +1006,13 @@ class UsersController extends AUserData {
 			case self::USER_FIELD_LANGUAGE:
 				$languagesCodes = $this->l10nFactory->findAvailableLanguages();
 				if (!in_array($value, $languagesCodes, true) && $value !== 'en') {
-					throw new OCSException('Invalid language', 102);
+					throw new OCSException($this->l10n->t('Invalid language'), 102);
 				}
 				$this->config->setUserValue($targetUser->getUID(), 'core', 'lang', $value);
 				break;
 			case self::USER_FIELD_LOCALE:
 				if (!$this->l10nFactory->localeExists($value)) {
-					throw new OCSException('Invalid locale', 102);
+					throw new OCSException($this->l10n->t('Invalid locale'), 102);
 				}
 				$this->config->setUserValue($targetUser->getUID(), 'core', 'locale', $value);
 				break;
@ -1415,11 +1395,11 @@ class UsersController extends AUserData {
 		if ($targetUser->getUID() === $loggedInUser->getUID()) {
 			if ($this->groupManager->isAdmin($loggedInUser->getUID())) {
 				if ($group->getGID() === 'admin') {
-					throw new OCSException('Cannot remove yourself from the admin group', 105);
+					throw new OCSException($this->l10n->t('Cannot remove yourself from the admin group'), 105);
 				}
 			} else {
 				// Not an admin, so the user must be a subadmin of this group, but that is not allowed.
-				throw new OCSException('Cannot remove yourself from this group as you are a SubAdmin', 105);
+				throw new OCSException($this->l10n->t('Cannot remove yourself from this group as you are a sub-admin'), 105);
 			}
 		} elseif (!$this->groupManager->isAdmin($loggedInUser->getUID())) {
 			/** @var IGroup[] $subAdminGroups */
@ -1432,7 +1412,7 @@ class UsersController extends AUserData {

 			if (count($userSubAdminGroups) <= 1) {
 				// Subadmin must not be able to remove a user from all their subadmin groups.
-				throw new OCSException('Not viable to remove user from the last group you are SubAdmin of', 105);
+				throw new OCSException($this->l10n->t('Not viable to remove user from the last group you are sub-admin of'), 105);
 			}
 		}

@ -1459,15 +1439,15 @@ class UsersController extends AUserData {

 		// Check if the user exists
 		if ($user === null) {
-			throw new OCSException('User does not exist', 101);
+			throw new OCSException($this->l10n->t('User does not exist'), 101);
 		}
 		// Check if group exists
 		if ($group === null) {
-			throw new OCSException('Group does not exist', 102);
+			throw new OCSException($this->l10n->t('Group does not exist'), 102);
 		}
 		// Check if trying to make subadmin of admin group
 		if ($group->getGID() === 'admin') {
-			throw new OCSException('Cannot create subadmins for admin group', 103);
+			throw new OCSException($this->l10n->t('Cannot create sub-admins for admin group'), 103);
 		}

 		$subAdminManager = $this->groupManager->getSubAdmin();
@ -1500,15 +1480,15 @@ class UsersController extends AUserData {

 		// Check if the user exists
 		if ($user === null) {
-			throw new OCSException('User does not exist', 101);
+			throw new OCSException($this->l10n->t('User does not exist'), 101);
 		}
 		// Check if the group exists
 		if ($group === null) {
-			throw new OCSException('Group does not exist', 101);
+			throw new OCSException($this->l10n->t('Group does not exist'), 101);
 		}
 		// Check if they are a subadmin of this said group
 		if (!$subAdminManager->isSubAdminOfGroup($user, $group)) {
-			throw new OCSException('User is not a subadmin of this group', 102);
+			throw new OCSException($this->l10n->t('User is not a sub-admin of this group'), 102);
 		}

 		// Go
@ -1562,7 +1542,7 @@ class UsersController extends AUserData {

 		$email = $targetUser->getEMailAddress();
 		if ($email === '' || $email === null) {
-			throw new OCSException('Email address not available', 101);
+			throw new OCSException($this->l10n->t('Email address not available'), 101);
 		}

 		try {
@ -1576,7 +1556,7 @@ class UsersController extends AUserData {
 					'exception' => $e,
 				]
 			);
-			throw new OCSException('Sending email failed', 102);
+			throw new OCSException($this->l10n->t('Sending email failed'), 102);
 		}

 		return new DataResponse();
--- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php
+++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php
@ -129,6 +129,10 @@ class UsersControllerTest extends TestCase {
 		$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
 		$this->phoneNumberUtil = new PhoneNumberUtil();

+		$l10n = $this->createMock(IL10N::class);
+		$l10n->method('t')->willReturnCallback(fn (string $txt, array $replacement = []) => sprintf($txt, ...$replacement));
+		$this->l10nFactory->method('get')->with('provisioning_api')->willReturn($l10n);
+
 		$this->api = $this->getMockBuilder(UsersController::class)
 			->setConstructorArgs([
 				'provisioning_api',
@ -138,9 +142,9 @@ class UsersControllerTest extends TestCase {
 				$this->groupManager,
 				$this->userSession,
 				$this->accountManager,
+				$this->l10nFactory,
 				$this->urlGenerator,
 				$this->logger,
-				$this->l10nFactory,
 				$this->newUserMailHelper,
 				$this->secureRandom,
 				$this->remoteWipe,
@ -274,12 +278,6 @@ class UsersControllerTest extends TestCase {
 			->method('isAdmin')
 			->with('adminUser')
 			->willReturn(true);
-		$l10n = $this->createMock(IL10N::class);
-		$this->l10nFactory
-			->expects($this->once())
-			->method('get')
-			->with('provisioning_api')
-			->willReturn($l10n);

 		$this->api->addUser('AlreadyExistingUser', 'password', '', '', []);
 	}
@ -287,7 +285,7 @@ class UsersControllerTest extends TestCase {

 	public function testAddUserNonExistingGroup() {
 		$this->expectException(\OCP\AppFramework\OCS\OCSException::class);
-		$this->expectExceptionMessage('group NonExistingGroup does not exist');
+		$this->expectExceptionMessage('Group NonExistingGroup does not exist');
 		$this->expectExceptionCode(104);

 		$this->userManager
@ -323,7 +321,7 @@ class UsersControllerTest extends TestCase {

 	public function testAddUserExistingGroupNonExistingGroup() {
 		$this->expectException(\OCP\AppFramework\OCS\OCSException::class);
-		$this->expectExceptionMessage('group NonExistingGroup does not exist');
+		$this->expectExceptionMessage('Group NonExistingGroup does not exist');
 		$this->expectExceptionCode(104);

 		$this->userManager
@ -400,6 +398,9 @@ class UsersControllerTest extends TestCase {
 	}

 	public function testAddUserSuccessfulWithDisplayName() {
+		/**
+		 * @var UserController
+		 */
 		$api = $this->getMockBuilder(UsersController::class)
 			->setConstructorArgs([
 				'provisioning_api',
@ -409,9 +410,9 @@ class UsersControllerTest extends TestCase {
 				$this->groupManager,
 				$this->userSession,
 				$this->accountManager,
+				$this->l10nFactory,
 				$this->urlGenerator,
 				$this->logger,
-				$this->l10nFactory,
 				$this->newUserMailHelper,
 				$this->secureRandom,
 				$this->remoteWipe,
@ -758,7 +759,7 @@ class UsersControllerTest extends TestCase {

 	public function testAddUserAsSubAdminNoGroup() {
 		$this->expectException(\OCP\AppFramework\OCS\OCSException::class);
-		$this->expectExceptionMessage('no group specified (required for subadmins)');
+		$this->expectExceptionMessage('No group specified (required for sub-admins)');
 		$this->expectExceptionCode(106);

 		$loggedInUser = $this->getMockBuilder(IUser::class)
@ -791,7 +792,7 @@ class UsersControllerTest extends TestCase {

 	public function testAddUserAsSubAdminValidGroupNotSubAdmin() {
 		$this->expectException(\OCP\AppFramework\OCS\OCSException::class);
-		$this->expectExceptionMessage('insufficient privileges for group ExistingGroup');
+		$this->expectExceptionMessage('Insufficient privileges for group ExistingGroup');
 		$this->expectExceptionCode(105);

 		$loggedInUser = $this->getMockBuilder(IUser::class)
@ -2033,7 +2034,7 @@ class UsersControllerTest extends TestCase {

 	public function testEditUserAdminUserSelfEditChangeInvalidQuota() {
 		$this->expectException(\OCP\AppFramework\OCS\OCSException::class);
-		$this->expectExceptionMessage('Invalid quota value ABC');
+		$this->expectExceptionMessage('Invalid quota value: ABC');
 		$this->expectExceptionCode(102);

 		$loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
@ -3199,7 +3200,7 @@ class UsersControllerTest extends TestCase {

 	public function testRemoveFromGroupAsSubAdminFromSubAdmin() {
 		$this->expectException(\OCP\AppFramework\OCS\OCSException::class);
-		$this->expectExceptionMessage('Cannot remove yourself from this group as you are a SubAdmin');
+		$this->expectExceptionMessage('Cannot remove yourself from this group as you are a sub-admin');
 		$this->expectExceptionCode(105);

 		$loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
@ -3254,7 +3255,7 @@ class UsersControllerTest extends TestCase {

 	public function testRemoveFromGroupAsSubAdminFromLastSubAdminGroup() {
 		$this->expectException(\OCP\AppFramework\OCS\OCSException::class);
-		$this->expectExceptionMessage('Not viable to remove user from the last group you are SubAdmin of');
+		$this->expectExceptionMessage('Not viable to remove user from the last group you are sub-admin of');
 		$this->expectExceptionCode(105);

 		$loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
@ -3394,7 +3395,7 @@ class UsersControllerTest extends TestCase {

 	public function testAddSubAdminToAdminGroup() {
 		$this->expectException(\OCP\AppFramework\OCS\OCSException::class);
-		$this->expectExceptionMessage('Cannot create subadmins for admin group');
+		$this->expectExceptionMessage('Cannot create sub-admins for admin group');
 		$this->expectExceptionCode(103);

 		$targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
@ -3517,7 +3518,7 @@ class UsersControllerTest extends TestCase {

 	public function testRemoveSubAdminFromNotASubadmin() {
 		$this->expectException(\OCP\AppFramework\OCS\OCSException::class);
-		$this->expectExceptionMessage('User is not a subadmin of this group');
+		$this->expectExceptionMessage('User is not a sub-admin of this group');
 		$this->expectExceptionCode(102);

 		$targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
@ -3692,9 +3693,9 @@ class UsersControllerTest extends TestCase {
 				$this->groupManager,
 				$this->userSession,
 				$this->accountManager,
+				$this->l10nFactory,
 				$this->urlGenerator,
 				$this->logger,
-				$this->l10nFactory,
 				$this->newUserMailHelper,
 				$this->secureRandom,
 				$this->remoteWipe,
@ -3779,9 +3780,9 @@ class UsersControllerTest extends TestCase {
 				$this->groupManager,
 				$this->userSession,
 				$this->accountManager,
+				$this->l10nFactory,
 				$this->urlGenerator,
 				$this->logger,
-				$this->l10nFactory,
 				$this->newUserMailHelper,
 				$this->secureRandom,
 				$this->remoteWipe,
@ -4048,9 +4049,6 @@ class UsersControllerTest extends TestCase {
 			->expects($this->once())
 			->method('getEmailAddress')
 			->willReturn('abc@example.org');
-		$l10n = $this->getMockBuilder(IL10N::class)
-			->disableOriginalConstructor()
-			->getMock();
 		$emailTemplate = $this->createMock(IEMailTemplate::class);
 		$this->newUserMailHelper
 			->expects($this->once())