Merge pull request #7873 from nextcloud/fix_5694

Don't perform CSRF check on OCS routes with Bearer auth

2 files changed, 28 insertions, 10 deletions

diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
index 1c049fb3621..f45c8f8726c 100644
--- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
@@ -170,10 +170,16 @@ class SecurityMiddleware extends Middleware {
 			 * Only allow the CSRF check to fail on OCS Requests. This kind of
 			 * hacks around that we have no full token auth in place yet and we
 			 * do want to offer CSRF checks for web requests.
+			 *
+			 * Additionally we allow Bearer authenticated requests to pass on OCS routes.
+			 * This allows oauth apps (e.g. moodle) to use the OCS endpoints
 			 */
 			if(!$this->request->passesCSRFCheck() && !(
-					$controller instanceof OCSController &&
-					$this->request->getHeader('OCS-APIREQUEST') === 'true')) {
+					$controller instanceof OCSController && (
+						$this->request->getHeader('OCS-APIREQUEST') === 'true' ||
+						strpos($this->request->getHeader('Authorization'), 'Bearer ') === 0
+					)
+				)) {
 				throw new CrossSiteRequestForgeryException();
 			}
 		}
diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
index b68f0cb1981..e36bd727bea 100644
--- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
@@ -387,11 +387,15 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 			->getMock();
 
 		return [
-			[$controller, false, true],
-			[$controller, true,  true],
-
-			[$ocsController, false, true],
-			[$ocsController, true,  false],
+			[$controller, false, false, true],
+			[$controller, false,  true, true],
+			[$controller,  true, false, true],
+			[$controller,  true,  true, true],
+
+			[$ocsController, false, false,  true],
+			[$ocsController, false,  true, false],
+			[$ocsController,  true, false, false],
+			[$ocsController,  true,  true, false],
 		];
 	}
 
@@ -399,13 +403,21 @@ class SecurityMiddlewareTest extends \Test\TestCase {
 	 * @dataProvider dataCsrfOcsController
 	 * @param Controller $controller
 	 * @param bool $hasOcsApiHeader
+	 * @param bool $hasBearerAuth
 	 * @param bool $exception
 	 */
-	public function testCsrfOcsController(Controller $controller, $hasOcsApiHeader, $exception) {
+	public function testCsrfOcsController(Controller $controller, bool $hasOcsApiHeader, bool $hasBearerAuth, bool $exception) {
 		$this->request
 			->method('getHeader')
-			->with('OCS-APIREQUEST')
-			->willReturn($hasOcsApiHeader ? 'true' : null);
+			->will(self::returnCallback(function ($header) use ($hasOcsApiHeader, $hasBearerAuth) {
+				if ($header === 'OCS-APIREQUEST' && $hasOcsApiHeader) {
+					return 'true';
+				}
+				if ($header === 'Authorization' && $hasBearerAuth) {
+					return 'Bearer TOKEN!';
+				}
+				return '';
+			}));
 		$this->request->expects($this->once())
 			->method('passesStrictCookieCheck')
 			->willReturn(true);