aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDaniel Kesselberg <mail@danielkesselberg.de>2023-08-15 18:58:52 +0200
committerDaniel Kesselberg <mail@danielkesselberg.de>2024-08-14 15:41:27 +0200
commit6e176840c882cfe11152de6350788d74374a54ae (patch)
tree36c324e1d646051ba36988094fdeb28371e39f1d
parent0f10cabf2a7ff6652f7b29e81f3682fac941e647 (diff)
downloadnextcloud-server-dept-remove-csrf-dependency-from-request.tar.gz
nextcloud-server-dept-remove-csrf-dependency-from-request.zip
feat: move csrf validation out of requestdept-remove-csrf-dependency-from-request
Signed-off-by: Daniel Kesselberg <mail@danielkesselberg.de>
-rw-r--r--apps/dav/appinfo/v1/caldav.php2
-rw-r--r--apps/dav/appinfo/v1/carddav.php2
-rw-r--r--apps/dav/appinfo/v1/webdav.php3
-rw-r--r--apps/dav/lib/Connector/Sabre/Auth.php4
-rw-r--r--apps/dav/lib/Server.php5
-rw-r--r--apps/dav/tests/unit/Connector/Sabre/AuthTest.php30
-rw-r--r--core/Controller/LoginController.php4
-rw-r--r--lib/composer/composer/autoload_classmap.php2
-rw-r--r--lib/composer/composer/autoload_static.php2
-rw-r--r--lib/private/AppFramework/DependencyInjection/DIContainer.php5
-rw-r--r--lib/private/AppFramework/Middleware/Security/CORSMiddleware.php4
-rw-r--r--lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php4
-rw-r--r--lib/private/EventSource.php4
-rw-r--r--lib/private/EventSourceFactory.php7
-rw-r--r--lib/private/Security/CSRF/CsrfValidator.php42
-rw-r--r--lib/private/Server.php4
-rw-r--r--lib/private/legacy/OC_JSON.php10
-rw-r--r--lib/public/IRequest.php1
-rw-r--r--lib/public/Security/CSRF/ICsrfValidator.php24
-rw-r--r--tests/Core/Controller/LoginControllerTest.php87
-rw-r--r--tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php30
-rw-r--r--tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php73
-rw-r--r--tests/lib/EventSourceFactoryTest.php4
-rw-r--r--tests/lib/Security/CSRF/CsrfTokenManagerTest.php12
-rw-r--r--tests/lib/Security/CSRF/CsrfValidatorTest.php96
25 files changed, 388 insertions, 73 deletions
diff --git a/apps/dav/appinfo/v1/caldav.php b/apps/dav/appinfo/v1/caldav.php
index bfa4d6184d5..a9637e0e167 100644
--- a/apps/dav/appinfo/v1/caldav.php
+++ b/apps/dav/appinfo/v1/caldav.php
@@ -18,6 +18,7 @@ use OCA\DAV\Connector\Sabre\ExceptionLoggerPlugin;
use OCA\DAV\Connector\Sabre\MaintenancePlugin;
use OCA\DAV\Connector\Sabre\Principal;
use OCP\Accounts\IAccountManager;
+use OCP\Security\CSRF\ICsrfValidator;
use Psr\Log\LoggerInterface;
$authBackend = new Auth(
@@ -26,6 +27,7 @@ $authBackend = new Auth(
\OC::$server->getRequest(),
\OC::$server->getTwoFactorAuthManager(),
\OC::$server->getBruteForceThrottler(),
+ \OC::$server->get(ICsrfValidator::class),
'principals/'
);
$principalBackend = new Principal(
diff --git a/apps/dav/appinfo/v1/carddav.php b/apps/dav/appinfo/v1/carddav.php
index 72be4712705..faed40801dc 100644
--- a/apps/dav/appinfo/v1/carddav.php
+++ b/apps/dav/appinfo/v1/carddav.php
@@ -19,6 +19,7 @@ use OCA\DAV\Connector\Sabre\MaintenancePlugin;
use OCA\DAV\Connector\Sabre\Principal;
use OCP\Accounts\IAccountManager;
use OCP\App\IAppManager;
+use OCP\Security\CSRF\ICsrfValidator;
use Psr\Log\LoggerInterface;
use Sabre\CardDAV\Plugin;
@@ -28,6 +29,7 @@ $authBackend = new Auth(
\OC::$server->getRequest(),
\OC::$server->getTwoFactorAuthManager(),
\OC::$server->getBruteForceThrottler(),
+ \OC::$server->get(ICsrfValidator::class),
'principals/'
);
$principalBackend = new Principal(
diff --git a/apps/dav/appinfo/v1/webdav.php b/apps/dav/appinfo/v1/webdav.php
index 1683c29ca80..8f03fee9499 100644
--- a/apps/dav/appinfo/v1/webdav.php
+++ b/apps/dav/appinfo/v1/webdav.php
@@ -5,6 +5,8 @@
* SPDX-FileCopyrightText: 2016 ownCloud, Inc.
* SPDX-License-Identifier: AGPL-3.0-only
*/
+
+use OCP\Security\CSRF\ICsrfValidator;
use Psr\Log\LoggerInterface;
// no php execution timeout for webdav
@@ -38,6 +40,7 @@ $authBackend = new \OCA\DAV\Connector\Sabre\Auth(
\OC::$server->getRequest(),
\OC::$server->getTwoFactorAuthManager(),
\OC::$server->getBruteForceThrottler(),
+ \OC::$server->get(ICsrfValidator::class),
'principals/'
);
$authPlugin = new \Sabre\DAV\Auth\Plugin($authBackend);
diff --git a/apps/dav/lib/Connector/Sabre/Auth.php b/apps/dav/lib/Connector/Sabre/Auth.php
index 9b67d960107..97ba05176bd 100644
--- a/apps/dav/lib/Connector/Sabre/Auth.php
+++ b/apps/dav/lib/Connector/Sabre/Auth.php
@@ -17,6 +17,7 @@ use OCP\IRequest;
use OCP\ISession;
use OCP\Security\Bruteforce\IThrottler;
use OCP\Security\Bruteforce\MaxDelayReached;
+use OCP\Security\CSRF\ICsrfValidator;
use Psr\Log\LoggerInterface;
use Sabre\DAV\Auth\Backend\AbstractBasic;
use Sabre\DAV\Exception\NotAuthenticated;
@@ -39,6 +40,7 @@ class Auth extends AbstractBasic {
IRequest $request,
Manager $twoFactorManager,
IThrottler $throttler,
+ private ICsrfValidator $csrfValidator,
string $principalPrefix = 'principals/users/') {
$this->session = $session;
$this->userSession = $userSession;
@@ -164,7 +166,7 @@ class Auth extends AbstractBasic {
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') {
diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php
index 4fdd70d05c7..8b866bdf517 100644
--- a/apps/dav/lib/Server.php
+++ b/apps/dav/lib/Server.php
@@ -58,6 +58,8 @@ use OCP\ICacheFactory;
use OCP\IRequest;
use OCP\Profiler\IProfiler;
use OCP\SabrePluginEvent;
+use OCP\Security\Bruteforce\IThrottler;
+use OCP\Security\CSRF\ICsrfValidator;
use Psr\Log\LoggerInterface;
use Sabre\CardDAV\VCFExportPlugin;
use Sabre\DAV\Auth\Plugin;
@@ -98,7 +100,8 @@ class Server {
\OC::$server->getUserSession(),
\OC::$server->getRequest(),
\OC::$server->getTwoFactorAuthManager(),
- \OC::$server->getBruteForceThrottler()
+ \OC::$server->get(IThrottler::class),
+ \OC::$server->get(ICsrfValidator::class)
);
// Set URL explicitly due to reverse-proxy situations
diff --git a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php
index dccb1edd0ae..a456ad80d64 100644
--- a/apps/dav/tests/unit/Connector/Sabre/AuthTest.php
+++ b/apps/dav/tests/unit/Connector/Sabre/AuthTest.php
@@ -14,6 +14,7 @@ use OCP\ISession;
use OCP\IUser;
use OCP\Security\Bruteforce\IThrottler;
use PHPUnit\Framework\MockObject\MockObject;
+use OCP\Security\CSRF\ICsrfValidator;
use Sabre\DAV\Server;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
@@ -38,6 +39,7 @@ class AuthTest extends TestCase {
private $twoFactorManager;
/** @var IThrottler&MockObject */
private $throttler;
+ private ICsrfValidator $csrfValidator;
protected function setUp(): void {
parent::setUp();
@@ -53,12 +55,14 @@ class AuthTest extends TestCase {
$this->throttler = $this->getMockBuilder(IThrottler::class)
->disableOriginalConstructor()
->getMock();
+ $this->csrfValidator = $this->createMock(ICsrfValidator::class);
$this->auth = new \OCA\DAV\Connector\Sabre\Auth(
$this->session,
$this->userSession,
$this->request,
$this->twoFactorManager,
- $this->throttler
+ $this->throttler,
+ $this->csrfValidator,
);
}
@@ -249,9 +253,9 @@ class AuthTest extends TestCase {
->expects($this->any())
->method('getUser')
->willReturn($user);
- $this->request
+ $this->csrfValidator
->expects($this->once())
- ->method('passesCSRFCheck')
+ ->method('validate')
->willReturn(false);
$expectedResponse = [
@@ -296,9 +300,9 @@ class AuthTest extends TestCase {
->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);
}
@@ -341,9 +345,9 @@ class AuthTest extends TestCase {
->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')
@@ -390,9 +394,9 @@ class AuthTest extends TestCase {
->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);
}
@@ -431,9 +435,9 @@ class AuthTest extends TestCase {
->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);
@@ -500,9 +504,9 @@ class AuthTest extends TestCase {
->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);
diff --git a/core/Controller/LoginController.php b/core/Controller/LoginController.php
index a433c3073b4..45849500e7c 100644
--- a/core/Controller/LoginController.php
+++ b/core/Controller/LoginController.php
@@ -41,6 +41,7 @@ use OCP\IUser;
use OCP\IUserManager;
use OCP\Notification\IManager;
use OCP\Security\Bruteforce\IThrottler;
+use OCP\Security\CSRF\ICsrfValidator;
use OCP\Util;
class LoginController extends Controller {
@@ -63,6 +64,7 @@ class LoginController extends Controller {
private IManager $manager,
private IL10N $l10n,
private IAppManager $appManager,
+ private ICsrfValidator $csrfValidator,
) {
parent::__construct($appName, $request);
}
@@ -284,7 +286,7 @@ class LoginController extends Controller {
?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
diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php
index 7c44d954223..0e76c200e4c 100644
--- a/lib/composer/composer/autoload_classmap.php
+++ b/lib/composer/composer/autoload_classmap.php
@@ -664,6 +664,7 @@ return array(
'OCP\\Security\\Bruteforce\\IThrottler' => $baseDir . '/lib/public/Security/Bruteforce/IThrottler.php',
'OCP\\Security\\Bruteforce\\MaxDelayReached' => $baseDir . '/lib/public/Security/Bruteforce/MaxDelayReached.php',
'OCP\\Security\\CSP\\AddContentSecurityPolicyEvent' => $baseDir . '/lib/public/Security/CSP/AddContentSecurityPolicyEvent.php',
+ 'OCP\\Security\\CSRF\\ICsrfValidator' => $baseDir . '/lib/public/Security/CSRF/ICsrfValidator.php',
'OCP\\Security\\Events\\GenerateSecurePasswordEvent' => $baseDir . '/lib/public/Security/Events/GenerateSecurePasswordEvent.php',
'OCP\\Security\\Events\\ValidatePasswordPolicyEvent' => $baseDir . '/lib/public/Security/Events/ValidatePasswordPolicyEvent.php',
'OCP\\Security\\FeaturePolicy\\AddFeaturePolicyEvent' => $baseDir . '/lib/public/Security/FeaturePolicy/AddFeaturePolicyEvent.php',
@@ -1838,6 +1839,7 @@ return array(
'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',
diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php
index 63fa40d39bf..2ac297a1b65 100644
--- a/lib/composer/composer/autoload_static.php
+++ b/lib/composer/composer/autoload_static.php
@@ -697,6 +697,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2
'OCP\\Security\\Bruteforce\\IThrottler' => __DIR__ . '/../../..' . '/lib/public/Security/Bruteforce/IThrottler.php',
'OCP\\Security\\Bruteforce\\MaxDelayReached' => __DIR__ . '/../../..' . '/lib/public/Security/Bruteforce/MaxDelayReached.php',
'OCP\\Security\\CSP\\AddContentSecurityPolicyEvent' => __DIR__ . '/../../..' . '/lib/public/Security/CSP/AddContentSecurityPolicyEvent.php',
+ 'OCP\\Security\\CSRF\\ICsrfValidator' => __DIR__ . '/../../..' . '/lib/public/Security/CSRF/ICsrfValidator.php',
'OCP\\Security\\Events\\GenerateSecurePasswordEvent' => __DIR__ . '/../../..' . '/lib/public/Security/Events/GenerateSecurePasswordEvent.php',
'OCP\\Security\\Events\\ValidatePasswordPolicyEvent' => __DIR__ . '/../../..' . '/lib/public/Security/Events/ValidatePasswordPolicyEvent.php',
'OCP\\Security\\FeaturePolicy\\AddFeaturePolicyEvent' => __DIR__ . '/../../..' . '/lib/public/Security/FeaturePolicy/AddFeaturePolicyEvent.php',
@@ -1871,6 +1872,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',
diff --git a/lib/private/AppFramework/DependencyInjection/DIContainer.php b/lib/private/AppFramework/DependencyInjection/DIContainer.php
index 208031b942f..2842a57d0e6 100644
--- a/lib/private/AppFramework/DependencyInjection/DIContainer.php
+++ b/lib/private/AppFramework/DependencyInjection/DIContainer.php
@@ -46,6 +46,7 @@ use OCP\ISession;
use OCP\IURLGenerator;
use OCP\IUserSession;
use OCP\Security\Bruteforce\IThrottler;
+use OCP\Security\CSRF\ICsrfValidator;
use OCP\Security\Ip\IRemoteAddress;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
@@ -209,7 +210,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
$c->get(IControllerMethodReflector::class),
$c->get(IUserSession::class),
$c->get(IThrottler::class),
- $c->get(LoggerInterface::class)
+ $c->get(LoggerInterface::class),
+ $c->get(ICsrfValidator::class),
)
);
$dispatcher->registerMiddleware(
@@ -235,6 +237,7 @@ class DIContainer extends SimpleContainer implements IAppContainer {
$c->get(AuthorizedGroupMapper::class),
$server->get(IUserSession::class),
$c->get(IRemoteAddress::class),
+ $c->get(ICsrfValidator::class),
);
$dispatcher->registerMiddleware($securityMiddleware);
$dispatcher->registerMiddleware(
diff --git a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
index 3f0755b1b91..269c1e5c1dc 100644
--- a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
@@ -21,6 +21,7 @@ use OCP\AppFramework\Middleware;
use OCP\IRequest;
use OCP\ISession;
use OCP\Security\Bruteforce\IThrottler;
+use OCP\Security\CSRF\ICsrfValidator;
use Psr\Log\LoggerInterface;
use ReflectionMethod;
@@ -45,6 +46,7 @@ class CORSMiddleware extends Middleware {
Session $session,
IThrottler $throttler,
private readonly LoggerInterface $logger,
+ private readonly ICsrfValidator $csrfValidator,
) {
$this->request = $request;
$this->reflector = $reflector;
@@ -73,7 +75,7 @@ class CORSMiddleware extends Middleware {
$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;
}
// Skip CORS check for requests with AppAPI auth.
diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
index b8de09072ce..6c03ecd96f5 100644
--- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
@@ -41,6 +41,7 @@ use OCP\INavigationManager;
use OCP\IRequest;
use OCP\IURLGenerator;
use OCP\IUserSession;
+use OCP\Security\CSRF\ICsrfValidator;
use OCP\Security\Ip\IRemoteAddress;
use OCP\Util;
use Psr\Log\LoggerInterface;
@@ -68,6 +69,7 @@ class SecurityMiddleware extends Middleware {
private AuthorizedGroupMapper $groupAuthorizationMapper,
private IUserSession $userSession,
private IRemoteAddress $remoteAddress,
+ private readonly ICsrfValidator $csrfValidator,
) {
}
@@ -207,7 +209,7 @@ class SecurityMiddleware extends Middleware {
return false;
}
- return !$this->request->passesCSRFCheck();
+ return !$this->csrfValidator->validate($this->request);
}
private function isValidOCSRequest(): bool {
diff --git a/lib/private/EventSource.php b/lib/private/EventSource.php
index dbeda25049e..bf56003365c 100644
--- a/lib/private/EventSource.php
+++ b/lib/private/EventSource.php
@@ -10,6 +10,7 @@ namespace OC;
use OCP\IEventSource;
use OCP\IRequest;
+use OCP\Security\CSRF\ICsrfValidator;
class EventSource implements IEventSource {
private bool $fallback = false;
@@ -18,6 +19,7 @@ class EventSource implements IEventSource {
public function __construct(
private IRequest $request,
+ private ICsrfValidator $csrfValidator,
) {
}
@@ -54,7 +56,7 @@ class EventSource implements IEventSource {
header('Location: '.\OC::$WEBROOT);
exit();
}
- if (!$this->request->passesCSRFCheck()) {
+ if (!$this->csrfValidator->validate($this->request)) {
$this->send('error', 'Possible CSRF attack. Connection will be closed.');
$this->close();
exit();
diff --git a/lib/private/EventSourceFactory.php b/lib/private/EventSourceFactory.php
index 57888b1be4c..ad6abee3cb8 100644
--- a/lib/private/EventSourceFactory.php
+++ b/lib/private/EventSourceFactory.php
@@ -12,14 +12,19 @@ namespace OC;
use OCP\IEventSource;
use OCP\IEventSourceFactory;
use OCP\IRequest;
+use OCP\Security\CSRF\ICsrfValidator;
class EventSourceFactory implements IEventSourceFactory {
public function __construct(
private IRequest $request,
+ private ICsrfValidator $csrfValidator,
) {
}
public function create(): IEventSource {
- return new EventSource($this->request);
+ return new EventSource(
+ $this->request,
+ $this->csrfValidator,
+ );
}
}
diff --git a/lib/private/Security/CSRF/CsrfValidator.php b/lib/private/Security/CSRF/CsrfValidator.php
new file mode 100644
index 00000000000..b3f678f9518
--- /dev/null
+++ b/lib/private/Security/CSRF/CsrfValidator.php
@@ -0,0 +1,42 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OC\Security\CSRF;
+
+use OCP\IRequest;
+use OCP\Security\CSRF\ICsrfValidator;
+
+class CsrfValidator implements ICsrfValidator {
+ public function __construct(
+ private CsrfTokenManager $csrfTokenManager
+ ) {
+ }
+
+ public function validate(IRequest $request): bool {
+ if (!$request->passesStrictCookieCheck()) {
+ return false;
+ }
+
+ if ($request->getHeader('OCS-APIRequest') !== '') {
+ return true;
+ }
+
+ $token = $request->getParam('requesttoken', '');
+ if ($token === '') {
+ $token = $request->getHeader('REQUESTTOKEN');
+ }
+ if ($token === '') {
+ return false;
+ }
+
+ $token = new CsrfToken($token);
+
+ return $this->csrfTokenManager->isTokenValid($token);
+ }
+}
diff --git a/lib/private/Server.php b/lib/private/Server.php
index a0072b43ee2..7cb40c032fc 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -98,6 +98,7 @@ use OC\Security\Crypto;
use OC\Security\CSP\ContentSecurityPolicyManager;
use OC\Security\CSP\ContentSecurityPolicyNonceManager;
use OC\Security\CSRF\CsrfTokenManager;
+use OC\Security\CSRF\CsrfValidator;
use OC\Security\CSRF\TokenStorage\SessionStorage;
use OC\Security\Hasher;
use OC\Security\Ip\RemoteAddress;
@@ -209,6 +210,7 @@ use OCP\Remote\IInstanceFactory;
use OCP\RichObjectStrings\IValidator;
use OCP\Route\IRouter;
use OCP\Security\Bruteforce\IThrottler;
+use OCP\Security\CSRF\ICsrfValidator;
use OCP\Security\IContentSecurityPolicyManager;
use OCP\Security\ICredentialsManager;
use OCP\Security\ICrypto;
@@ -1408,6 +1410,8 @@ class Server extends ServerContainer implements IServerContainer {
$this->registerAlias(\OCP\Security\Ip\IFactory::class, \OC\Security\Ip\Factory::class);
+ $this->registerAlias(ICsrfValidator::class, CsrfValidator::class);
+
$this->connectDispatcher();
}
diff --git a/lib/private/legacy/OC_JSON.php b/lib/private/legacy/OC_JSON.php
index a9682e2cce7..567a4b6b3d6 100644
--- a/lib/private/legacy/OC_JSON.php
+++ b/lib/private/legacy/OC_JSON.php
@@ -7,6 +7,8 @@
*/
use OC\Authentication\TwoFactorAuth\Manager as TwoFactorAuthManager;
+use OCP\IRequest;
+use OCP\Security\CSRF\ICsrfValidator;
class OC_JSON {
/**
@@ -45,12 +47,16 @@ class OC_JSON {
* @suppress PhanDeprecatedFunction
*/
public static function callCheck() {
- if (!\OC::$server->getRequest()->passesStrictCookieCheck()) {
+ $request = OC::$server->get(IRequest::class);
+
+ if (!$request->passesStrictCookieCheck()) {
header('Location: '.\OC::$WEBROOT);
exit();
}
- if (!\OC::$server->getRequest()->passesCSRFCheck()) {
+ $csrfValidator = OC::$server->get(ICsrfValidator::class);
+
+ if (!$csrfValidator->validate($request)) {
$l = \OC::$server->getL10N('lib');
self::error([ 'data' => [ 'message' => $l->t('Token expired. Please reload page.'), 'error' => 'token_expired' ]]);
exit();
diff --git a/lib/public/IRequest.php b/lib/public/IRequest.php
index 18efd7a6d16..db82d72891e 100644
--- a/lib/public/IRequest.php
+++ b/lib/public/IRequest.php
@@ -177,6 +177,7 @@ interface IRequest {
*
* @return bool true if CSRF check passed
* @since 6.0.0
+ * @deprecated 30.0.0 use \OCP\Security\CSRF\ICsrfValidator::validate instead
*/
public function passesCSRFCheck(): bool;
diff --git a/lib/public/Security/CSRF/ICsrfValidator.php b/lib/public/Security/CSRF/ICsrfValidator.php
new file mode 100644
index 00000000000..2c64c825f04
--- /dev/null
+++ b/lib/public/Security/CSRF/ICsrfValidator.php
@@ -0,0 +1,24 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCP\Security\CSRF;
+
+use OCP\IRequest;
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+/**
+ * @since 31.0.0
+ */
+interface ICsrfValidator {
+ /**
+ * Check if a request uses a valid csrf token.
+ *
+ * @since 31.0.0
+ */
+ public function validate(IRequest $request): bool;
+}
diff --git a/tests/Core/Controller/LoginControllerTest.php b/tests/Core/Controller/LoginControllerTest.php
index 50ecbe5e19c..67c4349957f 100644
--- a/tests/Core/Controller/LoginControllerTest.php
+++ b/tests/Core/Controller/LoginControllerTest.php
@@ -15,6 +15,9 @@ use OC\Authentication\Login\LoginData;
use OC\Authentication\Login\LoginResult;
use OC\Authentication\TwoFactorAuth\Manager;
use OC\Core\Controller\LoginController;
+use OC\Security\CSRF\CsrfToken;
+use OC\Security\CSRF\CsrfTokenManager;
+use OC\Security\CSRF\CsrfValidator;
use OC\User\Session;
use OCP\App\IAppManager;
use OCP\AppFramework\Http\RedirectResponse;
@@ -79,6 +82,9 @@ class LoginControllerTest extends TestCase {
/** @var IAppManager|MockObject */
private $appManager;
+ private CsrfTokenManager $csrfTokenManager;
+ private CsrfValidator $csrfValidator;
+
protected function setUp(): void {
parent::setUp();
$this->request = $this->createMock(IRequest::class);
@@ -101,6 +107,8 @@ class LoginControllerTest extends TestCase {
->willReturnCallback(function ($text, $parameters = []) {
return vsprintf($text, $parameters);
});
+ $this->csrfTokenManager = $this->createMock(CsrfTokenManager::class);
+ $this->csrfValidator = new CsrfValidator($this->csrfTokenManager);
$this->request->method('getRemoteAddress')
@@ -126,6 +134,7 @@ class LoginControllerTest extends TestCase {
$this->notificationManager,
$this->l,
$this->appManager,
+ $this->csrfValidator,
);
}
@@ -437,9 +446,16 @@ class LoginControllerTest extends TestCase {
$password = 'secret';
$loginPageUrl = '/login?redirect_url=/apps/files';
$loginChain = $this->createMock(LoginChain::class);
- $this->request
- ->expects($this->once())
- ->method('passesCSRFCheck')
+ $this->request->expects($this->once())
+ ->method('passesStrictCookieCheck')
+ ->willReturn(true);
+ $this->request->expects($this->once())
+ ->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('foobar');
+ $this->csrfTokenManager->expects($this->once())
+ ->method('isTokenValid')
+ ->with(new CsrfToken('foobar'))
->willReturn(true);
$loginData = new LoginData(
$this->request,
@@ -472,9 +488,16 @@ class LoginControllerTest extends TestCase {
$user = 'MyUserName';
$password = 'secret';
$loginChain = $this->createMock(LoginChain::class);
- $this->request
- ->expects($this->once())
- ->method('passesCSRFCheck')
+ $this->request->expects($this->once())
+ ->method('passesStrictCookieCheck')
+ ->willReturn(true);
+ $this->request->expects($this->once())
+ ->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('foobar');
+ $this->csrfTokenManager->expects($this->once())
+ ->method('isTokenValid')
+ ->with(new CsrfToken('foobar'))
->willReturn(true);
$loginData = new LoginData(
$this->request,
@@ -504,9 +527,16 @@ class LoginControllerTest extends TestCase {
$password = 'secret';
$originalUrl = 'another%20url';
$loginChain = $this->createMock(LoginChain::class);
- $this->request
- ->expects($this->once())
- ->method('passesCSRFCheck')
+ $this->request->expects($this->once())
+ ->method('passesStrictCookieCheck')
+ ->willReturn(true);
+ $this->request->expects($this->once())
+ ->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('foobar');
+ $this->csrfTokenManager->expects($this->once())
+ ->method('isTokenValid')
+ ->with(new CsrfToken('foobar'))
->willReturn(false);
$this->userSession
->method('isLoggedIn')
@@ -533,9 +563,16 @@ class LoginControllerTest extends TestCase {
$originalUrl = 'another url';
$redirectUrl = 'http://localhost/another url';
$loginChain = $this->createMock(LoginChain::class);
- $this->request
- ->expects($this->once())
- ->method('passesCSRFCheck')
+ $this->request->expects($this->once())
+ ->method('passesStrictCookieCheck')
+ ->willReturn(true);
+ $this->request->expects($this->once())
+ ->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('foobar');
+ $this->csrfTokenManager->expects($this->once())
+ ->method('isTokenValid')
+ ->with(new CsrfToken('foobar'))
->willReturn(false);
$this->userSession
->method('isLoggedIn')
@@ -565,9 +602,16 @@ class LoginControllerTest extends TestCase {
$password = 'secret';
$redirectUrl = 'https://next.cloud/apps/mail';
$loginChain = $this->createMock(LoginChain::class);
- $this->request
- ->expects($this->once())
- ->method('passesCSRFCheck')
+ $this->request->expects($this->once())
+ ->method('passesStrictCookieCheck')
+ ->willReturn(true);
+ $this->request->expects($this->once())
+ ->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('foobar');
+ $this->csrfTokenManager->expects($this->once())
+ ->method('isTokenValid')
+ ->with(new CsrfToken('foobar'))
->willReturn(true);
$loginData = new LoginData(
$this->request,
@@ -596,9 +640,16 @@ class LoginControllerTest extends TestCase {
public function testToNotLeakLoginName() {
$loginChain = $this->createMock(LoginChain::class);
- $this->request
- ->expects($this->once())
- ->method('passesCSRFCheck')
+ $this->request->expects($this->once())
+ ->method('passesStrictCookieCheck')
+ ->willReturn(true);
+ $this->request->expects($this->once())
+ ->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('foobar');
+ $this->csrfTokenManager->expects($this->once())
+ ->method('isTokenValid')
+ ->with(new CsrfToken('foobar'))
->willReturn(true);
$loginPageUrl = '/login?redirect_url=/apps/files';
$loginData = new LoginData(
diff --git a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php
index 00538dda4b0..1e01f99a5d9 100644
--- a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php
@@ -10,6 +10,8 @@ use OC\AppFramework\Http\Request;
use OC\AppFramework\Middleware\Security\CORSMiddleware;
use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
use OC\AppFramework\Utility\ControllerMethodReflector;
+use OC\Security\CSRF\CsrfTokenManager;
+use OC\Security\CSRF\CsrfValidator;
use OC\User\Session;
use OCP\AppFramework\Http\JSONResponse;
use OCP\AppFramework\Http\Response;
@@ -28,6 +30,8 @@ class CORSMiddlewareTest extends \Test\TestCase {
private $session;
/** @var IThrottler|MockObject */
private $throttler;
+ private CsrfTokenManager $csrfTokenManager;
+ private CsrfValidator $csrfValidator;
/** @var CORSMiddlewareController */
private $controller;
private LoggerInterface $logger;
@@ -38,6 +42,8 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->session = $this->createMock(Session::class);
$this->throttler = $this->createMock(IThrottler::class);
$this->logger = $this->createMock(LoggerInterface::class);
+ $this->csrfTokenManager = $this->createMock(CsrfTokenManager::class);
+ $this->csrfValidator = new CsrfValidator($this->csrfTokenManager);
$this->controller = new CORSMiddlewareController(
'test',
$this->createMock(IRequest::class)
@@ -65,7 +71,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->createMock(IConfig::class)
);
$this->reflector->reflect($this->controller, $method);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$response = $middleware->afterController($this->controller, $method, new Response());
$headers = $response->getHeaders();
@@ -82,7 +88,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->createMock(IRequestId::class),
$this->createMock(IConfig::class)
);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$response = $middleware->afterController($this->controller, __FUNCTION__, new Response());
$headers = $response->getHeaders();
@@ -106,7 +112,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->createMock(IConfig::class)
);
$this->reflector->reflect($this->controller, $method);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$response = $middleware->afterController($this->controller, $method, new Response());
$headers = $response->getHeaders();
@@ -136,7 +142,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->createMock(IConfig::class)
);
$this->reflector->reflect($this->controller, $method);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$response = new Response();
$response->addHeader('AcCess-control-Allow-Credentials ', 'TRUE');
@@ -162,7 +168,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->createMock(IConfig::class)
);
$this->reflector->reflect($this->controller, $method);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$this->session->expects($this->once())
->method('isLoggedIn')
->willReturn(false);
@@ -196,7 +202,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->createMock(IConfig::class)
);
$this->reflector->reflect($this->controller, $method);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$this->session->expects($this->once())
->method('isLoggedIn')
->willReturn(true);
@@ -237,7 +243,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
->with($this->equalTo('user'), $this->equalTo('pass'))
->willReturn(true);
$this->reflector->reflect($this->controller, $method);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$middleware->beforeController($this->controller, $method);
}
@@ -270,7 +276,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
->with($this->equalTo('user'), $this->equalTo('pass'))
->will($this->throwException(new \OC\Authentication\Exceptions\PasswordLoginForbiddenException));
$this->reflector->reflect($this->controller, $method);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$middleware->beforeController($this->controller, $method);
}
@@ -303,7 +309,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
->with($this->equalTo('user'), $this->equalTo('pass'))
->willReturn(false);
$this->reflector->reflect($this->controller, $method);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$middleware->beforeController($this->controller, $method);
}
@@ -317,7 +323,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->createMock(IRequestId::class),
$this->createMock(IConfig::class)
);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$response = $middleware->afterException($this->controller, __FUNCTION__, new SecurityException('A security exception'));
$expected = new JSONResponse(['message' => 'A security exception'], 500);
@@ -333,7 +339,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->createMock(IRequestId::class),
$this->createMock(IConfig::class)
);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$response = $middleware->afterException($this->controller, __FUNCTION__, new SecurityException('A security exception', 501));
$expected = new JSONResponse(['message' => 'A security exception'], 501);
@@ -352,7 +358,7 @@ class CORSMiddlewareTest extends \Test\TestCase {
$this->createMock(IRequestId::class),
$this->createMock(IConfig::class)
);
- $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
+ $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger, $this->csrfValidator);
$middleware->afterException($this->controller, __FUNCTION__, new \Exception('A regular exception'));
}
}
diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
index fb457579fac..f4a2bd7e884 100644
--- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
+++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php
@@ -18,6 +18,9 @@ use OC\AppFramework\Middleware\Security\Exceptions\SecurityException;
use OC\Appframework\Middleware\Security\Exceptions\StrictCookieMissingException;
use OC\AppFramework\Middleware\Security\SecurityMiddleware;
use OC\AppFramework\Utility\ControllerMethodReflector;
+use OC\Security\CSRF\CsrfToken;
+use OC\Security\CSRF\CsrfTokenManager;
+use OC\Security\CSRF\CsrfValidator;
use OC\Settings\AuthorizedGroupMapper;
use OC\User\Session;
use OCP\App\IAppManager;
@@ -65,6 +68,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
private $userSession;
/** @var AuthorizedGroupMapper|\PHPUnit\Framework\MockObject\MockObject */
private $authorizedGroupMapper;
+ private CsrfTokenManager $csrfTokenManager;
+ private CsrfValidator $csrfValidator;
protected function setUp(): void {
parent::setUp();
@@ -81,6 +86,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->navigationManager = $this->createMock(INavigationManager::class);
$this->urlGenerator = $this->createMock(IURLGenerator::class);
$this->l10n = $this->createMock(IL10N::class);
+ $this->csrfTokenManager = $this->createMock(CsrfTokenManager::class);
+ $this->csrfValidator = new CsrfValidator($this->csrfTokenManager);
$this->middleware = $this->getMiddleware(true, true, false);
$this->secException = new SecurityException('hey', false);
$this->secAjaxException = new SecurityException('hey', true);
@@ -108,7 +115,8 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->l10n,
$this->authorizedGroupMapper,
$this->userSession,
- $remoteIpAddress
+ $remoteIpAddress,
+ $this->csrfValidator,
);
}
@@ -321,12 +329,18 @@ class SecurityMiddlewareTest extends \Test\TestCase {
public function testCsrfCheck(string $method): void {
$this->expectException(\OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException::class);
- $this->request->expects($this->once())
- ->method('passesCSRFCheck')
- ->willReturn(false);
- $this->request->expects($this->once())
+ $this->request->expects($this->exactly(2))
->method('passesStrictCookieCheck')
->willReturn(true);
+ $this->request->expects($this->once())
+ ->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('');
+ $this->request->expects($this->once())
+ ->method('getHeader')
+ ->with('REQUESTTOKEN')
+ ->willReturn('');
+
$this->reader->reflect($this->controller, $method);
$this->middleware->beforeController($this->controller, $method);
}
@@ -347,11 +361,20 @@ class SecurityMiddlewareTest extends \Test\TestCase {
* @dataProvider dataPublicPage
*/
public function testPassesCsrfCheck(string $method): void {
- $this->request->expects($this->once())
- ->method('passesCSRFCheck')
+ $this->request->expects($this->exactly(2))
+ ->method('passesStrictCookieCheck')
->willReturn(true);
$this->request->expects($this->once())
- ->method('passesStrictCookieCheck')
+ ->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('');
+ $this->request->expects($this->once())
+ ->method('getHeader')
+ ->with('REQUESTTOKEN')
+ ->willReturn('foobar');
+ $this->csrfTokenManager->expects($this->once())
+ ->method('isTokenValid')
+ ->with(new CsrfToken('foobar'))
->willReturn(true);
$this->reader->reflect($this->controller, $method);
@@ -364,12 +387,21 @@ class SecurityMiddlewareTest extends \Test\TestCase {
public function testFailCsrfCheck(string $method): void {
$this->expectException(\OC\AppFramework\Middleware\Security\Exceptions\CrossSiteRequestForgeryException::class);
- $this->request->expects($this->once())
- ->method('passesCSRFCheck')
- ->willReturn(false);
- $this->request->expects($this->once())
+ $this->request->expects($this->exactly(2))
->method('passesStrictCookieCheck')
->willReturn(true);
+ $this->request->expects($this->once())
+ ->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('');
+ $this->request->expects($this->once())
+ ->method('getHeader')
+ ->with('REQUESTTOKEN')
+ ->willReturn('foobar');
+ $this->csrfTokenManager->expects($this->once())
+ ->method('isTokenValid')
+ ->with(new CsrfToken('foobar'))
+ ->willReturn(false);
$this->reader->reflect($this->controller, $method);
$this->middleware->beforeController($this->controller, $method);
@@ -386,6 +418,12 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->request->expects($this->once())
->method('passesStrictCookieCheck')
->willReturn(false);
+ $this->request->expects($this->never())
+ ->method('getParam');
+ $this->request->expects($this->never())
+ ->method('getHeader');
+ $this->csrfTokenManager->expects($this->never())
+ ->method('isTokenValid');
$this->reader->reflect($this->controller, $method);
$this->middleware->beforeController($this->controller, $method);
@@ -441,6 +479,9 @@ class SecurityMiddlewareTest extends \Test\TestCase {
$this->request
->method('getHeader')
->willReturnCallback(function ($header) use ($hasOcsApiHeader, $hasBearerAuth) {
+ if ($header === 'REQUESTTOKEN') {
+ return '';
+ }
if ($header === 'OCS-APIREQUEST' && $hasOcsApiHeader) {
return 'true';
}
@@ -449,9 +490,15 @@ class SecurityMiddlewareTest extends \Test\TestCase {
}
return '';
});
- $this->request->expects($this->once())
+ $this->request->expects($this->exactly(2))
->method('passesStrictCookieCheck')
->willReturn(true);
+ $this->request->expects($this->once())
+ ->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('');
+ $this->csrfTokenManager->expects($this->never())
+ ->method('isTokenValid');
$controller = new $controllerClass('test', $this->request);
diff --git a/tests/lib/EventSourceFactoryTest.php b/tests/lib/EventSourceFactoryTest.php
index c5e22a8fe34..abd23e53bb5 100644
--- a/tests/lib/EventSourceFactoryTest.php
+++ b/tests/lib/EventSourceFactoryTest.php
@@ -9,11 +9,13 @@ namespace Test;
use OC\EventSourceFactory;
use OCP\IEventSource;
use OCP\IRequest;
+use OCP\Security\CSRF\ICsrfValidator;
class EventSourceFactoryTest extends TestCase {
public function testCreate(): void {
$request = $this->createMock(IRequest::class);
- $factory = new EventSourceFactory($request);
+ $csrfValidator = $this->createMock(ICsrfValidator::class);
+ $factory = new EventSourceFactory($request, $csrfValidator);
$instance = $factory->create();
$this->assertInstanceOf(IEventSource::class, $instance);
diff --git a/tests/lib/Security/CSRF/CsrfTokenManagerTest.php b/tests/lib/Security/CSRF/CsrfTokenManagerTest.php
index c4fd480654d..8c19bc6e82d 100644
--- a/tests/lib/Security/CSRF/CsrfTokenManagerTest.php
+++ b/tests/lib/Security/CSRF/CsrfTokenManagerTest.php
@@ -131,14 +131,14 @@ class CsrfTokenManagerTest extends \Test\TestCase {
$xorB64 = 'BQcF';
$tokenVal = sprintf('%s:%s', $xorB64, base64_encode($a));
$this->storageInterface
- ->expects($this->once())
- ->method('hasToken')
- ->willReturn(true);
+ ->expects($this->once())
+ ->method('hasToken')
+ ->willReturn(true);
$token = new \OC\Security\CSRF\CsrfToken($tokenVal);
$this->storageInterface
- ->expects($this->once())
- ->method('getToken')
- ->willReturn($b);
+ ->expects($this->once())
+ ->method('getToken')
+ ->willReturn($b);
$this->assertSame(true, $this->csrfTokenManager->isTokenValid($token));
}
diff --git a/tests/lib/Security/CSRF/CsrfValidatorTest.php b/tests/lib/Security/CSRF/CsrfValidatorTest.php
new file mode 100644
index 00000000000..30aac3c7039
--- /dev/null
+++ b/tests/lib/Security/CSRF/CsrfValidatorTest.php
@@ -0,0 +1,96 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace Test\Security\CSRF;
+
+use OC\Security\CSRF\CsrfTokenManager;
+use OC\Security\CSRF\CsrfValidator;
+use OCP\IRequest;
+use Test\TestCase;
+
+class CsrfValidatorTest extends TestCase {
+ private CsrfTokenManager $csrfTokenManager;
+ private CsrfValidator $csrfValidator;
+
+ protected function setUp(): void {
+ parent::setUp();
+
+ $this->csrfTokenManager = $this->createMock(CsrfTokenManager::class);
+ $this->csrfValidator = new CsrfValidator($this->csrfTokenManager);
+ }
+
+ public function testFailStrictCookieCheck(): void {
+ $request = $this->createMock(IRequest::class);
+ $request->method('passesStrictCookieCheck')
+ ->willReturn(false);
+
+ $this->assertFalse($this->csrfValidator->validate($request));
+ }
+
+ public function testFailMissingToken(): void {
+ $request = $this->createMock(IRequest::class);
+ $request->method('passesStrictCookieCheck')
+ ->willReturn(true);
+ $request->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('');
+ $request->method('getHeader')
+ ->with('REQUESTTOKEN')
+ ->willReturn('');
+
+ $this->assertFalse($this->csrfValidator->validate($request));
+ }
+
+ public function testFailInvalidToken(): void {
+ $request = $this->createMock(IRequest::class);
+ $request->method('passesStrictCookieCheck')
+ ->willReturn(true);
+ $request->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('token123');
+ $request->method('getHeader')
+ ->with('REQUESTTOKEN')
+ ->willReturn('');
+
+ $this->csrfTokenManager
+ ->method('isTokenValid')
+ ->willReturn(false);
+
+ $this->assertFalse($this->csrfValidator->validate($request));
+ }
+
+ public function testPass(): void {
+ $request = $this->createMock(IRequest::class);
+ $request->method('passesStrictCookieCheck')
+ ->willReturn(true);
+ $request->method('getParam')
+ ->with('requesttoken', '')
+ ->willReturn('token123');
+ $request->method('getHeader')
+ ->with('REQUESTTOKEN')
+ ->willReturn('');
+
+ $this->csrfTokenManager
+ ->method('isTokenValid')
+ ->willReturn(true);
+
+ $this->assertTrue($this->csrfValidator->validate($request));
+ }
+
+ public function testPassWithOCSAPIRequestHeader(): void {
+ $request = $this->createMock(IRequest::class);
+ $request->method('passesStrictCookieCheck')
+ ->willReturn(true);
+ $request->method('getHeader')
+ ->with('OCS-APIRequest', '')
+ ->willReturn('yes');
+
+ $this->assertTrue($this->csrfValidator->validate($request));
+ }
+}