New user, with email validation, can finish registration with no password

Created on 12 June 2012, almost 13 years ago
Updated 12 June 2023, almost 2 years ago

Problem

Since the password field is not set to required for editing accounts (just registering new ones), when a reset email is sent for a new user, the user registration page is now treated like an edit page, with the $pass_reset variable set. (line 1054, user.module)

If a user, registers for the site, and clicks the one-time reset/validation form, they will be brought to the user edit page. This allows them to save their details without needing to enter a password.

To recreate:
1) turn email validation on
2) create new account
3) check e-mail and use the reset link provided
4) login to the site
5) click save to the user edit form without setting a password

Now the user will have to reset their password again in order to change it (since the 'current password' is unknown to the user)

Proposed resolution

add these lines after the $pass_reset var check (approx 1054-1059, user.module):

    } else {
      //we need to make sure the password is required if we're resetting it.
      $form['account']['pass']['#required'] = TRUE;
    }

This should require the password to be entered on validation emails, as well as password resets.

Remaining tasks

Review of the code snippet, creation of a patch, testing, security audit?

✨ Feature request
Status

Closed: duplicate

Version

9.5

Component
User systemΒ  β†’

Last updated 1 day ago

Created by

πŸ‡ΊπŸ‡ΈUnited States japerry KVUO

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs backport to D7

    After being applied to the 8.x branch, it should be considered for backport to the 7.x branch. Note: This tag should generally remain even after the backport has been written, approved, and committed.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡³πŸ‡ΏNew Zealand quietone

    @japerry, thank for bring this issue up.

    This was a bugsmash daily target a few days ago. It was discussed by lendude, larowlan and myself. lendude pointed out that "changing this would really mess with our maintainer login policy where our maintainer users are not allowed to have a password at all and you can only login via drush uli". I checked what OWASP had to say on the subject and it does not disagree with this practice. In the end we all agree that this is working as designed.

    Later, I tested the link and found that the flow was a bit confusing and that matches what is mentioned in #4. Then, I looked for duplicate issues and found ✨ Include fields for resetting password on the one-time password reset page Active . That issue has more discussion, and has had a usability review. It is also a feature request while this is a bug. I checked the definition of bug β†’ and this does not fit. So, I am changing to a feature request like the other issue.

    Since these are so similar, I am closing this as a duplicate and moving credit.

  • πŸ‡ΊπŸ‡ΈUnited States brad.bulger

    Closed but never solved, very nice. Maybe it doesn't fit "the definition of bug" (lol) but it sure seems like a problem to people who run into it. Another forever-patch.

Production build 0.71.5 2024