From 168d03d461b44c92669044d4fc4e5d59c3455f8a Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Fri, 26 Jun 2026 10:39:29 +0200 Subject: [PATCH] feat(stepup): add service display name to Stepup callout AuthnRequest Why is this change needed? Prior to this change, the Stepup Gateway SFO callout AuthnRequest contained no service name, so Stepup showed a generic proxy name in its 2FA UI instead of the actual application the user was logging in to. How does it address the issue? This change adds an mdui:UIInfo/mdui:DisplayName element to the samlp:Extensions of the callout AuthnRequest, using the user's current engine locale with an English fallback. The feature is controlled by the eb.stepup.send_service_name flag (disabled by default). Refs #2011 --- config/packages/engineblock_features.yaml | 1 + config/packages/parameters.yml.dist | 1 + library/EngineBlock/Corto/ProxyServer.php | 12 +- .../Stepup/StepupServiceNameExtension.php | 69 ++++++++ .../TestFeatureConfiguration.php | 1 + .../Features/Context/EngineBlockContext.php | 1 + .../Features/Stepup.feature | 17 ++ .../Stepup/StepupServiceNameExtensionTest.php | 166 ++++++++++++++++++ 8 files changed, 267 insertions(+), 1 deletion(-) create mode 100644 src/OpenConext/EngineBlock/Stepup/StepupServiceNameExtension.php create mode 100644 tests/unit/OpenConext/EngineBlock/Stepup/StepupServiceNameExtensionTest.php diff --git a/config/packages/engineblock_features.yaml b/config/packages/engineblock_features.yaml index 2fbf8dbeda..64db4320b3 100644 --- a/config/packages/engineblock_features.yaml +++ b/config/packages/engineblock_features.yaml @@ -15,5 +15,6 @@ parameters: eb.feature_enable_idp_initiated_flow: "%feature_enable_idp_initiated_flow%" eb.stepup.sfo.override_engine_entityid: "%feature_stepup_sfo_override_engine_entityid%" eb.stepup.send_user_attributes: "%feature_stepup_send_user_attributes%" + eb.stepup.send_service_name: "%feature_stepup_send_service_name%" eb.feature_enable_sram_interrupt: "%feature_enable_sram_interrupt%" eb.hide_bookmarkable_url: "%feature_hide_bookmarkable_url%" diff --git a/config/packages/parameters.yml.dist b/config/packages/parameters.yml.dist index 18212e78e5..dd72f2c37a 100644 --- a/config/packages/parameters.yml.dist +++ b/config/packages/parameters.yml.dist @@ -235,6 +235,7 @@ parameters: feature_enable_idp_initiated_flow: true feature_stepup_sfo_override_engine_entityid: false feature_stepup_send_user_attributes: false + feature_stepup_send_service_name: false feature_enable_sram_interrupt: false feature_hide_bookmarkable_url: false diff --git a/library/EngineBlock/Corto/ProxyServer.php b/library/EngineBlock/Corto/ProxyServer.php index e53e62b962..13eaeac5b8 100644 --- a/library/EngineBlock/Corto/ProxyServer.php +++ b/library/EngineBlock/Corto/ProxyServer.php @@ -27,6 +27,7 @@ use OpenConext\EngineBlock\Metadata\Service; use OpenConext\EngineBlock\Metadata\TransparentMfaEntity; use OpenConext\EngineBlock\Stepup\StepupGsspUserAttributeExtension; +use OpenConext\EngineBlock\Stepup\StepupServiceNameExtension; use OpenConext\EngineBlock\Metadata\X509\KeyPairFactory; use OpenConext\EngineBlockBundle\Authentication\AuthenticationState; use OpenConext\EngineBlockBundle\Exception\UnknownKeyIdException; @@ -486,7 +487,8 @@ public function sendStepupAuthenticationRequest( Loa $authnContextClassRef, NameID $nameId, bool $isForceAuthn, - Assertion $originalAssertion + Assertion $originalAssertion, + ?ServiceProvider $sp = null, ): void { $ebRequest = EngineBlock_Saml2_AuthnRequestFactory::createFromRequest( $spRequest, @@ -555,6 +557,13 @@ public function sendStepupAuthenticationRequest( } } + $isSendServiceNameConfigured = $features->hasFeature('eb.stepup.send_service_name'); + $isSendServiceNameEnabled = $features->isEnabled('eb.stepup.send_service_name'); + + if ($isSendServiceNameConfigured && $isSendServiceNameEnabled && $sp !== null) { + $locale = $container->getLocaleProvider()->getLocale(); + StepupServiceNameExtension::add($sspMessage, $sp, $locale); + } // Link with the original Request $diContainerRuntime = EngineBlock_ApplicationSingleton::getInstance()->getDiContainerRuntime(); @@ -635,6 +644,7 @@ public function handleStepupAuthenticationCallout( $nameId, $sp->getCoins()->isStepupForceAuthn(), $originalAssertion, + $sp, ); } diff --git a/src/OpenConext/EngineBlock/Stepup/StepupServiceNameExtension.php b/src/OpenConext/EngineBlock/Stepup/StepupServiceNameExtension.php new file mode 100644 index 0000000000..519d58ef38 --- /dev/null +++ b/src/OpenConext/EngineBlock/Stepup/StepupServiceNameExtension.php @@ -0,0 +1,69 @@ +createElementNS(self::MDUI_NS, 'mdui:UIInfo'); + $displayName = $dom->createElementNS(self::MDUI_NS, 'mdui:DisplayName'); + $displayName->setAttributeNS('http://www.w3.org/XML/1998/namespace', 'xml:lang', $resolvedLocale); + $displayName->textContent = $name; + $uiInfo->appendChild($displayName); + + $ext = $message->getExtensions(); + $ext['mdui:UIInfo'] = new Chunk($uiInfo); + $message->setExtensions($ext); + } + + /** + * @return array{string, string}|null + */ + private static function resolveName(ServiceProvider $sp, string $locale): ?array + { + $name = $sp->getMdui()->getDisplayNameOrNull($locale); + if (empty($name)) { + $name = $sp->{'name' . ucfirst($locale)}; + } + if (empty($name)) { + return null; + } + return [$locale, $name]; + } +} diff --git a/src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php b/src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php index 15512f7e62..feaab55b79 100644 --- a/src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php +++ b/src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php @@ -47,6 +47,7 @@ public function __construct() $this->setFeature(new Feature('eb.stepup.sfo.override_engine_entityid', false)); $this->setFeature(new Feature('eb.feature_enable_idp_initiated_flow', true)); $this->setFeature(new Feature('eb.stepup.send_user_attributes', true)); + $this->setFeature(new Feature('eb.stepup.send_service_name', false)); $this->setFeature(new Feature('eb.feature_enable_sram_interrupt', true)); $this->setFeature(new Feature('eb.hide_bookmarkable_url', false)); } diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php index 7e94d564b0..f7dba64249 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php @@ -688,6 +688,7 @@ public function theAuthnRequestToSubmitShouldMatchXpath($xpath) $xpathObject = new DOMXPath($authnRequest); $xpathObject->registerNamespace('gssp', 'urn:mace:surf.nl:stepup:gssp-extensions'); + $xpathObject->registerNamespace('mdui', 'urn:oasis:names:tc:SAML:metadata:ui'); $nodeList = $xpathObject->query($xpath); if (!$nodeList || $nodeList->length === 0) { diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Stepup.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Stepup.feature index ccb8d42c1d..d13e589d89 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Stepup.feature +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Stepup.feature @@ -291,3 +291,20 @@ Feature: And I pass through EngineBlock And I pass through the IdP Then the received AuthnRequest should match xpath '/samlp:AuthnRequest/samlp:Extensions/gssp:UserAttributes/saml:Attribute[@Name="urn:mace:dir:attribute-def:mail"]/saml:AttributeValue[text()="j.doe@institution-a.example.org"]' + + Scenario: Stepup callout includes service name in mdui:UIInfo when feature is enabled + Given the SP "SSO-SP" requires Stepup LoA "http://dev.openconext.local/assurance/loa2" + And feature "eb.stepup.send_service_name" is enabled + When I log in at "SSO-SP" + And I select "SSO-IdP" on the WAYF + And I pass through EngineBlock + And I pass through the IdP + Then the received AuthnRequest should match xpath '/samlp:AuthnRequest/samlp:Extensions/mdui:UIInfo/mdui:DisplayName' + + Scenario: Stepup callout does not include service name in mdui:UIInfo when feature is disabled + Given the SP "SSO-SP" requires Stepup LoA "http://dev.openconext.local/assurance/loa2" + When I log in at "SSO-SP" + And I select "SSO-IdP" on the WAYF + And I pass through EngineBlock + And I pass through the IdP + Then the received AuthnRequest should not match xpath '/samlp:AuthnRequest/samlp:Extensions/mdui:UIInfo/mdui:DisplayName' diff --git a/tests/unit/OpenConext/EngineBlock/Stepup/StepupServiceNameExtensionTest.php b/tests/unit/OpenConext/EngineBlock/Stepup/StepupServiceNameExtensionTest.php new file mode 100644 index 0000000000..d0ba3fb100 --- /dev/null +++ b/tests/unit/OpenConext/EngineBlock/Stepup/StepupServiceNameExtensionTest.php @@ -0,0 +1,166 @@ +spWithNames('My Service EN', 'Mijn Service NL'); + $request = new AuthnRequest(); + + StepupServiceNameExtension::add($request, $sp, 'nl'); + + $nodes = $this->queryXpath($request, './/mdui:DisplayName[@xml:lang="nl"]'); + $this->assertSame(1, $nodes->length); + $this->assertSame('Mijn Service NL', $nodes->item(0)->textContent); + } + + public function testFallsBackToEnglishWhenRequestedLocaleHasNoName(): void + { + $sp = $this->spWithNames('My Service EN'); + $request = new AuthnRequest(); + + StepupServiceNameExtension::add($request, $sp, 'nl'); + + $nodes = $this->queryXpath($request, './/mdui:DisplayName[@xml:lang="en"]'); + $this->assertSame(1, $nodes->length); + $this->assertSame('My Service EN', $nodes->item(0)->textContent); + } + + public function testAddsNoExtensionWhenNeitherLocaleNorEnglishHasName(): void + { + $sp = $this->spWithNames(); + $request = new AuthnRequest(); + + StepupServiceNameExtension::add($request, $sp, 'nl'); + + $ext = $request->getExtensions(); + $this->assertArrayNotHasKey('mdui:UIInfo', $ext); + } + + public function testMduiDisplayNameTakesPrecedenceOverFlatField(): void + { + $sp = $this->spWithMduiDisplayName('en', 'Mdui Service Name'); + $sp->nameEn = 'Flat Field Name'; + $request = new AuthnRequest(); + + StepupServiceNameExtension::add($request, $sp, 'en'); + + $nodes = $this->queryXpath($request, './/mdui:DisplayName[@xml:lang="en"]'); + $this->assertSame(1, $nodes->length); + $this->assertSame('Mdui Service Name', $nodes->item(0)->textContent); + } + + public function testLocaleTagMatchesActualContentLocale(): void + { + $sp = $this->spWithNames('Only English Name'); + $request = new AuthnRequest(); + + StepupServiceNameExtension::add($request, $sp, 'nl'); + + $nlNodes = $this->queryXpath($request, './/mdui:DisplayName[@xml:lang="nl"]'); + $enNodes = $this->queryXpath($request, './/mdui:DisplayName[@xml:lang="en"]'); + $this->assertSame(0, $nlNodes->length, 'Should not add nl tag when using en fallback'); + $this->assertSame(1, $enNodes->length, 'Should add en tag matching the actual content locale'); + } + + public function testFallsBackToFlatFieldWhenMduiDisplayNameIsEmpty(): void + { + $mduiJson = '{"DisplayName":{"name":"DisplayName","values":{"en":{"value":"","language":"en"}}},' + . '"Description":{"name":"Description"},"Keywords":{"name":"Keywords"},' + . '"Logo":{"name":"Logo"},"PrivacyStatementURL":{"name":"PrivacyStatementURL"}}'; + $sp = new ServiceProvider('https://sp.example.org', Mdui::fromJson($mduiJson)); + $sp->nameEn = 'Flat Field Name'; + $request = new AuthnRequest(); + + StepupServiceNameExtension::add($request, $sp, 'en'); + + $nodes = $this->queryXpath($request, './/mdui:DisplayName[@xml:lang="en"]'); + $this->assertSame(1, $nodes->length); + $this->assertSame('Flat Field Name', $nodes->item(0)->textContent); + } + + public function testPreservesExistingExtensions(): void + { + $sp = $this->spWithNames('My Service'); + $request = new AuthnRequest(); + $request->setExtensions(['existing:Extension' => new Chunk( + (new DOMDocument())->createElement('existing:Extension') + )]); + + StepupServiceNameExtension::add($request, $sp, 'en'); + + $ext = $request->getExtensions(); + $this->assertArrayHasKey('existing:Extension', $ext); + $this->assertArrayHasKey('mdui:UIInfo', $ext); + } + + private function spWithNames(string $nameEn = '', string $nameNl = '', string $namePt = ''): ServiceProvider + { + $sp = new ServiceProvider('https://sp.example.org'); + $sp->nameEn = $nameEn; + $sp->nameNl = $nameNl; + $sp->namePt = $namePt; + return $sp; + } + + private function spWithMduiDisplayName(string $locale, string $displayName): ServiceProvider + { + $mduiJson = sprintf( + '{"DisplayName":{"name":"DisplayName","values":{"%s":{"value":"%s","language":"%s"}}},' + . '"Description":{"name":"Description"},"Keywords":{"name":"Keywords"},' + . '"Logo":{"name":"Logo"},"PrivacyStatementURL":{"name":"PrivacyStatementURL"}}', + $locale, + $displayName, + $locale + ); + return new ServiceProvider('https://sp.example.org', Mdui::fromJson($mduiJson)); + } + + private function getExtensionElement(AuthnRequest $request): DOMElement + { + $ext = $request->getExtensions(); + $this->assertArrayHasKey('mdui:UIInfo', $ext); + $chunk = $ext['mdui:UIInfo']; + $this->assertInstanceOf(Chunk::class, $chunk); + return $chunk->getXML(); + } + + private function queryXpath(AuthnRequest $request, string $expression): DOMNodeList + { + $el = $this->getExtensionElement($request); + $xpath = new DOMXPath($el->ownerDocument); + $xpath->registerNamespace('mdui', 'urn:oasis:names:tc:SAML:metadata:ui'); + $xpath->registerNamespace('xml', 'http://www.w3.org/XML/1998/namespace'); + return $xpath->query($expression, $el); + } +}