OXDEV-9927 Handle user session

Author: NikolaIvanovski

src/Authentication/OAuth2/Infrastructure/Factory/OAuth2UserDTOFactory.php

@@ -25,7 +25,6 @@ public function createFromFacebookUser(FacebookUser $facebookUser): OAuth2UserDT
         );
     }
 
-    //TODO: use one factory for all providers
     public function createFromGoogleUser(GoogleUser $googleUser): OAuth2UserDTOInterface
     {
         return new OAuth2UserDTO(

src/Authentication/TwoFactorAuth/Controller/TwoFactorAuthController.php

@@ -37,13 +37,10 @@ public function handleOTP(): void
 
     public function generate(): void
     {
-        //todo: use session to get email/username
-        $username = uniqid();
-
         //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($username);
+        $authorizeService->generate();
     }
 }

src/Authentication/TwoFactorAuth/DTO/User.php

@@ -38,7 +38,6 @@ public function getAttempts(): ?int
 
     public function getExpiresAt(): ?DateTime
     {
-        //todo: possible bug - should be null if not set
         return $this->expiresAt;
     }
 }

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

@@ -25,7 +25,6 @@ public function __construct(
     public function getUserOTPData(string $userName): UserDTO
     {
         //todo: exception if not found
-        //todo: separate method for id and username?
         $builder = $this->queryBuilderFactory->create();
         $builder->select([
                 'OXID',
@@ -35,7 +34,6 @@ public function getUserOTPData(string $userName): UserDTO
             ])
             ->from('oxuser')
             ->where('oxusername = :userName')
-            ->orWhere('oxid = :userName')
             ->setParameter('userName', $userName);
 
         $userData = $builder->execute()->fetchAssociative();

src/Authentication/TwoFactorAuth/Service/AuthorizeService.php

@@ -13,6 +13,8 @@
 
 class AuthorizeService implements AuthorizeServiceInterface
 {
+    public const USER_SESSION_KEY = 'pending_authorized_user';
+
     public function __construct(
         private ModuleSettingsServiceInterface $moduleSettings,
         private VerificationCollectorServiceInterface $verificationCollectorService,
@@ -29,14 +31,15 @@ public function validate(string $inputCode): void
             $activeVerificator
         );
 
-        //todo: use session to get email/username or userId
-        $userName = $this->session->get('pending_otp_user');
+        $userName = $this->session->get(self::USER_SESSION_KEY);
 
         $verificator->validateCode($userName, $inputCode);
     }
 
-    public function generate(string $userName): void
+    public function generate(): void
     {
+        $userName = $this->session->get(self::USER_SESSION_KEY);
+
         $activeVerificator = $this->moduleSettings->getTwoFactorAuthType();
 
         $verificator = $this->verificationCollectorService->getVerificator(
@@ -45,7 +48,6 @@ public function generate(string $userName): void
 
         $OTPCode = $verificator->generate($userName);
 
-        //todo: module setting?
         $notifier = $this->notifierCollectorService->getNotifier('email');
         $notifier->notify($userName, $OTPCode);
     }

src/Authentication/TwoFactorAuth/Service/AuthorizeServiceInterface.php

@@ -11,7 +11,7 @@ interface AuthorizeServiceInterface
 {
     public function validate(string $inputCode): void;
 
-    public function generate(string $userName): void;
+    public function generate(): void;
 
     public function getVerificationUrl(): string;
 }

src/Authentication/TwoFactorAuth/Service/UserService.php

@@ -11,14 +11,12 @@
 
 use OxidEsales\EshopCommunity\Internal\Domain\Authentication\Bridge\PasswordServiceBridgeInterface;
 use OxidEsales\EshopCommunity\Internal\Framework\Session\SessionInterface;
-use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Infrastructure\Factory\UserFactoryInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Infrastructure\Repository\UserRepositoryInterface;
 
 class UserService implements UserServiceInterface
 {
     public function __construct(
         private AuthorizeServiceInterface $authorizeService,
-        private UserFactoryInterface $userFactory,
         private UserRepositoryInterface $userRepository,
         private PasswordServiceBridgeInterface $passwordServiceBridge,
         private SessionInterface $session
@@ -27,20 +25,19 @@ public function __construct(
 
     public function handleLogin($userName): void
     {
-        $this->authorizeService->generate($userName);
+        $this->session->set(AuthorizeService::USER_SESSION_KEY, $userName);
 
-        //todo: this should be handled by authorize service?
-        $this->session->set('pending_otp_user', $userName);
+        $this->authorizeService->generate();
         // redirect to controller and rende template
         // use template renderer to render the template
     }
 
     public function checkPassword(string $userName, string $password): bool
     {
-//        var_dump($userId);
 //        $userModel = $this->userFactory->create();
 //        $userModel->load($userId);
 
+        //todo: got exception if user not found
         $userPasswordHash = $this->userRepository->getUserPasswordHash($userName);
 //        var_dump($userPasswordHash);
         return $this->passwordServiceBridge

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

@@ -29,10 +29,10 @@ public function getName(): string
         return 'otp';
     }
 
-    public function validateCode(string $userId, string $inputCode): void
+    public function validateCode(string $userName, string $inputCode): void
     {
         //todo: userId from parameter or DTO
-        $otpData = $this->userRepository->getUserOTPData($userId);
+        $otpData = $this->userRepository->getUserOTPData($userName);
 
         $this->otpValidator->checkLoginAttempts($otpData->getAttempts());
         $this->otpValidator->checkExpirationTime($otpData->getExpiresAt());

src/Shared/Model/User.php

@@ -87,7 +87,6 @@ public function login($userName, $password, $setSessionCookie = false): bool
             return false; // invalid login
         }
 
-        //todo: re-login will send new OTP, should we avoid that?
         $userService->handleLogin($userName);
 
         //todo: redirect to correct page decided by verificator method? (otp: otp page, TOTP: totp page, etc)

tests/Unit/Authentication/TwoFactorAuth/Service/Verificator/OTP/OTPVerificatorTest.php

@@ -31,7 +31,7 @@ public function testVerificatorName()
 
     public function testInvalidCode(): void
     {
-        $userId = uniqid();
+        $userName = uniqid();
         $attempts = rand();
         $code = uniqid();
 
@@ -55,12 +55,12 @@ public function testInvalidCode(): void
 
         $this->expectException(InvalidCodeException::class);
 
-        $otpService->validateCode($userId, $code);
+        $otpService->validateCode($userName, $code);
     }
 
     public function testExpiredCode(): void
     {
-        $userId = uniqid();
+        $userName = uniqid();
         $attempts = rand();
         $code = uniqid();
 
@@ -84,12 +84,12 @@ public function testExpiredCode(): void
 
         $this->expectException(TimeExpiredException::class);
 
-        $otpService->validateCode($userId, $code);
+        $otpService->validateCode($userName, $code);
     }
 
     public function testAttemptsCode(): void
     {
-        $userId = uniqid();
+        $userName = uniqid();
         $attempts = rand();
         $code = uniqid();
 
@@ -113,7 +113,7 @@ public function testAttemptsCode(): void
 
         $this->expectException(AttemptLimitExceededException::class);
 
-        $otpService->validateCode($userId, $code);
+        $otpService->validateCode($userName, $code);
     }
 
     public function getSut(