diff --git a/lib/private/appframework/middleware/security/corsmiddleware.php b/lib/private/appframework/middleware/security/corsmiddleware.php
index 74b0dd0997..0e37e81c5b 100644
--- a/lib/private/appframework/middleware/security/corsmiddleware.php
+++ b/lib/private/appframework/middleware/security/corsmiddleware.php
@@ -23,6 +23,7 @@
namespace OC\AppFramework\Middleware\Security;
+use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OCP\AppFramework\Controller;
use OCP\AppFramework\Http;
diff --git a/lib/private/appframework/middleware/security/exceptions/appnotenabledexception.php b/lib/private/appframework/middleware/security/exceptions/appnotenabledexception.php
new file mode 100644
index 0000000000..54cb08f438
--- /dev/null
+++ b/lib/private/appframework/middleware/security/exceptions/appnotenabledexception.php
@@ -0,0 +1,36 @@
+
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @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
+ *
+ */
+
+namespace OC\Appframework\Middleware\Security\Exceptions;
+
+use OCP\AppFramework\Http;
+
+/**
+ * Class AppNotEnabledException is thrown when a resource for an application is
+ * requested that is not enabled.
+ *
+ * @package OC\Appframework\Middleware\Security\Exceptions
+ */
+class AppNotEnabledException extends SecurityException {
+ public function __construct() {
+ parent::__construct('App is not enabled', Http::STATUS_PRECONDITION_FAILED);
+ }
+}
diff --git a/lib/private/appframework/middleware/security/exceptions/crosssiterequestforgeryexception.php b/lib/private/appframework/middleware/security/exceptions/crosssiterequestforgeryexception.php
new file mode 100644
index 0000000000..c59c921f5e
--- /dev/null
+++ b/lib/private/appframework/middleware/security/exceptions/crosssiterequestforgeryexception.php
@@ -0,0 +1,36 @@
+
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @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
+ *
+ */
+
+namespace OC\Appframework\Middleware\Security\Exceptions;
+
+use OCP\AppFramework\Http;
+
+/**
+ * Class CrossSiteRequestForgeryException is thrown when a CSRF exception has
+ * been encountered.
+ *
+ * @package OC\Appframework\Middleware\Security\Exceptions
+ */
+class CrossSiteRequestForgeryException extends SecurityException {
+ public function __construct() {
+ parent::__construct('CSRF check failed', Http::STATUS_PRECONDITION_FAILED);
+ }
+}
diff --git a/lib/private/appframework/middleware/security/exceptions/notadminexception.php b/lib/private/appframework/middleware/security/exceptions/notadminexception.php
new file mode 100644
index 0000000000..3fa03cae89
--- /dev/null
+++ b/lib/private/appframework/middleware/security/exceptions/notadminexception.php
@@ -0,0 +1,36 @@
+
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @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
+ *
+ */
+
+namespace OC\Appframework\Middleware\Security\Exceptions;
+
+use OCP\AppFramework\Http;
+
+/**
+ * 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() {
+ parent::__construct('Logged in user must be an admin', Http::STATUS_FORBIDDEN);
+ }
+}
diff --git a/lib/private/appframework/middleware/security/exceptions/notloggedinexception.php b/lib/private/appframework/middleware/security/exceptions/notloggedinexception.php
new file mode 100644
index 0000000000..5f27625aa5
--- /dev/null
+++ b/lib/private/appframework/middleware/security/exceptions/notloggedinexception.php
@@ -0,0 +1,36 @@
+
+ *
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @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
+ *
+ */
+
+namespace OC\Appframework\Middleware\Security\Exceptions;
+
+use OCP\AppFramework\Http;
+
+/**
+ * Class NotLoggedInException is thrown when a resource has been requested by a
+ * guest user that is not accessible to the public.
+ *
+ * @package OC\Appframework\Middleware\Security\Exceptions
+ */
+class NotLoggedInException extends SecurityException {
+ public function __construct() {
+ parent::__construct('Current user is not logged in', Http::STATUS_UNAUTHORIZED);
+ }
+}
diff --git a/lib/private/appframework/middleware/security/securityexception.php b/lib/private/appframework/middleware/security/exceptions/securityexception.php
similarity index 63%
rename from lib/private/appframework/middleware/security/securityexception.php
rename to lib/private/appframework/middleware/security/exceptions/securityexception.php
index 6f96b9a1d8..9b99282ce8 100644
--- a/lib/private/appframework/middleware/security/securityexception.php
+++ b/lib/private/appframework/middleware/security/exceptions/securityexception.php
@@ -1,7 +1,6 @@
- * @author Thomas Müller
+ * @author Lukas Reschke
*
* @copyright Copyright (c) 2015, ownCloud, Inc.
* @license AGPL-3.0
@@ -20,20 +19,12 @@
*
*/
-
-namespace OC\AppFramework\Middleware\Security;
-
+namespace OC\AppFramework\Middleware\Security\Exceptions;
/**
- * Thrown when the security middleware encounters a security problem
+ * Class SecurityException is the base class for security exceptions thrown by
+ * the security middleware.
+ *
+ * @package OC\AppFramework\Middleware\Security\Exceptions
*/
-class SecurityException extends \Exception {
-
- /**
- * @param string $msg the security error message
- */
- public function __construct($msg, $code = 0) {
- parent::__construct($msg, $code);
- }
-
-}
+class SecurityException extends \Exception {}
diff --git a/lib/private/appframework/middleware/security/securitymiddleware.php b/lib/private/appframework/middleware/security/securitymiddleware.php
index 34c626ce8b..d0b7202a36 100644
--- a/lib/private/appframework/middleware/security/securitymiddleware.php
+++ b/lib/private/appframework/middleware/security/securitymiddleware.php
@@ -28,8 +28,13 @@
namespace OC\AppFramework\Middleware\Security;
use OC\AppFramework\Http;
+use OC\Appframework\Middleware\Security\Exceptions\AppNotEnabledException;
+use OC\Appframework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException;
+use OC\Appframework\Middleware\Security\Exceptions\NotAdminException;
+use OC\Appframework\Middleware\Security\Exceptions\NotLoggedInException;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OCP\AppFramework\Http\RedirectResponse;
+use OCP\AppFramework\Http\TemplateResponse;
use OCP\AppFramework\Middleware;
use OCP\AppFramework\Http\Response;
use OCP\AppFramework\Http\JSONResponse;
@@ -39,7 +44,7 @@ use OCP\IRequest;
use OCP\ILogger;
use OCP\AppFramework\Controller;
use OCP\Util;
-
+use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
/**
* Used to do all the authentication and checking stuff for a controller method
@@ -75,7 +80,7 @@ class SecurityMiddleware extends Middleware {
ILogger $logger,
$appName,
$isLoggedIn,
- $isAdminUser){
+ $isAdminUser) {
$this->navigationManager = $navigationManager;
$this->request = $request;
$this->reflector = $reflector;
@@ -95,7 +100,7 @@ class SecurityMiddleware extends Middleware {
* @param string $methodName the name of the method
* @throws SecurityException when a security check fails
*/
- public function beforeController($controller, $methodName){
+ public function beforeController($controller, $methodName) {
// this will set the current navigation entry of the app, use this only
// for normal HTML requests and not for AJAX requests
@@ -105,12 +110,12 @@ class SecurityMiddleware extends Middleware {
$isPublicPage = $this->reflector->hasAnnotation('PublicPage');
if(!$isPublicPage) {
if(!$this->isLoggedIn) {
- throw new SecurityException('Current user is not logged in', Http::STATUS_UNAUTHORIZED);
+ throw new NotLoggedInException();
}
if(!$this->reflector->hasAnnotation('NoAdminRequired')) {
if(!$this->isAdminUser) {
- throw new SecurityException('Logged in user must be an admin', Http::STATUS_FORBIDDEN);
+ throw new NotAdminException();
}
}
}
@@ -119,18 +124,18 @@ class SecurityMiddleware extends Middleware {
Util::callRegister();
if(!$this->reflector->hasAnnotation('NoCSRFRequired')) {
if(!$this->request->passesCSRFCheck()) {
- throw new SecurityException('CSRF check failed', Http::STATUS_PRECONDITION_FAILED);
+ throw new CrossSiteRequestForgeryException();
}
}
/**
* FIXME: Use DI once available
- * Checks if app is enabled (also inclues a check whether user is allowed to access the resource)
+ * Checks if app is enabled (also includes a check whether user is allowed to access the resource)
* The getAppPath() check is here since components such as settings also use the AppFramework and
* therefore won't pass this check.
*/
if(\OC_App::getAppPath($this->appName) !== false && !\OC_App::isEnabled($this->appName)) {
- throw new SecurityException('App is not enabled', Http::STATUS_PRECONDITION_FAILED);
+ throw new AppNotEnabledException();
}
}
@@ -146,28 +151,28 @@ class SecurityMiddleware extends Middleware {
* @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){
-
- if (stripos($this->request->getHeader('Accept'),'html')===false) {
+ public function afterException($controller, $methodName, \Exception $exception) {
+ if($exception instanceof SecurityException) {
+ if (stripos($this->request->getHeader('Accept'),'html') === false) {
$response = new JSONResponse(
array('message' => $exception->getMessage()),
$exception->getCode()
);
- $this->logger->debug($exception->getMessage());
} else {
-
- // TODO: replace with link to route
- $url = $this->urlGenerator->getAbsoluteURL('index.php');
- // add redirect URL to redirect to the previous page after login
- $url .= '?redirect_url=' . urlencode($this->request->server['REQUEST_URI']);
- $response = new RedirectResponse($url);
- $this->logger->debug($exception->getMessage());
+ if($exception instanceof NotLoggedInException) {
+ // TODO: replace with link to route
+ $url = $this->urlGenerator->getAbsoluteURL('index.php');
+ $url .= '?redirect_url=' . urlencode($this->request->server['REQUEST_URI']);
+ $response = new RedirectResponse($url);
+ } else {
+ $response = new TemplateResponse('core', '403', ['file' => $exception->getMessage()], 'guest');
+ $response->setStatus($exception->getCode());
+ }
}
+ $this->logger->debug($exception->getMessage());
return $response;
-
}
throw $exception;
diff --git a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php
index ca526fb859..cf5f97a046 100644
--- a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php
+++ b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php
@@ -14,7 +14,7 @@ namespace OC\AppFramework\Middleware\Security;
use OC\AppFramework\Http\Request;
use OC\AppFramework\Utility\ControllerMethodReflector;
-
+use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
use OCP\AppFramework\Http;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\Response;
@@ -91,7 +91,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
/**
* @CORS
- * @expectedException \OC\AppFramework\Middleware\Security\SecurityException
+ * @expectedException \OC\AppFramework\Middleware\Security\Exceptions\SecurityException
*/
public function testCorsIgnoredIfWithCredentialsHeaderPresent() {
$request = new Request(
@@ -160,7 +160,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
/**
* @CORS
- * @expectedException \OC\AppFramework\Middleware\Security\SecurityException
+ * @expectedException \OC\AppFramework\Middleware\Security\Exceptions\SecurityException
*/
public function testCORSShouldNotAllowCookieAuth() {
$request = new Request(
diff --git a/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php b/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php
index 347a0423ea..62223bbc2d 100644
--- a/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php
+++ b/tests/lib/appframework/middleware/security/SecurityMiddlewareTest.php
@@ -1,34 +1,40 @@
+ * @author Lukas Reschke
*
- * @author Bernhard Posselt
- * @copyright 2012 Bernhard Posselt
+ * @copyright Copyright (c) 2015, ownCloud, Inc.
+ * @license AGPL-3.0
*
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU AFFERO GENERAL PUBLIC LICENSE
- * License as published by the Free Software Foundation; either
- * version 3 of the License, or any later version.
+ * 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 library is distributed in the hope that it will be useful,
+ * 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.
+ * 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 along with this library. If not, see .
+ * You should have received a copy of the GNU Affero General Public License, version 3,
+ * along with this program. If not, see
*
*/
+
namespace OC\AppFramework\Middleware\Security;
use OC\AppFramework\Http;
use OC\AppFramework\Http\Request;
+use OC\Appframework\Middleware\Security\Exceptions\AppNotEnabledException;
+use OC\Appframework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException;
+use OC\Appframework\Middleware\Security\Exceptions\NotAdminException;
+use OC\Appframework\Middleware\Security\Exceptions\NotLoggedInException;
+use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
use OC\AppFramework\Utility\ControllerMethodReflector;
use OCP\AppFramework\Http\RedirectResponse;
use OCP\AppFramework\Http\JSONResponse;
+use OCP\AppFramework\Http\TemplateResponse;
class SecurityMiddlewareTest extends \Test\TestCase {
@@ -71,8 +77,12 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->secAjaxException = new SecurityException('hey', true);
}
-
- private function getMiddleware($isLoggedIn, $isAdminUser){
+ /**
+ * @param bool $isLoggedIn
+ * @param bool $isAdminUser
+ * @return SecurityMiddleware
+ */
+ private function getMiddleware($isLoggedIn, $isAdminUser) {
return new SecurityMiddleware(
$this->request,
$this->reader,
@@ -219,8 +229,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$sec = $this->getMiddleware($isLoggedIn, $isAdminUser);
- if($shouldFail){
- $this->setExpectedException('\OC\AppFramework\Middleware\Security\SecurityException');
+ if($shouldFail) {
+ $this->setExpectedException('\OC\AppFramework\Middleware\Security\Exceptions\SecurityException');
} else {
$this->assertTrue(true);
}
@@ -232,7 +242,7 @@ class SecurityMiddlewareTest extends \Test\TestCase {
/**
* @PublicPage
- * @expectedException \OC\AppFramework\Middleware\Security\SecurityException
+ * @expectedException \OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException
*/
public function testCsrfCheck(){
$this->request->expects($this->once())
@@ -311,25 +321,85 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->middleware->afterException($this->controller, 'test', $ex);
}
-
- public function testAfterExceptionReturnsRedirect(){
+ public function testAfterExceptionReturnsRedirectForNotLoggedInUser() {
$this->request = new Request(
- [
- 'server' =>
[
- 'HTTP_ACCEPT' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
- 'REQUEST_URI' => 'owncloud/index.php/apps/specialapp'
- ]
- ],
- $this->getMock('\OCP\Security\ISecureRandom'),
- $this->getMock('\OCP\IConfig')
+ 'server' =>
+ [
+ 'HTTP_ACCEPT' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
+ 'REQUEST_URI' => 'owncloud/index.php/apps/specialapp'
+ ]
+ ],
+ $this->getMock('\OCP\Security\ISecureRandom'),
+ $this->getMock('\OCP\IConfig')
+ );
+ $this->middleware = $this->getMiddleware(false, false);
+ $this->urlGenerator
+ ->expects($this->once())
+ ->method('getAbsoluteURL')
+ ->with('index.php')
+ ->will($this->returnValue('http://localhost/index.php'));
+ $this->logger
+ ->expects($this->once())
+ ->method('debug')
+ ->with('Current user is not logged in');
+ $response = $this->middleware->afterException(
+ $this->controller,
+ 'test',
+ new NotLoggedInException()
);
- $this->middleware = $this->getMiddleware(true, true);
- $response = $this->middleware->afterException($this->controller, 'test',
- $this->secException);
- $this->assertTrue($response instanceof RedirectResponse);
- $this->assertEquals('?redirect_url=owncloud%2Findex.php%2Fapps%2Fspecialapp', $response->getRedirectURL());
+ $expected = new RedirectResponse('http://localhost/index.php?redirect_url=owncloud%2Findex.php%2Fapps%2Fspecialapp');
+ $this->assertEquals($expected , $response);
+ }
+
+ /**
+ * @return array
+ */
+ public function exceptionProvider() {
+ return [
+ [
+ new AppNotEnabledException(),
+ ],
+ [
+ new CrossSiteRequestForgeryException(),
+ ],
+ [
+ new NotAdminException(),
+ ],
+ ];
+ }
+
+ /**
+ * @dataProvider exceptionProvider
+ * @param SecurityException $exception
+ */
+ public function testAfterExceptionReturnsTemplateResponse(SecurityException $exception) {
+ $this->request = new Request(
+ [
+ 'server' =>
+ [
+ 'HTTP_ACCEPT' => 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8',
+ 'REQUEST_URI' => 'owncloud/index.php/apps/specialapp'
+ ]
+ ],
+ $this->getMock('\OCP\Security\ISecureRandom'),
+ $this->getMock('\OCP\IConfig')
+ );
+ $this->middleware = $this->getMiddleware(false, false);
+ $this->logger
+ ->expects($this->once())
+ ->method('debug')
+ ->with($exception->getMessage());
+ $response = $this->middleware->afterException(
+ $this->controller,
+ 'test',
+ $exception
+ );
+
+ $expected = new TemplateResponse('core', '403', ['file' => $exception->getMessage()], 'guest');
+ $expected->setStatus($exception->getCode());
+ $this->assertEquals($expected , $response);
}