simplesamlphp / saml2

SimpleSAMLphp low-level SAML2 PHP library
https://www.simplesamlphp.org
GNU Lesser General Public License v2.1
284 stars 135 forks source link

REGRESSION: Unhandled exception for `Class "SAML2\Assert" not found` in `src/SAML2/HTTPPost.php` #338

Closed utexas-wcms closed 1 year ago

utexas-wcms commented 1 year ago

Describe the problem

After the update to version 4.6.9 we are getting the following error on all SAML login attempts:

SimpleSAML\Error\Error: UNHANDLEDEXCEPTION

Backtrace:
1 www/_include.php:20 (SimpleSAML_exception_handler)
0 [builtin] (N/A)
Caused by: Error: Class "SAML2\Assert" not found
Backtrace:
3 /code/vendor/simplesamlphp/saml2/src/SAML2/HTTPPost.php:98 (SAML2\HTTPPost::receive)
2 modules/saml/www/sp/saml2-acs.php:35 (require)
1 lib/SimpleSAML/Module.php:266 (SimpleSAML\Module::process)
0 www/module.php:10 (N/A)

Additional information

It appears that a call to Assert::notNull was added to src/SAML2/HTTPPost.php in https://github.com/simplesamlphp/saml2/commit/b4daf21db8dba7dd6c3591286794ce5c9ccc7dc1, but there is no corresponding use statement for use Webmozart\Assert\Assert; at the top of that file.

gravelpot commented 1 year ago

@tvdijen I inadvertently posted this issue from our organizational account, so wanted to identify myself as the actual reporter. Any feedback you have is very welcome -- we've got a lot of sites with broken auth right now.

akinspe commented 1 year ago

This is a serious problem. Basically 4.6.9 is completely broken

markfullmer commented 1 year ago

FWIW, our immediate workaround was to require "simplesamlphp/saml2":"4.6.8"

akinspe commented 1 year ago

@tvdijen I've created #339 to fix this. It would be great if this could be released ASAP

forevermatt commented 1 year ago

FWIW, our immediate workaround was to require "simplesamlphp/saml2":"4.6.8"

Another simple fix is to add this to your composer.json file, if you want to automatically allow the fix when it comes:

    "conflict": {
        "simplesamlphp/saml2": "4.6.9"
    },
thijskh commented 1 year ago

I've tagged v4.6.10 now. Sorry for the inconvenience!

tvdijen commented 1 year ago

Oh wow, this escalated quickly 😲 ! Really sorry folks!

tvdijen commented 1 year ago

Just to elaborate a bit on how this has happened;

We basically have muted our static analysis tool because at this point it reports to many errors to deal with. So, yay, our tests pass and I tag a new release, not noticing that our quality test went from one-hundred-something issues to one-hundred-something +1.

To make sure this doesn't happen again, I will unmute the static analysis and lower the bar a bit for it to pass, so that fundamental issues like this are caught at an early stage. We set the bar too high and well, it led to this mess.. Again sorry the inconvenience!