From 80ace32006965d7074876737f864b521ef3f2707 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Mon, 21 Aug 2023 14:52:43 +0200
Subject: [PATCH 1/7] GraphQL: Mask job fields when requested for a non-owned
 runner

Changelog: fixed
---
 .../resolvers/ci/runner_jobs_resolver.rb      |  2 +
 app/graphql/types/ci/job_base_field.rb        | 22 ++++++
 app/graphql/types/ci/job_type.rb              |  1 +
 spec/requests/api/graphql/ci/runner_spec.rb   | 73 +++++++++++++++++++
 4 files changed, 98 insertions(+)
 create mode 100644 app/graphql/types/ci/job_base_field.rb

diff --git a/app/graphql/resolvers/ci/runner_jobs_resolver.rb b/app/graphql/resolvers/ci/runner_jobs_resolver.rb
index 9fe25a4d13db5d..00e08ff4ca8773 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)
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 00000000000000..da5cb6b4886a1f
--- /dev/null
+++ b/app/graphql/types/ci/job_base_field.rb
@@ -0,0 +1,22 @@
+# 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)
+        return super if ctx[:job_field_authorization].nil?
+        return super if PUBLIC_FIELDS.include?(ctx[:current_field].original_name)
+        return super if ctx[:current_user].can?(ctx[:job_field_authorization], object)
+
+        nil
+      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 22eb32993c5c2d..976103e1510ac3 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/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb
index 3cfb98c57fdfb5..7603c256cc5985 100644
--- a/spec/requests/api/graphql/ci/runner_spec.rb
+++ b/spec/requests/api/graphql/ci/runner_spec.rb
@@ -357,6 +357,79 @@
         )
       end
     end
+
+    describe 'jobs' do
+      let(:query) do
+        %(
+          query {
+            runner(id: "#{runner.to_global_id}") { #{runner_query_fragment} }
+          }
+        )
+      end
+
+      context 'with job from non-owned project', :freeze_time do
+        let(:runner_query_fragment) do
+          %(
+            id
+            jobs {
+              nodes {
+                id status shortSha finishedAt duration queuedDuration tags webPath
+                project { id }
+                runner { id }
+              }
+            }
+          )
+        end
+
+        let(:runner) { project_runner }
+
+        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: %w[a b c],
+              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
-- 
GitLab


From 595a8fabff09e8a2f910941fd477f962737b1966 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Mon, 21 Aug 2023 18:17:04 +0200
Subject: [PATCH 2/7] Preload project_feature

---
 app/graphql/resolvers/ci/runner_jobs_resolver.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/graphql/resolvers/ci/runner_jobs_resolver.rb b/app/graphql/resolvers/ci/runner_jobs_resolver.rb
index 00e08ff4ca8773..39908d8fd11332 100644
--- a/app/graphql/resolvers/ci/runner_jobs_resolver.rb
+++ b/app/graphql/resolvers/ci/runner_jobs_resolver.rb
@@ -32,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] },
-- 
GitLab


From 1ce50ecbabb64d14647c2e26adb16505cedeef26 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Tue, 22 Aug 2023 12:17:27 +0200
Subject: [PATCH 3/7] Fix existing test setup

---
 spec/features/groups/group_runners_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/features/groups/group_runners_spec.rb b/spec/features/groups/group_runners_spec.rb
index 2a6f79a5d0f2c2..e15374a8629022 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
-- 
GitLab


From 88119abb5a848d54cf55b1522e835dce545e7899 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Tue, 22 Aug 2023 14:11:30 +0200
Subject: [PATCH 4/7] Address MR review comment

---
 spec/requests/api/graphql/ci/runner_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb
index 7603c256cc5985..a38dee1ef50fa6 100644
--- a/spec/requests/api/graphql/ci/runner_spec.rb
+++ b/spec/requests/api/graphql/ci/runner_spec.rb
@@ -367,7 +367,7 @@
         )
       end
 
