Allow password on registration without disabling e-mail verification

Interesting...

Perhaps this is one of those instances where ease of use is at odds against security. I think the purpose for requiring a user to change their password from the throw away one is that we don't want to override an administer's ability to prevent new logins. Maybe I'm just clueless

Aren't users still able to request a new password and try the login experience again?

Generated passwords avoid account creation by bots, I guess.
The other way around would be for people to set a password when they create the account, then the account would be activated only when they click a link in the mail. But AFAIK the only way right now is with a generated password.

try http://drupal.org/project/logintoboggan

As a possible solution was provided in #3 (i.e. use a contributed module) I'm setting this to fixed.

Feel free to reopen if you think that is wrong.

- Arie

Promoted to an usability task. I think there is a considerable margin for improvement here.

@elv and the rest of the usability team: could you suggest a mock-up of how this could be improved?

Doesn'st seem like a mockup would be required, only a patch to let the user enter a password instead of not?

@Bojhan: wouldn't it be better to think about the whole registration / login / request new password process?

Some questions:

- do we really need a "Request new password" link in the login block?
- do we need some explanation on the "Request new password form"?
- how to revamp the "Reset password" form? Currently this forms shows a lot of text including a frightful "This is a one-time login for and will expire on Fri, 12/12/2008 - 15:24." Could we have directly a new password box here, instead of redirecting the user to the "User account" form? etc.

To comment on all of this, I never got why we where breaking with convention of sending an e-mail with a password you have to reset. Lets keep this issue focused on that.

On to the other questions, we have to look at when something happends

- For the "Request new password" it is behavior we have to assume, since we have no data on it. When do people need a new password? Probally when they are failing to login, so it should be promted only then.

- Should be removed.

- Is there a security reason for this? I never understood why we needed to redirect instead of just allowing one to inmediadtly trowing in a new password.

In Drupal 7.0 there is an option to "Require e-mail verification when a visitor creates an account" at admin/config/people/accounts/settings.

New users will be required to validate their e-mail address prior to logging into the site, and will be assigned a system-generated password. With this setting disabled, users will be logged in immediately upon registering, and may select their own passwords during registration.

While disabling this option makes it possible for users to select their own passwords during registration, it also turns off e-mail verification. I can see a use case for having users selecting their own passwords on registration, but also require an e-mail verification.

Perhaps split this option into two options; one for requiring e-mail verification and one for enabling users to select their own passwords on registration.

This is an issue for me. LoginToboggan definitely doesn't solve it.

There's a module that enables the behavior (register with password, + authenticate email), currently only for D7, and I may backport it to 6.x:
http://drupal.org/project/user_registrationpassword

+ 1

+1 : the functionality for registration with custom password + email verification is desired by many, evidenced on multiple threads, various implementations, and requested by my personal clients. I am always considering/exploring for more concise ways of handling these options, and am still surprised that more robust core functionality hasn't been accomplished much earlier. For D6 I am using LoginTobogan registration block that can be configured as outlined in this thread { http://drupal.org/node/582764 } although this is not ideal-> folks dealing with similar problem; It is possible to use roles, redirects, email verify re-sends, and custom messages prompting users to check their mail to verify their account (various tactics discussed on threads), yet this is fairly involved, jarring from a new user experience, and complicated for developers. The options for both allowing user-chosen password and required email verification should be independently available to the admin in the core, or meanwhile, any module handling both functions.

#12 @EarthLaunch -> I have been reviewing the LT module and D7 registrationPassword as well; did you proceed with backporting that for D6? I have been considering creating some new code for D6 as well. Let me know how I can help.

Moving forward with Drupal core- I hope this common functionality can be integrated. Thank you to everyone on this thread/similars and the entire Drupal community!

Reported installs: 210 sites currently report using this module

Looks like not very popular feature

As the current (co-)maintainer of User Registration Password, some comments:

Would love to have this in core, and i guess more and more people have that idea, because the usage stats are only going up, but if that won't be the case, we will think about a port to D8. (still lots of time left before D8 is anywhere near a (stable) release.)

