Merge branch '27480_deploy_keys_should_not_show_up_in_users_keys_list' into 'master'

Make deploy key not show in User's keys list

Closes #27480

See merge request !9024

5 files changed, 53 insertions, 13 deletions

diff --git a/app/models/user.rb b/app/models/user.rb
index 867e61af56a..1649bf04eaa 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -51,7 +51,12 @@ class User < ActiveRecord::Base
   has_one :namespace, -> { where type: nil }, dependent: :destroy, foreign_key: :owner_id
 
   # Profile
-  has_many :keys, dependent: :destroy
+  has_many :keys, -> do
+    type = Key.arel_table[:type]
+    where(type.not_eq('DeployKey').or(type.eq(nil)))
+  end, dependent: :destroy
+  has_many :deploy_keys, -> { where(type: 'DeployKey') }, dependent: :destroy
+
   has_many :emails, dependent: :destroy
   has_many :personal_access_tokens, dependent: :destroy
   has_many :identities, dependent: :destroy, autosave: true
diff --git a/changelogs/unreleased/27480-deploy_keys_should_not_show_up_in_users_keys_list.yml b/changelogs/unreleased/27480-deploy_keys_should_not_show_up_in_users_keys_list.yml
new file mode 100644
index 00000000000..6e9192cb632
--- /dev/null
+++ b/changelogs/unreleased/27480-deploy_keys_should_not_show_up_in_users_keys_list.yml
@@ -0,0 +1,4 @@
+---
+title: Do not display deploy keys in user's own ssh keys list
+merge_request: 9024
+author:
diff --git a/spec/controllers/profiles/keys_controller_spec.rb b/spec/controllers/profiles/keys_controller_spec.rb
index 6bcfae0fc13..f7219690722 100644
--- a/spec/controllers/profiles/keys_controller_spec.rb
+++ b/spec/controllers/profiles/keys_controller_spec.rb
@@ -42,10 +42,9 @@ describe Profiles::KeysController do
     end
 
     describe "user with keys" do
-      before do
-        user.keys << create(:key)
-        user.keys << create(:another_key)
-      end
+      let!(:key) { create(:key, user: user) }
+      let!(:another_key) { create(:another_key, user: user) }
+      let!(:deploy_key) { create(:deploy_key, user: user) }
 
       it "does generally work" do
         get :get_keys, username: user.username
@@ -53,16 +52,16 @@ describe Profiles::KeysController do
         expect(response).to be_success
       end
 
-      it "renders all keys separated with a new line" do
+      it "renders all non deploy keys separated with a new line" do
         get :get_keys, username: user.username
 
-        expect(response.body).not_to eq("")
+        expect(response.body).not_to eq('')
         expect(response.body).to eq(user.all_ssh_keys.join("\n"))
 
-        # Unique part of key 1
-        expect(response.body).to match(/PWx6WM4lhHNedGfBpPJNPpZ/)
-        # Key 2
-        expect(response.body).to match(/AQDmTillFzNTrrGgwaCKaSj/)
+        expect(response.body).to include(key.key.sub(' dummy@gitlab.com', ''))
+        expect(response.body).to include(another_key.key)
+
+        expect(response.body).not_to include(deploy_key.key)
       end
 
       it "does not render the comment of the key" do
diff --git a/spec/factories/keys.rb b/spec/factories/keys.rb
index d69c5b38d0a..dd93b439b2b 100644
--- a/spec/factories/keys.rb
+++ b/spec/factories/keys.rb
@@ -2,10 +2,13 @@ FactoryGirl.define do
   factory :key do
     title
     key do
-      "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0= dummy@gitlab.com"
+      'ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0= dummy@gitlab.com'
     end
 
     factory :deploy_key, class: 'DeployKey' do
