diff --git a/app/models/user.rb b/app/models/user.rb index 867e61af56a944cd1eb72e09ec3bf1e867a5c3aa..1649bf04eaa0d1341754c6e13bb4a96a6cc05346 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 0000000000000000000000000000000000000000..6e9192cb63291b122519f9c4ceb01d1ce9deef05 --- /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 6bcfae0fc13d912c96ebd76d0562bf4bfad4aae5..f7219690722e7422cb7eeb41cf65a16c2f92bfb9 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 d69c5b38d0a63006052e5a34e2a6ff7e05aa9284..dd93b439b2be7e91f59406b8526ffefd7c57c408 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 7fd49c73b37e6b95390d04e725c0a1fce7c794f3..89cef7ab978154ecf2a59a9328fb4d2abbeededc 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)