D6: @EarthLaunch to bad i only just found this issue, (also @jlp1) see http://drupal.org/node/1708180 for a D6 port i rolled)

Current reported installs URP: 1100 see http://drupal.org/project/usage/user_registrationpassword

User stats:
D6: 26 sites (yeah, i backported it)
D7: 1358 (first week it's going down a bit)

It's not super popular, true, but the word is getting out that it's stable and that it exists. I found 1 company listing it on a document (case study) and 2 sites next to d.o. with a question where this was listed as a suggestion, so yes, it's getting out slow, but steady going up. Also have to admit that since the 1.2 release the module is getting more and more stable, before that the module sufferd some issues that are now fixed, but that bumped the stats a bit at the start i guess.

I have started working on an initial patch for Drupal 8! It's still a draft stage, but applies clean and it just works. (still wonder about what the best way would be to handle a couple of issues), so give it a try. (it does still need a couple tweaks and tests.)

Dear testbot, your so right. Let's try that again.

I created 2 patches, 1 rendering an extra checkbox and 1 transforming the existing checkbox on the 'config/people/accounts' page. You don't have to apply both, it's the same patch, only difference is how the config page works. (I believe the one with the extra checkbox works best / least confusing.)

This is the first version. This one transforms the 'Require e-mail verification when a visitor creates an account. ' checkbox into a radios with 1 extra option:
'Require a verification e-mail, but let users set their password directly on the registration form.'

And this is the second one. This one renders a second checkbox on the 'config/people/accounts' page.
If checked, people have to set a password during registration, just like the 3rd option from the previous patch.

Added a bit more info.

Thanks for the patch. One minor problem: it has TAB characters, please use two spaces. As the tag indicates, this would need a test. Finally, and this is the biggest problem I can see from a quick review, user_register_password_set seems to replicate a lot of code already in user_pass_reset.

Hrm, the tag didnt get added somehow.

chx, yes i have seen that too. I already did a quick attempt to combine it, but i figured let's first upload it to get some response (like this)

Also the menu item that's used is almost the same, but i was not sure if i should change user_pass_reset() without first discussing it.

This is a reroll from #22 big rewrite that changes the user_pass_reset() function, addressing the 'bigger issue' chx pointed at in #23. All duplicate code is almost gone and i hope all tabs are also gone.

+++ b/core/modules/user/lib/Drupal/user/AccountFormController.phpundefined
@@ -102,7 +102,9 @@ public function form(array $form, array &$form_state, EntityInterface $account)
+    // or verify_mail & password_register are set or the user is an admin.
+    elseif (!$config->get('verify_mail') || $config->get('verify_mail') && $config->get('password_register') || $admin) {
       $form['account']['pass'] = array(
         '#type' => 'password_confirm',

This logic seems a little strange. At least some () around && are missing.

+++ b/core/modules/user/lib/Drupal/user/AccountFormController.phpundefined
@@ -118,6 +120,13 @@ public function form(array $form, array &$form_state, EntityInterface $account)
+    // the newly created account.
+    if (!$admin && $register && $config->get('password_register')) {
+      $status = 0;
+    }

Why is this extra if necessary?

Can't the logic be done directly above?

Currently I find it more confusing than helping.

+++ b/core/modules/user/lib/Drupal/user/RegisterFormController.phpundefined
@@ -120,6 +121,14 @@ public function save(array $form, array &$form_state) {
+    elseif (!$admin && config('user.settings')->get('register') == USER_REGISTER_VISITORS && config('user.settings')->get('password_register') && !$account->status) {
+    	// Notify the user.
+    	_user_mail_notify('register_password_set', $account);
+
+    	drupal_set_message(t('A welcome message with further instructions has been sent to your e-mail address.'));
+    	$form_state['redirect'] = '';

Tabs! ;-)

---

Overall I like the patch, but I believe it needs more work in terms of _simplifying_ the whole logic. It might help to spell out the logic once as "text" here and then we can simplify it.

Leaving for review of others ...

Ok, let's try that again dear testbot.
- Added update function.
- Changed logic in multiple places.
- Moved and changed '$status = 0' to the submit function in RegisterFormController.php.
- Added a test implementation in user_pass_validate() to enable users to request the activation e-mail via the 'user/password' page. This only works if they never ever logged in before. This still needs work, same story: duplicating code. (WIP)
- Stripped some unwanted code, let's stick to the basics.

@Fabianx
1st & 3rd true.
2nd: '$status = 0' We need that to stop the account from being activated during registration. We need it to be activated during confirmation, not before.

Minor update, added current patches + updated some info.

Updated current info, added more details.

Added this issue to #1837054: [META] Refactor account workflow (and config)
Will also try to write a test in the next couple days for this if nobody else beats me to it.

adding tags.

updating issue summary to note which tasks are suitable for novice (screenshot, steps to reproduce, manual testing)

Updated issue to current.

#29: drupal_115801_user_registration_password_set_checkbox-2.patch queued for re-testing.

<<<<<<< HEAD
  form_load_include($form_state, 'inc', 'user', 'user.pages');
=======

  $form['registration_cancellation']['user_password_register'] = array(
    '#type' => 'checkbox',
    '#title' => t('Require people to choose a password during registration.'),
    '#description' => t('Let people enter a password during registration. If <em>Require e-mail verification</em> is disabled, this setting is automatically enabled.'),
    '#default_value' => $config->get('password_register'),
    '#states' => array(
      // Disable this option if email_verification is unchecked.
      'disabled' => array(
        'input[name="user_email_verification"]' => array('checked' => FALSE),
      ),
      // Enable this option if email_verification is checked.
      'enabled' => array(
        'input[name="user_email_verification"]' => array('checked' => TRUE),
      ),
    )
  );
  module_load_include('inc', 'user', 'user.pages');
>>>>>>> Applying patch

This was the conflict.

This is how I fixed it:

  $form['registration_cancellation']['user_password_register'] = array(
    '#type' => 'checkbox',
    '#title' => t('Require people to choose a password during registration.'),
    '#description' => t('Let people enter a password during registration. If <em>Require e-mail verification</em> is disabled, this setting is automatically enabled.'),
    '#default_value' => $config->get('password_register'),
    '#states' => array(
      // Disable this option if email_verification is unchecked.
      'disabled' => array(
        'input[name="user_email_verification"]' => array('checked' => FALSE),
      ),
      // Enable this option if email_verification is checked.
      'enabled' => array(
        'input[name="user_email_verification"]' => array('checked' => TRUE),
      ),
    )
  );
  module_load_include('inc', 'user', 'user.pages');
  

I removed:
<<<<<<< HEAD
form_load_include($form_state, 'inc', 'user', 'user.pages');
=======

because the patch has a call called module_load_include which appears to do the same thing.

Here's the current drupal 8 screenshot of the configuration page:

#34: drupal_115801_user_registration_password_set_checkbox-34.patch queued for re-testing.

+++ b/core/modules/user/user.installundefined
@@ -1056,5 +1056,24 @@ function user_update_8017() {
 /**
+ * Add variables for password_register to mail and settings.
+ *
+ * @ingroup config_upgrade
+ */
+function user_update_8011() {
+  update_variables_to_config('user.mail', array(
+    'register_password_set_activation_subject' => 'register_password_set_activation.subject',
+    'register_password_set_activation_body' => 'register_password_set_activation.body',
+    'register_password_set_subject' => 'register_password_set.subject',
+    'register_password_set_body' => 'register_password_set.body',
+  ));
+  update_variables_to_config('user.settings', array(
+    'user_password_register' => 'password_register',
+    'register_password_set_activation' => 'register_password_set_activation',
+    'register_password_set' => 'register_password_set',
+  ));
+}

is this a syntax error because we have cmi now?

nah, it's an actually php error, redeclaring a function. that update number is used. I changed it to the next update number.

#40: drupal_115801_user_registration_password_set_checkbox-40.patch queued for re-testing.

Updated the patch from #40, this might break some bits, but let's see.

Let's try that again. Removed whitespaces, added new options to scheme.yml and added setting to settings.yml.
Interdiff from 40. (forget the patch from #43)

Ok, this should work, but does require some more work to get the .schema sorted. .settings is ok, the password_register option does not need to be set by default, but this does make me wonder...
Again, interdiff from 40. (and some variable names really need work)

Updated issue summary. with pointers to novice.

After screenshot.

Updated to current.

checkbox says: Require people to ...
description says: Let people ...

sounds contradictory. we should maybe just leave off the first sentence of the description and let the checkbox option speak for itself.

(this is both needs work and needs review. :) )

removing needs screenshot tag and updating issue summary.
Add the tag back when a new patch is posted.

This is needs work for #49 But could also use some more review and manual testing of actually using the system with the various combinations of settings.

I change my mind, adding make needs screenshot for screenshots of the account creation pages, admin and user creating.

Added screenshots

#47: drupal_115801_user_registration_password_set_checkbox-47.patch queued for re-testing.

YesCT asked for before screenshots for the account creation pages for user and admin. I made them. I tried to apply the patch to make after screenshots, but the patch did not apply, so I queued the patch for re-testing.
This is the screen shot for a new user creating a new account:

This is the screen shot for an admin adding an account for a new user:

I set the status to needs work because the patch needs to be rerolled.

I went through the instructions here:
http://drupal.org/patch/reroll
the only conflict was this:
http://privatepaste.com/3883a8701b
I asked in irc, and larowlan said that it was somthing that had been converted to wscci.
larowlan told me to look in core/modules/user/lib/Drupal/user/, and I found a file core/modules/user/lib/Drupal/user/AccountSettingsForm.php that contained something that looked like the conflict.
So I took out the conflict, because it was written elsewhere.
no interdiff because this is a reroll

#55: drupal_115801_user_registration_password_set_checkbox-55.patch queued for re-testing.

Looks great UX, +1 to rtbc

@YesCT #49

I've removed the first line, looks better, might need to add a small bit of text, but we have a 'manual / docs' page to explain this in detail i figured.

This patch includes the missing configuration options, @EllaTheHarpy these are now in core/modules/user/lib/Drupal/user/AccountSettingsForm.php

@andypost thanks!

@all

• I have also added new before/after screenshots for the configuration page.
• We could use some more manual testing confirmations + overall reviews.
• We still require some tests.
• We could also already start on a small documentation-piece (maybe).

added remaining task to improve text

Tests:
- check presence of password field on form after option enabled|disabled
- check that user can login with the password
- make sure mail messages sent

suppose better to extend existing tests

@Rob C when you file new patches please provide a interdiff of changes, that makes people follow the changes

@andy

interdiff: if i change anything i'll create one. (see above) this is almost the exact same patch as in #47 ... and that has an interdiff. (this is #55 + missing config pages).

"suppose better to extend existing tests"

Agreed.

I'm not yet sure why it failed this time, because when i review the code, + try it myself, the assertText is exactly like it is on the test (line 88 or something). I did see pifr having some issues last night...

Currently working on this issue

#58: drupal_115801_user_registration_password_set_checkbox-58.patch queued for re-testing.

#58: drupal_115801_user_registration_password_set_checkbox-58.patch queued for re-testing.

Like i expected, now the tests does not fail.

Ok, next stop: write / modify tests.

@YesCT: how's the language now?

removing tag

#58: drupal_115801_user_registration_password_set_checkbox-58.patch queued for re-testing.

comparing the screenshots from #48 with the new ones in the issue summary, I think my concern in #50 is addressed.

The issue summary still needs screenshots of before and after the patch both for the user during account creation and also for admins, of each different combination of the checkboxes.

No interdiff, it's a reroll + minor tweaks.

Changes:
Due to some core changes i've removed the value we used to set to get to the correct $op for sending the correct e-mail.
Also removed some additional code that core also already solves in User.php (core entity plugin) Core already sends out the correct e-mail, so no need to send it from here anymore. (at least, looks like it)

Added the first test, might want to move 1 bit to the password reset test and add additional minor tests to confirm some more, but it's a start.

@YesCT: Ok, great.

Possible checkbox combinations:
[X] Require e-mail verification when a visitor creates an account.
[X] Require people to choose a password during registration.
This is the first screenshot, people are required to set a password during registration, while verification is enabled.

[X] Require e-mail verification when a visitor creates an account.
[_] Require people to choose a password during registration.
This is the second screenshot, people are not able to set a password during registration, because verification is enabled.

[_] Require e-mail verification when a visitor creates an account.
[..] Require people to choose a password during registration.
No separate screenshot, but this is the same as the second screenshot, users need to set a password during registration and are directly authenticated after submit. [..] is the disabled checkbox (via #states).

Ok, weird. Let's also test it.

Same patch, someone beat me to update 8020 :/

Minor tweak to how we call user_login_finalize(), now tests should not fail.

Updating tags, updating status.

Added checkbox combination description + screenshots for user/register.

Minor update to checkbox combinations, now it's correct.

This needs a reroll at the very least.

Patch reroll

user.mail schema updated.

Schema update and several typos fixed.

Refactored some legacy code.

The plugin.manager.entity service has been replaced by entity.manager since 8.0-alpha7. (https://www.drupal.org/node/2181815)

The patch in comment #88 calls plugin.manager.entity and therefore fails at line 869.

Changing service name as suggested by kevineinarsson.

New try, previous patch didn't apply correctly.

Test failed locally, working on it needs more work.

Patch seems to apply, but the tests fail because a user is created which is already activated (status = 1)?

I get a fatal error because test tries to find the user with the status = 0 in the selection. But because the user has status = 1, it can't find any and then tries to get the status of an item from an empty array ...

Have to figure out why the user is created with status = 1.
-> the if-statement was wrong: it checked if there was a uid available, while it should check if there wasn't

Setting the status to NULL seems to do the trick, except that the user now cannot activate its account: the reset link (user/reset/...) gives a nice access-denied-page (because the user is blocked...).

/me digs back in again for a little bit... added a new patch with some fixes/changes and a todo-list

Updated the patch a little bit:

• Fixed an assert that used a different string to test with, also assertRaw instead of assertText
• Fixed some checks for sending the correct email upon registration
• Fixed the setting status to NULL in the RegisterForm
• Corrected the message with further-instructions (email instead of e-mail, like the default one) + the assert for this string

Todo:

• Fix the failing part of the test: user cannot activate its account and login
• The password-reset link is weird as it tells the user to click the login-button and set a password, but a password was already set at registration?

Attaching rerolled version of the patch from #98

Go testbot!

Patch #102 does not apply anymore, here's a current reroll.

Here's a backport to 8.6

And a backport to 8.6.2.

Tests: Looks like simpletest api ran away and we must

use AssertMailTrait {
    getMails as drupalGetMails;
  }

Error: Call to undefined method Drupal\Tests\user\Functional\UserRegistrationTest::drupalGetMails()

Do remember something about AssertMailTrait, guess that didn't land yet when this patch was created - and this patch was never updated - until now.

+1

Patch rerolled.

Hi all. It looks like this one has been out here for a while and, with 50 followers, it looks like there's a lot of interest in it. We're going to apply it to a current project and report back regarding testing in a Drupal Commerce environment. If there's anything we can do along the way to help this along, we're all in.

Reroll for 8.9.x.

1 var changed ($admin > $admin_create) + added the trait. Didn't test it yet.

Does #121 work with D 8.8? I would test the functions with commerce

Manually tested with 8.9 and 9.0.

Re-rolled the patch in #121 against 9.1.x branch. Included a diff_reroll_115802_121-127.txt file as well. Please review.

Thank you!

Updated the patch. Please review.

Thank you!

@anushrikumari
I tested ur patch manually,
Looks good
Please fix the automated tests so that this can be moved further for RTBC

@anushrikumari
thank you for your great patch, it works perfectly.
unfortunatly i see that the patch doesn't pass the automated tests so it fails :(
did you fix this part?
tks

Fixed few phpcs issues along with few DIs.

Fixes minor issues with previous patch plus 9.2 ready.
Also works for 9.1

#143 works as advertised but it touched lines of code that it shouldn't.
The attached patch and MR 126 correct these changes.

An interdiff and a new patch are also attached, as well as the gitlab merge request.

I am leaving this to needs review, since I made changes to the patch.

Reroll for latest 9.2x

Reroll for latest 9.1.x

@xavier.masson patch #147, #148 giving error patch is not applying sharing screenshot ...

Rerolled the above patch and attached an interdiff as well. Please review.

In the rerolled patch, updated the changes in core/modules/user/tests/src/Functional/UserRegistrationTest.php and addressed #3212034: Account emails are missing newlines due to malformed YAML and #2844452: Export configuration YAML strings as multiline in core/modules/user/config/install/user.mail.yml .

Thank you!

#152 & #153 Patch Not apply.

HI @VladimirAus and @Prabhat Burnwal,

Above test cases, #152 and #153 is getting failed.
Can you please check it once again and attached some before and after screenshots.

Please refer attached screenshots.
Can be a move to Needs work.

Re-roll patch required for 9.3.x.

Re-roll path crated for 9.3.x.

Fixed the fail test, Added the missing schema for "user.settings:notify.register_password_set".

@vsujeetkumar what changes are you bringing in?
Would be good to have diff and description for comments 153-158.
For #152 I just rerolled patch to work on 9.2.x

@vsujeetkumar what changes are you bringing in?

@VladimirAus I have added a interdiff in #158, Please have a look.

Would be good to have diff and description for comments 153-158.

Patch #153 has missed some changes, Also It is related to 9.2.x. And we have already a patch #151 in 9.3.x, according to me #151, #152 and #153 all are the same patch so we can consider the interdiff in #158.

For #152 I just rerolled patch to work on 9.2.x

If we check the comment #149 it is already moved to 9.3.x, After that we have one patch #151 also, So is there any reason to moved it back to 9.2.x?

Fixed the fail test for 9.3.x, Please have a look.

Fixed the fail test, Please have a look.

Rerolled patch to work with the latest 9.3 beta2

I can confirm that patch in #152 doesn't apply for 9.2. Fix would be much appreciated, since Open Social sites are stuck with 9.2.

https://www.drupal.org/project/user_registrationpassword just got abandoned so this issue may be a bit more important now

maybe a stupid question, but:

how is the registration process on this site set up?

I would like to have the exact same procedure:

on registration form I enter email address and password, I get a confirmation email and when I klick it I get redirected to the site, where I tried to to leave a comment.

like here:
https://www.drupal.org/user/register?destination=node/115801

If visitors are allowed to create account without approval (UserInterface::REGISTER_VISITORS) and password is required only the "register_password_set_verification" should be sent.

With this patch "register_pending_approval" is sent. This email state that the account is not approved and that the user should wait on approval. It makes no sense.

Config also need to be updated if you check the "register_password_set" or else the mail will never be send.
If symfony_mailer is used, it will need to be patched aswell or the mail will be empty.

Patch works fine but how can activate the user?

i can't use [user:one-time-login-url] link.
If i click the link i get page not found.

Thank you

Tried to fix the custom errors in #173.

Added some functions from module "User Registration Password" to activate the user on one-time-link use.

fixed failed commands against #176

Fixed failed commands against #178

I have some concern with the approach here as it introduces changes to the meaning of "Blocked" accounts.

Currently, a blocked account is a blocked account. It can never login until activated.
After this change, a blocked account can be automatically unblocked via one-time-login-link if it was never logged in. There is no way to revoke that one-time-login-link to fully block such an account.

It's worth noting that this patch relies on the [user:one-time-login-validate-url] token/url rather than the [user:one-time-login-url] token/url to activate the account. That wasn't very clear to me when applying this patch.

Added reroll of patch #179 on Drupal 10.1.x and addressed Drupal CS issue as well.

Fixed the deprecation check issue