From b77dc7c11da87fe94496b30befc5f340b36b1c7c Mon Sep 17 00:00:00 2001 From: Michiel Kodde Date: Thu, 12 Sep 2024 13:36:58 +0200 Subject: [PATCH] Handle authentication timeout occurences --- .../AuthenticationStatusController.php | 26 ++---------- src/Tiqr/Legacy/TiqrService.php | 40 ++++++++++++++++--- src/Tiqr/TiqrServiceInterface.php | 2 + 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/Controller/AuthenticationStatusController.php b/src/Controller/AuthenticationStatusController.php index 469981d0..0c6a016f 100644 --- a/src/Controller/AuthenticationStatusController.php +++ b/src/Controller/AuthenticationStatusController.php @@ -57,6 +57,7 @@ public function __invoke(): JsonResponse return $this->refreshAuthenticationPage(); } + $isAuthenticated = $this->tiqrService->isAuthenticated(); if ($isAuthenticated) { @@ -65,7 +66,8 @@ public function __invoke(): JsonResponse return $this->refreshAuthenticationPage(); } - if ($this->authenticationChallengeIsExpired()) { + if ($this->tiqrService->isAuthenticationTimedOut()) { + $this->logger->info('The authentication timed out'); return $this->timeoutNeedsManualRetry(); } @@ -111,28 +113,6 @@ private function refreshAuthenticationPage(): JsonResponse return $this->generateAuthenticationStatusResponse('needs-refresh'); } - /** - * Check if the authentication challenge is expired. - * - * If the challenge is expired, the page should be refreshed so a new - * challenge and QR code is generated. - * - * @return bool - */ - private function authenticationChallengeIsExpired(): bool - { - // The use of authenticationUrl() here is a hack, because it depends on an implementation detail - // of this function. - // Effectively this does a $this->_stateStorage->getValue(self::PREFIX_CHALLENGE . $sessionKey); - // To check that the session key still exists in the Tiqr_Service's state storage - try { - $this->tiqrService->authenticationUrl(); - } catch (Exception) { - return true; - } - return false; - } - /** * Generate a response for authentication.html: Ask the user to retry. * diff --git a/src/Tiqr/Legacy/TiqrService.php b/src/Tiqr/Legacy/TiqrService.php index 915d390a..3e77d905 100644 --- a/src/Tiqr/Legacy/TiqrService.php +++ b/src/Tiqr/Legacy/TiqrService.php @@ -38,9 +38,11 @@ use Tiqr_Service; use Tiqr_StateStorage_StateStorageInterface; + /** * Wrapper around the legacy Tiqr service. * + * @SuppressWarnings(PHPMD.ExcessiveClassComplexity) * @SuppressWarnings(PHPMD.TooManyPublicMethods) * @SuppressWarnings(PHPMD.CouplingBetweenObjects) * It's Legacy. @@ -56,12 +58,17 @@ final class TiqrService implements TiqrServiceInterface */ private const ENROLLMENT_STARTED_AT = 'enrollment-started-at'; + /** + * Unix timestamp when the authentication started + */ + private const AUTHENTICATION_STARTED_AT = 'authentication-started-at'; + /** * The time (in seconds) that is extracted from the timeout * to prevent timeout issues right before the hard timeout * time is reached. */ - private const TIMEOUT_OFFSET = 296; + private const TIMEOUT_OFFSET = 2; private SessionInterface $session; @@ -115,10 +122,8 @@ public function generateEnrollmentKey(string $sari): string $userId = $this->generateId(); $this->logger->debug('Storing the userId=' . $userId . ' to session state'); $this->session->set('userId', $userId); - $registrationStartedAt = time(); - $this->logger->debug(sprintf('Storing the %s = %s', self::ENROLLMENT_STARTED_AT, $registrationStartedAt)); - $this->session->set(self::ENROLLMENT_STARTED_AT, $registrationStartedAt); + $this->recordStartTime(self::ENROLLMENT_STARTED_AT); // The session ID is used to link the tiqr library's enrollment session to the user's browser session $sessionId = $this->session->getId(); $this->logger->debug('Clearing the previous enrollment state(s)'); @@ -208,7 +213,7 @@ public function startAuthentication(string $userId, string $sari): string $this->session->set('sessionKey', $sessionKey); $this->setSariForSessionIdentifier($sessionKey, $sari); - + $this->recordStartTime(self::AUTHENTICATION_STARTED_AT); return $this->tiqrService->generateAuthURL($sessionKey); } catch (Exception $e) { // Catch errors from the tiqr-server and up-cycle them to exceptions that are meaningful to our domain @@ -470,15 +475,38 @@ protected function getEnrollmentTimeout(): int return Tiqr_Service::ENROLLMENT_EXPIRE; } + public function isAuthenticationTimedOut(): bool + { + $this->initSession(); + $this->logger->debug('Checking if authentication timeout is reached'); + $startedAt = $this->session->get(self::AUTHENTICATION_STARTED_AT); + assert(is_int($startedAt)); + return TimeoutHelper::isTimedOut( + time(), + $startedAt, + $this->getAuthenticationTimeout(), + self::TIMEOUT_OFFSET + ); + } + public function isEnrollmentTimedOut(): bool { $this->initSession(); $this->logger->debug('Checking if enrollment timeout is reached'); + $startedAt = $this->session->get(self::ENROLLMENT_STARTED_AT); + assert(is_int($startedAt)); return TimeoutHelper::isTimedOut( time(), - $this->session->get(self::ENROLLMENT_STARTED_AT), + $startedAt, $this->getEnrollmentTimeout(), self::TIMEOUT_OFFSET ); } + + private function recordStartTime(string $sessionFieldIdentifier): void + { + $startedAt = time(); + $this->logger->debug(sprintf('Storing the %s = %s', $sessionFieldIdentifier, $startedAt)); + $this->session->set($sessionFieldIdentifier, $startedAt); + } } diff --git a/src/Tiqr/TiqrServiceInterface.php b/src/Tiqr/TiqrServiceInterface.php index 181fd05d..d8bfd54c 100644 --- a/src/Tiqr/TiqrServiceInterface.php +++ b/src/Tiqr/TiqrServiceInterface.php @@ -244,4 +244,6 @@ public function getSariForSessionIdentifier(string $identifier): string; public function stateStorageHealthCheck(): HealthCheckResultDto; public function isEnrollmentTimedOut(): bool; + + public function isAuthenticationTimedOut(): bool; }