Add latest changes from gitlab-org/security/gitlab@14-2-stable-ee

16 files changed, 218 insertions, 39 deletions

diff --git a/app/controllers/projects/project_members_controller.rb b/app/controllers/projects/project_members_controller.rb
index d0987492d2d..b979276437c 100644
--- a/app/controllers/projects/project_members_controller.rb
+++ b/app/controllers/projects/project_members_controller.rb
@@ -34,13 +34,13 @@ class Projects::ProjectMembersController < Projects::ApplicationController
   end
 
   def import
-    @projects = current_user.authorized_projects.order_id_desc
+    @projects = Project.visible_to_user_and_access_level(current_user, Gitlab::Access::MAINTAINER).order_id_desc
   end
 
   def apply_import
     source_project = Project.find(params[:source_project_id])
 
-    if can?(current_user, :read_project_member, source_project)
+    if can?(current_user, :admin_project_member, source_project)
       status = @project.team.import(source_project, current_user)
       notice = status ? "Successfully imported" : "Import failed"
     else
diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb
index bdb645e1934..b0e690a74cd 100644
--- a/app/controllers/projects_controller.rb
+++ b/app/controllers/projects_controller.rb
@@ -19,6 +19,7 @@ class ProjectsController < Projects::ApplicationController
   before_action :redirect_git_extension, only: [:show]
   before_action :project, except: [:index, :new, :create, :resolve]
   before_action :repository, except: [:index, :new, :create, :resolve]
+  before_action :verify_git_import_enabled, only: [:create]
   before_action :project_export_enabled, only: [:export, :download_export, :remove_export, :generate_new_export]
   before_action :present_project, only: [:edit]
   before_action :authorize_download_code!, only: [:refs]
@@ -494,6 +495,10 @@ class ProjectsController < Projects::ApplicationController
     url_for(safe_params)
   end
 
+  def verify_git_import_enabled
+    render_404 if project_params[:import_url] && !git_import_enabled?
+  end
+
   def project_export_enabled
     render_404 unless Gitlab::CurrentSettings.project_export_enabled?
   end
diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb
index 4077a3d3dac..d040ac7f76c 100644
--- a/app/controllers/uploads_controller.rb
+++ b/app/controllers/uploads_controller.rb
@@ -36,14 +36,10 @@ class UploadsController < ApplicationController
   end
 
   def find_model
-    return unless params[:id]
-
     upload_model_class.find(params[:id])
   end
 
   def authorize_access!
-    return unless model
-
     authorized =
       case model
       when Note
@@ -68,8 +64,6 @@ class UploadsController < ApplicationController
   end
 
   def authorize_create_access!
-    return unless model
-
     authorized =
       case model
       when User
diff --git a/lib/api/projects.rb b/lib/api/projects.rb
index 28bcb382ecf..3ffedd46c31 100644
--- a/lib/api/projects.rb
+++ b/lib/api/projects.rb
@@ -89,6 +89,10 @@ module API
           Gitlab::AppLogger.info({ message: "File exceeds maximum size", file_bytes: file.size, project_id: user_project.id, project_path: user_project.full_path, upload_allowed: allowed })
         end
       end
+
+      def check_import_by_url_is_enabled
+        forbidden! unless Gitlab::CurrentSettings.import_sources&.include?('git')
+      end
     end
 
     helpers do
@@ -261,6 +265,7 @@ module API
         attrs = declared_params(include_missing: false)
         attrs = translate_params_for_compatibility(attrs)
         filter_attributes_using_license!(attrs)
+        check_import_by_url_is_enabled if params[:import_url].present?
         project = ::Projects::CreateService.new(current_user, attrs).execute
 
         if project.saved?
diff --git a/lib/gitlab/auth/auth_finders.rb b/lib/gitlab/auth/auth_finders.rb
index a7312ac759a..9c33a5fc872 100644
--- a/lib/gitlab/auth/auth_finders.rb
+++ b/lib/gitlab/auth/auth_finders.rb
@@ -338,6 +338,14 @@ module Gitlab
         Gitlab::PathRegex.repository_git_route_regex.match?(current_request.path)
       end
 
