Merge remote-tracking branch 'origin-ee/master' into fix_issue_407

CHANGELOG-EE

@@ -4,6 +4,7 @@ v 8.8.0 (unreleased)
   - [Elastic] Database indexer prints its status
   - [Elastic][Fix] Database indexer skips projects with invalid HEAD reference
   - Fix skipping pages when restoring backups
+  - Set KRB5 as default clone protocol when Kerberos is enabled and user is logged in (Borja Aparicio)
 
 v 8.7.2
   - Fix MR notifications for slack and hipchat when approvals are fullfiled. !325

app/assets/stylesheets/framework/common.scss

@@ -263,11 +263,6 @@ img.emoji {
   margin-bottom: 10px;
 }
 
-.group-name {
-  font-size: 14px;
-  line-height: 24px;
-}
-
 .available-groups form {
   margin: 5px 0;
 }

app/helpers/button_helper.rb

@@ -55,18 +55,17 @@ module ButtonHelper
   end
 
   def kerberos_clone_button(project)
-    klass = 'btn js-protocol-switch'
-    klass << ' active'      if default_clone_protocol == 'kerberos'
+    klass = 'kerberos-selector'
     klass << ' has-tooltip'
 
-    content_tag :button, 'KRB5',
+    content_tag :a, 'KRB5',
       class: klass,
+      href: project.kerberos_url_to_repo,
       data: {
-        clone: project.kerberos_url_to_repo,
-        container: 'body',
         html: 'true',
-        title: 'Get a Kerberos token for your account with kinit<br>to pull or push via Kerberos.'
-      },
-      type: :button
+        placement: 'right',
+        container: 'body',
+        title: 'Get a Kerberos token for your<br>account with kinit.'
+      }
   end
 end

app/helpers/projects_helper.rb

@@ -209,7 +209,10 @@ module ProjectsHelper
   end
 
   def default_url_to_repo(project = @project)
-    if default_clone_protocol == "ssh"
+    case default_clone_protocol
+    when 'krb5'
+      project.kerberos_url_to_repo
+    when 'ssh'
       project.ssh_url_to_repo
     else
       project.http_url_to_repo
@@ -217,7 +220,9 @@ module ProjectsHelper
   end
 
   def default_clone_protocol
-    if !current_user || current_user.require_ssh_key?
+    if alternative_kerberos_url? && current_user
+      "krb5"
+    elsif !current_user || current_user.require_ssh_key?
       gitlab_config.protocol
     else
       "ssh"

app/models/project.rb

@@ -269,7 +269,7 @@ class Project < ActiveRecord::Base
     state :failed
 
     after_transition any => :started, do: :schedule_add_import_job
-    after_transition any => :finished, do: :clear_import_data
+    after_transition any => :finished, do: :reset_cache_and_import_attrs
 
     after_transition started: :finished do |project, transaction|
       if project.mirror?
@@ -453,12 +453,12 @@ class Project < ActiveRecord::Base
     end
   end
 
-  def clear_import_data
+  def reset_cache_and_import_attrs
     update(import_error: nil)
 
     ProjectCacheWorker.perform_async(self.id)
 
-    self.import_data.destroy if self.import_data
+    self.import_data.destroy if !mirror? && import_data
   end
 
   def import_url=(value)

app/views/shared/_clone_panel.html.haml

@@ -13,8 +13,7 @@
         = http_clone_button(project)
       - if alternative_kerberos_url?
         %li
-          %a#kerberos-btn{href: @project.kerberos_url_to_repo}
-            KRB5
+          = kerberos_clone_button(project)
 
   = text_field_tag :project_clone, default_url_to_repo(project), class: "js-select-on-focus form-control", readonly: true
   .input-group-btn

features/project/create.feature

@@ -7,37 +7,8 @@ Feature: Project Create
   @javascript
   Scenario: User create a project
     Given I sign in as a user
-    When I visit new project page
-    And I have an ssh key
-    And fill project form with valid data
-    Then I should see project page
-    And I should see empty project instuctions
-
-  @javascript
-  Scenario: Empty project instructions with Kerberos disabled
-    Given I sign in as a user
-    And KRB5 disabled
     And I have an ssh key
     When I visit new project page
     And fill project form with valid data
-    Then I see empty project instuctions
-    And I click on HTTP
-    Then Remote url should update to http link
-    And If I click on SSH
-    Then Remote url should update to ssh link
-
-  @javascript
-  Scenario: Empty project instructions with Kerberos enabled
-    Given I sign in as a user
-    Given KRB5 enabled
-    And I have an ssh key
-    When I visit new project page
-    And fill project form with valid data
-    Then I see empty project instuctions
-    And I click on HTTP
-    Then Remote url should update to http link
-    And If I click on SSH
-    Then Remote url should update to ssh link
-    And If I click on KRB5
-    Then Remote url should update to kerberos link
-    And KRB5 disabled
+    Then I should see project page
+    And I should see empty project instructions

features/steps/project/create.rb

@@ -13,52 +13,9 @@ class Spinach::Features::ProjectCreate < Spinach::FeatureSteps
     expect(current_path).to eq namespace_project_path(Project.last.namespace, Project.last)
   end
 
-  step 'I should see empty project instuctions' do
+  step 'I should see empty project instructions' do
     expect(page).to have_content "git init"
     expect(page).to have_content "git remote"
     expect(page).to have_content Project.last.url_to_repo
   end
