diff --git a/lib/private/appframework/middleware/security/corsmiddleware.php b/lib/private/appframework/middleware/security/corsmiddleware.php index 600eb2318c..d7c42cd9b1 100644 --- a/lib/private/appframework/middleware/security/corsmiddleware.php +++ b/lib/private/appframework/middleware/security/corsmiddleware.php @@ -2,6 +2,7 @@ /** * @author Bernhard Posselt * @author Morris Jobke + * @author Lukas Reschke * * @copyright Copyright (c) 2015, ownCloud, Inc. * @license AGPL-3.0 @@ -23,6 +24,9 @@ namespace OC\AppFramework\Middleware\Security; use OC\AppFramework\Utility\ControllerMethodReflector; +use OCP\AppFramework\Controller; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\JSONResponse; use OCP\IRequest; use OCP\IUserSession; use OCP\AppFramework\Http\Response; @@ -57,8 +61,8 @@ class CORSMiddleware extends Middleware { * @param IUserSession $session */ public function __construct(IRequest $request, - ControllerMethodReflector $reflector, - IUserSession $session) { + ControllerMethodReflector $reflector, + IUserSession $session) { $this->request = $request; $this->reflector = $reflector; $this->session = $session; @@ -71,6 +75,7 @@ class CORSMiddleware extends Middleware { * @param Controller $controller the controller that is being called * @param string $methodName the name of the method that will be called on * the controller + * @throws SecurityException * @since 6.0.0 */ public function beforeController($controller, $methodName){ @@ -83,7 +88,7 @@ class CORSMiddleware extends Middleware { $this->session->logout(); if(!$this->session->login($user, $pass)) { - throw new SecurityException('CORS requires basic auth'); + throw new SecurityException('CORS requires basic auth', Http::STATUS_UNAUTHORIZED); } } } @@ -97,6 +102,7 @@ class CORSMiddleware extends Middleware { * the controller * @param Response $response the generated response from the controller * @return Response a Response object + * @throws SecurityException */ public function afterController($controller, $methodName, Response $response){ // only react if its a CORS request and if the request sends origin and @@ -106,7 +112,7 @@ class CORSMiddleware extends Middleware { // allow credentials headers must not be true or CSRF is possible // otherwise - foreach($response->getHeaders() as $header => $value ) { + foreach($response->getHeaders() as $header => $value) { if(strtolower($header) === 'access-control-allow-credentials' && strtolower(trim($value)) === 'true') { $msg = 'Access-Control-Allow-Credentials must not be '. @@ -121,5 +127,28 @@ class CORSMiddleware extends Middleware { return $response; } + /** + * If an SecurityException is being caught return a JSON error response + * + * @param Controller $controller the controller that is being called + * @param string $methodName the name of the method that will be called on + * the controller + * @param \Exception $exception the thrown exception + * @throws \Exception the passed in exception if it cant handle it + * @return Response a Response object or null in case that the exception could not be handled + */ + public function afterException($controller, $methodName, \Exception $exception){ + if($exception instanceof SecurityException){ + $response = new JSONResponse(['message' => $exception->getMessage()]); + if($exception->getCode() !== 0) { + $response->setStatus($exception->getCode()); + } else { + $response->setStatus(Http::STATUS_INTERNAL_SERVER_ERROR); + } + return $response; + } + + throw $exception; + } } diff --git a/lib/private/appframework/middleware/security/securitymiddleware.php b/lib/private/appframework/middleware/security/securitymiddleware.php index 715fb2457d..34c626ce8b 100644 --- a/lib/private/appframework/middleware/security/securitymiddleware.php +++ b/lib/private/appframework/middleware/security/securitymiddleware.php @@ -69,13 +69,13 @@ class SecurityMiddleware extends Middleware { * @param bool $isAdminUser */ public function __construct(IRequest $request, - ControllerMethodReflector $reflector, - INavigationManager $navigationManager, - IURLGenerator $urlGenerator, - ILogger $logger, - $appName, - $isLoggedIn, - $isAdminUser){ + ControllerMethodReflector $reflector, + INavigationManager $navigationManager, + IURLGenerator $urlGenerator, + ILogger $logger, + $appName, + $isLoggedIn, + $isAdminUser){ $this->navigationManager = $navigationManager; $this->request = $request; $this->reflector = $reflector; diff --git a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php index 5c93c95e18..ca526fb859 100644 --- a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php +++ b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php @@ -15,6 +15,8 @@ namespace OC\AppFramework\Middleware\Security; use OC\AppFramework\Http\Request; use OC\AppFramework\Utility\ControllerMethodReflector; +use OCP\AppFramework\Http; +use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\Response; @@ -181,4 +183,53 @@ class CORSMiddlewareTest extends \Test\TestCase { $middleware->beforeController($this, __FUNCTION__, new Response()); } + public function testAfterExceptionWithSecurityExceptionNoStatus() { + $request = new Request( + ['server' => [ + 'PHP_AUTH_USER' => 'user', + 'PHP_AUTH_PW' => 'pass' + ]], + $this->getMock('\OCP\Security\ISecureRandom'), + $this->getMock('\OCP\IConfig') + ); + $middleware = new CORSMiddleware($request, $this->reflector, $this->session); + $response = $middleware->afterException($this, __FUNCTION__, new SecurityException('A security exception')); + + $expected = new JSONResponse(['message' => 'A security exception'], 500); + $this->assertEquals($expected, $response); + } + + public function testAfterExceptionWithSecurityExceptionWithStatus() { + $request = new Request( + ['server' => [ + 'PHP_AUTH_USER' => 'user', + 'PHP_AUTH_PW' => 'pass' + ]], + $this->getMock('\OCP\Security\ISecureRandom'), + $this->getMock('\OCP\IConfig') + ); + $middleware = new CORSMiddleware($request, $this->reflector, $this->session); + $response = $middleware->afterException($this, __FUNCTION__, new SecurityException('A security exception', 501)); + + $expected = new JSONResponse(['message' => 'A security exception'], 501); + $this->assertEquals($expected, $response); + } + + /** + * @expectedException \Exception + * @expectedExceptionMessage A regular exception + */ + public function testAfterExceptionWithRegularException() { + $request = new Request( + ['server' => [ + 'PHP_AUTH_USER' => 'user', + 'PHP_AUTH_PW' => 'pass' + ]], + $this->getMock('\OCP\Security\ISecureRandom'), + $this->getMock('\OCP\IConfig') + ); + $middleware = new CORSMiddleware($request, $this->reflector, $this->session); + $middleware->afterException($this, __FUNCTION__, new \Exception('A regular exception')); + } + }