+      def git_lfs_request?
+        Gitlab::PathRegex.repository_git_lfs_route_regex.match?(current_request.path)
+      end
+
+      def git_or_lfs_request?
+        git_request? || git_lfs_request?
+      end
+
       def archive_request?
         current_request.path.include?('/-/archive/')
       end
diff --git a/lib/gitlab/auth/request_authenticator.rb b/lib/gitlab/auth/request_authenticator.rb
index dfc682e8a5c..08214bbd449 100644
--- a/lib/gitlab/auth/request_authenticator.rb
+++ b/lib/gitlab/auth/request_authenticator.rb
@@ -35,13 +35,31 @@ module Gitlab
           find_user_from_static_object_token(request_format) ||
           find_user_from_basic_auth_job ||
           find_user_from_job_token ||
-          find_user_from_lfs_token ||
-          find_user_from_personal_access_token ||
-          find_user_from_basic_auth_password
+          find_user_from_personal_access_token_for_api_or_git ||
+          find_user_for_git_or_lfs_request
       rescue Gitlab::Auth::AuthenticationError
         nil
       end
 
+      # To prevent Rack Attack from incorrectly rate limiting
+      # authenticated Git activity, we need to authenticate the user
+      # from other means (e.g. HTTP Basic Authentication) only if the
+      # request originated from a Git or Git LFS
+      # request. Repositories::GitHttpClientController or
+      # Repositories::LfsApiController normally does the authentication,
+      # but Rack Attack runs before those controllers.
+      def find_user_for_git_or_lfs_request
+        return unless git_or_lfs_request?
+
+        find_user_from_lfs_token || find_user_from_basic_auth_password
+      end
+
+      def find_user_from_personal_access_token_for_api_or_git
+        return unless api_request? || git_or_lfs_request?
+
+        find_user_from_personal_access_token
+      end
+
       def valid_access_token?(scopes: [])
         validate_access_token!(scopes: scopes)
 
diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb
index 4cb47ffc6d9..c648f4d1fd0 100644
--- a/lib/gitlab/path_regex.rb
+++ b/lib/gitlab/path_regex.rb
@@ -184,6 +184,10 @@ module Gitlab
       @repository_git_route_regex ||= /#{repository_route_regex}\.git/.freeze
     end
 
