diff options
author | blizzz <blizzz@arthur-schiwon.de> | 2021-05-13 23:39:20 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-05-13 23:39:20 +0200 |
commit | 0ab5b3e265671d8a367bd601c426d8b509811a34 (patch) | |
tree | 6346fea35c43402d93b990d757e1912c6268d38b /apps | |
parent | b8b2e796bfc604c36aed83fc6e8c509cac92ca26 (diff) | |
parent | b6c6527705695a343b055f89bdde5ec497914ff1 (diff) | |
download | nextcloud-server-0ab5b3e265671d8a367bd601c426d8b509811a34.tar.gz nextcloud-server-0ab5b3e265671d8a367bd601c426d8b509811a34.zip |
Merge pull request #26679 from nextcloud/bugfix/noid/fix-unauthorized-ocs-status-in-provisioning
Fix unauthorized OCS status in provisioning
Diffstat (limited to 'apps')
5 files changed, 34 insertions, 26 deletions
diff --git a/apps/provisioning_api/lib/Controller/GroupsController.php b/apps/provisioning_api/lib/Controller/GroupsController.php index 583af8454f6..05c738bdbd5 100644 --- a/apps/provisioning_api/lib/Controller/GroupsController.php +++ b/apps/provisioning_api/lib/Controller/GroupsController.php @@ -225,7 +225,7 @@ class GroupsController extends AUserData { return new DataResponse(['users' => $usersDetails]); } - throw new OCSException('User does not have access to specified group', OCSController::RESPOND_UNAUTHORISED); + throw new OCSException('The requested group could not be found', OCSController::RESPOND_NOT_FOUND); } /** @@ -271,7 +271,7 @@ class GroupsController extends AUserData { throw new OCSException('Not supported by backend', 101); } else { - throw new OCSException('', OCSController::RESPOND_UNAUTHORISED); + throw new OCSException('', OCSController::RESPOND_UNKNOWN_ERROR); } } diff --git a/apps/provisioning_api/lib/Controller/UsersController.php b/apps/provisioning_api/lib/Controller/UsersController.php index f254620f330..eeed61f6703 100644 --- a/apps/provisioning_api/lib/Controller/UsersController.php +++ b/apps/provisioning_api/lib/Controller/UsersController.php @@ -508,7 +508,7 @@ class UsersController extends AUserData { $data = $this->getUserData($userId, $includeScopes); // getUserData returns empty array if not enough permissions if (empty($data)) { - throw new OCSException('', OCSController::RESPOND_UNAUTHORISED); + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } return new DataResponse($data); } @@ -601,7 +601,7 @@ class UsersController extends AUserData { $targetUser = $this->userManager->get($userId); if ($targetUser === null) { - throw new OCSException('', OCSController::RESPOND_UNAUTHORISED); + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } $permittedFields = []; @@ -667,12 +667,12 @@ class UsersController extends AUserData { $permittedFields[] = 'quota'; } else { // No rights - throw new OCSException('', OCSController::RESPOND_UNAUTHORISED); + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } } // Check if permitted to edit this field if (!in_array($key, $permittedFields)) { - throw new OCSException('', OCSController::RESPOND_UNAUTHORISED); + throw new OCSException('', 103); } // Process the edit switch ($key) { @@ -689,7 +689,7 @@ class UsersController extends AUserData { $quota = \OCP\Util::computerFileSize($quota); } if ($quota === false) { - throw new OCSException('Invalid quota value '.$value, 103); + throw new OCSException('Invalid quota value '.$value, 102); } if ($quota === -1) { $quota = 'none'; @@ -789,14 +789,18 @@ class UsersController extends AUserData { $targetUser = $this->userManager->get($userId); - if ($targetUser === null || $targetUser->getUID() === $currentLoggedInUser->getUID()) { + if ($targetUser === null) { + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); + } + + if ($targetUser->getUID() === $currentLoggedInUser->getUID()) { throw new OCSException('', 101); } // If not permitted $subAdminManager = $this->groupManager->getSubAdmin(); if (!$this->groupManager->isAdmin($currentLoggedInUser->getUID()) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) { - throw new OCSException('', OCSController::RESPOND_UNAUTHORISED); + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } $this->remoteWipe->markAllTokensForWipe($targetUser); @@ -817,14 +821,18 @@ class UsersController extends AUserData { $targetUser = $this->userManager->get($userId); - if ($targetUser === null || $targetUser->getUID() === $currentLoggedInUser->getUID()) { + if ($targetUser === null) { + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); + } + + if ($targetUser->getUID() === $currentLoggedInUser->getUID()) { throw new OCSException('', 101); } // If not permitted $subAdminManager = $this->groupManager->getSubAdmin(); if (!$this->groupManager->isAdmin($currentLoggedInUser->getUID()) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) { - throw new OCSException('', OCSController::RESPOND_UNAUTHORISED); + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } // Go ahead with the delete @@ -878,7 +886,7 @@ class UsersController extends AUserData { // If not permitted $subAdminManager = $this->groupManager->getSubAdmin(); if (!$this->groupManager->isAdmin($currentLoggedInUser->getUID()) && !$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser)) { - throw new OCSException('', OCSController::RESPOND_UNAUTHORISED); + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } // enable/disable the user now @@ -925,7 +933,7 @@ class UsersController extends AUserData { return new DataResponse(['groups' => $groups]); } else { // Not permitted - throw new OCSException('', OCSController::RESPOND_UNAUTHORISED); + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } } } @@ -1133,7 +1141,7 @@ class UsersController extends AUserData { if (!$subAdminManager->isUserAccessible($currentLoggedInUser, $targetUser) && !$this->groupManager->isAdmin($currentLoggedInUser->getUID())) { // No rights - throw new OCSException('', OCSController::RESPOND_UNAUTHORISED); + throw new OCSException('', OCSController::RESPOND_NOT_FOUND); } $email = $targetUser->getEMailAddress(); diff --git a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php index a8bb8140060..cf51a148bba 100644 --- a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php +++ b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php @@ -30,10 +30,10 @@ namespace OCA\Provisioning_API\Middleware; use OCA\Provisioning_API\Middleware\Exceptions\NotSubAdminException; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; use OCP\AppFramework\Http\Response; use OCP\AppFramework\Middleware; use OCP\AppFramework\OCS\OCSException; -use OCP\AppFramework\OCSController; use OCP\AppFramework\Utility\IControllerMethodReflector; class ProvisioningApiMiddleware extends Middleware { @@ -84,7 +84,7 @@ class ProvisioningApiMiddleware extends Middleware { */ public function afterException($controller, $methodName, \Exception $exception) { if ($exception instanceof NotSubAdminException) { - throw new OCSException($exception->getMessage(), OCSController::RESPOND_UNAUTHORISED); + throw new OCSException($exception->getMessage(), Http::STATUS_FORBIDDEN); } throw $exception; diff --git a/apps/provisioning_api/tests/Controller/UsersControllerTest.php b/apps/provisioning_api/tests/Controller/UsersControllerTest.php index ed644f58cd8..c1a42457871 100644 --- a/apps/provisioning_api/tests/Controller/UsersControllerTest.php +++ b/apps/provisioning_api/tests/Controller/UsersControllerTest.php @@ -1207,7 +1207,7 @@ class UsersControllerTest extends TestCase { public function testGetUserDataAsSubAdminAndUserIsNotAccessible() { $this->expectException(\OCP\AppFramework\OCS\OCSException::class); - $this->expectExceptionCode(997); + $this->expectExceptionCode(998); $loggedInUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() @@ -1722,7 +1722,7 @@ class UsersControllerTest extends TestCase { public function testEditUserRegularUserSelfEditChangeQuota() { $this->expectException(\OCP\AppFramework\OCS\OCSException::class); - $this->expectExceptionCode(997); + $this->expectExceptionCode(103); $loggedInUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() @@ -1800,7 +1800,7 @@ class UsersControllerTest extends TestCase { public function testEditUserAdminUserSelfEditChangeInvalidQuota() { $this->expectException(\OCP\AppFramework\OCS\OCSException::class); $this->expectExceptionMessage('Invalid quota value ABC'); - $this->expectExceptionCode(103); + $this->expectExceptionCode(102); $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser @@ -2132,7 +2132,7 @@ class UsersControllerTest extends TestCase { public function testEditUserSubadminUserInaccessible() { $this->expectException(\OCP\AppFramework\OCS\OCSException::class); - $this->expectExceptionCode(997); + $this->expectExceptionCode(998); $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser @@ -2172,7 +2172,7 @@ class UsersControllerTest extends TestCase { public function testDeleteUserNotExistingUser() { $this->expectException(\OCP\AppFramework\OCS\OCSException::class); - $this->expectExceptionCode(101); + $this->expectExceptionCode(998); $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser @@ -2385,7 +2385,7 @@ class UsersControllerTest extends TestCase { public function testDeleteUserAsSubAdminAndUserIsNotAccessible() { $this->expectException(\OCP\AppFramework\OCS\OCSException::class); - $this->expectExceptionCode(997); + $this->expectExceptionCode(998); $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser @@ -2566,7 +2566,7 @@ class UsersControllerTest extends TestCase { public function testGetUsersGroupsForSubAdminUserAndUserIsInaccessible() { $this->expectException(\OCP\AppFramework\OCS\OCSException::class); - $this->expectExceptionCode(997); + $this->expectExceptionCode(998); $loggedInUser = $this->getMockBuilder(IUser::class)->disableOriginalConstructor()->getMock(); $loggedInUser @@ -3567,7 +3567,7 @@ class UsersControllerTest extends TestCase { public function testResendWelcomeMessageAsSubAdminAndUserIsNotAccessible() { $this->expectException(\OCP\AppFramework\OCS\OCSException::class); - $this->expectExceptionCode(997); + $this->expectExceptionCode(998); $loggedInUser = $this->getMockBuilder(IUser::class) ->disableOriginalConstructor() diff --git a/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php b/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php index 4565bde42a2..1977a4aa0f9 100644 --- a/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php +++ b/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php @@ -27,8 +27,8 @@ namespace OCA\Provisioning_API\Tests\Middleware; use OCA\Provisioning_API\Middleware\Exceptions\NotSubAdminException; use OCA\Provisioning_API\Middleware\ProvisioningApiMiddleware; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; use OCP\AppFramework\OCS\OCSException; -use OCP\AppFramework\OCSController; use OCP\AppFramework\Utility\IControllerMethodReflector; use Test\TestCase; @@ -115,7 +115,7 @@ class ProvisioningApiMiddlewareTest extends TestCase { } catch (OCSException $e) { $this->assertFalse($forwared); $this->assertSame($exception->getMessage(), $e->getMessage()); - $this->assertSame(OCSController::RESPOND_UNAUTHORISED, $e->getCode()); + $this->assertSame(Http::STATUS_FORBIDDEN, $e->getCode()); } catch (\Exception $e) { $this->assertTrue($forwared); $this->assertSame($exception, $e); |