Merge pull request #16 from owncloud/bugfix/no-access-to-unpriviledge…

…d-users

Move subadmin middleware to app

lib/AppInfo/Application.php

@@ -0,0 +1,45 @@
+<?php
+/**
+ * @author Thomas Müller <thomas.mueller@tmit.eu>
+ *
+ * @copyright Copyright (c) 2018, ownCloud GmbH
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\UserManagement\AppInfo;
+
+
+use OCA\UserManagement\SubadminMiddleware;
+use OCP\AppFramework\App;
+use OCP\IContainer;
+
+class Application extends App {
+
+	function __construct(array $urlParams = []) {
+		parent::__construct('user_management', $urlParams);
+
+		/**
+		 * Middleware
+		 */
+		$this->getContainer()
+			->registerService('SubadminMiddleware', function(IContainer $c){
+				return 	$c->query(SubadminMiddleware::class);
+			});
+		// Execute middlewares
+		$this->getContainer()->registerMiddleWare('SubadminMiddleware');
+	}
+
+}

lib/SubadminMiddleware.php

@@ -0,0 +1,104 @@
+<?php
+/**
+ * @author Lukas Reschke <lukas@statuscode.ch>
+ * @author Morris Jobke <hey@morrisjobke.de>
+ * @author Roeland Jago Douma <rullzer@owncloud.com>
+ * @author Thomas Müller <thomas.mueller@tmit.eu>
+ *
+ * @copyright Copyright (c) 2018, ownCloud GmbH
+ * @license AGPL-3.0
+ *
+ * This code is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License, version 3,
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>
+ *
+ */
+
+namespace OCA\UserManagement;
+
+use OC\AppFramework\Http;
+use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException;
+use OC\AppFramework\Utility\ControllerMethodReflector;
+use OCP\AppFramework\Http\TemplateResponse;
+use OCP\AppFramework\Middleware;
+use OCP\IGroupManager;
+use OCP\IUserSession;
+
+/**
+ * Verifies whether an user has at least subadmin rights.
+ * To bypass use the `@NoSubadminRequired` annotation
+ *
+ * @package OC\Settings\Middleware
+ */
+class SubadminMiddleware extends Middleware {
+	/** @var IUserSession */
+	private $userSession;
+	/** @var IGroupManager */
+	private $groupManager;
+	/** @var ControllerMethodReflector */
+	protected $reflector;
+
+	/**
+	 * @param ControllerMethodReflector $reflector
+	 * @param IGroupManager $groupManager
+	 * @param IUserSession $userSession
+	 */
+	public function __construct(ControllerMethodReflector $reflector,
+								IGroupManager $groupManager,
+								IUserSession $userSession) {
+		$this->reflector = $reflector;
+		$this->groupManager = $groupManager;
+		$this->userSession = $userSession;
+	}
+
+	/**
+	 * Check if sharing is enabled before the controllers is executed
+	 * @param \OCP\AppFramework\Controller $controller
+	 * @param string $methodName
+	 * @throws \Exception
+	 */
+	public function beforeController($controller, $methodName) {
+		if(!$this->reflector->hasAnnotation('NoSubadminRequired')) {
+			// Check if current user (active and not in incognito mode)
+			// can manage users
+			$hasUserManagementPrivileges = false;
+			$activeUser = $this->userSession->getUser();
+			if($activeUser !== null) {
+				//Admin and SubAdmins are allowed to access user management
+				$hasUserManagementPrivileges = $this->groupManager->isAdmin($activeUser->getUID())
+					|| $this->groupManager->getSubAdmin()->isSubAdmin($activeUser);
+			}
+
+			if(!$hasUserManagementPrivileges) {
+				throw new NotAdminException('Logged in user must be a subadmin');
+			}
+		}
+	}
+
+	/**
+	 * Return 403 page in case of an exception
+	 * @param \OCP\AppFramework\Controller $controller
+	 * @param string $methodName
+	 * @param \Exception $exception
+	 * @return TemplateResponse
+	 * @throws \Exception
+	 */
+	public function afterException($controller, $methodName, \Exception $exception) {
+		if($exception instanceof NotAdminException) {
+			$response = new TemplateResponse('core', '403', [], 'guest');
+			$response->setStatus(Http::STATUS_FORBIDDEN);
+			return $response;
+		}
+
+		throw $exception;
+	}
+
+}

tests/unit/SubadminMiddlewareTest.php

