search

OpenConext / Stepup-saml-bundle

A PHP Symfony bundle that adds SAML capabilities to your application using simplesamlphp/saml2
Apache License 2.0
14 stars 24 forks source link

Return null when AuthRequest has no Subject NameID #121

Closed pmeulen closed 1 year ago

pmeulen commented 1 year ago

When using AuthnRequest::GetNameId() or AuthnRequest::getNameIdFormat() on an AuthnRequest without a Subject these functions used to return null, currently they throw an exception because of a null dereference. This PR reinstates the old behaviour and adds a test for this scenario.

MKodde commented 1 year ago

I appreciate this fix. And it will solve some of the issues that have already been addressed in #119 .

Some things I'd like to investigate before moving forward with this:

  1. Why is gateway running into these problems. Why is the newer SAML bundle used in gateway? Downgrading there might be a more pragmatic solution. If we want to use a modern SAML2 library in the next GW release then this suggestion can be scrapped.
  2. We could merge #119 adding full SAML2 v4.6.4 support. But that also adds SF5 and PHP 8 support. And might not work with older projects. (I'll look into this)
  3. We could go forward with your work, but I feel we should fix more 4.6.4 compatibility issues while at it.

Let's discuss the best approach in the stand up later this morning.