diff --git a/app/graphql/resolvers/ci/runner_jobs_resolver.rb b/app/graphql/resolvers/ci/runner_jobs_resolver.rb index 9fe25a4d13db5d8919546b49a7e927444a24477c..39908d8fd11332bb6c07a6585e71a281da3d689a 100644 --- a/app/graphql/resolvers/ci/runner_jobs_resolver.rb +++ b/app/graphql/resolvers/ci/runner_jobs_resolver.rb @@ -18,6 +18,8 @@ class RunnerJobsResolver < BaseResolver alias_method :runner, :object def resolve_with_lookahead(statuses: nil) + context[:job_field_authorization] = :read_build # Instruct JobType to perform field-level authorization + jobs = ::Ci::JobsFinder.new(current_user: current_user, runner: runner, params: { scope: statuses }).execute apply_lookahead(jobs) @@ -30,7 +32,7 @@ def preloads previous_stage_jobs_or_needs: [:needs, :pipeline], artifacts: [:job_artifacts], pipeline: [:user], - project: [{ project: [:route, { namespace: [:route] }] }], + project: [{ project: [:route, { namespace: [:route] }, :project_feature] }], detailed_status: [ :metadata, { pipeline: [:merge_request] }, diff --git a/app/graphql/types/ci/job_base_field.rb b/app/graphql/types/ci/job_base_field.rb new file mode 100644 index 0000000000000000000000000000000000000000..f5bdd2260b554bbdd07cc5dc4144f6c30a3be18e --- /dev/null +++ b/app/graphql/types/ci/job_base_field.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Types + module Ci + # JobBaseField ensures that only allow-listed fields can be returned without a permission check. + # All other fields go through a permissions check based on the :job_field_authorization value passed in the context. + # rubocop: disable Graphql/AuthorizeTypes + class JobBaseField < ::Types::BaseField + PUBLIC_FIELDS = %i[allow_failure duration id kind status created_at finished_at queued_at queued_duration + updated_at runner short_sha].freeze + + def authorized?(object, args, ctx) + current_user = ctx[:current_user] + permission = ctx[:job_field_authorization] + + if permission.nil? || + PUBLIC_FIELDS.include?(ctx[:current_field].original_name) || + current_user.can?(permission, object) + return super + end + + false + end + end + # rubocop: enable Graphql/AuthorizeTypes + end +end diff --git a/app/graphql/types/ci/job_type.rb b/app/graphql/types/ci/job_type.rb index 22eb32993c5c2d166587d231d2cdb1f212029d16..976103e1510ac30d71869e30f2b7c070db3e864f 100644 --- a/app/graphql/types/ci/job_type.rb +++ b/app/graphql/types/ci/job_type.rb @@ -8,6 +8,7 @@ class JobType < BaseObject graphql_name 'CiJob' present_using ::Ci::BuildPresenter + field_class Types::Ci::JobBaseField connection_type_class Types::LimitedCountableConnectionType diff --git a/spec/features/groups/group_runners_spec.rb b/spec/features/groups/group_runners_spec.rb index 2a6f79a5d0f2c2320e385f40f834f10792d952e1..e15374a8629022fd3306df5e2f20d006fbf97c39 100644 --- a/spec/features/groups/group_runners_spec.rb +++ b/spec/features/groups/group_runners_spec.rb @@ -249,7 +249,7 @@ create(:ci_runner, :group, groups: [group], description: 'runner-foo') end - let_it_be(:group_runner_job) { create(:ci_build, runner: group_runner) } + let_it_be(:group_runner_job) { create(:ci_build, runner: group_runner, project: project) } context 'when logged in as group maintainer' do before do diff --git a/spec/graphql/types/ci/job_base_field_spec.rb b/spec/graphql/types/ci/job_base_field_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..e9b1407d249a5e1c1a4d12a31d94f76b62e5a47a --- /dev/null +++ b/spec/graphql/types/ci/job_base_field_spec.rb @@ -0,0 +1,144 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Types::Ci::JobBaseField, feature_category: :runner_fleet do + describe 'authorized?' do + let_it_be(:current_user) { create(:user) } + + let(:object) { double } + let(:ctx) { { current_user: current_user, current_field: current_field } } + let(:current_field) { instance_double(described_class, original_name: current_field_name.to_sym) } + let(:args) { {} } + + subject(:field) do + described_class.new(name: current_field_name, type: GraphQL::Types::String, null: true, **args) + end + + context 'when :job_field_authorization is specified' do + let(:ctx) { { current_user: current_user, current_field: current_field, job_field_authorization: :foo } } + + context 'with public field' do + using RSpec::Parameterized::TableSyntax + + where(:current_field_name) do + %i[allow_failure duration id kind status created_at finished_at queued_at queued_duration updated_at runner + short_sha] + end + + with_them do + it 'returns true without authorizing' do + is_expected.to be_authorized(object, nil, ctx) + end + end + end + + context 'with private field' do + let(:current_field_name) { 'private_field' } + + context 'when permission is not allowed' do + it 'returns false' do + expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(false) + + is_expected.not_to be_authorized(object, nil, ctx) + end + end + + context 'when permission is allowed' do + it 'returns true' do + expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true) + + is_expected.to be_authorized(object, nil, ctx) + end + end + end + end + + context 'when :job_field_authorization is not specified' do + let(:current_field_name) { 'status' } + + it 'defaults to true' do + is_expected.to be_authorized(object, nil, ctx) + end + + context 'when field is authorized' do + let(:args) { { authorize: :foo } } + + it 'tests the field authorization' do + expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(false) + + expect(field).not_to be_authorized(object, nil, ctx) + end + + it 'tests the field authorization, if provided, when it succeeds' do + expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true) + + expect(field).to be_authorized(object, nil, ctx) + end + end + + context 'with field resolver' do + let(:resolver) { Class.new } + let(:args) { { resolver_class: resolver } } + + it 'only tests the resolver authorization if it authorizes_object?' do + is_expected.to be_authorized(object, nil, ctx) + end + + context 'when resolver authorizes object' do + let(:resolver) do + Class.new do + include Gitlab::Graphql::Authorize::AuthorizeResource + + authorizes_object! + end + end + + it 'tests the resolver authorization, if provided' do + expect(resolver).to receive(:authorized?).with(object, ctx).and_return(false) + + expect(field).not_to be_authorized(object, nil, ctx) + end + + context 'when field is authorized' do + let(:args) { { authorize: :foo, resolver_class: resolver } } + + it 'tests field authorization before resolver authorization, when field auth fails' do + expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(false) + expect(resolver).not_to receive(:authorized?) + + expect(field).not_to be_authorized(object, nil, ctx) + end + + it 'tests field authorization before resolver authorization, when field auth succeeds' do + expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true) + expect(resolver).to receive(:authorized?).with(object, ctx).and_return(false) + + expect(field).not_to be_authorized(object, nil, ctx) + end + end + end + end + end + end + + describe '#resolve' do + context 'when late_extensions is given' do + it 'registers the late extensions after the regular extensions' do + extension_class = Class.new(GraphQL::Schema::Field::ConnectionExtension) + field = described_class.new(name: 'private_field', type: GraphQL::Types::String.connection_type, + null: true, late_extensions: [extension_class]) + + expect(field.extensions.last.class).to be(extension_class) + end + end + end + + include_examples 'Gitlab-style deprecations' do + def subject(args = {}) + base_args = { name: 'private_field', type: GraphQL::Types::String, null: true } + + described_class.new(**base_args.merge(args)) + end + end +end diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb index 3cfb98c57fdfb507be892136414ae444580d1874..3d7020b03b7361d4275085d439ca8a3b65f93c87 100644 --- a/spec/requests/api/graphql/ci/runner_spec.rb +++ b/spec/requests/api/graphql/ci/runner_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe 'Query.runner(id)', feature_category: :runner_fleet do +RSpec.describe 'Query.runner(id)', :freeze_time, feature_category: :runner_fleet do include GraphqlHelpers let_it_be(:user) { create(:user, :admin) } @@ -228,7 +228,7 @@ end end - context 'with build running', :freeze_time do + context 'with build running' do let!(:pipeline) { create(:ci_pipeline, project: project1) } let!(:runner_manager) do create(:ci_runner_machine, @@ -357,6 +357,77 @@ ) end end + + describe 'jobs' do + let(:query) do + %( + query { + runner(id: "#{project_runner.to_global_id}") { #{runner_query_fragment} } + } + ) + end + + context 'with a job from a non-owned project' do + let(:runner_query_fragment) do + %( + id + jobs { + nodes { + id status shortSha finishedAt duration queuedDuration tags webPath + project { id } + runner { id } + } + } + ) + end + + let_it_be(:owned_project_owner) { create(:user) } + let_it_be(:owned_project) { create(:project) } + let_it_be(:other_project) { create(:project) } + let_it_be(:project_runner) { create(:ci_runner, :project_type, projects: [other_project, owned_project]) } + let_it_be(:owned_project_pipeline) { create(:ci_pipeline, project: owned_project) } + let_it_be(:other_project_pipeline) { create(:ci_pipeline, project: other_project) } + let_it_be(:owned_build) do + create(:ci_build, :running, runner: project_runner, pipeline: owned_project_pipeline, + tag_list: %i[a b c], created_at: 1.hour.ago, started_at: 59.minutes.ago, finished_at: 30.minutes.ago) + end + + let_it_be(:other_build) do + create(:ci_build, :success, runner: project_runner, pipeline: other_project_pipeline, + tag_list: %i[d e f], created_at: 30.minutes.ago, started_at: 19.minutes.ago, finished_at: 1.minute.ago) + end + + before_all do + owned_project.add_owner(owned_project_owner) + end + + it 'returns empty values for sensitive fields in non-owned jobs' do + post_graphql(query, current_user: owned_project_owner) + + jobs_data = graphql_data_at(:runner, :jobs, :nodes) + expect(jobs_data).not_to be_nil + expect(jobs_data).to match([ + a_graphql_entity_for(other_build, + status: other_build.status.upcase, + project: nil, tags: nil, web_path: nil, + runner: a_graphql_entity_for(project_runner), + short_sha: other_build.short_sha, finished_at: other_build.finished_at&.iso8601, + duration: a_value_within(0.001).of(other_build.duration), + queued_duration: a_value_within(0.001).of((other_build.started_at - other_build.queued_at).to_f)), + a_graphql_entity_for(owned_build, + status: owned_build.status.upcase, + project: a_graphql_entity_for(owned_project), + tags: owned_build.tag_list.map(&:to_s), + web_path: ::Gitlab::Routing.url_helpers.project_job_path(owned_project, owned_build), + runner: a_graphql_entity_for(project_runner), + short_sha: owned_build.short_sha, + finished_at: owned_build.finished_at&.iso8601, + duration: a_value_within(0.001).of(owned_build.duration), + queued_duration: a_value_within(0.001).of((owned_build.started_at - owned_build.queued_at).to_f)) + ]) + end + end + end end describe 'for inactive runner' do @@ -501,8 +572,14 @@ end describe 'for runner with status' do - let_it_be(:stale_runner) { create(:ci_runner, description: 'Stale runner 1', created_at: 3.months.ago) } - let_it_be(:never_contacted_instance_runner) { create(:ci_runner, description: 'Missing runner 1', created_at: 1.month.ago, contacted_at: nil) } + let_it_be(:stale_runner) do + create(:ci_runner, description: 'Stale runner 1', + created_at: (3.months + 1.second).ago, contacted_at: (3.months + 1.second).ago) + end + + let_it_be(:never_contacted_instance_runner) do + create(:ci_runner, description: 'Missing runner 1', created_at: 1.month.ago, contacted_at: nil) + end let(:query) do %(