From 151a5285ddabde35f288d58bdc9cae76cbbe7253 Mon Sep 17 00:00:00 2001 From: Johan Kromhout Date: Wed, 5 Feb 2025 15:08:41 +0100 Subject: [PATCH] Feat: Implement Discovery support Prior to this change, there were cases where it wasn't clear which IdP end users should use. In these scenario's the users needed an IdP which was not recognisable for them. This change adds support for discovery IdP entries. Which are additional names / ways of finding an IdP in the WAYF. These can be configured in Manage. A discovery requires at least an english name, but can also include keywords or a custom logo, which is used on the consent page as well. Resolves https://github.com/OpenConext/OpenConext-engineblock/issues/1338 --- app/config/config.yml | 2 +- ci/qa/phpcbf.sh | 7 + .../Version20250206095609.php | 28 +++ .../EngineBlock/Application/DiContainer.php | 8 + .../Corto/Module/Service/ProvideConsent.php | 15 +- .../Corto/Module/Service/SingleSignOn.php | 60 +++++- library/EngineBlock/Corto/Module/Services.php | 6 +- .../Exception/InvalidDiscoveryException.php | 23 +++ .../EngineBlock/Metadata/Discovery.php | 118 +++++++++++ .../Entity/Assembler/DiscoveryAssembler.php | 97 +++++++++ .../Assembler/PushMetadataAssembler.php | 19 +- .../Metadata/Entity/IdentityProvider.php | 71 ++++++- .../Adapter/IdentityProviderEntity.php | 5 + .../Decorator/AbstractIdentityProvider.php | 5 + .../IdentityProviderEntityInterface.php | 2 + .../Controller/WayfController.php | 39 +++- ...edirectToFeedbackPageExceptionListener.php | 6 +- .../config/controllers/authentication.yml | 2 + .../Resources/config/services.yml | 4 + .../Service/DiscoverySelectionService.php | 72 +++++++ .../Service/IdpHistoryService.php | 27 +++ .../Twig/Extensions/Extension/Wayf.php | 129 ++++++++---- .../Controllers/WayfController.php | 3 +- .../Helper/TestEntitySeeder.php | 71 ++++++- .../wayf/WayfShowsConnectedIdps.spec.js | 4 +- .../skeune/wayf/wayf.general.spec.js | 8 + .../skeune/wayf/wayf.keyboard.spec.js | 4 + .../Assembler/DiscoveryAssemblerTest.php | 176 +++++++++++++++++ .../Assembler/PushMetadataAssemblerTest.php | 65 +++++- .../fixtures/metadata_idp_display_data.json | 62 ++++++ .../metadata_without_discoveries.json | 46 +++++ .../Service/DiscoverySelectionServiceTest.php | 186 ++++++++++++++++++ .../Module/Service/ProvideConsentTest.php | 10 +- .../Metadata/Factory/AbstractEntityTest.php | 8 +- .../wayf/deleteDisable/addSelectedIdp.js | 7 +- .../wayf/matchPreviouslySelectedWithCookie.js | 2 +- .../Consent/Attributes/list.html.twig | 8 +- .../Proxy/Partials/WAYF/idp/idp.html.twig | 1 + .../Proxy/Partials/WAYF/idp/idpForm.html.twig | 1 + .../View/Proxy/consent.html.twig | 6 +- .../View/Proxy/consent.html.twig | 6 +- 41 files changed, 1340 insertions(+), 79 deletions(-) create mode 100755 ci/qa/phpcbf.sh create mode 100644 database/DoctrineMigrations/Version20250206095609.php create mode 100644 src/OpenConext/EngineBlock/Exception/InvalidDiscoveryException.php create mode 100644 src/OpenConext/EngineBlock/Metadata/Discovery.php create mode 100644 src/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssembler.php create mode 100644 src/OpenConext/EngineBlockBundle/Service/DiscoverySelectionService.php create mode 100644 src/OpenConext/EngineBlockBundle/Service/IdpHistoryService.php create mode 100644 tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssemblerTest.php create mode 100644 tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/fixtures/metadata_idp_display_data.json create mode 100644 tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/fixtures/metadata_without_discoveries.json create mode 100644 tests/integration/OpenConext/EngineBlockBundle/Service/DiscoverySelectionServiceTest.php diff --git a/app/config/config.yml b/app/config/config.yml index d2859b13e8..2c64dd7306 100644 --- a/app/config/config.yml +++ b/app/config/config.yml @@ -99,7 +99,7 @@ doctrine: # when true, queries are logged to a 'doctrine' monolog channel logging: '%kernel.debug%' profiling: '%kernel.debug%' - server_version: 5.5 + server_version: '10.6.20-MariaDB' mapping_types: enum: string types: diff --git a/ci/qa/phpcbf.sh b/ci/qa/phpcbf.sh new file mode 100755 index 0000000000..497168f9d6 --- /dev/null +++ b/ci/qa/phpcbf.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash +set -e + +cd $(dirname $0)/../../ + +echo -e "\nPHP CodeSniffer\n" +./vendor/bin/phpcbf --standard=ci/qa-config/phpcs.xml src diff --git a/database/DoctrineMigrations/Version20250206095609.php b/database/DoctrineMigrations/Version20250206095609.php new file mode 100644 index 0000000000..1462f46c57 --- /dev/null +++ b/database/DoctrineMigrations/Version20250206095609.php @@ -0,0 +1,28 @@ +abortIf($this->connection->getDatabasePlatform()->getName() !== 'mysql', 'Migration can only be executed safely on \'mysql\'.'); + + $this->addSql('ALTER TABLE sso_provider_roles_eb5 ADD idp_discoveries LONGTEXT DEFAULT NULL COMMENT \'(DC2Type:json)\''); + } + + public function down(Schema $schema) : void + { + // this down() migration is auto-generated, please modify it to your needs + $this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'mysql', 'Migration can only be executed safely on \'mysql\'.'); + + $this->addSql('ALTER TABLE sso_provider_roles_eb5 DROP idp_discoveries'); + } +} diff --git a/library/EngineBlock/Application/DiContainer.php b/library/EngineBlock/Application/DiContainer.php index 09a6fdd8b4..f30156478f 100644 --- a/library/EngineBlock/Application/DiContainer.php +++ b/library/EngineBlock/Application/DiContainer.php @@ -463,6 +463,14 @@ public function getProcessingStateHelper() return $this->container->get('engineblock.service.processing_state_helper'); } + /** + * @return \OpenConext\EngineBlockBundle\Service\DiscoverySelectionService + */ + public function getDiscoverySelectionService() + { + return $this->container->get('engineblock.service.discovery_selection_service'); + } + public function getMfaHelper(): MfaHelperInterface { return $this->container->get('engineblock.service.mfa_helper'); diff --git a/library/EngineBlock/Corto/Module/Service/ProvideConsent.php b/library/EngineBlock/Corto/Module/Service/ProvideConsent.php index c41cd9bec5..43a55fb66b 100644 --- a/library/EngineBlock/Corto/Module/Service/ProvideConsent.php +++ b/library/EngineBlock/Corto/Module/Service/ProvideConsent.php @@ -21,6 +21,8 @@ use OpenConext\EngineBlock\Service\AuthenticationStateHelperInterface; use OpenConext\EngineBlock\Service\ConsentServiceInterface; use OpenConext\EngineBlock\Service\ProcessingStateHelperInterface; +use OpenConext\EngineBlockBundle\Service\DiscoverySelectionService; +use Psr\Log\LogLevel; use SAML2\Constants; use Symfony\Component\HttpFoundation\Request; use Twig\Environment; @@ -69,7 +71,8 @@ public function __construct( ConsentServiceInterface $consentService, AuthenticationStateHelperInterface $authStateHelper, Environment $twig, - ProcessingStateHelperInterface $processingStateHelper + ProcessingStateHelperInterface $processingStateHelper, + DiscoverySelectionService $discoverySelectionService ) { $this->_server = $server; @@ -80,6 +83,7 @@ public function __construct( $this->twig = $twig; $this->_processingStateHelper = $processingStateHelper; $this->logger = EngineBlock_ApplicationSingleton::getLog(); + $this->discoverySelectionService = $discoverySelectionService; } /** @@ -132,6 +136,14 @@ public function serve($serviceName, Request $httpRequest) $authenticationState = $this->_authenticationStateHelper->getAuthenticationState(); + $session = $httpRequest->getSession(); + if($session !== null){ + $idpDiscovery = $this->discoverySelectionService->getDiscoveryFromRequest($session, $identityProvider); + }else{ + $idpDiscovery = null; + $this->logger->log(LogLevel::ERROR, 'Discovery override failure: No session available.'); + } + if ($this->isConsentDisabled($spMetadataChain, $identityProvider)) { if (!$consentRepository->implicitConsentWasGivenFor($serviceProviderMetadata)) { $consentRepository->giveImplicitConsentFor($serviceProviderMetadata); @@ -217,6 +229,7 @@ public function serve($serviceName, Request $httpRequest) 'responseId' => $receivedRequest->getId(), 'sp' => $serviceProviderMetadata, 'idp' => $identityProvider, + 'idpDiscovery' => $idpDiscovery, 'idpSupport' => $this->getSupportContact($identityProvider), 'attributes' => $attributes, 'attributeSources' => $this->getAttributeSources($request->getId()), diff --git a/library/EngineBlock/Corto/Module/Service/SingleSignOn.php b/library/EngineBlock/Corto/Module/Service/SingleSignOn.php index f2cb467836..3477f525c4 100644 --- a/library/EngineBlock/Corto/Module/Service/SingleSignOn.php +++ b/library/EngineBlock/Corto/Module/Service/SingleSignOn.php @@ -20,8 +20,10 @@ use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider; use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider; use OpenConext\EngineBlock\Metadata\Factory\Factory\ServiceProviderFactory; +use OpenConext\EngineBlock\Metadata\Discovery; use OpenConext\EngineBlock\Metadata\X509\KeyPairFactory; use OpenConext\EngineBlockBundle\Exception\InvalidArgumentException as EngineBlockBundleInvalidArgumentException; +use OpenConext\EngineBlockBundle\Service\DiscoverySelectionService; use SAML2\AuthnRequest; use SAML2\Response; use SAML2\XML\saml\Issuer; @@ -49,16 +51,23 @@ class EngineBlock_Corto_Module_Service_SingleSignOn implements EngineBlock_Corto */ private $_serviceProviderFactory; + /** + * @var DiscoverySelectionService + */ + private $discoverySelectionService; + public function __construct( EngineBlock_Corto_ProxyServer $server, EngineBlock_Corto_XmlToArray $xmlConverter, Twig_Environment $twig, - ServiceProviderFactory $serviceProviderFactory + ServiceProviderFactory $serviceProviderFactory, + DiscoverySelectionService $discoverySelectionService ) { $this->_server = $server; $this->_xmlConverter = $xmlConverter; $this->twig = $twig; $this->_serviceProviderFactory = $serviceProviderFactory; + $this->discoverySelectionService = $discoverySelectionService; } public function serve($serviceName, Request $httpRequest) @@ -513,21 +522,54 @@ protected function _transformIdpsForWayf(array $idpEntityIds, $isDebugRequest, $ $name = $this->getName($currentLocale, $identityProvider, $additionalInfo); - $wayfIdp = array( - 'Name' => $name, - 'Logo' => $identityProvider->getMdui()->hasLogo() ? $identityProvider->getMdui()->getLogo()->url : '/images/placeholder.png', - 'Keywords' => $this->getKeywords($currentLocale, $identityProvider), - 'Access' => $isAccessible ? '1' : '0', - 'ID' => md5($identityProvider->entityId), - 'EntityID' => $identityProvider->entityId, - self::IS_DEFAULT_IDP_KEY => $isDefaultIdP + $wayfIdp = $this->buildIdp( + $name, + $identityProvider->getMdui()->hasLogo() ? $identityProvider->getMdui()->getLogo()->url : '/images/placeholder.png', + $this->getKeywords($currentLocale, $identityProvider), + $identityProvider->entityId, + $isAccessible, + $isDefaultIdP, + null ); $wayfIdps[] = $wayfIdp; + + foreach ($identityProvider->getDiscoveries() as $discovery) { + /** @var Discovery $discovery */ + $wayfIdps[] = $this->buildIdp( + $discovery->getName($currentLocale), + $discovery->hasLogo() ? $discovery->getLogo()->url : '/images/placeholder.png', + $discovery->getKeywordsArray($currentLocale), + $identityProvider->entityId, + $isAccessible, + $isDefaultIdP, + $this->discoverySelectionService->hash($discovery) + ); + } } return $wayfIdps; } + private function buildIdp( + ?string $name, + string $logo, + $keywords, + string $entityId, + bool $isAccessible, + bool $isDefaultIdP, + ?string $discoveryHash + ): array { + return array( + 'Name' => $name, + 'Logo' => $logo, + 'Keywords' => $keywords, + 'Access' => $isAccessible ? '1' : '0', + 'ID' => md5($entityId), + 'EntityID' => $entityId, + self::IS_DEFAULT_IDP_KEY => $isDefaultIdP, + 'DiscoveryHash' => $discoveryHash, + ); + } /** * @param Response|EngineBlock_Saml2_ResponseAnnotationDecorator $response */ diff --git a/library/EngineBlock/Corto/Module/Services.php b/library/EngineBlock/Corto/Module/Services.php index 73cb348286..d700844d3a 100644 --- a/library/EngineBlock/Corto/Module/Services.php +++ b/library/EngineBlock/Corto/Module/Services.php @@ -102,7 +102,8 @@ private function factoryService($className, EngineBlock_Corto_ProxyServer $serve $diContainer->getConsentService(), $diContainer->getAuthenticationStateHelper(), $diContainer->getTwigEnvironment(), - $diContainer->getProcessingStateHelper() + $diContainer->getProcessingStateHelper(), + $diContainer->getDiscoverySelectionService() ); case EngineBlock_Corto_Module_Service_ProcessConsent::class : return new EngineBlock_Corto_Module_Service_ProcessConsent( @@ -139,7 +140,8 @@ private function factoryService($className, EngineBlock_Corto_ProxyServer $serve $server, $diContainer->getXmlConverter(), $diContainer->getTwigEnvironment(), - $diContainer->getServiceProviderFactory() + $diContainer->getServiceProviderFactory(), + $diContainer->getDiscoverySelectionService() ); case EngineBlock_Corto_Module_Service_ContinueToIdp::class : return new EngineBlock_Corto_Module_Service_ContinueToIdp( diff --git a/src/OpenConext/EngineBlock/Exception/InvalidDiscoveryException.php b/src/OpenConext/EngineBlock/Exception/InvalidDiscoveryException.php new file mode 100644 index 0000000000..d28aed9da7 --- /dev/null +++ b/src/OpenConext/EngineBlock/Exception/InvalidDiscoveryException.php @@ -0,0 +1,23 @@ + $names + * @param array $keywords + */ + public static function create(array $names, array $keywords, ?Logo $logo): Discovery + { + $discovery = new self; + $discovery->logo = $logo; + + $discovery->names = array_filter($names); + $discovery->keywords = array_filter($keywords); + + if (!$discovery->isValid()) { + throw new InvalidDiscoveryException('The Discovery does not have a required english name.'); + } + + return $discovery; + } + + public function jsonSerialize() + { + return [ + 'names' => $this->names, + 'keywords' => $this->keywords, + 'logo' => $this->logo, + ]; + } + + public function hasLogo(): bool + { + return $this->logo !== null && $this->logo->url !== null; + } + + public function getLogo(): ?Logo + { + return $this->logo; + } + + public function getLanguage(): string + { + return $this->language; + } + + public function getName(string $locale): string + { + if ($locale !== '' && isset($this->names[$locale])) { + return $this->names[$locale]; + } + + return $this->names['en'] ?? ''; + } + + public function getKeywords(string $locale): string + { + if ($locale !== '' && isset($this->keywords[$locale])) { + return $this->keywords[$locale]; + } + + return $this->keywords['en'] ?? ''; + } + + /** + * @return string[] + */ + public function getKeywordsArray(string $locale): array + { + return explode(' ', $this->getKeywords($locale)); + } + + public function isValid(): bool + { + return $this->getName('en') !== ''; + } +} diff --git a/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssembler.php b/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssembler.php new file mode 100644 index 0000000000..87dd08910c --- /dev/null +++ b/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssembler.php @@ -0,0 +1,97 @@ +languageSupportProvider = $languageSupportProvider; + } + + public function assembleDiscoveries(stdClass $connection): array + { + if (!isset($connection->metadata->discoveries)) { + return []; + } + + $discoveries = []; + foreach ($connection->metadata->discoveries as $discovery) { + $names = $this->extractLocalizedFields($discovery, 'name'); + $keywords = $this->extractLocalizedFields($discovery, 'keywords'); + $logo = $this->assembleLogo($discovery); + + if (isset($names['en'])) { + $discoveries[] = Discovery::create($names, $keywords, $logo); + } + } + + return empty($discoveries) ? [] : ['discoveries' => $discoveries]; + } + + private function extractLocalizedFields(stdClass $discovery, string $fieldPrefix): array + { + $fields = []; + foreach ($this->languageSupportProvider->getSupportedLanguages() as $language) { + $accessor = sprintf('%s_%s', $fieldPrefix, $language); + if (isset($discovery->$accessor)) { + $fields[$language] = $discovery->$accessor; + } + } + + return array_filter(array_map('trim', $fields)); + } + + private function assembleLogo(stdClass $discovery): ?Logo + { + $logoFields = []; + $logoProperties = ['logo_url', 'logo_height', 'logo_width']; + + foreach ($logoProperties as $property) { + if (isset($discovery->$property)) { + $logoFields[$property] = $discovery->$property; + } + } + + if (!isset($logoFields['logo_url']) || trim($logoFields['logo_url']) === '') { + return null; + } + + $logo = new Logo($logoFields['logo_url']); + + if (isset($logoFields['logo_height'])) { + $logo->height = $logoFields['logo_height']; + } + if (isset($logoFields['logo_width'])) { + $logo->width = $logoFields['logo_width']; + } + + return $logo; + } +} diff --git a/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php b/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php index 6d71667d56..3f3c3a13ec 100644 --- a/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php +++ b/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php @@ -37,13 +37,16 @@ use OpenConext\EngineBlock\Metadata\Service; use OpenConext\EngineBlock\Metadata\ShibMdScope; use OpenConext\EngineBlock\Metadata\StepupConnections; +use OpenConext\EngineBlock\Metadata\Discovery; use OpenConext\EngineBlock\Metadata\Utils; use OpenConext\EngineBlock\Metadata\X509\X509CertificateFactory; use OpenConext\EngineBlock\Metadata\X509\X509CertificateLazyProxy; use OpenConext\EngineBlock\Validator\ValidatorInterface; +use OpenConext\EngineBlockBundle\Localization\LanguageSupportProvider; use Psr\Log\LoggerInterface; use RuntimeException; use stdClass; + use function array_key_exists; /** @@ -56,6 +59,11 @@ class PushMetadataAssembler implements MetadataAssemblerInterface */ private $allowedAcsLocationsValidator; + /** + * @var DiscoveryAssembler + */ + private $discoveryAssembler; + /** * @var LoggerInterface */ @@ -66,9 +74,13 @@ class PushMetadataAssembler implements MetadataAssemblerInterface */ private const FIELDS_MAX_LENGTH = 255; - public function __construct(ValidatorInterface $allowedAcsLocations, LoggerInterface $logger) - { + public function __construct( + ValidatorInterface $allowedAcsLocations, + LanguageSupportProvider $languageSupportProvider, + LoggerInterface $logger + ) { $this->allowedAcsLocationsValidator = $allowedAcsLocations; + $this->discoveryAssembler = new DiscoveryAssembler($languageSupportProvider); $this->logger = $logger; } @@ -316,6 +328,8 @@ private function assembleIdp(stdClass $connection) $properties += $this->assembleStepupConnections($connection); $properties += $this->assembleMfaEntities($connection); + $properties += $this->discoveryAssembler->assembleDiscoveries($connection); + return Utils::instantiate( IdentityProvider::class, $properties @@ -474,6 +488,7 @@ private function setPathFromObjectString(array $from, string $to, bool $limitlen if ($limitlength) { $reference = $this->limitValueLength($reference); } + return array($to => (string)$reference); } diff --git a/src/OpenConext/EngineBlock/Metadata/Entity/IdentityProvider.php b/src/OpenConext/EngineBlock/Metadata/Entity/IdentityProvider.php index b33b439bad..42817f141a 100644 --- a/src/OpenConext/EngineBlock/Metadata/Entity/IdentityProvider.php +++ b/src/OpenConext/EngineBlock/Metadata/Entity/IdentityProvider.php @@ -19,9 +19,11 @@ namespace OpenConext\EngineBlock\Metadata\Entity; use Doctrine\ORM\Mapping as ORM; +use InvalidArgumentException; +use OpenConext\EngineBlock\Exception\InvalidDiscoveryException; use OpenConext\EngineBlock\Metadata\Coins; use OpenConext\EngineBlock\Metadata\ConsentSettings; -use OpenConext\EngineBlock\Metadata\Factory\IdentityProviderEntityInterface; +use OpenConext\EngineBlock\Metadata\Discovery; use OpenConext\EngineBlock\Metadata\Logo; use OpenConext\EngineBlock\Metadata\Mdui; use OpenConext\EngineBlock\Metadata\MetadataRepository\Visitor\VisitorInterface; @@ -38,6 +40,7 @@ * @ORM\Entity * @SuppressWarnings(PHPMD.CamelCasePropertyName) * @SuppressWarnings(PHPMD.ExcessiveParameterList) + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) * * WARNING: Please don't use this entity directly but use the dedicated factory instead. * @see \OpenConext\EngineBlock\Factory\Factory\IdentityProviderFactory @@ -87,6 +90,13 @@ class IdentityProvider extends AbstractRole */ public $shibMdScopes = array(); + /** + * @var array + * + * @ORM\Column(name="idp_discoveries", type="json") + */ + private $discoveries; + /** * WARNING: Please don't use this entity directly but use the dedicated factory instead. * @see \OpenConext\EngineBlock\Factory\Factory\IdentityProviderFactory @@ -132,7 +142,8 @@ public function __construct( array $singleSignOnServices = array(), ConsentSettings $consentSettings = null, StepupConnections $stepupConnections = null, - MfaEntityCollection $mfaEntities = null + MfaEntityCollection $mfaEntities = null, + array $discoveries = [] ) { if (is_null($mdui)) { $mdui = Mdui::emptyMdui(); @@ -181,6 +192,9 @@ public function __construct( $signatureMethod, $mfaEntities ); + + $this->assertAllDiscoveries($discoveries); + $this->discoveries = $discoveries; } /** @@ -237,4 +251,57 @@ public function getConsentSettings() return $this->consentSettings; } + + /** + * @return array + */ + public function getDiscoveries(): array + { + $this->ensureDiscoveriesDeserialized(); + return $this->discoveries; + } + + /** + * @param array $discoveries + */ + public function setDiscoveries(array $discoveries) + { + $this->assertAllDiscoveries($discoveries); + $this->discoveries = $discoveries; + } + + private function ensureDiscoveriesDeserialized(): void + { + if (!is_array($this->discoveries)) { + $this->discoveries = []; + return; + } + + foreach ($this->discoveries as $index => $discovery) { + try { + if (!$discovery instanceof Discovery) { + $logo = new Logo($discovery['logo']['url']); + $logo->width = $discovery['logo']['width']; + $logo->height = $discovery['logo']['height']; + + $this->discoveries[$index] = Discovery::create( + $discovery['names'] ?? [], + $discovery['keywords'] ?? [], + $logo + ); + } + } catch (InvalidDiscoveryException $e) { + unset($this->discoveries[$index]); + } + } + } + + private function assertAllDiscoveries(array $discoveries): void + { + foreach ($discoveries as $discovery) { + if (!$discovery instanceof Discovery) { + throw new InvalidArgumentException('Discovery must be instance of Discovery'); + } + } + } } diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/IdentityProviderEntity.php b/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/IdentityProviderEntity.php index 69241ccb3c..01fa458ac8 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/IdentityProviderEntity.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/Adapter/IdentityProviderEntity.php @@ -260,4 +260,9 @@ public function getMdui(): Mdui { return $this->entity->getMdui(); } + + public function getDiscoveries(): array + { + return $this->entity->getDiscoveries(); + } } diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractIdentityProvider.php b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractIdentityProvider.php index 565cd0b8b6..5b662b4f99 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractIdentityProvider.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/Decorator/AbstractIdentityProvider.php @@ -239,4 +239,9 @@ public function getMdui(): Mdui { return $this->entity->getMdui(); } + + public function getDiscoveries(): array + { + return $this->entity->getDiscoveries(); + } } diff --git a/src/OpenConext/EngineBlock/Metadata/Factory/IdentityProviderEntityInterface.php b/src/OpenConext/EngineBlock/Metadata/Factory/IdentityProviderEntityInterface.php index 713a5832fb..52c17e332c 100644 --- a/src/OpenConext/EngineBlock/Metadata/Factory/IdentityProviderEntityInterface.php +++ b/src/OpenConext/EngineBlock/Metadata/Factory/IdentityProviderEntityInterface.php @@ -148,4 +148,6 @@ public function getConsentSettings(): ConsentSettings; public function getShibMdScopes(): array; public function getMdui(): Mdui; + + public function getDiscoveries(): array; } diff --git a/src/OpenConext/EngineBlockBundle/Controller/WayfController.php b/src/OpenConext/EngineBlockBundle/Controller/WayfController.php index de271ef5c8..8cb5e77b1d 100644 --- a/src/OpenConext/EngineBlockBundle/Controller/WayfController.php +++ b/src/OpenConext/EngineBlockBundle/Controller/WayfController.php @@ -22,6 +22,9 @@ use EngineBlock_Corto_Adapter; use OpenConext\EngineBlock\Service\SsoSessionService; use OpenConext\EngineBlockBridge\ResponseFactory; +use OpenConext\EngineBlockBundle\Service\DiscoverySelectionService; +use Psr\Log\LoggerInterface; +use Psr\Log\LogLevel; use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Request; use Twig_Environment; @@ -40,6 +43,15 @@ class WayfController * @var SsoSessionService */ private $sessionService; + /** + * @var DiscoverySelectionService + */ + private $discoverySelectionService; + + /** + * @var LoggerInterface + */ + private $logger; /** * @param EngineBlock_ApplicationSingleton $engineBlockApplicationSingleton @@ -49,22 +61,43 @@ class WayfController public function __construct( EngineBlock_ApplicationSingleton $engineBlockApplicationSingleton, Twig_Environment $twig, - SsoSessionService $sessionService + SsoSessionService $sessionService, + DiscoverySelectionService $discoverySelectionService, + LoggerInterface $logger ) { $this->engineBlockApplicationSingleton = $engineBlockApplicationSingleton; $this->twig = $twig; $this->sessionService = $sessionService; + $this->discoverySelectionService = $discoverySelectionService; + $this->logger = $logger; } /** * @return \Symfony\Component\HttpFoundation\RedirectResponse|Response */ - public function processWayfAction() + public function processWayfAction(Request $request) { $proxyServer = new EngineBlock_Corto_Adapter(); $proxyServer->processWayf(); - return ResponseFactory::fromEngineBlockResponse($this->engineBlockApplicationSingleton->getHttpResponse()); + $response = ResponseFactory::fromEngineBlockResponse($this->engineBlockApplicationSingleton->getHttpResponse()); + + $session = $request->getSession(); + if ($session === null) { + $this->logger->log(LogLevel::ERROR, 'Could not handle discovery override, no session available!'); + return $response; + } + + if ($request->request->get(DiscoverySelectionService::USED_DISCOVERY_HASH_PARAM, '') !== '') { + $this->discoverySelectionService->registerDiscoveryHash( + $session, + $request->request->get(DiscoverySelectionService::USED_DISCOVERY_HASH_PARAM) + ); + } else { + $this->discoverySelectionService->clearDiscoveryHash($session); + } + + return $response; } /** diff --git a/src/OpenConext/EngineBlockBundle/EventListener/RedirectToFeedbackPageExceptionListener.php b/src/OpenConext/EngineBlockBundle/EventListener/RedirectToFeedbackPageExceptionListener.php index aa17af6f18..16effd1f0a 100644 --- a/src/OpenConext/EngineBlockBundle/EventListener/RedirectToFeedbackPageExceptionListener.php +++ b/src/OpenConext/EngineBlockBundle/EventListener/RedirectToFeedbackPageExceptionListener.php @@ -205,13 +205,13 @@ public function onKernelException(GetResponseForExceptionEvent $event) } elseif ($exception instanceof EngineBlock_Corto_Exception_UserCancelledStepupCallout) { $message = $exception->getMessage(); $redirectToRoute = 'authentication_feedback_stepup_callout_user_cancelled'; - } else if ($exception instanceof EngineBlock_Corto_Exception_InvalidStepupLoaLevel) { + } elseif ($exception instanceof EngineBlock_Corto_Exception_InvalidStepupLoaLevel) { $message = $exception->getMessage(); $redirectToRoute = 'authentication_feedback_stepup_callout_unmet_loa'; - } else if ($exception instanceof EngineBlock_Corto_Exception_InvalidStepupCalloutResponse) { + } elseif ($exception instanceof EngineBlock_Corto_Exception_InvalidStepupCalloutResponse) { $message = $exception->getMessage(); $redirectToRoute = 'authentication_feedback_stepup_callout_unknown'; - } else if ($exception instanceof EntityCanNotBeFoundException) { + } elseif ($exception instanceof EntityCanNotBeFoundException) { $event->getRequest()->getSession()->set('feedback_custom', $exception->getMessage()); $redirectToRoute = 'authentication_feedback_metadata_entity_not_found'; } else { diff --git a/src/OpenConext/EngineBlockBundle/Resources/config/controllers/authentication.yml b/src/OpenConext/EngineBlockBundle/Resources/config/controllers/authentication.yml index c4379fb05a..69c9a3930d 100644 --- a/src/OpenConext/EngineBlockBundle/Resources/config/controllers/authentication.yml +++ b/src/OpenConext/EngineBlockBundle/Resources/config/controllers/authentication.yml @@ -60,6 +60,8 @@ services: - "@engineblock.compat.application" - "@twig" - "@engineblock.service.sso_session" + - "@engineblock.service.discovery_selection_service" + - "@engineblock.compat.logger" engineblock.controller.authentication.proxy: class: OpenConext\EngineBlockBundle\Controller\ProxyController diff --git a/src/OpenConext/EngineBlockBundle/Resources/config/services.yml b/src/OpenConext/EngineBlockBundle/Resources/config/services.yml index e2b0c5ac70..c3668506b3 100644 --- a/src/OpenConext/EngineBlockBundle/Resources/config/services.yml +++ b/src/OpenConext/EngineBlockBundle/Resources/config/services.yml @@ -291,6 +291,7 @@ services: class: OpenConext\EngineBlock\Metadata\Entity\Assembler\PushMetadataAssembler arguments: - "@engineblock.validator.allowed_scheme_validator" + - "@engineblock.language_support_provider" - "@engineblock.compat.logger" OpenConext\EngineBlock\Metadata\X509\KeyPairFactory: @@ -377,3 +378,6 @@ services: - "@translator" tags: - { name: 'twig.extension' } + + engineblock.service.discovery_selection_service: + class: OpenConext\EngineBlockBundle\Service\DiscoverySelectionService diff --git a/src/OpenConext/EngineBlockBundle/Service/DiscoverySelectionService.php b/src/OpenConext/EngineBlockBundle/Service/DiscoverySelectionService.php new file mode 100644 index 0000000000..c886bd9fb5 --- /dev/null +++ b/src/OpenConext/EngineBlockBundle/Service/DiscoverySelectionService.php @@ -0,0 +1,72 @@ +hash($discovery) === $hash; + } + + public function getDiscoveryFromRequest(SessionInterface $session, IdentityProvider $identityProvider): ?Discovery + { + $storedDiscoveryHash = $this->loadSelectedDiscoveryHashFromSession($session); + foreach ($identityProvider->getDiscoveries() as $discovery) { + if ($this->discoveryMatchesHash($discovery, $storedDiscoveryHash)) { + return $discovery; + } + } + return null; + } + + public function hash(Discovery $discovery): string + { + $string = json_encode($discovery); + return hash('sha256', $string); + } + + public function registerDiscoveryHash(SessionInterface $session, string $hash): void + { + $session->set(self::PREVIOUSLY_SELECTED_DISCOVERY_SESSION_NAME, $hash); + } + + public function clearDiscoveryHash(SessionInterface $session): void + { + $session->remove(self::PREVIOUSLY_SELECTED_DISCOVERY_SESSION_NAME); + } + + private function loadSelectedDiscoveryHashFromSession(SessionInterface $session): string + { + $previousSelection = $session->get(self::PREVIOUSLY_SELECTED_DISCOVERY_SESSION_NAME, false); + + if ($previousSelection) { + return $previousSelection; + } + + return ''; + } +} diff --git a/src/OpenConext/EngineBlockBundle/Service/IdpHistoryService.php b/src/OpenConext/EngineBlockBundle/Service/IdpHistoryService.php new file mode 100644 index 0000000000..e15ac1bfc5 --- /dev/null +++ b/src/OpenConext/EngineBlockBundle/Service/IdpHistoryService.php @@ -0,0 +1,27 @@ +previousSelection)) { - foreach ($this->previousSelection as $idp) { - $previousSelectionIndex[$idp['idp']] = $idp; - } - } + /** + * REVERT CHANGES??? + */ + public function getConnectedIdps(array $idpList, string $locale): ConnectedIdps + { + $previousSelectionIndex = $this->indexPreviousSelection(); - $formattedPreviousSelectionList = []; - $formattedIdpList = []; - foreach ($idpList as $idp) { - $idpKeywords = $idp['Keywords'] === 'Undefined' ? array() : array_values((array)$idp['Keywords']); + $formattedIdpList = $this->formatIdpList($idpList); + $previousSelected = $this->filterPreviouslySelected( + $formattedIdpList, + $previousSelectionIndex + ); - if (isset($previousSelectionIndex[$idp['EntityID']])) { - $formattedPreviousSelectionList[] = array_merge( - $previousSelectionIndex[$idp['EntityID']], - [ - 'entityId' => $idp['EntityID'], - 'connected' => $idp['Access'] === '1', - 'displayTitle' => $idp['Name'], - 'title' => strtolower($idp['Name']), - 'keywords' => strtolower(join('|', $idpKeywords)), - 'logo' => $idp['Logo'], - 'isDefaultIdp' => (bool) $idp['isDefaultIdp'], - ] - ); - } + return new ConnectedIdps($previousSelected, $formattedIdpList); + } - $formattedIdpList[] = [ - 'entityId' => $idp['EntityID'], - 'connected' => $idp['Access'] === '1', - 'displayTitle' => $idp['Name'], - 'title' => strtolower($idp['Name']), - 'keywords' => strtolower(join('|', $idpKeywords)), - 'logo' => $idp['Logo'], - 'isDefaultIdp' => (bool) $idp['isDefaultIdp'], - ]; + /** + * Create an index of previous selections by IDP identifier + * + * @return array> + */ + private function indexPreviousSelection(): array + { + if (empty($this->previousSelection)) { + return []; } - return new ConnectedIdps($formattedPreviousSelectionList, $formattedIdpList); + return array_column($this->previousSelection, null, 'idp'); + } + + /** + * Format a single IDP entry + * + * @param array $idp + * @return array + */ + private function formatIdpEntry(array $idp): array + { + $keywords = $idp['Keywords'] === 'Undefined' + ? [] + : array_values((array)$idp['Keywords']); + + // In SingleSignOn.php, the IDP is transformed into an array for the frontend + // Then, here, the array is transformed into another array for the frontend which is actually used in twig + return [ + 'entityId' => $idp['EntityID'], + 'connected' => $idp['Access'] === self::ACCESS_ENABLED, + 'displayTitle' => $idp['Name'], + 'title' => strtolower($idp['Name']), + 'keywords' => strtolower(implode('|', $keywords)), + 'logo' => $idp['Logo'], + 'isDefaultIdp' => (bool)$idp['isDefaultIdp'], + 'discoveryHash' => $idp['DiscoveryHash'] + ]; + } + + private function formatIdpList(array $idpList): array + { + return array_map( + function (array $idp) { + return $this->formatIdpEntry($idp); + }, + $idpList + ); + } + + private function filterPreviouslySelected( + array $formattedList, + array $previousSelectionIndex + ): array { + return array_filter( + array_map( + function (array $idp) use ($previousSelectionIndex) { + $entryKey = $this->entryKey($idp['entityId'], $idp['discoveryHash']); + if (!isset($previousSelectionIndex[$entryKey])) { + return null; + } + return array_merge( + $previousSelectionIndex[$entryKey], + $idp + ); + }, + $formattedList + ) + ); } /** @@ -188,4 +236,9 @@ private function loadPreviousSelectionFromCookie(RequestStack $requestStack) } return $previousSelectionIndexed; } + + public function entryKey(string $entityId, ?string $discoveryHash = null): string + { + return (new IdpHistoryService())->makeEntryKey($entityId, $discoveryHash); + } } diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/WayfController.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/WayfController.php index 96f78787af..349ed7cc3d 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/WayfController.php +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/WayfController.php @@ -44,6 +44,7 @@ public function wayfAction(Request $request) $request->cookies->set('lang', $currentLocale); $backLink = (bool) $request->get('backLink', false); $displayUnconnectedIdpsWayf = (bool) $request->get('displayUnconnectedIdpsWayf', false); + $addDiscoveries = (bool) $request->get('addDiscoveries', true); $rememberChoiceFeature = (bool) $request->get('rememberChoiceFeature', false); $cutoffPointForShowingUnfilteredIdps = $request->get('cutoffPointForShowingUnfilteredIdps', 100); $showIdPBanner = $request->get('showIdPBanner', true); @@ -56,7 +57,7 @@ public function wayfAction(Request $request) $randomIdps = (int) $request->get('randomIdps', 0); $idpList = $randomIdps === 0 - ? TestEntitySeeder::buildIdps($connectedIdps, $unconnectedIdps, $currentLocale, $defaultIdpEntityId) + ? TestEntitySeeder::buildIdps($connectedIdps, $unconnectedIdps, $currentLocale, $defaultIdpEntityId, $addDiscoveries) : TestEntitySeeder::buildRandomIdps($randomIdps, $currentLocale, $defaultIdpEntityId); return new Response($this->twig->render( diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Helper/TestEntitySeeder.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Helper/TestEntitySeeder.php index 1769e8188c..db2b196ba9 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Helper/TestEntitySeeder.php +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Helper/TestEntitySeeder.php @@ -22,6 +22,8 @@ use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider; use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider; use OpenConext\EngineBlock\Metadata\Logo; +use OpenConext\EngineBlock\Metadata\Discovery; +use OpenConext\EngineBlockBundle\Service\DiscoverySelectionService; use Webmozart\Assert\Assert; class TestEntitySeeder @@ -32,12 +34,13 @@ class TestEntitySeeder * This is not an array of IdentityProvider value objects, but a derivative that can be used for showing IdPs on * the WAYF. * + * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @param int $numberOfIdps * @param int $numberOfUnconnectedIdps * @param string $locale * @return array[] */ - public static function buildIdps($numberOfIdps, $numberOfUnconnectedIdps, $locale, $defaultIdpEntityId) + public static function buildIdps($numberOfIdps, $numberOfUnconnectedIdps, $locale, $defaultIdpEntityId, bool $addDiscoveries) { Assert::integer($numberOfIdps); Assert::integer($numberOfUnconnectedIdps); @@ -47,6 +50,7 @@ public static function buildIdps($numberOfIdps, $numberOfUnconnectedIdps, $local throw new LogicException('The number of IdPs that are to be created should be greater or equal to the number of unconnected IdPs'); } + // Discoveries $idps = []; for ($i=1; $i < (int) ($numberOfIdps - $numberOfUnconnectedIdps) + 1; $i++) { @@ -56,7 +60,29 @@ public static function buildIdps($numberOfIdps, $numberOfUnconnectedIdps, $local if ($defaultIdpEntityId === $entityId) { $isDefaultIdp = true; } - $idps[$entityId] = ['name' => $name, 'enabled' => true, 'isDefaultIdp' => $isDefaultIdp]; + + $discoveries = []; + if ($addDiscoveries && $i === 1) { + $discoveries = [ + Discovery::create( + ['en' => 'National University of the Netherlands', 'nl' => 'Rijksuniversiteit der Nederlanden'], + ['en' => 'royal', 'nl' => 'koninklijke'], + new Logo('/images/logo.png') + ), + Discovery::create( + ['en' => 'Foreign embassy of the Republic'], + [], + null + ) + ]; + } + + $idps[$entityId] = [ + 'name' => $name, + 'enabled' => true, + 'isDefaultIdp' => $isDefaultIdp, + 'discoveries' => $discoveries, + ]; } if ($numberOfUnconnectedIdps > 0) { @@ -67,7 +93,24 @@ public static function buildIdps($numberOfIdps, $numberOfUnconnectedIdps, $local if ($defaultIdpEntityId === $entityId) { $isDefaultIdp = true; } - $idps[$entityId] = ['name' => $name, 'enabled' => false, 'isDefaultIdp' => $isDefaultIdp]; + + $discoveries = []; + if ($addDiscoveries && $i === 1) { + $discoveries = [ + Discovery::create( + ['en' => 'Disconnected National University of the Netherlands', 'nl' => 'Disconnected Rijksuniversiteit der Nederlanden'], + ['en' => 'Disconnected royal', 'nl' => 'Disconnected koninklijke'], + new Logo('/images/logo.png') + ), + Discovery::create( + ['en' => 'Disconnected Foreign embassy of the Republic'], + [], + null + ) + ]; + } + + $idps[$entityId] = ['name' => $name, 'enabled' => false, 'isDefaultIdp' => $isDefaultIdp, 'discoveries' => $discoveries]; } } @@ -149,6 +192,7 @@ public static function buildRandomIdps($numberOfIdps, $locale, $defaultIdpEntity */ private static function transformIdpsForWayf(array $idpEntityIds, $currentLocale) { + $discoveryService = new DiscoverySelectionService(); $identityProviders = self::findIdentityProvidersByEntityId($idpEntityIds); $wayfIdps = array(); @@ -161,9 +205,22 @@ private static function transformIdpsForWayf(array $idpEntityIds, $currentLocale 'Access' => ($identityProvider->enabledInWayf) ? '1' : '0', 'ID' => md5($identityProvider->entityId), 'EntityID' => $identityProvider->entityId, - 'isDefaultIdp' => $idpEntityIds[$identityProvider->entityId]['isDefaultIdp'] + 'isDefaultIdp' => $idpEntityIds[$identityProvider->entityId]['isDefaultIdp'], ); $wayfIdps[] = $wayfIdp; + + foreach ($identityProvider->getDiscoveries() as $discovery) { + $wayfIdps[] = array( + 'Name' => $discovery->getName($currentLocale), + 'Logo' => $discovery->getLogo() ? $discovery->getLogo()->url : '/images/placeholder.png', + 'Keywords' => $discovery->getKeywords('en'), + 'Access' => ($identityProvider->enabledInWayf) ? '1' : '0', + 'ID' => md5($identityProvider->entityId), + 'EntityID' => $identityProvider->entityId, + 'isDefaultIdp' => $idpEntityIds[$identityProvider->entityId]['isDefaultIdp'], + 'DiscoveryHash' => $discoveryService->hash($discovery), + ); + } } $nameSort = function ($a, $b) { @@ -176,7 +233,10 @@ private static function transformIdpsForWayf(array $idpEntityIds, $currentLocale return $wayfIdps; } - private static function findIdentityProvidersByEntityId(array $idpEntityIds) + /** + * @return IdentityProvider[] + */ + private static function findIdentityProvidersByEntityId(array $idpEntityIds): array { $idps = []; foreach ($idpEntityIds as $idpEntityId => $idpData) { @@ -187,6 +247,7 @@ private static function findIdentityProvidersByEntityId(array $idpEntityIds) $idp->namePt = $idpData['name']; $idp->keywordsEn = ['Awesome IdP', 'Another keyword', 'Example']; $idp->enabledInWayf = $idpData['enabled']; + $idp->setDiscoveries($idpData['discoveries'] ?? []); $idps[] = $idp; } diff --git a/tests/e2e/cypress/integration/openconext/wayf/WayfShowsConnectedIdps.spec.js b/tests/e2e/cypress/integration/openconext/wayf/WayfShowsConnectedIdps.spec.js index ca478c2734..b752c417c2 100644 --- a/tests/e2e/cypress/integration/openconext/wayf/WayfShowsConnectedIdps.spec.js +++ b/tests/e2e/cypress/integration/openconext/wayf/WayfShowsConnectedIdps.spec.js @@ -4,7 +4,7 @@ context('WayfMouseBehaviour', () => { cy.visit('https://engine.dev.openconext.local/functional-testing/wayf'); // Load the connected IdPs by selecting their h3 titles - cy.countIdps(5) + cy.countIdps(7) .eq(2) .should('have.text', 'Connected IdP 3 en'); @@ -23,7 +23,7 @@ context('WayfMouseBehaviour', () => { }); it('Should show ten connected IdPs', () => { - cy.visit('https://engine.dev.openconext.local/functional-testing/wayf?connectedIdps=10'); + cy.visit('https://engine.dev.openconext.local/functional-testing/wayf?connectedIdps=10&addDiscoveries=0'); cy.countIdps(10); }); diff --git a/tests/e2e/cypress/integration/skeune/wayf/wayf.general.spec.js b/tests/e2e/cypress/integration/skeune/wayf/wayf.general.spec.js index a7c4432c70..5165d75973 100644 --- a/tests/e2e/cypress/integration/skeune/wayf/wayf.general.spec.js +++ b/tests/e2e/cypress/integration/skeune/wayf/wayf.general.spec.js @@ -87,6 +87,14 @@ context('WAYF behaviour not tied to mouse / keyboard navigation', () => { .should('contain.text', 'Connected IdP 4 en'); }); + it('Should be able to find IDP by entering keyword for discovery idp', () => { + cy.visit('https://engine.dev.openconext.local/functional-testing/wayf'); + cy.get(searchFieldSelector).type('royal'); + cy.get(matchSelector) + .should('have.length', 2) + .should('contain.text', 'National University of the Netherlands'); + }); + it('Should get the correct weight for an idp with a full match on the title', () => { cy.visit('https://engine.dev.openconext.local/functional-testing/wayf?connectedIdps=50'); cy.get(searchFieldSelector).type('Connected Idp 4 en'); diff --git a/tests/e2e/cypress/integration/skeune/wayf/wayf.keyboard.spec.js b/tests/e2e/cypress/integration/skeune/wayf/wayf.keyboard.spec.js index b9894be02c..a09e3b404e 100644 --- a/tests/e2e/cypress/integration/skeune/wayf/wayf.keyboard.spec.js +++ b/tests/e2e/cypress/integration/skeune/wayf/wayf.keyboard.spec.js @@ -72,6 +72,8 @@ context('WAYF when using the keyboard', () => { cy.pressArrowOnIdpList('down', idpClass, '3'); cy.pressArrowOnIdpList('down', idpClass, '4'); cy.pressArrowOnIdpList('down', idpClass, '5'); + cy.pressArrowOnIdpList('down', idpClass, '6'); + cy.pressArrowOnIdpList('down', idpClass, '7'); cy.pressArrowOnIdpList('down', searchFieldClass); }); @@ -79,6 +81,8 @@ context('WAYF when using the keyboard', () => { cy.visit('https://engine.dev.openconext.local/functional-testing/wayf?showIdpBanner=1'); cy.get(searchFieldSelector).focus(); cy.pressArrowOnIdpList('up', searchFieldClass); + cy.pressArrowOnIdpList('up', idpClass, '7'); + cy.pressArrowOnIdpList('up', idpClass, '6'); cy.pressArrowOnIdpList('up', idpClass, '5'); cy.pressArrowOnIdpList('up', idpClass, '4'); cy.pressArrowOnIdpList('up', idpClass, '3'); diff --git a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssemblerTest.php b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssemblerTest.php new file mode 100644 index 0000000000..9d633195c3 --- /dev/null +++ b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssemblerTest.php @@ -0,0 +1,176 @@ +createMock(LanguageSupportProvider::class); + $languageSupportProvider + ->method('getSupportedLanguages') + ->willReturn(['en', 'fr', 'de']); + + $this->discoveryAssembler = new DiscoveryAssembler($languageSupportProvider); + } + + public function testAssembleDiscoveriesReturnsEmptyArrayWhenNoDiscoveries(): void + { + $connection = new stdClass(); + $connection->metadata = new stdClass(); + + $result = $this->discoveryAssembler->assembleDiscoveries($connection); + + $this->assertEquals([], $result); + } + + public function testAssembleDiscoveriesProcessesValidDiscovery(): void + { + $connection = new stdClass(); + $connection->metadata = new stdClass(); + $connection->metadata->discoveries = [ + $this->createDiscoveryObject([ + 'name_en' => 'Test Discovery', + 'name_fr' => 'Découverte Test', + 'keywords_en' => 'test, discovery', + 'keywords_fr' => 'test, découverte', + 'logo_url' => 'http://example.com/logo.png', + 'logo_width' => 100, + 'logo_height' => 100 + ]) + ]; + + $result = $this->discoveryAssembler->assembleDiscoveries($connection); + + $this->assertArrayHasKey('discoveries', $result); + $this->assertCount(1, $result['discoveries']); + $this->assertInstanceOf(Discovery::class, $result['discoveries'][0]); + + /** @var Discovery $discovery */ + $discovery = $result['discoveries'][0]; + $this->assertEquals('Test Discovery', $discovery->getName('en')); + $this->assertEquals('Découverte Test', $discovery->getName('fr')); + $this->assertEquals('test, discovery', $discovery->getKeywords('en')); + $this->assertEquals('test, découverte', $discovery->getKeywords('fr')); + + $this->assertInstanceOf(Logo::class, $discovery->getLogo()); + $this->assertEquals('http://example.com/logo.png', $discovery->getLogo()->url); + $this->assertEquals(100, $discovery->getLogo()->width); + $this->assertEquals(100, $discovery->getLogo()->height); + } + + public function testAssembleDiscoveriesSkipsDiscoveryWithoutEnglishName(): void + { + $connection = new stdClass(); + $connection->metadata = new stdClass(); + $connection->metadata->discoveries = [ + $this->createDiscoveryObject([ + 'name_fr' => 'Découverte Test', + 'keywords_fr' => 'test, découverte' + ]) + ]; + + $result = $this->discoveryAssembler->assembleDiscoveries($connection); + + $this->assertEquals([], $result); + } + + public function testAssembleDiscoveriesWithTrimmedValues(): void + { + $connection = new stdClass(); + $connection->metadata = new stdClass(); + $connection->metadata->discoveries = [ + $this->createDiscoveryObject([ + 'name_en' => ' Test Discovery ', + 'name_fr' => ' Découverte Test ', + 'keywords_en' => ' test, discovery ', + 'keywords_fr' => ' test, découverte ' + ]) + ]; + + $result = $this->discoveryAssembler->assembleDiscoveries($connection); + + $this->assertArrayHasKey('discoveries', $result); + /** @var Discovery $discovery */ + $discovery = $result['discoveries'][0]; + $this->assertEquals('Test Discovery', $discovery->getName('en')); + $this->assertEquals('Découverte Test', $discovery->getName('fr')); + $this->assertEquals('test, discovery', $discovery->getKeywords('en')); + $this->assertEquals('test, découverte', $discovery->getKeywords('fr')); + } + + public function testAssembleDiscoveriesWithEmptyLogoUrl(): void + { + $connection = new stdClass(); + $connection->metadata = new stdClass(); + $connection->metadata->discoveries = [ + $this->createDiscoveryObject([ + 'name_en' => 'Test Discovery', + 'logo_url' => '', + 'logo_width' => 100, + 'logo_height' => 100 + ]) + ]; + + $result = $this->discoveryAssembler->assembleDiscoveries($connection); + + $this->assertArrayHasKey('discoveries', $result); + $this->assertFalse($result['discoveries'][0]->hasLogo()); + } + + public function testAssembleDiscoveriesWithLogoWithoutDimensions(): void + { + $connection = new stdClass(); + $connection->metadata = new stdClass(); + $connection->metadata->discoveries = [ + $this->createDiscoveryObject([ + 'name_en' => 'Test Discovery', + 'logo_url' => 'http://example.com/logo.png' + ]) + ]; + + $result = $this->discoveryAssembler->assembleDiscoveries($connection); + + $this->assertArrayHasKey('discoveries', $result); + /** @var Discovery $discovery */ + $discovery = $result['discoveries'][0]; + $this->assertInstanceOf(Logo::class, $discovery->getLogo()); + $this->assertEquals('http://example.com/logo.png', $discovery->getLogo()->url); + $this->assertNull($discovery->getLogo()->width); + $this->assertNull($discovery->getLogo()->height); + } + + private function createDiscoveryObject(array $properties): stdClass + { + $discovery = new stdClass(); + foreach ($properties as $key => $value) { + $discovery->$key = $value; + } + return $discovery; + } +} diff --git a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php index 8f25e8211c..22eeea1006 100644 --- a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php +++ b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php @@ -20,13 +20,16 @@ use Mockery as m; use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration; +use OpenConext\EngineBlock\Metadata\Discovery; use OpenConext\EngineBlock\Metadata\Entity\Assembler\PushMetadataAssembler; use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider; use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider; +use OpenConext\EngineBlock\Metadata\Logo; use OpenConext\EngineBlock\Metadata\MfaEntityCollection; use OpenConext\EngineBlock\Metadata\StepupConnections; use OpenConext\EngineBlock\Metadata\TransparentMfaEntity; use OpenConext\EngineBlock\Validator\AllowedSchemeValidator; +use OpenConext\EngineBlockBundle\Localization\LanguageSupportProvider; use PHPUnit\Framework\TestCase; use RuntimeException; use Psr\Log\LoggerInterface; @@ -40,7 +43,11 @@ class PushMetadataAssemblerTest extends TestCase public function setUp(): void { $logger = m::mock(LoggerInterface::class); - $this->assembler = new PushMetadataAssembler(new AllowedSchemeValidator(['http', 'https']), $logger); + $this->assembler = new PushMetadataAssembler( + new AllowedSchemeValidator(['http', 'https']), + new LanguageSupportProvider(['en', 'nl'], ['en', 'nl']), + $logger + ); } public function test_it_rejects_empty_push() @@ -465,4 +472,60 @@ private function validCoinValuesBoolForceAuthn() ["0", false] ]; } + + public function test_it_assembles_idp_details() + { + $input = $this->readFixture('metadata_idp_display_data.json'); + $connections = $this->assembler->assemble($input); + + /** @var IdentityProvider $idp */ + $idp = $connections[0]; + $this->assertInstanceOf(IdentityProvider::class, $idp); + + $this->assertSame('Dummy IdP', $idp->nameEn); + $this->assertSame('Dummy IdP nl', $idp->nameNl); + + $this->assertSame('idp keywords', $idp->keywordsEn); + $this->assertSame('idp keywords nl', $idp->keywordsNl); + $this->assertSame('idp keywords', $idp->getMdui()->getKeywords('en')); + $this->assertSame('idp keywords nl', $idp->getMdui()->getKeywords('nl')); + + $this->assertSame('https://engine.dev.openconext.local/images/logo.png?v=dev', $idp->logo->url); + $this->assertSame('https://engine.dev.openconext.local/images/logo.png?v=dev', $idp->getMdui()->getLogo()->url); + + $expectedDiscoveryLogo = new Logo('https://engine.dev.openconext.local/images/discovery_logo.png?v=dev'); + $expectedDiscoveryLogo->height = 96; + $expectedDiscoveryLogo->width = 128; + + $this->assertEquals( + [ + Discovery::create( + ['en' => 'Discovery #1', 'nl' => 'Discovery #1 nl'], + ['en' => 'disco keywords, (test)', 'nl' => 'disco keywords, nl'], + $expectedDiscoveryLogo + ), + Discovery::create(['en' => 'Minimal discovery'], [], null), + ], + $idp->getDiscoveries() + ); + } + + public function test_it_assembles_metadata_without_discoveries_details() + { + $input = $this->readFixture('metadata_without_discoveries.json'); + $connections = $this->assembler->assemble($input); + + /** @var IdentityProvider $idp */ + $idp = $connections[0]; + $this->assertInstanceOf(IdentityProvider::class, $idp); + + $this->assertSame('Dummy IdP', $idp->nameEn); + $this->assertSame('idp keywords', $idp->getMdui()->getKeywords('en')); + $this->assertSame('https://engine.dev.openconext.local/images/logo.png?v=dev', $idp->logo->url); + } + + private function readFixture(string $file): object + { + return json_decode(file_get_contents(__DIR__ . '/fixtures/'. basename($file)), false); + } } diff --git a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/fixtures/metadata_idp_display_data.json b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/fixtures/metadata_idp_display_data.json new file mode 100644 index 0000000000..3514319897 --- /dev/null +++ b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/fixtures/metadata_idp_display_data.json @@ -0,0 +1,62 @@ +{ + "2d96e27a-76cf-4ca2-ac70-ece5d4c49524": { + "stepup_connections": [ + { + "name": "https://serviceregistry.test2.openconext.nl/simplesaml/module.php/saml/sp/metadata.php/default-sp", + "level": "http://test.openconext.nl/assurance/loa2" + }, + { + "name": "http://mock-sp", + "level": "http://test.openconext.nl/assurance/loa3" + } + ], + "name": "https:\/\/serviceregistry.dev.openconext.local\/simplesaml\/module.php\/saml\/sp\/metadata.php\/default-idp", + "state": "prodaccepted", + "type": "saml20-idp", + "metadata": { + "discoveries": [ + { + "logo_url": "https://engine.dev.openconext.local/images/discovery_logo.png?v=dev", + "name_en": "Discovery #1", + "name_nl": "Discovery #1 nl", + "keywords_en": "disco keywords, (test)", + "keywords_nl": "disco keywords, nl", + "logo_height": "96", + "logo_width": "128" + }, + { + "name_en": "Minimal discovery" + } + ], + "OrganizationName": { + "en": "OpenConext DEV" + }, + "SingleSignOnService": [ + { + "Binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", + "Location": "https://mujina-idp.dev.openConext.local/SingleSignOnService" + } + ], + "certData": "MIIDEzCCAfugAwIBAgIJAKoK/heBjcOYMA0GCSqGSIb3DQEBBQUAMCAxHjAcBgNVBAoMFU9yZ2FuaXphdGlvbiwgQ049T0lEQzAeFw0xNTExMTExMDEyMTVaFw0yNTExMTAxMDEyMTVaMCAxHjAcBgNVBAoMFU9yZ2FuaXphdGlvbiwgQ049T0lEQzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANBGwJ/qpTQNiSgUglSE2UzEkUow+wS8r67etxoEhlzJZfgK/k5TfG1wICDqapHAxEVgUM10aBHRctNocA5wmlHtxdidhzRZroqHwpKy2BmsKX5Z2oK25RLpsyusB1KroemgA/CjUnI6rIL1xxFn3KyOFh1ZBLUQtKNQeMS7HFGgSDAp+sXuTFujz12LFDugX0T0KB5a1+0l8y0PEa0yGa1oi6seONx849ZHxM0PRvUunWkuTM+foZ0jZpFapXe02yWMqhc/2iYMieE/3GvOguJchJt6R+cut8VBb6ubKUIGK7pmoq/TB6DVXpvsHqsDJXechxcicu4pdKVDHSec850CAwEAAaNQME4wHQYDVR0OBBYEFK7RqjoodSYVXGTVEdLf3kJflP/sMB8GA1UdIwQYMBaAFK7RqjoodSYVXGTVEdLf3kJflP/sMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADggEBADNZkxlFXh4F45muCbnQd+WmaXlGvb9tkUyAIxVL8AIu8J18F420vpnGpoUAE+Hy3evBmp2nkrFAgmr055fAjpHeZFgDZBAPCwYd3TNMDeSyMta3Ka+oS7GRFDePkMEm+kH4/rITNKUF1sOvWBTSowk9TudEDyFqgGntcdu/l/zRxvx33y3LMG5USD0x4X4IKjRrRN1BbcKgi8dq10C3jdqNancTuPoqT3WWzRvVtB/q34B7F74/6JzgEoOCEHufBMp4ZFu54P0yEGtWfTwTzuoZobrChVVBt4w/XZagrRtUCDNwRpHNbpjxYudbqLqpi1MQpV9oht/BpTHVJG2i0ro=", + "coin": { + "guest_qualifier": "None" + }, + "description": { + "en": "Dummy IDP" + }, + "logo": [ + { + "url": "https://engine.dev.openconext.local/images/logo.png?v=dev" + } + ], + "name": { + "en": "Dummy IdP", + "nl": "Dummy IdP nl" + }, + "keywords": { + "en": "idp keywords", + "nl": "idp keywords nl" + } + } + } +} diff --git a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/fixtures/metadata_without_discoveries.json b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/fixtures/metadata_without_discoveries.json new file mode 100644 index 0000000000..c2af8b2404 --- /dev/null +++ b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/fixtures/metadata_without_discoveries.json @@ -0,0 +1,46 @@ +{ + "2d96e27a-76cf-4ca2-ac70-ece5d4c49524": { + "stepup_connections": [ + { + "name": "https://serviceregistry.test2.openconext.nl/simplesaml/module.php/saml/sp/metadata.php/default-sp", + "level": "http://test.openconext.nl/assurance/loa2" + }, + { + "name": "http://mock-sp", + "level": "http://test.openconext.nl/assurance/loa3" + } + ], + "name": "https:\/\/serviceregistry.dev.openconext.local\/simplesaml\/module.php\/saml\/sp\/metadata.php\/default-idp", + "state": "prodaccepted", + "type": "saml20-idp", + "metadata": { + "OrganizationName": { + "en": "OpenConext DEV" + }, + "SingleSignOnService": [ + { + "Binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", + "Location": "https://mujina-idp.dev.openConext.local/SingleSignOnService" + } + ], + "certData": "MIIDEzCCAfugAwIBAgIJAKoK/heBjcOYMA0GCSqGSIb3DQEBBQUAMCAxHjAcBgNVBAoMFU9yZ2FuaXphdGlvbiwgQ049T0lEQzAeFw0xNTExMTExMDEyMTVaFw0yNTExMTAxMDEyMTVaMCAxHjAcBgNVBAoMFU9yZ2FuaXphdGlvbiwgQ049T0lEQzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANBGwJ/qpTQNiSgUglSE2UzEkUow+wS8r67etxoEhlzJZfgK/k5TfG1wICDqapHAxEVgUM10aBHRctNocA5wmlHtxdidhzRZroqHwpKy2BmsKX5Z2oK25RLpsyusB1KroemgA/CjUnI6rIL1xxFn3KyOFh1ZBLUQtKNQeMS7HFGgSDAp+sXuTFujz12LFDugX0T0KB5a1+0l8y0PEa0yGa1oi6seONx849ZHxM0PRvUunWkuTM+foZ0jZpFapXe02yWMqhc/2iYMieE/3GvOguJchJt6R+cut8VBb6ubKUIGK7pmoq/TB6DVXpvsHqsDJXechxcicu4pdKVDHSec850CAwEAAaNQME4wHQYDVR0OBBYEFK7RqjoodSYVXGTVEdLf3kJflP/sMB8GA1UdIwQYMBaAFK7RqjoodSYVXGTVEdLf3kJflP/sMAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADggEBADNZkxlFXh4F45muCbnQd+WmaXlGvb9tkUyAIxVL8AIu8J18F420vpnGpoUAE+Hy3evBmp2nkrFAgmr055fAjpHeZFgDZBAPCwYd3TNMDeSyMta3Ka+oS7GRFDePkMEm+kH4/rITNKUF1sOvWBTSowk9TudEDyFqgGntcdu/l/zRxvx33y3LMG5USD0x4X4IKjRrRN1BbcKgi8dq10C3jdqNancTuPoqT3WWzRvVtB/q34B7F74/6JzgEoOCEHufBMp4ZFu54P0yEGtWfTwTzuoZobrChVVBt4w/XZagrRtUCDNwRpHNbpjxYudbqLqpi1MQpV9oht/BpTHVJG2i0ro=", + "coin": { + "guest_qualifier": "None" + }, + "description": { + "en": "Dummy IDP" + }, + "logo": [ + { + "url": "https://engine.dev.openconext.local/images/logo.png?v=dev" + } + ], + "name": { + "en": "Dummy IdP" + }, + "keywords": { + "en": "idp keywords" + } + } + } +} diff --git a/tests/integration/OpenConext/EngineBlockBundle/Service/DiscoverySelectionServiceTest.php b/tests/integration/OpenConext/EngineBlockBundle/Service/DiscoverySelectionServiceTest.php new file mode 100644 index 0000000000..bc37b7901c --- /dev/null +++ b/tests/integration/OpenConext/EngineBlockBundle/Service/DiscoverySelectionServiceTest.php @@ -0,0 +1,186 @@ +service = new DiscoverySelectionService(); + $this->session = $this->createMock(SessionInterface::class); + $this->identityProvider = $this->createMock(IdentityProvider::class); + $this->discovery = Discovery::create(['en' => 'IdP'], [], new Logo('https://example.org/image.png')); + } + + public function testDiscoveryMatchesHashReturnsTrue() + { + $hash = hash('sha256', json_encode($this->discovery)); + + $this->assertTrue( + $this->service->discoveryMatchesHash($this->discovery, $hash) + ); + } + + public function testDiscoveryMatchesHashReturnsFalse() + { + $hash = 'invalid_hash'; + + $this->assertFalse( + $this->service->discoveryMatchesHash($this->discovery, $hash) + ); + } + + public function testGetDiscoveryFromRequestReturnsMatchingDiscovery() + { + $hash = hash('sha256', json_encode($this->discovery)); + + $this->session->expects($this->once()) + ->method('get') + ->with( + 'discovery', + false + ) + ->willReturn($hash); + + $this->identityProvider->expects($this->once()) + ->method('getDiscoveries') + ->willReturn([$this->discovery]); + + $result = $this->service->getDiscoveryFromRequest($this->session, $this->identityProvider); + + $this->assertSame($this->discovery, $result); + } + + public function testGetDiscoveryFromRequestReturnsNullWhenNoMatch() + { + $this->session->expects($this->once()) + ->method('get') + ->with( + 'discovery', + false + ) + ->willReturn('non_matching_hash'); + + $this->identityProvider->expects($this->once()) + ->method('getDiscoveries') + ->willReturn([$this->discovery]); + + $result = $this->service->getDiscoveryFromRequest($this->session, $this->identityProvider); + + $this->assertNull($result); + } + + public function testGetDiscoveryFromRequestWithEmptySession() + { + $this->session->expects($this->once()) + ->method('get') + ->with( + 'discovery', + false + ) + ->willReturn(false); + + $this->identityProvider->expects($this->once()) + ->method('getDiscoveries') + ->willReturn([$this->discovery]); + + $result = $this->service->getDiscoveryFromRequest($this->session, $this->identityProvider); + + $this->assertNull($result); + } + + public function testRegisterDiscoveryHashStoresHashInSession() + { + $hash = 'test_hash'; + + $this->session->expects($this->once()) + ->method('set') + ->with('discovery', $hash); + + $this->service->registerDiscoveryHash($this->session, $hash); + } + + public function testClearDiscoveryHashRemovesHashFromSession() + { + $this->session->expects($this->once()) + ->method('remove') + ->with('discovery'); + + $this->service->clearDiscoveryHash($this->session); + } + + public function testGetDiscoveryFromRequestWithMultipleDiscoveries() + { + $discovery2 = $this->createMock(Discovery::class); + $hash = hash('sha256', json_encode($discovery2)); + + $this->session->expects($this->once()) + ->method('get') + ->with( + 'discovery', + false + ) + ->willReturn($hash); + + $this->identityProvider->expects($this->once()) + ->method('getDiscoveries') + ->willReturn([$this->discovery, $discovery2]); + + $result = $this->service->getDiscoveryFromRequest($this->session, $this->identityProvider); + + $this->assertEquals($discovery2, $result); + } + + public function testHashGeneratesCorrectHash() + { + $discoveryJson = json_encode($this->discovery); + $expectedHash = hash('sha256', $discoveryJson); + + $result = $this->service->hash($this->discovery); + + $this->assertEquals($expectedHash, $result); + } + +} diff --git a/tests/library/EngineBlock/Test/Corto/Module/Service/ProvideConsentTest.php b/tests/library/EngineBlock/Test/Corto/Module/Service/ProvideConsentTest.php index ff11aaa8f1..7a7e6593a6 100644 --- a/tests/library/EngineBlock/Test/Corto/Module/Service/ProvideConsentTest.php +++ b/tests/library/EngineBlock/Test/Corto/Module/Service/ProvideConsentTest.php @@ -28,6 +28,7 @@ use OpenConext\EngineBlock\Service\ProcessingStateHelper; use OpenConext\EngineBlock\Service\ProcessingStateHelperInterface; use OpenConext\EngineBlockBundle\Authentication\AuthenticationStateInterface; +use OpenConext\EngineBlockBundle\Service\DiscoverySelectionService; use PHPUnit\Framework\TestCase; use SAML2\Assertion; use SAML2\AuthnRequest; @@ -83,6 +84,11 @@ class EngineBlock_Test_Corto_Module_Service_ProvideConsentTest extends TestCase */ private $sessionMock; + /** + * @var DiscoverySelectionService + */ + private $discoverySelectionService; + public function setUp(): void { $diContainer = EngineBlock_ApplicationSingleton::getInstance()->getDiContainer(); @@ -97,6 +103,7 @@ public function setUp(): void $this->twig = $this->mockTwig(); $this->processingStateHelperMock = $this->mockProcessingStateHelper(); $this->httpRequestMock = $this->mockHttpRequest(); + $this->discoverySelectionService = Phake::mock(DiscoverySelectionService::class); } public function testConsentRequested() @@ -327,7 +334,8 @@ private function factoryService() $this->consentService, $this->authStateHelperMock, $this->twig, - $this->processingStateHelperMock + $this->processingStateHelperMock, + $this->discoverySelectionService ); } diff --git a/tests/unit/OpenConext/EngineBlock/Metadata/Factory/AbstractEntityTest.php b/tests/unit/OpenConext/EngineBlock/Metadata/Factory/AbstractEntityTest.php index bed994f653..465aca3ce2 100644 --- a/tests/unit/OpenConext/EngineBlock/Metadata/Factory/AbstractEntityTest.php +++ b/tests/unit/OpenConext/EngineBlock/Metadata/Factory/AbstractEntityTest.php @@ -32,6 +32,7 @@ use OpenConext\EngineBlock\Metadata\RequestedAttribute; use OpenConext\EngineBlock\Metadata\Service; use OpenConext\EngineBlock\Metadata\ShibMdScope; +use OpenConext\EngineBlock\Metadata\Discovery; use OpenConext\EngineBlock\Metadata\X509\X509Certificate; use PHPUnit\Framework\TestCase; use ReflectionClass; @@ -104,11 +105,12 @@ protected function runIdentityProviderAssertions(IdentityProviderEntityInterface 'singleSignOnServices' => function(IdentityProviderEntityInterface $entity) { return $entity->getSingleSignOnServices(); }, 'consentSettings' => function(IdentityProviderEntityInterface $entity) { return $entity->getConsentSettings(); }, 'shibMdScopes' => function(IdentityProviderEntityInterface $entity) { return $entity->getShibMdScopes(); }, + 'discoveries' => function(IdentityProviderEntityInterface $entity) { return $entity->getDiscoveries(); }, ]; $missing = array_diff_key($implemented, $assertions); $this->assertCount(0, $missing, 'missing tests for: '. json_encode($missing)); - $this->assertCount(32, $implemented); + $this->assertCount(33, $implemented); $this->assertCount(count($implemented), $assertions); foreach ($assertions as $name => $assertion) { @@ -201,6 +203,7 @@ protected function getOrmEntityIdentityProviderValues() 'setConsentSettings', 'toggleWorkflowState', 'hasCompleteOrganizationData', + 'setDiscoveries' ]; // Get all state from the old mutable entity @@ -439,6 +442,9 @@ protected function getIdentityProviderMockProperties( 'shibMdScopes' => [ $this->createMock(ShibMdScope::class), $this->createMock(ShibMdScope::class), + ], + 'discoveries' => [ + $this->createMock(Discovery::class), ] ]; } diff --git a/theme/base/javascripts/wayf/deleteDisable/addSelectedIdp.js b/theme/base/javascripts/wayf/deleteDisable/addSelectedIdp.js index d59be671cb..9880c6fe85 100644 --- a/theme/base/javascripts/wayf/deleteDisable/addSelectedIdp.js +++ b/theme/base/javascripts/wayf/deleteDisable/addSelectedIdp.js @@ -9,14 +9,15 @@ import Cookies from 'js-cookie'; */ export const addSelectedIdp = (element) => { const cookieName = JSON.parse(document.getElementById(configurationId).innerHTML).previousSelectionCookieName; - const entityId = element.getAttribute('data-entityid'); + const entryKey = element.getAttribute('data-idpkey'); + let alreadyInCookie = false; let cookie = Cookies.get(cookieName) || []; if (cookie.length) { cookie = JSON.parse(cookie); cookie.forEach(idp => { - if (idp.idp === entityId) { + if (idp.idp === entryKey) { idp.count += 1; savePreviousSelection(cookie, cookieName); alreadyInCookie = true; @@ -26,7 +27,7 @@ export const addSelectedIdp = (element) => { } if (!alreadyInCookie) { - cookie = [...cookie, { idp: entityId, count: 1 }]; + cookie = [...cookie, { idp: entryKey, count: 1 }]; savePreviousSelection(cookie, cookieName); } }; diff --git a/theme/base/javascripts/wayf/matchPreviouslySelectedWithCookie.js b/theme/base/javascripts/wayf/matchPreviouslySelectedWithCookie.js index cd54a51ab7..d85e7f5cf7 100644 --- a/theme/base/javascripts/wayf/matchPreviouslySelectedWithCookie.js +++ b/theme/base/javascripts/wayf/matchPreviouslySelectedWithCookie.js @@ -27,7 +27,7 @@ export const matchPreviouslySelectedWithCookie = () => { cookie = JSON.parse(cookie); // check if each idp in the cookie is in the list, if not add it cookie.forEach(idp => { - const id = `[data-entityid="${idp.idp}"]`; + const id = `[data-idpkey="${idp.idp}"]`; const inPreviouslySelected = document.querySelector(`${selectedIdpsSelector}${id}:first-of-type`); if (!inPreviouslySelected) { diff --git a/theme/base/templates/modules/Authentication/View/Proxy/Partials/Consent/Attributes/list.html.twig b/theme/base/templates/modules/Authentication/View/Proxy/Partials/Consent/Attributes/list.html.twig index 501274169f..c5a0d3aa62 100644 --- a/theme/base/templates/modules/Authentication/View/Proxy/Partials/Consent/Attributes/list.html.twig +++ b/theme/base/templates/modules/Authentication/View/Proxy/Partials/Consent/Attributes/list.html.twig @@ -6,7 +6,13 @@ {% set logoUrl = attributeSourceLogoUrl(attributeSource) %} {% else %} {% set organisationName = idpName %} - {% if idp.logo is not null %} + {% if idpDiscovery is defined and idpDiscovery is not null %} + {% if idpDiscovery.logo is not null and idpDiscovery.logo.url is not null %} + {% set logoUrl = idpDiscovery.logo.url %} + {% else %} + {% set logoUrl = '/images/placeholder.png' %} + {% endif %} + {% elseif idp.logo is not null %} {% set logoUrl = idp.logo.url %} {% else %} {% set logoUrl = '/images/placeholder.png' %} diff --git a/theme/base/templates/modules/Authentication/View/Proxy/Partials/WAYF/idp/idp.html.twig b/theme/base/templates/modules/Authentication/View/Proxy/Partials/WAYF/idp/idp.html.twig index a0eec8854b..316b858d5a 100644 --- a/theme/base/templates/modules/Authentication/View/Proxy/Partials/WAYF/idp/idp.html.twig +++ b/theme/base/templates/modules/Authentication/View/Proxy/Partials/WAYF/idp/idp.html.twig @@ -14,6 +14,7 @@ tabindex="0" data-entityid="{{ idp['entityId'] }}" data-title="{{ idp['displayTitle'] }}" + data-idpkey="{{ entryKey(idp['entityId'], idp['discoveryHash']) }}" >