[Pkg-owncloud-commits] [owncloud] 120/457: disallow cookie auth for cors requests

David Prévot taffit at moszumanska.debian.org
Sun Jun 28 20:05:43 UTC 2015


This is an automated email from the git hooks/post-receive script.

taffit pushed a commit to branch stable8
in repository owncloud.

commit c8e3599cad9c5174260fc1dbe340efac65f1d646
Author: Bernhard Posselt <dev at bernhard-posselt.com>
Date:   Fri May 22 13:17:27 2015 +0200

    disallow cookie auth for cors requests
    
    testing ...
    
    fixes
    
    fix test
    
    add php doc
    
    fix small mistake
    
    add another phpdoc
    
    remove not working cors annotations from files app
---
 apps/files/controller/apicontroller.php            |  2 -
 .../dependencyinjection/dicontainer.php            |  5 +-
 .../middleware/security/corsmiddleware.php         | 49 ++++++++++++--
 .../middleware/security/CORSMiddlewareTest.php     | 75 ++++++++++++++++++++--
 4 files changed, 118 insertions(+), 13 deletions(-)

diff --git a/apps/files/controller/apicontroller.php b/apps/files/controller/apicontroller.php
index 8fc22a8..0cc222d 100644
--- a/apps/files/controller/apicontroller.php
+++ b/apps/files/controller/apicontroller.php
@@ -92,7 +92,6 @@ class ApiController extends Controller {
 	 * replace the actual tag selection.
 	 *
 	 * @NoAdminRequired
-	 * @CORS
 	 *
 	 * @param string $path path
 	 * @param array|string $tags array of tags
@@ -126,7 +125,6 @@ class ApiController extends Controller {
 	 * Returns a list of all files tagged with the given tag.
 	 *
 	 * @NoAdminRequired
-	 * @CORS
 	 *
 	 * @param array|string $tagName tag name to filter by
 	 * @return DataResponse
diff --git a/lib/private/appframework/dependencyinjection/dicontainer.php b/lib/private/appframework/dependencyinjection/dicontainer.php
index 8501fc6..a11e4ee 100644
--- a/lib/private/appframework/dependencyinjection/dicontainer.php
+++ b/lib/private/appframework/dependencyinjection/dicontainer.php
@@ -291,7 +291,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
 		$this->registerService('CORSMiddleware', function($c) {
 			return new CORSMiddleware(
 				$c['Request'],
-				$c['ControllerMethodReflector']
+				$c['ControllerMethodReflector'],
+				$c['OCP\IUserSession']
 			);
 		});
 
@@ -306,8 +307,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
 		$middleWares = &$this->middleWares;
 		$this->registerService('MiddlewareDispatcher', function($c) use (&$middleWares) {
 			$dispatcher = new MiddlewareDispatcher();
-			$dispatcher->registerMiddleware($c['SecurityMiddleware']);
 			$dispatcher->registerMiddleware($c['CORSMiddleware']);
+			$dispatcher->registerMiddleware($c['SecurityMiddleware']);
 
 			foreach($middleWares as $middleWare) {
 				$dispatcher->registerMiddleware($c[$middleWare]);
diff --git a/lib/private/appframework/middleware/security/corsmiddleware.php b/lib/private/appframework/middleware/security/corsmiddleware.php
index 9837428..600eb23 100644
--- a/lib/private/appframework/middleware/security/corsmiddleware.php
+++ b/lib/private/appframework/middleware/security/corsmiddleware.php
@@ -24,30 +24,69 @@ namespace OC\AppFramework\Middleware\Security;
 
 use OC\AppFramework\Utility\ControllerMethodReflector;
 use OCP\IRequest;
+use OCP\IUserSession;
 use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Middleware;
 
 /**
- * This middleware sets the correct CORS headers on a response if the 
+ * This middleware sets the correct CORS headers on a response if the
  * controller has the @CORS annotation. This is needed for webapps that want
- * to access an API and dont run on the same domain, see 
+ * to access an API and dont run on the same domain, see
  * https://developer.mozilla.org/en-US/docs/Web/HTTP/Access_control_CORS
  */
 class CORSMiddleware extends Middleware {
 
+	/**
+	 * @var IRequest
+	 */
 	private $request;
+
+	/**
+	 * @var ControllerMethodReflector
+	 */
 	private $reflector;
 
 	/**
+	 * @var IUserSession
+	 */
+	private $session;
+
+	/**
 	 * @param IRequest $request
 	 * @param ControllerMethodReflector $reflector
+	 * @param IUserSession $session
 	 */
-	public function __construct(IRequest $request, 
-	                            ControllerMethodReflector $reflector) {
+	public function __construct(IRequest $request,
+	                            ControllerMethodReflector $reflector,
+	                            IUserSession $session) {
 		$this->request = $request;
 		$this->reflector = $reflector;
+		$this->session = $session;
 	}
 
+	/**
+	 * This is being run in normal order before the controller is being
+	 * called which allows several modifications and checks
+	 *
+	 * @param Controller $controller the controller that is being called
+	 * @param string $methodName the name of the method that will be called on
+	 *                           the controller
+	 * @since 6.0.0
+	 */
+	public function beforeController($controller, $methodName){
+		// ensure that @CORS annotated API routes are not used in conjunction
+		// with session authentication since this enables CSRF attack vectors
+		if ($this->reflector->hasAnnotation('CORS') &&
+			!$this->reflector->hasAnnotation('PublicPage'))  {
+			$user = $this->request->server['PHP_AUTH_USER'];
+			$pass = $this->request->server['PHP_AUTH_PW'];
+
+			$this->session->logout();
+			if(!$this->session->login($user, $pass)) {
+				throw new SecurityException('CORS requires basic auth');
+			}
+		}
+	}
 
 	/**
 	 * This is being run after a successful controllermethod call and allows
@@ -65,7 +104,7 @@ class CORSMiddleware extends Middleware {
 		if(isset($this->request->server['HTTP_ORIGIN']) &&
 			$this->reflector->hasAnnotation('CORS')) {
 
-			// allow credentials headers must not be true or CSRF is possible 
+			// allow credentials headers must not be true or CSRF is possible
 			// otherwise
 			foreach($response->getHeaders() as $header => $value ) {
 				if(strtolower($header) === 'access-control-allow-credentials' &&
diff --git a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php
index a4f3137..92ea545 100644
--- a/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php
+++ b/tests/lib/appframework/middleware/security/CORSMiddlewareTest.php
@@ -21,10 +21,12 @@ use OCP\AppFramework\Http\Response;
 class CORSMiddlewareTest extends \Test\TestCase {
 
 	private $reflector;
+	private $session;
 
 	protected function setUp() {
 		parent::setUp();
 		$this->reflector = new ControllerMethodReflector();
+		$this->session = $this->getMock('\OCP\IUserSession');
 	}
 
 	/**
@@ -41,7 +43,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMock('\OCP\IConfig')
 		);
 		$this->reflector->reflect($this, __FUNCTION__);
-		$middleware = new CORSMiddleware($request, $this->reflector);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
 
 		$response = $middleware->afterController($this, __FUNCTION__, new Response());
 		$headers = $response->getHeaders();
@@ -59,7 +61,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMock('\OCP\Security\ISecureRandom'),
 			$this->getMock('\OCP\IConfig')
 		);
-		$middleware = new CORSMiddleware($request, $this->reflector);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
 
 		$response = $middleware->afterController($this, __FUNCTION__, new Response());
 		$headers = $response->getHeaders();
@@ -77,7 +79,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMock('\OCP\IConfig')
 		);
 		$this->reflector->reflect($this, __FUNCTION__);
-		$middleware = new CORSMiddleware($request, $this->reflector);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
 
 		$response = $middleware->afterController($this, __FUNCTION__, new Response());
 		$headers = $response->getHeaders();
@@ -100,11 +102,76 @@ class CORSMiddlewareTest extends \Test\TestCase {
 			$this->getMock('\OCP\IConfig')
 		);
 		$this->reflector->reflect($this, __FUNCTION__);
-		$middleware = new CORSMiddleware($request, $this->reflector);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
 
 		$response = new Response();
 		$response->addHeader('AcCess-control-Allow-Credentials ', 'TRUE');
 		$middleware->afterController($this, __FUNCTION__, $response);
 	}
 
+	/**
+	 * @CORS
+	 * @PublicPage
+	 */
+	public function testNoCORSShouldAllowCookieAuth() {
+		$request = new Request(
+			[],
+			$this->getMock('\OCP\Security\ISecureRandom'),
+			$this->getMock('\OCP\IConfig')
+		);
+		$this->reflector->reflect($this, __FUNCTION__);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+
+		$middleware->beforeController($this, __FUNCTION__, new Response());
+	}
+
+	/**
+	 * @CORS
+	 */
+	public function testCORSShouldRelogin() {
+		$request = new Request(
+			['server' => [
+				'PHP_AUTH_USER' => 'user',
+				'PHP_AUTH_PW' => 'pass'
+			]],
+			$this->getMock('\OCP\Security\ISecureRandom'),
+			$this->getMock('\OCP\IConfig')
+		);
+		$this->session->expects($this->once())
+			->method('logout');
+		$this->session->expects($this->once())
+			->method('login')
+			->with($this->equalTo('user'), $this->equalTo('pass'))
+			->will($this->returnValue(true));
+		$this->reflector->reflect($this, __FUNCTION__);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+
+		$middleware->beforeController($this, __FUNCTION__, new Response());
+	}
+
+	/**
+	 * @CORS
+	 * @expectedException \OC\AppFramework\Middleware\Security\SecurityException
+	 */
+	public function testCORSShouldNotAllowCookieAuth() {
+		$request = new Request(
+			['server' => [
+				'PHP_AUTH_USER' => 'user',
+				'PHP_AUTH_PW' => 'pass'
+			]],
+			$this->getMock('\OCP\Security\ISecureRandom'),
+			$this->getMock('\OCP\IConfig')
+		);
+		$this->session->expects($this->once())
+			->method('logout');
+		$this->session->expects($this->once())
+			->method('login')
+			->with($this->equalTo('user'), $this->equalTo('pass'))
+			->will($this->returnValue(false));
+		$this->reflector->reflect($this, __FUNCTION__);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session);
+
+		$middleware->beforeController($this, __FUNCTION__, new Response());
+	}
+
 }

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/owncloud.git



More information about the Pkg-owncloud-commits mailing list