-      context 'with job from non-owned project', :freeze_time do
+      context 'with a job from a non-owned project', :freeze_time do
         let(:runner_query_fragment) do
           %(
             id
@@ -419,7 +419,7 @@
             a_graphql_entity_for(owned_build,
               status: owned_build.status.upcase,
               project: a_graphql_entity_for(owned_project),
-              tags: %w[a b c],
+              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,
-- 
GitLab


From 764dea0bc5987049eb68937de7f408120d06ac81 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Wed, 23 Aug 2023 14:08:11 +0200
Subject: [PATCH 5/7] Address MR review comments

---
 app/graphql/types/ci/job_base_field.rb       |  10 +-
 spec/graphql/types/ci/job_base_field_spec.rb | 112 +++++++++++++++++++
 spec/requests/api/graphql/ci/runner_spec.rb  |  10 +-
 3 files changed, 122 insertions(+), 10 deletions(-)
 create mode 100644 spec/graphql/types/ci/job_base_field_spec.rb

diff --git a/app/graphql/types/ci/job_base_field.rb b/app/graphql/types/ci/job_base_field.rb
index da5cb6b4886a1f..cc63a53bfc8c06 100644
--- a/app/graphql/types/ci/job_base_field.rb
+++ b/app/graphql/types/ci/job_base_field.rb
@@ -10,11 +10,13 @@ class JobBaseField < ::Types::BaseField
         updated_at runner short_sha].freeze
 
       def authorized?(object, args, ctx)
-        return super if ctx[:job_field_authorization].nil?
-        return super if PUBLIC_FIELDS.include?(ctx[:current_field].original_name)
-        return super if ctx[:current_user].can?(ctx[:job_field_authorization], object)
+        current_user = ctx[:current_user]
+        field_name = ctx[:current_field].original_name
+        permission = ctx[:job_field_authorization]
 
-        nil
+        return super if permission.nil? || PUBLIC_FIELDS.include?(field_name) || current_user.can?(permission, object)
+
+        false
       end
     end
     # rubocop: enable Graphql/AuthorizeTypes
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 00000000000000..cbfd28932d3598
--- /dev/null
+++ b/spec/graphql/types/ci/job_base_field_spec.rb
@@ -0,0 +1,112 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Types::Ci::JobBaseField do
+  describe 'authorized?' do
+    let(:object) { double }
+    let(:current_user) { nil }
+    let(:ctx) { { current_user: current_user, current_field: current_field } }
+    let(:current_field) { double(original_name: current_field_name)}
+    let(:current_field_name) { :test }
+
+    it 'defaults to true' do
+      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true)
+
+      expect(field).to be_authorized(object, nil, ctx)
+    end
+
+    it 'tests the field authorization, if provided' do
+      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, authorize: :foo)
+
+      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
+      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, authorize: :foo)
+
+      expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true)
+
+      expect(field).to be_authorized(object, nil, ctx)
+    end
+
+    it 'only tests the resolver authorization if it authorizes_object?' do
+      resolver = Class.new
+
+      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
+                                  resolver_class: resolver)
+
+      expect(field).to be_authorized(object, nil, ctx)
+    end
+
+    it 'tests the resolver authorization, if provided' do
+      resolver = Class.new do
+        include Gitlab::Graphql::Authorize::AuthorizeResource
+
+        authorizes_object!
+      end
+
+      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
+                                  resolver_class: resolver)
+
+      expect(resolver).to receive(:authorized?).with(object, ctx).and_return(false)
+
+      expect(field).not_to be_authorized(object, nil, ctx)
+    end
+
+    it 'tests field authorization before resolver authorization, when field auth fails' do
+      resolver = Class.new do
+        include Gitlab::Graphql::Authorize::AuthorizeResource
+
+        authorizes_object!
+      end
+
+      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
+                                  authorize: :foo,
+                                  resolver_class: resolver)
+
+      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
+      resolver = Class.new do
+        include Gitlab::Graphql::Authorize::AuthorizeResource
+
+        authorizes_object!
+      end
+
+      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
+                                  authorize: :foo,
+                                  resolver_class: resolver)
+
+      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
+
+  describe '#resolve' do
+    context "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: 'test', 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: 'test', 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 a38dee1ef50fa6..dff3311261a9e7 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,
@@ -362,12 +362,12 @@
       let(:query) do
         %(
           query {
-            runner(id: "#{runner.to_global_id}") { #{runner_query_fragment} }
+            runner(id: "#{project_runner.to_global_id}") { #{runner_query_fragment} }
           }
         )
       end
 
-      context 'with a job from a non-owned project', :freeze_time do
+      context 'with a job from a non-owned project' do
         let(:runner_query_fragment) do
           %(
             id
@@ -381,8 +381,6 @@
           )
         end
 
-        let(:runner) { project_runner }
-
         let_it_be(:owned_project_owner) { create(:user) }
         let_it_be(:owned_project) { create(:project) }
         let_it_be(:other_project) { create(:project) }
-- 
GitLab


From 828fa2f0c69ab8532aa6eeaa2f018abd86bf98ec Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Wed, 23 Aug 2023 14:45:04 +0200
Subject: [PATCH 6/7] Test JobBaseField

Test JobBaseField
---
 app/graphql/types/ci/job_base_field.rb       |   7 +-
 spec/graphql/types/ci/job_base_field_spec.rb | 146 +++++++++++--------
 2 files changed, 94 insertions(+), 59 deletions(-)

diff --git a/app/graphql/types/ci/job_base_field.rb b/app/graphql/types/ci/job_base_field.rb
index cc63a53bfc8c06..f5bdd2260b554b 100644
--- a/app/graphql/types/ci/job_base_field.rb
+++ b/app/graphql/types/ci/job_base_field.rb
@@ -11,10 +11,13 @@ class JobBaseField < ::Types::BaseField
 
       def authorized?(object, args, ctx)
         current_user = ctx[:current_user]
-        field_name = ctx[:current_field].original_name
         permission = ctx[:job_field_authorization]
 
-        return super if permission.nil? || PUBLIC_FIELDS.include?(field_name) || current_user.can?(permission, object)
+        if permission.nil? ||
+            PUBLIC_FIELDS.include?(ctx[:current_field].original_name) ||
+            current_user.can?(permission, object)
+          return super
+        end
 
         false
       end
diff --git a/spec/graphql/types/ci/job_base_field_spec.rb b/spec/graphql/types/ci/job_base_field_spec.rb
index cbfd28932d3598..e9b1407d249a5e 100644
--- a/spec/graphql/types/ci/job_base_field_spec.rb
+++ b/spec/graphql/types/ci/job_base_field_spec.rb
@@ -2,100 +2,132 @@
 
 require 'spec_helper'
 
-RSpec.describe Types::Ci::JobBaseField do
+RSpec.describe Types::Ci::JobBaseField, feature_category: :runner_fleet do
   describe 'authorized?' do
+    let_it_be(:current_user) { create(:user) }
+
     let(:object) { double }
-    let(:current_user) { nil }
     let(:ctx) { { current_user: current_user, current_field: current_field } }
-    let(:current_field) { double(original_name: current_field_name)}
-    let(:current_field_name) { :test }
-
-    it 'defaults to true' do
-      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true)
+    let(:current_field) { instance_double(described_class, original_name: current_field_name.to_sym) }
+    let(:args) { {} }
 
-      expect(field).to be_authorized(object, nil, ctx)
+    subject(:field) do
+      described_class.new(name: current_field_name, type: GraphQL::Types::String, null: true, **args)
     end
 
-    it 'tests the field authorization, if provided' do
-      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, authorize: :foo)
+    context 'when :job_field_authorization is specified' do
+      let(:ctx) { { current_user: current_user, current_field: current_field, job_field_authorization: :foo } }
 
