Merge branch 'sh-migrate-can-push-to-deploy-keys-projects-10-3' into 'security-10-3'

[10.3] Migrate `can_push` column from `keys` to `deploy_keys_project`

See merge request gitlab/gitlabhq!2276

(cherry picked from commit f6ca52d31bac350a23938e0aebf717c767b4710c)

1f2bd3c0 Backport to 10.3

12 files changed, 51 insertions, 52 deletions

diff --git a/spec/factories/deploy_keys_projects.rb b/spec/factories/deploy_keys_projects.rb
index 30a6d468ed3..4350652fb79 100644
--- a/spec/factories/deploy_keys_projects.rb
+++ b/spec/factories/deploy_keys_projects.rb
@@ -2,5 +2,9 @@ FactoryBot.define do
   factory :deploy_keys_project do
     deploy_key
     project
+
+    trait :write_access do
+      can_push true
+    end
   end
 end
diff --git a/spec/factories/keys.rb b/spec/factories/keys.rb
index 552b4b7e06e..f0c43f3d6f5 100644
--- a/spec/factories/keys.rb
+++ b/spec/factories/keys.rb
@@ -15,10 +15,6 @@ FactoryBot.define do
       factory :another_deploy_key, class: 'DeployKey'
     end
 
-    factory :write_access_key, class: 'DeployKey' do
-      can_push true
-    end
-
     factory :rsa_key_2048 do
       key do
         <<~KEY.delete("\n")
diff --git a/spec/features/admin/admin_deploy_keys_spec.rb b/spec/features/admin/admin_deploy_keys_spec.rb
index 241c7cbc34e..cb96830cb7c 100644
--- a/spec/features/admin/admin_deploy_keys_spec.rb
+++ b/spec/features/admin/admin_deploy_keys_spec.rb
@@ -17,6 +17,16 @@ RSpec.describe 'admin deploy keys' do
     end
   end
 
+  it 'shows all the projects the deploy key has write access' do
+    write_key = create(:deploy_keys_project, :write_access, deploy_key: deploy_key)
+
+    visit admin_deploy_keys_path
+
+    page.within(find('.deploy-keys-list', match: :first)) do
+      expect(page).to have_content(write_key.project.full_name)
+    end
+  end
+
   describe 'create a new deploy key' do
     let(:new_ssh_key) { attributes_for(:key)[:key] }
 
@@ -28,14 +38,12 @@ RSpec.describe 'admin deploy keys' do
     it 'creates a new deploy key' do
       fill_in 'deploy_key_title', with: 'laptop'
       fill_in 'deploy_key_key', with: new_ssh_key
-      check 'deploy_key_can_push'
       click_button 'Create'
 
       expect(current_path).to eq admin_deploy_keys_path
 
       page.within(find('.deploy-keys-list', match: :first)) do
         expect(page).to have_content('laptop')
-        expect(page).to have_content('Yes')
       end
     end
   end
@@ -48,14 +56,12 @@ RSpec.describe 'admin deploy keys' do
 
     it 'updates an existing deploy key' do
       fill_in 'deploy_key_title', with: 'new-title'
-      check 'deploy_key_can_push'
       click_button 'Save changes'
 
       expect(current_path).to eq admin_deploy_keys_path
 
       page.within(find('.deploy-keys-list', match: :first)) do
         expect(page).to have_content('new-title')
-        expect(page).to have_content('Yes')
       end
     end
   end
diff --git a/spec/features/projects/settings/repository_settings_spec.rb b/spec/features/projects/settings/repository_settings_spec.rb
index 81b282502fc..14670e91006 100644
--- a/spec/features/projects/settings/repository_settings_spec.rb
+++ b/spec/features/projects/settings/repository_settings_spec.rb
@@ -43,7 +43,7 @@ feature 'Repository settings' do
 
         fill_in 'deploy_key_title', with: 'new_deploy_key'
         fill_in 'deploy_key_key', with: new_ssh_key
-        check 'deploy_key_can_push'
+        check 'deploy_key_deploy_keys_projects_attributes_0_can_push'
         click_button 'Add key'
 
         expect(page).to have_content('new_deploy_key')
@@ -57,7 +57,7 @@ feature 'Repository settings' do
         find('li', text: private_deploy_key.title).click_link('Edit')
 
         fill_in 'deploy_key_title', with: 'updated_deploy_key'
