Merge pull request #26679 from nextcloud/bugfix/noid/fix-unauthorized-ocs-status-in-provisioning

Fix unauthorized OCS status in provisioning

3 files changed, 24 insertions, 16 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;