From c8b0821b4c32ceced984e99d3c89e100930bed84 Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Mon, 12 Oct 2026 14:07:46 +0200 Subject: [PATCH 1/8] Add per-SP WAYF remember-choice core Introduce the backend core for remembering a user's chosen IdP on a per-SP basis, gated behind two independent opt-ins: an EngineBlock-wide config flag and the SP's own Manage metadata coin. This replaces, for opted-in SPs, the existing single global "remember choice" cookie with a signed-per-entry cookie keyed by SP entityId, so different SPs can have independent (and independently expiring) remembered IdPs. Only true/"1" values for the SP metadata coin enable the feature for that SP; any other value (including "false" or "-1") leaves it disabled, so a malformed Manage value can never silently turn on choice-remembering behaviour. - Coins/ServiceProvider/PushMetadataAssembler gain the wayf_remember_choice coin (metadata:coin:wayf_remember_choice), defaulting to false and parsed with strict true-only semantics. - RememberedIdpCookie encodes/decodes the "rememberedidps" cookie (deflate + base64 JSON, one {idp, expires} entry per SP entityId), validates entry shape defensively, drops expired entries on every read, and prunes down to a configurable maximum (keeping the highest-expiry entries) only when a new entry is added. - DiContainerRuntime/EngineBlockBootstrapper bridge the new cookie service and the per-SP feature flag into the legacy Corto layer. - SingleSignOn bypasses WAYF using a remembered IdP when the SP has opted in and the choice is still valid for one of the candidate IdPs, preserving the existing zero/one-candidate and SSO-notification shortcuts; the legacy global "rememberchoice" cookie path is now gated off for SPs using the new per-SP mode instead of double-firing. - ContinueToIdp persists a checked "remember this choice" selection into the cookie and normalizes/prunes it on every request regardless of whether this SP is enrolled, so stale entries age out naturally. - The WAYF view model, Twig templates and rememberChoice.js are wired so the per-SP checkbox submits a hidden form field instead of writing a client-side cookie, leaving the legacy global-cookie behaviour untouched for SPs that haven't opted in. Refs: #2065 --- config/packages/parameters.yml.dist | 6 + config/services/services.yml | 10 + .../Corto/Module/Service/ContinueToIdp.php | 59 +++ .../Corto/Module/Service/SingleSignOn.php | 86 +++- src/OpenConext/EngineBlock/Metadata/Coins.php | 9 +- .../Assembler/PushMetadataAssembler.php | 13 + .../Metadata/Entity/ServiceProvider.php | 6 +- .../EngineBlock/Service/CookieService.php | 38 ++ .../Service/Wayf/RememberedIdpCookie.php | 314 +++++++++++++ .../Bridge/DiContainerRuntime.php | 11 + .../Bridge/EngineBlockBootstrapper.php | 8 + .../Service/WayfRenderer.php | 2 + .../Service/WayfViewModelFactory.php | 2 + .../Twig/Extensions/Extension/Wayf.php | 5 +- .../ViewModel/WayfViewModel.php | 1 + .../Assembler/PushMetadataAssemblerTest.php | 52 ++- .../Test/Corto/Module/BindingsTest.php | 12 + .../Module/Service/ContinueToIdpTest.php | 143 ++++++ .../Metadata/Entity/ServiceProviderTest.php | 7 + .../Service/Wayf/RememberedIdpCookieTest.php | 420 ++++++++++++++++++ .../DiContainerRuntimeTest.php | 48 +- .../Service/WayfRendererTest.php | 71 +++ .../Twig/Extensions/Extension/WayfTest.php | 29 ++ .../Twig/WayfScriptConfigRenderTest.php | 142 ++++++ theme/base/javascripts/wayf/rememberChoice.js | 20 +- theme/base/javascripts/wayf/submitForm.js | 2 +- .../Proxy/Partials/WAYF/idp/idpForm.html.twig | 1 + .../Partials/WAYF/scriptConfig.html.twig | 3 +- .../Authentication/View/Proxy/wayf.html.twig | 1 + 29 files changed, 1506 insertions(+), 15 deletions(-) create mode 100644 src/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookie.php create mode 100644 tests/library/EngineBlock/Test/Corto/Module/Service/ContinueToIdpTest.php create mode 100644 tests/unit/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookieTest.php create mode 100644 tests/unit/OpenConext/EngineBlockBundle/Twig/WayfScriptConfigRenderTest.php diff --git a/config/packages/parameters.yml.dist b/config/packages/parameters.yml.dist index da4509d200..d5695e5f28 100644 --- a/config/packages/parameters.yml.dist +++ b/config/packages/parameters.yml.dist @@ -180,6 +180,12 @@ parameters: ## Allow users to save their selected IdP and then auto-select it on returning visits. wayf.remember_choice: false + ## Allow SPs to opt in to remembered IdP choices scoped per SP through Manage metadata. + wayf.remember_choice_per_idp: false + ## Lifetime for remembered IdP choices scoped per SP, in seconds. + wayf.remember_choice_per_idp_lifetime: 7776000 + ## Maximum number of remembered IdP choice entries to retain per SP. + wayf.remember_choice_per_idp_max: 16 ## Toggle the default IdP quick link banner on the WAYF. wayf.display_default_idp_banner_on_wayf: true diff --git a/config/services/services.yml b/config/services/services.yml index f81b521e48..ec37af88e1 100644 --- a/config/services/services.yml +++ b/config/services/services.yml @@ -15,6 +15,7 @@ services: autowire: true autoconfigure: true arguments: + $rememberChoicePerIdp: '%wayf.remember_choice_per_idp%' $preferredIdpEntityIds: '%wayf.preferred_idp_entity_ids%' tags: - { name: kernel.event_subscriber } @@ -173,6 +174,15 @@ services: OpenConext\EngineBlock\Service\CookieService: + OpenConext\EngineBlock\Service\Wayf\RememberedIdpCookie: + autowire: true + arguments: + $lifetime: '%wayf.remember_choice_per_idp_lifetime%' + $maxEntries: '%wayf.remember_choice_per_idp_max%' + $cookieDomain: '%hostname%' + $cookiePath: '%cookie.path%' + $secure: '%cookie.secure%' + OpenConext\EngineBlock\Metadata\Factory\Factory\ServiceProviderFactory: arguments: - '@engineblock.compat.metadata.definitions' diff --git a/library/EngineBlock/Corto/Module/Service/ContinueToIdp.php b/library/EngineBlock/Corto/Module/Service/ContinueToIdp.php index 39c0bdee0f..5e679cd5ea 100644 --- a/library/EngineBlock/Corto/Module/Service/ContinueToIdp.php +++ b/library/EngineBlock/Corto/Module/Service/ContinueToIdp.php @@ -19,6 +19,7 @@ use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider; use OpenConext\EngineBlock\Metadata\Factory\Factory\ServiceProviderFactory; use OpenConext\EngineBlock\Metadata\X509\KeyPairFactory; +use OpenConext\EngineBlock\Service\Wayf\RememberedIdpCookie; use Symfony\Component\HttpFoundation\Request; use Twig\Environment; @@ -113,9 +114,67 @@ public function serve($serviceName, Request $httpRequest) $log->info('Raw HTTP request', array('http_request' => (string) $application->getHttpRequest())); } + $this->_persistRememberedChoice($sp, $idp, $httpRequest->request->get('rememberChoice')); + $this->_server->sendAuthenticationRequest($request, $selectedIdp); } + /** + * @param ServiceProvider $sp + * @param mixed $idp + * @param string|null $rememberChoice + */ + private function _persistRememberedChoice(ServiceProvider $sp, $idp, $rememberChoice) + { + $application = EngineBlock_ApplicationSingleton::getInstance(); + $container = $application->getDiContainer(); + $runtime = $application->getDiContainerRuntime(); + $log = $this->_server->getLogger(); + + $rememberedIdpCookie = $runtime->rememberedIdpCookie; + + $raw = $container->getSymfonyRequest()->cookies->get(RememberedIdpCookie::NAME); + $entries = $rememberedIdpCookie->loadValidEntries($raw, $log); + + if (!$this->_isRememberChoiceChecked($rememberChoice)) { + return; + } + + if (!RememberedIdpCookie::isEnabledForServiceProvider( + $container->getRememberChoice() === true, + $runtime->isRememberChoicePerIdpEnabled(), + $sp + )) { + return; + } + + $result = $rememberedIdpCookie->add($entries, $sp->entityId, $idp->entityId); + $rememberedIdpCookie->write($result['entries'], $result['expires']); + + $log->info(sprintf( + 'WAYF-remember-my-choice set for SP %s and IdP %s and expiry %d', + $sp->entityId, + $idp->entityId, + $result['expires'] + )); + + if ($result['dropped'] > 0) { + $log->info(sprintf( + 'WAYF-remember-my-choice %d entities dropped from cookie', + $result['dropped'] + )); + } + } + + /** + * @param string|null $rememberChoice + * @return bool + */ + private function _isRememberChoiceChecked($rememberChoice) + { + return $rememberChoice === '1'; + } + /** * @param EngineBlock_Corto_ProxyServer $proxyServer * @return ServiceProvider diff --git a/library/EngineBlock/Corto/Module/Service/SingleSignOn.php b/library/EngineBlock/Corto/Module/Service/SingleSignOn.php index 5a9b102505..a79633a8dd 100644 --- a/library/EngineBlock/Corto/Module/Service/SingleSignOn.php +++ b/library/EngineBlock/Corto/Module/Service/SingleSignOn.php @@ -20,7 +20,9 @@ use OpenConext\EngineBlock\Metadata\Factory\Factory\ServiceProviderFactory; use OpenConext\EngineBlock\Metadata\Discovery; use OpenConext\EngineBlock\Metadata\X509\KeyPairFactory; +use OpenConext\EngineBlock\Service\Wayf\RememberedIdpCookie; use OpenConext\EngineBlock\Service\Wayf\WayfIdp; +use OpenConext\EngineBlockBundle\Bridge\DiContainerRuntime; use OpenConext\EngineBlockBundle\Service\DiscoverySelectionService; use SAML2\AuthnRequest; use SAML2\Constants; @@ -221,7 +223,17 @@ public function serve($serviceName, Request $httpRequest) } // Auto-select IdP when 'wayf.rememberChoice' feature is enabled and is allowed for the current request - if (($container->getRememberChoice() === true) && !($request->getForceAuthn() || $request->isDebugRequest())) { + $runtime = $application->getDiContainerRuntime(); + if ($this->_sendAuthenticationRequestForRememberedIdpPerServiceProvider($request, $sp, $candidateIDPs)) { + return; + } + + // Auto-select IdP when 'wayf.rememberChoice' feature is enabled and is allowed for the current request, + // and the per-SP remembered-choice feature is not enabled. + if ($container->getRememberChoice() === true + && $runtime->isRememberChoicePerIdpEnabled() === false + && !($request->getForceAuthn() || $request->isDebugRequest()) + ) { $cookies = $container->getSymfonyRequest()->cookies->all(); if (array_key_exists('rememberchoice', $cookies)) { $remembered = json_decode($cookies['rememberchoice']); @@ -457,6 +469,64 @@ protected function _limitScopeToRememberedIdp( return $scopedIdps; } + /** + * @param string[] $candidateIDPs + */ + protected function _sendAuthenticationRequestForRememberedIdpPerServiceProvider( + EngineBlock_Saml2_AuthnRequestAnnotationDecorator $request, + ServiceProvider $sp, + array $candidateIDPs + ): bool { + $application = EngineBlock_ApplicationSingleton::getInstance(); + $container = $application->getDiContainer(); + $runtime = $application->getDiContainerRuntime(); + $log = $this->_server->getLogger(); + + if (!RememberedIdpCookie::isEnabledForServiceProvider( + $container->getRememberChoice() === true, + $runtime->isRememberChoicePerIdpEnabled(), + $sp + ) + || $request->getForceAuthn() + || $request->isDebugRequest() + ) { + return false; + } + + $rememberedIdp = $this->_findRememberedIdpForServiceProvider($runtime, $sp->entityId, $candidateIDPs); + if ($rememberedIdp === null) { + return false; + } + + $log->info(sprintf( + 'WAYF-remember-my-choice WAYF skipped for %s because of preselected %s', + $sp->entityId, + $rememberedIdp + )); + $this->_server->sendAuthenticationRequest($request, $rememberedIdp); + + return true; + } + + /** + * @param string[] $candidateIdpEntityIds + */ + protected function _findRememberedIdpForServiceProvider( + DiContainerRuntime $runtime, + string $serviceProviderEntityId, + array $candidateIdpEntityIds + ): ?string { + $rememberedIdpCookie = $runtime->rememberedIdpCookie; + $log = $this->_server->getLogger(); + + $raw = EngineBlock_ApplicationSingleton::getInstance()->getDiContainer() + ->getSymfonyRequest()->cookies->get(RememberedIdpCookie::NAME); + + $entries = $rememberedIdpCookie->loadValidEntries($raw, $log); + + return $rememberedIdpCookie->find($entries, $serviceProviderEntityId, $candidateIdpEntityIds); + } + protected function _showWayf(EngineBlock_Saml2_AuthnRequestAnnotationDecorator $request, array $candidateIdpEntityIds) { // Post to the 'continueToIdp' service @@ -482,6 +552,17 @@ protected function _showWayf(EngineBlock_Saml2_AuthnRequestAnnotationDecorator $ $diContainerRuntime = $application->getDiContainerRuntime(); + $isRememberChoicePerIdpEnabled = $diContainerRuntime->isRememberChoicePerIdpEnabled(); + $rememberChoicePerIdp = RememberedIdpCookie::isEnabledForServiceProvider( + $container->getRememberChoice() === true, + $isRememberChoicePerIdpEnabled, + $serviceProvider + ); + + $rememberChoiceFeature = $isRememberChoicePerIdpEnabled + ? $rememberChoicePerIdp + : $container->getRememberChoice(); + $output = $diContainerRuntime->wayfRenderer->render( idpList: $idpList, preferredIdpEntityIds: $diContainerRuntime->getPreferredIdpEntityIds(), @@ -491,10 +572,11 @@ protected function _showWayf(EngineBlock_Saml2_AuthnRequestAnnotationDecorator $ shouldDisplayBanner: (bool) $container->shouldDisplayDefaultIdpBannerOnWayf(), backLink: $container->isUiOptionReturnToSpActive(), cutoffPoint: $container->getCutoffPointForShowingUnfilteredIdps(), - rememberChoice: $container->getRememberChoice(), + rememberChoice: $rememberChoiceFeature, showRequestAccess: $serviceProvider->getCoins()->displayUnconnectedIdpsWayf(), requestId: $request->getId(), serviceProvider: $serviceProvider, + rememberChoicePerIdp: $rememberChoicePerIdp, ); $this->_server->sendOutput($output); } diff --git a/src/OpenConext/EngineBlock/Metadata/Coins.php b/src/OpenConext/EngineBlock/Metadata/Coins.php index d14c69f83d..3275c5a560 100644 --- a/src/OpenConext/EngineBlock/Metadata/Coins.php +++ b/src/OpenConext/EngineBlock/Metadata/Coins.php @@ -44,7 +44,8 @@ public static function createForServiceProvider( $additionalLogging, $signatureMethod, $stepupForceAuthn, - $collabEnabled + $collabEnabled, + bool $wayfRememberChoice = false ) { return new self([ 'isConsentRequired' => $isConsentRequired, @@ -63,6 +64,7 @@ public static function createForServiceProvider( 'stepupRequireLoa' => $stepupRequireLoa, 'stepupForceAuthn' => $stepupForceAuthn, 'collabEnabled' => $collabEnabled, + 'wayfRememberChoice' => $wayfRememberChoice, ]); } @@ -150,6 +152,11 @@ public function displayUnconnectedIdpsWayf() return $this->getValue('displayUnconnectedIdpsWayf', false); } + public function wayfRememberChoice(): bool + { + return $this->getValue('wayfRememberChoice', false) === true; + } + public function termsOfServiceUrl() { return $this->getValue('termsOfServiceUrl'); diff --git a/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php b/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php index f9bff297ad..6a493ce691 100644 --- a/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php +++ b/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php @@ -220,6 +220,10 @@ private function assembleSp(stdClass $connection) $properties += $this->setPathFromObjectBool([$connection, 'metadata:coin:transparant_issuer'], 'isTransparentIssuer'); $properties += $this->setPathFromObjectBool([$connection, 'metadata:coin:trusted_proxy'], 'isTrustedProxy'); $properties += $this->setPathFromObjectBool([$connection, 'metadata:coin:display_unconnected_idps_wayf'], 'displayUnconnectedIdpsWayf'); + $properties += $this->setPathFromConnectionStrictBool( + [$connection, 'metadata:coin:wayf_remember_choice'], + 'wayfRememberChoice' + ); $properties += $this->assembleIsConsentRequired($connection); @@ -458,6 +462,15 @@ private function setPathFromObjectBool(array $from, string $to): array return array($to => (bool)$reference); } + private function setPathFromConnectionStrictBool(array $from, string $to): array + { + $reference = $this->getValueFromPath($from); + if (is_null($reference)) { + return array($to => null); + } + return array($to => $reference === true || $reference === '1'); + } + private function getValueFromPath(array $from) { $pathParts = explode(':', $from[1]); diff --git a/src/OpenConext/EngineBlock/Metadata/Entity/ServiceProvider.php b/src/OpenConext/EngineBlock/Metadata/Entity/ServiceProvider.php index 7d81229c2b..5ac132d772 100644 --- a/src/OpenConext/EngineBlock/Metadata/Entity/ServiceProvider.php +++ b/src/OpenConext/EngineBlock/Metadata/Entity/ServiceProvider.php @@ -152,7 +152,8 @@ public function __construct( ?bool $stepupAllowNoToken = null, ?string $stepupRequireLoa = null, bool $stepupForceAuthn = false, - bool $collabEnabled = false + bool $collabEnabled = false, + bool $wayfRememberChoice = false ) { if (is_null($mdui)) { $mdui = Mdui::emptyMdui(); @@ -211,7 +212,8 @@ public function __construct( $additionalLogging, $signatureMethod, $stepupForceAuthn, - $collabEnabled + $collabEnabled, + $wayfRememberChoice ); } diff --git a/src/OpenConext/EngineBlock/Service/CookieService.php b/src/OpenConext/EngineBlock/Service/CookieService.php index 40cb377ee4..ab3566f1ae 100644 --- a/src/OpenConext/EngineBlock/Service/CookieService.php +++ b/src/OpenConext/EngineBlock/Service/CookieService.php @@ -47,6 +47,26 @@ public function setCookie( return setcookie($name, $value, $expire, $path, $domain, $secure, $httpOnly); } + public function setCookieWithSameSite( + string $name, + $value, + int $expire, + string $path, + string $domain, + bool $secure = true, + bool $httpOnly = true, + string $sameSite = 'None' + ): bool { + return setcookie($name, $value, [ + 'expires' => $expire, + 'path' => $path, + 'domain' => $domain, + 'secure' => $secure, + 'httponly' => $httpOnly, + 'samesite' => $sameSite, + ]); + } + /** * Clears the cookie for the specified name, path and domain. * @@ -66,4 +86,22 @@ public function clearCookie( ): bool { return setcookie($name, '', -1, $path, $domain, $secure, $httpOnly); } + + public function clearCookieWithSameSite( + string $name, + string $path, + string $domain, + bool $secure = true, + bool $httpOnly = true, + string $sameSite = 'None' + ): bool { + return setcookie($name, '', [ + 'expires' => -1, + 'path' => $path, + 'domain' => $domain, + 'secure' => $secure, + 'httponly' => $httpOnly, + 'samesite' => $sameSite, + ]); + } } diff --git a/src/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookie.php b/src/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookie.php new file mode 100644 index 0000000000..bd76df9f6e --- /dev/null +++ b/src/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookie.php @@ -0,0 +1,314 @@ +getCoins()->wayfRememberChoice() === true; + } + + public function __construct( + private readonly TimeProvider $timeProvider, + private readonly CookieService $cookieService, + private readonly int $lifetime, + private readonly int $maxEntries, + private readonly string $cookieDomain, + private readonly string $cookiePath, + private readonly bool $secure, + ) { + } + + /** + * @return array{ + * entries: array, + * invalid: bool, + * expired: int, + * changed: bool + * } + */ + public function normalize(?string $raw): array + { + if ($raw === null || $raw === '') { + return $this->normalizedResult([], false, 0, false); + } + + $decoded = base64_decode($raw, true); + if ($decoded === false) { + return $this->invalidResult(); + } + + $inflated = $this->inflate($decoded); + if ($inflated === null) { + return $this->invalidResult(); + } + + try { + $entries = json_decode($inflated, true, 512, JSON_THROW_ON_ERROR); + } catch (JsonException) { + return $this->invalidResult(); + } + + if (!is_array($entries)) { + return $this->invalidResult(); + } + + return $this->normalizeEntries($entries); + } + + /** + * @return array + */ + public function loadValidEntries(?string $raw, LoggerInterface $logger): array + { + $normalized = $this->normalize($raw); + + if ($normalized['invalid']) { + $logger->warning('Invalid rememberedidps cookie encountered, clearing it'); + $this->clear(); + + return []; + } + + if ($normalized['changed']) { + if (count($normalized['entries']) > 0) { + $this->write($normalized['entries']); + } else { + $this->clear(); + } + } + + return $normalized['entries']; + } + + /** @param array $entries */ + public function encode(array $entries): string + { + $json = json_encode($entries, JSON_THROW_ON_ERROR); + $deflated = gzdeflate($json); + if ($deflated === false) { + throw new RuntimeException('Unable to deflate remembered IdP cookie data'); + } + + return base64_encode($deflated); + } + + /** + * @param array $entries + * @param string[] $candidateIdpEntityIds + */ + public function find(array $entries, string $serviceProviderEntityId, array $candidateIdpEntityIds): ?string + { + if (!isset($entries[$serviceProviderEntityId])) { + return null; + } + + $entry = $entries[$serviceProviderEntityId]; + if (!$this->isValidEntry($serviceProviderEntityId, $entry) || $this->isExpired($entry['expires'])) { + return null; + } + + if (!in_array($entry['idp'], $candidateIdpEntityIds, true)) { + return null; + } + + return $entry['idp']; + } + + /** + * @param array $entries + * @return array{entries: array, expires: int, dropped: int} + */ + public function add(array $entries, string $serviceProviderEntityId, string $identityProviderEntityId): array + { + $expires = $this->cookieExpiresAt(); + $entries = $this->onlyValidUnexpiredEntries($entries); + $entries[$serviceProviderEntityId] = [ + 'idp' => $identityProviderEntityId, + 'expires' => $expires, + ]; + + $dropped = $this->pruneOverLimit($entries); + + return [ + 'entries' => $entries, + 'expires' => $expires, + 'dropped' => $dropped, + ]; + } + + public function cookieExpiresAt(): int + { + return $this->timeProvider->time() + $this->lifetime; + } + + /** + * @param array $entries + * @param int|null $expires Pass the expiry already computed by add() so the logged/returned + * value and the actual cookie expiry never drift apart. Defaults to a freshly computed + * expiry (e.g. when merely refreshing the cookie's lifetime after normalization). + */ + public function write(array $entries, ?int $expires = null): bool + { + return $this->cookieService->setCookieWithSameSite( + self::NAME, + $this->encode($entries), + $expires ?? $this->cookieExpiresAt(), + $this->cookiePath, + $this->cookieDomain, + $this->secure, + true, + 'None' + ); + } + + public function clear(): bool + { + return $this->cookieService->clearCookieWithSameSite( + self::NAME, + $this->cookiePath, + $this->cookieDomain, + $this->secure, + true, + 'None' + ); + } + + private function normalizeEntries(array $entries): array + { + $normalized = []; + $expired = 0; + + foreach ($entries as $serviceProviderEntityId => $entry) { + if (!$this->isValidEntry($serviceProviderEntityId, $entry)) { + return $this->invalidResult(); + } + + if ($this->isExpired($entry['expires'])) { + $expired++; + continue; + } + + $normalized[$serviceProviderEntityId] = $entry; + } + + return $this->normalizedResult($normalized, false, $expired, $expired > 0); + } + + private function inflate(string $decoded): ?string + { + set_error_handler(static fn(): bool => true); + try { + $inflated = gzinflate($decoded); + } finally { + restore_error_handler(); + } + + return $inflated === false ? null : $inflated; + } + + private function invalidResult(): array + { + return $this->normalizedResult([], true, 0, true); + } + + private function normalizedResult(array $entries, bool $invalid, int $expired, bool $changed): array + { + return [ + 'entries' => $entries, + 'invalid' => $invalid, + 'expired' => $expired, + 'changed' => $changed, + ]; + } + + private function isValidEntry(mixed $serviceProviderEntityId, mixed $entry): bool + { + if (!is_array($entry)) { + return false; + } + + $entryKeys = array_keys($entry); + sort($entryKeys); + + return is_string($serviceProviderEntityId) + && $serviceProviderEntityId !== '' + && $entryKeys === ['expires', 'idp'] + && is_string($entry['idp']) + && $entry['idp'] !== '' + && is_int($entry['expires']); + } + + private function isExpired(int $expires): bool + { + return $expires <= $this->timeProvider->time(); + } + + private function onlyValidUnexpiredEntries(array $entries): array + { + $validEntries = []; + foreach ($entries as $serviceProviderEntityId => $entry) { + if (!$this->isValidEntry($serviceProviderEntityId, $entry) || $this->isExpired($entry['expires'])) { + continue; + } + + $validEntries[$serviceProviderEntityId] = $entry; + } + + return $validEntries; + } + + private function pruneOverLimit(array &$entries): int + { + if (count($entries) <= $this->maxEntries) { + return 0; + } + + uksort($entries, function (string $left, string $right) use ($entries): int { + $expiryComparison = $entries[$right]['expires'] <=> $entries[$left]['expires']; + if ($expiryComparison !== 0) { + return $expiryComparison; + } + + return $left <=> $right; + }); + + $dropped = count($entries) - $this->maxEntries; + $entries = array_slice($entries, 0, $this->maxEntries, true); + + return $dropped; + } +} diff --git a/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php b/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php index 599e517aaf..3bf19fa452 100644 --- a/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php +++ b/src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php @@ -22,6 +22,7 @@ use OpenConext\EngineBlock\Request\CurrentCorrelationId; use OpenConext\EngineBlock\Service\FeedbackInfoCollectorInterface; use OpenConext\EngineBlock\Service\FeedbackStateHelperInterface; +use OpenConext\EngineBlock\Service\Wayf\RememberedIdpCookie; use OpenConext\EngineBlockBridge\Logger\LoginLogger; use OpenConext\EngineBlockBundle\Service\WayfRenderer; use Twig\Environment; @@ -35,6 +36,9 @@ final readonly class DiContainerRuntime { + /** + * @SuppressWarnings(PHPMD.ExcessiveParameterList) + */ public function __construct( public Environment $twig, public WayfRenderer $wayfRenderer, @@ -43,6 +47,8 @@ public function __construct( public FeedbackStateHelperInterface $feedbackStateHelper, public FeedbackInfoCollectorInterface $feedbackInfoCollector, public LoginLogger $loginLogger, + public RememberedIdpCookie $rememberedIdpCookie, + private bool $rememberChoicePerIdp = false, private array $preferredIdpEntityIds = [], ) { } @@ -51,4 +57,9 @@ public function getPreferredIdpEntityIds(): array { return $this->preferredIdpEntityIds; } + + public function isRememberChoicePerIdpEnabled(): bool + { + return $this->rememberChoicePerIdp; + } } diff --git a/src/OpenConext/EngineBlockBundle/Bridge/EngineBlockBootstrapper.php b/src/OpenConext/EngineBlockBundle/Bridge/EngineBlockBootstrapper.php index f4be71278f..42b71d8695 100644 --- a/src/OpenConext/EngineBlockBundle/Bridge/EngineBlockBootstrapper.php +++ b/src/OpenConext/EngineBlockBundle/Bridge/EngineBlockBootstrapper.php @@ -23,6 +23,7 @@ use OpenConext\EngineBlock\Request\CurrentCorrelationId; use OpenConext\EngineBlock\Service\FeedbackInfoCollectorInterface; use OpenConext\EngineBlock\Service\FeedbackStateHelperInterface; +use OpenConext\EngineBlock\Service\Wayf\RememberedIdpCookie; use OpenConext\EngineBlockBridge\Logger\LoginLogger; use OpenConext\EngineBlockBundle\Service\WayfRenderer; use Symfony\Component\EventDispatcher\EventSubscriberInterface; @@ -33,6 +34,9 @@ class EngineBlockBootstrapper implements EventSubscriberInterface { private readonly DiContainerRuntime $diContainerRuntime; + /** + * @SuppressWarnings(PHPMD.ExcessiveParameterList) + */ public function __construct( Environment $twig, WayfRenderer $wayfRenderer, @@ -41,6 +45,8 @@ public function __construct( FeedbackStateHelperInterface $feedbackStateHelper, FeedbackInfoCollectorInterface $feedbackInfoCollector, LoginLogger $loginLogger, + RememberedIdpCookie $rememberedIdpCookie, + bool $rememberChoicePerIdp = false, array $preferredIdpEntityIds = [], ) { $this->diContainerRuntime = new DiContainerRuntime( @@ -51,6 +57,8 @@ public function __construct( $feedbackStateHelper, $feedbackInfoCollector, $loginLogger, + $rememberedIdpCookie, + $rememberChoicePerIdp, $preferredIdpEntityIds, ); } diff --git a/src/OpenConext/EngineBlockBundle/Service/WayfRenderer.php b/src/OpenConext/EngineBlockBundle/Service/WayfRenderer.php index 7e56308ceb..68e694139e 100644 --- a/src/OpenConext/EngineBlockBundle/Service/WayfRenderer.php +++ b/src/OpenConext/EngineBlockBundle/Service/WayfRenderer.php @@ -50,6 +50,7 @@ public function render( bool $showRequestAccess, string $requestId, ServiceProvider $serviceProvider, + bool $rememberChoicePerIdp = false, ): string { $split = $this->splitter->split($idpList, $preferredIdpEntityIds); $showPreferredIdps = $split->hasPreferred(); @@ -74,6 +75,7 @@ public function render( showRequestAccess: $showRequestAccess, requestId: $requestId, serviceProvider: $serviceProvider, + rememberChoicePerIdp: $rememberChoicePerIdp, ); return $this->twig->render('@theme/Authentication/View/Proxy/wayf.html.twig', $viewModel->toArray()); diff --git a/src/OpenConext/EngineBlockBundle/Service/WayfViewModelFactory.php b/src/OpenConext/EngineBlockBundle/Service/WayfViewModelFactory.php index 67ae26b195..bf2cff7d92 100644 --- a/src/OpenConext/EngineBlockBundle/Service/WayfViewModelFactory.php +++ b/src/OpenConext/EngineBlockBundle/Service/WayfViewModelFactory.php @@ -49,6 +49,7 @@ public function create( bool $showRequestAccess, string $requestId, ServiceProvider $serviceProvider, + bool $rememberChoicePerIdp = false, ): WayfViewModel { return new WayfViewModel( action: $action, @@ -68,6 +69,7 @@ public function create( idpList: $idpList, regularIdpList: $regularIdpList, preferredIdpList: $preferredIdpList, + rememberChoicePerIdp: $rememberChoicePerIdp, ); } } diff --git a/src/OpenConext/EngineBlockBundle/Twig/Extensions/Extension/Wayf.php b/src/OpenConext/EngineBlockBundle/Twig/Extensions/Extension/Wayf.php index ccf28fbec9..1917ac5530 100644 --- a/src/OpenConext/EngineBlockBundle/Twig/Extensions/Extension/Wayf.php +++ b/src/OpenConext/EngineBlockBundle/Twig/Extensions/Extension/Wayf.php @@ -140,6 +140,7 @@ function (array $idp) use ($previousSelectionIndex) { * @param bool $showRequestAccess Show unconnected IdP's ? * @param bool $rememberChoiceFeature Remember the chosen IdP in Wayf? * @param int $cutoffPointForShowingUnfilteredIdps The cutoff point for showing unfiltered IdP's + * @param bool $rememberChoicePerIdp Remember the chosen IdP per SP instead of via the legacy global cookie? * * @return string Returns a json encoded config string. Used by the JavaScript of the Wayf to behave as intended. */ @@ -150,7 +151,8 @@ public function getWayfJsonConfig( $currentLocale, $showRequestAccess, $rememberChoiceFeature, - $cutoffPointForShowingUnfilteredIdps + $cutoffPointForShowingUnfilteredIdps, + $rememberChoicePerIdp = false ) { if ($showRequestAccess === true) { @@ -173,6 +175,7 @@ function ($idp) { 'cutoffPointForShowingUnfilteredIdps' => $cutoffPointForShowingUnfilteredIdps, 'rememberChoiceCookieName' => self::REMEMBER_CHOICE_COOKIE_NAME, 'rememberChoiceFeature' => $rememberChoiceFeature, + 'rememberChoicePerIdp' => $rememberChoicePerIdp, 'hideBookmarkableUrl' => $this->featureConfiguration->isEnabled('eb.hide_bookmarkable_url'), 'messages' => [ 'moreIdpResults' => $this->translator->trans('more_idp_results'), diff --git a/src/OpenConext/EngineBlockBundle/ViewModel/WayfViewModel.php b/src/OpenConext/EngineBlockBundle/ViewModel/WayfViewModel.php index 14afdb409c..d47e8461ff 100644 --- a/src/OpenConext/EngineBlockBundle/ViewModel/WayfViewModel.php +++ b/src/OpenConext/EngineBlockBundle/ViewModel/WayfViewModel.php @@ -52,6 +52,7 @@ public function __construct( public array $regularIdpList, /** @var WayfIdp[] */ public array $preferredIdpList, + public bool $rememberChoicePerIdp = false, ) { } diff --git a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php index 6ba075c2e9..a4fec6cb7e 100644 --- a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php +++ b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php @@ -160,7 +160,9 @@ public function testCoins($coinName, $roleType, $parameter, $type) switch ($type) { case 'bool': - $values = $this->validCoinValuesBool(); + $values = $coinName === 'wayf_remember_choice' + ? $this->validCoinValuesBoolStrictTrueOnly() + : $this->validCoinValuesBool(); break; case 'bool-forceAuthn': $values = $this->validCoinValuesBoolForceAuthn(); @@ -195,6 +197,40 @@ public function testCoins($coinName, $roleType, $parameter, $type) } } + public function test_it_disables_wayf_remember_choice_when_manage_coin_is_absent(): void + { + $connection = '{ + "2d96e27a-76cf-4ca2-ac70-ece5d4c49523": { + "allow_all_entities": true, + "allowed_connections": [], + "metadata": { + "coin": { + "stepup": {} + } + }, + "name": "https:\/\/role/sp", + "state": "prodaccepted", + "type": "saml20-sp" + } + }'; + + $roles = $this->assembler->assemble(json_decode($connection)); + + $this->assertInstanceOf(ServiceProvider::class, $roles[0]); + $this->assertFalse($roles[0]->getCoins()->wayfRememberChoice()); + + $connection = json_decode($connection); + + foreach (['false', '-1'] as $malformedTruthyString) { + $connection->{"2d96e27a-76cf-4ca2-ac70-ece5d4c49523"}->metadata->coin->wayf_remember_choice = $malformedTruthyString; + + $roles = $this->assembler->assemble($connection); + + $this->assertInstanceOf(ServiceProvider::class, $roles[0]); + $this->assertFalse($roles[0]->getCoins()->wayfRememberChoice()); + } + } + public function test_it_assembles_sfo_settings() { $connection = '{ @@ -381,6 +417,7 @@ public static function validCoins() ['transparant_issuer', 'saml20-sp', 'isTransparentIssuer', 'bool'], ['trusted_proxy', 'saml20-sp', 'isTrustedProxy', 'bool'], ['display_unconnected_idps_wayf', 'saml20-sp', 'displayUnconnectedIdpsWayf', 'bool'], + ['wayf_remember_choice', 'saml20-sp', 'wayfRememberChoice', 'bool'], ['eula', 'saml20-sp', 'termsOfServiceUrl', 'string'], ['do_not_add_attribute_aliases', 'saml20-sp', 'skipDenormalization', 'bool'], ['policy_enforcement_decision_required', 'saml20-sp', 'policyEnforcementDecisionRequired', 'bool'], @@ -417,6 +454,19 @@ private function validCoinValuesBool() ]; } + private function validCoinValuesBoolStrictTrueOnly() + { + return [ + [null, false], + [true, true], + [false, false], + ["1", true], + ["0", false], + ["false", false], + ["-1", false], + ]; + } + /** * The first option is the manage coin value, the second is the expected entity coin value after assembling * @return array diff --git a/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php b/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php index 5cc8850343..cde13ca8c4 100644 --- a/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php +++ b/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php @@ -22,8 +22,11 @@ use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider; use OpenConext\EngineBlock\Request\CorrelationIdServiceInterface; use OpenConext\EngineBlock\Request\CurrentCorrelationId; +use OpenConext\EngineBlock\Service\CookieService; use OpenConext\EngineBlock\Service\FeedbackInfoCollectorInterface; use OpenConext\EngineBlock\Service\FeedbackStateHelperInterface; +use OpenConext\EngineBlock\Service\TimeProvider\TimeProvider; +use OpenConext\EngineBlock\Service\Wayf\RememberedIdpCookie; use OpenConext\EngineBlockBridge\Logger\LoginLogger; use OpenConext\EngineBlockBundle\Bridge\DiContainerRuntime; use OpenConext\EngineBlockBundle\Service\WayfRenderer; @@ -73,6 +76,15 @@ public function setUp(): void $this->createStub(FeedbackStateHelperInterface::class), $this->createStub(FeedbackInfoCollectorInterface::class), $this->createStub(LoginLogger::class), + new RememberedIdpCookie( + $this->createStub(TimeProvider::class), + $this->createStub(CookieService::class), + 7776000, + 16, + 'engine.example.org', + '/', + true, + ), )); $this->bindings = new EngineBlock_Corto_Module_Bindings($this->proxyServer); diff --git a/tests/library/EngineBlock/Test/Corto/Module/Service/ContinueToIdpTest.php b/tests/library/EngineBlock/Test/Corto/Module/Service/ContinueToIdpTest.php new file mode 100644 index 0000000000..1ee2772367 --- /dev/null +++ b/tests/library/EngineBlock/Test/Corto/Module/Service/ContinueToIdpTest.php @@ -0,0 +1,143 @@ +originalDiContainer = $application->getDiContainer(); + + try { + $this->originalDiContainerRuntime = $application->getDiContainerRuntime(); + } catch (RuntimeException) { + $this->originalDiContainerRuntime = null; + } + } + + protected function tearDown(): void + { + $application = EngineBlock_ApplicationSingleton::getInstance(); + $this->setDiContainer($application, $this->originalDiContainer); + + if ($this->originalDiContainerRuntime !== null) { + $application->setDiContainerRuntime($this->originalDiContainerRuntime); + } + } + + #[DataProvider('rememberChoiceGateProvider')] + public function testPersistRememberedChoiceOnlyWritesTheCookieWhenTheGateIsFullyEnabled( + bool $globalRememberChoiceEnabled, + bool $rememberChoicePerIdpEnabled, + bool $spWayfRememberChoice, + bool $expectCookieWritten + ): void { + $application = EngineBlock_ApplicationSingleton::getInstance(); + + $diContainer = Phake::mock(EngineBlock_Application_DiContainer::class); + Phake::when($diContainer)->getRememberChoice()->thenReturn($globalRememberChoiceEnabled); + Phake::when($diContainer)->getSymfonyRequest()->thenReturn(Request::create('/')); + $this->setDiContainer($application, $diContainer); + + $cookieService = Phake::mock(CookieService::class); + Phake::when($cookieService)->setCookieWithSameSite(Phake::anyParameters())->thenReturn(true); + + $rememberedIdpCookie = new RememberedIdpCookie( + new TimeProvider(), + $cookieService, + 7776000, + 16, + 'engine.example.org', + '/', + true, + ); + + $application->setDiContainerRuntime(new DiContainerRuntime( + $this->createStub(Environment::class), + $this->createStub(WayfRenderer::class), + $this->createStub(CorrelationIdServiceInterface::class), + new CurrentCorrelationId(), + $this->createStub(FeedbackStateHelperInterface::class), + $this->createStub(FeedbackInfoCollectorInterface::class), + $this->createStub(LoginLogger::class), + $rememberedIdpCookie, + $rememberChoicePerIdpEnabled, + )); + + $proxyServerMock = Phake::mock(EngineBlock_Corto_ProxyServer::class); + Phake::when($proxyServerMock)->getLogger()->thenReturn(Phake::mock(LoggerInterface::class)); + + $service = new EngineBlock_Corto_Module_Service_ContinueToIdp( + $proxyServerMock, + Phake::mock(EngineBlock_Corto_XmlToArray::class), + $this->createStub(Environment::class), + Phake::mock(ServiceProviderFactory::class), + ); + + $sp = new ServiceProvider('https://sp.example.org', wayfRememberChoice: $spWayfRememberChoice); + $idp = new IdentityProvider('https://idp.example.org'); + + $method = new ReflectionMethod($service, '_persistRememberedChoice'); + $method->invoke($service, $sp, $idp, '1'); + + if ($expectCookieWritten) { + Phake::verify($cookieService)->setCookieWithSameSite(Phake::anyParameters()); + } else { + Phake::verifyNoInteraction($cookieService); + } + } + + public static function rememberChoiceGateProvider(): array + { + return [ + 'all enabled: cookie is written' => [true, true, true, true], + 'global remember choice disabled: not written' => [false, true, true, false], + 'per-idp feature disabled: not written' => [true, false, true, false], + 'sp opted out: not written' => [true, true, false, false], + 'all disabled: not written' => [false, false, false, false], + ]; + } + + private function setDiContainer(EngineBlock_ApplicationSingleton $application, $diContainer): void + { + $property = new ReflectionProperty(EngineBlock_ApplicationSingleton::class, '_diContainer'); + $property->setValue($application, $diContainer); + } +} diff --git a/tests/unit/OpenConext/EngineBlock/Metadata/Entity/ServiceProviderTest.php b/tests/unit/OpenConext/EngineBlock/Metadata/Entity/ServiceProviderTest.php index 73f5398627..517b3ada83 100644 --- a/tests/unit/OpenConext/EngineBlock/Metadata/Entity/ServiceProviderTest.php +++ b/tests/unit/OpenConext/EngineBlock/Metadata/Entity/ServiceProviderTest.php @@ -39,6 +39,13 @@ public function testInstantiation() $this->assertEquals($entityId, $sp->entityId); } + public function testWayfRememberChoiceIsDisabledByDefault(): void + { + $serviceProvider = new ServiceProvider('https://sp.example.org'); + + self::assertFalse($serviceProvider->getCoins()->wayfRememberChoice()); + } + /** * Expected behavior for display name retrieval is: * 1. display name in preferred locale diff --git a/tests/unit/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookieTest.php b/tests/unit/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookieTest.php new file mode 100644 index 0000000000..ad8cbb03d0 --- /dev/null +++ b/tests/unit/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookieTest.php @@ -0,0 +1,420 @@ +assertSame( + $expected, + RememberedIdpCookie::isEnabledForServiceProvider( + $globalRememberChoiceEnabled, + $rememberChoicePerIdpEnabled, + $serviceProvider + ) + ); + } + + public static function isEnabledForServiceProviderProvider(): array + { + return [ + 'all enabled' => [true, true, true, true], + 'global disabled' => [false, true, true, false], + 'per-idp feature disabled' => [true, false, true, false], + 'sp opted out' => [true, true, false, false], + 'all disabled' => [false, false, false, false], + ]; + } + + public function testValidEntryRoundTripsThroughCookieCodec(): void + { + $cookie = $this->buildCookie(); + + /** @var array $entries */ + $entries = [ + 'https://sp.example.org' => [ + 'idp' => 'https://idp.example.org', + 'expires' => 1700003600, + ], + ]; + + $encoded = $cookie->encode($entries); + $result = $cookie->normalize($encoded); + + $this->assertNotSame(json_encode($entries), $encoded); + $this->assertSame($entries, $result['entries']); + $this->assertFalse($result['invalid']); + $this->assertSame(0, $result['expired']); + $this->assertFalse($result['changed']); + } + + #[DataProvider('invalidCookieProvider')] + public function testInvalidCookieInputReturnsInvalidResultWithoutThrowing(string $raw): void + { + try { + $result = $this->buildCookie()->normalize($raw); + } catch (\Throwable $exception) { + $this->fail(sprintf('Invalid cookie input should not throw: %s', $exception->getMessage())); + } + + $this->assertTrue($result['invalid']); + $this->assertSame([], $result['entries']); + } + + public static function invalidCookieProvider(): array + { + return [ + 'invalid base64' => ['not valid base64!'], + 'invalid deflate data' => [base64_encode('not deflated')], + 'invalid JSON' => [self::rawJson('{not json')], + 'non-array root' => [self::rawJson('"not an array"')], + 'empty service provider entity id' => [self::rawJson('{"":{"idp":"https://idp.example.org","expires":1700003600}}')], + 'non-array entry' => [self::rawJson('{"https://sp.example.org":"not an entry"}')], + 'missing idp' => [self::rawJson('{"https://sp.example.org":{"expires":1700003600}}')], + 'non-string idp' => [self::rawJson('{"https://sp.example.org":{"idp":123,"expires":1700003600}}')], + 'empty idp' => [self::rawJson('{"https://sp.example.org":{"idp":"","expires":1700003600}}')], + 'missing expires' => [self::rawJson('{"https://sp.example.org":{"idp":"https://idp.example.org"}}')], + 'non-integer expires' => [self::rawJson('{"https://sp.example.org":{"idp":"https://idp.example.org","expires":"1700003600"}}')], + 'extra entry key' => [self::rawJson('{"https://sp.example.org":{"idp":"https://idp.example.org","expires":1700003600,"extra":"unexpected"}}')], + ]; + } + + public function testExpiredEntriesAreRemovedWhenNormalizing(): void + { + $raw = self::rawEntries([ + 'https://expired.example.org' => ['idp' => 'https://idp1.example.org', 'expires' => self::NOW - 1], + 'https://expires-now.example.org' => ['idp' => 'https://idp2.example.org', 'expires' => self::NOW], + 'https://valid.example.org' => ['idp' => 'https://idp3.example.org', 'expires' => self::NOW + 1], + ]); + + $result = $this->buildCookie()->normalize($raw); + + $this->assertSame( + ['https://valid.example.org' => ['idp' => 'https://idp3.example.org', 'expires' => self::NOW + 1]], + $result['entries'] + ); + $this->assertFalse($result['invalid']); + $this->assertSame(2, $result['expired']); + $this->assertTrue($result['changed']); + } + + public function testOnlyAddPrunesOverLimitEntriesByNewestExpiry(): void + { + $cookie = $this->buildCookie(); + $entries = $this->validEntries(17); + + $normalized = $cookie->normalize(self::rawEntries($entries)); + $this->assertCount(17, $normalized['entries']); + $this->assertFalse($normalized['changed']); + + $result = $cookie->add($entries, 'https://sp-new.example.org', 'https://idp-new.example.org'); + + $this->assertCount(self::MAX_ENTRIES, $result['entries']); + $this->assertSame(2, $result['dropped']); + $this->assertArrayNotHasKey('https://sp-01.example.org', $result['entries']); + $this->assertArrayNotHasKey('https://sp-02.example.org', $result['entries']); + $this->assertArrayHasKey('https://sp-17.example.org', $result['entries']); + $this->assertSame( + ['idp' => 'https://idp-new.example.org', 'expires' => self::NOW + self::LIFETIME], + $result['entries']['https://sp-new.example.org'] + ); + } + + public function testFindOnlyReturnsUnexpiredCandidateForServiceProvider(): void + { + $cookie = $this->buildCookie(); + $entries = [ + 'https://sp.example.org' => ['idp' => 'https://idp.example.org', 'expires' => self::NOW + 1], + 'https://expired.example.org' => ['idp' => 'https://expired-idp.example.org', 'expires' => self::NOW], + ]; + + $this->assertSame( + 'https://idp.example.org', + $cookie->find($entries, 'https://sp.example.org', ['https://idp.example.org']) + ); + $this->assertNull($cookie->find($entries, 'https://unknown-sp.example.org', ['https://idp.example.org'])); + $this->assertNull($cookie->find($entries, 'https://expired.example.org', ['https://expired-idp.example.org'])); + $this->assertNull($cookie->find($entries, 'https://sp.example.org', ['https://other-idp.example.org'])); + } + + public function testAddReplacesExistingSpEntryAndReportsDroppedEntries(): void + { + $cookie = $this->buildCookie(maxEntries: 2); + $entries = [ + 'https://sp.example.org' => ['idp' => 'https://old-idp.example.org', 'expires' => self::NOW + 10], + 'https://sp-keep.example.org' => ['idp' => 'https://idp-keep.example.org', 'expires' => self::NOW + 9], + 'https://sp-drop.example.org' => ['idp' => 'https://idp-drop.example.org', 'expires' => self::NOW + 1], + ]; + + $result = $cookie->add($entries, 'https://sp.example.org', 'https://new-idp.example.org'); + + $this->assertSame(self::NOW + self::LIFETIME, $result['expires']); + $this->assertSame(1, $result['dropped']); + $this->assertSame( + ['idp' => 'https://new-idp.example.org', 'expires' => self::NOW + self::LIFETIME], + $result['entries']['https://sp.example.org'] + ); + $this->assertArrayHasKey('https://sp-keep.example.org', $result['entries']); + $this->assertArrayNotHasKey('https://sp-drop.example.org', $result['entries']); + } + + public function testEmptyNormalizedResultIndicatesCookieShouldBeCleared(): void + { + $result = $this->buildCookie()->normalize(self::rawEntries([ + 'https://expired.example.org' => ['idp' => 'https://idp.example.org', 'expires' => self::NOW], + ])); + + $this->assertSame([], $result['entries']); + $this->assertFalse($result['invalid']); + $this->assertSame(1, $result['expired']); + $this->assertTrue($result['changed']); + } + + public function testWriteAndClearUseSameSiteAwareCookieOperations(): void + { + $cookieService = Phake::mock(CookieService::class); + Phake::when($cookieService)->setCookieWithSameSite(Phake::anyParameters())->thenReturn(true); + Phake::when($cookieService)->clearCookieWithSameSite(Phake::anyParameters())->thenReturn(true); + + $cookie = $this->buildCookie(cookieService: $cookieService); + $entries = [ + 'https://sp.example.org' => ['idp' => 'https://idp.example.org', 'expires' => self::NOW + self::LIFETIME], + ]; + $encoded = $cookie->encode($entries); + + $this->assertSame(self::NOW + self::LIFETIME, $cookie->cookieExpiresAt()); + $this->assertTrue($cookie->write($entries)); + $this->assertTrue($cookie->clear()); + + Phake::verify($cookieService)->setCookieWithSameSite( + RememberedIdpCookie::NAME, + $encoded, + self::NOW + self::LIFETIME, + self::COOKIE_PATH, + self::COOKIE_DOMAIN, + true, + true, + 'None' + ); + Phake::verify($cookieService)->clearCookieWithSameSite( + RememberedIdpCookie::NAME, + self::COOKIE_PATH, + self::COOKIE_DOMAIN, + true, + true, + 'None' + ); + } + + public function testWriteUsesExplicitlyProvidedExpiryWhenGiven(): void + { + $cookieService = Phake::mock(CookieService::class); + Phake::when($cookieService)->setCookieWithSameSite(Phake::anyParameters())->thenReturn(true); + + $cookie = $this->buildCookie(cookieService: $cookieService); + $entries = [ + 'https://sp.example.org' => ['idp' => 'https://idp.example.org', 'expires' => self::NOW + self::LIFETIME], + ]; + $explicitExpiry = self::NOW + self::LIFETIME + 1234; + + $this->assertTrue($cookie->write($entries, $explicitExpiry)); + + Phake::verify($cookieService)->setCookieWithSameSite( + RememberedIdpCookie::NAME, + $cookie->encode($entries), + $explicitExpiry, + self::COOKIE_PATH, + self::COOKIE_DOMAIN, + true, + true, + 'None' + ); + } + + public function testLoadValidEntriesLogsWarningAndClearsCookieOnInvalidInput(): void + { + $cookieService = Phake::mock(CookieService::class); + Phake::when($cookieService)->clearCookieWithSameSite(Phake::anyParameters())->thenReturn(true); + $logger = Phake::mock(LoggerInterface::class); + + $cookie = $this->buildCookie(cookieService: $cookieService); + + $entries = $cookie->loadValidEntries('not valid base64!', $logger); + + $this->assertSame([], $entries); + Phake::verify($logger)->warning('Invalid rememberedidps cookie encountered, clearing it'); + Phake::verify($cookieService)->clearCookieWithSameSite( + RememberedIdpCookie::NAME, + self::COOKIE_PATH, + self::COOKIE_DOMAIN, + true, + true, + 'None' + ); + Phake::verifyNoFurtherInteraction($cookieService); + } + + public function testLoadValidEntriesWritesCookieWhenNormalizationChangedEntries(): void + { + $cookieService = Phake::mock(CookieService::class); + Phake::when($cookieService)->setCookieWithSameSite(Phake::anyParameters())->thenReturn(true); + $logger = Phake::mock(LoggerInterface::class); + + $cookie = $this->buildCookie(cookieService: $cookieService); + $raw = self::rawEntries([ + 'https://expired.example.org' => ['idp' => 'https://idp1.example.org', 'expires' => self::NOW - 1], + 'https://valid.example.org' => ['idp' => 'https://idp2.example.org', 'expires' => self::NOW + 1], + ]); + + $entries = $cookie->loadValidEntries($raw, $logger); + + $this->assertSame( + ['https://valid.example.org' => ['idp' => 'https://idp2.example.org', 'expires' => self::NOW + 1]], + $entries + ); + Phake::verify($cookieService)->setCookieWithSameSite( + RememberedIdpCookie::NAME, + $cookie->encode($entries), + self::NOW + self::LIFETIME, + self::COOKIE_PATH, + self::COOKIE_DOMAIN, + true, + true, + 'None' + ); + Phake::verifyNoInteraction($logger); + } + + public function testLoadValidEntriesClearsCookieWhenNormalizationChangedEntriesToEmpty(): void + { + $cookieService = Phake::mock(CookieService::class); + Phake::when($cookieService)->clearCookieWithSameSite(Phake::anyParameters())->thenReturn(true); + $logger = Phake::mock(LoggerInterface::class); + + $cookie = $this->buildCookie(cookieService: $cookieService); + $raw = self::rawEntries([ + 'https://expired.example.org' => ['idp' => 'https://idp1.example.org', 'expires' => self::NOW], + ]); + + $entries = $cookie->loadValidEntries($raw, $logger); + + $this->assertSame([], $entries); + Phake::verify($cookieService)->clearCookieWithSameSite( + RememberedIdpCookie::NAME, + self::COOKIE_PATH, + self::COOKIE_DOMAIN, + true, + true, + 'None' + ); + Phake::verifyNoInteraction($logger); + } + + public function testLoadValidEntriesDoesNotWriteOrClearWhenUnchanged(): void + { + $cookieService = Phake::mock(CookieService::class); + $logger = Phake::mock(LoggerInterface::class); + + $cookie = $this->buildCookie(cookieService: $cookieService); + $entries = [ + 'https://sp.example.org' => ['idp' => 'https://idp.example.org', 'expires' => self::NOW + 1], + ]; + $raw = self::rawEntries($entries); + + $result = $cookie->loadValidEntries($raw, $logger); + + $this->assertSame($entries, $result); + Phake::verifyNoInteraction($cookieService); + Phake::verifyNoInteraction($logger); + } + + private function buildCookie(?CookieService $cookieService = null, int $maxEntries = self::MAX_ENTRIES): RememberedIdpCookie + { + return new RememberedIdpCookie( + $this->fixedTimeProvider(), + $cookieService ?? Phake::mock(CookieService::class), + self::LIFETIME, + $maxEntries, + self::COOKIE_DOMAIN, + self::COOKIE_PATH, + true, + ); + } + + private function fixedTimeProvider(): TimeProvider + { + return new class extends TimeProvider { + public function time() + { + return RememberedIdpCookieTest::NOW; + } + }; + } + + private static function rawEntries(array $entries): string + { + return self::rawJson((string) json_encode($entries)); + } + + private static function rawJson(string $json): string + { + return base64_encode((string) gzdeflate($json)); + } + + private function validEntries(int $count): array + { + $entries = []; + for ($i = 1; $i <= $count; $i++) { + $entries[sprintf('https://sp-%02d.example.org', $i)] = [ + 'idp' => sprintf('https://idp-%02d.example.org', $i), + 'expires' => self::NOW + $i, + ]; + } + + return $entries; + } +} diff --git a/tests/unit/OpenConext/EngineBlockBundle/DiContainerRuntimeTest.php b/tests/unit/OpenConext/EngineBlockBundle/DiContainerRuntimeTest.php index 13d9f5f314..bb93b5af78 100644 --- a/tests/unit/OpenConext/EngineBlockBundle/DiContainerRuntimeTest.php +++ b/tests/unit/OpenConext/EngineBlockBundle/DiContainerRuntimeTest.php @@ -20,8 +20,11 @@ use OpenConext\EngineBlock\Request\CorrelationIdServiceInterface; use OpenConext\EngineBlock\Request\CurrentCorrelationId; +use OpenConext\EngineBlock\Service\CookieService; use OpenConext\EngineBlock\Service\FeedbackInfoCollectorInterface; use OpenConext\EngineBlock\Service\FeedbackStateHelperInterface; +use OpenConext\EngineBlock\Service\TimeProvider\TimeProvider; +use OpenConext\EngineBlock\Service\Wayf\RememberedIdpCookie; use OpenConext\EngineBlockBridge\Logger\LoginLogger; use OpenConext\EngineBlockBundle\Bridge\DiContainerRuntime; use OpenConext\EngineBlockBundle\Service\WayfRenderer; @@ -46,11 +49,37 @@ public function testGetPreferredIdpEntityIdsReturnsConfiguredList(): void $this->assertSame($entityIds, $runtime->getPreferredIdpEntityIds()); } + public function testExposesRememberedIdpCookie(): void + { + $rememberedIdpCookie = $this->rememberedIdpCookieStub(); + + $runtime = $this->runtimeFactory(rememberedIdpCookie: $rememberedIdpCookie); + + $this->assertSame($rememberedIdpCookie, $runtime->rememberedIdpCookie); + } + + public function testIsRememberChoicePerIdpEnabledDefaultsToFalse(): void + { + $runtime = $this->runtimeFactory(); + + $this->assertFalse($runtime->isRememberChoicePerIdpEnabled()); + } + + public function testIsRememberChoicePerIdpEnabledReturnsConfiguredValue(): void + { + $runtime = $this->runtimeFactory(rememberChoicePerIdp: true); + + $this->assertTrue($runtime->isRememberChoicePerIdpEnabled()); + } + /** * @param array $entityIds */ - private function runtimeFactory(array $entityIds): DiContainerRuntime - { + private function runtimeFactory( + array $entityIds = [], + ?RememberedIdpCookie $rememberedIdpCookie = null, + bool $rememberChoicePerIdp = false, + ): DiContainerRuntime { return new DiContainerRuntime( $this->createStub(Environment::class), $this->createStub(WayfRenderer::class), @@ -59,7 +88,22 @@ private function runtimeFactory(array $entityIds): DiContainerRuntime $this->createStub(FeedbackStateHelperInterface::class), $this->createStub(FeedbackInfoCollectorInterface::class), $this->createStub(LoginLogger::class), + $rememberedIdpCookie ?? $this->rememberedIdpCookieStub(), + $rememberChoicePerIdp, $entityIds, ); } + + private function rememberedIdpCookieStub(): RememberedIdpCookie + { + return new RememberedIdpCookie( + $this->createStub(TimeProvider::class), + $this->createStub(CookieService::class), + 7776000, + 16, + 'engine.example.org', + '/', + true, + ); + } } diff --git a/tests/unit/OpenConext/EngineBlockBundle/Service/WayfRendererTest.php b/tests/unit/OpenConext/EngineBlockBundle/Service/WayfRendererTest.php index abef75c547..80c9c1fc59 100644 --- a/tests/unit/OpenConext/EngineBlockBundle/Service/WayfRendererTest.php +++ b/tests/unit/OpenConext/EngineBlockBundle/Service/WayfRendererTest.php @@ -77,6 +77,77 @@ private function buildViewModel(bool $showIdPBanner = false, bool $showPreferred ); } + public function testRememberChoicePerIdpDefaultsToFalseForExistingCallers(): void + { + $capturedRememberChoicePerIdp = 'not-set'; + + $this->factory->expects($this->once()) + ->method('create') + ->willReturnCallback(function () use (&$capturedRememberChoicePerIdp): WayfViewModel { + $namedArgs = func_get_args(); + $capturedRememberChoicePerIdp = $namedArgs[14] ?? 'missing'; + return $this->buildViewModel(); + }); + + $this->twig->method('render')->willReturn(''); + + $sp = $this->createStub(ServiceProvider::class); + $sp->method('getDisplayName')->willReturn('SP'); + + $this->renderer()->render( + idpList: [], + preferredIdpEntityIds: [], + action: '/sso', + currentLocale: 'en', + defaultIdpEntityId: '', + shouldDisplayBanner: false, + backLink: false, + cutoffPoint: 100, + rememberChoice: false, + showRequestAccess: false, + requestId: 'req-1', + serviceProvider: $sp, + ); + + $this->assertFalse($capturedRememberChoicePerIdp); + } + + public function testRememberChoicePerIdpIsPassedThroughToFactory(): void + { + $capturedRememberChoicePerIdp = 'not-set'; + + $this->factory->expects($this->once()) + ->method('create') + ->willReturnCallback(function () use (&$capturedRememberChoicePerIdp): WayfViewModel { + $namedArgs = func_get_args(); + $capturedRememberChoicePerIdp = $namedArgs[14] ?? 'missing'; + return $this->buildViewModel(); + }); + + $this->twig->method('render')->willReturn(''); + + $sp = $this->createStub(ServiceProvider::class); + $sp->method('getDisplayName')->willReturn('SP'); + + $this->renderer()->render( + idpList: [], + preferredIdpEntityIds: [], + action: '/sso', + currentLocale: 'en', + defaultIdpEntityId: '', + shouldDisplayBanner: false, + backLink: false, + cutoffPoint: 100, + rememberChoice: true, + showRequestAccess: false, + requestId: 'req-1', + serviceProvider: $sp, + rememberChoicePerIdp: true, + ); + + $this->assertTrue($capturedRememberChoicePerIdp); + } + #[DataProvider('bannerConditionProvider')] public function testBannerConditionPassedToFactory( bool $shouldDisplayBanner, diff --git a/tests/unit/OpenConext/EngineBlockBundle/Twig/Extensions/Extension/WayfTest.php b/tests/unit/OpenConext/EngineBlockBundle/Twig/Extensions/Extension/WayfTest.php index 09abb8cf76..f4a2456580 100644 --- a/tests/unit/OpenConext/EngineBlockBundle/Twig/Extensions/Extension/WayfTest.php +++ b/tests/unit/OpenConext/EngineBlockBundle/Twig/Extensions/Extension/WayfTest.php @@ -235,6 +235,7 @@ public function testGetWayfJsonConfig() $this->assertEquals(5, $config['cutoffPointForShowingUnfilteredIdps']); $this->assertEquals(Wayf::REMEMBER_CHOICE_COOKIE_NAME, $config['rememberChoiceCookieName']); $this->assertTrue($config['rememberChoiceFeature']); + $this->assertFalse($config['rememberChoicePerIdp']); $this->assertFalse($config['hideBookmarkableUrl']); $this->assertEquals('More results', $config['messages']['moreIdpResults']); $this->assertEquals('Request Access', $config['messages']['requestAccess']); @@ -254,6 +255,34 @@ public function testGetWayfJsonConfig() $this->assertEmpty($config['unconnectedIdps']); } + public function testGetWayfJsonConfigIncludesRememberChoicePerIdpWhenEnabled() + { + $connectedIdps = $this->createMock(ConnectedIdps::class); + $connectedIdps->method('getFormattedPreviousSelectionList')->willReturn([]); + $connectedIdps->method('getConnectedIdps')->willReturn([]); + $connectedIdps->method('getFormattedIdpList')->willReturn([]); + + $serviceProvider = $this->createMock(ServiceProvider::class); + $serviceProvider->entityId = 'https://sp.example.org'; + $serviceProvider->method('getDisplayName')->willReturn('Test SP'); + + $this->translator->method('trans')->willReturn(''); + + $jsonConfig = $this->wayf->getWayfJsonConfig( + $connectedIdps, + $serviceProvider, + 'en', + false, + true, + 5, + true + ); + + $config = json_decode($jsonConfig, true); + $this->assertTrue($config['rememberChoiceFeature']); + $this->assertTrue($config['rememberChoicePerIdp']); + } + public function testGetWayfJsonConfigIncludesHideBookmarkableUrlWhenFlagEnabled() { $featureConfiguration = $this->createMock(FeatureConfigurationInterface::class); diff --git a/tests/unit/OpenConext/EngineBlockBundle/Twig/WayfScriptConfigRenderTest.php b/tests/unit/OpenConext/EngineBlockBundle/Twig/WayfScriptConfigRenderTest.php new file mode 100644 index 0000000000..03349c283d --- /dev/null +++ b/tests/unit/OpenConext/EngineBlockBundle/Twig/WayfScriptConfigRenderTest.php @@ -0,0 +1,142 @@ +renderWayfConfig(true); + + $this->assertArrayHasKey('rememberChoicePerIdp', $config); + $this->assertTrue($config['rememberChoicePerIdp']); + } + + public function testRememberChoicePerIdpFalseReachesGeneratedScriptConfig(): void + { + $config = $this->renderWayfConfig(false); + + $this->assertArrayHasKey('rememberChoicePerIdp', $config); + $this->assertFalse($config['rememberChoicePerIdp']); + } + + private function renderWayfConfig(bool $rememberChoicePerIdp): array + { + $environment = $this->buildEnvironment(); + + $html = $environment->render('test/wayf_wrapper.html.twig', [ + 'serviceProvider' => new ServiceProvider('https://sp.example.org', displayNameEn: 'Test SP'), + 'connectedIdps' => new ConnectedIdps([], []), + 'showRequestAccess' => false, + 'rememberChoiceFeature' => true, + 'cutoffPointForShowingUnfilteredIdps' => 5, + 'rememberChoicePerIdp' => $rememberChoicePerIdp, + 'helpLink' => '/authentication/idp/help-discover', + 'displayLanguageSwitcher' => false, + ]); + + return $this->extractWayfConfig($html); + } + + private function buildEnvironment(): Environment + { + $basePath = realpath(__DIR__ . '/../../../../../'); + + $filesystemLoader = new FilesystemLoader(); + $filesystemLoader->addPath($basePath . '/theme/base/templates/modules', 'theme'); + $filesystemLoader->addPath($basePath . '/theme/base/templates/layouts', 'themeLayouts'); + + $arrayLoader = new ArrayLoader([ + 'test/wayf_wrapper.html.twig' => <<<'TWIG' +{% extends '@theme/Authentication/View/Proxy/wayf.html.twig' %} +{% block wayf_content %}{% endblock %} +TWIG, + ]); + + $environment = new Environment(new ChainLoader([$arrayLoader, $filesystemLoader])); + + $translator = m::mock(TranslatorInterface::class); + $translator->shouldReceive('trans')->andReturnUsing(static function ($id) { + // Needs to be a valid strip_tags() allowlist since warning.html.twig applies it to + // `allowedHtml|trans`; any other key is simply echoed back. + return $id === 'consent_warning_allowed_html' ? '

' : (string) $id; + }); + $environment->addExtension(new TranslationExtension($translator)); + $environment->addExtension(new AttributeExtension(Spaceless::class)); + + // These are only reachable when `displayLanguageSwitcher` is true, but Twig resolves + // function calls at compile time, so footer.html.twig (and the language-switcher macro + // file it imports) still needs them registered even though this test keeps the switcher + // hidden via `displayLanguageSwitcher: false`. + $environment->addFunction(new TwigFunction('locale', static fn (): string => 'en')); + $environment->addFunction(new TwigFunction('postData', static fn (): array => [])); + $environment->addFunction(new TwigFunction('supportedLocales', static fn (): array => ['en'])); + $environment->addFunction(new TwigFunction('queryStringFor', static fn (): string => '')); + + $wayf = new Wayf( + $this->createStub(RequestStack::class), + $translator, + $this->buildFeatureConfiguration(), + ); + $environment->addFunction(new TwigFunction('wayfConfig', $wayf->getWayfJsonConfig(...))); + + return $environment; + } + + private function buildFeatureConfiguration(): FeatureConfigurationInterface + { + $featureConfiguration = $this->createStub(FeatureConfigurationInterface::class); + $featureConfiguration->method('isEnabled')->willReturn(false); + + return $featureConfiguration; + } + + private function extractWayfConfig(string $html): array + { + $this->assertMatchesRegularExpression( + '##s', + $html, + ); + preg_match('##s', $html, $matches); + + return json_decode($matches[1], true, 512, JSON_THROW_ON_ERROR); + } +} diff --git a/theme/base/javascripts/wayf/rememberChoice.js b/theme/base/javascripts/wayf/rememberChoice.js index d28e4fdb92..4057c085b7 100644 --- a/theme/base/javascripts/wayf/rememberChoice.js +++ b/theme/base/javascripts/wayf/rememberChoice.js @@ -1,11 +1,23 @@ import {setCookie} from '../utility/setCookie'; -import {configurationId, rememberChoiceId} from '../selectors'; +import {configurationId, rememberChoiceId, idpFormSelector} from '../selectors'; -export const rememberChoice = (entityId) => { +export const rememberChoice = (element, entityId) => { const checkbox = document.getElementById(rememberChoiceId); if (!checkbox) return; if (!checkbox.checked) return; - const cookieName = JSON.parse(document.getElementById(configurationId).innerHTML).rememberChoiceCookieName; - setCookie(entityId, cookieName, 365, '/', true); + const config = JSON.parse(document.getElementById(configurationId).innerHTML); + + if (config.rememberChoicePerIdp) { + const form = element.querySelector(idpFormSelector); + if (!form) return; + + const rememberChoiceInput = form.elements['rememberChoice']; + if (!rememberChoiceInput) return; + + rememberChoiceInput.value = '1'; + return; + } + + setCookie(entityId, config.rememberChoiceCookieName, 365, '/', true); }; diff --git a/theme/base/javascripts/wayf/submitForm.js b/theme/base/javascripts/wayf/submitForm.js index 64edb368fc..0d1e6e3c14 100644 --- a/theme/base/javascripts/wayf/submitForm.js +++ b/theme/base/javascripts/wayf/submitForm.js @@ -30,7 +30,7 @@ export const submitForm = (e) => { } const entityId = element.getAttribute('data-entityid'); - rememberChoice(entityId); + rememberChoice(element, entityId); selectAndSubmit(element); }; diff --git a/theme/base/templates/modules/Authentication/View/Proxy/Partials/WAYF/idp/idpForm.html.twig b/theme/base/templates/modules/Authentication/View/Proxy/Partials/WAYF/idp/idpForm.html.twig index 2eb57a719f..645e1f6a3d 100644 --- a/theme/base/templates/modules/Authentication/View/Proxy/Partials/WAYF/idp/idpForm.html.twig +++ b/theme/base/templates/modules/Authentication/View/Proxy/Partials/WAYF/idp/idpForm.html.twig @@ -2,6 +2,7 @@ + {% include '@theme/Authentication/View/Proxy/Partials/WAYF/idp/idpSubmitButton.html.twig' with { hidden: true } only %}