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

Added password length check #506

Merged
merged 9 commits into from
Apr 8, 2022
Merged

Conversation

ArnyminerZ
Copy link
Collaborator

Should fix #430, not yet tested.

Passwords can't be longer than 100 characters. Added check for this
@ArnyminerZ ArnyminerZ added the bug Something isn't working label Apr 4, 2022
@ArnyminerZ ArnyminerZ self-assigned this Apr 4, 2022
@github-actions github-actions bot added the translation Translations label Apr 4, 2022
Copy link
Owner

@leikoilja leikoilja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!
However, i would argue that we should move this validation to glocaltokens package, since it is it's job to take in input parameters and return auth tokens. It is also probably glocaltoken that is freezing out if too long password is given.
We can move this validation to glocaltokens and raise an error during authentication. Here, we can capture the error and show the UI error message. That approach will also allow us to unittest lengthy passwords in glocaltokens. What do you think?

@ArnyminerZ
Copy link
Collaborator Author

@leikoilja I've opened leikoilja/glocaltokens#229 for this

Copy link
Owner

@leikoilja leikoilja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! Thanks @ArnyminerZ for the implementation 💥
Can you please make sure the linting checks passes and merge in when happy with it?

@ArnyminerZ ArnyminerZ merged commit b1fd957 into leikoilja:master Apr 8, 2022
@ArnyminerZ ArnyminerZ deleted the pass-len branch April 8, 2022 11:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working translation Translations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Unknown" Error on login - Looks like password is too long
2 participants