Skip to content

Commit 169941a

Browse files
committed
OXDEV-9078 Fix resend cooldown and attempts on OTP page
1 parent 00c44bb commit 169941a

12 files changed

Lines changed: 146 additions & 25 deletions

File tree

assets/out/src/js/module/resend-otp.js

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,6 @@ export class ResendOtp {
1010
submitButtonId = 'auth_submit',
1111
attemptsDisplayId = 'remaining-attempts',
1212
codeInputId = 'auth_code',
13-
// todo-high: use attempts from resend request response
14-
maxAttempts = 5,
1513
} = options;
1614

1715
this.btn = button;
@@ -20,9 +18,7 @@ export class ResendOtp {
2018
this.attemptsDisplay = document.getElementById(attemptsDisplayId);
2119
this.codeInput = document.getElementById(codeInputId);
2220

23-
// todo-low: add admin setting for cooldown seconds
2421
this.cooldownSeconds = Number(button.dataset.cooldown || 60);
25-
this.maxAttempts = maxAttempts;
2622
this.url = button.dataset.url;
2723
this.textDefault = button.dataset.textDefault;
2824
this.textSending = button.dataset.textSending;
@@ -55,16 +51,24 @@ export class ResendOtp {
5551
headers: { 'Content-Type': 'application/json' }
5652
});
5753

54+
if (response.status === 429) {
55+
const until = Date.now() + this.cooldownSeconds * 1000;
56+
this.storeUntil(until);
57+
this.startCooldown(until);
58+
return;
59+
}
60+
5861
if (!response.ok) {
5962
this.unlock();
63+
this.setHint(this.textError);
6064
return;
6165
}
6266

67+
const data = await response.json();
6368
const until = Date.now() + this.cooldownSeconds * 1000;
6469
this.storeUntil(until);
6570
this.startCooldown(until);
66-
67-
this.resetAttempts();
71+
this.resetAttempts(data.remainingAttempts);
6872

6973
} catch (e) {
7074
console.error(e);
@@ -73,9 +77,9 @@ export class ResendOtp {
7377
}
7478
}
7579

76-
resetAttempts() {
80+
resetAttempts(count) {
7781
if (this.attemptsDisplay) {
78-
this.attemptsDisplay.textContent = this.maxAttempts;
82+
this.attemptsDisplay.textContent = count;
7983
}
8084
this.enableSubmit();
8185
}
@@ -143,8 +147,16 @@ export class ResendOtp {
143147
}
144148

145149
restoreOnRefresh() {
146-
const until = this.getStoredUntil();
147-
if (until && until > Date.now()) {
150+
const stored = this.getStoredUntil();
151+
if (stored && stored > Date.now()) {
152+
this.startCooldown(stored);
153+
return;
154+
}
155+
156+
const serverRemaining = Number(this.btn.dataset.cooldownRemaining || 0);
157+
if (serverRemaining > 0) {
158+
const until = Date.now() + serverRemaining * 1000;
159+
this.storeUntil(until);
148160
this.startCooldown(until);
149161
}
150162
}

src/Authentication/TwoFactorAuth/Controller/TwoFactorAuthController.php

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
use OxidEsales\Eshop\Application\Controller\FrontendController;
1313
use OxidEsales\Eshop\Core\UtilsView;
14-
use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\InvalidCodeException;
14+
use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\CodeValidationException;
1515
use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Exception\ResendCooldownException;
1616
use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAResendableInterface;
1717
use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAServiceInterface;
@@ -47,7 +47,7 @@ public function handleOTP(): ?string
4747
try {
4848
$this->twoFAService->verify($userId, $code);
4949
$this->twoFAUserService->loginUser($userId);
50-
} catch (InvalidCodeException $e) {
50+
} catch (CodeValidationException $e) {
5151
$this->utilsView->addErrorToDisplay($e);
5252
}
5353

@@ -64,15 +64,12 @@ public function resendCode(): void
6464
$userId = $this->twoFAUserService->getPendingUserId();
6565
try {
6666
$this->twoFAService->resend($userId);
67-
$this->jsonResponse->send(['success' => true]);
67+
$this->jsonResponse->send([
68+
'success' => true,
69+
'remainingAttempts' => $this->twoFAService->getRemainingAttempts($userId),
70+
]);
6871
} catch (ResendCooldownException) {
6972
$this->jsonResponse->send(['success' => false], 429);
7073
}
7174
}
72-
73-
public function render()
74-
{
75-
// todo-high: send attempts left for the user
76-
return parent::render();
77-
}
7875
}

src/Authentication/TwoFactorAuth/Factory/services.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,4 @@ services:
1111

1212
OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAServiceInterface:
1313
factory: ['@OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Factory\TwoFAServiceFactoryInterface', 'create']
14+
public: true

src/Authentication/TwoFactorAuth/OTP/OtpFacade.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,9 @@ public function getRemainingAttempts(string $userId): int
8080

8181
return max(0, $this->codeValidator->getMaxAttempts() - $attempts);
8282
}
83+
84+
public function getCooldownRemaining(string $userId): int
85+
{
86+
return $this->sendPolicy->getCooldownRemaining($userId);
87+
}
8388
}