-      expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(false)
+      context 'with public field' do
+        using RSpec::Parameterized::TableSyntax
 
-      expect(field).not_to be_authorized(object, nil, ctx)
-    end
+        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
 
-    it 'tests the field authorization, if provided, when it succeeds' do
-      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true, authorize: :foo)
+        with_them do
+          it 'returns true without authorizing' do
+            is_expected.to be_authorized(object, nil, ctx)
+          end
+        end
+      end
 
-      expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true)
+      context 'with private field' do
+        let(:current_field_name) { 'private_field' }
 
-      expect(field).to be_authorized(object, nil, ctx)
-    end
+        context 'when permission is not allowed' do
+          it 'returns false' do
+            expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(false)
 
-    it 'only tests the resolver authorization if it authorizes_object?' do
-      resolver = Class.new
+            is_expected.not_to be_authorized(object, nil, ctx)
+          end
+        end
 
-      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
-                                  resolver_class: resolver)
+        context 'when permission is allowed' do
+          it 'returns true' do
+            expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true)
 
-      expect(field).to be_authorized(object, nil, ctx)
+            is_expected.to be_authorized(object, nil, ctx)
+          end
+        end
+      end
     end
 
-    it 'tests the resolver authorization, if provided' do
-      resolver = Class.new do
-        include Gitlab::Graphql::Authorize::AuthorizeResource
+    context 'when :job_field_authorization is not specified' do
+      let(:current_field_name) { 'status' }
 
