Merge branch 'b-7.4.x-generate-exceptions-OXDEV-9992' into b-7.4.x-2fa-OXDEV-9078

Author: RahatHameed

metadata.php

@@ -178,7 +178,7 @@
             'name'  => TwoFactorAuthModuleSettings::TWO_FACTOR_TYPE,
             'type'  => 'select',
             'constraints' => 'otp|totp',
-            'value' => ''
+            'value' => 'otp'
         ],
     ],
 ];

src/Authentication/TwoFactorAuth/Controller/TwoFactorAuthController.php

@@ -42,13 +42,4 @@ public function handleOTP(): void
             Registry::getUtilsView()->addErrorToDisplay($e->getMessage());
         }
     }
-
-    public function generate(): void
-    {
-        //todo: stop execution if not ajax
-        //todo: prevent spam by rate limiting
-        //todo: should return json response with success or error message
-        $authorizeService = $this->getService(AuthorizeServiceInterface::class);
-        $authorizeService->generate();
-    }
 }

src/Authentication/TwoFactorAuth/Exception/UserNotFoundException.php

@@ -0,0 +1,14 @@
+<?php
+
+/**
+ * Copyright © OXID eSales AG. All rights reserved.
+ * See LICENSE file for license details.
+ */
+
+declare(strict_types=1);
+
+namespace OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception;
+
+class UserNotFoundException extends \Exception
+{
+}

src/Authentication/TwoFactorAuth/Infrastructure/Repository/UserRepository.php

@@ -13,8 +13,8 @@
 use Doctrine\DBAL\Result;
 use OxidEsales\EshopCommunity\Internal\Framework\Database\QueryBuilderFactoryInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\DTO\User as UserDTO;
+use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\UserNotFoundException;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Infrastructure\Factory\UserFactoryInterface;
-use RuntimeException;
 
 class UserRepository implements UserRepositoryInterface
 {
@@ -26,7 +26,6 @@ public function __construct(
 
     public function getUserOTPData(string $userName): UserDTO
     {
-        //todo: exception if not found
         $builder = $this->queryBuilderFactory->create();
         $builder->select([
                 'OXID',
@@ -42,15 +41,14 @@ public function getUserOTPData(string $userName): UserDTO
         $queryResult = $builder->execute();
         $userData = $queryResult->fetchAssociative();
         if (!$userData) {
-            //todo: throw correct exception
-            throw new RuntimeException('User not found');
+            throw new UserNotFoundException();
         }
 
         return new UserDTO(
             $userData['OXID'],
             $userData['OESMOTPATTEMPTS'],
             $userData['OESMOTPCODE'],
-            new DateTime($userData['OESMOTPEXPTIME'])
+            $userData['OESMOTPEXPTIME'] ? new DateTime($userData['OESMOTPEXPTIME']) : null
         );
     }
 
@@ -90,7 +88,7 @@ public function resetCodeFields(string $userId): void
         $userModel->save();
     }
 
-    public function getUserPasswordHash(string $userName): string
+    public function getUserPasswordHash(string $userName): ?string
     {
         $builder = $this->queryBuilderFactory->create();
         $builder->select('OXPASSWORD')
@@ -100,6 +98,8 @@ public function getUserPasswordHash(string $userName): string
 
         /** @var Result $queryResult */
         $queryResult = $builder->execute();
-        return $queryResult->fetchOne();
+        $userPass = $queryResult->fetchOne();
+
+        return $userPass ?: null;
     }
 }

src/Authentication/TwoFactorAuth/Infrastructure/Repository/UserRepositoryInterface.php

@@ -20,5 +20,5 @@ public function resetCodeFields(string $userId): void;
 
     public function addOTPtoUser(string $userId, string $otp, DateTime $expiresAt): bool;
 
-    public function getUserPasswordHash(string $userId): string;
+    public function getUserPasswordHash(string $userId): ?string;
 }

src/Authentication/TwoFactorAuth/Service/ModuleSettingsService.php

@@ -33,6 +33,15 @@ public function getTwoFactorAuthType(): string
         return $this->getStringValue(self::TWO_FACTOR_TYPE);
     }
 
