Update user creation form to hide password and status fields when user is notified

@lauriii raised the question of whether this could be tied to the dev/prod toggle. I don't think it should be, because the usecase @pameeela has raised is to notify real new users via email when an account is created, but mine (from my own site-building days) is always to not notify users created by admin.

We could also use the dev/prod toggle to not send any emails from dev ever, but that's its own separate followup issue. (Edit: The dev/prod toggle should also have programmatically configurable behavior, rather than secret magic undocumented behavior, but that's a discussion for that issue and its followups.)

If anything (if we want fewer settings in the UI), I would almost make it only a site-wide checkbox, rather than a checkbox on every user creation form. That would more align with core-as-framework IMO. But that also seems like followup material to me.

@lauriii raised the question of whether this could be tied to the dev/prod toggle. I don't think it should be, because the usecase @pameeela has raised is to notify real new users via email when an account is created, but mine (from my own site-building days) is always to not notify users created by admin.

We could also use the dev/prod toggle to not send any emails from dev ever, but that's its own separate followup issue.

The email based workflow for adding new users is increasingly common nowadays. I do think we should default to that, given that how common it is. I don't even remember when was the last time I was passed a password to a product manually. I understand that some organizations may have a different workflow, but we should set the default based on what we think is the 80% use case. I don't really understand how this default would be different for Drupal CMS vs Drupal Core.

The dev/prod toggle idea I had, was to not check this automatically when working in a dev environment. That way you could still choose to sent an email in dev environment if you want to test this (or some other reason).

This could be a separate issue, but as a UX enhancement, I think we should be also be generating the password automatically.

If anything (if we want fewer settings in the UI), I would almost make it only a site-wide checkbox, rather than a checkbox on every user creation form. That would more align with core-as-framework IMO. But that also seems like followup material to me.

If we make this a site wide setting, it almost seems something that should be managed by ECA. However, that's something for future because that would require having ECA in core.

I'm more open to that instead of a site-wide setting to control what the default value should be. That said, if we remove the checkbox, we need to make changes to this page to make it clear that an email will be sent or not depending on what the setting is.

To me, changing the default value seems like the least disruptive way forward. I'd recommend we do that and open a follow-up to move this to a global setting. Based on that, the next steps would be:

1. Change the default value to checked (this issue)
2. Add an automatic password generation (follow-up)
3. Move this to a global setting for configuring the registration flow (follow-up)

Release management hat on: Changing the default value on the form would be too disruptive. I would say that's a major-only behavior change.

I think Starshot can flip the default value from core's, and which way the default behavior goes can at least be added as a site-wide config setting. That way, Starshot and site owners could override it. I think this is potentially something where the Starshot and core framework 80% case differ.

I'd still also lean toward exposing the site-wide default in the user settings admin UI, but we can separate out that decision to a followup as well if we want. That also would give us the opportunity to deprecate a checkbox on a more frequently used form, if we want.

Release management hat on: Changing the default value on the form would be too disruptive. I would say that's a major-only behavior change.

Would it be possible to document what the expected disruption would be to see if we could find ways to address those?

I think this is potentially something where the Starshot and core framework 80% case differ.

I don't really understand how this would be different between the two? Email based sign-offs are used commonly in both, consumer and enterprise products. For sites using SSO, this would be different, but in this case humans wouldn't be even accessing this form.

I don't necessarily think dev/prod is that relevant, if I'm making someone an account on dev I would want them to get an email too. But that's only on manual creation, not sure if we should distinguish? Meaning, if I'm manually creating an account for someone on dev, why would I NOT want them to be notified? If it's a test account I'd use my own email. If I'm making an account with someone else's email address, how would they access it without getting an email?

This is mainly because the emails may not be all that useful given that the user may not be able to access the site when it's in dev environment (e.g., localhost). I don't think this is that important use case at all to consider, just something that came to my mind.

From my experience , when creating an account manually by admin, it is probably 50:50 between situations, where notification email is desired and when it is not desired. I think the default off is reasonable there. The primary way to register on the site is by users themselves and there the emails are sent.

This was the reason I revisited this, I felt that we should sort this out first since it doesn't make sense to generate the password and NOT send an email.

There is a reason. Sending plaintext passwords in emails is not a good idea from the security point of view and that was a reason why core is not doing this anymore from 2010 (#660302-64: Migration path for changed user email tokens; don't complicate translation of default messages). I am strongly against the idea of sending plaintext password in emails again.

@pameeela Sorry I was not clear enough, but my second part of the comment was a reaction to the quoted text, but that is related mostly to a separate issue @lauriii mentioned earlier, not this change itself. It was not really clear to me, what other benefits (except that admin would not have to enter a random password) can be achieved by generating a password and sending the email vs generating and not sending it (with regards to the generation itself), if the notification email will be still the same in both cases (when password will be generated, or manually entered) - it will just contain a link to reset the password. But yes, this question belongs to a separate issue (or probably also here #3481866: [PP-2] Automatically generate a password on user creation). This issue is just about the checkbox/configuration setting.

The eventual ideal for me in this scenario would be that setting a password isn’t required on account creation, if you are using this workflow.

This was the intention. Many platforms only ask for email (and sometimes for name) when inviting new users to the platform and expect the user themselves to fill in all required information like password when they use their one time login link. If you are using this email based registration flow, it seems completely unnecessary for the person creating the user to have to enter a password, because it would be changed by the owner of the user on their first login.

There is an older related issue which proposes to make the password field optional (when creating an account for another user) - #397846: When creating an account for another user, make password field optional .

This is what I had in mind for the UX:

I'm thinking we could hide the status too when the user is invited to set their password. It doesn't make sense to notify the user that an account was created if the account is blocked.

I don't think this would be too disruptive for a minor release - it's only changing the default value of a checkbox on a form, not changing any other behaviour such as when users are saved programmatically. We can put it in the highlights post as a user facing change (albeit a small one) and in the release notes. We've changed significantly more in minor releases like adding publishing status to entity types etc.

Also if we look at project usage, there are only 7,000 sites on 11.0.x and 150,000 sites on 10.3.x, and 85,000 sites on 10.2.x. All of those sites are more likely to update to 11.1 than 11.0 (I know this will be what my 10.x sites do). So in practice it will be part of a major upgrade for more than 300,000 sites and part of a minor upgrade for less than 15,000. This is a good reason not to backport it to 10.4.x though.

Adding a configuration option is extra config + config schema + upgrade path etc. then maintenance over time.

I don't really understand the dev/prod toggle discussion here - sites in dev will either have broken email sending (on local) or they should be configured not to send emails at all (maillog or one of the other approaches). I do, out of paranoia, uncheck this box even if I have maillog or similar installed but that is years of superstition and past bad local development practices (and memories of before email spam checks were stricter and you could successfully deliver emails from postfix running on localhost with phpmail).

I don't really understand the dev/prod toggle discussion here - sites in dev will either have broken email sending (on local) or they should be configured not to send emails at all (maillog or one of the other approaches). I do, out of paranoia, uncheck this box even if I have maillog or similar installed but that is years of superstition and past bad local development practices (and memories of before email spam checks were stricter and you could successfully deliver emails from postfix running on localhost with phpmail).

There isn't really much into it. 😅 I'm doing the exact same out of paranoia (unchecking email checkboxes on dev regardless of using MailHog), which was why I was thinking that we could consider changing the default value depending on this setting. This isn't a critical part of the issue and I think we can discuss this in a follow-up if we want to.

And extra issue with flipping the default depending on dev/prod is that if you actually wanted to form alter to change the default, it could be very confusing that it's behaving differently on dev and prod and hard to test that the form alter worked. Given both me and @lauriii are paranoid about emails going out from localhost, maybe we should open a general issue for some minimal dev/prod email support, like config somewhere for 'don't send any emails in dev mode' which people could set in lieu of one of the other email catching solutions.

On the release management side I agree with @catch here; this is admin UI and we've made bigger changes in other areas of the admin UI in minor releases. BC policy allows us to "change [form] structures where necessary to make usability improvements or add features in minor versions" and "form classes [...] are not part of the API [...] unless specifically marked with an @api tag" - in fact Drupal\user\ProfileForm is explicitly tagged @internal.

I also would like to not add yet another config option that will rarely be changed on most sites; I think core can be opinionated here on UX and if anyone wants to change the behaviour then contrib or custom code can provide their own form or alter the existing form as they could before.

I think it would be even better if we can implement #18 here (including #19, as I thought the same when I saw the animation before I read the comment), instead of just changing the default value of the checkbox; even if the code is @internal if we can make the changes we want in one step instead of two then it will be better for anyone who is affected.

Thanks @catch I've updated the issue to reflect the consensus around not having a new config option for this.

Huh? When did that get consensus? I still don't agree. I suggested a config-only option, not adding it to the UI.

And I still feel this is too disruptive for a minor release because your sites start freaking sending real people emails when you create accounts. Spam by accident is much worse than no spam by accident; a behavior to send emails is outside the normal scope of the allowed changes policy. It's not just a form structure change; it's a form structure change that suddenly starts sending emails without warning .

Let's add a release note and change record here:

1. It's not uncommon to form alter this form.

2. A release note and change record means at least there's somewhere to point to if xjm's concerns come to pass and someone forgets to uncheck the relabelled/default-on checkbox.

I'm still concerned about the behavior change, as above, but I've also come to recognize that I'm in a minority here and that the non-sending-emails usecase is indeed the minority usecase. This is especially true if @pameeela and @lauriii both also have user research backing up that users would prefer emails be sent by default.

My reaction is probably stronger than others' for this disruption because of historical trauma from websites sending emails during user creation when they shouldn't. (The 20% usecase has just happened to be the usecase for every major site I've built.)

Based on how most websites behave these days, sending an email probably should always have been the default behavior (and I would have written a module to change the behavior in 2005, because it would have annoyed me every time to make the extra step of unchecking the box). It does make sense for production. The question is whether Drupal core out of the box is a dev thing or a production thing by default, and I guess we're thinking of it as more production-ready.

@catch reassured me that this change is only for the individual form operation via the UI and the pattern will not be extended to any other user creation operations.

FWIW I should have mentioned earlier that the form change itself is great and has good UX, regardless of its default state (as @pameeela mentioned).

Added a draft change record here as I couldn't find one. Does it need more detailed information or is a short summary enough? #3569591: Manual user creation now emails users by default

Reading the change record it looks good to me, setting RTBC as I believe this is appropriate here.

I came across this functionality in Drupal CMS, and like it, as I often forget to check the "send email" box on the standard form, and having to generate an initial password manually is a pain.

I do have a small concern that this this new behaviour will be the default for all sites. I wonder would it be worth adding a checkbox to a settings page to decide whether "email users" is selected by default. I think that could be decided in a follow on issue, and there's no need to delay this one.

I've tested the new behaviour and reviewed the change.

Change record looks good to me.

+1 to RTBC.

I have created follow on issue #3569615: Consider adding setting for default email to new users, to consider adding a setting to set the default value of "Email user with password setup instructions".

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The MR is just a little behind; fixed that, so restoring RTBC.

Committed/pushed to main and 11.x, thanks!

Excellent timing on this one, a client of mine wanted exactly this feature. Good excuse to upgrade to 11.3 too :) Thanks!

It will only be in 11.4, but the MR diff should apply cleanly to 11.3 at least.

@acbramley ...or you could install the Drupal CMS Helper module, which includes this functionality.

https://git.drupalcode.org/project/drupal/-/commit/5b9e9153598a7d279c5f8... applies cleanly to 11.3 :)

Pretty sure there's a problem here, have reverted for now, will post details later.

I have returned the change record to draft

I reverted this because a private security issue had been created related to the change and the commit wasn't in a release yet. That issue has now been cleared to be fixed in public by the security team (#3582300: Prevent empty passwords being stored).

The problem here, is the behaviour with the password when the field is hidden is undefined. We either need to add test coverage to ensure that the password is saved as NULL (and not a (hashed) empty string) or generate a random password when it's not set.

As this is a Drupal CMS release target, I'll try to get it over the line with the NULL behavior you mentioned.

I worked with Claude to write test coverage and a fix here. it pointed out some valuable information:

1. In the current implementation, if a user had entered a password into the field before clicking the notify checkbox (to hide it) that password would have been used
2. We can't set the password to NULL because in PasswordItem::presave if the entity is new we do $this->value = \Drupal::service('password')->hash(trim($this->value)); which would result in hashing an empty string (and produce a deprecation error because trim(NULL) is deprecated.

Went through the comments on the issue and the test and everything looks good. I think this is ready for RTBC

Committed and pushed to main

Setting to patch (to be ported) for 11.x (11.5) MR.

This isn't eligible for 11.4 because its a feature and has string changes.

Republished change record

Cleanly cherry-picked against 11.x, MR is up.

cspell was random on 11.x but seems like a good backport.

Committed and pushed 9432bd625e5 to 11.x. Thanks!