+      key do
+        'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDFf6RYK3qu/RKF/3ndJmL5xgMLp3O96x8lTay+QGZ0+9FnnAXMdUqBq/ZU6d/gyMB4IaW3nHzM1w049++yAB6UPCzMB8Uo27K5/jyZCtj7Vm9PFNjF/8am1kp46c/SeYicQgQaSBdzIW3UDEa1Ef68qroOlvpi9PYZ/tA7M0YP0K5PXX+E36zaIRnJVMPT3f2k+GnrxtjafZrwFdpOP/Fol5BQLBgcsyiU+LM1SuaCrzd8c9vyaTA1CxrkxaZh+buAi0PmdDtaDrHd42gqZkXCKavyvgM5o2CkQ5LJHCgzpXy05qNFzmThBSkb+XtoxbyagBiGbVZtSVow6Xa7qewz'
+      end
     end
 
     factory :personal_key do
@@ -14,7 +17,7 @@ FactoryGirl.define do
 
     factory :another_key do
       key do
-        "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDmTillFzNTrrGgwaCKaSj+QCz81E6jBc/s9av0+3b1Hwfxgkqjl4nAK/OD2NjgyrONDTDfR8cRN4eAAy6nY8GLkOyYBDyuc5nTMqs5z3yVuTwf3koGm/YQQCmo91psZ2BgDFTor8SVEE5Mm1D1k3JDMhDFxzzrOtRYFPci9lskTJaBjpqWZ4E9rDTD2q/QZntCqbC3wE9uSemRQB5f8kik7vD/AD8VQXuzKladrZKkzkONCPWsXDspUitjM8HkQdOf0PsYn1CMUC1xKYbCxkg5TkEosIwGv6CoEArUrdu/4+10LVslq494mAvEItywzrluCLCnwELfW+h/m8UHoVhZ"
+        'ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDmTillFzNTrrGgwaCKaSj+QCz81E6jBc/s9av0+3b1Hwfxgkqjl4nAK/OD2NjgyrONDTDfR8cRN4eAAy6nY8GLkOyYBDyuc5nTMqs5z3yVuTwf3koGm/YQQCmo91psZ2BgDFTor8SVEE5Mm1D1k3JDMhDFxzzrOtRYFPci9lskTJaBjpqWZ4E9rDTD2q/QZntCqbC3wE9uSemRQB5f8kik7vD/AD8VQXuzKladrZKkzkONCPWsXDspUitjM8HkQdOf0PsYn1CMUC1xKYbCxkg5TkEosIwGv6CoEArUrdu/4+10LVslq494mAvEItywzrluCLCnwELfW+h/m8UHoVhZ'
       end
 
       factory :another_deploy_key, class: 'DeployKey' do
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 7fd49c73b37..89cef7ab978 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -19,6 +19,7 @@ describe User, models: true do
     it { is_expected.to have_many(:project_members).dependent(:destroy) }
     it { is_expected.to have_many(:groups) }
     it { is_expected.to have_many(:keys).dependent(:destroy) }
+    it { is_expected.to have_many(:deploy_keys).dependent(:destroy) }
     it { is_expected.to have_many(:events).dependent(:destroy) }
     it { is_expected.to have_many(:recent_events).class_name('Event') }
     it { is_expected.to have_many(:issues).dependent(:destroy) }
@@ -303,6 +304,34 @@ describe User, models: true do
     end
   end
 
+  shared_context 'user keys' do
+    let(:user) { create(:user) }
+    let!(:key) { create(:key, user: user) }
+    let!(:deploy_key) { create(:deploy_key, user: user) }
+  end
+
+  describe '#keys' do
+    include_context 'user keys'
+
+    context 'with key and deploy key stored' do
+      it 'returns stored key, but not deploy_key' do
+        expect(user.keys).to include key
+        expect(user.keys).not_to include deploy_key
+      end
+    end
+  end
+
+  describe '#deploy_keys' do
+    include_context 'user keys'
+
+    context 'with key and deploy key stored' do
+      it 'returns stored deploy key, but not normal key' do
+        expect(user.deploy_keys).to include deploy_key
+        expect(user.deploy_keys).not_to include key
+      end
+    end
+  end
+
   describe '#confirm' do
     before do
       allow_any_instance_of(ApplicationSetting).to receive(:send_user_confirmation_email).and_return(true)