Merge branch 'sh-fix-ssh-mirrors' into 'master'

Don't put the password in the SSH remote if using public-key authentication

Closes #3352

See merge request !2837

changelogs/unreleased-ee/sh-fix-ssh-mirrors.yml

+---
+title: Don't put the password in the SSH remote if using public-key authentication
+merge_request: 2837
+author:

ee/app/models/ee/project.rb

@@ -156,7 +156,15 @@ module EE
     def fetch_mirror
       return unless mirror?
 
-      repository.fetch_upstream(self.import_url)
+      # Only send the password if it's needed
+      url =
+        if import_data&.password_auth?
+          import_url
+        else
+          username_only_import_url
+        end
+
+      repository.fetch_upstream(url)
     end
 
     def can_override_approvers?

ee/app/models/ee/project_import_data.rb

@@ -32,6 +32,10 @@ module EE
       ssh_import? && auth_method == 'ssh_public_key'
     end
 
+    def password_auth?
+      auth_method == 'password'
+    end
+
     def ssh_import?
       project&.import_url&.start_with?('ssh://')
     end

spec/ee/spec/models/ee/project_spec.rb

@@ -198,6 +198,24 @@ describe Project do
     end
   end
 
+  describe '#fetch_mirror' do
+    where(:import_url, :auth_method, :expected) do
+      'http://foo:bar@example.com' | 'password'       | 'http://foo:bar@example.com'
+      'ssh://foo:bar@example.com'  | 'password'       | 'ssh://foo:bar@example.com'
+      'ssh://foo:bar@example.com'  | 'ssh_public_key' | 'ssh://foo@example.com'
+    end
+
+    with_them do
+      let(:project) { build(:project, :mirror, import_url: import_url, import_data_attributes: { auth_method: auth_method } ) }
+
+      it do
+        expect(project.repository).to receive(:fetch_upstream).with(expected)
+
+        project.fetch_mirror
+      end
+    end
+  end
+
   describe '#mirror_waiting_duration' do
     it 'returns in seconds the time spent in the queue' do
       project = create(:project, :mirror, :import_scheduled)