Skip to content
10 changes: 7 additions & 3 deletions core/Middleware/TwoFactorMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -26,6 +27,7 @@
use OCP\ISession;
use OCP\IURLGenerator;
use OCP\IUser;
use ReflectionMethod;

class TwoFactorMiddleware extends Middleware {
public function __construct(
Expand All @@ -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 = $hasNoTwoFactorAttribute || $this->reflector->hasAnnotation('NoTwoFactorRequired');
if ($hasNoTwoFactorAnnotation || $hasNoTwoFactorAttribute) {
return;
}

Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
1 change: 1 addition & 0 deletions lib/composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
23 changes: 23 additions & 0 deletions lib/public/AppFramework/Http/Attribute/NoTwoFactorRequired.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php

declare(strict_types=1);

/**
* SPDX-FileCopyrightText: 2023 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCP\AppFramework\Http\Attribute;

use Attribute;

/**
* Attribute to mark controller methods as not requiring two-factor authentication.
*
* Use this wisely and only in 2FA auth apps, e.g. to allow setup during login.
*
* @since 33.0.0
*/
#[Attribute(Attribute::TARGET_METHOD)]
class NoTwoFactorRequired {
}
71 changes: 71 additions & 0 deletions tests/Core/Middleware/TwoFactorMiddlewareTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,77 @@ public function testRequires2FASetupDoneAnnotated(): void {
$this->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],
Expand Down
Loading