Problem/Motivation

When users are auto generated due to "Override registration settings", passwords seem to be created via user_password() with the default length (10 characters).

Proposed resolution

Do not set a password at all, as OIDC users are not supposed to have one.

CommentFileSizeAuthor
#8 openid-connect-remove-password-3214539-8.patch709 bytesHebl
Screenshot from 2021-05-18 14-09-32.png54.05 KBmsno
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

msno created an issue. See original summary.

msno’s picture

msno CreditAttribution: msno commented
Issue summary: View changes
jcnventura’s picture

jcnventura CreditAttribution: jcnventura at 1xINTERNET commented
Title: Optional length for auto generated passwords » Do not generate a password when registering a new user
Issue summary: View changes

Hi @msno. Welcome to Drupal.

Thanks for the report. This user_password() call has been present in the code for quite a long time, and nobody had noticed that issue until now.

The solution is actually to not set a password at all. Having a NULL password disables the ability of the user to use a password. Setting a larger password doesn't really solve the problem, only makes it harder to guess what the generated password is.

  • jcnventura committed d32d3a8 on 2.x 
    Issue #3214539 by msno, jcnventura: Do not generate a password when...
jcnventura’s picture

jcnventura CreditAttribution: jcnventura at 1xINTERNET commented
Status: Active » Fixed
msno’s picture

msno CreditAttribution: msno commented

I couldn't agree more - also found out that password could be omitted when creating a user programmatically and, hence, manual login is not possible.

Thanks for the quick reaction!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

Hebl’s picture

Hebl CreditAttribution: Hebl at manifesto commented
Version: 2.0.0-alpha1 » 8.x-1.3
FileSize
openid-connect-remove-password-3214539-8.patch709 bytes

I've added a patch which removes the password addition line for those who want to use the 8.x-1.3 version.

agn507’s picture

agn507 CreditAttribution: agn507 commented

Having trouble getting this patch to apply against 1.3. Any chance this can get rolled into 1.x without the patch?

Gathering patches for dependencies. This might take a minute.
- Installing drupal/openid_connect (1.3.0): Extracting archive
- Applying patches for drupal/openid_connect
https://www.drupal.org/files/issues/2023-10-23/openid-connect-remove-pas... (Do not generate a password when registering a new user)
Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/2023-10-23/openid-connect-remove-pas...

sanduhrs’s picture

sanduhrs
he/him
Primary language German
Location 🇪🇺 Heidelberg, Germany, Europe
CreditAttribution: sanduhrs as a volunteer commented
Version: 8.x-1.3 » 8.x-1.x-dev
Status: Closed (fixed) » Needs work
Issue tags: +needs backport to 1.x

This has been committed in #4, no need to patch.

  • sanduhrs committed 22a239d4 on 8.x-1.x 
    Issue #3214539 by Hebl, msno, jcnventura, sanduhrs: Do not generate a...
sanduhrs’s picture

sanduhrs
he/him
Primary language German
Location 🇪🇺 Heidelberg, Germany, Europe
CreditAttribution: sanduhrs as a volunteer commented
Status: Needs work » Fixed

Has been committed to 8.x-1.x, will be in the next release.

Harlor’s picture

Harlor CreditAttribution: Harlor at erdfisch commented

Oh this issue causes a Fatal on Drupal 10 :S

user_password() does not exist in Drupal 10 anymore.

I'm really surprised, that upgrade status did not report this.

The reasons seems to be that the function was found in UserPasswordFixture.php

sanduhrs’s picture

sanduhrs
he/him
Primary language German
Location 🇪🇺 Heidelberg, Germany, Europe
CreditAttribution: sanduhrs as a volunteer commented

Removing the usage of user_password() causes a fatal?

Hebl’s picture

Hebl CreditAttribution: Hebl at manifesto commented

Hey @sanduhrs,

Thanks a lot for adding the fix into 8.x-1.x, much appreciated.

Removing the user_password() does not cause a fatal (for me anyway). Having it left in on Drupal 10 however did cause the fatal error.

Thanks again!

Harlor’s picture

Harlor CreditAttribution: Harlor at erdfisch commented

That's what I meant.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.