From 0dd651efa99b1b1403e443da26aee6998c0f0c82 Mon Sep 17 00:00:00 2001 From: Paul Rijke Date: Thu, 6 Jun 2024 13:54:36 +0200 Subject: [PATCH 1/2] Add cookie handling listeners and fix kernel path and deprecations --- bin/console | 2 +- config/services.yaml | 10 ++ public/index.php | 2 +- src/Kernel.php | 4 +- .../AuthenticatedUserListener.php | 66 ++++++++++++ .../ExplicitSessionTimeoutListener.php | 102 ++++++++++++++++++ .../Session/SessionLifetimeGuardTest.php | 2 +- .../Security/Session/SessionStorageTest.php | 2 +- 8 files changed, 184 insertions(+), 6 deletions(-) create mode 100644 src/Surfnet/StepupRa/RaBundle/EventListener/AuthenticatedUserListener.php create mode 100644 src/Surfnet/StepupRa/RaBundle/EventListener/ExplicitSessionTimeoutListener.php diff --git a/bin/console b/bin/console index a58bfee2..44e1fdbd 100755 --- a/bin/console +++ b/bin/console @@ -1,7 +1,7 @@ #!/usr/bin/env php ['updateLastInteractionMoment', 6], + ]; + } + + public function updateLastInteractionMoment(RequestEvent $event): void + { + $token = $this->tokenStorage->getToken(); + + if ($token === null || !$this->sessionLifetimeGuard->sessionLifetimeWithinLimits($this->sessionStateHandler)) { + return; + } + $this->logger->notice('Logged in user with a session within time limits detected, updating session state'); + + // see ExplicitSessionTimeoutHandler for the rationale + if ($event->getRequest()->getMethod() === 'GET') { + $this->sessionStateHandler->setCurrentRequestUri($event->getRequest()->getRequestUri()); + } + $this->sessionStateHandler->updateLastInteractionMoment(); + } +} diff --git a/src/Surfnet/StepupRa/RaBundle/EventListener/ExplicitSessionTimeoutListener.php b/src/Surfnet/StepupRa/RaBundle/EventListener/ExplicitSessionTimeoutListener.php new file mode 100644 index 00000000..569e2fbe --- /dev/null +++ b/src/Surfnet/StepupRa/RaBundle/EventListener/ExplicitSessionTimeoutListener.php @@ -0,0 +1,102 @@ + ['checkSessionTimeout', 5], + ]; + } + + public function checkSessionTimeout(RequestEvent $event): void + { + $token = $this->tokenStorage->getToken(); + + if ($token === null || $this->sessionLifetimeGuard->sessionLifetimeWithinLimits($this->authenticatedSession)) { + return; + } + + $invalidatedBy = []; + if (!$this->sessionLifetimeGuard->sessionLifetimeWithinAbsoluteLimit($this->authenticatedSession)) { + $invalidatedBy[] = 'absolute'; + } + + if (!$this->sessionLifetimeGuard->sessionLifetimeWithinRelativeLimit($this->authenticatedSession)) { + $invalidatedBy[] = 'relative'; + } + + $this->logger->notice(sprintf( + 'Authenticated user found, but session was determined to be outside of the "%s" time limit. User will ' + . 'be logged out and redirected to session-expired page to attempt new login.', + implode(' and ', $invalidatedBy), + )); + + $request = $event->getRequest(); + + // if the current request was not a GET request we cannot safely redirect to that page after login as it + // may require a form resubmit for instance. Therefor, we redirect to the last GET request (either current + // or previous). + $afterLoginRedirectTo = $this->authenticatedSession->getCurrentRequestUri(); + + if ($event->getRequest()->getMethod() === 'GET') { + $afterLoginRedirectTo = $event->getRequest()->getRequestUri(); + } + + // log the user out using Symfony methodology, see the LogoutListener + $event->setResponse(new RedirectResponse($this->router->generate('selfservice_security_session_expired'))); + + // something to clear cookies + $this->eventDispatcher->dispatch(new LogoutEvent($request, $token)); + $this->tokenStorage->setToken(null); + + // the session is restarted after invalidation during the logout, so we can (re)store the last GET request + $this->authenticatedSession->setCurrentRequestUri($afterLoginRedirectTo); + } +} diff --git a/src/Surfnet/StepupRa/RaBundle/Tests/Security/Session/SessionLifetimeGuardTest.php b/src/Surfnet/StepupRa/RaBundle/Tests/Security/Session/SessionLifetimeGuardTest.php index 3599d5a5..7e206dac 100755 --- a/src/Surfnet/StepupRa/RaBundle/Tests/Security/Session/SessionLifetimeGuardTest.php +++ b/src/Surfnet/StepupRa/RaBundle/Tests/Security/Session/SessionLifetimeGuardTest.php @@ -239,7 +239,7 @@ private function setCurrentTime(DateTime $now = null): void { $nowProperty = new ReflectionProperty(DateTime::class, 'now'); $nowProperty->setAccessible(true); - $nowProperty->setValue($now); + $nowProperty->setValue(null, $now); } /** diff --git a/src/Surfnet/StepupRa/RaBundle/Tests/Security/Session/SessionStorageTest.php b/src/Surfnet/StepupRa/RaBundle/Tests/Security/Session/SessionStorageTest.php index 9e0c0de3..2b9fe9fe 100755 --- a/src/Surfnet/StepupRa/RaBundle/Tests/Security/Session/SessionStorageTest.php +++ b/src/Surfnet/StepupRa/RaBundle/Tests/Security/Session/SessionStorageTest.php @@ -343,6 +343,6 @@ private function setCurrentTime(DateTime $now = null): void { $nowProperty = new ReflectionProperty(DateTime::class, 'now'); $nowProperty->setAccessible(true); - $nowProperty->setValue($now); + $nowProperty->setValue(null, $now); } } From 3d28e82275060d6ccce39f00fa20f051b7a15de7 Mon Sep 17 00:00:00 2001 From: Paul Rijke Date: Wed, 12 Jun 2024 15:44:26 +0200 Subject: [PATCH 2/2] Process review feedback --- bin/console | 3 ++- composer.json | 2 +- config/routes/controllers.yaml | 2 +- public/index.php | 3 +-- src/{ => Surfnet/StepupRa}/Kernel.php | 4 ++-- .../RaBundle/EventListener/AuthenticatedUserListener.php | 4 +--- .../RaBundle/EventListener/ExplicitSessionTimeoutListener.php | 2 -- 7 files changed, 8 insertions(+), 12 deletions(-) rename src/{ => Surfnet/StepupRa}/Kernel.php (93%) diff --git a/bin/console b/bin/console index 44e1fdbd..0691a73a 100755 --- a/bin/console +++ b/bin/console @@ -1,7 +1,8 @@ #!/usr/bin/env php ['updateLastInteractionMoment', 6], ]; } diff --git a/src/Surfnet/StepupRa/RaBundle/EventListener/ExplicitSessionTimeoutListener.php b/src/Surfnet/StepupRa/RaBundle/EventListener/ExplicitSessionTimeoutListener.php index 569e2fbe..78a34060 100644 --- a/src/Surfnet/StepupRa/RaBundle/EventListener/ExplicitSessionTimeoutListener.php +++ b/src/Surfnet/StepupRa/RaBundle/EventListener/ExplicitSessionTimeoutListener.php @@ -49,8 +49,6 @@ public function __construct( public static function getSubscribedEvents(): array { return [ - // The firewall, which makes the token available, listens at P8 - // We must jump in after the firewall, forcing us to overwrite the translator locale. KernelEvents::REQUEST => ['checkSessionTimeout', 5], ]; }