Merge branch...

Merge branch '270584-createrepositoryfromurl-clone-cmd-wait-fatal-could-not-read-username-for-https-github-com' into 'master'

Validate import url viability in projects api before creation

See merge request gitlab-org/gitlab!72323

ee/lib/ee/api/projects.rb

@@ -82,6 +82,7 @@ module EE
             super
 
             verify_mirror_attrs!(project, attrs)
+            validate_git_import_url!(attrs[:import_url])
             verify_issuable_default_templates_attrs!(project, attrs)
             verify_merge_pipelines_attrs!(project, attrs)
           end

ee/spec/requests/api/projects_spec.rb

@@ -4,6 +4,7 @@ require 'spec_helper'
 
 RSpec.describe API::Projects do
   include ExternalAuthorizationServiceHelpers
+  include StubRequests
 
   let(:user) { create(:user) }
   let_it_be(:another_user) { create(:user) }
@@ -627,6 +628,15 @@ RSpec.describe API::Projects do
         }
       end
 
+      before do
+        git_response = {
+          status: 200,
+          body: '001e# service=git-upload-pack',
+          headers: { 'Content-Type': 'application/x-git-upload-pack-advertisement' }
+        }
+        stub_full_request("#{import_url}/info/refs?service=git-upload-pack", method: :get).to_return(git_response)
+      end
+
       it 'creates new project with pull mirroring set up' do
         post api('/projects', user), params: mirror_params
 
@@ -1097,6 +1107,19 @@ RSpec.describe API::Projects do
         }
       end
 
+      let(:git_response) do
+        {
+          status: 200,
+          body: '001e# service=git-upload-pack',
+          headers: { 'Content-Type': 'application/x-git-upload-pack-advertisement' }
+        }
+      end
+
+      before do
+        endpoint_url = "#{import_url}/info/refs?service=git-upload-pack"
+        stub_full_request(endpoint_url, method: :get).to_return(git_response)
+      end
+
       context 'when pull mirroring is not available' do
         before do
           stub_ee_application_setting(mirror_available: false)
@@ -1132,6 +1155,23 @@ RSpec.describe API::Projects do
         end
       end
 
+      context 'when import_url is not a valid git endpoint' do
+        let(:git_response) do
+          {
+            status: 301,
+            body: '',
+            headers: nil
+          }
+        end
+
+        it 'disallows creating a project with an import_url that is not reachable', :aggregate_failures do
+          subject
+
+          expect(response).to have_gitlab_http_status(:unprocessable_entity)
+          expect(json_response['message']).to eq("#{import_url} is not a valid HTTP Git repository")
+        end
+      end
+
       it 'updates mirror related attributes' do
         expect_any_instance_of(EE::ProjectImportState).to receive(:force_import_job!).once
 

lib/api/helpers/projects_helpers.rb

@@ -177,6 +177,17 @@ module API
 
       def filter_attributes_using_license!(attrs)
       end
+
+      def validate_git_import_url!(import_url, import_enabled: true)
+        return if import_url.blank?
+        return unless import_enabled
+
+        result = Import::ValidateRemoteGitEndpointService.new(url: import_url).execute # network call
+
+        if result.error?
+          render_api_error!(result.message, 422)
+        end
+      end
     end
   end
 end

lib/api/projects.rb

@@ -91,7 +91,7 @@ module API
       end
 
       def check_import_by_url_is_enabled
-        forbidden! unless Gitlab::CurrentSettings.import_sources&.include?('git')
+        Gitlab::CurrentSettings.import_sources&.include?('git') || forbidden!
       end
     end
 
@@ -269,7 +269,9 @@ 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?
+
+        validate_git_import_url!(params[:import_url], import_enabled: check_import_by_url_is_enabled)
+
         project = ::Projects::CreateService.new(current_user, attrs).execute
 
         if project.saved?
@@ -307,6 +309,8 @@ module API
         attrs = declared_params(include_missing: false)
         attrs = translate_params_for_compatibility(attrs)
         filter_attributes_using_license!(attrs)
+        validate_git_import_url!(params[:import_url])
+
         project = ::Projects::CreateService.new(user, attrs).execute
 
         if project.saved?

spec/requests/api/projects_spec.rb

@@ -48,6 +48,7 @@ end
 
 RSpec.describe API::Projects do
   include ProjectForksHelper
+  include StubRequests
 
   let_it_be(:user) { create(:user) }
   let_it_be(:user2) { create(:user) }
@@ -1159,6 +1160,34 @@ RSpec.describe API::Projects do
       expect(response).to have_gitlab_http_status(:forbidden)
     end
 
+    it 'disallows creating a project with an import_url that is not reachable', :aggregate_failures do
+      url = 'http://example.com'
+      endpoint_url = "#{url}/info/refs?service=git-upload-pack"
+      stub_full_request(endpoint_url, method: :get).to_return({ status: 301, body: '', headers: nil })
+      project_params = { import_url: url, 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(:unprocessable_entity)
+      expect(json_response['message']).to eq("#{url} is not a valid HTTP Git repository")
+    end
+
+    it 'creates a project with an import_url that is valid', :aggregate_failures do
+      url = 'http://example.com'
+      endpoint_url = "#{url}/info/refs?service=git-upload-pack"
+      git_response = {
+        status: 200,
+        body: '001e# service=git-upload-pack',
+        headers: { 'Content-Type': 'application/x-git-upload-pack-advertisement' }
+      }
+      stub_full_request(endpoint_url, method: :get).to_return(git_response)
+      project_params = { import_url: url, path: 'path-project-Foo', name: 'Foo Project' }
+
+      expect { post api('/projects', user), params: project_params }.to change { Project.count }.by(1)
+
+      expect(response).to have_gitlab_http_status(:created)
+    end
+
     it 'sets a project as public' do
       project = attributes_for(:project, visibility: 'public')