Merge branch '30769-deploy-keys-push-protected-branches' into 'master'

Add deploy keys to protected branches (DB and API)

See merge request gitlab-org/gitlab!34875

app/controllers/autocomplete_controller.rb

@@ -4,10 +4,6 @@ class AutocompleteController < ApplicationController
   skip_before_action :authenticate_user!, only: [:users, :award_emojis, :merge_request_target_branches]
 
   def users
-    project = Autocomplete::ProjectFinder
-      .new(current_user, params)
-      .execute
-
     group = Autocomplete::GroupFinder
       .new(current_user, project, params)
       .execute
@@ -50,8 +46,20 @@ class AutocompleteController < ApplicationController
     end
   end
 
+  def deploy_keys_with_owners
+    deploy_keys = DeployKeys::CollectKeysService.new(project, current_user).execute
+
+    render json: DeployKeySerializer.new.represent(deploy_keys, { with_owner: true, user: current_user })
+  end
+
   private
 
+  def project
+    @project ||= Autocomplete::ProjectFinder
+      .new(current_user, params)
+      .execute
+  end
+
   def target_branch_params
     params.permit(:group_id, :project_id).select { |_, v| v.present? }
   end

app/models/deploy_keys_project.rb

@@ -6,6 +6,7 @@ class DeployKeysProject < ApplicationRecord
   scope :without_project_deleted,  -> { joins(:project).where(projects: { pending_delete: false }) }
   scope :in_project, ->(project) { where(project: project) }
   scope :with_write_access, -> { where(can_push: true) }
+  scope :with_deploy_keys, -> { includes(:deploy_key) }
 
   accepts_nested_attributes_for :deploy_key
 

app/serializers/deploy_key_entity.rb

@@ -16,6 +16,7 @@ class DeployKeyEntity < Grape::Entity
     end
   end
   expose :can_edit
+  expose :user, as: :owner, using: ::API::Entities::UserBasic, if: -> (_, opts) { can_read_owner?(opts) }
 
   private
 
@@ -24,6 +25,10 @@ class DeployKeyEntity < Grape::Entity
       Ability.allowed?(options[:user], :update_deploy_keys_project, object.deploy_keys_project_for(options[:project]))
   end
 
+  def can_read_owner?(opts)
+    opts[:with_owner] && Ability.allowed?(options[:user], :read_user, object.user)
+  end
+
   def allowed_to_read_project?(project)
     if options[:readable_project_ids]
       options[:readable_project_ids].include?(project.id)

app/services/deploy_keys/collect_keys_service.rb

+# frozen_string_literal: true
+
+module DeployKeys
+  class CollectKeysService
+    def initialize(project, current_user)
+      @project = project
+      @current_user = current_user
+    end
+
+    def execute
+      return [] unless current_user && project && user_can_read_project
+
+      project.deploy_keys_projects
+        .with_deploy_keys
+        .with_write_access
+        .map(&:deploy_key)
+    end
+
+    private
+
+    def user_can_read_project
+      Ability.allowed?(current_user, :read_project, project)
+    end
+
+    attr_reader :project, :current_user
+  end
+end

changelogs/unreleased/30769-deploy-keys-push-protected-branches.yml

+---
+title: Expose project deploy keys for autocompletion
+merge_request: 34875
+author:
+type: added

config/routes.rb

@@ -76,6 +76,7 @@ Rails.application.routes.draw do
     get '/autocomplete/projects' => 'autocomplete#projects'
     get '/autocomplete/award_emojis' => 'autocomplete#award_emojis'
     get '/autocomplete/merge_request_target_branches' => 'autocomplete#merge_request_target_branches'
+    get '/autocomplete/deploy_keys_with_owners' => 'autocomplete#deploy_keys_with_owners'
 
     Gitlab.ee do
       get '/autocomplete/project_groups' => 'autocomplete#project_groups'

db/migrate/20200617205000_add_deploy_key_id_to_push_access_levels.rb

+# frozen_string_literal: true
+
+class AddDeployKeyIdToPushAccessLevels < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    unless column_exists?(:protected_branch_push_access_levels, :deploy_key_id)
+      add_column :protected_branch_push_access_levels, :deploy_key_id, :integer
+    end
+
+    add_concurrent_foreign_key :protected_branch_push_access_levels, :keys, column: :deploy_key_id, on_delete: :cascade
+    add_concurrent_index :protected_branch_push_access_levels, :deploy_key_id, name: 'index_deploy_key_id_on_protected_branch_push_access_levels'
+  end
+
+  def down
+    remove_column :protected_branch_push_access_levels, :deploy_key_id
+  end
+end

