feat(Security): Warn about using annotations instead of attributes

Signed-off-by: provokateurin <kate@provokateurin.de>

lib/private/AppFramework/DependencyInjection/DIContainer.php

@@ -207,7 +207,8 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta
 					$c->get(IRequest::class),
 					$c->get(IControllerMethodReflector::class),
 					$c->get(IUserSession::class),
-					$c->get(IThrottler::class)
+					$c->get(IThrottler::class),
+					$c->get(LoggerInterface::class)
 				)
 			);
 			$dispatcher->registerMiddleware(
@@ -251,6 +252,7 @@ public function __construct(string $appName, array $urlParams = [], ?ServerConta
 					$c->get(IUserSession::class),
 					$c->get(ITimeFactory::class),
 					$c->get(\OC\Authentication\Token\IProvider::class),
+					$c->get(LoggerInterface::class),
 				)
 			);
 			$dispatcher->registerMiddleware(

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

@@ -21,6 +21,7 @@
 use OCP\IRequest;
 use OCP\ISession;
 use OCP\Security\Bruteforce\IThrottler;
+use Psr\Log\LoggerInterface;
 use ReflectionMethod;
 
 /**
@@ -42,7 +43,9 @@ class CORSMiddleware extends Middleware {
 	public function __construct(IRequest $request,
 		ControllerMethodReflector $reflector,
 		Session $session,
-		IThrottler $throttler) {
+		IThrottler $throttler,
+		private readonly LoggerInterface $logger,
+	) {
 		$this->request = $request;
 		$this->reflector = $reflector;
 		$this->session = $session;
@@ -103,6 +106,7 @@ protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod,
 
 
 		if (!empty($reflectionMethod->getAttributes($attributeClass))) {
+			$this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . $annotationName . ' annotation and should use the #[' . $attributeClass . '] attribute instead');
 			return true;
 		}
 

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

@@ -20,6 +20,7 @@
 use OCP\IUserSession;
 use OCP\Session\Exceptions\SessionNotAvailableException;
 use OCP\User\Backend\IPasswordConfirmationBackend;
+use Psr\Log\LoggerInterface;
 use ReflectionMethod;
 
 class PasswordConfirmationMiddleware extends Middleware {
@@ -48,6 +49,7 @@ public function __construct(ControllerMethodReflector $reflector,
 		IUserSession $userSession,
 		ITimeFactory $timeFactory,
 		IProvider $tokenProvider,
+		private readonly LoggerInterface $logger,
 	) {
 		$this->reflector = $reflector;
 		$this->session = $session;
@@ -113,6 +115,7 @@ protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod,
 		}
 
 		if ($this->reflector->hasAnnotation($annotationName)) {
+			$this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . $annotationName . ' annotation and should use the #[' . $attributeClass . '] attribute instead');
 			return true;
 		}
 

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

@@ -243,6 +243,7 @@ protected function hasAnnotationOrAttribute(ReflectionMethod $reflectionMethod,
 		}
 
 		if ($this->reflector->hasAnnotation($annotationName)) {
+			$this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . $annotationName . ' annotation and should use the #[' . $attributeClass . '] attribute instead');
 			return true;
 		}
 

tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php

@@ -18,6 +18,7 @@
 use OCP\IRequestId;
 use OCP\Security\Bruteforce\IThrottler;
 use PHPUnit\Framework\MockObject\MockObject;
+use Psr\Log\LoggerInterface;
 use Test\AppFramework\Middleware\Security\Mock\CORSMiddlewareController;
 
 class CORSMiddlewareTest extends \Test\TestCase {
@@ -29,12 +30,14 @@ class CORSMiddlewareTest extends \Test\TestCase {
 	private $throttler;
 	/** @var CORSMiddlewareController */
 	private $controller;
+	private LoggerInterface $logger;
 
 	protected function setUp(): void {
 		parent::setUp();
 		$this->reflector = new ControllerMethodReflector();
 		$this->session = $this->createMock(Session::class);
 		$this->throttler = $this->createMock(IThrottler::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->controller = new CORSMiddlewareController(
 			'test',
 			$this->createMock(IRequest::class)
@@ -62,7 +65,7 @@ public function testSetCORSAPIHeader(string $method): void {
 			$this->createMock(IConfig::class)
 		);
 		$this->reflector->reflect($this->controller, $method);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 
 		$response = $middleware->afterController($this->controller, $method, new Response());
 		$headers = $response->getHeaders();
@@ -79,7 +82,7 @@ public function testNoAnnotationNoCORSHEADER(): void {
 			$this->createMock(IRequestId::class),
 			$this->createMock(IConfig::class)
 		);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 
 		$response = $middleware->afterController($this->controller, __FUNCTION__, new Response());
 		$headers = $response->getHeaders();
@@ -103,7 +106,7 @@ public function testNoOriginHeaderNoCORSHEADER(string $method): void {
 			$this->createMock(IConfig::class)
 		);
 		$this->reflector->reflect($this->controller, $method);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 
 		$response = $middleware->afterController($this->controller, $method, new Response());
 		$headers = $response->getHeaders();
@@ -133,7 +136,7 @@ public function testCorsIgnoredIfWithCredentialsHeaderPresent(string $method): v
 			$this->createMock(IConfig::class)
 		);
 		$this->reflector->reflect($this->controller, $method);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 
 		$response = new Response();
 		$response->addHeader('AcCess-control-Allow-Credentials ', 'TRUE');
@@ -159,7 +162,7 @@ public function testNoCORSOnAnonymousPublicPage(string $method): void {
 			$this->createMock(IConfig::class)
 		);
 		$this->reflector->reflect($this->controller, $method);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 		$this->session->expects($this->once())
 			->method('isLoggedIn')
 			->willReturn(false);
@@ -193,7 +196,7 @@ public function testCORSShouldNeverAllowCookieAuth(string $method): void {
 			$this->createMock(IConfig::class)
 		);
 		$this->reflector->reflect($this->controller, $method);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 		$this->session->expects($this->once())
 			->method('isLoggedIn')
 			->willReturn(true);
@@ -234,7 +237,7 @@ public function testCORSShouldRelogin(string $method): void {
 			->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);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 
 		$middleware->beforeController($this->controller, $method);
 	}
@@ -267,7 +270,7 @@ public function testCORSShouldFailIfPasswordLoginIsForbidden(string $method): vo
 			->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);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 
 		$middleware->beforeController($this->controller, $method);
 	}
