[Pkg-owncloud-commits] [owncloud] 01/62: disallow cookie auth for cors requests
David Prévot
taffit at moszumanska.debian.org
Tue Jun 23 23:39:32 UTC 2015
This is an automated email from the git hooks/post-receive script.
taffit pushed a commit to annotated tag v8.0.5beta
in repository owncloud.
commit 777a6c454eb19a71c80534c6bb8d6a6e982fac9d
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 eb19d1b..dc7a430 100644
--- a/apps/files/controller/apicontroller.php
+++ b/apps/files/controller/apicontroller.php
@@ -63,7 +63,6 @@ class ApiController extends Controller {
* replace the actual tag selection.
*
* @NoAdminRequired
- * @CORS
*
* @param string $path path
* @param array $tags array of tags
@@ -91,7 +90,6 @@ class ApiController extends Controller {
* Returns a list of all files tagged with the given tag.
*
* @NoAdminRequired
- * @CORS
*
* @param array $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 4229b25..d32bead 100644
--- a/lib/private/appframework/dependencyinjection/dicontainer.php
+++ b/lib/private/appframework/dependencyinjection/dicontainer.php
@@ -272,7 +272,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
$this->registerService('CORSMiddleware', function($c) {
return new CORSMiddleware(
$c['Request'],
- $c['ControllerMethodReflector']
+ $c['ControllerMethodReflector'],
+ $c['OCP\IUserSession']
);
});
@@ -287,8 +288,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 dca3996..4de2f0f 100644
--- a/lib/private/appframework/middleware/security/corsmiddleware.php
+++ b/lib/private/appframework/middleware/security/corsmiddleware.php
@@ -13,30 +13,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
@@ -54,7 +93,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 b4bbcce..5d3205b 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');
}
/**
@@ -35,7 +37,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
array('server' => array('HTTP_ORIGIN' => 'test'))
);
$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();
@@ -47,7 +49,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
$request = new Request(
array('server' => array('HTTP_ORIGIN' => 'test'))
);
- $middleware = new CORSMiddleware($request, $this->reflector);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session);
$response = $middleware->afterController($this, __FUNCTION__, new Response());
$headers = $response->getHeaders();
@@ -61,7 +63,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
public function testNoOriginHeaderNoCORSHEADER() {
$request = new Request();
$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();
@@ -78,11 +80,76 @@ class CORSMiddlewareTest extends \Test\TestCase {
array('server' => array('HTTP_ORIGIN' => 'test'))
);
$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');
$response = $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