From 6c96b3d07f7248d1ab4066dbf71025753de1a0a9 Mon Sep 17 00:00:00 2001 From: Lukas Reschke Date: Mon, 22 Feb 2016 09:41:56 +0100 Subject: [PATCH] Throw normal exceptions instead of eating them Partially addresses https://github.com/owncloud/core/issues/22550 Replaces https://github.com/owncloud/core/pull/20185 --- settings/middleware/subadminmiddleware.php | 14 ++++++++++---- .../middleware/subadminmiddlewaretest.php | 17 +++++++++++++---- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/settings/middleware/subadminmiddleware.php b/settings/middleware/subadminmiddleware.php index 00f221721a..8e138bdc1a 100644 --- a/settings/middleware/subadminmiddleware.php +++ b/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; } } diff --git a/tests/settings/middleware/subadminmiddlewaretest.php b/tests/settings/middleware/subadminmiddlewaretest.php index d0da19f60e..2b76e4beaa 100644 --- a/tests/settings/middleware/subadminmiddlewaretest.php +++ b/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 \Exception())); + $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->subadminMiddleware->afterException($this->controller, 'foo', new \Exception()); } }