Sign in or sign up before continuing. Don't have an account yet? Register now to get started.
Register now

Move the protected paths throttle from Omnibus to GitLab rails

After gitlab-foss#62756 (moved) / gitlab-foss!30467 (merged), we're logging the user information on auth.log, but only for Rack::Attack throttle events. It'd be useful to have the same information for blacklist events.

Technical bits

From https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/30467#note_189543725:

It looks like in Omnibus (/opt/gitlab/embedded/service/gitlab-rails/config/initializers/rack_attack.rb), we have:

  Rack::Attack.throttle('protected paths', limit: 10, period: 60.seconds) do |req|
    if req.post? && req.path =~ paths_regex
      req.ip
    end
  end

In this case we're returning the IP, and I think there are cases from support where we would like to have the username in this case.

Backend Plan

On %12.4

  • Move the protected paths into GitLab-Rails - !16463 (merged)
  • Handle the throttle like the other ones on RackAttackGlobal - !16463 (merged)
  • Update GitLab rails documentation - !16540 (merged)
  • Deprecate protected paths on Omnibus - omnibus-gitlab#4688 (closed) - omnibus-gitlab!3597 (diffs)
    • Users who have configured Omnibus can continue to use their deprecated settings.
    • Deprecation might be done with https://docs.gitlab.com/omnibus/development/adding-deprecation-messages.html
  • Update Omnibus documentation to indicate the setting is deprecated. - omnibus-gitlab!3597 (merged)

On %12.5

  • Clarify the documentation on https://docs.gitlab.com/ee/user/admin_area/settings/protected_paths.html#migrate-settings-from-gitlab-123-and-earlier
  • Fix documentation https://gitlab.com/gitlab-org/gitlab/blob/v12.4.2-ee/doc/user/admin_area/settings/protected_paths.md#migrate-settings-from-gitlab-123-and-earlier as mentioned in the comment below #29952 (comment 239927289)
  • Remove A web server restart is required after changing these settings. from app/views/admin/application_settings/network.html.haml => !26846 (merged)
  • Change Omnibus Protected Paths throttle is active. to Omnibus Protected Paths throttle is active, and takes priority over these settings. => !26846 (merged)

On %13.0

Because of the Omnibus deprecated policy, we have to wait until the next major release (%13.0) to continue with the following items:

  • Remove protected paths and rack attack from Omnibus, and update documentation - omnibus-gitlab#4750 (closed)

  • Remove protected paths code from GitLab codebase - !31221 (merged)

    • Remove rack_attack.rb.example from GitLab-Rails
    • Remove rack_attack.rb.example source installation documentation and other related text in this doc
    • Remove Omnibus specific methods on config/initializers/rack_attack_new.rb
    • Remove admin_area_protected_paths_enabled setting and related code !21090 (merged)
  • Remove protected paths code from Omnibus codebase omnibus-gitlab!3773 (merged)

    • omnibus-gitlab!4149 (merged)
    • omnibus-gitlab!4207 (merged)
  • We also need to update gitlab-rails Dockerfile from CNG mirror

  • Add to the 13.0 release post: Tell people to double-check that the protected paths application settings are the way they want them since the deprecated Omnibus code is being removed. It is possible that a small percentage of installations expect it to be enabled but it isn't: !19185 (comment 236722245)

    • Suggested to make a note about it on the "Upgrade Notes" section of the release post: gitlab-com/www-gitlab-com!48907 (comment 341014671)
    • It was added on removals gitlab-com/www-gitlab-com!48596 (merged)

On %13.1

  • [-] Rename rack_attack_new.rb back to rack_attack.rb. Follow up issue created #218291 (closed)

Development log

  • Merge request was created to add Protected paths configuration into GitLab-rails - !16463 (merged)
  • GitLab documentation is going to be updated on !16540 (merged)
  • An issue on Omnibus GitLab was created to decide the removal target date of the Protected Path settings on Omnibus omnibus-gitlab#4688 (closed)
    • Merge request that adds a deprecation notice and updates Omnibus documentation - omnibus-gitlab!3597 (merged)
  • Because the protected path throttle also exists on Omnibus, a manual test was done by Stan to check the behavior of both throttles
    • It was decided to ignore the application setting throttle if Omnibus settings are present !16463 (comment 218518797)
  • From the Omnibus Deprecation policy, removal target of Omnibus throttle will be on the next major release (%13.0)
    • Issue on Omnibus omnibus-gitlab#4750 (closed)
  • Merge request to fix a broken link on "Protected paths" setting was created - !17945 (merged)
Edited May 18, 2020 by Mayra Cabrera
Assignee Loading
Time tracking Loading