src/Authentication/TwoFactorAuth/OTP/Service/OtpSendPolicyService.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,25 @@ public function __construct(
2323
}
2424

2525
public function canSend(string $userId): bool
26+
{
27+
return $this->getCooldownRemaining($userId) === 0;
28+
}
29+
30+
public function getCooldownRemaining(string $userId): int
2631
{
2732
$state = $this->stateRepository->findByUserId($userId);
2833

2934
if ($state === null) {
30-
return true;
35+
return 0;
3136
}
3237

3338
$lastSentAt = $state->getLastSentAt();
3439
if ($lastSentAt === null) {
35-
return true;
40+
return 0;
3641
}
3742

3843
$elapsed = (new DateTimeImmutable())->getTimestamp() - $lastSentAt->getTimestamp();
3944

40-
return $elapsed >= self::RESEND_COOLDOWN_SECONDS;
45+
return max(0, self::RESEND_COOLDOWN_SECONDS - $elapsed);
4146
}
4247
}

src/Authentication/TwoFactorAuth/OTP/Service/OtpSendPolicyServiceInterface.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,4 +10,6 @@
1010
interface OtpSendPolicyServiceInterface
1111
{
1212
public function canSend(string $userId): bool;
13+
14+
public function getCooldownRemaining(string $userId): int;
1315
}

src/Authentication/TwoFactorAuth/Service/TwoFAResendableInterface.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,6 @@ interface TwoFAResendableInterface
1717
public function resend(string $userId): void;
1818

1919
public function getRemainingAttempts(string $userId): int;
20+
21+
public function getCooldownRemaining(string $userId): int;
2022
}

src/Shared/Core/ViewConfig.php

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010
namespace OxidEsales\SecurityModule\Shared\Core;
1111

1212
use OxidEsales\SecurityModule\Authentication\OAuth2\Service\ProviderCollectorInterface;
13+
use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAResendableInterface;
14+
use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAServiceInterface;
15+
use OxidEsales\SecurityModule\Authentication\TwoFactorAuth\Service\TwoFAUserServiceInterface;
1316
use OxidEsales\SecurityModule\Captcha\Captcha\Image\Service\ImageCaptchaService;
1417
use OxidEsales\SecurityModule\Captcha\Service\CaptchaServiceInterface;
1518
use OxidEsales\SecurityModule\PasswordPolicy\Service\ModuleSettingsServiceInterface as PasswordSettingsServiceInterface;
@@ -61,8 +64,24 @@ public function getActiveProviders(): iterable
6164

6265
public function getRemainingAttempts(): int
6366
{
64-
// todo-critical: implement
65-
return 5;
67+
$twoFAService = $this->getService(TwoFAServiceInterface::class);
68+
if (!$twoFAService instanceof TwoFAResendableInterface) {
69+
return 0;
70+
}
71+
72+
$userId = $this->getService(TwoFAUserServiceInterface::class)->getPendingUserId();
73+
return $twoFAService->getRemainingAttempts($userId);
74+
}
75+
76+
public function getResendCooldownRemaining(): int
77+
{
78+
$twoFAService = $this->getService(TwoFAServiceInterface::class);
79+
if (!$twoFAService instanceof TwoFAResendableInterface) {
80+
return 0;
81+
}
82+
83+
$userId = $this->getService(TwoFAUserServiceInterface::class)->getPendingUserId();
84+
return $twoFAService->getCooldownRemaining($userId);
6685
}
6786

6887
public function isExternalAuthUser(): bool

tests/Unit/Authentication/TwoFactorAuth/Controller/TwoFactorAuthControllerTest.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,14 @@ public function resendCodeSendsSuccessResponse(): void
8787
$twoFAServiceSpy->expects($this->once())
8888
->method('resend')
8989
->with($userId);
90+
$twoFAServiceSpy->method('getRemainingAttempts')
91+
->with($userId)
92+
->willReturn($remaining = 3);
9093

9194
$jsonResponseSpy = $this->createMock(JsonResponseInterface::class);
9295
$jsonResponseSpy->expects($this->once())
9396
->method('send')
94-
->with(['success' => true]);
97+
->with(['success' => true, 'remainingAttempts' => $remaining]);
9598

9699
$this->getSut(
97100
twoFAService: $twoFAServiceSpy,

tests/Unit/Authentication/TwoFactorAuth/OTP/OtpFacadeTest.php

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,17 @@ public function getRemainingAttemptsSubtractsCurrentAttempts(): void
260260
$this->assertSame(2, $sut->getRemainingAttempts(uniqid()));
261261
}
262262

263+
#[Test]
264+
public function getCooldownRemainingDelegatesToSendPolicy(): void
265+
{
266+
$sendPolicyMock = $this->createMock(OtpSendPolicyServiceInterface::class);
267+
$sendPolicyMock->method('getCooldownRemaining')
268+
->with($userId = uniqid())
269+
->willReturn(42);
270+
271+
$this->assertSame(42, $this->getSut(sendPolicy: $sendPolicyMock)->getCooldownRemaining($userId));
272+
}
273+
263274
#[Test]
264275
public function getRemainingAttemptsReturnsZeroWhenAttemptsExceedMax(): void
265276
{

0 commit comments

Comments
 (0)