OXDEV-9078 Refactor validator and resend otp

TitaKoleva · TitaKoleva · commit af164e28ddb4 · 2026-04-09T12:43:32.000+03:00
diff --git a/assets/out/src/js/module/resend-otp.js b/assets/out/src/js/module/resend-otp.js
@@ -10,6 +10,7 @@ export class ResendOtp {
             submitButtonId = 'auth_submit',
             attemptsDisplayId = 'remaining-attempts',
             codeInputId = 'auth_code',
+            // todo-high: use attempts from resend request response
             maxAttempts = 5,
         } = options;
 
@@ -19,6 +20,7 @@ export class ResendOtp {
         this.attemptsDisplay = document.getElementById(attemptsDisplayId);
         this.codeInput = document.getElementById(codeInputId);
 
+        // todo-low: add admin setting for cooldown seconds
         this.cooldownSeconds = Number(button.dataset.cooldown || 60);
         this.maxAttempts = maxAttempts;
         this.url = button.dataset.url;
diff --git a/src/Authentication/TwoFactorAuth/Controller/TwoFactorAuthController.php b/src/Authentication/TwoFactorAuth/Controller/TwoFactorAuthController.php
@@ -13,6 +13,7 @@
 use OxidEsales\Eshop\Core\UtilsView;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\InvalidCodeException;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\ResendCooldownException;
+use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAResendableInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAServiceInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAUserServiceInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Transput\AuthCodeRequestInterface;
@@ -55,6 +56,11 @@ public function handleOTP(): ?string
 
     public function resendCode(): void
     {
+        if (!$this->twoFAService instanceof TwoFAResendableInterface) {
+            $this->jsonResponse->send(['success' => false], 405);
+            return;
+        }
+
         $userId = $this->twoFAUserService->getPendingUserId();
         try {
             $this->twoFAService->resend($userId);
@@ -63,4 +69,10 @@ public function resendCode(): void
             $this->jsonResponse->send(['success' => false], 429);
         }
     }
+
+    public function render()
+    {
+        // todo-high: send attempts left for the user
+        return parent::render();
+    }
 }
diff --git a/src/Authentication/TwoFactorAuth/OTP/OtpFacade.php b/src/Authentication/TwoFactorAuth/OTP/OtpFacade.php
@@ -9,16 +9,17 @@
 
 namespace OxidEsales\SecurityModule\Authentication\TwoFactorAuth\OTP;
 
+use DateTimeImmutable;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\ResendCooldownException;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\OTP\Notifier\Factory\OtpNotifierFactoryInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\OTP\Service\OtpChallengeStateServiceInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\OTP\Service\OtpCodeGeneratorServiceInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\OTP\Service\OtpCodeValidatorServiceInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\OTP\Service\OtpSendPolicyServiceInterface;
-use DateTimeImmutable;
+use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAResendableInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAServiceInterface;
 
-class OtpFacade implements TwoFAServiceInterface
+class OtpFacade implements TwoFAServiceInterface, TwoFAResendableInterface
 {
     public function __construct(
         private OtpChallengeStateServiceInterface $stateService,
@@ -71,4 +72,12 @@ public function resend(string $userId): void
         $this->stateService->refreshChallengeState($userId, $code);
         $this->notifierFactory->create($userId)->notify($userId, $code);
     }
+
+    public function getRemainingAttempts(string $userId): int
+    {
+        $state = $this->stateService->getChallengeState($userId);
+        $attempts = $state?->getAttempts() ?? 0;
+
+        return max(0, $this->codeValidator->getMaxAttempts() - $attempts);
+    }
 }
diff --git a/src/Authentication/TwoFactorAuth/OTP/Service/OtpCodeValidatorService.php b/src/Authentication/TwoFactorAuth/OTP/Service/OtpCodeValidatorService.php
@@ -9,21 +9,52 @@
 
 namespace OxidEsales\SecurityModule\Authentication\TwoFactorAuth\OTP\Service;
 
+use DateTimeImmutable;
+use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\AttemptLimitExceededException;
+use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\InvalidCodeException;
+use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\TimeExpiredException;
 // phpcs:ignore Generic.Files.LineLength
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\OTP\Infrastructure\Repository\OtpChallengeStateRepositoryInterface;
 
 class OtpCodeValidatorService implements OtpCodeValidatorServiceInterface
 {
+    private const MAX_ATTEMPTS = 5;
+
     public function __construct(
-        /** @phpstan-ignore property.onlyWritten */
         private OtpChallengeStateRepositoryInterface $repository,
+        private OtpCodeHasherServiceInterface $codeHasher,
     ) {
     }
 
     public function validateCode(
         string $userId,
         #[\SensitiveParameter] string $inputCode
     ): void {
-        // todo-critical: implement, also remove the phpstan-ignore then
+        $state = $this->repository->findByUserId($userId);
+
+        if ($state === null) {
+            throw new InvalidCodeException();
+        }
+
+        if ($state->getAttempts() >= self::MAX_ATTEMPTS) {
+            throw new AttemptLimitExceededException();
+        }
+
+        if (new DateTimeImmutable() > $state->getExpiresAt()) {
+            throw new TimeExpiredException();
+        }
+
+        if ($this->codeHasher->hash($inputCode) !== $state->getCodeHash()) {
+            $this->repository->incrementAttempts($userId);
+            if (($state->getAttempts() + 1) >= self::MAX_ATTEMPTS) {
+                throw new AttemptLimitExceededException();
+            }
+            throw new InvalidCodeException();
+        }
+    }
+
+    public function getMaxAttempts(): int
+    {
+        return self::MAX_ATTEMPTS;
     }
 }
diff --git a/src/Authentication/TwoFactorAuth/OTP/Service/OtpCodeValidatorServiceInterface.php b/src/Authentication/TwoFactorAuth/OTP/Service/OtpCodeValidatorServiceInterface.php
@@ -20,4 +20,6 @@ public function validateCode(
         string $userId,
         #[\SensitiveParameter] string $inputCode
     ): void;
+
+    public function getMaxAttempts(): int;
 }
diff --git a/src/Authentication/TwoFactorAuth/Service/TwoFAResendableInterface.php b/src/Authentication/TwoFactorAuth/Service/TwoFAResendableInterface.php
@@ -0,0 +1,20 @@
+<?php
+
+/**
+ * Copyright © OXID eSales AG. All rights reserved.
+ * See LICENSE file for license details.
+ */
+
+declare(strict_types=1);
+
+namespace OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service;
+
+use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\ResendCooldownException;
+
+interface TwoFAResendableInterface
+{
+    /** @throws ResendCooldownException */
+    public function resend(string $userId): void;
+
+    public function getRemainingAttempts(string $userId): int;
+}
diff --git a/src/Authentication/TwoFactorAuth/Service/TwoFAServiceInterface.php b/src/Authentication/TwoFactorAuth/Service/TwoFAServiceInterface.php
@@ -8,7 +8,6 @@
 namespace OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service;
 
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\CodeValidationException;
-use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\ResendCooldownException;
 
 interface TwoFAServiceInterface
 {
@@ -22,9 +21,4 @@ public function invalidateChallenge(string $userId): void;
      * @throws CodeValidationException
      */
     public function verify(string $userId, #[\SensitiveParameter] string $code): void;
-
-    // todo-low: consider extracting resend to a separate ResendableInterface,
-    // not all 2FA methods support it (e.g. TOTP)
-    /** @throws ResendCooldownException */
-    public function resend(string $userId): void;
 }
diff --git a/tests/Unit/Authentication/TwoFactorAuth/Controller/TwoFactorAuthControllerTest.php b/tests/Unit/Authentication/TwoFactorAuth/Controller/TwoFactorAuthControllerTest.php
@@ -13,6 +13,7 @@
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Controller\TwoFactorAuthController;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\InvalidCodeException;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\ResendCooldownException;
+use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAResendableInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAServiceInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAUserServiceInterface;
 use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Transput\AuthCodeRequestInterface;
@@ -79,15 +80,18 @@ public function resendCodeSendsSuccessResponse(): void
         $twoFAUserServiceStub->method('getPendingUserId')
             ->willReturn($userId = uniqid());
 
-        $twoFAServiceSpy = $this->createMock(TwoFAServiceInterface::class);
+        $twoFAServiceSpy = $this->createMockForIntersectionOfInterfaces([
+            TwoFAServiceInterface::class,
+            TwoFAResendableInterface::class,
+        ]);
         $twoFAServiceSpy->expects($this->once())
             ->method('resend')
             ->with($userId);
 
         $jsonResponseSpy = $this->createMock(JsonResponseInterface::class);
         $jsonResponseSpy->expects($this->once())
             ->method('send')
-            ->with(['success' => true], 200);
+            ->with(['success' => true]);
 
         $this->getSut(
             twoFAService: $twoFAServiceSpy,
@@ -103,7 +107,10 @@ public function resendCodeSends429OnCooldown(): void
         $twoFAUserServiceStub->method('getPendingUserId')
             ->willReturn($userId = uniqid());
 
-        $twoFAServiceStub = $this->createMock(TwoFAServiceInterface::class);
+        $twoFAServiceStub = $this->createMockForIntersectionOfInterfaces([
+            TwoFAServiceInterface::class,
+            TwoFAResendableInterface::class,
+        ]);
         $twoFAServiceStub->expects($this->once())
             ->method('resend')
             ->with($userId)
@@ -121,6 +128,20 @@ public function resendCodeSends429OnCooldown(): void
         )->resendCode();
     }
 
