-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Limit the rate of logins #554
Conversation
self._assert_response(response, success=True) | ||
|
||
def test_login_ratelimited(self): | ||
# try logging in 30 times |
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.
comment doesn't match the code
Okay, I've addressed all the current comments. Could you take another look? |
try: | ||
user = authenticate(username=username, password=password, request=request) | ||
except RateLimitException: | ||
log.warning('OpendID - Too many failed login attempts.') |
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.
s/OpendID/OpenId/. Also, shouldn't this write to AUDIT_LOG instead of log?
Looks good.... |
…file-view Set request.user = student when impersonating a student
…eoquiz-version Push up to version 0.1.2 of in video quiz
ENH: grades now returned on a hierarchical level bases.
ENH: grades now returned on a hierarchical level bases.
…edx#554) In https://github.com/edx/edx-platform/pull/28233, the logic was updated to only return a URL if the content was still accessible to the learner. This handles the case of the URL being null
This pull request adds rate limiting to our authentication backend.
This pull request covers:
These things are deliberately not tested after talking to Test Engineering:
This does not cover additional work needed for logging, since that relies on pull request 539 to implemented.
@brianhw @nedbat , review?