Problem/Motivation
The status report now contains a check for duplicate email address and reports them with the heading "Conflicting user emails". However, accounts may be created without an email, i.e. with a NULL email, and these accounts incorrectly trigger the warning. Further, the drupal.org page linked to by the message contains a SQL query that does not return the NULL email messages, confusing the administrator as to which accounts are causing the warning.
Steps to reproduce
Create two different user accounts, both without an email address. View the status report and note the warning about conflicting user emails.
Proposed resolution
Exclude NULL email messages from the query that triggers the warning. In user.install, userRequirements():
$query = \Drupal::database()->select('users_field_data');
$query->addExpression('LOWER(mail)', 'lower_mail');
$query->groupBy('lower_mail');
$query->having('COUNT(uid) > :matches', [':matches' => 1]);
$conflicts = $query->countQuery()->execute()->fetchField();
add
$query->isNotNull('mail');
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | 3488835-drupal--false-report.diff | 3.85 KB | robloach |
Issue fork drupal-3488835
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3488835-status-report-confuses
changes, plain diff MR !10283
Comments
Comment #2
atul_ghate commentedI will work on this issue.
Comment #3
quietone commented@atul_ghate, thanks for working on this issue. I am going to unassign because for Drupal core, it is preferred that contributors add a comment that they are working on an issue instead of assigning it to themselves. See Assigning ownership of a Drupal core issue.
Comment #6
danchadwick commentedComment #7
atul_ghate commentedComment #9
mcdruid commentedI've added the tests that the Security Team used when working on SA-CORE-2024-004; we'd typically wait a few weeks before committing tests that accompany a security fix, but there's no need to do so in this case.
We should add to these tests to verify the behaviour of
user_requirements()when there are multiple users with a blank email.We might want to account for the fact that could mean a couple of different things e.g. a truly NULL field in the db, or an empty string.
Plus we need to check that all 3 of core's db drivers behave themselves.
Comment #10
mcdruid commentedAdded a new test that verifies
user_requirements()doesn't incorrectly flag a couple of user accounts that both have their mail set to an empty string.This passes locally for me with mysql.
Comment #11
mcdruid commentedComment #12
mcdruid commentedTests pass with mysql, sqlite, and postgres (including the new one).
Anyone happy to RTBC this on that basis?
Comment #13
robloachTested this out, and worked flawlessly. Was confused when running the related SQL it didn't output any results. The MR fixes the false positive.
The diff applies to Drupal 10.3 as well, so just a rebase/cherry-pick should work fine, but here's the same patch if needed for Drupal 10.3.x
Comment #14
wylbur commentedTested this on websites reporting the error, but which lacked accounts that differed by upper/lower case characters. Applying the patch resolved the error message on the status page.
My testing was on with mysql databases. Will see if I can setup tests on the others.
Comment #24
larowlanIssue credits from the private issue
Comment #29
larowlanCommitted to 11.x and backported to 10.4.x, 10.5.x and 11.1.x
10.3.x and 11.0.x are security only.
Comment #31
w01f commentedThis says committed for 11.1.x, but I have several sites that were updated to the most recent 11.1.6 and they still show that error. I've checked and there are no valid cases of duplicate emails.
Comment #32
poker10 commented@w01f have you tried running the SQL query mentioned here: https://www.drupal.org/node/3486109 , to check, what is causing the warning message?