Merge pull request #22551 from owncloud/make-exceptions-easier-to-debug

Throw normal exceptions instead of eating them

settings/middleware/subadminmiddleware.php

@@ -23,6 +23,7 @@
 namespace OC\Settings\Middleware;
 
 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;
@@ -58,7 +59,7 @@ class SubadminMiddleware extends Middleware {
 	public function beforeController($controller, $methodName) {
 		if(!$this->reflector->hasAnnotation('NoSubadminRequired')) {
 			if(!$this->isSubAdmin) {
-				throw new \Exception('Logged in user must be a subadmin');
+				throw new NotAdminException('Logged in user must be a subadmin');
 			}
 		}
 	}
@@ -69,11 +70,16 @@ class SubadminMiddleware extends Middleware {
 	 * @param string $methodName
 	 * @param \Exception $exception
 	 * @return TemplateResponse
+	 * @throws \Exception
 	 */
 	public function afterException($controller, $methodName, \Exception $exception) {
-		$response = new TemplateResponse('core', '403', array(), 'guest');
-		$response->setStatus(Http::STATUS_FORBIDDEN);
-		return $response;
+		if($exception instanceof NotAdminException) {
+			$response = new TemplateResponse('core', '403', array(), 'guest');
+			$response->setStatus(Http::STATUS_FORBIDDEN);
+			return $response;
+		}
+
+		throw $exception;
 	}
 
 }

tests/settings/middleware/subadminmiddlewaretest.php

@@ -10,6 +10,7 @@
 
 namespace OC\Settings\Middleware;
 
+use OC\Appframework\Middleware\Security\Exceptions\NotAdminException;
 use OC\AppFramework\Utility\ControllerMethodReflector;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http\TemplateResponse;
@@ -41,8 +42,7 @@ class SubadminMiddlewareTest extends \Test\TestCase {
 	}
 
 	/**
-	 * @expectedException \Exception
-	 * @expectedExceptionMessage Logged in user must be a subadmin
+	 * @expectedException \OC\Appframework\Middleware\Security\Exceptions\NotAdminException
 	 */
 	public function testBeforeControllerAsUserWithExemption() {
 		$this->reflector
@@ -81,9 +81,18 @@ class SubadminMiddlewareTest extends \Test\TestCase {
 		$this->subadminMiddlewareAsSubAdmin->beforeController($this->controller, 'foo');
 	}
 
-	public function testAfterException() {
+	public function testAfterNotAdminException() {
+		$expectedResponse = new TemplateResponse('core', '403', array(), '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', array(), 'guest');
 		$expectedResponse->setStatus(403);
-		$this->assertEquals($expectedResponse, $this->subadminMiddleware->afterException($this->controller, 'foo', new \Exception()));
+		$this->subadminMiddleware->afterException($this->controller, 'foo', new \Exception());
 	}
 }