-        check 'deploy_key_can_push'
+        check 'deploy_key_deploy_keys_projects_attributes_0_can_push'
         click_button 'Save changes'
 
         expect(page).to have_content('updated_deploy_key')
@@ -74,11 +74,9 @@ feature 'Repository settings' do
         find('li', text: private_deploy_key.title).click_link('Edit')
 
         fill_in 'deploy_key_title', with: 'updated_deploy_key'
-        check 'deploy_key_can_push'
         click_button 'Save changes'
 
         expect(page).to have_content('updated_deploy_key')
-        expect(page).to have_content('Write access allowed')
       end
 
       scenario 'remove an existing deploy key' do
diff --git a/spec/javascripts/deploy_keys/components/key_spec.js b/spec/javascripts/deploy_keys/components/key_spec.js
index 2f28c5bbf01..b7aadf604a4 100644
--- a/spec/javascripts/deploy_keys/components/key_spec.js
+++ b/spec/javascripts/deploy_keys/components/key_spec.js
@@ -53,18 +53,24 @@ describe('Deploy keys key', () => {
       ).toBe('Remove');
     });
 
-    it('shows write access text when key has write access', (done) => {
-      vm.deployKey.can_push = true;
+    it('shows write access title when key has write access', (done) => {
+      vm.deployKey.deploy_keys_projects[0].can_push = true;
 
       Vue.nextTick(() => {
         expect(
-          vm.$el.querySelector('.write-access-allowed'),
-        ).not.toBeNull();
-
-        expect(
-          vm.$el.querySelector('.write-access-allowed').textContent.trim(),
+          vm.$el.querySelector('.deploy-project-label').getAttribute('data-original-title'),
         ).toBe('Write access allowed');
+        done();
+      });
+    });
+
+    it('does not show write access title when key has write access', (done) => {
+      vm.deployKey.deploy_keys_projects[0].can_push = false;
 
+      Vue.nextTick(() => {
+        expect(
+          vm.$el.querySelector('.deploy-project-label').getAttribute('data-original-title'),
+        ).toBe('Read access only');
         done();
       });
     });
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index a6fbec295b5..cc202ce8bca 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -136,8 +136,8 @@ describe Gitlab::Auth do
 
       it 'grants deploy key write permissions' do
         project = create(:project)
-        key = create(:deploy_key, can_push: true)
-        create(:deploy_keys_project, deploy_key: key, project: project)
+        key = create(:deploy_key)
+        create(:deploy_keys_project, :write_access, deploy_key: key, project: project)
         token = Gitlab::LfsToken.new(key).token
 
         expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}")
@@ -146,7 +146,7 @@ describe Gitlab::Auth do
 
       it 'does not grant deploy key write permissions' do
         project = create(:project)
-        key = create(:deploy_key, can_push: true)
+        key = create(:deploy_key)
         token = Gitlab::LfsToken.new(key).token
 
         expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: "lfs+deploy-key-#{key.id}")
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 4290fbb0087..2009a8ac48c 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -51,12 +51,12 @@ describe Gitlab::GitAccess do
     context 'when the project exists' do
       context 'when actor exists' do
         context 'when actor is a DeployKey' do
-          let(:deploy_key) { create(:deploy_key, user: user, can_push: true) }
+          let(:deploy_key) { create(:deploy_key, user: user) }
           let(:actor) { deploy_key }
 
           context 'when the DeployKey has access to the project' do
             before do
-              deploy_key.projects << project
+              deploy_key.deploy_keys_projects.create(project: project, can_push: true)
             end
 
             it 'allows push and pull access' do
@@ -696,15 +696,13 @@ describe Gitlab::GitAccess do
   end
 
   describe 'deploy key permissions' do
-    let(:key) { create(:deploy_key, user: user, can_push: can_push) }
+    let(:key) { create(:deploy_key, user: user) }
     let(:actor) { key }
 
     context 'when deploy_key can push' do
-      let(:can_push) { true }
-
       context 'when project is authorized' do
         before do
-          key.projects << project
+          key.deploy_keys_projects.create(project: project, can_push: true)
         end
 
         it { expect { push_access_check }.not_to raise_error }
@@ -732,11 +730,9 @@ describe Gitlab::GitAccess do
     end
 
     context 'when deploy_key cannot push' do
-      let(:can_push) { false }
-
       context 'when project is authorized' do
         before do
