Fixed #34429 -- Allowed setting unusable passwords for users in the auth forms.

[fsbraun]

https://code.djangoproject.com/ticket/34429

Feature added

This PR introduces a new functionality to the admin change password form: the checkbox (on by default) "Allow login"

If unchecked, the password field disappears and the form sets an unusable password upon submission. The user is warned before.

Tests

Existing tests have been changed to contain the new form field.

A new test was added to test the functionality.

Other effects

This PR adds three new strings which will have to be translated.

[fsbraun]

@ngnpope Thank you for the suggestions! Let me briefly comment and ask one more question:

• Probably we do not need to wait for DOMContentLoaded since the script is loaded as async. It would probably miss the event (which might be fired before it is loaded). I'll remove the event in total.
• Using the hidden property might clash with custom css which might set something like .form-row { display: block } effectively disabling the hidden property. OK, if I stay with style.display? I believe it is more robust.

Will, of course, also make the other changes.

[fsbraun]

One more thing to add: I did use function () { ... } instead of () => { ... } since otherwise this is not defined properly.

[shangxiao]

I have a remark about the wording

Allow login? If unchecked, the user will not be able to log in.

I think people are going to be confused about what the difference is between this and the active flag on the edit screen:

Active. Designates whether this user should be treated as active. Unselect this instead of deleting accounts.