-        authorizes_object!
+      it 'defaults to true' do
+        is_expected.to be_authorized(object, nil, ctx)
       end
 
-      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
-                                  resolver_class: resolver)
+      context 'when field is authorized' do
+        let(:args) { { authorize: :foo } }
 
-      expect(resolver).to receive(:authorized?).with(object, ctx).and_return(false)
+        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
+          expect(field).not_to be_authorized(object, nil, ctx)
+        end
 
-    it 'tests field authorization before resolver authorization, when field auth fails' do
-      resolver = Class.new do
-        include Gitlab::Graphql::Authorize::AuthorizeResource
+        it 'tests the field authorization, if provided, when it succeeds' do
+          expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true)
 
-        authorizes_object!
+          expect(field).to be_authorized(object, nil, ctx)
+        end
       end
 
-      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
-                                  authorize: :foo,
-                                  resolver_class: resolver)
+      context 'with field resolver' do
+        let(:resolver) { Class.new }
+        let(:args) { { resolver_class: resolver } }
 
-      expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(false)
-      expect(resolver).not_to receive(:authorized?)
+        it 'only tests the resolver authorization if it authorizes_object?' do
+          is_expected.to be_authorized(object, nil, ctx)
+        end
 
-      expect(field).not_to be_authorized(object, nil, ctx)
-    end
+        context 'when resolver authorizes object' do
+          let(:resolver) do
+            Class.new do
+              include Gitlab::Graphql::Authorize::AuthorizeResource
 
-    it 'tests field authorization before resolver authorization, when field auth succeeds' do
-      resolver = Class.new do
-        include Gitlab::Graphql::Authorize::AuthorizeResource
+              authorizes_object!
+            end
+          end
 
-        authorizes_object!
-      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
 
-      field = described_class.new(name: 'test', type: GraphQL::Types::String, null: true,
-                                  authorize: :foo,
-                                  resolver_class: resolver)
+          context 'when field is authorized' do
+            let(:args) { { authorize: :foo, resolver_class: resolver } }
 
-      expect(Ability).to receive(:allowed?).with(current_user, :foo, object).and_return(true)
-      expect(resolver).to receive(:authorized?).with(object, ctx).and_return(false)
+            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)
+              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 "late_extensions is given" 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: 'test', type: GraphQL::Types::String.connection_type, null: true, late_extensions: [extension_class])
+        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
@@ -104,7 +136,7 @@
 
   include_examples 'Gitlab-style deprecations' do
     def subject(args = {})
-      base_args = { name: 'test', type: GraphQL::Types::String, null: true }
+      base_args = { name: 'private_field', type: GraphQL::Types::String, null: true }
 
       described_class.new(**base_args.merge(args))
     end
-- 
GitLab


From b025d249948cb296f8db0185465a5c1fb83a6e55 Mon Sep 17 00:00:00 2001
From: Pedro Pombeiro <noreply@pedro.pombei.ro>
Date: Wed, 23 Aug 2023 17:27:19 +0200
Subject: [PATCH 7/7] Fix test broken by freeze_time

---
 spec/requests/api/graphql/ci/runner_spec.rb | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/spec/requests/api/graphql/ci/runner_spec.rb b/spec/requests/api/graphql/ci/runner_spec.rb
index dff3311261a9e7..3d7020b03b7361 100644
--- a/spec/requests/api/graphql/ci/runner_spec.rb
+++ b/spec/requests/api/graphql/ci/runner_spec.rb
@@ -572,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
       %(
-- 
GitLab