Dark hackery to not always disable CSRF for OCS controllers
This commit is contained in:
parent
54ae8eede3
commit
f7f5216aa3
|
@ -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();
|
||||
}
|
||||
}
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue