From c0adfa437548a10a0542b6d36ab20011ddfdb93e Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Mon, 15 Jan 2018 22:05:06 +0100 Subject: [PATCH 1/2] Don't perform CSRF check on OCS routes with Bearer auth Fixes #5694 Signed-off-by: Roeland Jago Douma --- .../Middleware/Security/SecurityMiddleware.php | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index 1c049fb362..f45c8f8726 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(); } } From 7405dfb5447a324b2f5504b3a540c8d35b0b21cb Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Thu, 25 Jan 2018 16:10:38 +0100 Subject: [PATCH 2/2] Update tests Signed-off-by: Roeland Jago Douma --- .../Security/SecurityMiddlewareTest.php | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index b68f0cb198..e36bd727be 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], + [$controller, false, false, true], + [$controller, false, true, true], + [$controller, true, false, true], + [$controller, true, true, true], - [$ocsController, false, true], - [$ocsController, true, false], + [$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);