Merge pull request #4786 from nextcloud/backport/4785/stable28

[stable28] fix(Delegation): Fix checks

Author: icewind1991

lib/AuthorizedAdminSettingMiddleware.php

@@ -21,28 +21,25 @@
 
 namespace OCA\GroupFolders;
 
-use Exception;
+use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException;
+use OCA\GroupFolders\Folder\FolderManager;
 use OCA\GroupFolders\Service\DelegationService;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Http\TemplateResponse;
 use OCP\AppFramework\Middleware;
 use OCP\AppFramework\Utility\IControllerMethodReflector;
 use OCP\IRequest;
+use OCP\IUserSession;
 
 class AuthorizedAdminSettingMiddleware extends Middleware {
-	private DelegationService $delegatedService;
-	private IControllerMethodReflector $reflector;
-	private IRequest $request;
-
 	public function __construct(
-		DelegationService $delegatedService,
-		IControllerMethodReflector $reflector,
-		IRequest $request
+		private DelegationService $delegatedService,
+		private IControllerMethodReflector $reflector,
+		private IRequest $request,
+		private IUserSession $userSession,
+		private FolderManager $folderManager,
 	) {
-		$this->delegatedService = $delegatedService;
-		$this->reflector = $reflector;
-		$this->request = $request;
 	}
 
 	/**
@@ -54,11 +51,24 @@ public function __construct(
 	 *
 	 */
 	public function beforeController($controller, $methodName) {
-		if ($this->reflector->hasAnnotation('RequireGroupFolderAdmin')) {
-			if (!$this->delegatedService->hasApiAccess()) {
-				throw new Exception('Logged in user must be an admin, a sub admin or gotten special right to access this setting');
+		if (!$this->reflector->hasAnnotation('RequireGroupFolderAdmin')) {
+			return;
+		}
+
+		if ($this->delegatedService->isAdminNextcloud() || $this->delegatedService->isDelegatedAdmin()) {
+			return;
+		}
+
+		if ($this->delegatedService->hasOnlyApiAccess()
+			&& ($user = $this->userSession->getUser()) !== null
+			&& ($id = $this->request->getParam('id')) !== null) {
+			/** @var string $id */
+			if ($this->folderManager->canManageACL((int)$id, $user)) {
+				return;
 			}
 		}
+
+		throw new NotAdminException('Logged in user must be an admin, a sub admin or gotten special right to access this setting');
 	}
 
 	/**

psalm.xml

@@ -11,6 +11,8 @@
 >
 	<stubs>
 		<file name="tests/stub.phpstub" preloadClasses="true"/>
+		<file name="tests/stubs/oc_appframework_middleware_security_exceptions_notadminexception.php" />
+		<file name="tests/stubs/oc_appframework_middleware_security_exceptions_securityexception.php" />
 	</stubs>
 	<projectFiles>
 		<directory name="lib" />

tests/phpunit.xml

@@ -19,6 +19,7 @@
   </coverage>
   <testsuite name="Nextcloud - Group folders App Tests">
     <directory suffix=".php">.</directory>
+	<exclude>./stubs/</exclude>
   </testsuite>
   <logging/>
 </phpunit>

tests/stubs/oc_appframework_middleware_security_exceptions_notadminexception.php

@@ -0,0 +1,21 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\AppFramework\Middleware\Security\Exceptions;
+
+/**
+ * Class NotAdminException is thrown when a resource has been requested by a
+ * non-admin user that is not accessible to non-admin users.
+ *
+ * @package OC\AppFramework\Middleware\Security\Exceptions
+ */
+class NotAdminException extends SecurityException {
+	public function __construct(string $message) {
+	}
+}

tests/stubs/oc_appframework_middleware_security_exceptions_securityexception.php

@@ -0,0 +1,19 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * SPDX-FileCopyrightText: 2016-2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-FileCopyrightText: 2016 ownCloud, Inc.
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\AppFramework\Middleware\Security\Exceptions;
+
+/**
+ * Class SecurityException is the base class for security exceptions thrown by
+ * the security middleware.
+ *
+ * @package OC\AppFramework\Middleware\Security\Exceptions
+ */
+class SecurityException extends \Exception {
+}