This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
Only do rc_login ratelimiting on succesful login.
#6335
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
erikjohnston
force-pushed
the
erikj/rc_login_cleanups
branch
2 times, most recently
from
3e49197 to
17f6269
Compare
We were doing this in a number of places which meant that some login code paths incremented the counter multiple times. It was also applying ratelimiting to UIA endpoints, which was probably not intentional. In particular, some custom auth modules were calling `check_user_exists`, which incremented the counters, meaning that people would fail to login sometimes.
erikjohnston
force-pushed
the
erikj/rc_login_cleanups
branch
from
17f6269 to
4fc53bf
Compare
Co-Authored-By: Andrew Morgan <[email protected]> Co-Authored-By: Brendan Abolivier <[email protected]>
babolivier
pushed a commit
that referenced
this pull request
Sep 1, 2021
* commit '83446a18f': Lint Apply suggestions from code review Newsfile Add failed auth ratelimiting to UIA Only do `rc_login` ratelimiting on succesful login.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We were doing this in a number of places which meant that some login code paths incremented the counter multiple times.
It was also applying ratelimiting to UIA endpoints, which was probably not intentional.
In particular, some custom auth modules were calling
check_user_exists, which incremented the counters, meaning that people would fail to login sometimes.A side effect of this is that we don't ratelimit the SSO path (as that was obscurely relying on
check_user_existsbeing ratelimited??) until after we've successfully authed with the SSO provider, and rely on the remote SSO provider to do its own ratelimiting. We do still ratelimit when the client then logs in with the provider token via/login.This also applies a separate rate limit to failed UIA auth attempts, to protect someone attempting to extract a password from a logged in session.
Note: no failed rate limiting is applied by token style logins as they're not associated with any user ID.