Skip to content

Commit

Permalink
fix(provisioning_api): Translate exceptions shown in the frontend + r…
Browse files Browse the repository at this point in the history
…eplace some deprecations

Signed-off-by: Ferdinand Thiessen <[email protected]>
  • Loading branch information
susnux committed Jan 31, 2024
1 parent 6594360 commit 8a25bee
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 70 deletions.
78 changes: 29 additions & 49 deletions apps/provisioning_api/lib/Controller/UsersController.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand All @@ -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(
Expand All @@ -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);
}

/**
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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;
}
Expand Down Expand Up @@ -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 */
Expand All @@ -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);
}
}

Expand All @@ -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();
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand All @@ -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();
Expand Down
40 changes: 19 additions & 21 deletions apps/provisioning_api/tests/Controller/UsersControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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,
Expand Down Expand Up @@ -274,20 +278,14 @@ 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', '', '', []);
}


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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -400,6 +398,9 @@ public function testAddUserSuccessful() {
}

public function testAddUserSuccessfulWithDisplayName() {
/**
* @var UserController
*/
$api = $this->getMockBuilder(UsersController::class)
->setConstructorArgs([
'provisioning_api',
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -4048,9 +4049,6 @@ public function testResendWelcomeMessageSuccessWithFallbackLanguage() {
->expects($this->once())
->method('getEmailAddress')
->willReturn('[email protected]');
$l10n = $this->getMockBuilder(IL10N::class)
->disableOriginalConstructor()
->getMock();
$emailTemplate = $this->createMock(IEMailTemplate::class);
$this->newUserMailHelper
->expects($this->once())
Expand Down

0 comments on commit 8a25bee

Please sign in to comment.