@@ -300,7 +303,7 @@ public function testCORSShouldNotAllowCookieAuth(string $method): void {
 			->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);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 
 		$middleware->beforeController($this->controller, $method);
 	}
@@ -314,7 +317,7 @@ public function testAfterExceptionWithSecurityExceptionNoStatus() {
 			$this->createMock(IRequestId::class),
 			$this->createMock(IConfig::class)
 		);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 		$response = $middleware->afterException($this->controller, __FUNCTION__, new SecurityException('A security exception'));
 
 		$expected = new JSONResponse(['message' => 'A security exception'], 500);
@@ -330,7 +333,7 @@ public function testAfterExceptionWithSecurityExceptionWithStatus() {
 			$this->createMock(IRequestId::class),
 			$this->createMock(IConfig::class)
 		);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 		$response = $middleware->afterException($this->controller, __FUNCTION__, new SecurityException('A security exception', 501));
 
 		$expected = new JSONResponse(['message' => 'A security exception'], 501);
@@ -349,7 +352,7 @@ public function testAfterExceptionWithRegularException() {
 			$this->createMock(IRequestId::class),
 			$this->createMock(IConfig::class)
 		);
-		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler);
+		$middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger);
 		$middleware->afterException($this->controller, __FUNCTION__, new \Exception('A regular exception'));
 	}
 }

tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php

@@ -16,6 +16,7 @@
 use OCP\ISession;
 use OCP\IUser;
 use OCP\IUserSession;
+use Psr\Log\LoggerInterface;
 use Test\AppFramework\Middleware\Security\Mock\PasswordConfirmationMiddlewareController;
 use Test\TestCase;
 
@@ -35,6 +36,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
 	/** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
 	private $timeFactory;
 	private IProvider|\PHPUnit\Framework\MockObject\MockObject $tokenProvider;
+	private LoggerInterface $logger;
 
 	protected function setUp(): void {
 		$this->reflector = new ControllerMethodReflector();
@@ -43,6 +45,7 @@ protected function setUp(): void {
 		$this->user = $this->createMock(IUser::class);
 		$this->timeFactory = $this->createMock(ITimeFactory::class);
 		$this->tokenProvider = $this->createMock(IProvider::class);
+		$this->logger = $this->createMock(LoggerInterface::class);
 		$this->controller = new PasswordConfirmationMiddlewareController(
 			'test',
 			$this->createMock(IRequest::class)
@@ -54,6 +57,7 @@ protected function setUp(): void {
 			$this->userSession,
 			$this->timeFactory,
 			$this->tokenProvider,
+			$this->logger,
 		);
 	}