summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2022-06-29 14:10:39 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2022-06-29 14:11:04 +0000
commit25344e300eb871a7ce61f734c5e8f47d3e2f3aae (patch)
tree2bb88a57b0ff4fe16ae02446fdf69c95a4f5b599
parent2059bbcc3ac8a3f05d625669bc9e03a126b24ff8 (diff)
downloadgitlab-ce-25344e300eb871a7ce61f734c5e8f47d3e2f3aae.tar.gz
Add latest changes from gitlab-org/security/gitlab@14-10-stable-ee
-rw-r--r--app/finders/ci/runner_jobs_finder.rb12
-rw-r--r--app/models/ci/project_mirror.rb2
-rw-r--r--app/models/user.rb14
-rw-r--r--doc/api/runners.md3
-rw-r--r--lib/api/ci/runners.rb2
-rw-r--r--spec/finders/ci/runner_jobs_finder_spec.rb53
-rw-r--r--spec/models/user_spec.rb35
-rw-r--r--spec/requests/api/ci/runners_spec.rb17
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,13 +6,15 @@ 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
@@ -20,6 +22,14 @@ module Ci
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 26d47de4f00..25ac394793b 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1646,6 +1646,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
if ci_owned_runners_cross_joins_fix_enabled?
@@ -2116,6 +2124,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?
@@ -2267,7 +2279,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 304f2494f70..6ecdd620d38 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 3c9e887e751..a4c6f9b6822 100644
--- a/lib/api/ci/runners.rb
+++ b/lib/api/ci/runners.rb
@@ -127,7 +127,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 bc425b15c6e..6fe4b3a815f 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -4045,6 +4045,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 d6ebc197ab0..587a6921087 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