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

Fix #154 #155

Merged
merged 1 commit into from
Jan 29, 2016
Merged

Fix #154 #155

merged 1 commit into from
Jan 29, 2016

Conversation

michalsn
Copy link
Contributor

This one fixes #154.

I'm not quite sure if validate method shoud contains isThrottled check... maybe someone can came up with scenario where this behavior will be an issue?

@lonnieezell
Copy link
Member

Originally I was thinking that validate() could be used as a standalone method where it would be bad to have the isThrottled check in there... but I can't think of why I was thinking that at the time.

At some point, I'd like to get the whole throttling process moved to it's own library that can be used elsewhere, but need to do some more studying up on the best way to handle that site-wide.

@michalsn
Copy link
Contributor Author

Well... so far I couldn't find any other use case for validate method.

Another possibility is to add one more parameter i.e. validate($credentials, $return_user=false, $record=true) - a little ugly but will work.

lonnieezell added a commit that referenced this pull request Jan 29, 2016
@lonnieezell lonnieezell merged commit 5149cc9 into ci-bonfire:develop Jan 29, 2016
@michalsn michalsn deleted the patch/patch-7 branch January 29, 2016 15:00
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.

[Auth] Login security issue
2 participants