+    #[Test]
+    public function resendCodeSends405WhenServiceIsNotResendable(): void
+    {
+        $jsonResponseSpy = $this->createMock(JsonResponseInterface::class);
+        $jsonResponseSpy->expects($this->once())
+            ->method('send')
+            ->with(['success' => false], 405);
+
+        $this->getSut(
+            twoFAService: $this->createStub(TwoFAServiceInterface::class),
+            jsonResponse: $jsonResponseSpy,
+        )->resendCode();
+    }
+
     private function getSut(
         TwoFAServiceInterface $twoFAService = null,
         TwoFAUserServiceInterface $twoFAUserService = null,
diff --git a/tests/Unit/Authentication/TwoFactorAuth/OTP/OtpFacadeTest.php b/tests/Unit/Authentication/TwoFactorAuth/OTP/OtpFacadeTest.php
@@ -229,6 +229,54 @@ public function resendRefreshesStateAndNotifiesWhenAllowed(): void
         $sut->resend(userId: $userId);
     }
 
+    #[Test]
+    public function getRemainingAttemptsReturnsMaxWhenNoChallengeState(): void
+    {
+        $stateServiceStub = $this->createStub(OtpChallengeStateServiceInterface::class);
+        $stateServiceStub->method('getChallengeState')->willReturn(null);
+
+        $codeValidatorStub = $this->createStub(OtpCodeValidatorServiceInterface::class);
+        $codeValidatorStub->method('getMaxAttempts')->willReturn(5);
+
+        $sut = $this->getSut(stateService: $stateServiceStub, codeValidator: $codeValidatorStub);
+
+        $this->assertSame(5, $sut->getRemainingAttempts(uniqid()));
+    }
+
+    #[Test]
+    public function getRemainingAttemptsSubtractsCurrentAttempts(): void
+    {
+        $stateStub = $this->createStub(OtpChallengeStateInterface::class);
+        $stateStub->method('getAttempts')->willReturn(3);
+
+        $stateServiceStub = $this->createStub(OtpChallengeStateServiceInterface::class);
+        $stateServiceStub->method('getChallengeState')->willReturn($stateStub);
+
+        $codeValidatorStub = $this->createStub(OtpCodeValidatorServiceInterface::class);
+        $codeValidatorStub->method('getMaxAttempts')->willReturn(5);
+
+        $sut = $this->getSut(stateService: $stateServiceStub, codeValidator: $codeValidatorStub);
+
+        $this->assertSame(2, $sut->getRemainingAttempts(uniqid()));
+    }
+
+    #[Test]
+    public function getRemainingAttemptsReturnsZeroWhenAttemptsExceedMax(): void
+    {
+        $stateStub = $this->createStub(OtpChallengeStateInterface::class);
+        $stateStub->method('getAttempts')->willReturn(7);
+
+        $stateServiceStub = $this->createStub(OtpChallengeStateServiceInterface::class);
+        $stateServiceStub->method('getChallengeState')->willReturn($stateStub);
+
+        $codeValidatorStub = $this->createStub(OtpCodeValidatorServiceInterface::class);
+        $codeValidatorStub->method('getMaxAttempts')->willReturn(5);
+
+        $sut = $this->getSut(stateService: $stateServiceStub, codeValidator: $codeValidatorStub);
+
+        $this->assertSame(0, $sut->getRemainingAttempts(uniqid()));
+    }
+
     private function getSut(
         OtpChallengeStateServiceInterface $stateService = null,
         OtpCodeValidatorServiceInterface $codeValidator = null,
diff --git a/tests/Unit/Authentication/TwoFactorAuth/OTP/Service/OtpCodeValidatorServiceTest.php b/tests/Unit/Authentication/TwoFactorAuth/OTP/Service/OtpCodeValidatorServiceTest.php

Original file line number	Diff line number	Diff line change
`@@ -20,4 +20,6 @@ public function validateCode(`
`20`	`20`	`string $userId,`
`21`	`21`	`#[\SensitiveParameter] string $inputCode`
`22`	`22`	`): void;`
	`23`	`+`
	`24`	`+ public function getMaxAttempts(): int;`
`23`	`25`	`}`