BigW Consortium Gitlab
Reduce Rack Attack false positives causing 403 errors during HTTP authentication ### What does this MR do? This MR reduces false positives causing `403 Forbidden` messages after HTTP authentication. A Git client may attempt to access a repository without a password. If it receives a 401 error, the client often will try again, this time supplying a password. The problem is that `grack_auth.rb` considers a blank password an authentication failure and increases a Redis counter each time this happens. With enough requests, an IP can be banned temporarily even though previous attempts may have been successful. This leads users to see `403 Forbidden` errors until the ban times out (default: 1 hour). To reduce the chance of a false positive, this MR resets the counter upon a successful authentication from an IP. In addition, this MR logs when a user has been banned and introduces the ability to disable Rack Attack via a config variable. ### Are there points in the code the reviewer needs to double check? rack-attack v4.2.0 doesn't support the ability to clear counters out of the box, so `rack_attack_helpers.rb` includes a number of monkey patches to make it work. It looks like this functionality may be added in v4.3.0. I've also sent pull requests to rack-attack to add the functionality necessary to delete a key. Each time an authentication is successful, the Redis counter for that IP is cleared. I deemed it better to clear the counter than to allow for blank passwords, since the latter seems like a security risk. ### Why was this MR needed? It was quite difficult to figure out why users were seeing `403 Forbidden`, which is why the log message was added. Users were getting a lot of false positives when accessing repositories with HTTPS. Including the username in the HTTPS URL (e.g. `https://username@mydomain.com/account/repo.git`) caused authentication failures because while the git client provided the username, it left the password blank, leading to an authentication failure. ### What are the relevant issue numbers / [Feature requests](http://feedback.gitlab.com/)? See Issue #1171 https://github.com/kickstarter/rack-attack/issues/113 See merge request !392
Name |
Last commit
|
Last update |
---|---|---|
.. | ||
backend | Loading commit data... | |
bitbucket_import | Loading commit data... | |
diff | Loading commit data... | |
github_import | Loading commit data... | |
gitlab_import | Loading commit data... | |
gitorious_import | Loading commit data... | |
graphs | Loading commit data... | |
ldap | Loading commit data... | |
middleware | Loading commit data... | |
oauth | Loading commit data... | |
satellite | Loading commit data... | |
sidekiq_middleware | Loading commit data... | |
access.rb | Loading commit data... | |
app_logger.rb | Loading commit data... | |
auth.rb | Loading commit data... | |
bitbucket_import.rb | Loading commit data... | |
blacklist.rb | Loading commit data... | |
closing_issue_extractor.rb | Loading commit data... | |
compare_result.rb | Loading commit data... | |
config_helper.rb | Loading commit data... | |
contributions_calendar.rb | Loading commit data... | |
contributors.rb | Loading commit data... | |
current_settings.rb | Loading commit data... | |
force_push_check.rb | Loading commit data... | |
git.rb | Loading commit data... | |
git_access.rb | Loading commit data... | |
git_access_status.rb | Loading commit data... | |
git_access_wiki.rb | Loading commit data... | |
git_logger.rb | Loading commit data... | |
git_ref_validator.rb | Loading commit data... | |
identifier.rb | Loading commit data... | |
import_formatter.rb | Loading commit data... | |
inline_diff.rb | Loading commit data... | |
issues_labels.rb | Loading commit data... | |
logger.rb | Loading commit data... | |
markdown.rb | Loading commit data... | |
markdown_helper.rb | Loading commit data... | |
note_data_builder.rb | Loading commit data... | |
popen.rb | Loading commit data... | |
production_logger.rb | Loading commit data... | |
project_search_results.rb | Loading commit data... | |
push_data_builder.rb | Loading commit data... | |
reference_extractor.rb | Loading commit data... | |
regex.rb | Loading commit data... | |
search_results.rb | Loading commit data... | |
seeder.rb | Loading commit data... | |
sidekiq_logger.rb | Loading commit data... | |
snippet_search_results.rb | Loading commit data... | |
theme.rb | Loading commit data... | |
upgrader.rb | Loading commit data... | |
url_builder.rb | Loading commit data... | |
user_access.rb | Loading commit data... | |
utils.rb | Loading commit data... | |
version_info.rb | Loading commit data... | |
visibility_level.rb | Loading commit data... |