db/structure.sql

@@ -14502,7 +14502,8 @@ CREATE TABLE public.protected_branch_push_access_levels (
     created_at timestamp without time zone NOT NULL,
     updated_at timestamp without time zone NOT NULL,
     user_id integer,
-    group_id integer
+    group_id integer,
+    deploy_key_id integer
 );
 
 CREATE SEQUENCE public.protected_branch_push_access_levels_id_seq
@@ -19027,6 +19028,8 @@ CREATE INDEX index_dependency_proxy_blobs_on_group_id_and_file_name ON public.de
 
 CREATE INDEX index_dependency_proxy_group_settings_on_group_id ON public.dependency_proxy_group_settings USING btree (group_id);
 
+CREATE INDEX index_deploy_key_id_on_protected_branch_push_access_levels ON public.protected_branch_push_access_levels USING btree (deploy_key_id);
+
 CREATE INDEX index_deploy_keys_projects_on_deploy_key_id ON public.deploy_keys_projects USING btree (deploy_key_id);
 
 CREATE INDEX index_deploy_keys_projects_on_project_id ON public.deploy_keys_projects USING btree (project_id);
@@ -20910,6 +20913,9 @@ ALTER TABLE ONLY public.vulnerabilities
 ALTER TABLE ONLY public.vulnerabilities
     ADD CONSTRAINT fk_131d289c65 FOREIGN KEY (milestone_id) REFERENCES public.milestones(id) ON DELETE SET NULL;
 