Nick I know you requested the addition of ? and I'm not saying it's good or bad but it is now inconsistent with other checkboxes (I'd just leave it off personally) 😛

[shangxiao]

Can anything be done to stop the jumping checkbox? 🤔

https://github.com/django/django/assets/1845938/fe192289-704e-4db0-9f6f-9667d913c698

[shangxiao]

Anyhoo nice work so far fsbraun! 🏆

[fsbraun]

@shangxiao Thanks very much for the comments:

• The jumping can be stopped by putting the warning below the checkbox and above the submit button
• Thanks for pointing out the "active" alternative. Really, an unusable password only prevents login using username and password (as @xi probably also pointed out). I tried to better name those.

[fsbraun]

Thanks, everybody, for the constructive feedback. I have updated the PR:

• @knyghty The help text now reads

  Upon submission an unusable password will be set which prevents the user from logging in using their username and password. The user's current password will be lost. Potential other means of authentication remain unaffected.

• @xi This is gender-neural and clarifies that the current password will be lost

• @knyghty I have moved the warning into the help_text div. This implies that once the form field sets the aria-describedby property, it will include the help text and the warning. No need for two id to be provided to aria-describedby (which would be possible as far as I understand the docs).

[fsbraun]

Is there anything I can do to move this forward?

[fsbraun]

@felixxm Thank you for your detailed feedback. Indeed, I overlooked that this should also apply to the user add form. This is fixed now:

• Since the change password and user add form are quite different (form with template and an admin form) I have refactored the HTML to avoid repetition and make it usable for both forms (using the <template> tag).
• Indentation has been fixed
• The initial value of usable_password now is has_usable_password()
• I annotated the docs with versionchanged
• Updated the docstrings for the tests
• Added a Selenium test
• Fixed the HTML layout issue (wrongly placed >)

Please let me know if there is anything else I can do!

[fsbraun]

@felixxm Is there anything I can do to get this PR over the finish line? Would you like to take a final look?

[nessita]

I think people are going to be confused about what the difference is between this and the active flag on the edit screen:

I started reviewing this PR and the first think I thought was exactly this. Was there a consideration of being more "honest" with the admin user operating the form, something like:

Label: "Unset the password" Help text: "Unsetting the password will render the login credentials unusable. The User will no longer be able to log in until a new password is set."

[fsbraun]

@nessita Thanks for taking the time to look through this.

Indeed, the wording has been discussed here (and I also tried to get some input here).

I did consider "Set unusable password" but this would require the user to understand the implications, namely, that the user cannot log in using a username and password. That's the root of the wording.

IMHO, "unsetting" a password could be confused with "set no password". Also, even with an unusable password, you can potentially still log in with a different authentication backend.

Current text: Allow login with username and password

Help text: If unchecked, the user will have an unusable password and not be able to log in using their username and any password. (I just realized this had a spelling error)

Warning: Upon submission, an unusable password will be set which prevents the user from logging in using their username and password. The user's current password will be lost. Potential other means of authentication remain unaffected.

I am not hung up on this, but also would like it to be as clear as possible. What's your call?

[nessita]

@nessita Thanks for taking the time to look through this.

Indeed, the wording has been discussed here (and I also tried to get some input here).

I did consider "Set unusable password" but this would require the user to understand the implications, namely, that the user cannot log in using a username and password. That's the root of the wording.

IMHO, "unsetting" a password could be confused with "set no password". Also, even with an unusable password, you can potentially still log in with a different authentication backend.

Current text: Allow login with username and password

My main concern with this sentence is that, given that Django allows the User model to be customized so that the USERNAME_FIELD could be something other than username, like email or something system specific, saying username here would not be a accurate for those cases.

Also, considering that this flow occurs within the context of the admin, I would suggest to be more direct with the admin user and explicitly saying that the password will be set and it will be unususable. Otherwise I can see admin user scratching their head wondering what's the difference with marking a user as inactive.

Help text: If unchecked, the user will have an unusable password and not be able to log in using their username and any password. (I just realized this had a spelling error)

Same concern about saying username, we could say "credentials" as I suggested?

Warning: Upon submission, an unusable password will be set which prevents the user from logging in using their username and password. The user's current password will be lost. Potential other means of authentication remain unaffected.

I would omit "upon submission", I think this is implied from every help text. With that in mind, I would suggest:

An unusable password will be set which prevents the user from logging in until a new password is set. The user's current password will be lost, though other means of authentication (if configured) remain unaffected.

I am not hung up on this, but also would like it to be as clear as possible. What's your call?

I proposed a few alternatives above. The summary would be:

• not saying "username and password" explicitly, and
• being clear about how this action is different from making a user inactive

[nessita]

There is also a potentially awkward user flow when:

1. A user tesuser1 has an unusable password
2. Later on, the admin user goes to this user's change form to set a valid password:
3. The admin user is presented with a "change password" form where no password fields are available:

I wonder if we should enable the password fields when going from "change the password using "?

[fsbraun]

@nessita Indeed, I initially had the password fields enabled by default. @felixxm requested it differently here: https://github.com/django/django/pull/16942#discussion_r1261060471

I can revert the change. A third option might be to change the help text if the user does not have a usable password to something like: "Currently this user does not have a usable password. Enable this to set a password for this user".

An update for wording is coming up.

[nessita]

@nessita Indeed, I initially had the password fields enabled by default. @felixxm requested it differently here: #16942 (comment)

Oh, I see. I can understand the goal of having the form properly initialized, but I guess is weird when coming from "no password set". I guess we can proceed as is and then evaluate potential/future ticket reports.

I can revert the change.

No need for now.

A third option might be to change the help text if the user does not have a usable password to something like: "Currently this user does not have a usable password. Enable this to set a password for this user".

I like this proposal. May I suggest:

The current password is not usable, check this option to set a new password for user authentication.

[fsbraun]

@nessita Thanks once more for helping to clarify all this! Here's my update. Messages are:

Label: Set password

Help text (1/2) for "add user" and "change password" if the current password is usable: Not setting a password will render the login credentials unusable. The user will no longer be able authenticate using a password until a new password is set.

Help text (2/2) for "change password" if the current password is unusable: The current password is not usable, check this option to set a new password for user authentication.

Warning: If no password is set, the user is prevented from authenticating using any password. A user's current password will be lost. Potential other means of authentication remain unaffected.

(Updated after cd00672 for better consistency with No password set. wording in the change user form.)

[fsbraun]

I'll give it a try. There's an issue with postgres selenium tests which I cannot reproduce.

[sarahboyce]

I'll give it a try. There's an issue with postgres selenium tests which I cannot reproduce.

It appears to be an issue with flakiness rather than to do with your change set 👍

[nessita]

I'll give it a try. There's an issue with postgres selenium tests which I cannot reproduce.

Thanks! re: selenium, see Sarah's comment (test is flaky).

I'll do another UI review now. If possible, would you also, please, rebase onto main?

[nessita]

Hey Fabian, I did a UI review as promised. This is looking good! Thank you for being so responsive and open to our feedback.

Before going any deeper in a potential "password change mixin", I'd like to explore a different path for the user details/edit page. I was thinking that we could consider leaving the change password form as is, and instead have a new, simpler, form for just setting the password as unusable.

So, when viewing user details, the admin user is presented with a legend similar to this:

If the user has a usable password: "Raw passwords are not stored, so there is no way to see this user’s password, but you can change the password using <this form>, or disable authentication via <this form>".

If the user has a unusable password: "Re-enable authentication for this user by setting a password using <this form>"

The user creation form will remain as proposed in this PR.

I know we have been going back and forth with this PR, and I'm deeply thankful for your patience, hard work and willingness to adapt. At the same time, I'm evaluating these user experience adjustments as these pages are very high profile and it's important we get this right for both new users and advanced users.

What do you think?

[nessita]

Hey @fsbraun, I see that your pushed the changes for the password mixin, thanks! Perhaps let's hold on my last UI change proposal until I review this latest revision, I would not want you to keep investing in this without more concrete feedback from me.

Are you in the Discord Django channel? I was wondering if a sync chat would help the (my?) thinking process.

[fsbraun]

@nessita Just pushed the commit including the Mixin before I saw your thoughts on different forms. Here's my 5 cents on the option to offer two different password change forms:

• On the plus side: I find the text in the user change form explanation texts better. They are also closer to the "competing" active switch for the user. I'd slightly change them to:
  • Raw passwords are not stored, so there is no way to see this user’s password, but you can change the password using <this form>, or disable password-based authentication via <this form>"
  • Re-enable password-based authentication for this user by setting a password using <this form>
• A potential minus: The click-flow will be different for creating a user and changing a user's password

To me, this is not a clear call. Given the repeated discussion about active users and users with an unusable password, I'd probably nudge to separating the forms.

Also, I enjoy pushing things forward. Let's get to the best solution!

[fsbraun]

@nessita Can you share a link for the discord channel?

[nessita]

@nessita Can you share a link for the discord channel?

I think so, I'm new to discord so I think I got the right link :-D:

https://discord.gg/WHW9YkFg

I'll be relocating for about an hour, but I'll be online afterwards. I would suggest joining the #contributor-discussion channel, where we specifically discuss Django contributions. I'm nessita there as well!

[fsbraun]

@nessita Thinking a bit more, I come to the conclusion that I would keep the single password change form and change the message on the user change form. Something like this:

• Raw passwords are not stored, so there is no way to see this user’s password, but you can change the password or disable password-based authentication via <this form>"
• Re-enable password-based authentication for this user by setting a password using <this form>

[nessita]

@nessita Thinking a bit more, I come to the conclusion that I would keep the single password change form and change the message on the user change form. Something like this:

* Raw passwords are not stored, so there is no way to see this user’s password, but you can change the password or disable password-based authentication via `<this form>`"

* Re-enable password-based authentication for this user by setting a password using `<this form>`

Fair enough, I'll do another full review today then. Thanks!

[nessita]

Hello @fsbraun!

I have made another UI review today. After looking at the UI/UX flow with a fresh head, these are my conclusions:

1- The User creation form is great, both at the usability and clarity level. I think the phrasing and functionality are just right, so I don't think changes are needed. Some screenshots (for other people that may be reading this conversation):

2- The User edition form for a user that already has an unusable password has a very clear and precise message, so again I wouldn't make any changes here:

3- BUT, when clicking the link in the screenshot above, the admin user is presented with a form that I think we need to tweak. I think the form is confusing because the admin user has to click on "set password" to being able to set a password, but the admin user just clicked on the link to set a password.

4- The above is related to another UX issue that I think we also need to tweak. The User edition form for a user with an usable password has a decent message for the password field: But when clicking on "this form", the admin user is taken to a form where the boolean "Set password" can cause confusion: "why is there a checkbox to change the password and not just the two password fields?". Also, when unchecking "Set password", the admin users has this form then: Which is even more confusing because the submit button says "Change password" but there is no password field to do anything with :-).

