backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 38 forks source link

[UX] Login settings: Consolidate all "Limit login attempts" settings. #6567

Open klonos opened 3 months ago

klonos commented 3 months ago

The form in admin/config/people/login has the following groups of settings:

I propose that we group the 3 last items together, under a single "Failed login attempts settings" fieldset, which should be collapsed by default (since these settings are kinda advanced, and the defaults should just work for most cases).

Having the fieldset collapsed by default should also leave more room in this form for the settings that we are looking to introduce as part of #3309, which are less advanced.

We should also remove instances of "user" from labels and help text. There's quite a few of them in this form.

klonos commented 3 months ago

PR ready for initial UI review (tests still running): https://github.com/backdrop/backdrop/pull/4763

Before/after screenshot (the new fieldset is collapsed by default - expanded here): image ...and how the form looks by default, with the fieldset collapsed: image

There are many text changes, because:

avpaderno commented 3 months ago

I like that the If there are excessive failed login attempts, the offending IP address or user account will be temporarily blocked. sentence is placed before the relative settings, and not at the bottom of the form as it is now. I would not expect that the description for the Log excessive login attempts checkbox would also explain what all those settings are (and that is probably the reason I have never read that description).

klonos commented 3 months ago

I was also thinking to re-word the help text for the two radio options.

Current:

Considering this instead (bolding the bits that are important):

And some visuals: image

...basically emphasizing the bits that would otherwise be a warning for caution.

klonos commented 3 months ago

...now that I'm thinking about it, I liked how the original versions of these texts mentioned the words "lock out" instead of "block". I had to change it to "block" because I changed "users" to "user accounts", but if we changed that to "people" instead, it could be "lock people out" (of the site) instead of "block user accounts". I think I will change the PR to that instead.

klonos commented 3 months ago

...PR updated. Here's before/after of that specific part of the form: image

I initially changed the radio label to Identify people attempting to log in, using but that ("identifying people") didn't sound right. So I went with Identify the source of failed login attempts, using instead in the end. Ideas for anything better than that?

klonos commented 3 months ago

...I also don't really like less secure, especially considering the fact that we have that as the default. Perhaps say less strict or less accurate instead? Thoughts?

izmeez commented 3 months ago

Grouping the failed login items into a fieldset and having the info at the top of the fieldset look good.

The issue of other detailed wording changes may be more contentious. I don't really like more secure and less secure and not even sure they are true. I prefer the phrase more strict and less strict but again I don't know that it's true that combined ID & IP address is less.

avpaderno commented 3 months ago

I would rather not use secure nor strict. More likely to lock people out of the site and Less likely to lock people out of the site should be sufficient.

klonos commented 3 months ago

Thanks for the feedback @izmeez and @kiamlaluno 🙏🏼

What do you thing about using more accurate/less accurate? ...if these don't sound good, then I'm also inclined to go with @kiamlaluno's suggestion and just omit the more secure/less secure altogether. ...would love some feedback from others on that.

avpaderno commented 3 months ago

To make my previous comment clearer: I understand that secure means avoiding that people who try to guess the password used for an account can successfully do that. I would just avoid to define a method more secure than the other, and say that a method could (more likely) lock out people who try to access their own account.

klonos commented 3 months ago

Perhaps, we should just say exactly that(?):

Makes guessing passwords harder, but is more likely to lock people out of the site.

...I don't think that we should say the opposite ("makes guessing passwords easy") for the other option though. Perhaps throwing the word "even" into the mix would improve things:

Makes guessing passwords even harder, but is more likely to lock people out of the site.

klonos commented 3 months ago

...although "makes guessing passwords harder" is not a very accurate description of what this setting does from a technical point of view 🤔 ...it doesn't make guessing harder per se - it blocks more guessing attempts.

avpaderno commented 3 months ago

That's correct: It just limits the number of wrong passwords can be entered. As for Makes guessing passwords harder, that is not necessary true: In the case I know the password associated with an account is the person's date birthday, neither one or the other method makes guessing the password harder for me.

klonos commented 3 months ago

Thanks for all the feedback @kiamlaluno 🙏🏼 ...it is really helpful. I think I'll wait for another person or two to chime in, and then will most likely remove the mentions of "more secure"/"less secure" entirely (unless people think that they do actually have value and are accurate descriptions, or can suggest any better alternative).

klonos commented 3 months ago

...one final thought with regards to that setting specifically: should we move the default option (UID + IP combo) to be the first of the two available options? It feels odd the way it is placed last now.