diff --git a/library/EngineBlock/Corto/Module/Service/AssertionConsumer.php b/library/EngineBlock/Corto/Module/Service/AssertionConsumer.php index e7ee06d4f2..cc7db2f110 100644 --- a/library/EngineBlock/Corto/Module/Service/AssertionConsumer.php +++ b/library/EngineBlock/Corto/Module/Service/AssertionConsumer.php @@ -207,7 +207,13 @@ public function serve($serviceName, Request $httpRequest) $nameId = clone $receivedResponse->getNameId(); $authnClassRef = $this->_stepupGatewayCallOutHelper->getStepupLoa($idp, $sp, $authnRequestLoas, $pdpLoas); - $this->_server->sendStepupAuthenticationRequest($receivedRequest, $currentProcessStep->getRole(), $authnClassRef, $nameId); + $this->_server->sendStepupAuthenticationRequest( + $receivedRequest, + $currentProcessStep->getRole(), + $authnClassRef, + $nameId, + $sp->getCoins()->isStepupForceAuthn() + ); } /** diff --git a/library/EngineBlock/Corto/ProxyServer.php b/library/EngineBlock/Corto/ProxyServer.php index 9330db0ecf..6ff15934df 100644 --- a/library/EngineBlock/Corto/ProxyServer.php +++ b/library/EngineBlock/Corto/ProxyServer.php @@ -457,7 +457,8 @@ public function sendStepupAuthenticationRequest( EngineBlock_Saml2_AuthnRequestAnnotationDecorator $spRequest, IdentityProvider $identityProvider, Loa $authnContextClassRef, - NameID $nameId + NameID $nameId, + bool $isForceAuthn ) { $ebRequest = EngineBlock_Saml2_AuthnRequestFactory::createFromRequest( $spRequest, @@ -467,6 +468,8 @@ public function sendStepupAuthenticationRequest( 'stepupAssertionConsumerService' ); + $ebRequest->setForceAuthn($isForceAuthn); + $sspMessage = $ebRequest->getSspMessage(); if (!$sspMessage instanceof AuthnRequest) { throw new EngineBlock_Corto_ProxyServer_Exception(sprintf('Unknown message type: "%s"', get_class($sspMessage))); diff --git a/library/EngineBlock/Saml2/AuthnRequestAnnotationDecorator.php b/library/EngineBlock/Saml2/AuthnRequestAnnotationDecorator.php index 3df48b0fd2..2f10449b46 100644 --- a/library/EngineBlock/Saml2/AuthnRequestAnnotationDecorator.php +++ b/library/EngineBlock/Saml2/AuthnRequestAnnotationDecorator.php @@ -179,4 +179,9 @@ public function isTransparent() { return $this->transparent; } + + public function setForceAuthn(bool $isForceAuthn) + { + $this->sspMessage->setForceAuthn($isForceAuthn); + } } diff --git a/src/OpenConext/EngineBlock/Metadata/Coins.php b/src/OpenConext/EngineBlock/Metadata/Coins.php index efb88ea3a5..d3c194579a 100644 --- a/src/OpenConext/EngineBlock/Metadata/Coins.php +++ b/src/OpenConext/EngineBlock/Metadata/Coins.php @@ -43,7 +43,8 @@ public static function createForServiceProvider( $stepupRequireLoa, $disableScoping, $additionalLogging, - $signatureMethod + $signatureMethod, + $stepupForceAuthn ) { return new self([ 'isConsentRequired' => $isConsentRequired, @@ -60,6 +61,7 @@ public static function createForServiceProvider( 'signatureMethod' => $signatureMethod, 'stepupAllowNoToken' => $stepupAllowNoToken, 'stepupRequireLoa' => $stepupRequireLoa, + 'stepupForceAuthn' => $stepupForceAuthn, ]); } @@ -178,6 +180,15 @@ public function stepupRequireLoa() return $this->getValue('stepupRequireLoa', ''); } + /** + * Should the Stepup authentication request (to the Stepup Gateway) + * have the ForceAuthn attribute in the AuthnRequest? + */ + public function isStepupForceAuthn() + { + return $this->getValue('stepupForceAuthn', false); + } + // IDP public function guestQualifier() { diff --git a/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php b/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php index 473a3dde8d..b2d3e35f7b 100644 --- a/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php +++ b/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssembler.php @@ -273,7 +273,13 @@ private function assembleSp(stdClass $connection) ), 'stepupAllowNoToken' ); - + $properties += $this->setPathFromObjectBool( + array( + $connection, + 'metadata:coin:stepup:forceauthn' + ), + 'stepupForceAuthn' + ); return Utils::instantiate( ServiceProvider::class, $properties diff --git a/src/OpenConext/EngineBlock/Metadata/Entity/ServiceProvider.php b/src/OpenConext/EngineBlock/Metadata/Entity/ServiceProvider.php index 7cb04e6780..0611af14a4 100644 --- a/src/OpenConext/EngineBlock/Metadata/Entity/ServiceProvider.php +++ b/src/OpenConext/EngineBlock/Metadata/Entity/ServiceProvider.php @@ -105,54 +105,6 @@ class ServiceProvider extends AbstractRole /** * WARNING: Please don't use this entity directly but use the dedicated factory instead. * @see \OpenConext\EngineBlock\Factory\Factory\ServiceProviderFactory - * - * @param string $entityId - * @param Organization $organizationEn - * @param Organization $organizationNl - * @param Organization $organizationPt - * @param Service $singleLogoutService - * @param bool $additionalLogging - * @param X509Certificate[] $certificates - * @param ContactPerson[] $contactPersons - * @param string $descriptionEn - * @param string $descriptionNl - * @param string $descriptionPt - * @param bool $disableScoping - * @param string $displayNameEn - * @param string $displayNameNl - * @param string $displayNamePt - * @param string $keywordsEn - * @param string $keywordsNl - * @param string $keywordsPt - * @param Logo $logo - * @param string $nameEn - * @param string $nameNl - * @param string $namePt - * @param null $nameIdFormat - * @param array $supportedNameIdFormats - * @param bool $requestsMustBeSigned - * @param string $signatureMethod - * @param string $workflowState - * @param array $allowedIdpEntityIds - * @param bool $allowAll - * @param array $assertionConsumerServices - * @param bool $displayUnconnectedIdpsWayf - * @param null $termsOfServiceUrl - * @param bool $isConsentRequired - * @param bool $isTransparentIssuer - * @param bool $isTrustedProxy - * @param null $requestedAttributes - * @param bool $skipDenormalization - * @param bool $policyEnforcementDecisionRequired - * @param bool $requesteridRequired - * @param bool $signResponse - * @param string $manipulation - * @param AttributeReleasePolicy $attributeReleasePolicy - * @param string|null $supportUrlEn - * @param string|null $supportUrlNl - * @param string|null $supportUrlPt - * @param bool|null $stepupAllowNoToken - * @param bool|null $stepupRequireLoa */ public function __construct( $entityId, @@ -160,51 +112,52 @@ public function __construct( Organization $organizationNl = null, Organization $organizationPt = null, Service $singleLogoutService = null, - $additionalLogging = false, + bool $additionalLogging = false, array $certificates = array(), array $contactPersons = array(), - $descriptionEn = '', - $descriptionNl = '', - $descriptionPt = '', - $disableScoping = false, - $displayNameEn = '', - $displayNameNl = '', - $displayNamePt = '', - $keywordsEn = '', - $keywordsNl = '', - $keywordsPt = '', - Logo $logo = null, - $nameEn = '', - $nameNl = '', - $namePt = '', - $nameIdFormat = null, - $supportedNameIdFormats = array( + string $descriptionEn = '', + string $descriptionNl = '', + string $descriptionPt = '', + bool $disableScoping = false, + ?string $displayNameEn = '', + ?string $displayNameNl = '', + ?string $displayNamePt = '', + ?string $keywordsEn = '', + ?string $keywordsNl = '', + ?string $keywordsPt = '', + ?Logo $logo = null, + ?string $nameEn = '', + ?string $nameNl = '', + ?string $namePt = '', + ?string $nameIdFormat = null, + array $supportedNameIdFormats = array( Constants::NAMEID_TRANSIENT, Constants::NAMEID_PERSISTENT, ), - $requestsMustBeSigned = false, - $signatureMethod = XMLSecurityKey::RSA_SHA256, - $workflowState = self::WORKFLOW_STATE_DEFAULT, + bool $requestsMustBeSigned = false, + string $signatureMethod = XMLSecurityKey::RSA_SHA256, + string $workflowState = self::WORKFLOW_STATE_DEFAULT, array $allowedIdpEntityIds = array(), - $allowAll = false, + bool $allowAll = false, array $assertionConsumerServices = array(), - $displayUnconnectedIdpsWayf = false, - $termsOfServiceUrl = null, - $isConsentRequired = true, - $isTransparentIssuer = false, - $isTrustedProxy = false, + bool $displayUnconnectedIdpsWayf = false, + string $termsOfServiceUrl = null, + bool $isConsentRequired = true, + bool $isTransparentIssuer = false, + bool $isTrustedProxy = false, $requestedAttributes = null, - $skipDenormalization = false, - $policyEnforcementDecisionRequired = false, - $requesteridRequired = false, - $signResponse = false, - $manipulation = '', + bool $skipDenormalization = false, + bool $policyEnforcementDecisionRequired = false, + bool $requesteridRequired = false, + bool $signResponse = false, + string $manipulation = '', AttributeReleasePolicy $attributeReleasePolicy = null, - $supportUrlEn = null, - $supportUrlNl = null, - $supportUrlPt = null, - $stepupAllowNoToken = null, - $stepupRequireLoa = null + string $supportUrlEn = null, + string $supportUrlNl = null, + string $supportUrlPt = null, + bool $stepupAllowNoToken = null, + string $stepupRequireLoa = null, + bool $stepupForceAuthn = false ) { parent::__construct( $entityId, @@ -257,7 +210,8 @@ public function __construct( $stepupRequireLoa, $disableScoping, $additionalLogging, - $signatureMethod + $signatureMethod, + $stepupForceAuthn ); } diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/StepupMockController.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/StepupMockController.php index 05bb2a59dc..ae5ae817b7 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/StepupMockController.php +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Controllers/StepupMockController.php @@ -20,6 +20,7 @@ use Exception; use OpenConext\EngineBlockFunctionalTestingBundle\Mock\MockStepupGateway; use SAML2\Constants; +use SAML2\HTTPRedirect; use SAML2\Response as SamlResponse; use Symfony\Bundle\FrameworkBundle\Controller\Controller; use Symfony\Component\HttpFoundation\Request; @@ -62,10 +63,14 @@ public function ssoAction(Request $request) // Parse available responses $responses = $this->getAvailableResponses($request); + $redirectBinding = new HTTPRedirect(); + $message = $redirectBinding->receive(); + // Present response $body = $this->twig->render( '@OpenConextEngineBlockFunctionalTesting/Sso/consumeAssertion.html.twig', [ + 'receivedAuthnRequest' => $message->toUnsignedXML()->ownerDocument->saveXml(), 'responses' => $responses, ] ); diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php index 8b60123fa9..7e388c5edf 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/EngineBlockContext.php @@ -39,6 +39,9 @@ use RuntimeException; use SAML2\Constants; use SAML2\DOMDocumentFactory; +use function assertStringNotMatchesFormat; +use function assertStringStartsWith; +use function preg_match; use function sprintf; /** @@ -584,6 +587,34 @@ public function cleanUpAuthenticationLoopGuard() } } + /** + * @Then /^the received AuthnRequest should not match xpath '([^']*)'$/ + */ + public function theReceivedAuthnRequestShouldNotMatchXpath($xpath) + { + $session = $this->getMinkContext()->getSession(); + try { + $this->theAuthnRequestToSubmitShouldMatchXpath($xpath); + throw new RuntimeException('The xpath was found in the AuthnRequest, it should not'); + } catch (ExpectationException $e) { + if (false === preg_match('/The xpath "(w+)" did not result in at least one match./', $e->getMessage())) { + throw new ExpectationException( + 'Unexepected match on the xpath that should NOT match the AuthnRequest xml', + $session, + $e + ); + } + } + } + + /** + * @Then /^the received AuthnRequest should match xpath '([^']*)'$/ + */ + public function theReceivedAuthnRequestShouldMatchXpath($xpath) + { + return $this->theAuthnRequestToSubmitShouldMatchXpath($xpath); + } + /** * @Then /^the AuthnRequest to submit should match xpath '([^']*)'$/ */ @@ -591,7 +622,6 @@ public function theAuthnRequestToSubmitShouldMatchXpath($xpath) { $session = $this->getMinkContext()->getSession(); $mink = $session->getPage(); - $authnRequestElement = $mink->find('css', 'input[name="authnRequestXml"]'); if ($authnRequestElement === null) { throw new ExpectationException('Element with the name "authnRequestXml" could not be found', $session); diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/StepupContext.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/StepupContext.php index 1a24fbed82..0d1facf65d 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/StepupContext.php +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/StepupContext.php @@ -135,6 +135,19 @@ public function spAllowsNoStepup($spName) ->save(); } + /** + * @Given /^the SP "([^"]*)" forces stepup authentication$/ + */ + public function spForcesAuthn($spName) + { + /** @var MockServiceProvider $mockSp */ + $mockSp = $this->mockSpRegistry->get($spName); + + $this->serviceRegistryFixture + ->setStepupForceAuthn($mockSp->entityId(), true) + ->save(); + } + /** * @Given /^the SP "([^"]*)" requires Stepup LoA "([^"]*)"$/ */ diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Stepup.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Stepup.feature index 6d795d8b5b..c9f6cd6819 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Stepup.feature +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Stepup.feature @@ -81,7 +81,24 @@ Feature: And I pass through EngineBlock Then the url should match "/functional-testing/SSO-SP/acs" - Scenario: LoA 1 is allowed, but refrains from doing a step up callout + Scenario: Stepup authentication is forced when coin:stepup:forceauthn is configured for the SP + Given SP "SSO-SP" requests LoA "http://vm.openconext.org/assurance/loa3" + And the SP "SSO-SP" forces stepup authentication + 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[@ForceAuthn="true"]' + + Scenario: Stepup authentication is NOT forced when coin:stepup:forceauthn is not configured for the SP + Given SP "SSO-SP" requests LoA "http://vm.openconext.org/assurance/loa3" + 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[@ForceAuthn="true"]' + + Scenario: LoA 1 is allowed, but refrains from doing a step up callout Given SP "SSO-SP" requests LoA "http://vm.openconext.org/assurance/loa1" When I log in at "SSO-SP" And I select "SSO-IdP" on the WAYF @@ -191,6 +208,29 @@ Feature: And I pass through EngineBlock Then the url should match "/functional-testing/Proxy-SP/acs" + Scenario: Step-up ForceAuthn should be requested for the proxied SP when using a trusted proxy setup and if configured in the proxied SP + Given the SP "SSO-SP" requires Stepup LoA "http://vm.openconext.org/assurance/loa2" + And the SP "SSO-SP" forces stepup authentication + And SP "Proxy-SP" is authenticating for SP "SSO-SP" + And SP "Proxy-SP" is a trusted proxy + And SP "Proxy-SP" signs its requests + When I log in at "Proxy-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[@ForceAuthn="true"]' + + Scenario: Step-up ForceAuthn should not be requested for the proxied SP when using a trusted proxy setup and if configured in the proxied SP + Given the SP "SSO-SP" requires Stepup LoA "http://vm.openconext.org/assurance/loa2" + And SP "Proxy-SP" is authenticating for SP "SSO-SP" + And SP "Proxy-SP" is a trusted proxy + And SP "Proxy-SP" signs its requests + When I log in at "Proxy-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[@ForceAuthn="true"]' + Scenario: Stepup authentication should fail when stepup is misconfigured Given the SP "SSO-SP" requires Stepup LoA "http://typo-in-config.org/assurance/laos3" When I log in at "SSO-SP" diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Fixtures/ServiceRegistryFixture.php b/src/OpenConext/EngineBlockFunctionalTestingBundle/Fixtures/ServiceRegistryFixture.php index c754c16697..0cc1cba336 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Fixtures/ServiceRegistryFixture.php +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Fixtures/ServiceRegistryFixture.php @@ -401,6 +401,13 @@ public function setSpStepupRequireLoa($entityId, $requiredLoa) return $this; } + public function setStepupForceAuthn($entityId, $isForceAuthn) + { + $this->setCoin($this->getServiceProvider($entityId), 'stepupForceAuthn', $isForceAuthn); + + return $this; + } + public function setSpStepupAllowNoToken($entityId) { $this->setCoin($this->getServiceProvider($entityId), 'stepupAllowNoToken', true); diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/views/Sso/consumeAssertion.html.twig b/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/views/Sso/consumeAssertion.html.twig index 134807c01e..c9273d062d 100644 --- a/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/views/Sso/consumeAssertion.html.twig +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Resources/views/Sso/consumeAssertion.html.twig @@ -17,4 +17,8 @@ {% endfor %} + +
+ +
diff --git a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php index 8bb72cc001..d6092a3c30 100644 --- a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php +++ b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php @@ -123,7 +123,9 @@ public function testCoins($coinName, $roleType, $parameter, $type) { "allow_all_entities": true, "allowed_connections": [], "metadata": { - "coin": {} + "coin": { + "stepup": {} + } }, "name": "https:\/\/role/sp", "state": "prodaccepted", @@ -138,6 +140,9 @@ public function testCoins($coinName, $roleType, $parameter, $type) { case 'bool'; $values = $this->validCoinValuesBool(); break; + case 'bool-forceAuthn'; + $values = $this->validCoinValuesBoolForceAuthn(); + break; case 'bool-negative'; $values = $this->validCoinValuesBoolNegative(); break; @@ -155,11 +160,19 @@ public function testCoins($coinName, $roleType, $parameter, $type) { } foreach ($values as $assertion) { - $input->{"2d96e27a-76cf-4ca2-ac70-ece5d4c49523"}->metadata->coin->$coinName = $assertion[0]; - + if ($coinName === 'forceauthn') { + // forceauthn is a stepup coin, situated in an additional json object named: stepup + $input->{"2d96e27a-76cf-4ca2-ac70-ece5d4c49523"}->metadata->coin->stepup->$coinName = $assertion[0]; + } else { + $input->{"2d96e27a-76cf-4ca2-ac70-ece5d4c49523"}->metadata->coin->$coinName = $assertion[0]; + } $roles = $this->assembler->assemble($input); - $this->assertSame($assertion[1], $roles[0]->getCoins()->{$parameter}(), "Invalid coin conversion for {$roleType}:{$coinName}($type) expected '{$assertion[1]}' but encountered '{$roles[0]->getCoins()->{$parameter}()}'"); + $this->assertSame( + $assertion[1], + $roles[0]->getCoins()->{$parameter}(), + "Invalid coin conversion for {$roleType}:{$coinName}($type) expected '{$assertion[1]}' but encountered '{$roles[0]->getCoins()->{$parameter}()}'" + ); } } @@ -354,6 +367,7 @@ public function validCoins() ['policy_enforcement_decision_required', 'saml20-sp', 'policyEnforcementDecisionRequired', 'bool'], ['requesterid_required', 'saml20-sp', 'requesteridRequired', 'bool'], ['sign_response', 'saml20-sp', 'signResponse', 'bool'], + ['forceauthn', 'saml20-sp', 'isStepupForceAuthn', 'bool-forceAuthn'], // IDP ['guest_qualifier', 'saml20-idp', 'guestQualifier', 'string-guest-qualifier'], @@ -432,4 +446,12 @@ private function validCoinValuesStringGuestQualifier() { ["string", "string"], ]; } + + private function validCoinValuesBoolForceAuthn() + { + return [ + ["1", true], + ["0", false] + ]; + } } diff --git a/tests/unit/OpenConext/EngineBlock/Metadata/Factory/Factory/ServiceProviderFactoryTest.php b/tests/unit/OpenConext/EngineBlock/Metadata/Factory/Factory/ServiceProviderFactoryTest.php index f4030530cc..3dd469c708 100644 --- a/tests/unit/OpenConext/EngineBlock/Metadata/Factory/Factory/ServiceProviderFactoryTest.php +++ b/tests/unit/OpenConext/EngineBlock/Metadata/Factory/Factory/ServiceProviderFactoryTest.php @@ -210,7 +210,8 @@ public function test_eb_properties() null, false, false, - XMLSecurityKey::RSA_SHA256 + XMLSecurityKey::RSA_SHA256, + false ); $overrides['attributeReleasePolicy'] = null; $overrides['allowedIdpEntityIds'] = []; @@ -344,7 +345,8 @@ public function test_stepup_properties() null, false, false, - XMLSecurityKey::RSA_SHA256 + XMLSecurityKey::RSA_SHA256, + false ); $overrides['attributeReleasePolicy'] = null; $overrides['allowedIdpEntityIds'] = [];