feat: move csrf validation out of request

CsrfTokenManager is a heavy dependency that needs a user session and working memory cache.

Signed-off-by: Daniel Kesselberg <[email protected]>

apps/dav/appinfo/v1/caldav.php

@@ -43,6 +43,7 @@
 	\OC::$server->getRequest(),
 	\OC::$server->getTwoFactorAuthManager(),
 	\OC::$server->getBruteForceThrottler(),
+	\OC::$server->get(\OC\Security\CSRF\CsrfValidator::class),
 	'principals/'
 );
 $principalBackend = new Principal(

apps/dav/appinfo/v1/carddav.php

@@ -48,6 +48,7 @@
 	\OC::$server->getRequest(),
 	\OC::$server->getTwoFactorAuthManager(),
 	\OC::$server->getBruteForceThrottler(),
+	\OC::$server->get(\OC\Security\CSRF\CsrfValidator::class),
 	'principals/'
 );
 $principalBackend = new Principal(

apps/dav/appinfo/v1/webdav.php

@@ -59,6 +59,7 @@
 	\OC::$server->getRequest(),
 	\OC::$server->getTwoFactorAuthManager(),
 	\OC::$server->getBruteForceThrottler(),
+	\OC::$server->get(\OC\Security\CSRF\CsrfValidator::class),
 	'principals/'
 );
 $authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend);

apps/dav/lib/Connector/Sabre/Auth.php

@@ -37,6 +37,7 @@
 use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
 use OC\Authentication\TwoFactorAuth\Manager;
 use OC\Security\Bruteforce\Throttler;
+use OC\Security\CSRF\CsrfValidator;
 use OC\User\LoginException;
 use OC\User\Session;
 use OCA\DAV\Connector\Sabre\Exception\PasswordLoginForbidden;
