From b3e30166efa8ca6ec7bc57f43ee56ec50145bc86 Mon Sep 17 00:00:00 2001 From: Nicholas Gray Date: Wed, 13 Mar 2024 18:05:11 +0100 Subject: [PATCH 1/3] Feature (): Add options to enforce 2FA on specific users and/or particular Authentication Providers --- .../Middleware/SecondFactorMiddleware.php | 30 +++++++++++++++---- Configuration/Settings.yaml | 4 ++- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/Classes/Http/Middleware/SecondFactorMiddleware.php b/Classes/Http/Middleware/SecondFactorMiddleware.php index f83be12..5586989 100644 --- a/Classes/Http/Middleware/SecondFactorMiddleware.php +++ b/Classes/Http/Middleware/SecondFactorMiddleware.php @@ -55,10 +55,23 @@ class SecondFactorMiddleware implements MiddlewareInterface protected $secondFactorSessionStorageService; /** - * @Flow\InjectConfiguration(path="enforceTwoFactorAuthentication") + * @Flow\InjectConfiguration(path="enforceTwoFactorAuthenticationForAllUserAccounts") * @var bool */ - protected $enforceTwoFactorAuthentication; + protected $enforceTwoFactorAuthenticationForAllUserAccounts; + + /** + * @Flow\InjectConfiguration(path="enforceTwoFactorAuthenticationForAuthenticationProviders") + * @var array + */ + protected $enforceTwoFactorAuthenticationForAuthenticcationProviders; + + /** + * @Flow\InjectConfiguration(path="enforceTwoFactorAuthenticationForSpecificUserAccounts") + * @var array + */ + protected $enforceTwoFactorAuthenticationForSpecificUserAccounts; + /** * This middleware checks if the user is authenticated with a second factor "if necessary". @@ -124,6 +137,7 @@ class SecondFactorMiddleware implements MiddlewareInterface */ public function process(ServerRequestInterface $request, RequestHandlerInterface $handler): ResponseInterface { + $authenticationTokens = $this->securityContext->getAuthenticationTokens(); // 1. Skip, if no authentication tokens are present, because we're not on a secured route. @@ -151,12 +165,15 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface return $handler->handle($request); } + $account = $this->securityContext->getAccount(); // 4. Skip, if second factor is not set up for account and not enforced via settings. if ( !$this->secondFactorRepository->isEnabledForAccount($account) - && !$this->enforceTwoFactorAuthentication + && !$this->enforceTwoFactorAuthenticationForAllUserAccounts + && !in_array($account->getAccountIdentifier(),$this->enforceTwoFactorAuthenticationForSpecificUserAccounts) + && !in_array($account->getAuthenticationProviderName(),$this->enforceTwoFactorAuthenticationForAuthenticcationProviders) ) { $this->log('Second factor not enabled for account and not enforced by system, skipping second factor.'); @@ -198,8 +215,11 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface // 7. Redirect to 2FA setup, if second factor is not set up for account but is enforced by system. // Skip, if already on 2FA setup route. if ( - $this->enforceTwoFactorAuthentication && - !$this->secondFactorRepository->isEnabledForAccount($account) + ($this->enforceTwoFactorAuthenticationForAllUserAccounts + || in_array($account->getAccountIdentifier(),$this->enforceTwoFactorAuthenticationForSpecificUserAccounts) + || in_array($account->getAuthenticationProviderName(),$this->enforceTwoFactorAuthenticationForAuthenticcationProviders) + ) + && !$this->secondFactorRepository->isEnabledForAccount($account) ) { // WHY: We use the request URI as state here to keep the middleware from entering a redirect loop. $isSettingUp2FA = str_ends_with($request->getUri()->getPath(), self::SECOND_FACTOR_SETUP_URI); diff --git a/Configuration/Settings.yaml b/Configuration/Settings.yaml index 5e6c71d..11e2204 100644 --- a/Configuration/Settings.yaml +++ b/Configuration/Settings.yaml @@ -45,6 +45,8 @@ Neos: Sandstorm: NeosTwoFactorAuthentication: # enforce 2FA for all users - enforceTwoFactorAuthentication: false + enforceTwoFactorAuthenticationForAllUserAccounts: false + enforceTwoFactorAuthenticationForAuthenticationProviders : [] + enforceTwoFactorAuthenticationForSpecificUserAccounts: [] # (optional) if set this will be used as a naming convention for the TOTP. If empty the Site name will be used issuerName: '' From 47ef940262c9181bf632e3f947a238e8d0852524 Mon Sep 17 00:00:00 2001 From: Nicholas Gray Date: Mon, 25 Mar 2024 12:52:54 +0100 Subject: [PATCH 2/3] Addressed comments of pull request --- .../Middleware/SecondFactorMiddleware.php | 25 +++++++++---------- Configuration/Settings.yaml | 8 +++--- README.md | 10 ++++++++ 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/Classes/Http/Middleware/SecondFactorMiddleware.php b/Classes/Http/Middleware/SecondFactorMiddleware.php index 5586989..8463df9 100644 --- a/Classes/Http/Middleware/SecondFactorMiddleware.php +++ b/Classes/Http/Middleware/SecondFactorMiddleware.php @@ -55,22 +55,22 @@ class SecondFactorMiddleware implements MiddlewareInterface protected $secondFactorSessionStorageService; /** - * @Flow\InjectConfiguration(path="enforceTwoFactorAuthenticationForAllUserAccounts") + * @Flow\InjectConfiguration(path="enforceTwoFactorAuthentication") * @var bool */ - protected $enforceTwoFactorAuthenticationForAllUserAccounts; + protected $enforceTwoFactorAuthentication; /** - * @Flow\InjectConfiguration(path="enforceTwoFactorAuthenticationForAuthenticationProviders") + * @Flow\InjectConfiguration(path="enforce2FAForAuthenticationProviders") * @var array */ - protected $enforceTwoFactorAuthenticationForAuthenticcationProviders; + protected $enforce2FAForAuthenticationProviders; /** - * @Flow\InjectConfiguration(path="enforceTwoFactorAuthenticationForSpecificUserAccounts") + * @Flow\InjectConfiguration(path="enforce2FAForRoles") * @var array */ - protected $enforceTwoFactorAuthenticationForSpecificUserAccounts; + protected $enforce2FAForRoles; /** @@ -165,15 +165,14 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface return $handler->handle($request); } - $account = $this->securityContext->getAccount(); // 4. Skip, if second factor is not set up for account and not enforced via settings. if ( !$this->secondFactorRepository->isEnabledForAccount($account) - && !$this->enforceTwoFactorAuthenticationForAllUserAccounts - && !in_array($account->getAccountIdentifier(),$this->enforceTwoFactorAuthenticationForSpecificUserAccounts) - && !in_array($account->getAuthenticationProviderName(),$this->enforceTwoFactorAuthenticationForAuthenticcationProviders) + && !$this->enforceTwoFactorAuthentication + && !count(array_intersect(array_map(fn($item) => $item->getIdentifier(),$account->getRoles()),$this->enforce2FAForRoles)) + && !in_array($account->getAuthenticationProviderName(),$this->enforce2FAForAuthenticationProviders) ) { $this->log('Second factor not enabled for account and not enforced by system, skipping second factor.'); @@ -215,9 +214,9 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface // 7. Redirect to 2FA setup, if second factor is not set up for account but is enforced by system. // Skip, if already on 2FA setup route. if ( - ($this->enforceTwoFactorAuthenticationForAllUserAccounts - || in_array($account->getAccountIdentifier(),$this->enforceTwoFactorAuthenticationForSpecificUserAccounts) - || in_array($account->getAuthenticationProviderName(),$this->enforceTwoFactorAuthenticationForAuthenticcationProviders) + ($this->enforceTwoFactorAuthentication + || count(array_intersect(array_map(fn($item) => $item->getIdentifier(),$account->getRoles()),$this->enforce2FAForRoles)) + || in_array($account->getAuthenticationProviderName(),$this->enforce2FAForAuthenticationProviders) ) && !$this->secondFactorRepository->isEnabledForAccount($account) ) { diff --git a/Configuration/Settings.yaml b/Configuration/Settings.yaml index 11e2204..8dc9f40 100644 --- a/Configuration/Settings.yaml +++ b/Configuration/Settings.yaml @@ -45,8 +45,10 @@ Neos: Sandstorm: NeosTwoFactorAuthentication: # enforce 2FA for all users - enforceTwoFactorAuthenticationForAllUserAccounts: false - enforceTwoFactorAuthenticationForAuthenticationProviders : [] - enforceTwoFactorAuthenticationForSpecificUserAccounts: [] + enforceTwoFactorAuthentication: false + # enforce 2FA for specific authentication providers + enforce2FAForAuthenticationProviders : [] + # enforce 2FA for specific roles + enforce2FAForRoles: [] # (optional) if set this will be used as a naming convention for the TOTP. If empty the Site name will be used issuerName: '' diff --git a/README.md b/README.md index 18d0584..f086fc1 100644 --- a/README.md +++ b/README.md @@ -24,6 +24,16 @@ Sandstorm: ``` With this setting, no user can login into the CMS without setting up a second factor first. +In addition, you can enforce 2FA for specific authentication providers and/or roles by adding following to your `Settings.yaml` +```yml +Sandstorm: + NeosTwoFactorAuthentication: + # enforce 2FA for specific authentication providers + enforce2FAForAuthenticationProviders : ['Neos.Neos:Backend'] + # enforce 2FA for specific roles + enforce2FAForRoles: ['Neos.Neos:Administrator'] +``` + ### Issuer Naming To override the default sitename as issuer label, you can define one via the configuration settings: ```yml From 2ff42cbf4305c9c6bbd2d2db60b0930a54cd39d1 Mon Sep 17 00:00:00 2001 From: Robert Baruck Date: Wed, 27 Mar 2024 13:00:55 +0100 Subject: [PATCH 3/3] TASK: Fix regression regarding deletion of second factor and new enforcment conditions --- Classes/Controller/BackendController.php | 20 +++-- .../Repository/SecondFactorRepository.php | 6 -- .../Middleware/SecondFactorMiddleware.php | 47 +++-------- Classes/Service/SecondFactorService.php | 79 +++++++++++++++++++ 4 files changed, 98 insertions(+), 54 deletions(-) create mode 100644 Classes/Service/SecondFactorService.php diff --git a/Classes/Controller/BackendController.php b/Classes/Controller/BackendController.php index 9686f09..86e223f 100644 --- a/Classes/Controller/BackendController.php +++ b/Classes/Controller/BackendController.php @@ -18,6 +18,7 @@ use Sandstorm\NeosTwoFactorAuthentication\Domain\Model\SecondFactor; use Sandstorm\NeosTwoFactorAuthentication\Domain\Model\Dto\SecondFactorDto; use Sandstorm\NeosTwoFactorAuthentication\Domain\Repository\SecondFactorRepository; +use Sandstorm\NeosTwoFactorAuthentication\Service\SecondFactorService; use Sandstorm\NeosTwoFactorAuthentication\Service\SecondFactorSessionStorageService; use Sandstorm\NeosTwoFactorAuthentication\Service\TOTPService; @@ -71,10 +72,10 @@ class BackendController extends AbstractModuleController protected $defaultViewObjectName = FusionView::class; /** - * @Flow\InjectConfiguration(path="enforceTwoFactorAuthentication") - * @var bool + * @Flow\Inject + * @var SecondFactorService */ - protected $enforceTwoFactorAuthentication; + protected $secondFactorService; /** * used to list all second factors of the current user @@ -177,14 +178,11 @@ public function deleteAction(SecondFactor $secondFactor): void { $account = $this->securityContext->getAccount(); - if ( - $this->securityContext->hasRole('Neos.Neos:Administrator') - || $secondFactor->getAccount() === $account - ) { - if ( - $this->enforceTwoFactorAuthentication - && count($this->secondFactorRepository->findByAccount($account)) <= 1 - ) { + $isAdministrator = $this->securityContext->hasRole('Neos.Neos:Administrator'); + $isOwner = $secondFactor->getAccount() === $account; + + if ($isAdministrator || $isOwner) { + if (!$this->secondFactorService->canOneSecondFactorBeDeletedForAccount($account)) { $this->addFlashMessage( $this->translator->translateById( 'module.index.delete.flashMessage.cannotRemoveLastSecondFactor', diff --git a/Classes/Domain/Repository/SecondFactorRepository.php b/Classes/Domain/Repository/SecondFactorRepository.php index 7a9334a..b166d21 100644 --- a/Classes/Domain/Repository/SecondFactorRepository.php +++ b/Classes/Domain/Repository/SecondFactorRepository.php @@ -16,12 +16,6 @@ */ class SecondFactorRepository extends Repository { - public function isEnabledForAccount(Account $account): bool - { - $factors = $this->findByAccount($account); - return count($factors) > 0; - } - /** * @throws IllegalObjectTypeException */ diff --git a/Classes/Http/Middleware/SecondFactorMiddleware.php b/Classes/Http/Middleware/SecondFactorMiddleware.php index 8463df9..ad1c922 100644 --- a/Classes/Http/Middleware/SecondFactorMiddleware.php +++ b/Classes/Http/Middleware/SecondFactorMiddleware.php @@ -15,7 +15,7 @@ use Psr\Http\Server\RequestHandlerInterface; use Psr\Log\LoggerInterface; use Sandstorm\NeosTwoFactorAuthentication\Domain\AuthenticationStatus; -use Sandstorm\NeosTwoFactorAuthentication\Domain\Repository\SecondFactorRepository; +use Sandstorm\NeosTwoFactorAuthentication\Service\SecondFactorService; use Sandstorm\NeosTwoFactorAuthentication\Service\SecondFactorSessionStorageService; class SecondFactorMiddleware implements MiddlewareInterface @@ -30,12 +30,6 @@ class SecondFactorMiddleware implements MiddlewareInterface */ protected $securityContext; - /** - * @Flow\Inject - * @var SecondFactorRepository - */ - protected $secondFactorRepository; - /** * @Flow\Inject * @var ActionRequestFactory @@ -55,23 +49,10 @@ class SecondFactorMiddleware implements MiddlewareInterface protected $secondFactorSessionStorageService; /** - * @Flow\InjectConfiguration(path="enforceTwoFactorAuthentication") - * @var bool - */ - protected $enforceTwoFactorAuthentication; - - /** - * @Flow\InjectConfiguration(path="enforce2FAForAuthenticationProviders") - * @var array - */ - protected $enforce2FAForAuthenticationProviders; - - /** - * @Flow\InjectConfiguration(path="enforce2FAForRoles") - * @var array + * @Flow\Inject + * @var SecondFactorService */ - protected $enforce2FAForRoles; - + protected SecondFactorService $secondFactorService; /** * This middleware checks if the user is authenticated with a second factor "if necessary". @@ -167,13 +148,11 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface $account = $this->securityContext->getAccount(); + $isEnabledForAccount = $this->secondFactorService->isSecondFactorEnabledForAccount($account); + $isEnforcedForAccount = $this->secondFactorService->isSecondFactorEnforcedForAccount($account); + // 4. Skip, if second factor is not set up for account and not enforced via settings. - if ( - !$this->secondFactorRepository->isEnabledForAccount($account) - && !$this->enforceTwoFactorAuthentication - && !count(array_intersect(array_map(fn($item) => $item->getIdentifier(),$account->getRoles()),$this->enforce2FAForRoles)) - && !in_array($account->getAuthenticationProviderName(),$this->enforce2FAForAuthenticationProviders) - ) { + if (!$isEnabledForAccount && !$isEnforcedForAccount) { $this->log('Second factor not enabled for account and not enforced by system, skipping second factor.'); return $handler->handle($request); @@ -193,7 +172,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface // 6. Redirect to 2FA login, if second factor is set up for account but not authenticated. // Skip, if already on 2FA login route. if ( - $this->secondFactorRepository->isEnabledForAccount($account) + $isEnabledForAccount && $authenticationStatus === AuthenticationStatus::AUTHENTICATION_NEEDED ) { // WHY: We use the request URI as state here to keep the middleware from entering a redirect loop. @@ -213,13 +192,7 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface // 7. Redirect to 2FA setup, if second factor is not set up for account but is enforced by system. // Skip, if already on 2FA setup route. - if ( - ($this->enforceTwoFactorAuthentication - || count(array_intersect(array_map(fn($item) => $item->getIdentifier(),$account->getRoles()),$this->enforce2FAForRoles)) - || in_array($account->getAuthenticationProviderName(),$this->enforce2FAForAuthenticationProviders) - ) - && !$this->secondFactorRepository->isEnabledForAccount($account) - ) { + if ($isEnforcedForAccount && !$isEnabledForAccount) { // WHY: We use the request URI as state here to keep the middleware from entering a redirect loop. $isSettingUp2FA = str_ends_with($request->getUri()->getPath(), self::SECOND_FACTOR_SETUP_URI); if ($isSettingUp2FA) { diff --git a/Classes/Service/SecondFactorService.php b/Classes/Service/SecondFactorService.php new file mode 100644 index 0000000..57fd988 --- /dev/null +++ b/Classes/Service/SecondFactorService.php @@ -0,0 +1,79 @@ +enforceTwoFactorAuthentication; + $isEnforcedForRoles = count(array_intersect( + array_map(fn($item) => $item->getIdentifier(), $account->getRoles()), + $this->enforce2FAForRoles + )); + $isEnforcedForAuthenticationProviders = in_array( + $account->getAuthenticationProviderName(), + $this->enforce2FAForAuthenticationProviders + ); + + return $isEnforcedForAll || $isEnforcedForRoles || $isEnforcedForAuthenticationProviders; + } + + /** + * Check if the account has setup at least 1 second factor. + */ + public function isSecondFactorEnabledForAccount(Account $account): bool + { + $factors = $this->secondFactorRepository->findByAccount($account); + return count($factors) > 0; + } + + /** + * Check if the account can delete 1 second factor. + * + * Second factor can only be deleted if it is not enforced for the account or if the account has multiple factors. + */ + public function canOneSecondFactorBeDeletedForAccount(Account $account): bool + { + $isEnforcedForAccount = $this->isSecondFactorEnforcedForAccount($account); + $hasMultipleFactors = count($this->secondFactorRepository->findByAccount($account)) > 1; + + return !$isEnforcedForAccount || $hasMultipleFactors; + } +}