-          key.projects << project
+          key.deploy_keys_projects.create(project: project, can_push: false)
         end
 
         it { expect { push_access_check }.to raise_unauthorized(described_class::ERROR_MESSAGES[:deploy_key_upload]) }
diff --git a/spec/models/deploy_keys_project_spec.rb b/spec/models/deploy_keys_project_spec.rb
index 0345fefb254..fca3090ff4a 100644
--- a/spec/models/deploy_keys_project_spec.rb
+++ b/spec/models/deploy_keys_project_spec.rb
@@ -8,7 +8,7 @@ describe DeployKeysProject do
 
   describe "Validation" do
     it { is_expected.to validate_presence_of(:project_id) }
-    it { is_expected.to validate_presence_of(:deploy_key_id) }
+    it { is_expected.to validate_presence_of(:deploy_key) }
   end
 
   describe "Destroying" do
diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb
index 1f1e6ea17e4..0772b3f2e64 100644
--- a/spec/requests/api/deploy_keys_spec.rb
+++ b/spec/requests/api/deploy_keys_spec.rb
@@ -110,7 +110,7 @@ describe API::DeployKeys do
     end
 
     it 'accepts can_push parameter' do
-      key_attrs = attributes_for :write_access_key
+      key_attrs = attributes_for(:another_key).merge(can_push: true)
 
       post api("/projects/#{project.id}/deploy_keys", admin), key_attrs
 
@@ -160,16 +160,6 @@ describe API::DeployKeys do
       expect(json_response['title']).to eq('new title')
       expect(json_response['can_push']).to eq(true)
     end
-
-    it 'updates a private ssh key from projects user has access with correct attributes' do
-      create(:deploy_keys_project, project: project2, deploy_key: private_deploy_key)
-
-      put api("/projects/#{project.id}/deploy_keys/#{private_deploy_key.id}", admin), { title: 'new title', can_push: true }
-
-      expect(json_response['id']).to eq(private_deploy_key.id)
-      expect(json_response['title']).to eq('new title')
-      expect(json_response['can_push']).to eq(true)
-    end
   end
 
   describe 'DELETE /projects/:id/deploy_keys/:key_id' do
diff --git a/spec/requests/api/v3/deploy_keys_spec.rb b/spec/requests/api/v3/deploy_keys_spec.rb
index 785bc1eb4ba..501af587ad4 100644
--- a/spec/requests/api/v3/deploy_keys_spec.rb
+++ b/spec/requests/api/v3/deploy_keys_spec.rb
@@ -107,7 +107,7 @@ describe API::V3::DeployKeys do
       end
 
       it 'accepts can_push parameter' do
-        key_attrs = attributes_for :write_access_key
+        key_attrs = attributes_for(:another_key).merge(can_push: true)
 
         post v3_api("/projects/#{project.id}/#{path}", admin), key_attrs
 
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index 5e59bb0d585..bee918a20aa 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -781,11 +781,11 @@ describe 'Git LFS API and storage' do
         end
 
         context 'when deploy key has project push access' do
-          let(:key) { create(:deploy_key, can_push: true) }
+          let(:key) { create(:deploy_key) }
           let(:authorization) { authorize_deploy_key }
 
           let(:update_user_permissions) do
-            project.deploy_keys << key
+            project.deploy_keys_projects.create(deploy_key: key, can_push: true)
           end
 
           it_behaves_like 'pushes new LFS objects'
diff --git a/spec/serializers/deploy_key_entity_spec.rb b/spec/serializers/deploy_key_entity_spec.rb
index d3aefa2c9eb..2bd8162d1b7 100644
--- a/spec/serializers/deploy_key_entity_spec.rb
+++ b/spec/serializers/deploy_key_entity_spec.rb
@@ -21,18 +21,21 @@ describe DeployKeyEntity do
         user_id: deploy_key.user_id,
         title: deploy_key.title,
         fingerprint: deploy_key.fingerprint,
-        can_push: deploy_key.can_push,
         destroyed_when_orphaned: true,
         almost_orphaned: false,
         created_at: deploy_key.created_at,
         updated_at: deploy_key.updated_at,
         can_edit: false,
-        projects: [
+        deploy_keys_projects: [
           {
-            id: project.id,
-            name: project.name,
-            full_path: project_path(project),
-            full_name: project.full_name
+            can_push: false,
+            project:
+            {
+              id: project.id,
+              name: project.name,
+              full_path: project_path(project),
+              full_name: project.full_name
+            }
           }
         ]
       }