@@ -61,17 +62,21 @@ class Auth extends AbstractBasic {
 	private Manager $twoFactorManager;
 	private Throttler $throttler;
 
+	private CsrfValidator $csrfValidator;
+
 	public function __construct(ISession $session,
 								Session $userSession,
 								IRequest $request,
 								Manager $twoFactorManager,
 								Throttler $throttler,
+								CsrfValidator $csrfValidator,
 								string $principalPrefix = 'principals/users/') {
 		$this->session = $session;
 		$this->userSession = $userSession;
 		$this->twoFactorManager = $twoFactorManager;
 		$this->request = $request;
 		$this->throttler = $throttler;
+		$this->csrfValidator = $csrfValidator;
 		$this->principalPrefix = $principalPrefix;
 
 		// setup realm
@@ -191,7 +196,7 @@ private function requiresCSRFCheck(): bool {
 	private function auth(RequestInterface $request, ResponseInterface $response): array {
 		$forcedLogout = false;
 
-		if (!$this->request->passesCSRFCheck() &&
+		if (!$this->csrfValidator->validate($this->request) &&
 			$this->requiresCSRFCheck()) {
 			// In case of a fail with POST we need to recheck the credentials
 			if ($this->request->getMethod() === 'POST') {

apps/dav/lib/Server.php

@@ -35,6 +35,8 @@
  */
 namespace OCA\DAV;
 
+use OC\Security\Bruteforce\Throttler;
+use OC\Security\CSRF\CsrfValidator;
 use OCA\DAV\AppInfo\PluginManager;
 use OCA\DAV\BulkUpload\BulkUploadPlugin;
 use OCA\DAV\CalDAV\BirthdayService;
@@ -120,7 +122,8 @@ public function __construct(IRequest $request, string $baseUri) {
 			\OC::$server->getUserSession(),
 			\OC::$server->getRequest(),
 			\OC::$server->getTwoFactorAuthManager(),
-			\OC::$server->getBruteForceThrottler()
+			\OC::$server->get(Throttler::class),
+			\OC::$server->get(CsrfValidator::class)
 		);
 
 		// Set URL explicitly due to reverse-proxy situations

apps/dav/tests/unit/Connector/Sabre/AuthTest.php

@@ -31,6 +31,7 @@
 
 use OC\Authentication\TwoFactorAuth\Manager;
 use OC\Security\Bruteforce\Throttler;
+use OC\Security\CSRF\CsrfValidator;
 use OC\User\Session;
 use OCP\IRequest;
 use OCP\ISession;
@@ -59,6 +60,7 @@ class AuthTest extends TestCase {
 	private $twoFactorManager;
 	/** @var Throttler */
 	private $throttler;
+	private CsrfValidator $csrfValidator;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -74,12 +76,14 @@ protected function setUp(): void {
 		$this->throttler = $this->getMockBuilder(Throttler::class)
 			->disableOriginalConstructor()
 			->getMock();
+		$this->csrfValidator = $this->createMock(CsrfValidator::class);
 		$this->auth = new \OCA\DAV\Connector\Sabre\Auth(
 			$this->session,
 			$this->userSession,
 			$this->request,
 			$this->twoFactorManager,
-			$this->throttler
+			$this->throttler,
+			$this->csrfValidator,
 		);
 	}
 
@@ -270,9 +274,9 @@ public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenForNonGet(): void
 			->expects($this->any())
 			->method('getUser')
 			->willReturn($user);
-		$this->request
+		$this->csrfValidator
 			->expects($this->once())
-			->method('passesCSRFCheck')
+			->method('validate')
 			->willReturn(false);
 
 		$expectedResponse = [
@@ -322,9 +326,9 @@ public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenAndCorrectlyDavAu
 			->expects($this->any())
 			->method('getUser')
 			->willReturn($user);
-		$this->request
+		$this->csrfValidator
 			->expects($this->once())
-			->method('passesCSRFCheck')
+			->method('validate')
 			->willReturn(false);
 		$this->auth->check($request, $response);
 	}
@@ -372,9 +376,9 @@ public function testAuthenticateAlreadyLoggedInWithoutTwoFactorChallengePassed()
 			->expects($this->any())
 			->method('getUser')
 			->willReturn($user);
-		$this->request
+		$this->csrfValidator
 			->expects($this->once())
-			->method('passesCSRFCheck')
+			->method('validate')
 			->willReturn(true);
 		$this->twoFactorManager->expects($this->once())
 			->method('needsSecondFactor')
@@ -426,9 +430,9 @@ public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenAndIncorrectlyDav
 			->expects($this->any())
 			->method('getUser')
 			->willReturn($user);
-		$this->request
+		$this->csrfValidator
 			->expects($this->once())
-			->method('passesCSRFCheck')
+			->method('validate')
 			->willReturn(false);
 		$this->auth->check($request, $response);
 	}
@@ -472,9 +476,9 @@ public function testAuthenticateAlreadyLoggedInWithoutCsrfTokenForNonGetAndDeskt
 			->expects($this->any())
 			->method('getUser')
 			->willReturn($user);
-		$this->request
+		$this->csrfValidator
 			->expects($this->once())
-			->method('passesCSRFCheck')
+			->method('validate')
 			->willReturn(false);
 
 		$this->auth->check($request, $response);
@@ -541,9 +545,9 @@ public function testAuthenticateAlreadyLoggedInWithCsrfTokenForGet(): void {
 			->expects($this->any())
 			->method('getUser')
 			->willReturn($user);
-		$this->request
+		$this->csrfValidator
 			->expects($this->once())
-			->method('passesCSRFCheck')
+			->method('validate')
 			->willReturn(true);
 
 		$response = $this->auth->check($request, $response);

core/Controller/LoginController.php

@@ -38,6 +38,7 @@
 use OC\Authentication\Login\LoginData;
 use OC\Authentication\WebAuthn\Manager as WebAuthnManager;
 use OC\Security\Bruteforce\Throttler;
+use OC\Security\CSRF\CsrfValidator;
 use OC\User\Session;
 use OC_App;
 use OCP\AppFramework\Controller;
@@ -76,6 +77,7 @@ public function __construct(
 		private WebAuthnManager $webAuthnManager,
 		private IManager $manager,
 		private IL10N $l10n,
+		private CsrfValidator $csrfValidator,
 	) {
 		parent::__construct($appName, $request);
 	}
@@ -273,7 +275,7 @@ public function tryLogin(Chain $loginChain,
 							 string $redirect_url = null,
 							 string $timezone = '',
 							 string $timezone_offset = ''): RedirectResponse {
-		if (!$this->request->passesCSRFCheck()) {
+		if (!$this->csrfValidator->validate($this->request)) {
 			if ($this->userSession->isLoggedIn()) {
 				// If the user is already logged in and the CSRF check does not pass then
 				// simply redirect the user to the correct page as required. This is the

lib/composer/composer/autoload_classmap.php

@@ -1553,6 +1553,7 @@
     'OC\\Security\\CSRF\\CsrfToken' => $baseDir . '/lib/private/Security/CSRF/CsrfToken.php',
     'OC\\Security\\CSRF\\CsrfTokenGenerator' => $baseDir . '/lib/private/Security/CSRF/CsrfTokenGenerator.php',
     'OC\\Security\\CSRF\\CsrfTokenManager' => $baseDir . '/lib/private/Security/CSRF/CsrfTokenManager.php',
+    'OC\\Security\\CSRF\\CsrfValidator' => $baseDir . '/lib/private/Security/CSRF/CsrfValidator.php',
     'OC\\Security\\CSRF\\TokenStorage\\SessionStorage' => $baseDir . '/lib/private/Security/CSRF/TokenStorage/SessionStorage.php',
     'OC\\Security\\Certificate' => $baseDir . '/lib/private/Security/Certificate.php',
     'OC\\Security\\CertificateManager' => $baseDir . '/lib/private/Security/CertificateManager.php',

lib/composer/composer/autoload_static.php

@@ -1586,6 +1586,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
         'OC\\Security\\CSRF\\CsrfToken' => __DIR__ . '/../../..' . '/lib/private/Security/CSRF/CsrfToken.php',
         'OC\\Security\\CSRF\\CsrfTokenGenerator' => __DIR__ . '/../../..' . '/lib/private/Security/CSRF/CsrfTokenGenerator.php',
         'OC\\Security\\CSRF\\CsrfTokenManager' => __DIR__ . '/../../..' . '/lib/private/Security/CSRF/CsrfTokenManager.php',
+        'OC\\Security\\CSRF\\CsrfValidator' => __DIR__ . '/../../..' . '/lib/private/Security/CSRF/CsrfValidator.php',
         'OC\\Security\\CSRF\\TokenStorage\\SessionStorage' => __DIR__ . '/../../..' . '/lib/private/Security/CSRF/TokenStorage/SessionStorage.php',
         'OC\\Security\\Certificate' => __DIR__ . '/../../..' . '/lib/private/Security/Certificate.php',
         'OC\\Security\\CertificateManager' => __DIR__ . '/../../..' . '/lib/private/Security/CertificateManager.php',

lib/private/AppFramework/DependencyInjection/DIContainer.php

@@ -233,7 +233,8 @@ public function __construct(string $appName, array $urlParams = [], ServerContai
 					$c->get(IRequest::class),
 					$c->get(IControllerMethodReflector::class),
 					$c->get(IUserSession::class),
-					$c->get(OC\Security\Bruteforce\Throttler::class)
+					$c->get(OC\Security\Bruteforce\Throttler::class),
+					$c->get(OC\Security\CSRF\CsrfValidator::class),
 				)
 			);
 			$dispatcher->registerMiddleware(
@@ -257,7 +258,8 @@ public function __construct(string $appName, array $urlParams = [], ServerContai
 				$server->getAppManager(),
 				$server->getL10N('lib'),
 				$c->get(AuthorizedGroupMapper::class),
-				$server->get(IUserSession::class)
+				$server->get(IUserSession::class),
+				$c->get(OC\Security\CSRF\CsrfValidator::class),
 			);
 			$dispatcher->registerMiddleware($securityMiddleware);
 			$dispatcher->registerMiddleware(

lib/private/AppFramework/Middleware/Security/CORSMiddleware.php

@@ -30,6 +30,7 @@
 use OC\AppFramework\Utility\ControllerMethodReflector;
 use OC\Authentication\Exceptions\PasswordLoginForbiddenException;
 use OC\Security\Bruteforce\Throttler;
+use OC\Security\CSRF\CsrfValidator;
 use OC\User\Session;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
@@ -56,6 +57,7 @@ class CORSMiddleware extends Middleware {
 	private $session;
 	/** @var Throttler */
 	private $throttler;
+	private CsrfValidator $csrfValidator;
 
 	/**
 	 * @param IRequest $request
@@ -66,11 +68,13 @@ class CORSMiddleware extends Middleware {
 	public function __construct(IRequest $request,
 								ControllerMethodReflector $reflector,
 								Session $session,
-								Throttler $throttler) {
+								Throttler $throttler,
+								CsrfValidator $csrfValidator) {
 		$this->request = $request;
 		$this->reflector = $reflector;
 		$this->session = $session;
 		$this->throttler = $throttler;
+		$this->csrfValidator = $csrfValidator;
 	}
 
 	/**
@@ -94,7 +98,7 @@ public function beforeController($controller, $methodName) {
 			$pass = array_key_exists('PHP_AUTH_PW', $this->request->server) ? $this->request->server['PHP_AUTH_PW'] : null;
 
 			// Allow to use the current session if a CSRF token is provided
-			if ($this->request->passesCSRFCheck()) {
+			if ($this->csrfValidator->validate($this->request)) {
 				return;
 			}
 			$this->session->logout();

lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php

@@ -44,6 +44,7 @@
 use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
 use OC\AppFramework\Middleware\Security\Exceptions\StrictCookieMissingException;
 use OC\AppFramework\Utility\ControllerMethodReflector;
+use OC\Security\CSRF\CsrfValidator;
 use OC\Settings\AuthorizedGroupMapper;
 use OCP\App\AppPathNotFoundException;
 use OCP\App\IAppManager;
@@ -102,6 +103,7 @@ class SecurityMiddleware extends Middleware {
 	private $groupAuthorizationMapper;
 	/** @var IUserSession */
 	private $userSession;
+	private CsrfValidator $csrfValidator;
 
 	public function __construct(IRequest $request,
 								ControllerMethodReflector $reflector,
@@ -115,7 +117,8 @@ public function __construct(IRequest $request,
 								IAppManager $appManager,
 								IL10N $l10n,
 								AuthorizedGroupMapper $mapper,
-								IUserSession $userSession
+								IUserSession $userSession,
+								CsrfValidator $csrfValidator,
 	) {
 		$this->navigationManager = $navigationManager;
 		$this->request = $request;
@@ -130,6 +133,7 @@ public function __construct(IRequest $request,
 		$this->l10n = $l10n;
 		$this->groupAuthorizationMapper = $mapper;
 		$this->userSession = $userSession;
+		$this->csrfValidator = $csrfValidator;
 	}
 
 	/**
@@ -215,7 +219,7 @@ public function beforeController($controller, $methodName) {
 			 * 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() && !(
+			if (!$this->csrfValidator->validate($this->request) && !(
 				$controller instanceof OCSController && (
 					$this->request->getHeader('OCS-APIREQUEST') === 'true' ||
 					str_starts_with($this->request->getHeader('Authorization'), 'Bearer ')

lib/private/EventSourceFactory.php

@@ -22,16 +22,15 @@
 
 namespace OC;
 
+use OC\Security\CSRF\CsrfValidator;
 use OCP\IEventSource;
 use OCP\IEventSourceFactory;
 use OCP\IRequest;
 
 class EventSourceFactory implements IEventSourceFactory {
-	private IRequest $request;
-
-
-	public function __construct(IRequest $request) {
-		$this->request = $request;
+	public function __construct(
+		private IRequest $request,
+		private CsrfValidator $csrfValidator) {
 	}
 
 	/**
@@ -41,6 +40,9 @@ public function __construct(IRequest $request) {
 	 * @since 28.0.0
 	 */
 	public function create(): IEventSource {
-		return new \OC_EventSource($this->request);
+		return new \OC_EventSource(
+			$this->request,
+			$this->csrfValidator
+		);
 	}
 }