From 222fda90362a3be9e54323af32234d038b99908d Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 29 Jun 2022 14:11:15 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@15-1-stable-ee --- app/finders/ci/runner_jobs_finder.rb | 12 ++++++- app/models/ci/project_mirror.rb | 2 ++ app/models/user.rb | 14 +++++++- doc/api/runners.md | 3 +- lib/api/ci/runners.rb | 2 +- spec/finders/ci/runner_jobs_finder_spec.rb | 53 ++++++++++++++++++++++++++++-- spec/models/user_spec.rb | 35 ++++++++++++++++++++ spec/requests/api/ci/runners_spec.rb | 17 ++++++++++ 8 files changed, 132 insertions(+), 6 deletions(-) diff --git a/app/finders/ci/runner_jobs_finder.rb b/app/finders/ci/runner_jobs_finder.rb index 9dc3c2a2427..b659eda6646 100644 --- a/app/finders/ci/runner_jobs_finder.rb +++ b/app/finders/ci/runner_jobs_finder.rb @@ -6,19 +6,29 @@ module Ci ALLOWED_INDEXED_COLUMNS = %w[id].freeze - def initialize(runner, params = {}) + def initialize(runner, current_user, params = {}) @runner = runner + @user = current_user @params = params end def execute items = @runner.builds + items = by_permission(items) items = by_status(items) sort_items(items) end private + # rubocop: disable CodeReuse/ActiveRecord + def by_permission(items) + return items if @user.can_read_all_resources? + + items.for_project(@user.authorized_project_mirrors(Gitlab::Access::REPORTER).select(:project_id)) + end + # rubocop: enable CodeReuse/ActiveRecord + # rubocop: disable CodeReuse/ActiveRecord def by_status(items) return items unless Ci::HasStatus::AVAILABLE_STATUSES.include?(params[:status]) diff --git a/app/models/ci/project_mirror.rb b/app/models/ci/project_mirror.rb index 9000d1791a6..15a161d5b7c 100644 --- a/app/models/ci/project_mirror.rb +++ b/app/models/ci/project_mirror.rb @@ -4,6 +4,8 @@ module Ci # This model represents a shadow table of the main database's projects table. # It allows us to navigate the project and namespace hierarchy on the ci database. class ProjectMirror < ApplicationRecord + include FromUnion + belongs_to :project scope :by_namespace_id, -> (namespace_id) { where(namespace_id: namespace_id) } diff --git a/app/models/user.rb b/app/models/user.rb index c86fb56795c..40096dfa411 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1657,6 +1657,14 @@ class User < ApplicationRecord true end + def authorized_project_mirrors(level) + projects = Ci::ProjectMirror.by_project_id(ci_project_mirrors_for_project_members(level)) + + namespace_projects = Ci::ProjectMirror.by_namespace_id(ci_namespace_mirrors_for_group_members(level).select(:namespace_id)) + + Ci::ProjectMirror.from_union([projects, namespace_projects]) + end + def ci_owned_runners @ci_owned_runners ||= begin Ci::Runner @@ -2113,6 +2121,10 @@ class User < ApplicationRecord end # rubocop: enable CodeReuse/ServiceClass + def ci_project_mirrors_for_project_members(level) + project_members.where('access_level >= ?', level).pluck(:source_id) + end + def notification_email_verified return if notification_email.blank? || temp_oauth_email? @@ -2250,7 +2262,7 @@ class User < ApplicationRecord end def ci_owned_project_runners_from_project_members - project_ids = project_members.where('access_level >= ?', Gitlab::Access::MAINTAINER).pluck(:source_id) + project_ids = ci_project_mirrors_for_project_members(Gitlab::Access::MAINTAINER) Ci::Runner .joins(:runner_projects) diff --git a/doc/api/runners.md b/doc/api/runners.md index 2b31c1cc064..8af388a2b74 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -359,7 +359,8 @@ and will be removed in [GitLab 16.0](https://gitlab.com/gitlab-org/gitlab/-/issu > [Introduced](https://gitlab.com/gitlab-org/gitlab-foss/-/merge_requests/15432) in GitLab 10.3. -List jobs that are being processed or were processed by specified runner. +List jobs that are being processed or were processed by the specified runner. The list of jobs is limited +to projects where the user has at least the Reporter role. ```plaintext GET /runners/:id/jobs diff --git a/lib/api/ci/runners.rb b/lib/api/ci/runners.rb index 7863cfd1e79..06bfee59140 100644 --- a/lib/api/ci/runners.rb +++ b/lib/api/ci/runners.rb @@ -128,7 +128,7 @@ module API runner = get_runner(params[:id]) authenticate_list_runners_jobs!(runner) - jobs = ::Ci::RunnerJobsFinder.new(runner, params).execute + jobs = ::Ci::RunnerJobsFinder.new(runner, current_user, params).execute present paginate(jobs), with: Entities::Ci::JobBasicWithProject end diff --git a/spec/finders/ci/runner_jobs_finder_spec.rb b/spec/finders/ci/runner_jobs_finder_spec.rb index 3569582d70f..755b21ec08f 100644 --- a/spec/finders/ci/runner_jobs_finder_spec.rb +++ b/spec/finders/ci/runner_jobs_finder_spec.rb @@ -5,12 +5,17 @@ require 'spec_helper' RSpec.describe Ci::RunnerJobsFinder do let(:project) { create(:project) } let(:runner) { create(:ci_runner, :instance) } + let(:user) { create(:user) } + let(:params) { {} } - subject { described_class.new(runner, params).execute } + subject { described_class.new(runner, user, params).execute } + + before do + project.add_developer(user) + end describe '#execute' do context 'when params is empty' do - let(:params) { {} } let!(:job) { create(:ci_build, runner: runner, project: project) } let!(:job1) { create(:ci_build, project: project) } @@ -20,6 +25,50 @@ RSpec.describe Ci::RunnerJobsFinder do end end + context 'when the user has guest access' do + it 'does not returns jobs the user does not have permission to see' do + another_project = create(:project) + job = create(:ci_build, runner: runner, project: another_project) + + another_project.add_guest(user) + + is_expected.not_to match_array(job) + end + end + + context 'when the user has permission to read all resources' do + let(:user) { create(:user, :admin) } + + it 'returns all the jobs assigned to a runner' do + jobs = create_list(:ci_build, 5, runner: runner, project: project) + + is_expected.to match_array(jobs) + end + end + + context 'when the user has different access levels in different projects' do + it 'returns only the jobs the user has permission to see' do + guest_project = create(:project) + reporter_project = create(:project) + + _guest_jobs = create_list(:ci_build, 2, runner: runner, project: guest_project) + reporter_jobs = create_list(:ci_build, 3, runner: runner, project: reporter_project) + + guest_project.add_guest(user) + reporter_project.add_reporter(user) + + is_expected.to match_array(reporter_jobs) + end + end + + context 'when the user has reporter access level or greater' do + it 'returns jobs assigned to the Runner that the user has accesss to' do + jobs = create_list(:ci_build, 3, runner: runner, project: project) + + is_expected.to match_array(jobs) + end + end + context 'when params contains status' do Ci::HasStatus::AVAILABLE_STATUSES.each do |target_status| context "when status is #{target_status}" do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index dcf6b224009..abc02dd1f55 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4057,6 +4057,41 @@ RSpec.describe User do end end + describe '#authorized_project_mirrors' do + it 'returns project mirrors where the user has access equal to or above the given level' do + guest_project = create(:project) + reporter_project = create(:project) + maintainer_project = create(:project) + + guest_group = create(:group) + reporter_group = create(:group) + maintainer_group = create(:group) + + _guest_group_project = create(:project, group: guest_group) + reporter_group_project = create(:project, group: reporter_group) + maintainer_group_project = create(:project, group: maintainer_group) + + user = create(:user) + + guest_project.add_guest(user) + reporter_project.add_reporter(user) + maintainer_project.add_maintainer(user) + + guest_group.add_guest(user) + reporter_group.add_reporter(user) + maintainer_group.add_maintainer(user) + + project_mirrors = user.authorized_project_mirrors(Gitlab::Access::REPORTER) + + expect(project_mirrors.pluck(:project_id)).to contain_exactly( + reporter_group_project.id, + maintainer_group_project.id, + reporter_project.id, + maintainer_project.id + ) + end + end + shared_context '#ci_owned_runners' do let(:user) { create(:user) } diff --git a/spec/requests/api/ci/runners_spec.rb b/spec/requests/api/ci/runners_spec.rb index 3000bdc2ce7..31b85a0b1d6 100644 --- a/spec/requests/api/ci/runners_spec.rb +++ b/spec/requests/api/ci/runners_spec.rb @@ -804,6 +804,23 @@ RSpec.describe API::Ci::Runners do expect(json_response).to be_an(Array) expect(json_response.length).to eq(2) end + + context 'when user does not have authorization to see all jobs' do + it 'shows only jobs it has permission to see' do + create(:ci_build, :running, runner: two_projects_runner, project: project) + create(:ci_build, :running, runner: two_projects_runner, project: project2) + + project.add_guest(user2) + project2.add_maintainer(user2) + get api("/runners/#{two_projects_runner.id}/jobs", user2) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to include_pagination_headers + + expect(json_response).to be_an(Array) + expect(json_response.length).to eq(1) + end + end end context 'when valid status is provided' do -- cgit v1.2.3