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

Fire signal when user attempts to login with invalid password #1

Merged
merged 2 commits into from
Aug 14, 2018

Conversation

CodeRhymesLife
Copy link
Collaborator

Addresses the following issue in Flask-User where there is no way to be notified when a user attempts to login with an invalid password.

These changes:

  1. Create a signal for invalid password
  2. Fire signal when user's password is invalid
  3. Update documentation

Once this is in I'll submit a PR against Flask-User.

@CodeRhymesLife CodeRhymesLife self-assigned this Aug 14, 2018
@CodeRhymesLife CodeRhymesLife requested a review from pbugni August 14, 2018 01:07
@@ -7,5 +7,6 @@
user_forgot_password # a user submitted a reset password request
user_logged_in # a user logged in
user_logged_out # a user logged out
user_password_invalid # a user attempted to login with invalid password
Copy link

Choose a reason for hiding this comment

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

hmm, don't love "invalid", as it's a failed verification or lookup, not that the password itself is invalid.
user_password_attempt_failed ?

@@ -36,6 +36,9 @@
# Sent when a user logged out
user_logged_out = _signals.signal('user.user_logged_out')

# Sent when a user attempted to login and password verification failed
user_password_failed = _signals.signal('user.password_failed')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looking a little closer, his other variables follow this pattern:

user_<action>

Our doesn't. To follow this pattern we should go with something like:

user_failed_password_verification

This is unfortunately a long name. I'm going to stick with user_password_failed, submit a PR and leave a comment to see what he thinks.

@CodeRhymesLife CodeRhymesLife merged commit 94ff450 into uwcirg:master Aug 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants