Merge branch...

Merge branch '331805-validate-if-an-actual-git-repo-is-behind-the-url-when-importing-repo-by-url' into 'master'

Validate if an actual GIT repo is behind the URL when importing Repo by URL

See merge request gitlab-org/gitlab!70165

app/assets/javascripts/projects/project_new.js

 import $ from 'jquery';
+import { debounce } from 'lodash';
 import DEFAULT_PROJECT_TEMPLATES from 'ee_else_ce/projects/default_project_templates';
+import axios from '../lib/utils/axios_utils';
 import {
   convertToTitleCase,
   humanize,
@@ -9,6 +11,23 @@ import {
 
 let hasUserDefinedProjectPath = false;
 let hasUserDefinedProjectName = false;
+const invalidInputClass = 'gl-field-error-outline';
+
+const validateImportCredentials = (url, user, password) => {
+  const endpoint = `${gon.relative_url_root}/import/url/validate`;
+  return axios
+    .post(endpoint, {
+      url,
+      user,
+      password,
+    })
+    .then(({ data }) => data)
+    .catch(() => ({
+      // intentionally reporting success in case of validation error
+      // we do not want to block users from trying import in case of validation exception
+      success: true,
+    }));
+};
 
 const onProjectNameChange = ($projectNameInput, $projectPathInput) => {
   const slug = slugify(convertUnicodeToAscii($projectNameInput.val()));
@@ -85,7 +104,10 @@ const bindHowToImport = () => {
 const bindEvents = () => {
   const $newProjectForm = $('#new_project');
   const $projectImportUrl = $('#project_import_url');
-  const $projectImportUrlWarning = $('.js-import-url-warning');
+  const $projectImportUrlUser = $('#project_import_url_user');
+  const $projectImportUrlPassword = $('#project_import_url_password');
+  const $projectImportUrlError = $('.js-import-url-error');
+  const $projectImportForm = $('.project-import form');
   const $projectPath = $('.tab-pane.active #project_path');
   const $useTemplateBtn = $('.template-button > input');
   const $projectFieldsForm = $('.project-fields-form');
@@ -139,12 +161,15 @@ const bindEvents = () => {
     $projectPath.val($projectPath.val().trim());
   });
 
-  function updateUrlPathWarningVisibility() {
-    const url = $projectImportUrl.val();
-    const URL_PATTERN = /(?:git|https?):\/\/.*\/.*\.git$/;
-    const isUrlValid = URL_PATTERN.test(url);
-    $projectImportUrlWarning.toggleClass('hide', isUrlValid);
-  }
+  const updateUrlPathWarningVisibility = debounce(async () => {
+    const { success: isUrlValid } = await validateImportCredentials(
+      $projectImportUrl.val(),
+      $projectImportUrlUser.val(),
+      $projectImportUrlPassword.val(),
+    );
+    $projectImportUrl.toggleClass(invalidInputClass, !isUrlValid);
+    $projectImportUrlError.toggleClass('hide', isUrlValid);
+  }, 500);
 
   let isProjectImportUrlDirty = false;
   $projectImportUrl.on('blur', () => {
@@ -153,11 +178,24 @@ const bindEvents = () => {
   });
   $projectImportUrl.on('keyup', () => {
     deriveProjectPathFromUrl($projectImportUrl);
-    // defer error message till first input blur
+  });
+
+  [$projectImportUrl, $projectImportUrlUser, $projectImportUrlPassword].forEach(($f) => {
+    $f.on('input', () => {
       if (isProjectImportUrlDirty) {
         updateUrlPathWarningVisibility();
       }
     });
+  });
+
+  $projectImportForm.on('submit', (e) => {
+    const $invalidFields = $projectImportForm.find(`.${invalidInputClass}`);
+    if ($invalidFields.length > 0) {
+      $invalidFields[0].focus();
+      e.preventDefault();
+      e.stopPropagation();
+    }
+  });
 
   $('.js-import-git-toggle-button').on('click', () => {
     const $projectMirror = $('#project_mirror');

app/controllers/import/url_controller.rb

+# frozen_string_literal: true
+
+class Import::UrlController < ApplicationController
+  feature_category :importers
+
+  def validate
+    result = Import::ValidateRemoteGitEndpointService.new(validate_params).execute
+    if result.success?
+      render json: { success: true }
+    else
+      render json: { success: false, message: result.message }
+    end
+  end
+
+  private
+
+  def validate_params
+    params.permit(:user, :password, :url)
+  end
+end

app/services/import/validate_remote_git_endpoint_service.rb

+# frozen_string_literal: true
+
+module Import
+  class ValidateRemoteGitEndpointService
+    # Validates if the remote endpoint is a valid GIT repository
+    # Only smart protocol is supported
+    # Validation rules are taken from https://git-scm.com/docs/http-protocol#_smart_clients
+
+    GIT_SERVICE_NAME = "git-upload-pack"
+    GIT_EXPECTED_FIRST_PACKET_LINE = "# service=#{GIT_SERVICE_NAME}"
+    GIT_BODY_MESSAGE_REGEXP = /^[0-9a-f]{4}#{GIT_EXPECTED_FIRST_PACKET_LINE}/.freeze
+    # https://github.com/git/git/blob/master/Documentation/technical/protocol-common.txt#L56-L59
+    GIT_PROTOCOL_PKT_LEN = 4
+    GIT_MINIMUM_RESPONSE_LENGTH = GIT_PROTOCOL_PKT_LEN + GIT_EXPECTED_FIRST_PACKET_LINE.length
+    EXPECTED_CONTENT_TYPE = "application/x-#{GIT_SERVICE_NAME}-advertisement"
+
+    def initialize(params)
+      @params = params
+    end
+
+    def execute
+      uri = Gitlab::Utils.parse_url(@params[:url])
+
+      return error("Invalid URL") unless uri
+
+      uri.fragment = nil
+      url = Gitlab::Utils.append_path(uri.to_s, "/info/refs?service=#{GIT_SERVICE_NAME}")
+
+      response_body = ''
+      result = nil
+      Gitlab::HTTP.try_get(url, stream_body: true, follow_redirects: false, basic_auth: auth) do |fragment|
+        response_body += fragment
+        next if response_body.length < GIT_MINIMUM_RESPONSE_LENGTH
+
+        result = if status_code_is_valid(fragment) && content_type_is_valid(fragment) && response_body_is_valid(response_body)
+                   :success
+                 else
+                   :error
+                 end
+
+        # We are interested only in the first chunks of the response
+        # So we're using stream_body: true and breaking when receive enough body
+        break
+      end
+
+      if result == :success
+        ServiceResponse.success
+      else
+        ServiceResponse.error(message: "#{uri} is not a valid HTTP Git repository")
+      end
+    end
+
+    private
+
+    def auth
+      unless @params[:user].to_s.blank?
+        {
+          username: @params[:user],
+          password: @params[:password]
+        }
+      end
+    end
+
+    def status_code_is_valid(fragment)
+      fragment.http_response.code == '200'
+    end
+
+    def content_type_is_valid(fragment)
+      fragment.http_response['content-type'] == EXPECTED_CONTENT_TYPE
+    end
+
+    def response_body_is_valid(response_body)
+      response_body.match?(GIT_BODY_MESSAGE_REGEXP)
+    end
+  end
+end

app/views/projects/_import_project_pane.html.haml

@@ -83,7 +83,7 @@
 
 
   .js-toggle-content.toggle-import-form{ class: ('hide' if active_tab != 'import') }
-    = form_for @project, html: { class: 'new_project' } do |f|
+    = form_for @project, html: { class: 'new_project gl-show-field-errors' } do |f|
       %hr
       = render "shared/import_form", f: f
       = render 'projects/new_project_fields', f: f, project_name_id: "import-url-name", hide_init_with_readme: true, track_label: track_label

app/views/shared/_import_form.html.haml

@@ -9,17 +9,12 @@
     = f.text_field :import_url, value: import_url.sanitized_url,
         autocomplete: 'off', class: 'form-control gl-form-input', placeholder: 'https://gitlab.company.com/group/project.git', required: true
     = render 'shared/global_alert',
-      variant: :warning,
-      alert_class: 'gl-mt-3 js-import-url-warning hide',
+      variant: :danger,
+      alert_class: 'gl-mt-3 js-import-url-error hide',
       dismissible: false,
       close_button_class: 'js-close-2fa-enabled-success-alert' do
       .gl-alert-body
-        = s_('Import|A repository URL usually ends in a .git suffix, although this is not required. Double check to make sure your repository URL is correct.')
-
-    .gl-alert.gl-alert-not-dismissible.gl-alert-warning.gl-mt-3.hide#project_import_url_warning
-      .gl-alert-container
-        = sprite_icon('warning-solid', css_class: 'gl-icon s16 gl-alert-icon gl-alert-icon-no-title')
-        .gl-alert-content{ role: 'alert' }
+        = s_('Import|There is not a valid Git repository at this URL. If your HTTP repository is not publicly accessible, verify your credentials.')
   .row
     .form-group.col-md-6
       = f.label :import_url_user, class: 'label-bold' do

config/routes/import.rb

@@ -12,6 +12,10 @@ end
 namespace :import do
   resources :available_namespaces, only: [:index], controller: :available_namespaces
 
+  namespace :url do
+    post :validate
+  end
+
   resource :github, only: [:create, :new], controller: :github do
     post :personal_access_token
     get :status

ee/spec/features/projects/new_project_spec.rb

@@ -69,6 +69,8 @@ RSpec.describe 'New project', :js do
       end
 
       it '"Import project" tab creates projects with features enabled' do
+        stub_request(:get, "http://foo.git/info/refs?service=git-upload-pack").to_return(status: 200, body: "001e# servdice=git-upload-pack")
+
         visit new_project_path
         find('[data-qa-panel-name="import_project"]').click # rubocop:disable QA/SelectorUsage
 
@@ -76,6 +78,9 @@ RSpec.describe 'New project', :js do
           first('.js-import-git-toggle-button').click
 
           fill_in 'project_import_url', with: 'http://foo.git'
+
+          wait_for_requests
+
           fill_in 'project_name', with: 'import-project-with-features1'
           fill_in 'project_path', with: 'import-project-with-features1'
           choose 'project_visibility_level_20'

locale/gitlab.pot

@@ -17327,7 +17327,7 @@ msgstr[1] ""
 msgid "Importing..."
 msgstr ""
 
-msgid "Import|A repository URL usually ends in a .git suffix, although this is not required. Double check to make sure your repository URL is correct."
+msgid "Import|There is not a valid Git repository at this URL. If your HTTP repository is not publicly accessible, verify your credentials."
 msgstr ""
 
 msgid "Improve customer support with Service Desk"

spec/features/projects/new_project_spec.rb

@@ -296,12 +296,16 @@ RSpec.describe 'New project', :js do
           expect(git_import_instructions).to have_content 'Git repository URL'
         end
 
-        it 'reports error if repo URL does not end with .git' do
+        it 'reports error if repo URL is not a valid Git repository' do
+          stub_request(:get, "http://foo/bar/info/refs?service=git-upload-pack").to_return(status: 200, body: "not-a-git-repo")
+
           fill_in 'project_import_url', with: 'http://foo/bar'
           # simulate blur event
           find('body').click
 
-          expect(page).to have_text('A repository URL usually ends in a .git suffix')
+          wait_for_requests
+
+          expect(page).to have_text('There is not a valid Git repository at this URL')
         end
 
         it 'keeps "Import project" tab open after form validation error' do

spec/requests/import/url_controller_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Import::UrlController do
+  let_it_be(:user) { create(:user) }
+
+  before do
+    login_as(user)
+  end
+
+  describe 'POST #validate' do
+    it 'reports success when service reports success status' do
+      allow_next_instance_of(Import::ValidateRemoteGitEndpointService) do |validate_endpoint_service|
+        allow(validate_endpoint_service).to receive(:execute).and_return(ServiceResponse.success)
+      end
+
+      post import_url_validate_path, params: { url: 'https://fake.repo' }
+
+      expect(json_response).to eq({ 'success' => true })
+    end
+
+    it 'exposes error message when service reports error' do
+      expect_next_instance_of(Import::ValidateRemoteGitEndpointService) do |validate_endpoint_service|
+        expect(validate_endpoint_service).to receive(:execute).and_return(ServiceResponse.error(message: 'foobar'))
+      end
+
+      post import_url_validate_path, params: { url: 'https://fake.repo' }
+
+      expect(json_response).to eq({ 'success' => false, 'message' => 'foobar' })
+    end
+
+    context 'with an anonymous user' do
+      before do
+        sign_out(user)
+      end
+
+      it 'redirects to sign-in page' do
+        post import_url_validate_path
+
+        expect(response).to redirect_to(new_user_session_path)
+      end
+    end
+  end
+end

spec/services/import/validate_remote_git_endpoint_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Import::ValidateRemoteGitEndpointService do
+  include StubRequests
+
+  let_it_be(:base_url) { 'http://demo.host/path' }
+  let_it_be(:endpoint_url) { "#{base_url}/info/refs?service=git-upload-pack" }
+  let_it_be(:error_message) { "#{base_url} is not a valid HTTP Git repository" }
+
+  describe '#execute' do
+    let(:valid_response) do
+      { status: 200,
+        body: '001e# service=git-upload-pack',
+        headers: { 'Content-Type': 'application/x-git-upload-pack-advertisement' } }
+    end
+
+    it 'correctly handles URLs with fragment' do
+      allow(Gitlab::HTTP).to receive(:get)
+
+      described_class.new(url: "#{base_url}#somehash").execute
+
+      expect(Gitlab::HTTP).to have_received(:get).with(endpoint_url, basic_auth: nil, stream_body: true, follow_redirects: false)
+    end
+
+    context 'when receiving HTTP response' do
+      subject { described_class.new(url: base_url) }
+
+      it 'returns success when HTTP response is valid and contains correct payload' do
+        stub_full_request(endpoint_url, method: :get).to_return(valid_response)
+
+        result = subject.execute
+
+        expect(result).to be_a(ServiceResponse)
+        expect(result.success?).to be(true)
+      end
+
+      it 'reports error when status code is not 200' do
+        stub_full_request(endpoint_url, method: :get).to_return(valid_response.merge({ status: 301 }))
+
+        result = subject.execute
+
+        expect(result).to be_a(ServiceResponse)
+        expect(result.error?).to be(true)
+        expect(result.message).to eq(error_message)
+      end
+
+      it 'reports error when required header is missing' do
+        stub_full_request(endpoint_url, method: :get).to_return(valid_response.merge({ headers: nil }))
+
+        result = subject.execute
+
+        expect(result).to be_a(ServiceResponse)
+        expect(result.error?).to be(true)
+        expect(result.message).to eq(error_message)
+      end
+
+      it 'reports error when body is in invalid format' do
+        stub_full_request(endpoint_url, method: :get).to_return(valid_response.merge({ body: 'invalid content' }))
+
+        result = subject.execute
+
+        expect(result).to be_a(ServiceResponse)
+        expect(result.error?).to be(true)
+        expect(result.message).to eq(error_message)
+      end
+
+      it 'reports error when exception is raised' do
+        stub_full_request(endpoint_url, method: :get).to_raise(SocketError.new('dummy message'))
+
+        result = subject.execute
+
+        expect(result).to be_a(ServiceResponse)
+        expect(result.error?).to be(true)
+        expect(result.message).to eq(error_message)
+      end
+    end
+
+    it 'passes basic auth when credentials are provided' do
+      allow(Gitlab::HTTP).to receive(:get)
+
+      described_class.new(url: "#{base_url}#somehash", user: 'user', password: 'password').execute
+
+      expect(Gitlab::HTTP).to have_received(:get).with(endpoint_url, basic_auth: { username: 'user', password: 'password' }, stream_body: true, follow_redirects: false)
+    end
+  end
+end