• Dmitriy Zaporozhets's avatar
    Merge branch 'git-auth-rack-attack-improvements' into 'master' · fc4af9b1
    Dmitriy Zaporozhets authored
    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
    fc4af9b1
To find the state of this project's repository at the time of any of these versions, check out the tags.
CHANGELOG 53.3 KB