From 8a25bee5890c1f37266ca14f1aeb941b4bd7a79c Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Wed, 31 Jan 2024 12:48:34 +0100 Subject: [PATCH] fix(provisioning_api): Translate exceptions shown in the frontend + replace some deprecations Signed-off-by: Ferdinand Thiessen --- .../lib/Controller/UsersController.php | 78 +++++++------------ .../tests/Controller/UsersControllerTest.php | 40 +++++----- 2 files changed, 48 insertions(+), 70 deletions(-) diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index 352970faa37a1..ae362c9d4044c 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -63,6 +63,7 @@ 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 @@ * @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 @@ public function __construct( 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 @@ public function __construct( $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); } /** @@ -437,21 +417,21 @@ public function addUser( 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 @@ public function addUser( $group = $this->groupManager->get($groupid); // Check if group exists if ($group === null) { - throw new OCSException('Subadmin group does not exist', 102); + throw new OCSException('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('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('No permissions to promote sub-admins'); } $subadminGroups[] = $group; } @@ -1415,11 +1395,11 @@ public function removeFromGroup(string $userId, string $groupid): DataResponse { 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 @@ public function removeFromGroup(string $userId, string $groupid): DataResponse { 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 @@ public function addSubAdmin(string $userId, string $groupid): DataResponse { // 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 @@ public function removeSubAdmin(string $userId, string $groupid): DataResponse { // 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 @@ public function resendWelcomeMessage(string $userId): DataResponse { $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 @@ public function resendWelcomeMessage(string $userId): DataResponse { 'exception' => $e, ] ); - throw new OCSException('Sending email failed', 102); + throw new OCSException($this->l10n->t('Sending email failed'), 102); } return new DataResponse(); diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index a48461c0a27b5..5d586466138b5 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -129,6 +129,10 @@ protected function setUp(): void { $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 @@ protected function setUp(): void { $this->groupManager, $this->userSession, $this->accountManager, + $this->l10nFactory, $this->urlGenerator, $this->logger, - $this->l10nFactory, $this->newUserMailHelper, $this->secureRandom, $this->remoteWipe, @@ -274,12 +278,6 @@ public function testAddUserAlreadyExisting() { ->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 @@ public function testAddUserAlreadyExisting() { 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 @@ public function testAddUserNonExistingGroup() { 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 @@ public function testAddUserSuccessful() { } public function testAddUserSuccessfulWithDisplayName() { + /** + * @var UserController + */ $api = $this->getMockBuilder(UsersController::class) ->setConstructorArgs([ 'provisioning_api', @@ -409,9 +410,9 @@ public function testAddUserSuccessfulWithDisplayName() { $this->groupManager, $this->userSession, $this->accountManager, + $this->l10nFactory, $this->urlGenerator, $this->logger, - $this->l10nFactory, $this->newUserMailHelper, $this->secureRandom, $this->remoteWipe, @@ -758,7 +759,7 @@ public function testAddUserUnsuccessful() { 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 @@ public function testAddUserAsSubAdminNoGroup() { 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) @@ -3199,7 +3200,7 @@ public function testRemoveFromGroupAsAdminFromAdmin() { 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 @@ public function testRemoveFromGroupAsSubAdminFromSubAdmin() { 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 @@ public function testAddSubAdminWithNotExistingTargetGroup() { 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 @@ public function testRemoveSubAdminNotExistingTargetGroup() { 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 @@ public function testGetCurrentUserLoggedIn() { $this->groupManager, $this->userSession, $this->accountManager, + $this->l10nFactory, $this->urlGenerator, $this->logger, - $this->l10nFactory, $this->newUserMailHelper, $this->secureRandom, $this->remoteWipe, @@ -3779,9 +3780,9 @@ public function testGetUser() { $this->groupManager, $this->userSession, $this->accountManager, + $this->l10nFactory, $this->urlGenerator, $this->logger, - $this->l10nFactory, $this->newUserMailHelper, $this->secureRandom, $this->remoteWipe, @@ -4048,9 +4049,6 @@ public function testResendWelcomeMessageSuccessWithFallbackLanguage() { ->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())