@@ -0,0 +1,145 @@
+<?php
+/**
+ * @author Lukas Reschke
+ * @copyright Copyright (c) 2014 Lukas Reschke lukas@owncloud.com
+ *
+ * This file is licensed under the Affero General Public License version 3 or
+ * later.
+ * See the COPYING-README file.
+ */
+
+namespace OCA\UserManagement\Test\Unit;
+
+use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException;
+use OC\AppFramework\Utility\ControllerMethodReflector;
+use OCA\UserManagement\SubadminMiddleware;
+use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http\TemplateResponse;
+use \OCP\IGroupManager;
+use \OCP\ISubAdminManager;
+use \OCP\IUserSession;
+
+/**
+ * Verifies whether an user has at least subadmin rights.
+ * To bypass use the `@NoSubadminRequired` annotation
+ *
+ * @package Tests\Settings\Middleware
+ */
+class SubadminMiddlewareTest extends \Test\TestCase {
+	/** @var SubadminMiddleware */
+	private $subadminMiddleware;
+	/** @var ControllerMethodReflector */
+	private $reflector;
+	/** @var Controller */
+	private $controller;
+	/** @var \OCP\IGroupManager */
+	private $groupManager;
+	/** @var \OCP\ISubAdminManager */
+	private $subadminManager;
+	/** @var \OCP\IUserSession */
+	private $session;
+
+	protected function setUp() {
+		$this->groupManager = $this->getMockBuilder(IGroupManager::class)
+			->disableOriginalConstructor()->getMock();
+
+		$this->subadminManager = $this->getMockBuilder(ISubAdminManager::class)
+			->disableOriginalConstructor()->getMock();
+		$this->groupManager->expects($this->any())
+			->method('getSubAdmin')
+			->will($this->returnValue($this->subadminManager));
+		$this->groupManager->expects($this->any())
+			->method('isAdmin')
+			->will($this->returnValue(false));
+
+		$this->session = $this->getMockBuilder(IUserSession::class)
+			->disableOriginalConstructor()->getMock();
+		$user = $this->getMockBuilder('\OC\User\User')
+			->disableOriginalConstructor()->getMock();
+		$user->expects($this->any())
+			->method('getUID')
+			->will($this->returnValue('foo'));
+		$this->session
+			->expects($this->any())
+			->method('getUser')
+			->will($this->returnValue($user));
+
+		$this->reflector = $this->getMockBuilder(ControllerMethodReflector::class)
+			->disableOriginalConstructor()->getMock();
+		$this->controller = $this->getMockBuilder(Controller::class)
+			->disableOriginalConstructor()->getMock();
+
+		$this->subadminMiddleware = new SubadminMiddleware($this->reflector, $this->groupManager, $this->session);
+	}
+
+	/**
+	 * @expectedException \OC\AppFramework\Middleware\Security\Exceptions\NotAdminException
+	 */
+	public function testBeforeControllerAsUserWithExemption() {
+		$this->reflector
+			->expects($this->once())
+			->method('hasAnnotation')
+			->with('NoSubadminRequired')
+			->will($this->returnValue(false));
+
+		$this->subadminManager->expects($this->any())
+			->method('isSubAdmin')
+			->will($this->returnValue(false));
+		$this->subadminMiddleware->beforeController($this->controller, 'foo');
+	}
+
+
+	public function testBeforeControllerAsUserWithoutExemption() {
+		$this->reflector
+			->expects($this->once())
+			->method('hasAnnotation')
+			->with('NoSubadminRequired')
+			->will($this->returnValue(true));
+
+		$this->subadminManager->expects($this->any())
+			->method('isSubAdmin')
+			->will($this->returnValue(false));
+		$this->subadminMiddleware->beforeController($this->controller, 'foo');
+	}
+
+	public function testBeforeControllerAsSubAdminWithoutExemption() {
+		$this->reflector
+			->expects($this->once())
+			->method('hasAnnotation')
+			->with('NoSubadminRequired')
+			->will($this->returnValue(false));
+
+		$this->subadminManager->expects($this->any())
+			->method('isSubAdmin')
+			->will($this->returnValue(true));
+		$this->subadminMiddleware->beforeController($this->controller, 'foo');
+	}
+
+	public function testBeforeControllerAsSubAdminWithExemption() {
+		$this->reflector
+			->expects($this->once())
+			->method('hasAnnotation')
+			->with('NoSubadminRequired')
+			->will($this->returnValue(true));
+
+		$this->subadminManager->expects($this->any())
+			->method('isSubAdmin')
+			->will($this->returnValue(true));
+		$this->subadminMiddleware->beforeController($this->controller, 'foo');
+	}
+
+	public function testAfterNotAdminException() {
+		$expectedResponse = new TemplateResponse('core', '403', [], 'guest');
+		$expectedResponse->setStatus(403);
+		$this->assertEquals($expectedResponse, $this->subadminMiddleware->afterException($this->controller, 'foo', new NotAdminException()));
+	}
+
+	/**
+	 * @expectedException \Exception
+	 */
+	public function testAfterRegularException() {
+		$expectedResponse = new TemplateResponse('core', '403', [], 'guest');
+		$expectedResponse->setStatus(403);
+		$this->subadminMiddleware->afterException($this->controller, 'foo', new \Exception());
+	}
+}