+ALTER TABLE ONLY public.protected_branch_push_access_levels
+    ADD CONSTRAINT fk_15d2a7a4ae FOREIGN KEY (deploy_key_id) REFERENCES public.keys(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.internal_ids
     ADD CONSTRAINT fk_162941d509 FOREIGN KEY (namespace_id) REFERENCES public.namespaces(id) ON DELETE CASCADE;
 
@@ -23688,6 +23694,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200617001848
 20200617002030
 20200617150041
+20200617205000
 20200618105638
 20200618134223
 20200618134723

lib/gitlab/import_export/project/import_export.yml

@@ -316,6 +316,7 @@ excluded_attributes:
     - :protected_branch_id
   push_access_levels:
     - :protected_branch_id
+    - :deploy_key_id
   unprotect_access_levels:
     - :protected_branch_id
   create_access_levels:

spec/controllers/autocomplete_controller_spec.rb

@@ -365,6 +365,56 @@ RSpec.describe AutocompleteController do
     end
   end
 
+  context 'GET deploy_keys_with_owners' do
+    let!(:deploy_key) { create(:deploy_key, user: user) }
+    let!(:deploy_keys_project) { create(:deploy_keys_project, :write_access, project: project, deploy_key: deploy_key) }
+
+    context 'unauthorized user' do
+      it 'returns a not found response' do
+        get(:deploy_keys_with_owners, params: { project_id: project.id })
+
+        expect(response).to have_gitlab_http_status(:redirect)
+      end
+    end
+
+    context 'when the user who can read the project is logged in' do
+      before do
+        sign_in(user)
+      end
+
+      it 'renders the deploy key in a json payload, with its owner' do
+        get(:deploy_keys_with_owners, params: { project_id: project.id })
+
+        expect(json_response.count).to eq(1)
+        expect(json_response.first['title']).to eq(deploy_key.title)
+        expect(json_response.first['owner']['id']).to eq(deploy_key.user.id)
+      end
+
+      context 'with an unknown project' do
+        it 'returns a not found response' do
+          get(:deploy_keys_with_owners, params: { project_id: 9999 })
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
+
+      context 'and the user cannot read the owner of the key' do
+        before do
+          allow(Ability).to receive(:allowed?).and_call_original
+          allow(Ability).to receive(:allowed?).with(user, :read_user, deploy_key.user).and_return(false)
+        end
+
+        it 'returns a payload without owner' do
+          get(:deploy_keys_with_owners, params: { project_id: project.id })
+
+          expect(json_response.count).to eq(1)
+          expect(json_response.first['title']).to eq(deploy_key.title)
+          expect(json_response.first['owner']).to be_nil
+        end
+      end
+    end
+  end
+
   context 'Get merge_request_target_branches' do
     let!(:merge_request) { create(:merge_request, source_project: project, target_branch: 'feature') }
 

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -588,6 +588,7 @@ ProtectedBranch::PushAccessLevel:
 - updated_at
 - user_id
 - group_id
+- deploy_key_id
 ProtectedBranch::UnprotectAccessLevel:
 - id
 - protected_branch_id

spec/models/deploy_keys_project_spec.rb

@@ -13,6 +13,21 @@ RSpec.describe DeployKeysProject do
     it { is_expected.to validate_presence_of(:deploy_key) }
   end
 
+  describe '.with_deploy_keys' do
+    subject(:scoped_query) { described_class.with_deploy_keys.last }
+
+    it 'includes deploy_keys in query' do
+      project = create(:project)
+      create(:deploy_keys_project, project: project, deploy_key: create(:deploy_key))
+
+      includes_query_count = ActiveRecord::QueryRecorder.new { scoped_query }.count
+      deploy_key_query_count = ActiveRecord::QueryRecorder.new { scoped_query.deploy_key }.count
+
+      expect(includes_query_count).to eq(2)
+      expect(deploy_key_query_count).to eq(0)
+    end
+  end
+
   describe "Destroying" do
     let(:project)     { create(:project) }
     subject           { create(:deploy_keys_project, project: project) }

spec/serializers/deploy_key_entity_spec.rb

@@ -9,8 +9,9 @@ RSpec.describe DeployKeyEntity do
   let(:project) { create(:project, :internal)}
   let(:project_private) { create(:project, :private)}
   let(:deploy_key) { create(:deploy_key) }
+  let(:options) { { user: user } }
 
-  let(:entity) { described_class.new(deploy_key, user: user) }
+  let(:entity) { described_class.new(deploy_key, options) }
 
   before do
     project.deploy_keys << deploy_key
@@ -74,4 +75,42 @@ RSpec.describe DeployKeyEntity do
       it { expect(entity_public.as_json).to include(can_edit: true) }
     end
   end
+
+  describe 'with_owner option' do
+    it 'does not return an owner payload when it is set to false' do
+      options[:with_owner] = false
+
+      payload = entity.as_json
+
+      expect(payload[:owner]).not_to be_present
+    end
+
+    describe 'when with_owner is set to true' do
+      before do
+        options[:with_owner] = true
+      end
+
+      it 'returns an owner payload' do
+        payload = entity.as_json
+
+        expect(payload[:owner]).to be_present
+        expect(payload[:owner].keys).to include(:id, :name, :username, :avatar_url)
+      end
+
+      it 'does not return an owner if current_user cannot read the owner' do
+        allow(Ability).to receive(:allowed?).and_call_original
+        allow(Ability).to receive(:allowed?).with(options[:user], :read_user, deploy_key.user).and_return(false)
+
+        payload = entity.as_json
+
+        expect(payload[:owner]).to be_nil
+      end
+    end
+  end
+
+  it 'does not return an owner payload with_owner option not passed in' do
+    payload = entity.as_json
+
+    expect(payload[:owner]).not_to be_present
+  end
 end

spec/services/deploy_keys/collect_keys_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe DeployKeys::CollectKeysService do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project, :private) }
+
+  subject { DeployKeys::CollectKeysService.new(project, user) }
+
+  before do
+    project&.add_developer(user)
+  end
+
+  context 'when no project is passed in' do
+    let(:project) { nil }
+
+    it 'returns an empty Array' do
+      expect(subject.execute).to be_empty
+    end
+  end
+
+  context 'when no user is passed in' do
+    let(:user) { nil }
+
+    it 'returns an empty Array' do
+      expect(subject.execute).to be_empty
+    end
+  end
+
+  context 'when a project is passed in' do
+    let_it_be(:deploy_keys_project) { create(:deploy_keys_project, :write_access, project: project) }
+    let_it_be(:deploy_key) { deploy_keys_project.deploy_key }
+
+    it 'only returns deploy keys with write access' do
+      create(:deploy_keys_project, project: project)
+
+      expect(subject.execute).to contain_exactly(deploy_key)
+    end
+
+    it 'returns deploy keys only for this project' do
+      other_project = create(:project)
+      create(:deploy_keys_project, :write_access, project: other_project)
+
+      expect(subject.execute).to contain_exactly(deploy_key)
+    end
+  end
+
+  context 'when the user cannot read the project' do
+    before do
+      project.members.delete_all
+    end
+
+    it 'returns an empty Array' do
+      expect(subject.execute).to be_empty
+    end
+  end
+end