-
Notifications
You must be signed in to change notification settings - Fork 69
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
Add check if user exceeded attempt of logins #36
Conversation
a3b1f33
to
b04f63d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One style change, otherwise LGTM!
@@ -25,13 +41,40 @@ def add_new_table(self): | |||
User.info = relationship(UserInfo, backref='users') | |||
UserInfo.__table__.create(self.db.bind) | |||
|
|||
def exceed_atempts_of_login(self, username): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method does two things:
- Check if the user has exceeded their limit
- Record that the user has attempted a log in
Instead, this should be three functions:
- Records that the user has attempted to log in (mutating the dictionary)
- Check if the user has exceeded their limit (check the dictionary)
- Reset user's entry in dictionary after a successful login
This clearly separates different actions on the data, making it easier to understand in the longer run.
The previous version to check if a user exceeded a number of failed logins everything was checked in a single method. This commit splits the responsabilities between 4 functions with small responsabilites each
eed363c
to
3197739
Compare
LGTM! |
Closes #21