Feature/issue 1864 optional domain hint (#1984)

kayjoosten · web-flow · commit 4e1fbba575cb · 2026-04-23T14:02:09.000+02:00
* Add coin:azure_domain_hint to append whr= on HTTP-Redirect AuthnRequests (#1864) When coin:azure_domain_hint is set on an IdP, EngineBlock appends a whr=<domain> query parameter to the HTTP-Redirect URL it sends as the AuthnRequest to that IdP. This allows Microsoft Azure / EntraID to skip the account picker for users whose realm is already known. - Add azureDomainHint field + getter to Coins (IdP coins) - Add azureDomainHint constructor param to IdentityProvider entity - Map metadata:coin:azure_domain_hint in PushMetadataAssembler - Append whr= in Bindings::send() HTTP-Redirect branch when IdP has the coin - Integration test: azure_domain_hint coin round-trips through PushMetadataAssembler - Legacy test: Bindings appends / omits whr= based on coin presence - Behat scenario: AzureDomainHint.feature covers the full SSO flow * fix: use correct URL separator when appending whr= to redirect URL * fix: check whr= absence at IdP redirect URL in negative Behat scenario Added IDP "<name>" prefers HTTP Redirect binding step and used it in the negative scenario so the URL assertion fires at the actual IdP redirect URL rather than at an intermediate EngineBlock page. * fix: add string type hint to setAzureDomainHintForIdp parameter * fix: add missing string type hint and improve docblocks and test assertions * chore: remove useless comments
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,10 @@ More information about our release strategy can be found in
 the [Development Guidelines](https://github.com/OpenConext/OpenConext-engineblock/wiki/Development-Guidelines#release-notes) on
 the EngineBlock wiki.
 
+## UNRELEASED
+Features:
+* Added `coin:azure_domain_hint` configuration option for IdPs. When set, EngineBlock appends a `whr=<domain>` query parameter to the HTTP-Redirect AuthnRequest sent to the IdP, allowing Microsoft Azure / EntraID to skip the account picker (#1864).
+
 ## UNRELEASED 7.2.0
 Upgrade to Symfony 7.4
 Upgrade to `doctrine/dbal` 4
diff --git a/library/EngineBlock/Corto/Module/Bindings.php b/library/EngineBlock/Corto/Module/Bindings.php
@@ -710,6 +710,15 @@ public function send(
             }
 
             $url = $sspBinding->getRedirectURL($sspMessage);
+
+            if ($remoteEntity instanceof IdentityProvider) {
+                $domainHint = $remoteEntity->getCoins()->azureDomainHint();
+                if (!empty($domainHint)) {
+                    $separator = str_contains($url, '?') ? '&' : '?';
+                    $url .= $separator . http_build_query(['whr' => $domainHint]);
+                }
+            }
+
             $this->_server->redirect($url, $message);
         }
         else {
diff --git a/src/OpenConext/EngineBlock/Metadata/Coins.php b/src/OpenConext/EngineBlock/Metadata/Coins.php
@@ -77,7 +77,8 @@ public static function createForIdentityProvider(
         $signatureMethod,
         $mfaEntities,
         $defaultRAC,
-        $policyEnforcementDecisionRequired
+        bool $policyEnforcementDecisionRequired,
+        ?string $azureDomainHint = null
     ) {
         return new self([
             'guestQualifier' => $guestQualifier,
@@ -90,6 +91,7 @@ public static function createForIdentityProvider(
             'mfaEntities' => $mfaEntities,
             'defaultRAC' => $defaultRAC,
             'policyEnforcementDecisionRequired' => $policyEnforcementDecisionRequired,
+            'azureDomainHint' => $azureDomainHint,
         ]);
     }
 
@@ -250,6 +252,11 @@ public function mfaEntities(): MfaEntityCollection
         return $this->getValue('mfaEntities', MfaEntityCollection::fromCoin([]));
     }
 
+    public function azureDomainHint(): ?string
+    {
+        return $this->getValue('azureDomainHint');
+    }
+
     private function getValue($key, $default = null)
     {
         if (!array_key_exists($key, $this->values)) {
diff --git a/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php b/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php
@@ -269,6 +269,11 @@ private function assembleIdp(stdClass $connection)
             'policyEnforcementDecisionRequired'
         );
 
+        $properties += $this->setPathFromObjectString(
+            [$connection, 'metadata:coin:azure_domain_hint'],
+            'azureDomainHint'
+        );
+
         return Utils::instantiate(
             IdentityProvider::class,
             $properties
diff --git a/src/OpenConext/EngineBlock/Metadata/Entity/IdentityProvider.php b/src/OpenConext/EngineBlock/Metadata/Entity/IdentityProvider.php
@@ -109,6 +109,7 @@ class IdentityProvider extends AbstractRole
     /**
      * WARNING: Please don't use this entity directly but use the dedicated factory instead.
      * @see \OpenConext\EngineBlock\Metadata\Factory\Factory\IdentityProviderFactory
+     * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
      */
     public function __construct(
         $entityId,
@@ -154,7 +155,8 @@ public function __construct(
         ?MfaEntityCollection $mfaEntities = null,
         array $discoveries = [],
         ?string $defaultRAC = null,
-        bool $policyEnforcementDecisionRequired = false
+        bool $policyEnforcementDecisionRequired = false,
+        ?string $azureDomainHint = null
     ) {
         if (is_null($mdui)) {
             $mdui = Mdui::emptyMdui();
@@ -203,7 +205,8 @@ public function __construct(
             $signatureMethod,
             $mfaEntities,
             $defaultRAC,
-            $policyEnforcementDecisionRequired
+            $policyEnforcementDecisionRequired,
+            $azureDomainHint
         );
 
         $this->assertAllDiscoveries($discoveries);
diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/AzureDomainHint.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/AzureDomainHint.feature
@@ -0,0 +1,30 @@
+Feature: Azure / EntraID domain hint
+  In order to skip the Microsoft Azure account picker for users whose realm is known
+  As an IdP operator
+  I want to configure coin:azure_domain_hint on an IdP so that EngineBlock appends whr=<domain>
+  to the HTTP-Redirect AuthnRequest URL it sends to that IdP
+
+  Background:
+    Given an EngineBlock instance on "dev.openconext.local"
+      And no registered SPs
+      And no registered Idps
+      And an Identity Provider named "Azure IdP"
+      And a Service Provider named "Dummy SP"
+
+  Scenario: EngineBlock appends whr query parameter when coin:azure_domain_hint is configured
+    Given IDP "Azure IdP" has Azure domain hint "hartingcollege.nl"
+     When I log in at "Dummy SP"
+     Then the full url should match "whr=hartingcollege\.nl"
+      And I pass through the IdP
+      And I give my consent
+      And I pass through EngineBlock
+     Then the url should match "functional-testing/Dummy%20SP/acs"
+
+  Scenario: EngineBlock does not append whr query parameter when coin:azure_domain_hint is not configured
+    Given IDP "Azure IdP" prefers HTTP Redirect binding
+     When I log in at "Dummy SP"
+     Then the url should not match "whr="
+      And I pass through the IdP
+      And I give my consent
+      And I pass through EngineBlock
+     Then the url should match "functional-testing/Dummy%20SP/acs"
diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/MinkContext.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/MinkContext.php
@@ -30,6 +30,7 @@
 
 /**
  * Mink-enabled context.
+ * @SuppressWarnings(PHPMD.TooManyPublicMethods)
  */
 class MinkContext extends BaseMinkContext
 {
@@ -216,6 +217,34 @@ public function theResponseShouldNotMatchXpath($xpath)
         }
     }
 
+    /**
+     * @Then /^the full url should match (?P<pattern>"(?:[^"]|\\")*")$/
+     */
+    public function assertFullUrlRegExp($pattern)
+    {
+        $url = $this->getSession()->getCurrentUrl();
+        if (!preg_match($this->fixStepArgument($pattern), $url)) {
+            throw new ExpectationException(
+                sprintf('Current page "%s" does not match the regex "%s".', $url, $pattern),
+                $this->getSession()
+            );
+        }
+    }
+
+    /**
+     * @Then /^the (?i)url(?-i) should not match (?P<pattern>"(?:[^"]|\\")*")$/
+     */
+    public function assertUrlNotRegExp($pattern)
+    {
+        $url = $this->getSession()->getCurrentUrl();
+        if (preg_match($this->fixStepArgument($pattern), $url)) {
+            throw new ExpectationException(
+                sprintf('URL "%s" matched pattern "%s", but it should not.', $url, $pattern),
+                $this->getSession()
+            );
+        }
+    }
+
     /**
      * @Given /^I should see URL "([^"]*)"$/
      */
diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/MockIdpContext.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/MockIdpContext.php
@@ -129,6 +129,34 @@ public function idpRequiresAPolicyEnforcementDecision($idpName)
             ->save();
     }
 
+    /**
+     * @Given /^IDP "([^"]*)" has Azure domain hint "([^"]*)"$/
+     * @param string $idpName
+     * @param string $domainHint
+     */
+    public function idpHasAzureDomainHint($idpName, $domainHint)
+    {
+        $idp = $this->mockIdpRegistry->get($idpName);
+
+        $this->serviceRegistryFixture
+            ->setAzureDomainHintForIdp($idp->entityId(), $domainHint)
+            ->preferHttpRedirectBindingForIdp($idp->entityId())
+            ->save();
+    }
+
+    /**
+     * @Given /^IDP "([^"]*)" prefers HTTP Redirect binding$/
+     * @param string $idpName
+     */
+    public function idpPrefersHttpRedirectBinding($idpName)
+    {
+        $idp = $this->mockIdpRegistry->get($idpName);
+
+        $this->serviceRegistryFixture
+            ->preferHttpRedirectBindingForIdp($idp->entityId())
+            ->save();
+    }
+
     /**
      * @Given /^an Identity Provider named "([^"]*)" with logo "([^"]*)"$/
      */
diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Fixtures/ServiceRegistryFixture.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Fixtures/ServiceRegistryFixture.php
@@ -62,7 +62,6 @@ class ServiceRegistryFixture
      */
     private $entityManager;
 
-
     public function __construct(DoctrineMetadataRepository $repository, EntityManager $em)
     {
         $this->repository = $repository;
@@ -342,6 +341,42 @@ public function requiresPolicyEnforcementDecisionForIdp($entityId)
         return $this;
     }
 
+    public function setAzureDomainHintForIdp(string $entityId, string $domainHint): self
+    {
+        $idp = $this->getIdentityProvider($entityId);
+        assert($idp instanceof IdentityProvider);
+
+        $this->setCoin($idp, 'azureDomainHint', $domainHint);
+
+        return $this;
+    }
+
+    public function preferHttpRedirectBindingForIdp(string $entityId): self
+    {
+        $idp = $this->getIdentityProvider($entityId);
+        assert($idp instanceof IdentityProvider);
+
+        $redirectServices = array_values(array_filter(
+            $idp->singleSignOnServices,
+            static fn(Service $s) => $s->binding === Constants::BINDING_HTTP_REDIRECT
+        ));
+
+        assert(
+            count($redirectServices) > 0,
+            'IdP has no HTTP-Redirect SSO binding; domain hint test would be meaningless'
+        );
+
+        $idp->singleSignOnServices = $redirectServices;
+
+        // Force Doctrine to recognise the property change under DEFERRED_EXPLICIT change tracking.
+        $this->entityManager->getUnitOfWork()->recomputeSingleEntityChangeSet(
+            $this->entityManager->getClassMetadata(IdentityProvider::class),
+            $idp
+        );
+
+        return $this;
+    }
+
     public function requireAttributeAggregationForSp($entityId)
     {
         $arp = $this->getServiceProvider($entityId)->attributeReleasePolicy;
diff --git a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php
@@ -396,6 +396,7 @@ public static function validCoins()
             ['schachomeorganization', 'saml20-idp', 'schacHomeOrganization', 'string'],
             ['policy_enforcement_decision_required', 'saml20-idp', 'policyEnforcementDecisionRequired', 'bool'],
             ['hidden', 'saml20-idp', 'hidden', 'bool'],
+            ['azure_domain_hint', 'saml20-idp', 'azureDomainHint', 'string'],
 
             // Abstract
             ['disable_scoping', 'saml20-idp', 'disableScoping', 'bool'],
diff --git a/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php b/tests/library/EngineBlock/Test/Corto/Module/BindingsTest.php
@@ -18,13 +18,15 @@
 
 use Mockery as m;
 use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
+use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider;
 use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
 use OpenConext\EngineBlockBundle\Bridge\DiContainerRuntime;
 use PHPUnit\Framework\TestCase;
 use SAML2\Assertion;
 use SAML2\Assertion\Validation\ConstraintValidator\NotBefore;
 use SAML2\Assertion\Validation\ConstraintValidator\NotOnOrAfter;
 use SAML2\Assertion\Validation\Result;
+use SAML2\AuthnRequest;
 use SAML2\Constants;
 use SAML2\Response;
 
@@ -40,20 +42,26 @@ class EngineBlock_Test_Corto_Module_BindingsTest extends TestCase
      */
     private $bindings;
 
+    /**
+     * @var EngineBlock_Corto_ProxyServer
+     */
+    private $proxyServer;
+
     public function setUp(): void
     {
-        $proxyServer = Phake::mock('EngineBlock_Corto_ProxyServer');
-        Phake::when($proxyServer)->getSigningCertificates(false)->thenReturn(
+        $this->proxyServer = Phake::mock('EngineBlock_Corto_ProxyServer');
+        Phake::when($this->proxyServer)->getSigningCertificates(false)->thenReturn(
             new EngineBlock_X509_KeyPair(
                 new EngineBlock_X509_Certificate(openssl_x509_read(file_get_contents(__DIR__.'/test.pem.crt'))),
                 new EngineBlock_X509_PrivateKey(__DIR__.'/test.pem.key')
             )
         );
+        Phake::when($this->proxyServer)->getConfig('WantsAuthnRequestsSigned')->thenReturn(false);
 
         $engineBlock = \EngineBlock_ApplicationSingleton::getInstance();
         $engineBlock->setDiContainerRuntime(new DiContainerRuntime(Phake::mock(Twig\Environment::class)));
 
-        $this->bindings = new EngineBlock_Corto_Module_Bindings($proxyServer);
+        $this->bindings = new EngineBlock_Corto_Module_Bindings($this->proxyServer);
     }
 
     public function testResponseRedirectIsNotSupported()
@@ -142,4 +150,78 @@ public function responseProvider()
 
         return array_merge_recursive($responseFiles, $certificateFiles);
     }
+
+    public function testAzureDomainHintIsAppendedToRedirectUrl()
+    {
+        $authnRequest = new AuthnRequest();
+        $authnRequest->setDestination('https://login.microsoftonline.com/tenant-id/saml2');
+
+        $request = new EngineBlock_Saml2_AuthnRequestAnnotationDecorator($authnRequest);
+        $request->setDeliverByBinding(Constants::BINDING_HTTP_REDIRECT);
+
+        $idp = new IdentityProvider(
+            entityId: 'https://idp.example.com',
+            azureDomainHint: 'hartingcollege.nl'
+        );
+
+        $this->bindings->send($request, $idp);
+
+        Phake::verify($this->proxyServer)->redirect(
+            Phake::capture($redirectUrl),
+            Phake::capture($capturedMessage)
+        );
+
+        $parsed = parse_url($redirectUrl);
+        $this->assertArrayHasKey('query', $parsed, 'Redirect URL must have a query string');
+        parse_str($parsed['query'], $params);
+        $this->assertArrayHasKey('whr', $params, 'whr query parameter must be present');
+        $this->assertSame('hartingcollege.nl', $params['whr']);
+    }
+
+    public function testAzureDomainHintIsNotAddedWhenNotConfigured()
+    {
+        $authnRequest = new AuthnRequest();
+        $authnRequest->setDestination('https://idp.example.com/sso');
+
+        $request = new EngineBlock_Saml2_AuthnRequestAnnotationDecorator($authnRequest);
+        $request->setDeliverByBinding(Constants::BINDING_HTTP_REDIRECT);
+
+        $idp = new IdentityProvider('https://idp.example.com');
+
+        $this->bindings->send($request, $idp);
+
+        Phake::verify($this->proxyServer)->redirect(
+            Phake::capture($redirectUrl),
+            Phake::capture($capturedMessage)
+        );
+
+        $this->assertStringNotContainsString('whr=', $redirectUrl);
+    }
+
+    public function testAzureDomainHintWhrParamIsCorrectlyUrlEncoded()
+    {
+        $authnRequest = new AuthnRequest();
+        $authnRequest->setDestination('https://login.microsoftonline.com/tenant-id/saml2');
+
+        $request = new EngineBlock_Saml2_AuthnRequestAnnotationDecorator($authnRequest);
+        $request->setDeliverByBinding(Constants::BINDING_HTTP_REDIRECT);
+
+        $idp = new IdentityProvider(
+            entityId: 'https://idp.example.com',
+            azureDomainHint: 'example.nl'
+        );
+
+        $this->bindings->send($request, $idp);
+
+        Phake::verify($this->proxyServer)->redirect(
+            Phake::capture($redirectUrl),
+            Phake::capture($capturedMessage)
+        );
+
+        $parsed = parse_url($redirectUrl);
+        $this->assertArrayHasKey('query', $parsed, 'Redirect URL must have a query string');
+        parse_str($parsed['query'], $params);
+        $this->assertArrayHasKey('whr', $params, 'whr query parameter must be present');
+        $this->assertSame('example.nl', $params['whr']);
+    }
 }