For the reasons above (2 and 3), I'm convinced that we need to split the password change form into two: one for changing a password for an existing user, and another one for unsetting the password for an existing user. This way we can considerably simplify the UX in each case and we could link from one form to the other if necessary.

If you don't have any availability to work on this, I can help doing the form split right after DjangoCon US which is next week. Let me know and thanks for your continued effort to push this to the finish line!

[fsbraun]

Hi @nessita ! I will give it a stab next week.

[thibaudcolas]

@nessita @fsbraun based on your request, we’ve reviewed this together with Wagtail UI designers & UI developers @benenright, @laymonage, @nicklee and myself.

Our specific goal was to provide:

Review IA of the fields + help text as-is in Fabian’s PR Some thoughts on "one form vs. two forms"

---

So their guidance is:

1. Use the same "one form with conditional fields" pattern for both "add" and "edit" for consistency and clarity. With separate "set password" and "remove password" forms, the individual forms would be clearer, but having to choose between forms (or understand there are two forms) would add confusion.
2. On the forms, switch from a "Set password" checkbox to two radio buttons, so both alternatives can be more clearly labelled.
3. As far as field order, makes sense for the "Set password" field to be where it is now.
4. The "set password: true" should always be selected by default, as that’s the most likely intent for both a new user, and editing an existing user regardless of that user’s current status.

And additionally (separate PR?), change the wording and placement of this "this form" link so it’s more obvious what the link takes you to. Possibly use a button rather than a link in text, with that button label being clear about the form’s purpose.

I didn’t ask them too much input on exact wording but they did say:

• The help text needs work
• Consider terms like "Password-less" or "Other authentication mechanism"
• Even better if we could add "most common use cases" scenarios as examples in the help text, like "Use this for SSO" or similar

I’ll try to get back to the wording on my own based on their input and propose specific tweaks.

---

Congratulations @benenright & @nicklee on a first contribution to Django 😉