feat(security): Add PHP \Attribute for remaining security annotations

Signed-off-by: Joas Schilling <coding@schilljs.com>

4 files changed, 132 insertions, 27 deletions

diff --git a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
index 30ba8d8d6e4..e177a612d96 100644
--- a/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/CORSMiddleware.php
@@ -33,10 +33,13 @@ use OC\Security\Bruteforce\Throttler;
 use OC\User\Session;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Http;
+use OCP\AppFramework\Http\Attribute\CORS;
+use OCP\AppFramework\Http\Attribute\PublicPage;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\Response;
 use OCP\AppFramework\Middleware;
 use OCP\IRequest;
+use ReflectionMethod;
 
 /**
  * This middleware sets the correct CORS headers on a response if the
@@ -81,9 +84,12 @@ class CORSMiddleware extends Middleware {
 	 * @since 6.0.0
 	 */
 	public function beforeController($controller, $methodName) {
+		$reflectionMethod = new ReflectionMethod($controller, $methodName);
+
 		// ensure that @CORS annotated API routes are not used in conjunction
 		// with session authentication since this enables CSRF attack vectors
-		if ($this->reflector->hasAnnotation('CORS') && (!$this->reflector->hasAnnotation('PublicPage') || $this->session->isLoggedIn())) {
+		if ($this->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class) &&
+			(!$this->hasAnnotationOrAttribute($reflectionMethod, 'PublicPage', PublicPage::class) || $this->session->isLoggedIn())) {
 			$user = array_key_exists('PHP_AUTH_USER', $this->request->server) ? $this->request->server['PHP_AUTH_USER'] : null;
 			$pass = array_key_exists('PHP_AUTH_PW', $this->request->server) ? $this->request->server['PHP_AUTH_PW'] : null;
 
@@ -103,7 +109,28 @@ class CORSMiddleware extends Middleware {
 	}
 
 	/**
-	 * This is being run after a successful controllermethod call and allows
+	 * @template T
+	 *
+	 * @param ReflectionMethod $reflectionMethod
+	 * @param string $annotationName
+	 * @param class-string<T> $attributeClass
+	 * @return boolean
+	 */
+	protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, string $annotationName, string $attributeClass): bool {
+		if ($this->reflector->hasAnnotation($annotationName)) {
+			return true;
+		}
+
+
+		if (!empty($reflectionMethod->getAttributes($attributeClass))) {
+			return true;
+		}
+
+		return false;
+	}
+
+	/**
+	 * This is being run after a successful controller method call and allows
 	 * the manipulation of a Response object. The middleware is run in reverse order
 	 *
 	 * @param Controller $controller the controller that is being called
@@ -114,23 +141,25 @@ class CORSMiddleware extends Middleware {
 	 * @throws SecurityException
 	 */
 	public function afterController($controller, $methodName, Response $response) {
-		// only react if its a CORS request and if the request sends origin and
+		// only react if it's a CORS request and if the request sends origin and
 
-		if (isset($this->request->server['HTTP_ORIGIN']) &&
-			$this->reflector->hasAnnotation('CORS')) {
-			// allow credentials headers must not be true or CSRF is possible
-			// otherwise
-			foreach ($response->getHeaders() as $header => $value) {
-				if (strtolower($header) === 'access-control-allow-credentials' &&
-				   strtolower(trim($value)) === 'true') {
-					$msg = 'Access-Control-Allow-Credentials must not be '.
-						   'set to true in order to prevent CSRF';
-					throw new SecurityException($msg);
+		if (isset($this->request->server['HTTP_ORIGIN'])) {
+			$reflectionMethod = new ReflectionMethod($controller, $methodName);
+			if ($this->hasAnnotationOrAttribute($reflectionMethod, 'CORS', CORS::class)) {
+				// allow credentials headers must not be true or CSRF is possible
+				// otherwise
+				foreach ($response->getHeaders() as $header => $value) {
+					if (strtolower($header) === 'access-control-allow-credentials' &&
+					   strtolower(trim($value)) === 'true') {
+						$msg = 'Access-Control-Allow-Credentials must not be '.
+							   'set to true in order to prevent CSRF';
+						throw new SecurityException($msg);
+					}
 				}
-			}
 
-			$origin = $this->request->server['HTTP_ORIGIN'];
-			$response->addHeader('Access-Control-Allow-Origin', $origin);
+				$origin = $this->request->server['HTTP_ORIGIN'];
+				$response->addHeader('Access-Control-Allow-Origin', $origin);
+			}
 		}
 		return $response;
 	}
diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php
index 0ee9fdff881..a72a7a40016 100644
--- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php
@@ -26,11 +26,13 @@ namespace OC\AppFramework\Middleware\Security;
 use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException;
 use OC\AppFramework\Utility\ControllerMethodReflector;
 use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired;
 use OCP\AppFramework\Middleware;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\ISession;
 use OCP\IUserSession;
 use OCP\User\Backend\IPasswordConfirmationBackend;
+use ReflectionMethod;
 
 class PasswordConfirmationMiddleware extends Middleware {
 	/** @var ControllerMethodReflector */
@@ -68,7 +70,9 @@ class PasswordConfirmationMiddleware extends Middleware {
 	 * @throws NotConfirmedException
 	 */
 	public function beforeController($controller, $methodName) {
-		if ($this->reflector->hasAnnotation('PasswordConfirmationRequired')) {
+		$reflectionMethod = new ReflectionMethod($controller, $methodName);
+
+		if ($this->hasAnnotationOrAttribute($reflectionMethod, 'PasswordConfirmationRequired', PasswordConfirmationRequired::class)) {
 			$user = $this->userSession->getUser();
 			$backendClassName = '';
 			if ($user !== null) {
@@ -89,4 +93,24 @@ class PasswordConfirmationMiddleware extends Middleware {
 			}
 		}
 	}
+
+	/**
+	 * @template T
+	 *
+	 * @param ReflectionMethod $reflectionMethod
+	 * @param string $annotationName
+	 * @param class-string<T> $attributeClass
+	 * @return boolean
+	 */
+	protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, string $annotationName, string $attributeClass): bool {
+		if (!empty($reflectionMethod->getAttributes($attributeClass))) {
+			return true;
+		}
+
+		if ($this->reflector->hasAnnotation($annotationName)) {
+			return true;
+		}
+
+		return false;
+	}
 }
diff --git a/lib/private/AppFramework/Middleware/Security/ReloadExecutionMiddleware.php b/lib/private/AppFramework/Middleware/Security/ReloadExecutionMiddleware.php
index 5a635e3f284..a6a07538345 100644
--- a/lib/private/AppFramework/Middleware/Security/ReloadExecutionMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/ReloadExecutionMiddleware.php
@@ -58,7 +58,7 @@ class ReloadExecutionMiddleware extends Middleware {
 
 			return new RedirectResponse($this->urlGenerator->linkToRouteAbsolute(
 				'core.login.showLoginForm',
-				['clear' => true] // this param the the code in login.js may be removed when the "Clear-Site-Data" is working in the browsers
+				['clear' => true] // this param the code in login.js may be removed when the "Clear-Site-Data" is working in the browsers
 			));
 		}
 
diff --git a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
index eb8c1b8dc43..8e0794f9645 100644
--- a/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
+++ b/lib/private/AppFramework/Middleware/Security/SecurityMiddleware.php
@@ -48,6 +48,12 @@ use OC\Settings\AuthorizedGroupMapper;
 use OCP\App\AppPathNotFoundException;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting;
+use OCP\AppFramework\Http\Attribute\NoAdminRequired;
+use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
+use OCP\AppFramework\Http\Attribute\PublicPage;
+use OCP\AppFramework\Http\Attribute\StrictCookiesRequired;
+use OCP\AppFramework\Http\Attribute\SubAdminRequired;
 use OCP\AppFramework\Http\JSONResponse;
 use OCP\AppFramework\Http\RedirectResponse;
 use OCP\AppFramework\Http\Response;
@@ -61,6 +67,7 @@ use OCP\IURLGenerator;
 use OCP\IUserSession;
 use OCP\Util;
 use Psr\Log\LoggerInterface;
+use ReflectionMethod;
 
 /**
  * Used to do all the authentication and checking stuff for a controller method
@@ -145,22 +152,24 @@ class SecurityMiddleware extends Middleware {
 			$this->navigationManager->setActiveEntry('spreed');
 		}
 
+		$reflectionMethod = new ReflectionMethod($controller, $methodName);
+
 		// security checks
-		$isPublicPage = $this->reflector->hasAnnotation('PublicPage');
+		$isPublicPage = $this->hasAnnotationOrAttribute($reflectionMethod, 'PublicPage', PublicPage::class);
 		if (!$isPublicPage) {
 			if (!$this->isLoggedIn) {
 				throw new NotLoggedInException();
 			}
 			$authorized = false;
-			if ($this->reflector->hasAnnotation('AuthorizedAdminSetting')) {
+			if ($this->hasAnnotationOrAttribute($reflectionMethod, 'AuthorizedAdminSetting', AuthorizedAdminSetting::class)) {
 				$authorized = $this->isAdminUser;
 
-				if (!$authorized && $this->reflector->hasAnnotation('SubAdminRequired')) {
+				if (!$authorized && $this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)) {
 					$authorized = $this->isSubAdmin;
 				}
 
 				if (!$authorized) {
-					$settingClasses = explode(';', $this->reflector->getAnnotationParameter('AuthorizedAdminSetting', 'settings'));
+					$settingClasses = $this->getAuthorizedAdminSettingClasses($reflectionMethod);
 					$authorizedClasses = $this->groupAuthorizationMapper->findAllClassesForUser($this->userSession->getUser());
 					foreach ($settingClasses as $settingClass) {
 						$authorized = in_array($settingClass, $authorizedClasses, true);
@@ -174,14 +183,14 @@ class SecurityMiddleware extends Middleware {
 					throw new NotAdminException($this->l10n->t('Logged in user must be an admin, a sub admin or gotten special right to access this setting'));
 				}
 			}
-			if ($this->reflector->hasAnnotation('SubAdminRequired')
+			if ($this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)
 				&& !$this->isSubAdmin
 				&& !$this->isAdminUser
 				&& !$authorized) {
 				throw new NotAdminException($this->l10n->t('Logged in user must be an admin or sub admin'));
 			}
-			if (!$this->reflector->hasAnnotation('SubAdminRequired')
-				&& !$this->reflector->hasAnnotation('NoAdminRequired')
+			if (!$this->hasAnnotationOrAttribute($reflectionMethod, 'SubAdminRequired', SubAdminRequired::class)
+				&& !$this->hasAnnotationOrAttribute($reflectionMethod, 'NoAdminRequired', NoAdminRequired::class)
 				&& !$this->isAdminUser
 				&& !$authorized) {
 				throw new NotAdminException($this->l10n->t('Logged in user must be an admin'));
@@ -189,14 +198,15 @@ class SecurityMiddleware extends Middleware {
 		}
 
 		// Check for strict cookie requirement
-		if ($this->reflector->hasAnnotation('StrictCookieRequired') || !$this->reflector->hasAnnotation('NoCSRFRequired')) {
+		if ($this->hasAnnotationOrAttribute($reflectionMethod, 'StrictCookieRequired', StrictCookiesRequired::class) ||
+			!$this->hasAnnotationOrAttribute($reflectionMethod, 'NoCSRFRequired', NoCSRFRequired::class)) {
 			if (!$this->request->passesStrictCookieCheck()) {
 				throw new StrictCookieMissingException();
 			}
 		}
 		// CSRF check - also registers the CSRF token since the session may be closed later
 		Util::callRegister();
-		if (!$this->reflector->hasAnnotation('NoCSRFRequired')) {
+		if (!$this->hasAnnotationOrAttribute($reflectionMethod, 'NoCSRFRequired', NoCSRFRequired::class)) {
 			/*
 			 * 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
@@ -233,6 +243,48 @@ class SecurityMiddleware extends Middleware {
 	}
 
 	/**
+	 * @template T
+	 *
+	 * @param ReflectionMethod $reflectionMethod
+	 * @param string $annotationName
+	 * @param class-string<T> $attributeClass
+	 * @return boolean
+	 */
+	protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod, string $annotationName, string $attributeClass): bool {
+		if (!empty($reflectionMethod->getAttributes($attributeClass))) {
+			return true;
+		}
+
+		if ($this->reflector->hasAnnotation($annotationName)) {
+			return true;
+		}
+
+		return false;
+	}
+
+	/**
+	 * @param ReflectionMethod $reflectionMethod
+	 * @return string[]
+	 */
+	protected function getAuthorizedAdminSettingClasses(ReflectionMethod $reflectionMethod): array {
+		$classes = [];
+		if ($this->reflector->hasAnnotation('AuthorizedAdminSetting')) {
+			$classes = explode(';', $this->reflector->getAnnotationParameter('AuthorizedAdminSetting', 'settings'));
+		}
+
+		$attributes = $reflectionMethod->getAttributes(AuthorizedAdminSetting::class);
+		if (!empty($attributes)) {
+			foreach ($attributes as $attribute) {
+				/** @var AuthorizedAdminSetting $setting */
+				$setting = $attribute->newInstance();
+				$classes[] = $setting->getSettings();
+			}
+		}
+
+		return $classes;
+	}
+
+	/**
 	 * If an SecurityException is being caught, ajax requests return a JSON error
 	 * response and non ajax requests redirect to the index
 	 *