[PHP 8.1] [Drupal 10] Upgrade grahl/ldap fork to follow symfony/ldap 5.4.x and require graph/ldap:^5.4

We also have the issue for Collection::offsetUnset()

Deprecated function: Return type of Symfony\Component\Ldap\Adapter\ExtLdap\Collection::offsetUnset($offset) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void

The original symfony/ldap has its last 5.x version 5.4.8 which implements #[\ReturnTypeWillChange].

Could you at least update grahl/ldap fork up to 5.4.8 so we don't broke things switching to new version, but at least have no warning with PHP 8.1 ?

Given that grahl/ldap is intended to be backwards compatible to PHP 5.3

I don't understand why micahw156 talk about backwards compatibility with PHP 5.3, since the module require at least PHP 7.2.

So, any chance we update grahl/ldap fork up to 5.4.8 please ?

@micahw156 PHP 5.3 is supported for Drupal 7 core

Here are the PHP requirements for Drupal 8+

There are more than just a couple. Here are all the ones I get:

Deprecated function: Return type of Symfony\Component\Ldap\Adapter\ExtLdap\Collection::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 21 of /var/www/html/vendor/grahl/ldap/Adapter/ExtLdap/Collection.php)

Deprecated function: Return type of Symfony\Component\Ldap\Adapter\ExtLdap\Collection::getIterator() should either be compatible with IteratorAggregate::getIterator(): Traversable, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 21 of /var/www/html/vendor/grahl/ldap/Adapter/ExtLdap/Collection.php)

Deprecated function: Return type of Symfony\Component\Ldap\Adapter\ExtLdap\Collection::offsetExists($offset) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 21 of /var/www/html/vendor/grahl/ldap/Adapter/ExtLdap/Collection.php)

Deprecated function: Return type of Symfony\Component\Ldap\Adapter\ExtLdap\Collection::offsetGet($offset) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 21 of /var/www/html/vendor/grahl/ldap/Adapter/ExtLdap/Collection.php)

Deprecated function: Return type of Symfony\Component\Ldap\Adapter\ExtLdap\Collection::offsetSet($offset, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 21 of /var/www/html/vendor/grahl/ldap/Adapter/ExtLdap/Collection.php)

Deprecated function: Return type of Symfony\Component\Ldap\Adapter\ExtLdap\Collection::offsetUnset($offset) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 21 of /var/www/html/vendor/grahl/ldap/Adapter/ExtLdap/Collection.php)

+1 tried using the official Drupal Image
FROM drupal:9.4.2-php8.1-apache-buster
then went back to
FROM drupal:9.4.2-php8.0-apache-buster
and it works

Updating issue name to more accurately represent the issue.

I think we still can't improve subject from #10, this subject does not well represent the issue.

The 5.4 version is confusing between php version and graph/ldap

Patch to remove notice waiting for grahl update

I'm bumping this up to major. PHP 7.4 is EOL this month. We also need to figure out what to do about grahl/ldap.

I looked at the Drupal user profile for grahl, and his bio says

no longer actively using Drupal

He also has not updated grahl/ldap in 3 years, and he hasn't committed anything to this module in over a year (and the module page says "Seeking new maintainer"). It's probably safe to say that we shouldn't use an unmaintained project as a dependency. Should we fork that grahl/ldap project, or what should be done about that? Nobody else has access to maintain that code, as far as I know...

In my opinion, we should use https://github.com/symfony/ldap instead of a fork that isn't being actively updated or maintained. I understand that there's an issue with Drupal CI because it requires a PHP extension in composer.json, but maybe that can be fixed with something like this: https://stackoverflow.com/a/55431247

I still think we should directly use symfony/ldap instead of grahl/ldap, which is unmaintained, but I'm updating the title to be more compact.

Hi,
Thanks to all those are working on this module. I understand that this is not the right place for posting this. While trying to install ldap module in fresh drupal 9.5 rc site, I understand that this module depends on "authorization" module which is yet to have a stable release for drupal 9.x. Since it is not yet released, automated drupal 10 compatibility test is not happening.

Hello everyone, I am also interested in learning what the steps would be to adopting symfony/ldap such that the issue @solidegloria identified in #17 could be solved and accepted for D9 and D10 for the whole community. Thank you!

{Edit: I started to look at the history that prompted @grahl to do the big task of forking symfony/ldap. I wonder if maybe there could be another version (10.x or 3.x, etc.) with the requirement of PHP 8+ to help with 3097854#4]

Posted a question here: #2949210: Drupalci Roadmap

Maybe there is a way to use drupalci.yml to install the php-ldap extension?

Hello,
i applied patch #12 to vendor/grahl/ldap/Adapter/ExtLdap/Collection.php but i'm still getting notice which reveals LDAP user password!!

Deprecated function: Return type of Symfony\Component\OptionsResolver\OptionsResolver::offsetExists($option) should either be compatible with ArrayAccess::offsetExists(mixed $offset): bool, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 28 of vendor/symfony/options-resolver/OptionsResolver.php).
include('/var/www/html/drupal9/vendor/symfony/options-resolver/OptionsResolver.php') (Line: 578)
Composer\Autoload\ClassLoader::Composer\Autoload\{closure}('/var/www/html/drupal9/vendor/composer/../symfony/options-resolver/OptionsResolver.php') (Line: 432)
Composer\Autoload\ClassLoader->loadClass('Symfony\Component\OptionsResolver\OptionsResolver') (Line: 26)
Symfony\Component\Ldap\Adapter\AbstractConnection->__construct(Array) (Line: 41)
Symfony\Component\Ldap\Adapter\ExtLdap\Adapter->getConnection() (Line: 40)
Symfony\Component\Ldap\Ldap->bind('cn=xxxx', 'PASSWORD') (Line: 143)
Drupal\ldap_servers\LdapBridge->bind() (Line: 907)
Drupal\ldap_authentication\Controller\LoginValidatorBase->bindToServer() (Line: 114)
Drupal\ldap_authentication\Controller\LoginValidatorLoginForm->testCredentials() (Line: 51)
Drupal\ldap_authentication\Controller\LoginValidatorLoginForm->processLogin() (Line: 34)
Drupal\ldap_authentication\Controller\LoginValidatorLoginForm->validateLogin(Object) (Line: 224)
ldap_authentication_user_login_authenticate_validate(Array, Object)
call_user_func_array('ldap_authentication_user_login_authenticate_validate', Array) (Line: 82)
Drupal\Core\Form\FormValidator->executeValidateHandlers(Array, Object) (Line: 275)
Drupal\Core\Form\FormValidator->doValidateForm(Array, Object, 'user_login_form') (Line: 118)
Drupal\Core\Form\FormValidator->validateForm('user_login_form', Array, Object) (Line: 591)
Drupal\Core\Form\FormBuilder->processForm('user_login_form', Array, Object) (Line: 323)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 580)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 169)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 81)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 707)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

The same in:
Deprecated function: Return type of Symfony\Component\OptionsResolver\OptionsResolver::offsetGet($option, bool $triggerDeprecation = true) should either be compatible with ArrayAccess::offsetGet(mixed $offset): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 28 of vendor/symfony/options-resolver/OptionsResolver.php).

Deprecated function: Return type of Symfony\Component\OptionsResolver\OptionsResolver::offsetSet($option, $value) should either be compatible with ArrayAccess::offsetSet(mixed $offset, mixed $value): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 28 of vendor/symfony/options-resolver/OptionsResolver.php).

Deprecated function: Return type of Symfony\Component\OptionsResolver\OptionsResolver::offsetUnset($option) should either be compatible with ArrayAccess::offsetUnset(mixed $offset): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 28 of vendor/symfony/options-resolver/OptionsResolver.php).

Deprecated function: Return type of Symfony\Component\OptionsResolver\OptionsResolver::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in include() (line 28 of vendor/symfony/options-resolver/OptionsResolver.php).

Why patch #12 didn't help?

I recently upgraded Drupal to 9.5.0 and php to 8.1

You cannot patch a vendor file, as it will be reinstalled with Composer every time composer install is run, I think.

Also, maybe this is a good place to use the new SensitiveParameter attribute as of PHP 8.2: https://php.watch/versions/8.2/backtrace-parameter-redaction

> You cannot patch a vendor file, as it will be reinstalled with Composer every time composer install is run, I think.

With cweagans/composer-patches you can patch every composer managed code library. When included and configured to use a patch this will be done when composer update or install process runs.

The patch in question is not for this Drupal module; it's for the grahl/ldap project on GitHub, which is a dependency of this module.

The Drupal-10-compatibility-issue has a patch, which includes the dependency replacement (grahl/ldap becomes synfony/ldap): #3297516-7: Automated Drupal 10 compatibility fixes.
Seems the approach there was: declare "ext-ldap" dependency and use "--ignore-platform-reqs" to bypass tests here.
Theoretically that should be an issue for automated test here, but testing, and more importantly using, the module in projects should be possible.

#3297516: Automated Drupal 10 compatibility fixes