summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2019-12-03 21:06:23 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2019-12-03 21:06:23 +0000
commit4529c19950e412f0461910585414f8633d3b1b18 (patch)
tree00b75c579ef52b41fea09c516cd5286dee5df703
parentab7cf450ba19cf80b9534f25dc707b33845e3014 (diff)
downloadgitlab-ce-4529c19950e412f0461910585414f8633d3b1b18.tar.gz
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--app/controllers/application_controller.rb12
-rw-r--r--changelogs/unreleased/35617-only-blacklist-git-auth.yml5
-rw-r--r--config/initializers/rack_attack_git_basic_auth.rb14
-rw-r--r--config/routes/project.rb30
-rw-r--r--doc/development/packages.md23
-rw-r--r--doc/user/project/merge_requests/creating_merge_requests.md2
-rw-r--r--doc/user/project/repository/img/web_editor_new_branch_from_issue.pngbin2715 -> 0 bytes
-rw-r--r--doc/user/project/repository/img/web_editor_new_branch_from_issue_create_button_v12_6.pngbin0 -> 70114 bytes
-rw-r--r--doc/user/project/repository/img/web_editor_new_branch_from_issue_v_12_6.pngbin0 -> 76938 bytes
-rw-r--r--doc/user/project/repository/web_editor.md34
-rw-r--r--lib/gitlab/auth.rb24
-rw-r--r--lib/gitlab/auth/ip_rate_limiter.rb29
-rw-r--r--spec/controllers/application_controller_spec.rb26
-rw-r--r--spec/lib/gitlab/auth/ip_rate_limiter_spec.rb32
-rw-r--r--spec/lib/gitlab/auth_spec.rb104
-rw-r--r--spec/requests/git_http_spec.rb2
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
deleted file mode 100644
index 4e156b8adc8..00000000000
--- a/doc/user/project/repository/img/web_editor_new_branch_from_issue.png
+++ /dev/null
Binary files differ
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
new 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
Binary files differ
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
new file mode 100644
index 00000000000..d5a92546d40
--- /dev/null
+++ b/doc/user/project/repository/img/web_editor_new_branch_from_issue_v_12_6.png
Binary files differ
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.
-![New Branch Button](img/web_editor_new_branch_from_issue.png)
+![Create Button](img/web_editor_new_branch_from_issue_create_button_v12_6.png)
-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**.
+
+![New Branch Button](img/web_editor_new_branch_from_issue_v_12_6.png)
+
+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,