Skip to content

Commit

Permalink
Handle authentication timeout occurences
Browse files Browse the repository at this point in the history
  • Loading branch information
MKodde committed Sep 12, 2024
1 parent 01e6079 commit b77dc7c
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 29 deletions.
26 changes: 3 additions & 23 deletions src/Controller/AuthenticationStatusController.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public function __invoke(): JsonResponse
return $this->refreshAuthenticationPage();
}


$isAuthenticated = $this->tiqrService->isAuthenticated();

if ($isAuthenticated) {
Expand All @@ -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();
}

Expand Down Expand Up @@ -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.
*
Expand Down
40 changes: 34 additions & 6 deletions src/Tiqr/Legacy/TiqrService.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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;

This comment has been minimized.

Copy link
@mharte-ib

mharte-ib Sep 16, 2024

Contributor

Is this change intended?


private SessionInterface $session;

Expand Down Expand Up @@ -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)');
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
2 changes: 2 additions & 0 deletions src/Tiqr/TiqrServiceInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -244,4 +244,6 @@ public function getSariForSessionIdentifier(string $identifier): string;
public function stateStorageHealthCheck(): HealthCheckResultDto;

public function isEnrollmentTimedOut(): bool;

public function isAuthenticationTimedOut(): bool;
}

0 comments on commit b77dc7c

Please sign in to comment.