-
-  step 'KRB5 enabled' do
-    # Enable Kerberos in an alternative port to force Kerberos button and URL to show up in the UI
-    allow(Gitlab.config.kerberos).to receive(:enabled).and_return(true)
-    allow(Gitlab.config.kerberos).to receive(:use_dedicated_port).and_return(true)
-  end
-
-  step 'KRB5 disabled' do
-    allow(Gitlab.config.kerberos).to receive(:enabled).and_return(false)
-  end
-
-  step 'I see empty project instuctions' do
-    expect(page).to have_content "git init"
-    expect(page).to have_content "git remote"
-    expect(page).to have_content Project.last.url_to_repo
-  end
-
-  step 'I click on HTTP' do
-    find('#clone-dropdown').click
-    find('.http-selector').click
-  end
-
-  step 'Remote url should update to http link' do
-    expect(page).to have_content "git remote add origin #{Project.last.http_url_to_repo}"
-  end
-
-  step 'If I click on SSH' do
-    find('#clone-dropdown').click
-    find('.ssh-selector').click
-  end
-
-  step 'Remote url should update to ssh link' do
-    expect(page).to have_content "git remote add origin #{Project.last.url_to_repo}"
-  end
-
-  step 'If I click on KRB5' do
-    find('#clone-dropdown').click
-    find('#kerberos-btn').click
-  end
-
-  step 'Remote url should update to kerberos link' do
-    expect(page).to have_content "git remote add origin #{Project.last.kerberos_url_to_repo}"
-  end
 end

spec/factories/projects.rb

@@ -92,6 +92,12 @@ FactoryGirl.define do
       mirror true
       mirror_user_id { creator_id }
     end
+
+    trait :empty_repo do
+      after(:create) do |project|
+        project.create_repository
+      end
+    end
   end
 
   # Project with empty repository
@@ -99,9 +105,7 @@ FactoryGirl.define do
   # This is a case when you just created a project
   # but not pushed any code there yet
   factory :project_empty_repo, parent: :empty_project do
-    after :create do |project|
-      project.create_repository
-    end
+    empty_repo
   end
 
   # Project with test repository

spec/features/projects/developer_views_empty_project_instructions_spec.rb

+require 'rails_helper'
+
+feature 'Developer views empty project instructions', feature: true do
+  let(:project) { create(:empty_project, :empty_repo) }
+  let(:developer) { create(:user) }
+
+  background do
+    project.team << [developer, :developer]
+
+    login_as(developer)
+  end
+
+  context 'without an SSH key' do
+    scenario 'defaults to HTTP' do
+      visit_project
+
+      expect_instructions_for('http')
+    end
+
+    scenario 'switches to SSH', js: true do
+      visit_project
+
+      select_protocol('SSH')
+
+      expect_instructions_for('ssh')
+    end
+  end
+
+  context 'with an SSH key' do
+    background do
+      create(:personal_key, user: developer)
+    end
+
+    scenario 'defaults to SSH' do
+      visit_project
+
+      expect_instructions_for('ssh')
+    end
+
+    scenario 'switches to HTTP', js: true do
+      visit_project
+
+      select_protocol('HTTP')
+
+      expect_instructions_for('http')
+    end
+  end
+
+  context 'with Kerberos enabled' do
+    background do
+      allow(Gitlab.config.kerberos).to receive(:enabled).and_return(true)
+    end
+
+    scenario 'defaults to KRB5' do
+      visit_project
+
+      expect_instructions_for('kerberos')
+    end
+  end
+
+  def visit_project
+    visit namespace_project_path(project.namespace, project)
+  end
+
+  def select_protocol(protocol)
+    find('#clone-dropdown').click
+    find(".#{protocol.downcase}-selector").click
+  end
+
+  def expect_instructions_for(protocol)
+    msg = :"#{protocol.downcase}_url_to_repo"
+
+    expect(page).to have_content("git clone #{project.send(msg)}")
+  end
+end

spec/helpers/projects_helper_spec.rb

@@ -88,22 +88,31 @@ describe ProjectsHelper do
   end
 
   describe 'default_clone_protocol' do
-    describe 'using HTTP' do
+    context 'when user is not logged in and gitlab protocol is HTTP' do
       it 'returns HTTP' do
-        expect(helper).to receive(:current_user).and_return(nil)
+        allow(helper).to receive(:current_user).and_return(nil)
 
         expect(helper.send(:default_clone_protocol)).to eq('http')
       end
     end
 
-    describe 'using HTTPS' do
+    context 'when user is not logged in and gitlab protocol is HTTPS' do
       it 'returns HTTPS' do
-        allow(Gitlab.config.gitlab).to receive(:protocol).and_return('https')
-        expect(helper).to receive(:current_user).and_return(nil)
+        stub_config_setting(protocol: 'https')
+        allow(helper).to receive(:current_user).and_return(nil)
 
         expect(helper.send(:default_clone_protocol)).to eq('https')
       end
     end
+
+    context 'when gitlab.config.kerberos is enabled and user is logged in' do
+      it 'returns krb5 as default protocol' do
+        allow(Gitlab.config.kerberos).to receive(:enabled).and_return(true)
+        allow(helper).to receive(:current_user).and_return(double)
+
+        expect(helper.send(:default_clone_protocol)).to eq('krb5')
+      end
+    end
   end
 
   describe '#license_short_name' do

spec/models/project_spec.rb

@@ -894,4 +894,27 @@ describe Project, models: true do
       end
     end
   end
+
+  describe 'handling import URL' do
+    context 'when project is a mirror' do
+      it 'returns the full URL' do
+        project = create(:project, :mirror, import_url: 'http://user:pass@test.com')
+
+        project.import_finish
+
+        expect(project.reload.import_url).to eq('http://user:pass@test.com')
+      end
+    end
+
+    context 'when project is not a mirror' do
+      it 'returns the sanitized URL' do
+        project = create(:project, import_status: 'started', import_url: 'http://user:pass@test.com')
+
+        project.import_finish
+
+        expect(project.reload.import_url).to eq('http://test.com')
+      end
+    end
+
+  end
 end