diff --git a/Classes/Controller/BackendController.php b/Classes/Controller/BackendController.php index c6be578..4a4ff86 100644 --- a/Classes/Controller/BackendController.php +++ b/Classes/Controller/BackendController.php @@ -184,7 +184,7 @@ public function newWebAuthnAction(): void */ public function createAction(string $secret, string $secondFactorFromApp, string $name = ''): void { - $isValid = TOTPService::checkIfOtpIsValid($secret, $secondFactorFromApp); + $isValid = $this->tOTPService->checkIfOtpIsValid($secret, $secondFactorFromApp); if (!$isValid) { $this->addFlashMessage( diff --git a/Classes/Controller/LoginController.php b/Classes/Controller/LoginController.php index e703991..0c23b80 100644 --- a/Classes/Controller/LoginController.php +++ b/Classes/Controller/LoginController.php @@ -242,7 +242,7 @@ public function setupWebAuthnAction(?string $username = null): void */ public function createSecondFactorAction(string $secret, string $secondFactorFromApp, string $name = ''): void { - $isValid = TOTPService::checkIfOtpIsValid($secret, $secondFactorFromApp); + $isValid = $this->tOTPService->checkIfOtpIsValid($secret, $secondFactorFromApp); if (!$isValid) { $this->addFlashMessage( @@ -441,7 +441,7 @@ private function enteredTotpMatchesAnyTotpFactor(string $enteredSecondFactor, Ac { $totpFactors = $this->secondFactorRepository->findByAccountAndType($account, SecondFactor::TYPE_TOTP); foreach ($totpFactors as $secondFactor) { - if (TOTPService::checkIfOtpIsValid($secondFactor->getSecret(), $enteredSecondFactor)) { + if ($this->tOTPService->checkIfOtpIsValid($secondFactor->getSecret(), $enteredSecondFactor)) { return true; } } diff --git a/Classes/Service/TOTPService.php b/Classes/Service/TOTPService.php index bab5e20..8bb7283 100644 --- a/Classes/Service/TOTPService.php +++ b/Classes/Service/TOTPService.php @@ -4,8 +4,8 @@ use chillerlan\QRCode\QRCode; use chillerlan\QRCode\QROptions; -use Neos\Flow\Security\Account; use Neos\Flow\Annotations as Flow; +use Neos\Flow\Security\Account; use Neos\Neos\Domain\Repository\DomainRepository; use Neos\Neos\Domain\Repository\SiteRepository; use OTPHP\TOTP; @@ -14,31 +14,48 @@ class TOTPService { /** * @Flow\Inject - * @var DomainRepository */ - protected $domainRepository; + protected DomainRepository $domainRepository; /** * @Flow\Inject - * @var SiteRepository */ - protected $siteRepository; + protected SiteRepository $siteRepository; /** * @Flow\InjectConfiguration(path="issuerName") - * @var string | null */ - protected $issuerName; + protected string|null $issuerName; + + /** + * @Flow\InjectConfiguration(path="totpLeewayInSeconds") + */ + protected int $totpLeewayInSeconds; public static function generateNewTotp(): TOTP { return TOTP::create(); } - public static function checkIfOtpIsValid(string $secret, string $submittedOtp): bool + public function checkIfOtpIsValid(string $secret, string $submittedOtp): bool { $otp = TOTP::create($secret); - return $otp->verify($submittedOtp); + + $leeway = (int)$this->totpLeewayInSeconds; + $period = $otp->getPeriod(); + + if ($leeway <= 0) { + // No leeway configured: exact-match verification (single window). + return $otp->verify($submittedOtp); + } + + if ($leeway >= $period) { + // The leeway MUST be lower than the TOTP period, otherwise verify() throws. + // Clamp to the maximum allowed value instead of failing at login time. + $leeway = $period - 1; + } + + return $otp->verify($submittedOtp, null, $leeway); } public function generateQRCodeForTokenAndAccount(TOTP $otp, Account $account): string diff --git a/Configuration/Settings.2FA.yaml b/Configuration/Settings.2FA.yaml index 148e510..8200bb5 100644 --- a/Configuration/Settings.2FA.yaml +++ b/Configuration/Settings.2FA.yaml @@ -8,6 +8,10 @@ Sandstorm: enforce2FAForRoles: [] # (optional) if set this will be used as a naming convention for the TOTP. If empty the Site name will be used issuerName: '' + # Acceptable TOTP clock drift in seconds. verify() will accept codes from + # (now - leeway) through (now + leeway). 0 disables leeway (exact match only). + # MUST be lower than the 30s TOTP period; values >= 30 are clamped to 29. + totpLeewayInSeconds: 0 webAuthn: # Human-readable relying party name shown in the browser's WebAuthn prompt. diff --git a/README.md b/README.md index 7254bf9..cbf50d0 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,21 @@ Sandstorm: issuerName: "" ``` +### TOTP leeway + +By default, TOTP codes are verified against the current 30-second window only, with no tolerance for +clock drift between the user's device and the server. If users occasionally hit "invalid code" +errors near the boundary of a code's lifetime, you can allow some drift via: + +```yml +Sandstorm: + NeosTwoFactorAuthentication: + # Acceptable TOTP clock drift in seconds. Codes from (now - leeway) through (now + leeway) + # are accepted. 0 disables leeway (exact match only). MUST be lower than the 30s TOTP period; + # values >= 30 are clamped to 29. + totpLeewayInSeconds: 5 +``` + ## Tested 2FA apps Thx to @Sebobo @Benjamin-K for creating a list of supported and testet apps!