diff --git a/CHANGELOG.md b/CHANGELOG.md index 77704990b5..0e414bd49b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,17 @@ on the EngineBlock wiki. ## Unreleased +Features: + +* Added a per-SP "remember my choice" mode for the WAYF, enabled with + `feature_enable_wayf_remember_choice_per_idp` and the SP metadata coin `coin:wayf_remember_choice`. See + `docs/wayf_remember_choice.md`. +* The per-SP mode and the existing global mode (`wayf.remember_choice`) are mutually exclusive. EngineBlock refuses to + build its container (for example during `cache:clear` or `cache:warmup`) when both are `true`. Enable only one. +* The per-SP mode does not require `wayf.remember_choice`. Leave `wayf.remember_choice` set to `false` when enabling it. +* The cookie removal page `/authentication/idp/remove-cookies` is now also available in the per-SP mode and lists the + `rememberedidps` cookie. Before, it was only available when `wayf.remember_choice` was `true`. + Updated: * Updated Composer and Yarn (theme and Cypress) dependencies within their current version constraints to resolve open Dependabot alerts. (#2121) diff --git a/composer.json b/composer.json index 1dbb587bab..fc1162ebbd 100644 --- a/composer.json +++ b/composer.json @@ -22,6 +22,7 @@ "ext-mbstring": "*", "ext-openssl": "*", "ext-pdo": "*", + "ext-zlib": "*", "beberlei/assert": "^3.3.3", "composer/package-versions-deprecated": "^1.11", "doctrine/dbal": "^4.3.3", @@ -54,7 +55,6 @@ "twig/twig": "^3.22.0" }, "require-dev": { - "ext-zlib": "*", "behat/behat": "^3.26", "behat/mink": "^1.12", "behat/mink-browserkit-driver": "^2.2", diff --git a/composer.lock b/composer.lock index addc188264..089d9bc674 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "eac6cb4224142cc12bca38fd15b97b40", + "content-hash": "39389aeede70165ef58f524f94cedc13", "packages": [ { "name": "beberlei/assert", @@ -12245,11 +12245,10 @@ "ext-json": "*", "ext-mbstring": "*", "ext-openssl": "*", - "ext-pdo": "*" - }, - "platform-dev": { + "ext-pdo": "*", "ext-zlib": "*" }, + "platform-dev": {}, "platform-overrides": { "php": "8.5" }, diff --git a/config/packages/engineblock_features.yaml b/config/packages/engineblock_features.yaml index 2fbf8dbeda..3caad22fec 100644 --- a/config/packages/engineblock_features.yaml +++ b/config/packages/engineblock_features.yaml @@ -17,3 +17,4 @@ parameters: eb.stepup.send_user_attributes: "%feature_stepup_send_user_attributes%" eb.feature_enable_sram_interrupt: "%feature_enable_sram_interrupt%" eb.hide_bookmarkable_url: "%feature_hide_bookmarkable_url%" + eb.feature_enable_wayf_remember_choice_per_idp: "%feature_enable_wayf_remember_choice_per_idp%" diff --git a/config/packages/parameters.yml.dist b/config/packages/parameters.yml.dist index da4509d200..cd1431b0ff 100644 --- a/config/packages/parameters.yml.dist +++ b/config/packages/parameters.yml.dist @@ -179,7 +179,16 @@ parameters: wayf.cutoff_point_for_showing_unfiltered_idps: 50 ## Allow users to save their selected IdP and then auto-select it on returning visits. + ## This is the global mode: one remembered IdP for all SPs, stored in a client-side cookie. + ## Mutually exclusive with feature_enable_wayf_remember_choice_per_idp: enabling both fails the + ## container build. See docs/wayf_remember_choice.md. wayf.remember_choice: false + ## Lifetime for remembered IdP choices scoped per SP, in seconds (7776000 seconds = 90 days). + wayf.remember_choice_per_idp_lifetime: 7776000 + ## Maximum number of remembered IdP choice entries to retain per SP. Once exceeded, the + ## oldest-expiring entries are evicted first; the newly added choice is always kept and no + ## error is raised (see RememberedIdpCookie::pruneOverLimit()). + 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 @@ -235,6 +244,10 @@ parameters: feature_stepup_send_user_attributes: false feature_enable_sram_interrupt: false feature_hide_bookmarkable_url: false + ## Allow SPs to opt in to remembered IdP choices scoped per SP through Manage metadata. + ## This is the per-SP mode and replaces the global mode: it does not require wayf.remember_choice. + ## Mutually exclusive with wayf.remember_choice: enabling both fails the container build. + feature_enable_wayf_remember_choice_per_idp: false ########################################################################################## ## PROFILE SETTINGS diff --git a/config/services/services.yml b/config/services/services.yml index f81b521e48..40ce701699 100644 --- a/config/services/services.yml +++ b/config/services/services.yml @@ -15,6 +15,7 @@ services: autowire: true autoconfigure: true arguments: + $featureConfiguration: '@OpenConext\EngineBlockBundle\Configuration\FeatureConfiguration' $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: '' + $cookiePath: '%cookie.path%' + $secure: '%cookie.secure%' + OpenConext\EngineBlock\Metadata\Factory\Factory\ServiceProviderFactory: arguments: - '@engineblock.compat.metadata.definitions' diff --git a/docs/index.md b/docs/index.md index bcb30a928c..e82f0a0a5b 100644 --- a/docs/index.md +++ b/docs/index.md @@ -10,6 +10,7 @@ 1. [PHP testing][php-testing] 1. [Javscript testing][js-testing] 1. [Logging][logging] +1. [WAYF remember my choice](wayf_remember_choice.md) [license]: https://github.com/OpenConext/OpenConext-engineblock/tree/master/docs/LICENSE [release]: https://github.com/OpenConext/OpenConext-engineblock/tree/master/docs/release_procedure.md diff --git a/docs/sso_session_cookie.md b/docs/sso_session_cookie.md index 178afb9919..750b7a4f95 100644 --- a/docs/sso_session_cookie.md +++ b/docs/sso_session_cookie.md @@ -16,8 +16,9 @@ An authentication with the Identity Provider is successful when the SAML respons The value of the cookie contains an array of the entity ID('s) of the successfully authenticated Identity Providers. This cookie will be deleted at `/logout` or will expire according to the set `sso_session_cookie_max_age`. If the SSO Cookie Session feature is enabled it is also possible to manually delete this cookie at -`/authentication/idp/remove-cookies`. Do note however, that the remove cookies endpoint is only available if -`wayf.remember_choice=true` +`/authentication/idp/remove-cookies`. Do note however, that the remove cookies endpoint is only available if one of the +WAYF remember my choice modes is enabled (`wayf.remember_choice` or +`feature_enable_wayf_remember_choice_per_idp`), see [WAYF remember my choice](wayf_remember_choice.md). ## Configuration of SSO Session Cookie in Engineblock diff --git a/docs/wayf_remember_choice.md b/docs/wayf_remember_choice.md new file mode 100644 index 0000000000..cd1a8a0761 --- /dev/null +++ b/docs/wayf_remember_choice.md @@ -0,0 +1,98 @@ +# WAYF remember my choice + +EngineBlock can remember which Identity Provider (IdP) a user picked on the WAYF, so that the user skips the WAYF on a +returning visit. There are two ways to do this. They are **mutually exclusive**: you enable one or the other, never +both. + +| | Global mode | Per-SP mode | +|---|---|---| +| Parameter | `wayf.remember_choice` | `feature_enable_wayf_remember_choice_per_idp` | +| Scope of the remembered choice | One IdP for all SPs | One IdP per SP | +| SP opt-in required | No, applies to all SPs | Yes, SP metadata coin `coin:wayf_remember_choice` | +| Cookie name | `rememberchoice` | `rememberedidps` | +| Written by | Browser (JavaScript) | EngineBlock (server side) | +| Cookie lifetime | 365 days | `wayf.remember_choice_per_idp_lifetime` (default 90 days) | +| Cookie attributes | Secure, SameSite=None | Secure, HttpOnly, SameSite=None, scoped to the EngineBlock host | + +## Choosing a mode + +Enable at most one of the following in `config/packages/parameters.yml`: + + # Global mode + wayf.remember_choice: true + feature_enable_wayf_remember_choice_per_idp: false + +or + + # Per-SP mode + wayf.remember_choice: false + feature_enable_wayf_remember_choice_per_idp: true + +If both are `false`, the WAYF has no remember my choice checkbox and EngineBlock never skips the WAYF based on an +earlier choice. + +### Both modes enabled is an error + +If both parameters are `true`, EngineBlock refuses to build its container and fails with: + + The WAYF remember-my-choice modes are mutually exclusive: "wayf.remember_choice" and + "feature_enable_wayf_remember_choice_per_idp" cannot both be true. Enable only one of them. + +The check runs when the Symfony container is compiled, so a wrong configuration is caught by `cache:clear` or +`cache:warmup` during deployment, before any user is affected. Fix the configuration by setting one of the two +parameters to `false`. + +The per-SP mode does not depend on `wayf.remember_choice`. Earlier development versions required both to be `true`; +that combination is now rejected. + +## Global mode + +With `wayf.remember_choice: true` the WAYF shows a "remember my choice" checkbox. When the user checks it and picks an +IdP, the browser stores the IdP entity ID in the `rememberchoice` cookie. On the next authentication request, +EngineBlock skips the WAYF and sends the user to that IdP if it is a candidate IdP for the requesting SP. The choice is +shared by all SPs. + +The remembered IdP is ignored for `ForceAuthn` and debug requests. + +The cookie can be removed on `/authentication/idp/remove-cookies`, see [Removing the cookies](#removing-the-cookies). + +## Per-SP mode + +With `feature_enable_wayf_remember_choice_per_idp: true` the remembered choice is stored per SP. The feature is +opt-in per SP through Manage metadata: + + metadata:coin:wayf_remember_choice: true + +Only the boolean `true` (or `"1"`) opts an SP in. An SP without the coin never uses the per-SP cookie, and never falls +back to the global cookie. + +When a user checks the checkbox and picks an IdP for an opted-in SP, EngineBlock stores the `(SP, IdP)` pair in the +`rememberedidps` cookie. On the next authentication request from that SP, EngineBlock skips the WAYF if the remembered +IdP is still a candidate IdP. The one-IdP shortcut and the SSO notification shortcut always take precedence, and +`ForceAuthn` and debug requests ignore the remembered IdP. + +Related parameters: + + # Lifetime of a remembered choice, in seconds (7776000 seconds = 90 days) + wayf.remember_choice_per_idp_lifetime: 7776000 + + # Maximum number of remembered SPs per cookie. The entries that expire soonest are evicted first. + wayf.remember_choice_per_idp_max: 16 + +The cookie content is validated strictly. Invalid or expired entries are never used to select an IdP and are removed +from the cookie. + +The cookie can be removed on `/authentication/idp/remove-cookies`, see [Removing the cookies](#removing-the-cookies). + +## Removing the cookies + +The page `/authentication/idp/remove-cookies` lets a user inspect and remove the cookies EngineBlock has set, +including `rememberchoice` and `rememberedidps`. The page is available when either mode is enabled, and returns a 404 +when both are disabled. + +## Switching modes + +Cookies written in one mode are not read in the other. After switching, users have to make their choice once more. +Switching from global to per-SP mode requires that you first set `wayf.remember_choice` to `false`, and then enable +`feature_enable_wayf_remember_choice_per_idp` and the `coin:wayf_remember_choice` coin on the SPs that should +use it. diff --git a/library/EngineBlock/Corto/Module/Service/ContinueToIdp.php b/library/EngineBlock/Corto/Module/Service/ContinueToIdp.php index 39c0bdee0f..488527124b 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,73 @@ 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; + + $entries = $rememberedIdpCookie->loadValidEntriesFromRequest($container->getSymfonyRequest(), $log); + + if (!$this->_isRememberChoiceChecked($rememberChoice)) { + return; + } + + if (!RememberedIdpCookie::isEnabledForServiceProvider( + $runtime->rememberChoicePerIdp, + $sp + )) { + return; + } + + $result = $rememberedIdpCookie->add($entries, $sp->entityId, $idp->entityId); + if (!$rememberedIdpCookie->write($result['entries'], $result['expires'])) { + $log->error(sprintf( + 'WAYF-remember-my-choice cookie write failed for SP %s and IdP %s', + $sp->entityId, + $idp->entityId + )); + + return; + } + + $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..5f2fb779cf 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; @@ -220,8 +222,19 @@ 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())) { + // Auto-select IdP when the per-SP remember-my-choice feature is enabled for this SP + $runtime = $application->getDiContainerRuntime(); + if ($this->_sendAuthenticationRequestForRememberedIdpPerServiceProvider($request, $sp, $candidateIDPs)) { + return; + } + + // Auto-select IdP when the global 'wayf.remember_choice' feature is enabled. It is mutually exclusive + // with 'feature_enable_wayf_remember_choice_per_idp', which is enforced when the container is compiled. + $legacyRememberChoiceAllowed = $container->getRememberChoice() === true + && !$request->getForceAuthn() + && !$request->isDebugRequest(); + + if ($legacyRememberChoiceAllowed) { $cookies = $container->getSymfonyRequest()->cookies->all(); if (array_key_exists('rememberchoice', $cookies)) { $remembered = json_decode($cookies['rememberchoice']); @@ -457,6 +470,60 @@ protected function _limitScopeToRememberedIdp( return $scopedIdps; } + /** + * @param string[] $candidateIDPs + */ + protected function _sendAuthenticationRequestForRememberedIdpPerServiceProvider( + EngineBlock_Saml2_AuthnRequestAnnotationDecorator $request, + ServiceProvider $sp, + array $candidateIDPs + ): bool { + $runtime = EngineBlock_ApplicationSingleton::getInstance()->getDiContainerRuntime(); + $log = $this->_server->getLogger(); + + $notEligibleForRememberedIdp = !RememberedIdpCookie::isEnabledForServiceProvider( + $runtime->rememberChoicePerIdp, + $sp + ) || $request->getForceAuthn() || $request->isDebugRequest(); + + if ($notEligibleForRememberedIdp) { + 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(); + + $entries = $rememberedIdpCookie->loadValidEntriesFromRequest( + EngineBlock_ApplicationSingleton::getInstance()->getDiContainer()->getSymfonyRequest(), + $log + ); + + return $rememberedIdpCookie->find($entries, $serviceProviderEntityId, $candidateIdpEntityIds); + } + protected function _showWayf(EngineBlock_Saml2_AuthnRequestAnnotationDecorator $request, array $candidateIdpEntityIds) { // Post to the 'continueToIdp' service @@ -482,6 +549,16 @@ protected function _showWayf(EngineBlock_Saml2_AuthnRequestAnnotationDecorator $ $diContainerRuntime = $application->getDiContainerRuntime(); + $isRememberChoicePerIdpEnabled = $diContainerRuntime->rememberChoicePerIdp; + $rememberChoicePerIdp = RememberedIdpCookie::isEnabledForServiceProvider( + $isRememberChoicePerIdpEnabled, + $serviceProvider + ); + + $rememberChoiceFeature = $isRememberChoicePerIdpEnabled + ? $rememberChoicePerIdp + : $container->getRememberChoice(); + $output = $diContainerRuntime->wayfRenderer->render( idpList: $idpList, preferredIdpEntityIds: $diContainerRuntime->getPreferredIdpEntityIds(), @@ -491,10 +568,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..30a35e2b6a --- /dev/null +++ b/src/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookie.php @@ -0,0 +1,325 @@ +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']; + } + + /** + * @return array + */ + public function loadValidEntriesFromRequest(Request $request, LoggerInterface $logger): array + { + return $this->loadValidEntries($request->cookies->get(self::NAME), $logger); + } + + /** @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, self::MAX_INFLATED_BYTES); + } 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..ed3ea6cb98 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, + public bool $rememberChoicePerIdp = false, private array $preferredIdpEntityIds = [], ) { } diff --git a/src/OpenConext/EngineBlockBundle/Bridge/EngineBlockBootstrapper.php b/src/OpenConext/EngineBlockBundle/Bridge/EngineBlockBootstrapper.php index f4be71278f..7ef6af6f2c 100644 --- a/src/OpenConext/EngineBlockBundle/Bridge/EngineBlockBootstrapper.php +++ b/src/OpenConext/EngineBlockBundle/Bridge/EngineBlockBootstrapper.php @@ -23,7 +23,9 @@ 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\Configuration\FeatureConfigurationInterface; use OpenConext\EngineBlockBundle\Service\WayfRenderer; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\HttpKernel\KernelEvents; @@ -31,8 +33,13 @@ class EngineBlockBootstrapper implements EventSubscriberInterface { + private const REMEMBER_CHOICE_PER_IDP_FEATURE = 'eb.feature_enable_wayf_remember_choice_per_idp'; + private readonly DiContainerRuntime $diContainerRuntime; + /** + * @SuppressWarnings(PHPMD.ExcessiveParameterList) + */ public function __construct( Environment $twig, WayfRenderer $wayfRenderer, @@ -41,6 +48,8 @@ public function __construct( FeedbackStateHelperInterface $feedbackStateHelper, FeedbackInfoCollectorInterface $feedbackInfoCollector, LoginLogger $loginLogger, + RememberedIdpCookie $rememberedIdpCookie, + FeatureConfigurationInterface $featureConfiguration, array $preferredIdpEntityIds = [], ) { $this->diContainerRuntime = new DiContainerRuntime( @@ -51,6 +60,9 @@ public function __construct( $feedbackStateHelper, $feedbackInfoCollector, $loginLogger, + $rememberedIdpCookie, + $featureConfiguration->hasFeature(self::REMEMBER_CHOICE_PER_IDP_FEATURE) + && $featureConfiguration->isEnabled(self::REMEMBER_CHOICE_PER_IDP_FEATURE), $preferredIdpEntityIds, ); } diff --git a/src/OpenConext/EngineBlockBundle/Controller/WayfController.php b/src/OpenConext/EngineBlockBundle/Controller/WayfController.php index 9c927b46bf..8016b702fb 100644 --- a/src/OpenConext/EngineBlockBundle/Controller/WayfController.php +++ b/src/OpenConext/EngineBlockBundle/Controller/WayfController.php @@ -22,6 +22,7 @@ use EngineBlock_Corto_Adapter; use EngineBlock_Exception; use OpenConext\EngineBlock\Service\SsoSessionService; +use OpenConext\EngineBlock\Service\Wayf\RememberedIdpCookie; use OpenConext\EngineBlockBridge\ResponseFactory; use OpenConext\EngineBlockBundle\Service\DiscoverySelectionService; use Psr\Log\LoggerInterface; @@ -104,9 +105,22 @@ public function helpDiscoverAction() return new Response($this->twig->render('@theme/Authentication/View/IdentityProvider/help-discover.html.twig')); } + private function isRememberChoiceEnabled(): bool + { + return $this->engineBlockApplicationSingleton->getDiContainer()->getRememberChoice() === true + || $this->engineBlockApplicationSingleton->getDiContainerRuntime()->rememberChoicePerIdp; + } + + private function clearRememberedIdpCookie(string $cookie): void + { + if ($cookie === RememberedIdpCookie::NAME) { + $this->engineBlockApplicationSingleton->getDiContainerRuntime()->rememberedIdpCookie->clear(); + } + } + private function getCookies(): array { - $cookies = ['main', 'rememberchoice', 'lang', 'selectedidps']; + $cookies = ['main', 'rememberchoice', RememberedIdpCookie::NAME, 'lang', 'selectedidps']; if ($this->engineBlockApplicationSingleton->getDiContainer()->getFeatureConfiguration() ->hasFeature("eb.enable_sso_session_cookie")) { @@ -119,8 +133,7 @@ private function getCookies(): array #[Route(path: '/authentication/idp/remove-cookies', name: 'authentication_wayf_remove_cookie', methods: ['GET', 'POST'])] public function cookieAction(Request $request) { - $application = $this->engineBlockApplicationSingleton; - if (($application->getDiContainer()->getRememberChoice() === true)) { + if ($this->isRememberChoiceEnabled()) { $postData = $request->request->all(); $cookiesSet = $request->cookies->all(); $cookies = $this->getCookies(); @@ -132,6 +145,7 @@ public function cookieAction(Request $request) if (array_key_exists((string) $cookie, $cookiesSet)) { unset($cookiesSet[$cookie]); $response->headers->clearCookie($cookie); + $this->clearRememberedIdpCookie($cookie); } } // Clear all session-data on the server @@ -145,6 +159,7 @@ public function cookieAction(Request $request) if (array_key_exists('remove_'.$cookie, $postData)) { unset($cookiesSet[$cookie]); $response->headers->clearCookie($cookie); + $this->clearRememberedIdpCookie($cookie); if ($cookie === SsoSessionService::SSO_SESSION_COOKIE_NAME) { $this->sessionService->clearSsoSessionCookie(); } diff --git a/src/OpenConext/EngineBlockBundle/DependencyInjection/Compiler/RememberChoiceModeCompilerPass.php b/src/OpenConext/EngineBlockBundle/DependencyInjection/Compiler/RememberChoiceModeCompilerPass.php new file mode 100644 index 0000000000..9ce053c626 --- /dev/null +++ b/src/OpenConext/EngineBlockBundle/DependencyInjection/Compiler/RememberChoiceModeCompilerPass.php @@ -0,0 +1,57 @@ + $this->resolveBool($container, self::GLOBAL_PARAMETER), + self::PER_IDP_PARAMETER => $this->resolveBool($container, self::PER_IDP_PARAMETER), + ]); + + if ($featureConfiguration->isEnabled(self::GLOBAL_PARAMETER) + && $featureConfiguration->isEnabled(self::PER_IDP_PARAMETER) + ) { + throw new InvalidConfigurationException(sprintf( + 'The WAYF remember-my-choice modes are mutually exclusive: "%s" and "%s" cannot both be true. ' + . 'Enable only one of them.', + self::GLOBAL_PARAMETER, + self::PER_IDP_PARAMETER + )); + } + } + + private function resolveBool(ContainerBuilder $container, string $parameter): bool + { + return $container->hasParameter($parameter) + && filter_var($container->getParameter($parameter), FILTER_VALIDATE_BOOLEAN); + } +} diff --git a/src/OpenConext/EngineBlockBundle/OpenConextEngineBlockBundle.php b/src/OpenConext/EngineBlockBundle/OpenConextEngineBlockBundle.php index 03ef13c3ff..f081172e8c 100644 --- a/src/OpenConext/EngineBlockBundle/OpenConextEngineBlockBundle.php +++ b/src/OpenConext/EngineBlockBundle/OpenConextEngineBlockBundle.php @@ -21,10 +21,17 @@ use EngineBlock_ApplicationSingleton; use OpenConext\EngineBlock\Logger\Handler\FingersCrossed\ManualOrDecoratedActivationStrategy; use OpenConext\EngineBlock\Request\RequestId; +use OpenConext\EngineBlockBundle\DependencyInjection\Compiler\RememberChoiceModeCompilerPass; +use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\HttpKernel\Bundle\Bundle; class OpenConextEngineBlockBundle extends Bundle { + public function build(ContainerBuilder $container): void + { + $container->addCompilerPass(new RememberChoiceModeCompilerPass()); + } + public function boot() { $engineBlockApplicationSingleton = EngineBlock_ApplicationSingleton::getInstance(); 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..d7634f0956 --- /dev/null +++ b/tests/library/EngineBlock/Test/Corto/Module/Service/ContinueToIdpTest.php @@ -0,0 +1,196 @@ +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 $rememberChoicePerIdpEnabled, + bool $spWayfRememberChoice, + bool $expectCookieWritten + ): void { + $application = EngineBlock_ApplicationSingleton::getInstance(); + + $diContainer = Phake::mock(EngineBlock_Application_DiContainer::class); + 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], + 'per-idp feature disabled: not written' => [false, true, false], + 'sp opted out: not written' => [true, false, false], + 'all disabled: not written' => [false, false, false], + ]; + } + + public function testPersistRememberedChoiceLogsAnErrorWhenCookieWriteFails(): void + { + $application = EngineBlock_ApplicationSingleton::getInstance(); + + $diContainer = Phake::mock(EngineBlock_Application_DiContainer::class); + Phake::when($diContainer)->getSymfonyRequest()->thenReturn(Request::create('/')); + $this->setDiContainer($application, $diContainer); + + $cookieService = Phake::mock(CookieService::class); + Phake::when($cookieService)->setCookieWithSameSite(Phake::anyParameters())->thenReturn(false); + + $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, + true, + )); + + $logger = Phake::mock(LoggerInterface::class); + $proxyServerMock = Phake::mock(EngineBlock_Corto_ProxyServer::class); + Phake::when($proxyServerMock)->getLogger()->thenReturn($logger); + + $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: true); + $idp = new IdentityProvider('https://idp.example.org'); + + $method = new ReflectionMethod($service, '_persistRememberedChoice'); + $method->invoke($service, $sp, $idp, '1'); + + Phake::verify($logger)->error( + 'WAYF-remember-my-choice cookie write failed for SP https://sp.example.org and IdP https://idp.example.org' + ); + Phake::verify($logger, Phake::never())->info(Phake::anyParameters()); + } + + 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..35388cdc83 --- /dev/null +++ b/tests/unit/OpenConext/EngineBlock/Service/Wayf/RememberedIdpCookieTest.php @@ -0,0 +1,450 @@ +assertSame( + $expected, + RememberedIdpCookie::isEnabledForServiceProvider( + $rememberChoicePerIdpEnabled, + $serviceProvider + ) + ); + } + + public static function isEnabledForServiceProviderProvider(): array + { + return [ + 'all enabled' => [true, true, true], + 'per-idp feature disabled' => [false, true, false], + 'sp opted out' => [true, false, false], + 'all disabled' => [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"}}')], + 'oversized decompressed payload exceeds the bound' => [ + base64_encode((string) gzdeflate((string) json_encode([ + 'https://sp.example.org' => [ + 'idp' => 'https://idp.example.org/' . str_repeat('a', 100000), + 'expires' => 1700003600, + ], + ]))), + ], + ]; + } + + 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); + } + + public function testLoadValidEntriesFromRequestReadsCookieFromRequest(): void + { + $entries = [ + 'https://sp.example.org' => ['idp' => 'https://idp.example.org', 'expires' => self::NOW + 1], + ]; + $request = Request::create('/'); + $request->cookies->set(RememberedIdpCookie::NAME, self::rawEntries($entries)); + $logger = Phake::mock(LoggerInterface::class); + + $result = $this->buildCookie()->loadValidEntriesFromRequest($request, $logger); + + $this->assertSame($entries, $result); + } + + public function testLoadValidEntriesFromRequestHandlesMissingCookie(): void + { + $request = Request::create('/'); + $logger = Phake::mock(LoggerInterface::class); + + $result = $this->buildCookie()->loadValidEntriesFromRequest($request, $logger); + + $this->assertSame([], $result); + } + + 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/DependencyInjection/Compiler/RememberChoiceModeCompilerPassTest.php b/tests/unit/OpenConext/EngineBlockBundle/DependencyInjection/Compiler/RememberChoiceModeCompilerPassTest.php new file mode 100644 index 0000000000..8e71f0d751 --- /dev/null +++ b/tests/unit/OpenConext/EngineBlockBundle/DependencyInjection/Compiler/RememberChoiceModeCompilerPassTest.php @@ -0,0 +1,79 @@ +containerWith($global, $perIdp); + + (new RememberChoiceModeCompilerPass())->process($container); + + $this->addToAssertionCount(1); + } + + public static function validCombinationProvider(): array + { + return [ + 'both disabled' => [false, false], + 'only global enabled' => [true, false], + 'only per-idp enabled' => [false, true], + 'parameters absent' => [null, null], + 'global enabled, per-idp absent' => [true, null], + 'per-idp enabled, global absent' => [null, true], + ]; + } + + public function testRejectsBothRememberChoiceModesEnabled(): void + { + $container = $this->containerWith(true, true); + + $this->expectException(InvalidConfigurationException::class); + $this->expectExceptionMessage('mutually exclusive'); + $this->expectExceptionMessage('wayf.remember_choice'); + $this->expectExceptionMessage('feature_enable_wayf_remember_choice_per_idp'); + + (new RememberChoiceModeCompilerPass())->process($container); + } + + private function containerWith(?bool $global, ?bool $perIdp): ContainerBuilder + { + $container = new ContainerBuilder(); + + if ($global !== null) { + $container->setParameter(RememberChoiceModeCompilerPass::GLOBAL_PARAMETER, $global); + } + + if ($perIdp !== null) { + $container->setParameter(RememberChoiceModeCompilerPass::PER_IDP_PARAMETER, $perIdp); + } + + return $container; + } +} diff --git a/tests/unit/OpenConext/EngineBlockBundle/DiContainerRuntimeTest.php b/tests/unit/OpenConext/EngineBlockBundle/DiContainerRuntimeTest.php index 13d9f5f314..3dfcf7a9bb 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->rememberChoicePerIdp); + } + + public function testIsRememberChoicePerIdpEnabledReturnsConfiguredValue(): void + { + $runtime = $this->runtimeFactory(rememberChoicePerIdp: true); + + $this->assertTrue($runtime->rememberChoicePerIdp); + } + /** * @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/EngineBlockBootstrapperTest.php b/tests/unit/OpenConext/EngineBlockBundle/EngineBlockBootstrapperTest.php new file mode 100644 index 0000000000..3cafd0647c --- /dev/null +++ b/tests/unit/OpenConext/EngineBlockBundle/EngineBlockBootstrapperTest.php @@ -0,0 +1,121 @@ +createStub(FeatureConfigurationInterface::class); + $featureConfiguration->method('hasFeature') + ->with('eb.feature_enable_wayf_remember_choice_per_idp') + ->willReturn(true); + $featureConfiguration->method('isEnabled') + ->with('eb.feature_enable_wayf_remember_choice_per_idp') + ->willReturn(true); + + $bootstrapper = $this->bootstrapperFactory($featureConfiguration); + + $this->assertTrue($this->diContainerRuntimeOf($bootstrapper)->rememberChoicePerIdp); + } + + #[Test] + public function it_disables_remember_choice_per_idp_when_the_feature_is_disabled(): void + { + $featureConfiguration = $this->createStub(FeatureConfigurationInterface::class); + $featureConfiguration->method('hasFeature') + ->with('eb.feature_enable_wayf_remember_choice_per_idp') + ->willReturn(true); + $featureConfiguration->method('isEnabled') + ->with('eb.feature_enable_wayf_remember_choice_per_idp') + ->willReturn(false); + + $bootstrapper = $this->bootstrapperFactory($featureConfiguration); + + $this->assertFalse($this->diContainerRuntimeOf($bootstrapper)->rememberChoicePerIdp); + } + + #[Test] + public function it_disables_remember_choice_per_idp_when_the_feature_is_unknown(): void + { + $featureConfiguration = $this->createStub(FeatureConfigurationInterface::class); + $featureConfiguration->method('hasFeature') + ->with('eb.feature_enable_wayf_remember_choice_per_idp') + ->willReturn(false); + + $bootstrapper = $this->bootstrapperFactory($featureConfiguration); + + $this->assertFalse($this->diContainerRuntimeOf($bootstrapper)->rememberChoicePerIdp); + } + + private function bootstrapperFactory(FeatureConfigurationInterface $featureConfiguration): EngineBlockBootstrapper + { + return new EngineBlockBootstrapper( + $this->createStub(Environment::class), + $this->createStub(WayfRenderer::class), + new CorrelationIdService(new CorrelationIdRepository(new RequestStack()), new CurrentCorrelationId()), + new CurrentCorrelationId(), + $this->createStub(FeedbackStateHelperInterface::class), + $this->createStub(FeedbackInfoCollectorInterface::class), + $this->createStub(LoginLogger::class), + $this->rememberedIdpCookieStub(), + $featureConfiguration, + ); + } + + private function diContainerRuntimeOf(EngineBlockBootstrapper $bootstrapper): DiContainerRuntime + { + $property = new ReflectionProperty(EngineBlockBootstrapper::class, 'diContainerRuntime'); + + return $property->getValue($bootstrapper); + } + + 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/README.md b/theme/README.md index c24896ad5a..99ccaf5f5d 100644 --- a/theme/README.md +++ b/theme/README.md @@ -166,7 +166,7 @@ There are a lot of error pages. To test all different kinds, you can use the ur - redirect page: `templates > modules > authentication > view > proxy > redirect.html.twig`. - spinner page: `templates > modules > authentication > view > proxy > form.html.twig`. To test it disable the onload handler on the body-tag and go to your profile (or load the page without JS). - index.html.twig: `templates > modules > authentication > view > index > index.html.twig`. You can use `https://engine.dev.openconext.local/` to develop the page. -- cookie removal page: `templates > modules > authentication > view > identityprovider > remove-cookies.html.twig`. You can use `https://engine.dev.openconext.local/authentication/idp/remove-cookies` to develop the page. The page is only accessible if you set the `wayf.remember_choice` parameter in `parameters.yml` to true. +- cookie removal page: `templates > modules > authentication > view > identityprovider > remove-cookies.html.twig`. You can use `https://engine.dev.openconext.local/authentication/idp/remove-cookies` to develop the page. The page is only accessible if you set the `wayf.remember_choice` or the `feature_enable_wayf_remember_choice_per_idp` parameter in `parameters.yml` to true. - debug page: `templates > modules > authentication > view > proxy > debug-idp-response.html.twig`. You can use `https://engine.dev.openconext.local/authentication/sp/debug` to develop the page. - logout page: `templates > modules > logout > view > index > index.html.twig`. You can use `https://engine.dev.openconext.local/logout` to develop the page. 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 %}