New 4.x branch for Drupal 10 compatibility with SimpleSamlPHP dev-master

Worked a bit on this, but this is all blocked on being able to install simplesamlphp on Drupal 10/Symfony6.

See https://github.com/simplesamlphp/simplesamlphp/issues/1588 and https://github.com/simplesamlphp/simplesamlphp/pull/1675.

Remove core, also update the test module.

Lets see what happens if we do this. I had two merge requests merged into simplesamlphp/simplesamlphp, requiring dev-master into a Drupal 10 project works now, but there's still a minor thing with symfony/translation-contracts versions, not sure if DrupalCI can handle that.

I think this is good to go. I saw that the simplesamlphp_auth code Berdir got merged upstream was reverted from the 2.0 branch.. hence the dev-master reference. We can always update this later once it works with 2.0.

As discussed, I disagree.

Depending on a development branch is not compatible with a stable branch, they could introduce breaking changes at any point.

And 2.x will likely never be PHP 8 compatible, but a 3.x release might not be too far off: https://github.com/simplesamlphp/simplesamlphp/pull/1675#issuecomment-12....

So, I will likely create a new 4.x branch, commit this there, maybe support only dev-master, maybe 1.x and dev-master, and then switch to 3.x once that's available and make it stable.

Here is an updated patch that will be committed to a new 4.x branch, upped the core version to 9.4 and simplified the moduleHandler changes. Ignored the coding standard changes in the MR as these should not only be in 4.x and also dropped some of my minor phpunit class cleanups, same for that.

Oh, new test fails, classy is gone. Considering to backport a handful of the changes to 8.x-3.x as well, like this to keep future updates easier to be in sync.

https://www.drupal.org/project/simplesamlphp_auth/releases/4.x-dev

Keeping this open for now while I consider to backport some of the changes to keep the branches in sync.

I attempted to use this release of the module to allow us to update our php version to 8.x, but I got an error message about simpleSAMLphp v1.19.1 being incompatible with php versions equal or greater than 8.0. I read in the simpleSAMLphp git issues something that made me think there was a version of simpleSAMLphp that was compatible with php 8.x. I only noticed this morning that the version of Drupal we're using refuses to do database updates because our version of php is out of date (7.4.x). This is a pretty big issue, because we require simpleSAMLphp for authentication, but we need this module to be updated. Is there a different version of this we should use to be compatible with php 8?

See #3285729: PHP 8 Incompatibility, simplesamlphp is PHP 8+ compatible, you just need to make sure you are using the most recent patch version, require that explicitly.

Actually, it looks like I was able to update the SimpleSAMLphp library to 1.19.6, and it looks like that version is compatible with php 8.0.24. I'm not 100% sure it's truly compatible; there are some issues in the SSP github issue queue that indicate that there are blocking bugs that still haven't been resolved.

Thanks @Berdir!

1. +++ b/src/Service/SimplesamlphpAuthManager.php
   @@ -292,12 +292,8 @@ class SimplesamlphpAuthManager {
   -    foreach ($this->moduleHandler->getImplementations('simplesamlphp_auth_allow_login') as $module) {
   -      if ($this->moduleHandler->invoke($module, 'simplesamlphp_auth_allow_login', [$attributes]) === FALSE) {
   -        return FALSE;
   -      }
   -    }
   -    return TRUE;
   +    $responses = $this->moduleHandler->invokeAll('simplesamlphp_auth_allow_login', [$attributes]);
   +    return !in_array(FALSE, $responses, TRUE);
   

   The previous code would exit on the first FALSE whereas the new code will fire all hooks potentially wasting cycles. Not sure it matters but pointing this out.

2. +++ b/src/Service/SimplesamlphpDrupalAuth.php
   @@ -179,12 +179,12 @@ class SimplesamlphpDrupalAuth {
   -      if ($this->config->get('autoenablesaml')) {
   -        // Allow other modules to decide if there is an existing Drupal user,
   -        // based on the supplied SAML atttributes.
   -        $attributes = $this->simplesamlAuth->getAttributes();
   -        foreach ($this->moduleHandler->getImplementations('simplesamlphp_auth_existing_user') as $module) {
   -          $return_value = $this->moduleHandler->invoke($module, 'simplesamlphp_auth_existing_user', [$attributes]);
   +      // Allow other modules to decide if there is an existing Drupal user,
   +      // based on the supplied SAML atttributes.
   +      $attributes = $this->simplesamlAuth->getAttributes();
   +      if (method_exists($this->moduleHandler, 'invokeAllWith')) {
   +        $this->moduleHandler->invokeAllWith('simplesamlphp_auth_existing_user', function (callable $hook) use ($attributes, $authname) {
   +          $return_value = $hook($attributes);
              if ($return_value instanceof UserInterface) {
   

   This change doesn't look right for two reasons:
   1. We're no longer wrapping if ($this->config->get('autoenablesaml')) {
   2. We're no longer setting $account in the main method here - so logic that follows on from this is likely to be broken.

Addressed #21 in #3334908: Make 4.x behave the same way as 8.x-3.x

Here's a patch for 8.x-3.x that changes the minimum version to 9.4 but doesn't yet allow Drupal 10 and fixes all the deprecations. If there was an 8.x-3.x release with this change it'd be easier to prepare projects for Drupal 10. We could even consider allowing D10 on this branch and making 4.x just about supporting the new version of the library when it comes out.

9.4 is the lowest security supported version of Drupal 9 and Drupal 8 is out of support so I think this change is okay.

Note this patch includes the improvements from #3334908: Make 4.x behave the same way as 8.x-3.x

Missed something... oops... Forgot to add back in the check for if ($this->config->get('autoenablesaml')) { that was remvoed by #13

Thanks, did run the tests manually for now. And 4.x and 8.x-3.x are identical now except the core requirement and dependencies.

I am running into this error using the 4.x branch. Unable to install module simplesamlphp/simplesamlphp-assets-base, package name must be on the form "VENDOR/simplesamlphp-module-MODULENAME". For context, we are setting up a test instance of Drupal 10 as a starting point for our upgrade from Drupal 9. Currently, this is the only remaining blocker to getting composer to update.

I am happy to work on this, but I am unsure how to start. I did look through the https://github.com/simplesamlphp/simplesamlphp repo. I noticed that the simplasamlphp-assets-base package doesn't seem to make it into their releases.

i don't know what depends on it and why you get that error, but we have passing tests on D10. That doesn't mean much, but it it at least shows that this 4.x *can* be installed on D10. i suggest you create a new issue and check where/why you depend on that. Maybe that no longer exists on dev-master?

Hello,

I know this thread is closed, but someone can help me please.

I tried to download the module from branch 4 with composer, but I get this error:

simplesamlphp/simplesamlphp[v1.19.0, ..., v1.19.1] require php >=7.1 <8.0 -> your php version (8.1.15) does not satisfy that requirement.
- simplesamlphp/simplesamlphp[v1.19.2, ..., v1.19.7] require symfony/console ^4.4 || ^5.0 -> found symfony/console[v4.4.0, ..., v4.4.49, v5.0.0, ..., v5.4.19] but the package is fixed to v6.2.5 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.
- simplesamlphp/simplesamlphp v1.19.6 requires gettext/gettext v4.x-dev#3e7460f8d9c90174824e3f39240bd578bb3d376a -> found gettext/gettext[4.x-dev] but it does not match your minimum-stability.
- drupal/simplesamlphp_auth 4.x-dev is an alias of drupal/simplesamlphp_auth dev-4.x and thus requires it to be installed too.
- drupal/simplesamlphp_auth dev-4.x requires simplesamlphp/simplesamlphp ^1.19 | dev-master -> satisfiable by simplesamlphp/simplesamlphp[v1.19.0, ..., v1.19.7].
- Root composer.json requires drupal/simplesamlphp_auth 4.x-dev@dev -> satisfiable by drupal/simplesamlphp_auth[4.x-dev (alias of dev-4.x)].

But D10 require php 8 or higher.

How can I install this module from branch 4, compatible with D10, but get error on php version?

Thanks in advance

Same error as #27

I am running into this error using the 4.x branch. Unable to install module simplesamlphp/simplesamlphp-assets-base, package name must be on the form "VENDOR/simplesamlphp-module-MODULENAME"

This occurs if you have previously installed the simplesamlphp assets into the vendor directory before updating. You can resolve this by removing the vendor directory, then doing the composer update. FWIW, this is not really an issue with the simplesamlphp_auth Drupal module.

I tried to download the module from branch 4 with composer, but I get this error:
simplesamlphp/simplesamlphp[v1.19.0, ..., v1.19.1] require php >=7.1 <8.0 -> your php version (8.1.15) does not satisfy that requirement.

Try explicitly requiring simplesamlphp/simplesamlphp ^2 first.