From 4828d217d71e6d3a739c9c9d6fe180dc1bd17e8d Mon Sep 17 00:00:00 2001 From: Michiel Kodde Date: Wed, 4 Sep 2024 08:59:39 +0200 Subject: [PATCH] Ensure the NameID is correctly set Due to the NameId juggling taking place during the Input filtering. There MUST be a point in time when we set the correct outgoing NameId value and format. And that is done in the AddIdentityAttributes filter. During the optimization I moved the 'were done here' return statement to a block that seemed to make more sense to me. But that caused the NameId from not being set even though it was resolved. So I moved that block back to the position where it is supposed to be. It is meant as a guard from injustly overwriting the eduPersonTargettedId. Not for preventing the setting of the correct NameId in the subject. --- .../Filter/Command/AddIdentityAttributes.php | 11 +-- .../Features/NameIdFormat.feature | 69 +++++++++++++++++++ 2 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 src/OpenConext/EngineBlockFunctionalTestingBundle/Features/NameIdFormat.feature diff --git a/library/EngineBlock/Corto/Filter/Command/AddIdentityAttributes.php b/library/EngineBlock/Corto/Filter/Command/AddIdentityAttributes.php index 49326b28b..be6ba7df9 100644 --- a/library/EngineBlock/Corto/Filter/Command/AddIdentityAttributes.php +++ b/library/EngineBlock/Corto/Filter/Command/AddIdentityAttributes.php @@ -16,6 +16,7 @@ * limitations under the License. */ +use OpenConext\EngineBlock\Metadata\AttributeReleasePolicy; use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider; use Psr\Log\LoggerInterface; use SAML2\Constants; @@ -84,11 +85,6 @@ public function execute() $isResolved = true; $this->_response->getAssertion()->setNameId($nameId); } - - // If there's an ARP, but it does not contain the EPTI, we're done now. - if (!$arp->hasAttribute(Constants::EPTI_URN_MACE)) { - return; - } } if (!$isResolved || !isset($nameId)){ @@ -105,6 +101,11 @@ public function execute() $this->_response->getAssertion()->setNameId($nameId); } + // If there's an ARP, but it does not contain the EPTI, we're done now. + if ($arp instanceof AttributeReleasePolicy && !$arp->hasAttribute(Constants::EPTI_URN_MACE)) { + return; + } + // We arrive here if either: // 1) the ARP is NULL, this means no ARP = let everything through including EPTI; or // 2) the ARP is not null and does contain the EPTI attribute. diff --git a/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/NameIdFormat.feature b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/NameIdFormat.feature new file mode 100644 index 000000000..b3ebd3276 --- /dev/null +++ b/src/OpenConext/EngineBlockFunctionalTestingBundle/Features/NameIdFormat.feature @@ -0,0 +1,69 @@ +Feature: + To ensure no confusion about the NameID Format + As EngineBlock + I want to be sure after ARP my name id format is presented correctly to the SP + + Background: + Given an EngineBlock instance on "vm.openconext.org" + And no registered SPs + And no registered Idps + And an Identity Provider named "SSO-IdP" + And a Service Provider named "SSO-SP" + + Scenario: EngineBlock should not update the Unspecified NameIdFormat when no ARP filters are applied + Given SP "SSO-SP" uses the Unspecified NameID format + When I log in at "SSO-SP" + And I pass through EngineBlock + And I pass through the IdP + When I give my consent + And I pass through EngineBlock + And the response should match xpath '/samlp:Response/saml:Assertion/saml:Subject/saml:NameID[@Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified"]' + + Scenario: EngineBlock should not update the Unspecified NameIdFormat when the ARP is applied + Given SP "SSO-SP" uses the Unspecified NameID format + And SP "SSO-SP" allows an attribute named "urn:mace:dir:attribute-def:uid" + When I log in at "SSO-SP" + And I pass through EngineBlock + And I pass through the IdP + When I give my consent + And I pass through EngineBlock + And the response should match xpath '/samlp:Response/saml:Assertion/saml:Subject/saml:NameID[@Format="urn:oasis:names:tc:SAML:1.1:nameid-format:unspecified"]' + + Scenario: EngineBlock should not update the Persistent NameIdFormat when no ARP filters are applied + Given SP "SSO-SP" uses the Persistent NameID format + When I log in at "SSO-SP" + And I pass through EngineBlock + And I pass through the IdP + When I give my consent + And I pass through EngineBlock + And the response should match xpath '/samlp:Response/saml:Assertion/saml:Subject/saml:NameID[@Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"]' + + Scenario: EngineBlock should not update the Persistent NameIdFormat when the ARP is applied + Given SP "SSO-SP" uses the Persistent NameID format + And SP "SSO-SP" allows an attribute named "urn:mace:dir:attribute-def:uid" + And SP "SSO-SP" allows an attribute named "urn:mace:terena.org:attribute-def:schacHomeOrganization" + When I log in at "SSO-SP" + And I pass through EngineBlock + And I pass through the IdP + When I give my consent + And I pass through EngineBlock + And the response should match xpath '/samlp:Response/saml:Assertion/saml:Subject/saml:NameID[@Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"]' + + Scenario: EngineBlock should not update the Transient NameIdFormat when no ARP filters are applied + Given SP "SSO-SP" uses the Transient NameID format + When I log in at "SSO-SP" + And I pass through EngineBlock + And I pass through the IdP + When I give my consent + And I pass through EngineBlock + And the response should match xpath '/samlp:Response/saml:Assertion/saml:Subject/saml:NameID[@Format="urn:oasis:names:tc:SAML:2.0:nameid-format:transient"]' + + Scenario: EngineBlock should not update the Transient NameIdFormat when the ARP is applied + Given SP "SSO-SP" uses the Transient NameID format + And SP "SSO-SP" allows an attribute named "urn:mace:terena.org:attribute-def:schacHomeOrganization" + When I log in at "SSO-SP" + And I pass through EngineBlock + And I pass through the IdP + When I give my consent + And I pass through EngineBlock + And the response should match xpath '/samlp:Response/saml:Assertion/saml:Subject/saml:NameID[@Format="urn:oasis:names:tc:SAML:2.0:nameid-format:transient"]'