Merge branch...

Merge branch '329695-add-read_commit_status-field-authorization-to-pipeline-fields-that-can-return-jobs' into 'master'

Resolve "Add `:read_commit_status` field authorization to Pipeline fields that can return jobs" [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!60754

app/graphql/types/ci/pipeline_type.rb

@@ -60,12 +60,17 @@ module Types
       field :committed_at, Types::TimeType, null: true,
             description: "Timestamp of the pipeline's commit."
 
-      field :stages, Types::Ci::StageType.connection_type, null: true,
+      field :stages,
+            type: Types::Ci::StageType.connection_type,
+            null: true,
+            authorize: :read_commit_status,
             description: 'Stages of the pipeline.',
             extras: [:lookahead],
             resolver: Resolvers::Ci::PipelineStagesResolver
 
-      field :user, Types::UserType, null: true,
+      field :user,
+            type: Types::UserType,
+            null: true,
             description: 'Pipeline user.'
 
       field :retryable, GraphQL::BOOLEAN_TYPE,
@@ -81,12 +86,14 @@ module Types
       field :jobs,
             ::Types::Ci::JobType.connection_type,
             null: true,
+            authorize: :read_commit_status,
             description: 'Jobs belonging to the pipeline.',
             resolver: ::Resolvers::Ci::JobsResolver
 
       field :job,
             type: ::Types::Ci::JobType,
             null: true,
+            authorize: :read_commit_status,
             description: 'A specific job in this pipeline, either by name or ID.' do
         argument :id,
                  type: ::Types::GlobalIDType[::CommitStatus],
@@ -98,7 +105,10 @@ module Types
                  description: 'Name of the job.'
       end
 
-      field :source_job, Types::Ci::JobType, null: true,
+      field :source_job,
+            type: Types::Ci::JobType,
+            null: true,
+            authorize: :read_commit_status,
             description: 'Job where pipeline was triggered from.'
 
       field :downstream, Types::Ci::PipelineType.connection_type, null: true,

app/graphql/types/ci/stage_type.rb

@@ -2,20 +2,26 @@
 
 module Types
   module Ci
-    # rubocop: disable Graphql/AuthorizeTypes
     class StageType < BaseObject
       graphql_name 'CiStage'
+      authorize :read_commit_status
 
-      field :name, GraphQL::STRING_TYPE, null: true,
-        description: 'Name of the stage.'
-      field :groups, Ci::GroupType.connection_type, null: true,
-        extras: [:lookahead],
-        description: 'Group of jobs for the stage.'
-      field :detailed_status, Types::Ci::DetailedStatusType, null: true,
-        description: 'Detailed status of the stage.'
-      field :jobs, Ci::JobType.connection_type, null: true,
-        description: 'Jobs for the stage.',
-        method: 'latest_statuses'
+      field :name,
+            type: GraphQL::STRING_TYPE,
+            null: true,
+            description: 'Name of the stage.'
+      field :groups,
+            type: Ci::GroupType.connection_type,
+            null: true,
+            extras: [:lookahead],
+            description: 'Group of jobs for the stage.'
+      field :detailed_status, Types::Ci::DetailedStatusType,
+            null: true,
+            description: 'Detailed status of the stage.'
+      field :jobs, Ci::JobType.connection_type,
+            null: true,
+            description: 'Jobs for the stage.',
+            method: 'latest_statuses'
 
       def detailed_status
         object.detailed_status(current_user)
@@ -37,33 +43,6 @@ module Types
               key = indexed[stage_id]
               groups = ::Ci::Group.fabricate(project, key.stage, statuses)
 
-              if Feature.enabled?(:ci_no_empty_groups, project)
-                groups.each do |group|
-                  rejected = group.jobs.reject { |job| Ability.allowed?(current_user, :read_commit_status, job) }
-                  group.jobs.select! { |job| Ability.allowed?(current_user, :read_commit_status, job) }
-                  next unless group.jobs.empty?
-
-                  exc = StandardError.new('Empty Ci::Group')
-                  traces = rejected.map do |job|
-                    trace = []
-                    policy = Ability.policy_for(current_user, job)
-                    policy.debug(:read_commit_status, trace)
-                    trace
-                  end
-                  extra = {
-                    current_user_id: current_user&.id,
-                    project_id: project.id,
-                    pipeline_id: pl.id,
-                    stage_id: stage_id,
-                    group_name: group.name,
-                    rejected_job_ids: rejected.map(&:id),
-                    rejected_traces: traces
-                  }
-                  Gitlab::ErrorTracking.track_exception(exc, extra)
-                end
-                groups.reject! { |group| group.jobs.empty? }
-              end
-
               loader.call(key, groups)
             end
           end

app/graphql/types/project_type.rb

@@ -180,14 +180,15 @@ module Types
           resolver: Resolvers::IssuesResolver.single
 
     field :packages,
-         description: 'Packages of the project.',
-         resolver: Resolvers::ProjectPackagesResolver
+          description: 'Packages of the project.',
+          resolver: Resolvers::ProjectPackagesResolver
 
     field :jobs,
-         Types::Ci::JobType.connection_type,
-         null: true,
-         description: 'Jobs of a project. This field can only be resolved for one project in any single request.',
-         resolver: Resolvers::ProjectJobsResolver
+          type: Types::Ci::JobType.connection_type,
+          null: true,
+          authorize: :read_commit_status,
+          description: 'Jobs of a project. This field can only be resolved for one project in any single request.',
+          resolver: Resolvers::ProjectJobsResolver
 
     field :pipelines,
           null: true,

app/policies/ci/stage_policy.rb

+# frozen_string_literal: true
+
+module Ci
+  class StagePolicy < BasePolicy
+    delegate :pipeline
+  end
+end

changelogs/unreleased/329695-add-read_commit_status-field-authorization-to-pipeline-fields-that.yml

+---
+title: Adds field authorization to pipeline fields
+merge_request: 60754
+author:
+type: changed

config/feature_flags/development/ci_no_empty_groups.yml

----
-name: ci_no_empty_groups
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/58789
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/327139
-milestone: '13.11'
-type: development
-group: group::verify
-default_enabled: false

spec/requests/api/graphql/ci/pipelines_spec.rb

@@ -51,6 +51,87 @@ RSpec.describe 'Query.project(fullPath).pipelines' do
     end
   end
 
+  describe '.stages' do
+    let_it_be(:project) { create(:project, :repository) }
+    let_it_be(:pipeline) { create(:ci_empty_pipeline, project: project) }
+    let_it_be(:stage) { create(:ci_stage_entity, pipeline: pipeline, project: project) }
+    let_it_be(:other_stage) { create(:ci_stage_entity, pipeline: pipeline, project: project, name: 'other') }
+
+    let(:first_n) { var('Int') }
+    let(:query_path) do
+      [
+        [:project, { full_path: project.full_path }],
+        [:pipelines],
+        [:nodes],
+        [:stages, { first: first_n }],
+        [:nodes]
+      ]
+    end
+
+    let(:query) do
+      with_signature([first_n], wrap_fields(query_graphql_path(query_path, :name)))
+    end
+
+    before_all do
+      # see app/services/ci/ensure_stage_service.rb to explain why we use stage_id
+      create(:ci_build, pipeline: pipeline, stage_id: stage.id, name: 'linux: [foo]')
+      create(:ci_build, pipeline: pipeline, stage_id: stage.id, name: 'linux: [bar]')
+      create(:ci_build, pipeline: pipeline, stage_id: other_stage.id, name: 'linux: [baz]')
+    end
+
+    it 'is null if the user is a guest' do
+      project.add_guest(user)
+
+      post_graphql(query, current_user: user, variables: first_n.with(1))
+
+      expect(graphql_data_at(:project, :pipelines, :nodes)).to contain_exactly a_hash_including('stages' => be_nil)
+    end
+
+    it 'is present if the user has reporter access' do
+      project.add_reporter(user)
+
+      post_graphql(query, current_user: user)
+
+      expect(graphql_data_at(:project, :pipelines, :nodes, :stages, :nodes, :name))
+        .to contain_exactly(eq(stage.name), eq(other_stage.name))
+    end
+
+    describe '.groups' do
+      let(:query_path) do
+        [
+          [:project, { full_path: project.full_path }],
+          [:pipelines],
+          [:nodes],
+          [:stages],
+          [:nodes],
+          [:groups],
+          [:nodes]
+        ]
+      end
+
+      let(:query) do
+        wrap_fields(query_graphql_path(query_path, :name))
+      end
+
+      it 'is empty if the user is a guest' do
+        project.add_guest(user)
+
+        post_graphql(query, current_user: user)
+
+        expect(graphql_data_at(:project, :pipelines, :nodes, :stages, :nodes, :groups)).to be_empty
+      end
+
+      it 'is present if the user has reporter access' do
+        project.add_reporter(user)
+
+        post_graphql(query, current_user: user)
+
+        expect(graphql_data_at(:project, :pipelines, :nodes, :stages, :nodes, :groups, :nodes, :name))
+          .to contain_exactly('linux', 'linux')
+      end
+    end
+  end
+
   describe '.jobs' do
     let(:first_n) { var('Int') }
     let(:query_path) do