diff --git a/app/controllers/concerns/workhorse_authorization.rb b/app/controllers/concerns/workhorse_authorization.rb index 648e6f409e6..f9b85944307 100644 --- a/app/controllers/concerns/workhorse_authorization.rb +++ b/app/controllers/concerns/workhorse_authorization.rb @@ -26,7 +26,7 @@ module WorkhorseAuthorization def file_is_valid?(file) return false unless file.is_a?(::UploadedFile) - file_extension_whitelist.include?(File.extname(file.original_filename).downcase.delete('.')) + file_extension_allowlist.include?(File.extname(file.original_filename).downcase.delete('.')) end def uploader_class @@ -37,7 +37,7 @@ module WorkhorseAuthorization raise NotImplementedError end - def file_extension_whitelist + def file_extension_allowlist ImportExportUploader::EXTENSION_ALLOWLIST end end diff --git a/app/controllers/projects/releases/evidences_controller.rb b/app/controllers/projects/releases/evidences_controller.rb index 1e2dbf8047c..41e2ce81eb8 100644 --- a/app/controllers/projects/releases/evidences_controller.rb +++ b/app/controllers/projects/releases/evidences_controller.rb @@ -20,7 +20,6 @@ module Projects private def authorize_read_release_evidence! - access_denied! unless Feature.enabled?(:release_evidence, project, default_enabled: true) access_denied! unless can?(current_user, :read_release_evidence, evidence) end diff --git a/app/models/deploy_key.rb b/app/models/deploy_key.rb index 8f5a713af3f..4ed38f578ee 100644 --- a/app/models/deploy_key.rb +++ b/app/models/deploy_key.rb @@ -7,12 +7,16 @@ 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 + + has_many :deploy_keys_projects_with_write_access, -> { with_write_access }, class_name: "DeployKeysProject" + has_many :projects_with_write_access, -> { includes(:route) }, class_name: 'Project', through: :deploy_keys_projects_with_write_access, source: :project has_many :protected_branch_push_access_levels, class_name: '::ProtectedBranch::PushAccessLevel' scope :in_projects, ->(projects) { joins(:deploy_keys_projects).where(deploy_keys_projects: { project_id: projects }) } scope :with_write_access, -> { joins(:deploy_keys_projects).merge(DeployKeysProject.with_write_access) } scope :are_public, -> { where(public: true) } scope :with_projects, -> { includes(deploy_keys_projects: { project: [:route, namespace: :route] }) } + scope :including_projects_with_write_access, -> { includes(:projects_with_write_access) } accepts_nested_attributes_for :deploy_keys_projects @@ -52,10 +56,6 @@ class DeployKey < Key end end - 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).with_write_access query = query.where(id: deploy_key) if deploy_key diff --git a/config/feature_flags/development/release_evidence.yml b/config/feature_flags/development/release_evidence.yml deleted file mode 100644 index 3ba5ed86c71..00000000000 --- a/config/feature_flags/development/release_evidence.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: release_evidence -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/26509 -rollout_issue_url: -milestone: '12.10' -type: development -group: group::release -default_enabled: true diff --git a/doc/api/deploy_keys.md b/doc/api/deploy_keys.md index bb719b5bc79..b244384bd6a 100644 --- a/doc/api/deploy_keys.md +++ b/doc/api/deploy_keys.md @@ -15,8 +15,16 @@ endpoint requires an administrator role and is not available on GitLab.com. GET /deploy_keys ``` +Supported attributes: + +| Attribute | Type | Required | Description | +|:------------|:---------|:---------|:----------------------| +| `public` | boolean | **{dotted-circle}** No | Only return deploy keys that are public. Defaults to `false`. | + +Example request: + ```shell -curl --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/deploy_keys" +curl --header "PRIVATE-TOKEN: " "https://gitlab.example.com/api/v4/deploy_keys?public=true" ``` Example response: @@ -27,13 +35,36 @@ Example response: "id": 1, "title": "Public key", "key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=", - "created_at": "2013-10-02T10:12:29Z" + "fingerprint": "7f:72:08:7d:0e:47:48:ec:37:79:b2:76:68:b5:87:65", + "created_at": "2013-10-02T10:12:29Z", + "projects_with_write_access": [ + { + "id": 73, + "description": null, + "name": "project2", + "name_with_namespace": "Sidney Jones / project2", + "path": "project2", + "path_with_namespace": "sidney_jones/project2", + "created_at": "2021-10-25T18:33:17.550Z" + }, + { + "id": 74, + "description": null, + "name": "project3", + "name_with_namespace": "Sidney Jones / project3", + "path": "project3", + "path_with_namespace": "sidney_jones/project3", + "created_at": "2021-10-25T18:33:17.666Z" + } + ] }, { "id": 3, "title": "Another Public key", "key": "ssh-rsa AAAAB3NzaC1yc2EAAAABJQAAAIEAiPWx6WM4lhHNedGfBpPJNPpZ7yKu+dnn1SJejgt4596k6YjzGGphH2TUxwKzxcKDKKezwkpfnxPkSMkuEspGRt/aZZ9wa++Oi7Qkr8prgHc4soW6NUlfDzpvZK2H5E7eQaSeP3SAwGmQKUFHCddNaP0L+hM7zhFNzjFvpaMgJw0=", - "created_at": "2013-10-02T11:12:29Z" + "fingerprint": "64:d3:73:d4:83:70:ab:41:96:68:d5:3d:a5:b0:34:ea", + "created_at": "2013-10-02T11:12:29Z", + "projects_with_write_access": [] } ] ``` diff --git a/lib/api/deploy_keys.rb b/lib/api/deploy_keys.rb index 9f0f569b711..0ab9fe6644c 100644 --- a/lib/api/deploy_keys.rb +++ b/lib/api/deploy_keys.rb @@ -23,11 +23,14 @@ module API desc 'Return all deploy keys' params do use :pagination + optional :public, type: Boolean, default: false, desc: "Only return deploy keys that are public" end get "deploy_keys" do authenticated_as_admin! - present paginate(DeployKey.all), with: Entities::DeployKey + deploy_keys = params[:public] ? DeployKey.are_public : DeployKey.all + + present paginate(deploy_keys.including_projects_with_write_access), with: Entities::DeployKey, include_projects_with_write_access: true end params do diff --git a/lib/api/entities/deploy_key.rb b/lib/api/entities/deploy_key.rb index ed922c24eda..e8537c4c677 100644 --- a/lib/api/entities/deploy_key.rb +++ b/lib/api/entities/deploy_key.rb @@ -4,6 +4,9 @@ module API module Entities class DeployKey < Entities::SSHKey expose :key + expose :fingerprint + + expose :projects_with_write_access, using: Entities::ProjectIdentity, if: -> (_, options) { options[:include_projects_with_write_access] } end end end diff --git a/spec/fixtures/api/schemas/public_api/v4/deploy_key.json b/spec/fixtures/api/schemas/public_api/v4/deploy_key.json new file mode 100644 index 00000000000..3dbdfcc95a1 --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/deploy_key.json @@ -0,0 +1,25 @@ +{ + "type": "object", + "required": [ + "id", + "title", + "created_at", + "expires_at", + "key", + "fingerprint", + "projects_with_write_access" + ], + "properties": { + "id": { "type": "integer" }, + "title": { "type": "string" }, + "created_at": { "type": "string", "format": "date-time" }, + "expires_at": { "type": ["string", "null"], "format": "date-time" }, + "key": { "type": "string" }, + "fingerprint": { "type": "string" }, + "projects_with_write_access": { + "type": "array", + "items": { "$ref": "project/identity.json" } + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/api/schemas/public_api/v4/deploy_keys.json b/spec/fixtures/api/schemas/public_api/v4/deploy_keys.json new file mode 100644 index 00000000000..82ddbdddbee --- /dev/null +++ b/spec/fixtures/api/schemas/public_api/v4/deploy_keys.json @@ -0,0 +1,4 @@ +{ + "type": "array", + "items": { "$ref": "deploy_key.json" } +} diff --git a/spec/models/deploy_key_spec.rb b/spec/models/deploy_key_spec.rb index fa78527e366..c22bad0e062 100644 --- a/spec/models/deploy_key_spec.rb +++ b/spec/models/deploy_key_spec.rb @@ -5,6 +5,17 @@ require 'spec_helper' RSpec.describe DeployKey, :mailer do describe "Associations" do it { is_expected.to have_many(:deploy_keys_projects) } + it do + is_expected.to have_many(:deploy_keys_projects_with_write_access) + .conditions(can_push: true) + .class_name('DeployKeysProject') + end + it do + is_expected.to have_many(:projects_with_write_access) + .class_name('Project') + .through(:deploy_keys_projects_with_write_access) + .source(:project) + end it { is_expected.to have_many(:projects) } it { is_expected.to have_many(:protected_branch_push_access_levels) } end diff --git a/spec/requests/api/deploy_keys_spec.rb b/spec/requests/api/deploy_keys_spec.rb index a01c66a311c..1daa7c38e04 100644 --- a/spec/requests/api/deploy_keys_spec.rb +++ b/spec/requests/api/deploy_keys_spec.rb @@ -8,8 +8,9 @@ RSpec.describe API::DeployKeys do let_it_be(:admin) { create(:admin) } let_it_be(:project) { create(:project, creator_id: user.id) } let_it_be(:project2) { create(:project, creator_id: user.id) } - - let(:deploy_key) { create(:deploy_key, public: true) } + let_it_be(:project3) { create(:project, creator_id: user.id) } + let_it_be(:deploy_key) { create(:deploy_key, public: true) } + let_it_be(:deploy_key_private) { create(:deploy_key, public: false) } let!(:deploy_keys_project) do create(:deploy_keys_project, project: project, deploy_key: deploy_key) @@ -33,13 +34,56 @@ RSpec.describe API::DeployKeys do end context 'when authenticated as admin' do + let_it_be(:pat) { create(:personal_access_token, user: admin) } + + def make_api_request(params = {}) + get api('/deploy_keys', personal_access_token: pat), params: params + end + it 'returns all deploy keys' do - get api('/deploy_keys', admin) + make_api_request expect(response).to have_gitlab_http_status(:ok) expect(response).to include_pagination_headers + expect(response).to match_response_schema('public_api/v4/deploy_keys') expect(json_response).to be_an Array - expect(json_response.first['id']).to eq(deploy_keys_project.deploy_key.id) + + expect(json_response[0]['id']).to eq(deploy_key.id) + expect(json_response[1]['id']).to eq(deploy_key_private.id) + end + + it 'avoids N+1 database queries', :use_sql_query_cache, :request_store do + create(:deploy_keys_project, :write_access, project: project2, deploy_key: deploy_key) + + control = ActiveRecord::QueryRecorder.new(skip_cached: false) { make_api_request } + + deploy_key2 = create(:deploy_key, public: true) + create(:deploy_keys_project, :write_access, project: project3, deploy_key: deploy_key2) + + expect { make_api_request }.not_to exceed_all_query_limit(control) + end + + context 'when `public` parameter is `true`' do + it 'only returns public deploy keys' do + make_api_request({ public: true }) + + expect(json_response.length).to eq(1) + expect(json_response.first['id']).to eq(deploy_key.id) + end + end + + context 'projects_with_write_access' do + let!(:deploy_keys_project2) { create(:deploy_keys_project, :write_access, project: project2, deploy_key: deploy_key) } + let!(:deploy_keys_project3) { create(:deploy_keys_project, :write_access, project: project3, deploy_key: deploy_key) } + + it 'returns projects with write access' do + make_api_request + + response_projects_with_write_access = json_response.first['projects_with_write_access'] + + expect(response_projects_with_write_access[0]['id']).to eq(project2.id) + expect(response_projects_with_write_access[1]['id']).to eq(project3.id) + end end end end @@ -58,6 +102,7 @@ RSpec.describe API::DeployKeys do expect(response).to include_pagination_headers expect(json_response).to be_an Array expect(json_response.first['title']).to eq(deploy_key.title) + expect(json_response.first).not_to have_key(:projects_with_write_access) end it 'returns multiple deploy keys without N + 1' do @@ -77,6 +122,7 @@ RSpec.describe API::DeployKeys do expect(response).to have_gitlab_http_status(:ok) expect(json_response['title']).to eq(deploy_key.title) + expect(json_response).not_to have_key(:projects_with_write_access) end it 'returns 404 Not Found with invalid ID' do