+    public function saveIsTwoFactorAuthEnabled(bool $value): void
+    {
+        $this->moduleSettingService->saveBoolean(
+            self::ACTIVE,
+            $value,
+            Module::MODULE_ID
+        );
+    }
+
     private function getStringValue(string $key): string
     {
         return $this->moduleSettingService->getString(

src/Authentication/TwoFactorAuth/Service/ModuleSettingsServiceInterface.php

@@ -12,4 +12,6 @@ interface ModuleSettingsServiceInterface
     public function isTwoFactorAuthEnabled(): bool;
 
     public function getTwoFactorAuthType(): string;
+
+    public function saveIsTwoFactorAuthEnabled(bool $value): void;
 }

src/Authentication/TwoFactorAuth/Service/UserService.php

@@ -9,7 +9,8 @@
 
 namespace OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service;
 
-use OxidEsales\Eshop\Core\Registry;
+use OxidEsales\Eshop\Core\Request;
+use OxidEsales\Eshop\Core\Utils;
 use OxidEsales\EshopCommunity\Internal\Domain\Authentication\Bridge\PasswordServiceBridgeInterface;
 use OxidEsales\EshopCommunity\Internal\Framework\Session\SessionInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Infrastructure\Repository\UserRepositoryInterface;
@@ -20,7 +21,9 @@ public function __construct(
         private AuthorizeServiceInterface $authorizeService,
         private UserRepositoryInterface $userRepository,
         private PasswordServiceBridgeInterface $pwdServiceBridge,
-        private SessionInterface $session
+        private SessionInterface $session,
+        private Request $request,
+        private Utils $utils,
     ) {
     }
 
@@ -29,21 +32,27 @@ public function handleLogin(string $userName): void
         $this->session->set(AuthorizeService::USER_SESSION_KEY, $userName);
         $this->session->set(
             AuthorizeService::OTP_TARGET_URL,
-            //todo: bind registry
-            Registry::getRequest()->getRequestUrl()
+            $this->request->getRequestUrl()
         );
 
+        //todo: prevent spam by rate limiting
         $this->authorizeService->generate();
 
-        //todo: return full url
         $redirectUrl = $this->authorizeService->getVerificationUrl();
-        Registry::getUtils()->redirect(Registry::getConfig()->getShopHomeUrl() . 'cl=' . $redirectUrl);
+        $this->utils->redirect($redirectUrl);
     }
 
     public function checkPassword(string $userName, string $password): bool
     {
-        //todo: got exception if user not found
-        $userPasswordHash = $this->userRepository->getUserPasswordHash($userName);
+        try {
+            $userPasswordHash = $this->userRepository->getUserPasswordHash($userName);
+        } catch (\Throwable $e) {
+            return false;
+        }
+
+        if ($userPasswordHash === null) {
+            return false;
+        }
 
         return $this->pwdServiceBridge
             ->verifyPassword($password, $userPasswordHash);

src/Authentication/TwoFactorAuth/Service/Verificator/OTP/OTPVerificator.php

@@ -9,7 +9,7 @@
 
 namespace OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\Verificator\OTP;
 
-use OxidEsales\Eshop\Core\Registry;
+use OxidEsales\Eshop\Core\Config;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\InvalidCodeException;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Infrastructure\Repository\UserRepositoryInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\Verificator\OTP\Generator\OTPGeneratorInterface;
@@ -22,6 +22,7 @@ public function __construct(
         private OTPGeneratorInterface $otpGenerator,
         private OTPValidatorInterface $otpValidator,
         private UserRepositoryInterface $userRepository,
+        private Config $config,
     ) {
     }
 
@@ -50,10 +51,7 @@ public function validateCode(string $userName, string $inputCode): void
 
     public function generate(string $userName): string
     {
-        //todo: userId from parameter or DTO
         $otpData = $this->userRepository->getUserOTPData($userName);
-
-        //todo: stop if user not found?
         //todo: wait time between generations, in case of abuse like
         //spamming the generate button or
         //user hit limit and try to generate new code to bypass it
@@ -63,6 +61,6 @@ public function generate(string $userName): string
 
     public function getVerificationUrl(): string
     {
-        return 'twofactorauth';
+        return $this->config->getShopHomeUrl() . 'cl=twofactorauth';
     }
 }

src/Authentication/TwoFactorAuth/Service/Verificator/services.yaml

@@ -5,6 +5,8 @@ services:
   _defaults:
     autowire: true
     public: false
+    bind:
+      OxidEsales\Eshop\Core\Config: '@=service("OxidEsales\\SecurityModule\\Core\\Registry").getConfig()'
 
   OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\Verificator\OTP\OTPVerificator:
     tags: [ 'security.twofa.tag.verificator' ]