Merge branch...

Merge branch '345705-ensure-proper-validation-for-git-url-is-performed-for-all-requests' into 'master'

Ensure proper validation for GIT url is performed for all requests

See merge request gitlab-org/gitlab!78495

app/assets/javascripts/pages/projects/imports/new/index.js

+import initProjectNew from '~/projects/project_new';
+
+initProjectNew.bindEvents();

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 { DEFAULT_DEBOUNCE_AND_THROTTLE_MS } from '../lib/utils/constants';
 import axios from '../lib/utils/axios_utils';
 import {
   convertToTitleCase,
@@ -13,20 +14,26 @@ let hasUserDefinedProjectPath = false;
 let hasUserDefinedProjectName = false;
 const invalidInputClass = 'gl-field-error-outline';
 
+const cancelSource = axios.CancelToken.source();
+const endpoint = `${gon.relative_url_root}/import/url/validate`;
+let importCredentialsValidationPromise = null;
 const validateImportCredentials = (url, user, password) => {
-  const endpoint = `${gon.relative_url_root}/import/url/validate`;
-  return axios
-    .post(endpoint, {
-      url,
-      user,
-      password,
-    })
+  cancelSource.cancel();
+  importCredentialsValidationPromise = axios
+    .post(endpoint, { url, user, password }, { cancelToken: cancelSource.cancel() })
     .then(({ data }) => data)
-    .catch(() => ({
+    .catch((thrown) =>
+      axios.isCancel(thrown)
+        ? {
+            cancelled: true,
+          }
+        : {
             // 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,
-    }));
+          },
+    );
+  return importCredentialsValidationPromise;
 };
 
 const onProjectNameChange = ($projectNameInput, $projectPathInput) => {
@@ -72,7 +79,7 @@ const deriveProjectPathFromUrl = ($projectImportUrl) => {
     .parents('.toggle-import-form')
     .find('#project_path');
 
-  if (hasUserDefinedProjectPath) {
+  if (hasUserDefinedProjectPath || $currentProjectPath.length === 0) {
     return;
   }
 
@@ -114,7 +121,7 @@ const bindEvents = () => {
   const $projectImportUrlUser = $('#project_import_url_user');
   const $projectImportUrlPassword = $('#project_import_url_password');
   const $projectImportUrlError = $('.js-import-url-error');
-  const $projectImportForm = $('.project-import form');
+  const $projectImportForm = $('form.js-project-import');
   const $projectPath = $('.tab-pane.active #project_path');
   const $useTemplateBtn = $('.template-button > input');
   const $projectFieldsForm = $('.project-fields-form');
@@ -124,7 +131,7 @@ const bindEvents = () => {
   const $projectTemplateButtons = $('.project-templates-buttons');
   const $projectName = $('.tab-pane.active #project_name');
 
-  if ($newProjectForm.length !== 1) {
+  if ($newProjectForm.length !== 1 && $projectImportForm.length !== 1) {
     return;
   }
 
@@ -168,20 +175,28 @@ const bindEvents = () => {
     $projectPath.val($projectPath.val().trim());
   });
 
-  const updateUrlPathWarningVisibility = debounce(async () => {
-    const { success: isUrlValid } = await validateImportCredentials(
+  const updateUrlPathWarningVisibility = async () => {
+    const { success: isUrlValid, cancelled } = await validateImportCredentials(
       $projectImportUrl.val(),
       $projectImportUrlUser.val(),
       $projectImportUrlPassword.val(),
     );
+    if (cancelled) {
+      return;
+    }
+
     $projectImportUrl.toggleClass(invalidInputClass, !isUrlValid);
     $projectImportUrlError.toggleClass('hide', isUrlValid);
-  }, 500);
+  };
+  const debouncedUpdateUrlPathWarningVisibility = debounce(
+    updateUrlPathWarningVisibility,
+    DEFAULT_DEBOUNCE_AND_THROTTLE_MS,
+  );
 
   let isProjectImportUrlDirty = false;
   $projectImportUrl.on('blur', () => {
     isProjectImportUrlDirty = true;
-    updateUrlPathWarningVisibility();
+    debouncedUpdateUrlPathWarningVisibility();
   });
   $projectImportUrl.on('keyup', () => {
     deriveProjectPathFromUrl($projectImportUrl);
@@ -190,17 +205,33 @@ const bindEvents = () => {
   [$projectImportUrl, $projectImportUrlUser, $projectImportUrlPassword].forEach(($f) => {
     $f.on('input', () => {
       if (isProjectImportUrlDirty) {
-        updateUrlPathWarningVisibility();
+        debouncedUpdateUrlPathWarningVisibility();
       }
     });
   });
 
-  $projectImportForm.on('submit', (e) => {
+  $projectImportForm.on('submit', async (e) => {
+    e.preventDefault();
+
+    if (importCredentialsValidationPromise === null) {
+      // we didn't validate credentials yet
+      debouncedUpdateUrlPathWarningVisibility.cancel();
+      updateUrlPathWarningVisibility();
+    }
+
+    const submitBtn = $projectImportForm.find('input[type="submit"]');
+
+    submitBtn.disable();
+    await importCredentialsValidationPromise;
+    submitBtn.enable();
+
     const $invalidFields = $projectImportForm.find(`.${invalidInputClass}`);
     if ($invalidFields.length > 0) {
       $invalidFields[0].focus();
-      e.preventDefault();
-      e.stopPropagation();
+    } else {
+      // calling .submit() on HTMLFormElement does not trigger 'submit' event
+      // We are using this behavior to bypass this handler and avoid infinite loop
+      $projectImportForm[0].submit();
     }
   });
 

app/views/projects/_import_project_pane.html.haml

@@ -82,7 +82,7 @@
 
 
   .js-toggle-content.toggle-import-form{ class: ('hide' if active_tab != 'import') }
-    = form_for @project, html: { class: 'new_project gl-show-field-errors' } do |f|
+    = form_for @project, html: { class: 'new_project gl-show-field-errors js-project-import' } 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/projects/imports/new.html.haml

@@ -12,8 +12,8 @@
         :preserve
           #{h(@project.import_state.last_error)}
 
-= form_for @project, url: project_import_path(@project), method: :post do |f|
+= form_for @project, url: project_import_path(@project), method: :post, html: { class: 'js-project-import' } do |f|
   = render "shared/import_form", f: f
 
   .form-actions
-    = f.submit 'Start import', class: "gl-button btn btn-confirm"
+    = f.submit 'Start import', class: "gl-button btn btn-confirm", data: { disable_with: false }

ee/spec/features/projects/new_project_spec.rb

@@ -68,7 +68,7 @@ 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")
+        stub_request(:get, "http://foo.git/info/refs?service=git-upload-pack").to_return(status: 200, body: "001e# service=git-upload-pack", headers: { 'Content-Type': 'application/x-git-upload-pack-advertisement' })
 
         visit new_project_path
         click_link 'Import project'
@@ -84,6 +84,7 @@ RSpec.describe 'New project', :js do
           fill_in 'project_path', with: 'import-project-with-features1'
           choose 'project_visibility_level_20'
           click_button 'Create project'
+          wait_for_requests
 
           created_project = Project.last
 

spec/features/projects/new_project_spec.rb

@@ -306,10 +306,24 @@ RSpec.describe 'New project', :js do
           expect(page).to have_text('There is not a valid Git repository at this URL')
         end
 
+        it 'reports error if repo URL is not a valid Git repository and submit button is clicked immediately' 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'
+          click_on 'Create project'
+
+          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
           collision_project = create(:project, name: 'test-name-collision', namespace: user.namespace)
+          stub_request(:get, "http://foo/bar/info/refs?service=git-upload-pack").to_return({ status: 200,
+            body: '001e# service=git-upload-pack',
+            headers: { 'Content-Type': 'application/x-git-upload-pack-advertisement' } })
 
-          fill_in 'project_import_url', with: collision_project.http_url_to_repo
+          fill_in 'project_import_url', with: 'http://foo/bar'
           fill_in 'project_name', with: collision_project.name
 
           click_on 'Create project'
@@ -319,6 +333,38 @@ RSpec.describe 'New project', :js do
         end
       end
 
+      context 'when import is initiated from project page' do
+        before do
+          project_without_repo = create(:project, name: 'project-without-repo', namespace: user.namespace)
+          visit project_path(project_without_repo)
+          click_on 'Import repository'
+        end
+
+        it 'reports error when invalid url is provided' 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'
+
+          click_on 'Start import'
+          wait_for_requests
+
+          expect(page).to have_text('There is not a valid Git repository at this URL')
+        end
+
+        it 'initiates import when valid repo url is provided' do
+          stub_request(:get, "http://foo/bar/info/refs?service=git-upload-pack").to_return({ status: 200,
+            body: '001e# service=git-upload-pack',
+            headers: { 'Content-Type': 'application/x-git-upload-pack-advertisement' } })
+
+          fill_in 'project_import_url', with: 'http://foo/bar'
+
+          click_on 'Start import'
+          wait_for_requests
+
+          expect(page).to have_text('Import in progress')
+        end
+      end
+
       context 'from GitHub' do
         before do
           first('.js-import-github').click