diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index daac36606f..08af42b521 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -42,6 +42,7 @@ use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Middleware; use OCP\AppFramework\Http\Response; use OCP\AppFramework\Http\JSONResponse; +use OCP\AppFramework\OCSController; use OCP\INavigationManager; use OCP\IURLGenerator; use OCP\IRequest; @@ -112,7 +113,7 @@ class SecurityMiddleware extends Middleware { * This runs all the security checks before a method call. The * security checks are determined by inspecting the controller method * annotations - * @param string $controller the controllername or string + * @param Controller $controller the controller * @param string $methodName the name of the method * @throws SecurityException when a security check fails */ @@ -145,7 +146,14 @@ class SecurityMiddleware extends Middleware { // CSRF check - also registers the CSRF token since the session may be closed later Util::callRegister(); if(!$this->reflector->hasAnnotation('NoCSRFRequired')) { - if(!$this->request->passesCSRFCheck()) { + /* + * 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. + */ + if(!$this->request->passesCSRFCheck() && !( + $controller instanceof OCSController && + $this->request->getHeader('OCS_APIREQUEST') === true)) { throw new CrossSiteRequestForgeryException(); } } diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index 487b83c0be..6f67593213 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -35,22 +35,38 @@ use OC\Appframework\Middleware\Security\Exceptions\StrictCookieMissingException; use OC\AppFramework\Middleware\Security\SecurityMiddleware; use OC\AppFramework\Utility\ControllerMethodReflector; use OC\Security\CSP\ContentSecurityPolicy; +use OC\Security\CSP\ContentSecurityPolicyManager; +use OCP\AppFramework\Controller; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\TemplateResponse; +use OCP\ILogger; +use OCP\INavigationManager; +use OCP\IRequest; +use OCP\IURLGenerator; class SecurityMiddlewareTest extends \Test\TestCase { + /** @var SecurityMiddleware|\PHPUnit_Framework_MockObject_MockObject */ private $middleware; + /** @var Controller|\PHPUnit_Framework_MockObject_MockObject */ private $controller; + /** @var SecurityException */ private $secException; + /** @var SecurityException */ private $secAjaxException; + /** @var IRequest|\PHPUnit_Framework_MockObject_MockObject */ private $request; + /** @var ControllerMethodReflector */ private $reader; + /** @var ILogger|\PHPUnit_Framework_MockObject_MockObject */ private $logger; + /** @var INavigationManager|\PHPUnit_Framework_MockObject_MockObject */ private $navigationManager; + /** @var IURLGenerator|\PHPUnit_Framework_MockObject_MockObject */ private $urlGenerator; + /** @var ContentSecurityPolicyManager|\PHPUnit_Framework_MockObject_MockObject */ private $contentSecurityPolicyManager; protected function setUp() { @@ -354,6 +370,45 @@ class SecurityMiddlewareTest extends \Test\TestCase { $this->middleware->beforeController(__CLASS__, __FUNCTION__); } + public function dataCsrfOcsController() { + $controller = $this->getMockBuilder('OCP\AppFramework\Controller') + ->disableOriginalConstructor() + ->getMock(); + $ocsController = $this->getMockBuilder('OCP\AppFramework\OCSController') + ->disableOriginalConstructor() + ->getMock(); + + return [ + [$controller, false, true], + [$controller, true, true], + + [$ocsController, false, true], + [$ocsController, true, true], + ]; + } + + /** + * @dataProvider dataCsrfOcsController + * @param Controller $controller + * @param bool $hasOcsApiHeader + * @param bool $exception + */ + public function testCsrfOcsController(Controller $controller, $hasOcsApiHeader, $exception) { + $this->request + ->method('getHeader') + ->willReturn($hasOcsApiHeader ? 'true' : null); + $this->request->expects($this->once()) + ->method('passesStrictCookieCheck') + ->willReturn(true); + + try { + $this->middleware->beforeController($controller, 'foo'); + $this->assertFalse($exception); + } catch (CrossSiteRequestForgeryException $e) { + $this->assertTrue($exception); + } + } + /** * @NoCSRFRequired * @NoAdminRequired