Commit d1ff11b5 authored by Etienne Baqué's avatar Etienne Baqué

Refactoring to ease dev on protected branches

Extracted code from big MR:
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/36345
parent 6fdcefb6
......@@ -53,9 +53,7 @@ class AutocompleteController < ApplicationController
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 })
render json: DeployKeySerializer.new.represent(DeployKey.with_write_access_for_project(project), { with_owner: true, user: current_user })
end
private
......
......@@ -7,8 +7,9 @@ class DeployKey < Key
has_many :deploy_keys_projects, inverse_of: :deploy_key, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent
has_many :projects, through: :deploy_keys_projects
scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where('deploy_keys_projects.project_id in (?)', projects) }
scope :are_public, -> { where(public: true) }
scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where(deploy_keys_projects: { project_id: projects }) }
scope :with_write_access, -> { joins(:deploy_keys_projects).where(deploy_keys_projects: { can_push: true }) }
scope :are_public, -> { where(public: true) }
scope :with_projects, -> { includes(deploy_keys_projects: { project: [:route, namespace: :route] }) }
ignore_column :can_push, remove_after: '2019-12-15', remove_with: '12.6'
......@@ -54,4 +55,11 @@ class DeployKey < Key
def projects_with_write_access
Project.with_route.where(id: deploy_keys_projects.with_write_access.select(:project_id))
end
def self.with_write_access_for_project(project, deploy_key: nil)
query = in_projects(project&.id).with_write_access
query = query.where(id: deploy_key.id) if deploy_key
query
end
end
......@@ -6,7 +6,6 @@ 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
......
# 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
......@@ -90,9 +90,13 @@ module Gitlab
def can_collaborate?(ref)
assert_project!
can_push? || branch_allows_collaboration_for?(ref)
end
def branch_allows_collaboration_for?(ref)
# Checking for an internal project or group to prevent an infinite loop:
# https://gitlab.com/gitlab-org/gitlab/issues/36805
can_push? || (!project.internal? && project.branch_allows_collaboration?(user, ref))
(!project.internal? && project.branch_allows_collaboration?(user, ref))
end
def permission_cache
......
......@@ -382,6 +382,17 @@ RSpec.describe AutocompleteController do
sign_in(user)
end
context 'and they cannot read the project' do
it 'returns a not found response' do
allow(Ability).to receive(:allowed?).and_call_original
allow(Ability).to receive(:allowed?).with(user, :read_project, project).and_return(false)
get(:deploy_keys_with_owners, params: { project_id: project.id })
expect(response).to have_gitlab_http_status(:not_found)
end
end
it 'renders the deploy key in a json payload, with its owner' do
get(:deploy_keys_with_owners, params: { project_id: project.id })
......
......@@ -40,4 +40,56 @@ RSpec.describe DeployKey, :mailer do
end
end
end
describe '.with_write_access_for_project' do
let_it_be(:project) { create(:project, :private) }
subject { described_class.with_write_access_for_project(project) }
context 'when no project is passed in' do
let(:project) { nil }
it { is_expected.to be_empty }
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).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).to contain_exactly(deploy_key)
end
context 'and a specific deploy key is passed in' do
subject { described_class.with_write_access_for_project(project, deploy_key: specific_deploy_key) }
context 'and this deploy key is not linked to the project' do
let(:specific_deploy_key) { create(:deploy_key) }
it { is_expected.to be_empty }
end
context 'and this deploy key has not write access to the project' do
let(:specific_deploy_key) { create(:deploy_key, deploy_keys_projects: [create(:deploy_keys_project, project: project)]) }
it { is_expected.to be_empty }
end
context 'and this deploy key has write access to the project' do
let(:specific_deploy_key) { create(:deploy_key, deploy_keys_projects: [create(:deploy_keys_project, :write_access, project: project)]) }
it { is_expected.to contain_exactly(specific_deploy_key) }
end
end
end
end
end
......@@ -13,21 +13,6 @@ 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) }
......
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment