More graceful handling in GroupUserUpdateProcessor on invalid configuration [#3192905]

Problem/Motivation

If a user could not be created by the ldap module because the ldap user has no mail property the following errors shows up in the watchdog log

Failed to create Drupal account 'SAMAAccount-Property' because email address could not be derived by LDAP User module

But in the user interface the following error show up on near by every admin site:

The website encountered an unexpected error. Please try again later.
TypeError: Argument 1 passed to Drupal\ldap_user\Processor\DrupalUserProcessor::drupalUserLogsIn() must implement interface Drupal\user\UserInterface, null given, called in C:\bin\workspaces\Krzn\310_Dinslaken\m310-www\docroot\modules\contrib\ldap\ldap_user\src\Processor\GroupUserUpdateProcessor.php on line 300 in Drupal\ldap_user\Processor\DrupalUserProcessor->drupalUserLogsIn() (line 449 of modules\contrib\ldap\ldap_user\src\Processor\DrupalUserProcessor.php).
Drupal\ldap_user\Processor\DrupalUserProcessor->drupalUserLogsIn(NULL) (Line: 300)
Drupal\ldap_user\Processor\GroupUserUpdateProcessor->processAccount(Object, 'samaccountname') (Line: 244)
Drupal\ldap_user\Processor\GroupUserUpdateProcessor->runQuery('accounts') (Line: 38)
ldap_user_cron()
call_user_func_array('ldap_user_cron', Array) (Line: 392)
Drupal\Core\Extension\ModuleHandler->invoke('ldap_user', 'cron') (Line: 236)
Drupal\Core\Cron->invokeCronHandlers() (Line: 134)
Drupal\Core\Cron->run() (Line: 75)
Drupal\Core\ProxyClass\Cron->run() (Line: 65)
Drupal\automated_cron\EventSubscriber\AutomatedCron->onTerminate(Object, 'kernel.terminate', Object)
call_user_func(Array, Object, 'kernel.terminate', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.terminate', Object) (Line: 88)
Symfony\Component\HttpKernel\HttpKernel->terminate(Object, Object) (Line: 32)
Stack\StackedHttpKernel->terminate(Object, Object) (Line: 686)
Drupal\Core\DrupalKernel->terminate(Object, Object) (Line: 22)

Steps to reproduce

Create a User without any mail property in you LDAP
Try to run the cron

Proposed resolution

I think the source have to indentified and a save exit or jump over and log should be considered.

Comment	File	Size	Author
#9	ldap-fatal_during_cron_on_email_conflict-3192905-9.patch	696 bytes	aaronbauman
#7	ldap-fatal_during_cron_on_email_conflict-3192905-7.patch	760 bytes	aaronbauman

Issue fork ldap-3192905

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

3192905-more-graceful-handling changes, plain diff MR !32
Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

15 January 2021 at 06:42

sunlix created an issue. See original summary.

Comment #2

grahl

he/him

Zürich

commented 17 January 2021 at 19:28

Assigned:

Unassigned

» grahl

On the one hand I'd consider this "working as designed" since this seems to be a missing account name attribute (and not email). Though if it were "mail" the email templates for missing addresses are intended to be used here where accounts are missing that attribute.

But I agree that we can be a bit more graceful here in the bulk updating and not error out in that case.

Comment #3

sunlix

English

Wesel

commented 18 January 2021 at 08:51

In my case it was the mail attribute. My colleague has added this to the AD entry and things became fine again. Because the entry was 10 years old and back in that time the mail attribute was not a required field.

The problem from a usabillity perspective is, that other people manage the AD/LDAP's than the Drupal developer.
In my case a new user was added to the permitted Drupal group but without the mail attribute.
By adding the new user the production system was "broken" (feeling by looking at the error messages printed out).

It would not be that big thing in case the cron would eject or ignore the broken user entry from AD/LDAP.
But it did not end gracefully, so you get the stack trace on any admin page.

Comment #4

grahl

he/him

Zürich

commented 16 May 2021 at 17:59

Title:	Massive errors if user could not be created by LDAP module	» More graceful handling in GroupUserUpdateProcessor on invalid configuration
Version:	8.x-4.0-beta1	» 8.x-4.x-dev
Assigned:	grahl	» Unassigned
Priority:	Normal	» Minor

Comment #5

aaronbauman

he/him

commented 16 February 2023 at 20:49

Similar issue here, and it's happening on the latest release.
If I have an LDAP user with an email that already exists in Drupal, what's the suggested workaround here?

Currently ldap_user cron is breaking cron entirely, which is obviously untenable.

Comment #6

rudi teschner commented 14 March 2023 at 14:44

In my opinion the only plausible solution would be to skip the conflicting user, add the proper watchdog message and continue syncing the rest of the users.

Comment #7

aaronbauman

he/him

commented 14 March 2023 at 14:50

Priority:	Minor	» Normal
Status:	Active	» Needs review

Status	File	Size
new	ldap-fatal_during_cron_on_email_conflict-3192905-7.patch	760 bytes

Here's the patch that's working for me.
Setting priority back to Normal, but I could see a case that throwing a fatal during cron run is at least Major.

Comment #8

rudi teschner commented 14 March 2023 at 15:32

The patch is looking good, at first glance it also prevents a warning I got

Warning: array_flip(): Can only flip STRING and INTEGER values! in Drupal\Core\Entity\EntityStorageBase->loadMultiple() (line 312 of core/lib/Drupal/Core/Entity/EntityStorageBase.php).
Drupal\Core\Entity\EntityStorageBase->loadMultiple() (Line: 296)
Drupal\Core\Entity\EntityStorageBase->load() (Line: 301)
Drupal\ldap_user\Processor\GroupUserUpdateProcessor->processAccount() (Line: 246)
Drupal\ldap_user\Processor\GroupUserUpdateProcessor->runQuery() (Line: 42)
ldap_user_cron()