Merge pull request #16579 from owncloud/stable8-cors-no-cookie-auth

Disallow cookie auth for cors requests stable8

2 files changed, 47 insertions, 7 deletions

diff --git a/lib/private/appframework/dependencyinjection/dicontainer.php b/lib/private/appframework/dependencyinjection/dicontainer.php
index 4229b251e29..d32bead1f05 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 dca3996ea2e..4de2f0f1db8 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' &&