From 98422d40b45647d74532a83eefd722758f55a82f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Wed, 11 Mar 2026 11:13:30 +0100 Subject: [PATCH 1/6] fix: Move hasAnnotationOrAttribute to the reflector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../Utility/ControllerMethodReflector.php | 41 +++++++++++++++---- .../Utility/IControllerMethodReflector.php | 11 +++++ 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/lib/private/AppFramework/Utility/ControllerMethodReflector.php b/lib/private/AppFramework/Utility/ControllerMethodReflector.php index b63be92dc3bee..1360ca2737ce8 100644 --- a/lib/private/AppFramework/Utility/ControllerMethodReflector.php +++ b/lib/private/AppFramework/Utility/ControllerMethodReflector.php @@ -9,28 +9,35 @@ namespace OC\AppFramework\Utility; use OCP\AppFramework\Utility\IControllerMethodReflector; +use Psr\Log\LoggerInterface; /** * Reads and parses annotations from doc comments */ class ControllerMethodReflector implements IControllerMethodReflector { - public $annotations = []; - private $types = []; - private $parameters = []; + public array $annotations = []; + private array $types = []; + private array $parameters = []; private array $ranges = []; private int $startLine = 0; private string $file = ''; + private ?\ReflectionMethod $reflectionMethod = null; + + public function __construct( + private readonly LoggerInterface $logger, + ) { + } /** * @param object $object an object or classname * @param string $method the method which we want to inspect */ public function reflect($object, string $method) { - $reflection = new \ReflectionMethod($object, $method); - $this->startLine = $reflection->getStartLine(); - $this->file = $reflection->getFileName(); + $this->reflectionMethod = new \ReflectionMethod($object, $method); + $this->startLine = $this->reflectionMethod->getStartLine(); + $this->file = $this->reflectionMethod->getFileName(); - $docs = $reflection->getDocComment(); + $docs = $this->reflectionMethod->getDocComment(); if ($docs !== false) { // extract everything prefixed by @ and first letter uppercase @@ -69,7 +76,7 @@ public function reflect($object, string $method) { } } - foreach ($reflection->getParameters() as $param) { + foreach ($this->reflectionMethod->getParameters() as $param) { // extract type information from PHP 7 scalar types and prefer them over phpdoc annotations $type = $param->getType(); if ($type instanceof \ReflectionNamedType) { @@ -114,6 +121,24 @@ public function getParameters(): array { return $this->parameters; } + /** + * @template T + * + * @param class-string $attributeClass + */ + public function hasAnnotationOrAttribute(?string $annotationName, string $attributeClass): bool { + if (!empty($this->reflectionMethod->getAttributes($attributeClass))) { + return true; + } + + if ($annotationName && $this->hasAnnotation($annotationName)) { + $this->logger->debug($this->reflectionMethod->getDeclaringClass()->getName() . '::' . $this->reflectionMethod->getName() . ' uses the @' . $annotationName . ' annotation and should use the #[' . $attributeClass . '] attribute instead'); + return true; + } + + return false; + } + /** * Check if a method contains an annotation * @param string $name the name of the annotation diff --git a/lib/public/AppFramework/Utility/IControllerMethodReflector.php b/lib/public/AppFramework/Utility/IControllerMethodReflector.php index 95d7fbebb561d..480b4a3576c6c 100644 --- a/lib/public/AppFramework/Utility/IControllerMethodReflector.php +++ b/lib/public/AppFramework/Utility/IControllerMethodReflector.php @@ -56,4 +56,15 @@ public function getParameters(): array; * @see https://help.nextcloud.com/t/how-should-we-use-php8-attributes/104278 */ public function hasAnnotation(string $name): bool; + + /** + * @template T + * + * Check if a method contains an annotation or an attribute. + * Log a debug line if the annotation is used. + * + * @param class-string $attributeClass + * @since 34.0.0 + */ + public function hasAnnotationOrAttribute(?string $annotationName, string $attributeClass): bool; } From bc0df9ee44e40252102e1685811fb35bf5489f98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Wed, 11 Mar 2026 11:25:31 +0100 Subject: [PATCH 2/6] fix: Fix AuthorizedAdminSetting attribute behavior MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../lib/Middleware/ProvisioningApiMiddleware.php | 3 ++- apps/settings/lib/Middleware/SubadminMiddleware.php | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php index 1989ef5d4c128..2e4e6a3bf2cf3 100644 --- a/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php +++ b/apps/provisioning_api/lib/Middleware/ProvisioningApiMiddleware.php @@ -11,6 +11,7 @@ use OCA\Provisioning_API\Middleware\Exceptions\NotSubAdminException; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; use OCP\AppFramework\Http\Response; use OCP\AppFramework\Middleware; use OCP\AppFramework\OCS\OCSException; @@ -40,7 +41,7 @@ public function __construct( */ public function beforeController($controller, $methodName) { // If AuthorizedAdminSetting, the check will be done in the SecurityMiddleware - if (!$this->isAdmin && !$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->isSubAdmin && !$this->reflector->hasAnnotation('AuthorizedAdminSetting')) { + if (!$this->isAdmin && !$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->isSubAdmin && !$this->reflector->hasAnnotationOrAttribute('AuthorizedAdminSetting', AuthorizedAdminSetting::class)) { throw new NotSubAdminException(); } } diff --git a/apps/settings/lib/Middleware/SubadminMiddleware.php b/apps/settings/lib/Middleware/SubadminMiddleware.php index 02d68e138dae4..eb27e27c74cca 100644 --- a/apps/settings/lib/Middleware/SubadminMiddleware.php +++ b/apps/settings/lib/Middleware/SubadminMiddleware.php @@ -14,6 +14,7 @@ use OC\AppFramework\Middleware\Security\Exceptions\NotAdminException; use OC\AppFramework\Utility\ControllerMethodReflector; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; use OCP\AppFramework\Http\TemplateResponse; use OCP\AppFramework\Middleware; use OCP\Group\ISubAdmin; @@ -48,7 +49,7 @@ private function isSubAdmin(): bool { * @throws \Exception */ public function beforeController($controller, $methodName) { - if (!$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->reflector->hasAnnotation('AuthorizedAdminSetting')) { + if (!$this->reflector->hasAnnotation('NoSubAdminRequired') && !$this->reflector->hasAnnotationOrAttribute('AuthorizedAdminSetting', AuthorizedAdminSetting::class)) { if (!$this->isSubAdmin()) { throw new NotAdminException($this->l10n->t('Logged in account must be a sub admin')); } From d20ea7dab243fa1baec06c5e72904057bc712637 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Wed, 11 Mar 2026 11:36:01 +0100 Subject: [PATCH 3/6] fix: Remove code duplication by using the new method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- .../FlowV2EphemeralSessionsMiddleware.php | 8 +----- .../PasswordConfirmationMiddleware.php | 19 +++---------- .../Middleware/SessionMiddleware.php | 27 ++----------------- 3 files changed, 7 insertions(+), 47 deletions(-) diff --git a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php index 115a823598367..658a120320e2f 100644 --- a/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php +++ b/lib/private/AppFramework/Middleware/FlowV2EphemeralSessionsMiddleware.php @@ -18,7 +18,6 @@ use OCP\ISession; use OCP\IUserSession; use Psr\Log\LoggerInterface; -use ReflectionMethod; // Will close the session if the user session is ephemeral. // Happens when the user logs in via the login flow v2. @@ -61,12 +60,7 @@ public function beforeController(Controller $controller, string $methodName) { return; } - $reflectionMethod = new ReflectionMethod($controller, $methodName); - if (!empty($reflectionMethod->getAttributes(PublicPage::class))) { - return; - } - - if ($this->reflector->hasAnnotation('PublicPage')) { + if ($this->reflector->hasAnnotationOrAttribute('PublicPage', PublicPage::class)) { return; } diff --git a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php index ce997d1701600..60b925876b1e2 100644 --- a/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php +++ b/lib/private/AppFramework/Middleware/Security/PasswordConfirmationMiddleware.php @@ -46,9 +46,7 @@ public function __construct( * @throws NotConfirmedException */ public function beforeController(Controller $controller, string $methodName) { - $reflectionMethod = new ReflectionMethod($controller, $methodName); - - if (!$this->needsPasswordConfirmation($reflectionMethod)) { + if (!$this->needsPasswordConfirmation()) { return; } @@ -79,6 +77,7 @@ public function beforeController(Controller $controller, string $methodName) { return; } + $reflectionMethod = new ReflectionMethod($controller, $methodName); if ($this->isPasswordConfirmationStrict($reflectionMethod)) { $authHeader = $this->request->getHeader('Authorization'); if (!str_starts_with(strtolower($authHeader), 'basic ')) { @@ -101,18 +100,8 @@ public function beforeController(Controller $controller, string $methodName) { } } - private function needsPasswordConfirmation(ReflectionMethod $reflectionMethod): bool { - $attributes = $reflectionMethod->getAttributes(PasswordConfirmationRequired::class); - if (!empty($attributes)) { - return true; - } - - if ($this->reflector->hasAnnotation('PasswordConfirmationRequired')) { - $this->logger->debug($reflectionMethod->getDeclaringClass()->getName() . '::' . $reflectionMethod->getName() . ' uses the @' . 'PasswordConfirmationRequired' . ' annotation and should use the #[PasswordConfirmationRequired] attribute instead'); - return true; - } - - return false; + private function needsPasswordConfirmation(): bool { + return $this->reflector->hasAnnotationOrAttribute('PasswordConfirmationRequired', PasswordConfirmationRequired::class); } private function isPasswordConfirmationStrict(ReflectionMethod $reflectionMethod): bool { diff --git a/lib/private/AppFramework/Middleware/SessionMiddleware.php b/lib/private/AppFramework/Middleware/SessionMiddleware.php index b7b0fb118c2d3..84f6c4ddb45d4 100644 --- a/lib/private/AppFramework/Middleware/SessionMiddleware.php +++ b/lib/private/AppFramework/Middleware/SessionMiddleware.php @@ -14,7 +14,6 @@ use OCP\AppFramework\Http\Response; use OCP\AppFramework\Middleware; use OCP\ISession; -use ReflectionMethod; class SessionMiddleware extends Middleware { /** @var ControllerMethodReflector */ @@ -34,18 +33,7 @@ public function __construct(ControllerMethodReflector $reflector, * @param string $methodName */ public function beforeController($controller, $methodName) { - /** - * Annotation deprecated with Nextcloud 26 - */ - $hasAnnotation = $this->reflector->hasAnnotation('UseSession'); - if ($hasAnnotation) { - $this->session->reopen(); - return; - } - - $reflectionMethod = new ReflectionMethod($controller, $methodName); - $hasAttribute = !empty($reflectionMethod->getAttributes(UseSession::class)); - if ($hasAttribute) { + if ($this->reflector->hasAnnotationOrAttribute('UseSession', UseSession::class)) { $this->session->reopen(); } } @@ -57,18 +45,7 @@ public function beforeController($controller, $methodName) { * @return Response */ public function afterController($controller, $methodName, Response $response) { - /** - * Annotation deprecated with Nextcloud 26 - */ - $hasAnnotation = $this->reflector->hasAnnotation('UseSession'); - if ($hasAnnotation) { - $this->session->close(); - return $response; - } - - $reflectionMethod = new ReflectionMethod($controller, $methodName); - $hasAttribute = !empty($reflectionMethod->getAttributes(UseSession::class)); - if ($hasAttribute) { + if ($this->reflector->hasAnnotationOrAttribute('UseSession', UseSession::class)) { $this->session->close(); } From c75c69890651d7a02c6a26c16040caa9747f1872 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Wed, 11 Mar 2026 12:15:53 +0100 Subject: [PATCH 4/6] fix(tests): Adapt Middleware tests to API change MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removed a few tests rendered obsolete by the refactoring. Signed-off-by: Côme Chilliet --- .../ProvisioningApiMiddlewareTest.php | 6 +- .../Middleware/SubadminMiddlewareTest.php | 19 +++++-- .../Utility/ControllerMethodReflector.php | 4 ++ .../lib/AppFramework/Http/DispatcherTest.php | 2 +- .../Security/BruteForceMiddlewareTest.php | 2 +- .../Security/CORSMiddlewareTest.php | 6 +- .../PasswordConfirmationMiddlewareTest.php | 2 +- .../Security/RateLimitingMiddlewareTest.php | 2 +- .../Security/SecurityMiddlewareTest.php | 3 +- .../Middleware/SessionMiddlewareTest.php | 56 +++++-------------- .../Utility/ControllerMethodReflectorTest.php | 31 +++++----- 11 files changed, 64 insertions(+), 69 deletions(-) diff --git a/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php b/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php index 0e84adca09f5b..0b55424a2558d 100644 --- a/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php +++ b/apps/provisioning_api/tests/Middleware/ProvisioningApiMiddlewareTest.php @@ -53,10 +53,14 @@ public function testBeforeController(bool $subadminRequired, bool $isAdmin, bool ); $this->reflector->method('hasAnnotation') - ->willReturnCallback(function ($annotation) use ($subadminRequired, $hasSettingAuthorizationAnnotation) { + ->willReturnCallback(function ($annotation) use ($subadminRequired) { if ($annotation === 'NoSubAdminRequired') { return !$subadminRequired; } + return false; + }); + $this->reflector->method('hasAnnotationOrAttribute') + ->willReturnCallback(function ($annotation, $attribute) use ($hasSettingAuthorizationAnnotation) { if ($annotation === 'AuthorizedAdminSetting') { return $hasSettingAuthorizationAnnotation; } diff --git a/apps/settings/tests/Middleware/SubadminMiddlewareTest.php b/apps/settings/tests/Middleware/SubadminMiddlewareTest.php index 37cfb5ccc5969..489359c34693e 100644 --- a/apps/settings/tests/Middleware/SubadminMiddlewareTest.php +++ b/apps/settings/tests/Middleware/SubadminMiddlewareTest.php @@ -14,6 +14,7 @@ use OC\AppFramework\Utility\ControllerMethodReflector; use OCA\Settings\Middleware\SubadminMiddleware; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\AuthorizedAdminSetting; use OCP\AppFramework\Http\TemplateResponse; use OCP\Group\ISubAdmin; use OCP\IL10N; @@ -62,11 +63,16 @@ public function testBeforeControllerAsUserWithoutAnnotation(): void { $this->expectException(NotAdminException::class); $this->reflector - ->expects($this->exactly(2)) + ->expects($this->exactly(1)) ->method('hasAnnotation') ->willReturnMap([ ['NoSubAdminRequired', false], - ['AuthorizedAdminSetting', false], + ]); + $this->reflector + ->expects($this->exactly(1)) + ->method('hasAnnotationOrAttribute') + ->willReturnMap([ + ['AuthorizedAdminSetting', AuthorizedAdminSetting::class, false], ]); $this->subAdminManager @@ -94,11 +100,16 @@ public function testBeforeControllerWithAnnotation(): void { public function testBeforeControllerAsSubAdminWithoutAnnotation(): void { $this->reflector - ->expects($this->exactly(2)) + ->expects($this->exactly(1)) ->method('hasAnnotation') ->willReturnMap([ ['NoSubAdminRequired', false], - ['AuthorizedAdminSetting', false], + ]); + $this->reflector + ->expects($this->exactly(1)) + ->method('hasAnnotationOrAttribute') + ->willReturnMap([ + ['AuthorizedAdminSetting', AuthorizedAdminSetting::class, false], ]); $this->subAdminManager diff --git a/lib/private/AppFramework/Utility/ControllerMethodReflector.php b/lib/private/AppFramework/Utility/ControllerMethodReflector.php index 1360ca2737ce8..1d30ff2e1376c 100644 --- a/lib/private/AppFramework/Utility/ControllerMethodReflector.php +++ b/lib/private/AppFramework/Utility/ControllerMethodReflector.php @@ -33,6 +33,10 @@ public function __construct( * @param string $method the method which we want to inspect */ public function reflect($object, string $method) { + $this->annotations = []; + $this->types = []; + $this->parameters = []; + $this->ranges = []; $this->reflectionMethod = new \ReflectionMethod($object, $method); $this->startLine = $this->reflectionMethod->getStartLine(); $this->file = $this->reflectionMethod->getFileName(); diff --git a/tests/lib/AppFramework/Http/DispatcherTest.php b/tests/lib/AppFramework/Http/DispatcherTest.php index 1743474358b0c..03a1242dd1fef 100644 --- a/tests/lib/AppFramework/Http/DispatcherTest.php +++ b/tests/lib/AppFramework/Http/DispatcherTest.php @@ -123,7 +123,7 @@ protected function setUp(): void { $this->request = $this->createMock(Request::class); - $this->reflector = new ControllerMethodReflector(); + $this->reflector = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->dispatcher = new Dispatcher( $this->http, diff --git a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php index 0bf99390c22e8..e386cb82d2cfa 100644 --- a/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/BruteForceMiddlewareTest.php @@ -30,7 +30,7 @@ class BruteForceMiddlewareTest extends TestCase { protected function setUp(): void { parent::setUp(); - $this->reflector = new ControllerMethodReflector(); + $this->reflector = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->throttler = $this->createMock(IThrottler::class); $this->request = $this->createMock(IRequest::class); $this->logger = $this->createMock(LoggerInterface::class); diff --git a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php index c325ae638fb96..8ffe343effc48 100644 --- a/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/CORSMiddlewareTest.php @@ -36,7 +36,7 @@ class CORSMiddlewareTest extends \Test\TestCase { protected function setUp(): void { parent::setUp(); - $this->reflector = new ControllerMethodReflector(); + $this->reflector = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->session = $this->createMock(Session::class); $this->throttler = $this->createMock(IThrottler::class); $this->logger = $this->createMock(LoggerInterface::class); @@ -82,6 +82,7 @@ public function testNoAnnotationNoCORSHEADER(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); + $this->reflector->reflect($this->controller, __FUNCTION__); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); $response = $middleware->afterController($this->controller, __FUNCTION__, new Response()); @@ -303,6 +304,7 @@ public function testAfterExceptionWithSecurityExceptionNoStatus(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); + $this->reflector->reflect($this->controller, __FUNCTION__); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); $response = $middleware->afterException($this->controller, __FUNCTION__, new SecurityException('A security exception')); @@ -319,6 +321,7 @@ public function testAfterExceptionWithSecurityExceptionWithStatus(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); + $this->reflector->reflect($this->controller, __FUNCTION__); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); $response = $middleware->afterException($this->controller, __FUNCTION__, new SecurityException('A security exception', 501)); @@ -338,6 +341,7 @@ public function testAfterExceptionWithRegularException(): void { $this->createMock(IRequestId::class), $this->createMock(IConfig::class) ); + $this->reflector->reflect($this->controller, __FUNCTION__); $middleware = new CORSMiddleware($request, $this->reflector, $this->session, $this->throttler, $this->logger); $middleware->afterException($this->controller, __FUNCTION__, new \Exception('A regular exception')); } diff --git a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php index 90e801ca47174..e0d1c254c9dbf 100644 --- a/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/PasswordConfirmationMiddlewareTest.php @@ -45,7 +45,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase { private Manager $userManager; protected function setUp(): void { - $this->reflector = new ControllerMethodReflector(); + $this->reflector = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->session = $this->createMock(ISession::class); $this->userSession = $this->createMock(IUserSession::class); $this->user = $this->createMock(IUser::class); diff --git a/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php index cf2a569f911e3..beb9424424f7b 100644 --- a/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/RateLimitingMiddlewareTest.php @@ -45,7 +45,7 @@ protected function setUp(): void { $this->request = $this->createMock(IRequest::class); $this->userSession = $this->createMock(IUserSession::class); - $this->reflector = new ControllerMethodReflector(); + $this->reflector = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->limiter = $this->createMock(Limiter::class); $this->session = $this->createMock(ISession::class); $this->appConfig = $this->createMock(IAppConfig::class); diff --git a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php index 0c6fc21357d9a..b7d5f17cf057f 100644 --- a/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/Security/SecurityMiddlewareTest.php @@ -83,7 +83,7 @@ protected function setUp(): void { 'test', $this->request ); - $this->reader = new ControllerMethodReflector(); + $this->reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $this->logger = $this->createMock(LoggerInterface::class); $this->navigationManager = $this->createMock(INavigationManager::class); $this->urlGenerator = $this->createMock(IURLGenerator::class); @@ -444,6 +444,7 @@ public function testCsrfOcsController(string $controllerClass, bool $hasOcsApiHe ->willReturn(true); $controller = new $controllerClass('test', $this->request); + $this->reader->reflect($controller, 'foo'); try { $this->middleware->beforeController($controller, 'foo'); diff --git a/tests/lib/AppFramework/Middleware/SessionMiddlewareTest.php b/tests/lib/AppFramework/Middleware/SessionMiddlewareTest.php index 234696ddcf1f1..bc612c30316d8 100644 --- a/tests/lib/AppFramework/Middleware/SessionMiddlewareTest.php +++ b/tests/lib/AppFramework/Middleware/SessionMiddlewareTest.php @@ -11,6 +11,7 @@ use OC\AppFramework\Middleware\SessionMiddleware; use OC\AppFramework\Utility\ControllerMethodReflector; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\UseSession; use OCP\AppFramework\Http\Response; use OCP\IRequest; use OCP\ISession; @@ -19,8 +20,8 @@ use Test\TestCase; class SessionMiddlewareTest extends TestCase { - private ControllerMethodReflector|MockObject $reflector; - private ISession|MockObject $session; + private ControllerMethodReflector&MockObject $reflector; + private ISession&MockObject $session; private Controller $controller; private SessionMiddleware $middleware; @@ -39,70 +40,39 @@ protected function setUp(): void { public function testSessionNotClosedOnBeforeController(): void { $this->configureSessionMock(0, 1); $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') + ->method('hasAnnotationOrAttribute') + ->with('UseSession', UseSession::class) ->willReturn(true); $this->middleware->beforeController($this->controller, 'withAnnotation'); } - public function testSessionNotClosedOnBeforeControllerWithAttribute(): void { - $this->configureSessionMock(0, 1); - $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') - ->willReturn(false); - - $this->middleware->beforeController($this->controller, 'withAttribute'); - } - public function testSessionClosedOnAfterController(): void { $this->configureSessionMock(1); $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') + ->method('hasAnnotationOrAttribute') + ->with('UseSession', UseSession::class) ->willReturn(true); $this->middleware->afterController($this->controller, 'withAnnotation', new Response()); } - public function testSessionClosedOnAfterControllerWithAttribute(): void { - $this->configureSessionMock(1); - $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') - ->willReturn(true); - - $this->middleware->afterController($this->controller, 'withAttribute', new Response()); - } - public function testSessionReopenedAndClosedOnBeforeController(): void { $this->configureSessionMock(1, 1); $this->reflector->expects(self::exactly(2)) - ->method('hasAnnotation') - ->with('UseSession') + ->method('hasAnnotationOrAttribute') + ->with('UseSession', UseSession::class) ->willReturn(true); $this->middleware->beforeController($this->controller, 'withAnnotation'); $this->middleware->afterController($this->controller, 'withAnnotation', new Response()); } - public function testSessionReopenedAndClosedOnBeforeControllerWithAttribute(): void { - $this->configureSessionMock(1, 1); - $this->reflector->expects(self::exactly(2)) - ->method('hasAnnotation') - ->with('UseSession') - ->willReturn(false); - - $this->middleware->beforeController($this->controller, 'withAttribute'); - $this->middleware->afterController($this->controller, 'withAttribute', new Response()); - } - public function testSessionClosedOnBeforeController(): void { $this->configureSessionMock(0); $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') + ->method('hasAnnotationOrAttribute') + ->with('UseSession', UseSession::class) ->willReturn(false); $this->middleware->beforeController($this->controller, 'without'); @@ -111,8 +81,8 @@ public function testSessionClosedOnBeforeController(): void { public function testSessionNotClosedOnAfterController(): void { $this->configureSessionMock(0); $this->reflector->expects(self::once()) - ->method('hasAnnotation') - ->with('UseSession') + ->method('hasAnnotationOrAttribute') + ->with('UseSession', UseSession::class) ->willReturn(false); $this->middleware->afterController($this->controller, 'without', new Response()); diff --git a/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php b/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php index 0d95dcac45fc6..56e3070f5dc68 100644 --- a/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php +++ b/tests/lib/AppFramework/Utility/ControllerMethodReflectorTest.php @@ -9,6 +9,7 @@ namespace Test\AppFramework\Utility; use OC\AppFramework\Utility\ControllerMethodReflector; +use Psr\Log\LoggerInterface; class BaseController { /** @@ -69,7 +70,7 @@ class ControllerMethodReflectorTest extends \Test\TestCase { * @Annotation */ public function testReadAnnotation(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'testReadAnnotation' @@ -82,7 +83,7 @@ public function testReadAnnotation(): void { * @Annotation(parameter=value) */ public function testGetAnnotationParameterSingle(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( self::class, __FUNCTION__ @@ -95,7 +96,7 @@ public function testGetAnnotationParameterSingle(): void { * @Annotation(parameter1=value1, parameter2=value2,parameter3=value3) */ public function testGetAnnotationParameterMultiple(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( self::class, __FUNCTION__ @@ -111,7 +112,7 @@ public function testGetAnnotationParameterMultiple(): void { * @param test */ public function testReadAnnotationNoLowercase(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'testReadAnnotationNoLowercase' @@ -127,7 +128,7 @@ public function testReadAnnotationNoLowercase(): void { * @param int $test */ public function testReadTypeIntAnnotations(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'testReadTypeIntAnnotations' @@ -145,7 +146,7 @@ public function arguments3($a, float $b, int $c, $d) { } public function testReadTypeIntAnnotationsScalarTypes(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'arguments3' @@ -163,7 +164,7 @@ public function testReadTypeIntAnnotationsScalarTypes(): void { * @param double $test something special */ public function testReadTypeDoubleAnnotations(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'testReadTypeDoubleAnnotations' @@ -177,7 +178,7 @@ public function testReadTypeDoubleAnnotations(): void { * @param string $foo */ public function testReadTypeWhitespaceAnnotations(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'testReadTypeWhitespaceAnnotations' @@ -190,7 +191,7 @@ public function testReadTypeWhitespaceAnnotations(): void { public function arguments($arg, $arg2 = 'hi') { } public function testReflectParameters(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'arguments' @@ -203,7 +204,7 @@ public function testReflectParameters(): void { public function arguments2($arg) { } public function testReflectParameters2(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect( '\Test\AppFramework\Utility\ControllerMethodReflectorTest', 'arguments2' @@ -214,7 +215,7 @@ public function testReflectParameters2(): void { public function testInheritance(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect('Test\AppFramework\Utility\EndController', 'test'); $this->assertTrue($reader->hasAnnotation('Annotation')); @@ -222,7 +223,7 @@ public function testInheritance(): void { public function testInheritanceOverride(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect('Test\AppFramework\Utility\EndController', 'test2'); $this->assertTrue($reader->hasAnnotation('NoAnnotation')); @@ -231,14 +232,14 @@ public function testInheritanceOverride(): void { public function testInheritanceOverrideNoDocblock(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect('Test\AppFramework\Utility\EndController', 'test3'); $this->assertFalse($reader->hasAnnotation('Annotation')); } public function testRangeDetectionPsalm(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect('Test\AppFramework\Utility\EndController', 'test4'); $rangeInfo1 = $reader->getRange('rangedOne'); @@ -259,7 +260,7 @@ public function testRangeDetectionPsalm(): void { } public function testRangeDetectionNative(): void { - $reader = new ControllerMethodReflector(); + $reader = new ControllerMethodReflector(\OCP\Server::get(LoggerInterface::class)); $reader->reflect('Test\AppFramework\Utility\EndController', 'test5'); $rangeInfo1 = $reader->getRange('rangedOne'); From 1226c6c0ee81384ac5b44b01adb7a9051b32cf91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Wed, 11 Mar 2026 14:43:50 +0100 Subject: [PATCH 5/6] =?UTF-8?q?chore:=20Un-deprecate=20IControllerMethodRe?= =?UTF-8?q?flector=20as=20it=E2=80=99s=20now=20useful=20for=20attributes?= =?UTF-8?q?=20as=20well?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It should be extended later to add methods to get attributes from reflection, and maybe a getter to the reflectionMethod object to avoid middlewares building their own. Signed-off-by: Côme Chilliet --- .../Utility/IControllerMethodReflector.php | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/lib/public/AppFramework/Utility/IControllerMethodReflector.php b/lib/public/AppFramework/Utility/IControllerMethodReflector.php index 480b4a3576c6c..d4e36e310e084 100644 --- a/lib/public/AppFramework/Utility/IControllerMethodReflector.php +++ b/lib/public/AppFramework/Utility/IControllerMethodReflector.php @@ -11,22 +11,13 @@ /** * Interface ControllerMethodReflector * - * Reads and parses annotations from doc comments + * You can inject this interface in your Middleware, and it will be prefilled with information related to the called controller method + * + * Reads and parses annotations from doc comments (deprecated) and PHP attributes * * @since 8.0.0 - * @deprecated 22.0.0 will be obsolete with native attributes in PHP8 - * @see https://help.nextcloud.com/t/how-should-we-use-php8-attributes/104278 */ interface IControllerMethodReflector { - /** - * @param object $object an object or classname - * @param string $method the method which we want to inspect - * @return void - * @since 8.0.0 - * @deprecated 17.0.0 Reflect should not be called multiple times and only be used internally. This will be removed in Nextcloud 18 - */ - public function reflect($object, string $method); - /** * Inspects the PHPDoc parameters for types * From 3bf8592e8353ce24072e494d1d71e0ed4f71ebad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=B4me=20Chilliet?= Date: Wed, 11 Mar 2026 16:23:21 +0100 Subject: [PATCH 6/6] chore: Update psalm baseline MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Côme Chilliet --- build/psalm-baseline.xml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml index e74ca6ef4c8b5..cb32dd4a925f3 100644 --- a/build/psalm-baseline.xml +++ b/build/psalm-baseline.xml @@ -1596,9 +1596,6 @@ - - - @@ -2224,9 +2221,6 @@ - - - @@ -3202,9 +3196,6 @@ - - -