Skip to content

Commit 46b36b7

Browse files
committed
fix(provisioning_api): Translate exceptions shown in the frontend + replace some deprecations
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
1 parent 6594360 commit 46b36b7

File tree

2 files changed

+52
-74
lines changed

2 files changed

+52
-74
lines changed

apps/provisioning_api/lib/Controller/UsersController.php

Lines changed: 31 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
use OCP\IConfig;
6464
use OCP\IGroup;
6565
use OCP\IGroupManager;
66+
use OCP\IL10N;
6667
use OCP\IPhoneNumberUtil;
6768
use OCP\IRequest;
6869
use OCP\IURLGenerator;
@@ -79,22 +80,8 @@
7980
* @psalm-import-type Provisioning_APIUserDetails from ResponseDefinitions
8081
*/
8182
class UsersController extends AUserData {
82-
/** @var IURLGenerator */
83-
protected $urlGenerator;
84-
/** @var LoggerInterface */
85-
private $logger;
86-
/** @var IFactory */
87-
protected $l10nFactory;
88-
/** @var NewUserMailHelper */
89-
private $newUserMailHelper;
90-
/** @var ISecureRandom */
91-
private $secureRandom;
92-
/** @var RemoteWipe */
93-
private $remoteWipe;
94-
/** @var KnownUserService */
95-
private $knownUserService;
96-
/** @var IEventDispatcher */
97-
private $eventDispatcher;
83+
84+
private IL10N $l10n;
9885

9986
public function __construct(
10087
string $appName,
@@ -104,14 +91,14 @@ public function __construct(
10491
IGroupManager $groupManager,
10592
IUserSession $userSession,
10693
IAccountManager $accountManager,
107-
IURLGenerator $urlGenerator,
108-
LoggerInterface $logger,
10994
IFactory $l10nFactory,
110-
NewUserMailHelper $newUserMailHelper,
111-
ISecureRandom $secureRandom,
112-
RemoteWipe $remoteWipe,
113-
KnownUserService $knownUserService,
114-
IEventDispatcher $eventDispatcher,
95+
private IURLGenerator $urlGenerator,
96+
private LoggerInterface $logger,
97+
private NewUserMailHelper $newUserMailHelper,
98+
private ISecureRandom $secureRandom,
99+
private RemoteWipe $remoteWipe,
100+
private KnownUserService $knownUserService,
101+
private IEventDispatcher $eventDispatcher,
115102
private IPhoneNumberUtil $phoneNumberUtil,
116103
) {
117104
parent::__construct(
@@ -125,14 +112,7 @@ public function __construct(
125112
$l10nFactory
126113
);
127114

128-
$this->urlGenerator = $urlGenerator;
129-
$this->logger = $logger;
130-
$this->l10nFactory = $l10nFactory;
131-
$this->newUserMailHelper = $newUserMailHelper;
132-
$this->secureRandom = $secureRandom;
133-
$this->remoteWipe = $remoteWipe;
134-
$this->knownUserService = $knownUserService;
135-
$this->eventDispatcher = $eventDispatcher;
115+
$this->l10n = $l10nFactory->get($appName);
136116
}
137117

138118
/**
@@ -437,21 +417,21 @@ public function addUser(
437417

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

443423
if ($groups !== []) {
444424
foreach ($groups as $group) {
445425
if (!$this->groupManager->groupExists($group)) {
446-
throw new OCSException('group ' . $group . ' does not exist', 104);
426+
throw new OCSException($this->l10n->t('Group %1$s does not exist', [$group]), 104);
447427
}
448428
if (!$isAdmin && !$subAdminManager->isSubAdminOfGroup($user, $this->groupManager->get($group))) {
449-
throw new OCSException('insufficient privileges for group ' . $group, 105);
429+
throw new OCSException($this->l10n->t('Insufficient privileges for group %1$s', [$group]), 105);
450430
}
451431
}
452432
} else {
453433
if (!$isAdmin) {
454-
throw new OCSException('no group specified (required for subadmins)', 106);
434+
throw new OCSException($this->l10n->t('No group specified (required for sub-admins)'), 106);
455435
}
456436
}
457437

@@ -461,15 +441,15 @@ public function addUser(
461441
$group = $this->groupManager->get($groupid);
462442
// Check if group exists
463443
if ($group === null) {
464-
throw new OCSException('Subadmin group does not exist', 102);
444+
throw new OCSException('Sub-admin group does not exist', 102);
465445
}
466446
// Check if trying to make subadmin of admin group
467447
if ($group->getGID() === 'admin') {
468-
throw new OCSException('Cannot create subadmins for admin group', 103);
448+
throw new OCSException('Cannot create sub-admins for admin group', 103);
469449
}
470450
// Check if has permission to promote subadmins
471451
if (!$subAdminManager->isSubAdminOfGroup($user, $group) && !$isAdmin) {
472-
throw new OCSForbiddenException('No permissions to promote subadmins');
452+
throw new OCSForbiddenException('No permissions to promote sub-admins');
473453
}
474454
$subadminGroups[] = $group;
475455
}
@@ -545,7 +525,7 @@ public function addUser(
545525

546526
// Send new user mail only if a mail is set
547527
if ($email !== '') {
548-
$newUser->setEMailAddress($email);
528+
$newUser->setSystemEMailAddress($email);
549529
if ($this->config->getAppValue('core', 'newUser.sendEmail', 'yes') === 'yes') {
550530
try {
551531
$emailTemplate = $this->newUserMailHelper->generateTemplate($newUser, $generatePasswordResetToken);
@@ -1058,7 +1038,7 @@ public function editUser(string $userId, string $key, string $value): DataRespon
10581038
break;
10591039
case IAccountManager::PROPERTY_EMAIL:
10601040
if (filter_var($value, FILTER_VALIDATE_EMAIL) || $value === '') {
1061-
$targetUser->setEMailAddress($value);
1041+
$targetUser->setSystemEMailAddress($value);
10621042
} else {
10631043
throw new OCSException('', 102);
10641044
}
@@ -1415,11 +1395,11 @@ public function removeFromGroup(string $userId, string $groupid): DataResponse {
14151395
if ($targetUser->getUID() === $loggedInUser->getUID()) {
14161396
if ($this->groupManager->isAdmin($loggedInUser->getUID())) {
14171397
if ($group->getGID() === 'admin') {
1418-
throw new OCSException('Cannot remove yourself from the admin group', 105);
1398+
throw new OCSException($this->l10n->t('Cannot remove yourself from the admin group'), 105);
14191399
}
14201400
} else {
14211401
// Not an admin, so the user must be a subadmin of this group, but that is not allowed.
1422-
throw new OCSException('Cannot remove yourself from this group as you are a SubAdmin', 105);
1402+
throw new OCSException($this->l10n->t('Cannot remove yourself from this group as you are a sub-admin'), 105);
14231403
}
14241404
} elseif (!$this->groupManager->isAdmin($loggedInUser->getUID())) {
14251405
/** @var IGroup[] $subAdminGroups */
@@ -1432,7 +1412,7 @@ public function removeFromGroup(string $userId, string $groupid): DataResponse {
14321412

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

@@ -1459,15 +1439,15 @@ public function addSubAdmin(string $userId, string $groupid): DataResponse {
14591439

14601440
// Check if the user exists
14611441
if ($user === null) {
1462-
throw new OCSException('User does not exist', 101);
1442+
throw new OCSException($this->l10n->t('User does not exist'), 101);
14631443
}
14641444
// Check if group exists
14651445
if ($group === null) {
1466-
throw new OCSException('Group does not exist', 102);
1446+
throw new OCSException($this->l10n->t('Group does not exist'), 102);
14671447
}
14681448
// Check if trying to make subadmin of admin group
14691449
if ($group->getGID() === 'admin') {
1470-
throw new OCSException('Cannot create subadmins for admin group', 103);
1450+
throw new OCSException($this->l10n->t('Cannot create sub-admins for admin group'), 103);
14711451
}
14721452

14731453
$subAdminManager = $this->groupManager->getSubAdmin();
@@ -1500,15 +1480,15 @@ public function removeSubAdmin(string $userId, string $groupid): DataResponse {
15001480

15011481
// Check if the user exists
15021482
if ($user === null) {
1503-
throw new OCSException('User does not exist', 101);
1483+
throw new OCSException($this->l10n->t('User does not exist'), 101);
15041484
}
15051485
// Check if the group exists
15061486
if ($group === null) {
1507-
throw new OCSException('Group does not exist', 101);
1487+
throw new OCSException($this->l10n->t('Group does not exist'), 101);
15081488
}
15091489
// Check if they are a subadmin of this said group
15101490
if (!$subAdminManager->isSubAdminOfGroup($user, $group)) {
1511-
throw new OCSException('User is not a subadmin of this group', 102);
1491+
throw new OCSException($this->l10n->t('User is not a sub-admin of this group'), 102);
15121492
}
15131493

15141494
// Go
@@ -1562,7 +1542,7 @@ public function resendWelcomeMessage(string $userId): DataResponse {
15621542

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

15681548
try {
@@ -1576,7 +1556,7 @@ public function resendWelcomeMessage(string $userId): DataResponse {
15761556
'exception' => $e,
15771557
]
15781558
);
1579-
throw new OCSException('Sending email failed', 102);
1559+
throw new OCSException($this->l10n->t('Sending email failed'), 102);
15801560
}
15811561

15821562
return new DataResponse();

apps/provisioning_api/tests/Controller/UsersControllerTest.php

Lines changed: 21 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ protected function setUp(): void {
129129
$this->eventDispatcher = $this->createMock(IEventDispatcher::class);
130130
$this->phoneNumberUtil = new PhoneNumberUtil();
131131

132+
$l10n = $this->createMock(IL10N::class);
133+
$l10n->method('t')->willReturnCallback(fn(string $txt, array $replacement = []) => sprintf($txt, ...$replacement));
134+
$this->l10nFactory->method('get')->with('provisioning_api')->willReturn($l10n);
135+
132136
$this->api = $this->getMockBuilder(UsersController::class)
133137
->setConstructorArgs([
134138
'provisioning_api',
@@ -138,9 +142,9 @@ protected function setUp(): void {
138142
$this->groupManager,
139143
$this->userSession,
140144
$this->accountManager,
145+
$this->l10nFactory,
141146
$this->urlGenerator,
142147
$this->logger,
143-
$this->l10nFactory,
144148
$this->newUserMailHelper,
145149
$this->secureRandom,
146150
$this->remoteWipe,
@@ -274,20 +278,14 @@ public function testAddUserAlreadyExisting() {
274278
->method('isAdmin')
275279
->with('adminUser')
276280
->willReturn(true);
277-
$l10n = $this->createMock(IL10N::class);
278-
$this->l10nFactory
279-
->expects($this->once())
280-
->method('get')
281-
->with('provisioning_api')
282-
->willReturn($l10n);
283281

284282
$this->api->addUser('AlreadyExistingUser', 'password', '', '', []);
285283
}
286284

287285

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

293291
$this->userManager
@@ -323,7 +321,7 @@ public function testAddUserNonExistingGroup() {
323321

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

329327
$this->userManager
@@ -400,6 +398,9 @@ public function testAddUserSuccessful() {
400398
}
401399

402400
public function testAddUserSuccessfulWithDisplayName() {
401+
/**
402+
* @var UserController
403+
*/
403404
$api = $this->getMockBuilder(UsersController::class)
404405
->setConstructorArgs([
405406
'provisioning_api',
@@ -409,9 +410,9 @@ public function testAddUserSuccessfulWithDisplayName() {
409410
$this->groupManager,
410411
$this->userSession,
411412
$this->accountManager,
413+
$this->l10nFactory,
412414
$this->urlGenerator,
413415
$this->logger,
414-
$this->l10nFactory,
415416
$this->newUserMailHelper,
416417
$this->secureRandom,
417418
$this->remoteWipe,
@@ -522,7 +523,7 @@ public function testAddUserSuccessfulGeneratePassword() {
522523
->willReturn(false);
523524
$newUser = $this->createMock(IUser::class);
524525
$newUser->expects($this->once())
525-
->method('setEMailAddress');
526+
->method('setSystemEMailAddress');
526527
$this->userManager
527528
->expects($this->once())
528529
->method('createUser')
@@ -758,7 +759,7 @@ public function testAddUserUnsuccessful() {
758759

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

764765
$loggedInUser = $this->getMockBuilder(IUser::class)
@@ -791,7 +792,7 @@ public function testAddUserAsSubAdminNoGroup() {
791792

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

797798
$loggedInUser = $this->getMockBuilder(IUser::class)
@@ -1540,7 +1541,7 @@ public function testEditUserRegularUserSelfEditChangeEmailValid() {
15401541
->willReturn($targetUser);
15411542
$targetUser
15421543
->expects($this->once())
1543-
->method('setEMailAddress')
1544+
->method('setSystemEMailAddress')
15441545
->with('demo@nextcloud.com');
15451546
$targetUser
15461547
->expects($this->any())
@@ -3199,7 +3200,7 @@ public function testRemoveFromGroupAsAdminFromAdmin() {
31993200

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

32053206
$loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
@@ -3254,7 +3255,7 @@ public function testRemoveFromGroupAsSubAdminFromSubAdmin() {
32543255

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

32603261
$loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
@@ -3394,7 +3395,7 @@ public function testAddSubAdminWithNotExistingTargetGroup() {
33943395

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

34003401
$targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
@@ -3517,7 +3518,7 @@ public function testRemoveSubAdminNotExistingTargetGroup() {
35173518

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

35233524
$targetUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock();
@@ -3692,9 +3693,9 @@ public function testGetCurrentUserLoggedIn() {
36923693
$this->groupManager,
36933694
$this->userSession,
36943695
$this->accountManager,
3696+
$this->l10nFactory,
36953697
$this->urlGenerator,
36963698
$this->logger,
3697-
$this->l10nFactory,
36983699
$this->newUserMailHelper,
36993700
$this->secureRandom,
37003701
$this->remoteWipe,
@@ -3779,9 +3780,9 @@ public function testGetUser() {
37793780
$this->groupManager,
37803781
$this->userSession,
37813782
$this->accountManager,
3783+
$this->l10nFactory,
37823784
$this->urlGenerator,
37833785
$this->logger,
3784-
$this->l10nFactory,
37853786
$this->newUserMailHelper,
37863787
$this->secureRandom,
37873788
$this->remoteWipe,
@@ -4048,9 +4049,6 @@ public function testResendWelcomeMessageSuccessWithFallbackLanguage() {
40484049
->expects($this->once())
40494050
->method('getEmailAddress')
40504051
->willReturn('abc@example.org');
4051-
$l10n = $this->getMockBuilder(IL10N::class)
4052-
->disableOriginalConstructor()
4053-
->getMock();
40544052
$emailTemplate = $this->createMock(IEMailTemplate::class);
40554053
$this->newUserMailHelper
40564054
->expects($this->once())

0 commit comments

Comments
 (0)