+    def repository_git_lfs_route_regex
+      @repository_git_lfs_route_regex ||= %r{#{repository_git_route_regex}\/(info\/lfs|gitlab-lfs)\/}.freeze
+    end
+
     def repository_wiki_git_route_regex
       @repository_wiki_git_route_regex ||= /#{full_namespace_route_regex}\.*\.wiki\.git/.freeze
     end
diff --git a/spec/controllers/projects/project_members_controller_spec.rb b/spec/controllers/projects/project_members_controller_spec.rb
index be5c1f0d428..c352524ec14 100644
--- a/spec/controllers/projects/project_members_controller_spec.rb
+++ b/spec/controllers/projects/project_members_controller_spec.rb
@@ -624,9 +624,9 @@ RSpec.describe Projects::ProjectMembersController do
       end
     end
 
-    context 'when user can access source project members' do
+    context 'when user can admin source project members' do
       before do
-        another_project.add_guest(user)
+        another_project.add_maintainer(user)
       end
 
       include_context 'import applied'
@@ -640,7 +640,11 @@ RSpec.describe Projects::ProjectMembersController do
       end
     end
 
-    context 'when user is not member of a source project' do
+    context "when user can't admin source project members" do
+      before do
+        another_project.add_developer(user)
+      end
+
       include_context 'import applied'
 
       it 'does not import team members' do
diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 8afb80d9cc5..9d070061850 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -408,6 +408,47 @@ RSpec.describe ProjectsController do
     end
   end
 
+  describe 'POST create' do
+    let!(:params) do
+      {
+        path: 'foo',
+        description: 'bar',
+        import_url: project.http_url_to_repo,
+        namespace_id: user.namespace.id
+      }
+    end
+
+    subject { post :create, params: { project: params } }
+
+    before do
+      sign_in(user)
+    end
+
+    context 'when import by url is disabled' do
+      before do
+        stub_application_setting(import_sources: [])
+      end
+
+      it 'does not create project and reports an error' do
+        expect { subject }.not_to change { Project.count }
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    context 'when import by url is enabled' do
+      before do
+        stub_application_setting(import_sources: ['git'])
+      end
+
+      it 'creates project' do
+        expect { subject }.to change { Project.count }
+
+        expect(response).to have_gitlab_http_status(:redirect)
+      end
+    end
+  end
+
   describe 'GET edit' do
     it 'allows an admin user to access the page', :enable_admin_mode do
       sign_in(create(:user, :admin))
diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb
index 043fd97f1ad..2aa9b86b20e 100644
--- a/spec/controllers/uploads_controller_spec.rb
+++ b/spec/controllers/uploads_controller_spec.rb
@@ -666,6 +666,6 @@ RSpec.describe UploadsController do
   def post_authorize(verified: true)
     request.headers.merge!(workhorse_internal_api_request_header) if verified
 
-    post :authorize, params: { model: 'personal_snippet', id: model.id }, format: :json
+    post :authorize, params: params, format: :json
   end
 end
diff --git a/spec/lib/gitlab/auth/request_authenticator_spec.rb b/spec/lib/gitlab/auth/request_authenticator_spec.rb
index 28e93a8da52..2543eb3a5e9 100644
--- a/spec/lib/gitlab/auth/request_authenticator_spec.rb
+++ b/spec/lib/gitlab/auth/request_authenticator_spec.rb
@@ -81,32 +81,72 @@ RSpec.describe Gitlab::Auth::RequestAuthenticator do
       expect(subject.find_sessionless_user(:api)).to eq job_token_user
     end
 
-    it 'returns lfs_token user if no job_token user found' do
-      allow_any_instance_of(described_class)
-        .to receive(:find_user_from_lfs_token)
-        .and_return(lfs_token_user)
-
-      expect(subject.find_sessionless_user(:api)).to eq lfs_token_user
-    end
-
-    it 'returns basic_auth_access_token user if no lfs_token user found' do
+    it 'returns nil even if basic_auth_access_token is available' do
       allow_any_instance_of(described_class)
         .to receive(:find_user_from_personal_access_token)
         .and_return(basic_auth_access_token_user)
 
-      expect(subject.find_sessionless_user(:api)).to eq basic_auth_access_token_user
+      expect(subject.find_sessionless_user(:api)).to be_nil
     end
 
-    it 'returns basic_auth_access_password user if no basic_auth_access_token user found' do
+    it 'returns nil even if find_user_from_lfs_token is available' do
       allow_any_instance_of(described_class)
-        .to receive(:find_user_from_basic_auth_password)
-        .and_return(basic_auth_password_user)
+        .to receive(:find_user_from_lfs_token)
+        .and_return(lfs_token_user)
 
-      expect(subject.find_sessionless_user(:api)).to eq basic_auth_password_user
+      expect(subject.find_sessionless_user(:api)).to be_nil
     end
 
     it 'returns nil if no user found' do
-      expect(subject.find_sessionless_user(:api)).to be_blank
+      expect(subject.find_sessionless_user(:api)).to be_nil
+    end
+
+    context 'in an API request' do
+      before do
+        env['SCRIPT_NAME'] = '/api/v4/projects'
+      end
+
+      it 'returns basic_auth_access_token user if no job_token_user found' do
+        allow_any_instance_of(described_class)
+          .to receive(:find_user_from_personal_access_token)
+          .and_return(basic_auth_access_token_user)
+
+        expect(subject.find_sessionless_user(:api)).to eq basic_auth_access_token_user
+      end
+    end
+
+    context 'in a Git request' do
+      before do
+        env['SCRIPT_NAME'] = '/group/project.git/info/refs'
+      end
+
+      it 'returns lfs_token user if no job_token user found' do
+        allow_any_instance_of(described_class)
+          .to receive(:find_user_from_lfs_token)
+          .and_return(lfs_token_user)
+
+        expect(subject.find_sessionless_user(nil)).to eq lfs_token_user
+      end
+
+      it 'returns basic_auth_access_token user if no lfs_token user found' do
+        allow_any_instance_of(described_class)
+          .to receive(:find_user_from_personal_access_token)
+          .and_return(basic_auth_access_token_user)
+
+        expect(subject.find_sessionless_user(nil)).to eq basic_auth_access_token_user
+      end
+
+      it 'returns basic_auth_access_password user if no basic_auth_access_token user found' do
+        allow_any_instance_of(described_class)
+          .to receive(:find_user_from_basic_auth_password)
+          .and_return(basic_auth_password_user)
+
+        expect(subject.find_sessionless_user(nil)).to eq basic_auth_password_user
+      end
+
+      it 'returns nil if no user found' do
+        expect(subject.find_sessionless_user(nil)).to be_blank
+      end
     end
 
     it 'rescue Gitlab::Auth::AuthenticationError exceptions' do
diff --git a/spec/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb
index d343634fb92..aa13660deb4 100644
--- a/spec/lib/gitlab/path_regex_spec.rb
+++ b/spec/lib/gitlab/path_regex_spec.rb
@@ -468,6 +468,7 @@ RSpec.describe Gitlab::PathRegex do
     end
 
     let_it_be(:git_paths) { container_paths.map { |path| path + '.git' } }
+    let_it_be(:git_lfs_paths) { git_paths.flat_map { |path| [path + '/info/lfs/', path + '/gitlab-lfs/'] } }
     let_it_be(:snippet_paths) { container_paths.grep(%r{snippets/\d}) }
     let_it_be(:wiki_git_paths) { (container_paths - snippet_paths).map { |path| path + '.wiki.git' } }
     let_it_be(:invalid_git_paths) { invalid_paths.map { |path| path + '.git' } }
@@ -498,6 +499,15 @@ RSpec.describe Gitlab::PathRegex do
       end
     end
 
+    describe '.repository_git_lfs_route_regex' do
+      subject { %r{\A#{described_class.repository_git_lfs_route_regex}\z} }
+
+      it 'matches the expected paths' do
+        expect_route_match(git_lfs_paths)
+        expect_no_route_match(container_paths + invalid_paths + git_paths + invalid_git_paths)
+      end
+    end
+
     describe '.repository_wiki_git_route_regex' do
       subject { %r{\A#{described_class.repository_wiki_git_route_regex}\z} }
 
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 3622eedfed5..6e0549d519f 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -1149,6 +1149,16 @@ RSpec.describe API::Projects do
       expect(response).to have_gitlab_http_status(:bad_request)
     end
 
+    it 'disallows creating a project with an import_url when git import source is disabled' do
+      stub_application_setting(import_sources: nil)
+
+      project_params = { import_url: 'http://example.com', path: 'path-project-Foo', name: 'Foo Project' }
+      expect { post api('/projects', user), params: project_params }
+        .not_to change {  Project.count }
+
+      expect(response).to have_gitlab_http_status(:forbidden)
+    end
+
     it 'sets a project as public' do
       project = attributes_for(:project, visibility: 'public')
 
diff --git a/spec/requests/rack_attack_global_spec.rb b/spec/requests/rack_attack_global_spec.rb
index a0f9d4c11ed..48a21e55b7e 100644
--- a/spec/requests/rack_attack_global_spec.rb
+++ b/spec/requests/rack_attack_global_spec.rb
@@ -763,17 +763,28 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac
     end
 
     context 'authenticated with lfs token' do
-      it 'request is authenticated by token in basic auth' do
-        lfs_token = Gitlab::LfsToken.new(user)
-        encoded_login = ["#{user.username}:#{lfs_token.token}"].pack('m0')
+      let(:lfs_url) { '/namespace/repo.git/info/lfs/objects/batch' }
+      let(:lfs_token) { Gitlab::LfsToken.new(user) }
+      let(:encoded_login) { ["#{user.username}:#{lfs_token.token}"].pack('m0') }
+      let(:headers) { { 'AUTHORIZATION' => "Basic #{encoded_login}" } }
 
+      it 'request is authenticated by token in basic auth' do
         expect_authenticated_request
 
-        get url, headers: { 'AUTHORIZATION' => "Basic #{encoded_login}" }
+        get lfs_url, headers: headers
+      end
+
+      it 'request is not authenticated with API URL' do
+        expect_unauthenticated_request
+
+        get url, headers: headers
       end
     end
 
     context 'authenticated with regular login' do
+      let(:encoded_login) { ["#{user.username}:#{user.password}"].pack('m0') }
+      let(:headers) { { 'AUTHORIZATION' => "Basic #{encoded_login}" } }
+
       it 'request is authenticated after login' do
         login_as(user)
 
@@ -782,12 +793,30 @@ RSpec.describe 'Rack Attack global throttles', :use_clean_rails_memory_store_cac
         get url
       end
 
-      it 'request is authenticated by credentials in basic auth' do
-        encoded_login = ["#{user.username}:#{user.password}"].pack('m0')
+      it 'request is not authenticated by credentials in basic auth' do
+        expect_unauthenticated_request
 
-        expect_authenticated_request
+        get url, headers: headers
+      end
+
+      context 'with POST git-upload-pack' do
+        it 'request is authenticated by credentials in basic auth' do
+          expect(::Gitlab::Workhorse).to receive(:verify_api_request!)
+
+          expect_authenticated_request
 
-        get url, headers: { 'AUTHORIZATION' => "Basic #{encoded_login}" }
+          post '/namespace/repo.git/git-upload-pack', headers: headers
+        end
+      end
+
+      context 'with GET info/refs' do
+        it 'request is authenticated by credentials in basic auth' do
+          expect(::Gitlab::Workhorse).to receive(:verify_api_request!)
+
+          expect_authenticated_request
+
+          get '/namespace/repo.git/info/refs?service=git-upload-pack', headers: headers
+        end
       end
     end
   end
diff --git a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
index a20c1d78912..62b35923bcd 100644
--- a/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
+++ b/spec/support/shared_examples/controllers/uploads_actions_shared_examples.rb
@@ -323,6 +323,16 @@ RSpec.shared_examples 'handle uploads authorize' do
       end
     end
 
+    context 'when id is not passed as a param' do
+      let(:params) { super().without(:id) }
+
+      it 'returns 404 status' do
+        post_authorize
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
     context 'when a user can upload a file' do
       before do
         sign_in(user)
diff --git a/workhorse/internal/artifacts/entry.go b/workhorse/internal/artifacts/entry.go
index 91a0843f1ee..d5b3dfb672c 100644
--- a/workhorse/internal/artifacts/entry.go
+++ b/workhorse/internal/artifacts/entry.go
@@ -14,6 +14,7 @@ import (
 	"syscall"
 
 	"gitlab.com/gitlab-org/labkit/log"
+	"gitlab.com/gitlab-org/labkit/mask"
 
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/helper"
 	"gitlab.com/gitlab-org/gitlab/workhorse/internal/senddata"
@@ -35,7 +36,7 @@ func (e *entry) Inject(w http.ResponseWriter, r *http.Request, sendData string)
 
 	log.WithContextFields(r.Context(), log.Fields{
 		"entry":   params.Entry,
-		"archive": params.Archive,
+		"archive": mask.URL(params.Archive),
 		"path":    r.URL.Path,
 	}).Print("SendEntry: sending")