From 12f85438157b7153f51581485fc24ef4090139b7 Mon Sep 17 00:00:00 2001 From: jld3103 Date: Mon, 3 Jul 2023 18:59:51 +0200 Subject: Rewrite OCS CSRF check to be readable Signed-off-by: jld3103 --- .../Middleware/Security/SecurityMiddleware.php | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) (limited to 'lib/private/AppFramework') diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php index 04f79361bc8..db6c7a02c77 100644 --- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php @@ -206,7 +206,7 @@ class SecurityMiddleware extends Middleware { } // CSRF check - also registers the CSRF token since the session may be closed later Util::callRegister(); - if (!$this->hasAnnotationOrAttribute($reflectionMethod, 'NoCSRFRequired', NoCSRFRequired::class)) { + if ($this->isInvalidCSRFRequired($reflectionMethod)) { /* * 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 @@ -215,12 +215,7 @@ class SecurityMiddleware extends Middleware { * 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' || - str_starts_with($this->request->getHeader('Authorization'), 'Bearer ') - ) - )) { + if (!$controller instanceof OCSController || !$this->isValidOCSRequest()) { throw new CrossSiteRequestForgeryException(); } } @@ -242,6 +237,19 @@ class SecurityMiddleware extends Middleware { } } + private function isInvalidCSRFRequired(ReflectionMethod $reflectionMethod): bool { + if ($this->hasAnnotationOrAttribute($reflectionMethod, 'NoCSRFRequired', NoCSRFRequired::class)) { + return false; + } + + return !$this->request->passesCSRFCheck(); + } + + private function isValidOCSRequest(): bool { + return $this->request->getHeader('OCS-APIREQUEST') === 'true' + || str_starts_with($this->request->getHeader('Authorization'), 'Bearer '); + } + /** * @template T * -- cgit v1.2.3