diff options
| -rw-r--r-- | app/controllers/application_controller.rb | 12 | ||||
| -rw-r--r-- | changelogs/unreleased/35617-only-blacklist-git-auth.yml | 5 | ||||
| -rw-r--r-- | config/initializers/rack_attack_git_basic_auth.rb | 14 | ||||
| -rw-r--r-- | config/routes/project.rb | 30 | ||||
| -rw-r--r-- | doc/development/packages.md | 23 | ||||
| -rw-r--r-- | doc/user/project/merge_requests/creating_merge_requests.md | 2 | ||||
| -rw-r--r-- | doc/user/project/repository/img/web_editor_new_branch_from_issue.png | bin | 2715 -> 0 bytes | |||
| -rw-r--r-- | doc/user/project/repository/img/web_editor_new_branch_from_issue_create_button_v12_6.png | bin | 0 -> 70114 bytes | |||
| -rw-r--r-- | doc/user/project/repository/img/web_editor_new_branch_from_issue_v_12_6.png | bin | 0 -> 76938 bytes | |||
| -rw-r--r-- | doc/user/project/repository/web_editor.md | 34 | ||||
| -rw-r--r-- | lib/gitlab/auth.rb | 24 | ||||
| -rw-r--r-- | lib/gitlab/auth/ip_rate_limiter.rb | 29 | ||||
| -rw-r--r-- | spec/controllers/application_controller_spec.rb | 26 | ||||
| -rw-r--r-- | spec/lib/gitlab/auth/ip_rate_limiter_spec.rb | 32 | ||||
| -rw-r--r-- | spec/lib/gitlab/auth_spec.rb | 104 | ||||
| -rw-r--r-- | spec/requests/git_http_spec.rb | 2 |
16 files changed, 231 insertions, 106 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 4d55d7f00f0..ee2b3741ac9 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -74,6 +74,18 @@ class ApplicationController < ActionController::Base render_403 end + rescue_from Gitlab::Auth::IpBlacklisted do + Gitlab::AuthLogger.error( + message: 'Rack_Attack', + env: :blocklist, + remote_ip: request.ip, + request_method: request.request_method, + path: request.fullpath + ) + + head :forbidden + end + rescue_from Gitlab::Auth::TooManyIps do |e| head :forbidden, retry_after: Gitlab::Auth::UniqueIpsLimiter.config.unique_ips_limit_time_window end diff --git a/changelogs/unreleased/35617-only-blacklist-git-auth.yml b/changelogs/unreleased/35617-only-blacklist-git-auth.yml new file mode 100644 index 00000000000..ce77fb14b7f --- /dev/null +++ b/changelogs/unreleased/35617-only-blacklist-git-auth.yml @@ -0,0 +1,5 @@ +--- +title: Only blacklist IPs from Git requests +merge_request: 20828 +author: +type: changed diff --git a/config/initializers/rack_attack_git_basic_auth.rb b/config/initializers/rack_attack_git_basic_auth.rb deleted file mode 100644 index 71e5e2969ce..00000000000 --- a/config/initializers/rack_attack_git_basic_auth.rb +++ /dev/null @@ -1,14 +0,0 @@ -# Tell the Rack::Attack Rack middleware to maintain an IP blacklist. -# We update the blacklist in Gitlab::Auth::IpRateLimiter. -Rack::Attack.blocklist('Git HTTP Basic Auth') do |req| - rate_limiter = Gitlab::Auth::IpRateLimiter.new(req.ip) - - next false if !rate_limiter.enabled? || rate_limiter.trusted_ip? - - Rack::Attack::Allow2Ban.filter(req.ip, Gitlab.config.rack_attack.git_basic_auth) do - # This block only gets run if the IP was not already banned. - # Return false, meaning that we do not see anything wrong with the - # request at this time - false - end -end diff --git a/config/routes/project.rb b/config/routes/project.rb index 848846b5f5b..8a5e20c8eff 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -58,7 +58,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :trace, defaults: { format: 'json' } get :raw get :terminal - get '/terminal.ws/authorize', to: 'jobs#terminal_websocket_authorize', constraints: { format: nil } + get '/terminal.ws/authorize', to: 'jobs#terminal_websocket_authorize', format: false end resource :artifacts, only: [] do @@ -228,7 +228,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :metrics get :additional_metrics get :metrics_dashboard - get '/terminal.ws/authorize', to: 'environments#terminal_websocket_authorize', constraints: { format: nil } + get '/terminal.ws/authorize', to: 'environments#terminal_websocket_authorize', format: false get '/prometheus/api/v1/*proxy_path', to: 'environments/prometheus_api#proxy', as: :prometheus_api end @@ -328,13 +328,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :test_reports get :exposed_artifacts - scope constraints: { format: nil }, action: :show do - get :commits, defaults: { tab: 'commits' } - get :pipelines, defaults: { tab: 'pipelines' } - get :diffs, defaults: { tab: 'diffs' } - end - - scope constraints: { format: 'json' }, as: :json do + scope constraints: ->(req) { req.format == :json }, as: :json do get :commits get :pipelines get :diffs, to: 'merge_requests/diffs#show' @@ -344,6 +338,12 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :cached_widget, to: 'merge_requests/content#cached_widget' end + scope action: :show do + get :commits, defaults: { tab: 'commits' } + get :pipelines, defaults: { tab: 'pipelines' } + get :diffs, defaults: { tab: 'diffs' } + end + get :diff_for_path, controller: 'merge_requests/diffs' scope controller: 'merge_requests/conflicts' do @@ -372,16 +372,16 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do scope path: 'new', as: :new_merge_request do get '', action: :new - scope constraints: { format: nil }, action: :new do - get :diffs, defaults: { tab: 'diffs' } - get :pipelines, defaults: { tab: 'pipelines' } - end - - scope constraints: { format: 'json' }, as: :json do + scope constraints: ->(req) { req.format == :json }, as: :json do get :diffs get :pipelines end + scope action: :new do + get :diffs, defaults: { tab: 'diffs' } + get :pipelines, defaults: { tab: 'pipelines' } + end + get :diff_for_path get :branch_from get :branch_to diff --git a/doc/development/packages.md b/doc/development/packages.md index 951a8f2b346..b2045c699a3 100644 --- a/doc/development/packages.md +++ b/doc/development/packages.md @@ -74,6 +74,29 @@ PUT https://gitlab.com/api/v4/projects/<your_project_id>/packages/npm/ Group-level and instance-level endpoints are good to have but are optional. +### Remote hierarchy + +Packages are scoped within various levels of access, which is generally configured by setting your remote. A +remote endpoint may be set at the project level, meaning when installing packages, only packages belonging to that +project will be visible. Alternatively, a group-level endpoint may be used to allow visibility to all packages +within a given group. Lastly, an instance-level endpoint can be used to allow visibility to all packages within an +entire GitLab instance. + +Using group and project level endpoints will allow for more flexibility in package naming, however, more remotes +will have to be managed. Using instance level endpoints requires [stricter naming conventions](#naming-conventions). + +The current state of existing package registries availability is: + +| Repository Type | Project Level | Group Level | Instance Level | +|-----------------|---------------|-------------|----------------| +| Maven | Yes | Yes | Yes | +| Conan | No - [open issue](https://gitlab.com/gitlab-org/gitlab/issues/11679) | No - [open issue](https://gitlab.com/gitlab-org/gitlab/issues/11679) | Yes | +| NPM | No - [open issue](https://gitlab.com/gitlab-org/gitlab/issues/36853) | Yes | No - [open issue](https://gitlab.com/gitlab-org/gitlab/issues/36853) | + +NOTE: **Note:** NPM is currently a hybrid of the instance level and group level. +It is using the top-level group or namespace as the defining portion of the name +(for example, `@my-group-name/my-package-name`). + ## Naming conventions To avoid name conflict for instance-level endpoints you will need to define a package naming convention diff --git a/doc/user/project/merge_requests/creating_merge_requests.md b/doc/user/project/merge_requests/creating_merge_requests.md index 084ebf32a92..1dec58a8bb0 100644 --- a/doc/user/project/merge_requests/creating_merge_requests.md +++ b/doc/user/project/merge_requests/creating_merge_requests.md @@ -31,6 +31,8 @@ also appear in the top right of the: In this case, the merge request will use the most recent branch you pushed changes to as the source branch, and `master` in the current project as the target. +You can also [create a new merge request directly from an issue](../repository/web_editor.md#create-a-new-branch-from-an-issue). + ## Workflow for new merge requests On the **New Merge Request** page, you can start by filling in the title and description diff --git a/doc/user/project/repository/img/web_editor_new_branch_from_issue.png b/doc/user/project/repository/img/web_editor_new_branch_from_issue.png Binary files differdeleted file mode 100644 index 4e156b8adc8..00000000000 --- a/doc/user/project/repository/img/web_editor_new_branch_from_issue.png +++ /dev/null diff --git a/doc/user/project/repository/img/web_editor_new_branch_from_issue_create_button_v12_6.png b/doc/user/project/repository/img/web_editor_new_branch_from_issue_create_button_v12_6.png Binary files differnew file mode 100644 index 00000000000..f40cc187b46 --- /dev/null +++ b/doc/user/project/repository/img/web_editor_new_branch_from_issue_create_button_v12_6.png diff --git a/doc/user/project/repository/img/web_editor_new_branch_from_issue_v_12_6.png b/doc/user/project/repository/img/web_editor_new_branch_from_issue_v_12_6.png Binary files differnew file mode 100644 index 00000000000..d5a92546d40 --- /dev/null +++ b/doc/user/project/repository/img/web_editor_new_branch_from_issue_v_12_6.png diff --git a/doc/user/project/repository/web_editor.md b/doc/user/project/repository/web_editor.md index ef7cfdb8d8e..f41ff12d0a4 100644 --- a/doc/user/project/repository/web_editor.md +++ b/doc/user/project/repository/web_editor.md @@ -95,20 +95,31 @@ There are multiple ways to create a branch from GitLab's web interface. In case your development workflow dictates to have an issue for every merge request, you can quickly create a branch right on the issue page which will be -tied with the issue itself. You can see a **New branch** button after the issue -description, unless there is already a branch with the same name or a referenced -merge request. +tied with the issue itself. You can see a **Create merge request** dropdown +below the issue description unless there is already a branch with the same +name or a referenced merge request. - + -Once you click it, a new branch will be created that diverges from the default +This dropdown contains the options **Create merge request and branch** and **Create branch**. + + + +Once you choose one of these options, a new branch or branch and merge request +will be created, based on the default branch of your project, by default `master`. The branch name will be based on the title of the issue and as a prefix, it will have its internal ID. Thus, the example -screenshot above will yield a branch named -`23177-add-support-for-rich-references-to-referables`. - -Since GitLab 9.0, when you click the `New branch` in an empty repository project, GitLab automatically creates the master branch, commits a blank `README.md` file to it and creates and redirects you to a new branch based on the issue title. -If your [project is already configured with a deployment service][project-services-doc] (e.g. Kubernetes), GitLab takes one step further and prompts you to set up [auto deploy][auto-deploy-doc] by helping you create a `.gitlab-ci.yml` file. +screenshot above will create a branch named +`2-make-static-site-auto-deploy-and-serve`. + +When you click the **Create branch** button in an empty +repository project, GitLab automatically creates a `master` branch, commits +a blank `README.md` file to it, and creates and redirects you to a new branch +based on the issue title. +If your [project is already configured with a deployment service](../integrations/project_services.md), +such as Kubernetes, GitLab takes one step further and prompts you to set up +[auto deploy](../../../topics/autodevops/index.md#auto-deploy) +by helping you create a `.gitlab-ci.yml` file. After the branch is created, you can edit files in the repository to fix the issue. When a merge request is created based on the newly created branch, @@ -116,9 +127,6 @@ the description field will automatically display the [issue closing pattern](../ `Closes #ID`, where `ID` the ID of the issue. This will close the issue once the merge request is merged. -[project-services-doc]: ../integrations/project_services.md -[auto-deploy-doc]: ../../../topics/autodevops/index.md#auto-deploy - ### Create a new branch from a project's dashboard If you want to make changes to several files before creating a new merge diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index 4217859f9fb..dfdba617cb6 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -3,6 +3,7 @@ module Gitlab module Auth MissingPersonalAccessTokenError = Class.new(StandardError) + IpBlacklisted = Class.new(StandardError) # Scopes used for GitLab API access API_SCOPES = [:api, :read_user].freeze @@ -35,6 +36,10 @@ module Gitlab def find_for_git_client(login, password, project:, ip:) raise "Must provide an IP for rate limiting" if ip.nil? + rate_limiter = Gitlab::Auth::IpRateLimiter.new(ip) + + raise IpBlacklisted if !skip_rate_limit?(login: login) && rate_limiter.banned? + # `user_with_password_for_git` should be the last check # because it's the most expensive, especially when LDAP # is enabled. @@ -48,7 +53,7 @@ module Gitlab user_with_password_for_git(login, password) || Gitlab::Auth::Result.new - rate_limit!(ip, success: result.success?, login: login) unless skip_rate_limit?(login: login) + rate_limit!(rate_limiter, success: result.success?, login: login) Gitlab::Auth::UniqueIpsLimiter.limit_user!(result.actor) return result if result.success? || authenticate_using_internal_or_ldap_password? @@ -96,10 +101,11 @@ module Gitlab end end + private + # rubocop:disable Gitlab/RailsLogger - def rate_limit!(ip, success:, login:) - rate_limiter = Gitlab::Auth::IpRateLimiter.new(ip) - return unless rate_limiter.enabled? + def rate_limit!(rate_limiter, success:, login:) + return if skip_rate_limit?(login: login) if success # Repeated login 'failures' are normal behavior for some Git clients so @@ -109,18 +115,16 @@ module Gitlab else # Register a login failure so that Rack::Attack can block the next # request from this IP if needed. - rate_limiter.register_fail! - - if rate_limiter.banned? - Rails.logger.info "IP #{ip} failed to login " \ + # This returns true when the failures are over the threshold and the IP + # is banned. + if rate_limiter.register_fail! + Rails.logger.info "IP #{rate_limiter.ip} failed to login " \ "as #{login} but has been temporarily banned from Git auth" end end end # rubocop:enable Gitlab/RailsLogger - private - def skip_rate_limit?(login:) ::Ci::Build::CI_REGISTRY_USER == login end diff --git a/lib/gitlab/auth/ip_rate_limiter.rb b/lib/gitlab/auth/ip_rate_limiter.rb index acb46abb6f3..f301a2ec2e8 100644 --- a/lib/gitlab/auth/ip_rate_limiter.rb +++ b/lib/gitlab/auth/ip_rate_limiter.rb @@ -9,41 +9,48 @@ module Gitlab def initialize(ip) @ip = ip - @banned = false - end - - def enabled? - config.enabled end def reset! + return if skip_rate_limit? + Rack::Attack::Allow2Ban.reset(ip, config) end def register_fail! - return false if trusted_ip? + return false if skip_rate_limit? # Allow2Ban.filter will return false if this IP has not failed too often yet - @banned = Rack::Attack::Allow2Ban.filter(ip, config) do + Rack::Attack::Allow2Ban.filter(ip, config) do # We return true to increment the count for this IP true end end def banned? - @banned - end + return false if skip_rate_limit? - def trusted_ip? - trusted_ips.any? { |netmask| netmask.include?(ip) } + Rack::Attack::Allow2Ban.banned?(ip) end private + def skip_rate_limit? + !enabled? || trusted_ip? + end + + def enabled? + config.enabled + end + def config Gitlab.config.rack_attack.git_basic_auth end + def trusted_ip? + trusted_ips.any? { |netmask| netmask.include?(ip) } + end + def trusted_ips strong_memoize(:trusted_ips) do config.ip_whitelist.map do |proxy| diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index 04bbffc587f..9c6ea13f948 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -867,5 +867,31 @@ describe ApplicationController do it { is_expected.not_to redirect_to users_sign_up_welcome_path } end + + describe 'rescue_from Gitlab::Auth::IpBlacklisted' do + controller(described_class) do + skip_before_action :authenticate_user! + + def index + raise Gitlab::Auth::IpBlacklisted + end + end + + it 'returns a 403 and logs the request' do + expect(Gitlab::AuthLogger).to receive(:error).with({ + message: 'Rack_Attack', + env: :blocklist, + remote_ip: '1.2.3.4', + request_method: 'GET', + path: '/anonymous' + }) + + request.remote_addr = '1.2.3.4' + + get :index + + expect(response).to have_gitlab_http_status(:forbidden) + end + end end end diff --git a/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb b/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb index 8d6bf45ab30..aea1b2921b6 100644 --- a/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb +++ b/spec/lib/gitlab/auth/ip_rate_limiter_spec.rb @@ -62,4 +62,36 @@ describe Gitlab::Auth::IpRateLimiter, :use_clean_rails_memory_store_caching do it_behaves_like 'whitelisted IPs' end end + + shared_examples 'skips the rate limiter' do + it 'does not call Rack::Attack::Allow2Ban.reset!' do + expect(Rack::Attack::Allow2Ban).not_to receive(:reset!) + + subject.reset! + end + + it 'does not call Rack::Attack::Allow2Ban.banned?' do + expect(Rack::Attack::Allow2Ban).not_to receive(:banned?) + + subject.banned? + end + + it 'does not call Rack::Attack::Allow2Ban.filter' do + expect(Rack::Attack::Allow2Ban).not_to receive(:filter) + + subject.register_fail! + end + end + + context 'when IP is whitlisted' do + let(:ip) { '127.0.0.1' } + + it_behaves_like 'skips the rate limiter' + end + + context 'when rate limiter is disabled' do + let(:options) { { enabled: false } } + + it_behaves_like 'skips the rate limiter' + end end diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb index dc4b0b5b1b6..d3f3b67aa90 100644 --- a/spec/lib/gitlab/auth_spec.rb +++ b/spec/lib/gitlab/auth_spec.rb @@ -79,6 +79,66 @@ describe Gitlab::Auth do end describe 'find_for_git_client' do + describe 'rate limiting' do + before do + stub_rack_attack_setting(enabled: true, ip_whitelist: []) + end + + context 'when IP is already banned' do + subject { gl_auth.find_for_git_client('username', 'password', project: nil, ip: 'ip') } + + before do + expect_next_instance_of(Gitlab::Auth::IpRateLimiter) do |rate_limiter| + expect(rate_limiter).to receive(:banned?).and_return(true) + end + end + + it 'raises an IpBlacklisted exception' do + expect { subject }.to raise_error(Gitlab::Auth::IpBlacklisted) + end + end + + context 'for CI registry user' do + let_it_be(:build) { create(:ci_build, :running) } + + it 'skips rate limiting for successful auth' do + expect_next_instance_of(Gitlab::Auth::IpRateLimiter) do |rate_limiter| + expect(rate_limiter).not_to receive(:reset!) + end + + gl_auth.find_for_git_client('gitlab-ci-token', build.token, project: build.project, ip: 'ip') + end + + it 'skips rate limiting for failed auth' do + expect_next_instance_of(Gitlab::Auth::IpRateLimiter) do |rate_limiter| + expect(rate_limiter).not_to receive(:register_fail!) + end + + gl_auth.find_for_git_client('gitlab-ci-token', 'wrong_token', project: build.project, ip: 'ip') + end + end + + context 'for other users' do + let_it_be(:user) { create(:user) } + + it 'resets rate limit for successful auth' do + expect_next_instance_of(Gitlab::Auth::IpRateLimiter) do |rate_limiter| + expect(rate_limiter).to receive(:reset!) + end + + gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip') + end + + it 'registers failure for failed auth' do + expect_next_instance_of(Gitlab::Auth::IpRateLimiter) do |rate_limiter| + expect(rate_limiter).to receive(:register_fail!) + end + + gl_auth.find_for_git_client(user.username, 'wrong_password', project: nil, ip: 'ip') + end + end + end + context 'build token' do subject { gl_auth.find_for_git_client('gitlab-ci-token', build.token, project: project, ip: 'ip') } @@ -86,10 +146,6 @@ describe Gitlab::Auth do let!(:build) { create(:ci_build, :running) } let(:project) { build.project } - before do - expect(gl_auth).not_to receive(:rate_limit!).with('ip', success: true, login: 'gitlab-ci-token') - end - it 'recognises user-less build' do expect(subject).to eq(Gitlab::Auth::Result.new(nil, build.project, :ci, described_class.build_authentication_abilities)) end @@ -106,10 +162,6 @@ describe Gitlab::Auth do let!(:build) { create(:ci_build, status: build_status) } let(:project) { build.project } - before do - expect(gl_auth).not_to receive(:rate_limit!).with('ip', success: false, login: 'gitlab-ci-token') - end - it 'denies authentication' do expect(subject).to eq(Gitlab::Auth::Result.new) end @@ -121,14 +173,12 @@ describe Gitlab::Auth do project.create_drone_ci_service(active: true) project.drone_ci_service.update(token: 'token') - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: 'drone-ci-token') expect(gl_auth.find_for_git_client('drone-ci-token', 'token', project: project, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, project, :ci, described_class.build_authentication_abilities)) end it 'recognizes master passwords' do user = create(:user, password: 'password') - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: user.username) expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, described_class.full_authentication_abilities)) end @@ -145,7 +195,6 @@ describe Gitlab::Auth do user = create(:user) token = Gitlab::LfsToken.new(user).token - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: user.username) expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :lfs_token, described_class.read_write_project_authentication_abilities)) end @@ -153,7 +202,6 @@ describe Gitlab::Auth do key = create(:deploy_key) token = Gitlab::LfsToken.new(key).token - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}") expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, described_class.read_only_authentication_abilities)) end @@ -171,7 +219,6 @@ describe Gitlab::Auth do create(:deploy_keys_project, :write_access, deploy_key: key, project: project) token = Gitlab::LfsToken.new(key).token - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}") expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: project, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, described_class.read_write_authentication_abilities)) end @@ -179,7 +226,6 @@ describe Gitlab::Auth do key = create(:deploy_key) token = Gitlab::LfsToken.new(key).token - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}") expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: project, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, described_class.read_only_authentication_abilities)) end end @@ -190,14 +236,12 @@ describe Gitlab::Auth do let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) } it 'succeeds for OAuth tokens with the `api` scope' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: 'oauth2') expect(gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :oauth, described_class.full_authentication_abilities)) end it 'fails for OAuth tokens with other scopes' do token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'read_user') - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'oauth2') expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil)) end @@ -257,7 +301,7 @@ describe Gitlab::Auth do end context 'while using regular user and password' do - it 'falls through lfs authentication' do + it 'goes through lfs authentication' do user = create( :user, username: 'normal_user', @@ -268,7 +312,7 @@ describe Gitlab::Auth do .to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, described_class.full_authentication_abilities)) end - it 'fails through oauth authentication when the username is oauth2' do + it 'goes through oauth authentication when the username is oauth2' do user = create( :user, username: 'oauth2', @@ -283,7 +327,6 @@ describe Gitlab::Auth do it 'returns double nil for invalid credentials' do login = 'foo' - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login) expect(gl_auth.find_for_git_client(login, 'bar', project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new) end @@ -301,10 +344,6 @@ describe Gitlab::Auth do let(:user) { create(:user, username: username, password: 'my-secret') } let(:deploy_token) { create(:deploy_token, username: username, read_registry: false, projects: [project]) } - before do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: username) - end - it 'succeeds for the token' do auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, [:download_code]) @@ -328,13 +367,11 @@ describe Gitlab::Auth do it 'succeeds for the right token' do auth_success = Gitlab::Auth::Result.new(read_repository, project, :deploy_token, [:download_code]) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: 'deployer') expect(gl_auth.find_for_git_client('deployer', read_repository.token, project: project, ip: 'ip')) .to eq(auth_success) end it 'fails for the wrong token' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'deployer') expect(gl_auth.find_for_git_client('deployer', read_registry.token, project: project, ip: 'ip')) .to eq(auth_failure) end @@ -347,13 +384,11 @@ describe Gitlab::Auth do it 'succeeds for the right token' do auth_success = Gitlab::Auth::Result.new(read_repository, project, :deploy_token, [:download_code]) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: 'deployer') expect(gl_auth.find_for_git_client('deployer', read_repository.token, project: project, ip: 'ip')) .to eq(auth_success) end it 'fails for the wrong token' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'deployer') expect(gl_auth.find_for_git_client('deployer', read_registry.token, project: project, ip: 'ip')) .to eq(auth_failure) end @@ -367,7 +402,6 @@ describe Gitlab::Auth do it 'succeeds when login and token are valid' do auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, [:download_code]) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: login) expect(gl_auth.find_for_git_client(login, deploy_token.token, project: project, ip: 'ip')) .to eq(auth_success) end @@ -376,32 +410,27 @@ describe Gitlab::Auth do deploy_token = create(:deploy_token, username: 'deployer', read_registry: false, projects: [project]) auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, [:download_code]) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: 'deployer') expect(gl_auth.find_for_git_client('deployer', deploy_token.token, project: project, ip: 'ip')) .to eq(auth_success) end it 'fails when login is not valid' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'random_login') expect(gl_auth.find_for_git_client('random_login', deploy_token.token, project: project, ip: 'ip')) .to eq(auth_failure) end it 'fails when token is not valid' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login) expect(gl_auth.find_for_git_client(login, '123123', project: project, ip: 'ip')) .to eq(auth_failure) end it 'fails if token is nil' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login) expect(gl_auth.find_for_git_client(login, nil, project: project, ip: 'ip')) .to eq(auth_failure) end it 'fails if token is not related to project' do another_deploy_token = create(:deploy_token) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login) expect(gl_auth.find_for_git_client(login, another_deploy_token.token, project: project, ip: 'ip')) .to eq(auth_failure) end @@ -410,7 +439,6 @@ describe Gitlab::Auth do deploy_token.revoke! expect(deploy_token.revoked?).to be_truthy - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'deploy-token') expect(gl_auth.find_for_git_client('deploy-token', deploy_token.token, project: project, ip: 'ip')) .to eq(auth_failure) end @@ -428,31 +456,26 @@ describe Gitlab::Auth do it 'succeeds when login and token are valid' do auth_success = Gitlab::Auth::Result.new(deploy_token, project, :deploy_token, [:read_container_image]) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: login) expect(gl_auth.find_for_git_client(login, deploy_token.token, project: nil, ip: 'ip')) .to eq(auth_success) end it 'fails when login is not valid' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'random_login') expect(gl_auth.find_for_git_client('random_login', deploy_token.token, project: project, ip: 'ip')) .to eq(auth_failure) end it 'fails when token is not valid' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login) expect(gl_auth.find_for_git_client(login, '123123', project: project, ip: 'ip')) .to eq(auth_failure) end it 'fails if token is nil' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login) expect(gl_auth.find_for_git_client(login, nil, project: nil, ip: 'ip')) .to eq(auth_failure) end it 'fails if token is not related to project' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login) expect(gl_auth.find_for_git_client(login, 'abcdef', project: nil, ip: 'ip')) .to eq(auth_failure) end @@ -461,7 +484,6 @@ describe Gitlab::Auth do deploy_token.revoke! expect(deploy_token.revoked?).to be_truthy - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'deploy-token') expect(gl_auth.find_for_git_client('deploy-token', deploy_token.token, project: nil, ip: 'ip')) .to eq(auth_failure) end @@ -473,7 +495,6 @@ describe Gitlab::Auth do end it 'fails when login and token are valid' do - expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login) expect(gl_auth.find_for_git_client(login, deploy_token.token, project: nil, ip: 'ip')) .to eq(auth_failure) end @@ -586,7 +607,6 @@ describe Gitlab::Auth do private def expect_results_with_abilities(personal_access_token, abilities, success = true) - expect(gl_auth).to receive(:rate_limit!).with('ip', success: success, login: '') expect(gl_auth.find_for_git_client('', personal_access_token&.token, project: nil, ip: 'ip')) .to eq(Gitlab::Auth::Result.new(personal_access_token&.user, nil, personal_access_token.nil? ? nil : :personal_access_token, abilities)) end diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb index 1b17d492b0c..42b4bd71b88 100644 --- a/spec/requests/git_http_spec.rb +++ b/spec/requests/git_http_spec.rb @@ -456,7 +456,7 @@ describe 'Git HTTP requests' do end it "responds with status 403" do - expect(Rack::Attack::Allow2Ban).to receive(:filter).and_return(true) + expect(Rack::Attack::Allow2Ban).to receive(:banned?).and_return(true) expect(Gitlab::AuthLogger).to receive(:error).with({ message: 'Rack_Attack', env: :blocklist, |
