Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

password complexity requirement not shown to user #8910

Closed
2 of 4 tasks
8ctopus opened this issue Nov 10, 2019 · 7 comments · Fixed by #9074
Closed
2 of 4 tasks

password complexity requirement not shown to user #8910

8ctopus opened this issue Nov 10, 2019 · 7 comments · Fixed by #9074
Labels
topic/ui Change the appearance of the Gitea UI

Comments

@8ctopus
Copy link
Contributor

8ctopus commented Nov 10, 2019

  • Gitea version (or commit ref): 1.10.0-rc2
  • Git version: 2.17.1
  • Operating system: Ubuntu 18.04.3 LTS
  • Database (use [x]):
    • MySQL
  • Can you reproduce the bug at https://try.gitea.io:
    • Yes (provide example URL)
    • No
    • Not relevant

Description

The password complexity requirement rules are not shown on account creation/password reset so it's hard for users to figure out what is a valid password and what isn't. Therefore it would be great to show the rules in the error message as on the screenshot.

2019-11-10_113606

@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Nov 10, 2019
@lunny
Copy link
Member

lunny commented Nov 10, 2019

This is something annoying.

@bwenrich
Copy link
Contributor

This would be a good usability improvement, so users will know immediately what they have to correct.

Ideally, it could identify which rule(s) are not passing, but I see two problems with that based on existing logic in https://github.com/go-gitea/gitea/blob/master/modules/password/password.go#L58:

  1. The function returns only a boolean, without identifying the failed rule(s)
  2. The function appears to exit as soon as the first failure is found. So it is possible that another complexity rule would also fail, but it not yet checked.

Since the complexity rules are also configurable with parameters like MIN_PASSWORD_LENGTH and PASSWORD_COMPLEXITY, there would need to be some kind of logic to dynamically construct the summary of complexity rules out of locale strings.

@zeripath
Copy link
Contributor

@bwenrich would you be able to put up a pr?

@8ctopus
Copy link
Contributor Author

8ctopus commented Nov 11, 2019

@bwenrich some of the logic must be implemented separately as if you make a password that is too short, you will get the appropriate error message: "Password length cannot be less than 6 characters."

@bwenrich
Copy link
Contributor

Thank you @zeripath and @8ctopus for the feedback. I am not familiar yet with the full logic flow of the password validation and all the places it might be used.

I see now that there is an existing separate check for setting.MinPasswordLength which produces the "password_too_short" locale message. And then the password.IsComplexEnough function checks other complexity rules associated with the "password_complexity" locale message.

My opinion is that there should be some functionality to show all the password requirements to the user (both length and complexity), though I don't know how to implement this.

Should there also be some way to see this (ie: a tooltip or collapseable menu) before submitting the page?

@guillep2k
Copy link
Member

I don't think it's necessary to show exactly what failed. Only what rules are in place. e.g.:

You password don't pass the minimum requirements:
- Minimum length of 6 characters.
- At least one digit.
- At least one uppercase letter ...

etc. This message requires less changes and it's easier to synthesize and translate.

@8ctopus
Copy link
Contributor Author

8ctopus commented Nov 12, 2019

agreed just rules would suffice.

@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants