Merge branch 'ml-fix-2fa-ssh-test-on-review-apps' into 'master'

Don't use HTTPS port for SSH

See merge request gitlab-org/gitlab!56435

qa/qa/support/ssh.rb

@@ -51,11 +51,21 @@ module QA
       private
 
       def uri_port
-        uri.port && (uri.port != 80) ? uri.port : nil
+        use_typical_params? ? nil : uri.port
       end
 
       def git_user
-        QA::Runtime::Env.running_in_ci? || [443, 80].include?(uri.port) ? 'git' : Etc.getlogin
+        QA::Runtime::Env.running_in_ci? || use_typical_params? ? 'git' : Etc.getlogin
+      end
+
+      # Checks if typical parameters should be used. That means the SSH port will not be
+      # needed because it's port 22, and the git user is named 'git'. We assume that
+      # typical parameters should be used if the host URI includes a typical HTTP(S)
+      # port (80 or 443)
+      #
+      # @return [Boolean] whether typical SSH port and git user parameters should be used
+      def use_typical_params?
+        [443, 80].include?(uri.port)
       end
     end
   end

qa/spec/support/ssh_spec.rb

@@ -26,6 +26,16 @@ RSpec.describe QA::Support::SSH do
       end
     end
 
+    context 'when no port specified in https uri' do
+      let(:uri) { 'https://foo.com' }
+
+      it 'does not provide port in ssh command' do
+        expect(ssh).to receive(:run).with(expected_ssh_command_no_port, any_args).and_return(result)
+
+        call_method
+      end
+    end
+
     context 'when port 80 specified in uri' do
       let(:uri) { 'http://foo.com:80' }
 
@@ -86,6 +96,18 @@ RSpec.describe QA::Support::SSH do
       end
     end
 
+    context 'when running on a review app in CI' do
+      let(:uri) { 'https://gitlab-review.app' }
+
+      before do
+        allow(QA::Runtime::Env).to receive(:running_in_ci?).and_return(true)
+      end
+
+      it 'returns git user' do
+        expect(ssh.send(:git_user)).to eq('git')
+      end
+    end
+
     context 'when running against environment on a port other than 80 or 443' do
       let(:uri) { 'http://localhost:3000' }