Add latest changes from gitlab-org/gitlab@master

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

changelogs/unreleased/35617-only-blacklist-git-auth.yml

+---
+title: Only blacklist IPs from Git requests
+merge_request: 20828
+author:
+type: changed

config/initializers/rack_attack_git_basic_auth.rb

-# 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

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

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

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

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

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

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|

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

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

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,