diff --git a/core/Middleware/TwoFactorMiddleware.php b/core/Middleware/TwoFactorMiddleware.php index 0dea402f127ab..e98078cbcdb31 100644 --- a/core/Middleware/TwoFactorMiddleware.php +++ b/core/Middleware/TwoFactorMiddleware.php @@ -18,6 +18,7 @@ use OC\User\Session; use OCA\TwoFactorNextcloudNotification\Controller\APIController; use OCP\AppFramework\Controller; +use OCP\AppFramework\Http\Attribute\NoTwoFactorRequired; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Middleware; use OCP\AppFramework\Utility\IControllerMethodReflector; @@ -26,6 +27,7 @@ use OCP\ISession; use OCP\IURLGenerator; use OCP\IUser; +use ReflectionMethod; class TwoFactorMiddleware extends Middleware { public function __construct( @@ -43,9 +45,11 @@ public function __construct( * @param string $methodName */ public function beforeController($controller, $methodName) { - if ($this->reflector->hasAnnotation('NoTwoFactorRequired')) { - // Route handler explicitly marked to work without finished 2FA are - // not blocked + // Route handler explicitly marked to work without finished 2FA are not blocked + $reflectionMethod = new ReflectionMethod($controller, $methodName); + $hasNoTwoFactorAttribute = !empty($reflectionMethod->getAttributes(NoTwoFactorRequired::class)); + $hasNoTwoFactorAnnotation = $this->reflector->hasAnnotation('NoTwoFactorRequired'); + if ($hasNoTwoFactorAnnotation || $hasNoTwoFactorAttribute) { return; } diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 7cca0c7a41455..881b8f9274b16 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -92,6 +92,7 @@ 'OCP\\AppFramework\\Http\\Attribute\\IgnoreOpenAPI' => $baseDir . '/lib/public/AppFramework/Http/Attribute/IgnoreOpenAPI.php', 'OCP\\AppFramework\\Http\\Attribute\\NoAdminRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoAdminRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\NoCSRFRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoCSRFRequired.php', + 'OCP\\AppFramework\\Http\\Attribute\\NoTwoFactorRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\OpenAPI' => $baseDir . '/lib/public/AppFramework/Http/Attribute/OpenAPI.php', 'OCP\\AppFramework\\Http\\Attribute\\PasswordConfirmationRequired' => $baseDir . '/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\PublicPage' => $baseDir . '/lib/public/AppFramework/Http/Attribute/PublicPage.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index c9fcc66a253f1..fe71f1c623016 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -133,6 +133,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OCP\\AppFramework\\Http\\Attribute\\IgnoreOpenAPI' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/IgnoreOpenAPI.php', 'OCP\\AppFramework\\Http\\Attribute\\NoAdminRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoAdminRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\NoCSRFRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoCSRFRequired.php', + 'OCP\\AppFramework\\Http\\Attribute\\NoTwoFactorRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\OpenAPI' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/OpenAPI.php', 'OCP\\AppFramework\\Http\\Attribute\\PasswordConfirmationRequired' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/PasswordConfirmationRequired.php', 'OCP\\AppFramework\\Http\\Attribute\\PublicPage' => __DIR__ . '/../../..' . '/lib/public/AppFramework/Http/Attribute/PublicPage.php', diff --git a/lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php b/lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php new file mode 100644 index 0000000000000..64c3219f6a797 --- /dev/null +++ b/lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php @@ -0,0 +1,23 @@ +middleware->beforeController($twoFactorChallengeController, 'index'); } + public function testBeforeControllerWithNoTwoFactorRequiredAnnotation(): void { + // Simulate a logged-in user where 2FA might otherwise trigger + $this->userSession->expects($this->any()) + ->method('isLoggedIn') + ->willReturn(true); + + $user = $this->createMock(\OCP\IUser::class); + $this->userSession->expects($this->any()) + ->method('getUser') + ->willReturn($user); + + $this->twoFactorManager->expects($this->any()) + ->method('isTwoFactorAuthenticated') + ->with($user) + ->willReturn(false); + + // Reflector returns true for annotation, so 2FA check should be skipped: + $this->reflector->expects($this->once()) + ->method('hasAnnotation') + ->with('NoTwoFactorRequired') + ->willReturn(true); + + // No attributes used here - just a plain controller + $controller = new class('app', $this->request) extends \OCP\AppFramework\Controller { + public function testMethod() {} + }; + + // Should NOT throw, since the annotation triggers bypass + $this->middleware->beforeController($controller, 'testMethod'); + + // If we get here, the test passes + $this->assertTrue(true); + } + + public function testBeforeControllerWithNoTwoFactorRequiredAttribute(): void { + // Simulate not logged in, or a case where normally 2FA might be triggered + $this->userSession->expects($this->any()) + ->method('isLoggedIn') + ->willReturn(true); + + $user = $this->createMock(\OCP\IUser::class); + $this->userSession->expects($this->any()) + ->method('getUser') + ->willReturn($user); + + // By default, pretend 2FA is required for this user + $this->twoFactorManager->expects($this->any()) + ->method('isTwoFactorAuthenticated') + ->with($user) + ->willReturn(false); + + // The reflector must indicate that there is NO annotation present, + // so the attribute is the only thing that could bypass 2FA + $this->reflector->expects($this->once()) + ->method('hasAnnotation') + ->with('NoTwoFactorRequired') + ->willReturn(false); + + // Dynamically define a controller class with the attribute (PHP 8+ required) + $controller = new class('app', $this->request) extends \OCP\AppFramework\Controller { + #[\OCP\AppFramework\Http\Attribute\NoTwoFactorRequired] + public function testMethod() {} + }; + + // Should NOT throw, since the attribute is present and triggers bypass + $this->middleware->beforeController($controller, 'testMethod'); + + // If no exception is thrown, the test passes + $this->assertTrue(true); + } + public static function dataRequires2FASetupDone(): array { return [ [false, false, false],