Allow runner job artifact request using token belonging to a dependent job

This opens the runner job artifact request API to requests that
use either of the following tokens:
1. Job token belonging to the job that owns the artifact (existing behaviour).
   This token is valid regardless of the status of the job.
2. Job token belonging to the job that depends on the artifact (new behaviour).
   This token is valid only if the dependent job is still running.

app/models/ci/build.rb

@@ -757,7 +757,7 @@ def needs_touch?
     end
     def valid_token?(token)
-      self.token && ActiveSupport::SecurityUtils.secure_compare(token, self.token)
+      self.token && token.present? && ActiveSupport::SecurityUtils.secure_compare(token, self.token)
     end
     # acts_as_taggable uses this method create/remove tags with contexts

config/feature_flags/development/ci_authenticate_running_job_token_for_artifacts.yml

+---
+name: ci_authenticate_running_job_token_for_artifacts
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/83713
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/357075
+milestone: '15.0'
+type: development
+group: group::pipeline insights
+default_enabled: false

config/feature_flags/development/ci_expose_running_job_token_for_artifacts.yml

+---
+name: ci_expose_running_job_token_for_artifacts
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/83713
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/357075
+milestone: '15.0'
+type: development
+group: group::pipeline insights
+default_enabled: false

ee/spec/requests/api/ci/runner_spec.rb

@@ -3,14 +3,20 @@
 require 'spec_helper'
 RSpec.describe API::Ci::Runner do
-  let_it_be(:project) { create(:project, :repository) }
-  describe '/api/v4/jobs' do
-    let(:runner) { create(:ci_runner, :project, projects: [project]) }
+  let_it_be_with_reload(:project) { create(:project, :repository) }
+  let_it_be(:user) { create(:user) }
+  let_it_be(:ref) { 'master' }
+  let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: ref) }
+  let_it_be(:runner) { create(:ci_runner, :project, projects: [project]) }
+
+  before_all do
+    project.add_developer(user)
+  end
+  describe '/api/v4/jobs' do
     describe 'POST /api/v4/jobs/request' do
       context 'secrets management' do
-        let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') }
         let(:valid_secrets) do
           {
             DATABASE_PASSWORD: {
@@ -114,10 +120,120 @@
       def request_job_with_secrets_supported
         request_job info: { features: { vault_secrets: true } }
       end
+
+      def request_job(token = runner.token, **params)
+        post api('/jobs/request'), params: params.merge(token: token)
+      end
     end
-    def request_job(token = runner.token, **params)
-      post api('/jobs/request'), params: params.merge(token: token)
+    describe 'GET api/v4/jobs/:id/artifacts' do
+      let_it_be(:job) { create(:ci_build, :success, ref: ref, pipeline: pipeline, user: user) }
+
+      before_all do
+        create(:ci_job_artifact, :archive, job: job)
+      end
+
+      shared_examples 'successful artifact download' do
+        it 'downloads artifacts' do
+          download_artifact
+
+          expect(response).to have_gitlab_http_status(:ok)
+        end
+      end
+
+      shared_examples 'forbidden request' do
+        it 'responds with forbidden' do
+          download_artifact
+
+          expect(response).to have_gitlab_http_status(:forbidden)
+        end
+      end
+
+      context 'when a job has a cross-project dependency' do
+        let_it_be(:downstream_project) { create(:project) }
+        let_it_be_with_reload(:downstream_project_dev) { create(:user) }
+
+        let_it_be(:options) do
+          {
+            cross_dependencies: [
+              {
+                project: project.full_path,
+                ref: ref,
+                job: job.name,
+                artifacts: true
+              }
+            ]
+
+          }
+        end
+
+        let_it_be_with_reload(:downstream_ci_build) do
+          create(:ci_build, :running, project: downstream_project, user: user, options: options)
+        end
+
+        let(:token) { downstream_ci_build.token }
+
+        before_all do
+          downstream_project.add_developer(user)
+          downstream_project.add_developer(downstream_project_dev)
+        end
+
+        before do
+          stub_licensed_features(cross_project_pipelines: true)
+        end
+
+        context 'when the job is created by a user with sufficient permission in upstream project' do
+          it_behaves_like 'successful artifact download'
+
+          context 'and the upstream project has disabled public builds' do
+            before do
+              project.update!(public_builds: false)
+            end
+
+            it_behaves_like 'successful artifact download'
+          end
+        end
+
+        context 'when the job is created by a user without sufficient permission in upstream project' do
+          before do
+            downstream_ci_build.update!(user: downstream_project_dev)
+          end
+
+          it_behaves_like 'forbidden request'
+
+          context 'and the upstream project has disabled public builds' do
+            before do
+              project.update!(public_builds: false)
+            end
+
+            it_behaves_like 'forbidden request'
+          end
+        end
+
+        context 'when the upstream project is public and the job user does not have permission in the project' do
+          before do
+            project.update!(visibility_level: Gitlab::VisibilityLevel::PUBLIC)
+            downstream_ci_build.update!(user: downstream_project_dev)
+          end
+
+          it_behaves_like 'successful artifact download'
+
+          context 'and the upstream project has disabled public builds' do
+            before do
+              project.update!(public_builds: false)
+            end
+
+            it_behaves_like 'forbidden request'
+          end
+        end
+      end
+
+      def download_artifact(params = {}, request_headers = headers)
+        params = params.merge(token: token)
+        job.reload
+
+        get api("/jobs/#{job.id}/artifacts"), params: params, headers: request_headers
+      end
     end
   end
 end

lib/api/ci/helpers/runner.rb

@@ -53,7 +53,7 @@ def authenticate_job!(require_running: true, heartbeat_runner: false)
           # https://gitlab.com/gitlab-org/gitlab/-/issues/327703
           forbidden! unless job
-          forbidden! unless job_token_valid?(job)
+          forbidden! unless job.valid_token?(job_token)
           forbidden!('Project has been deleted!') if job.project.nil? || job.project.pending_delete?
           forbidden!('Job has been erased!') if job.erased?
@@ -77,6 +77,12 @@ def authenticate_job!(require_running: true, heartbeat_runner: false)
           job
         end
+        def authenticate_job_via_dependent_job!
+          forbidden! unless current_authenticated_job
+          forbidden! unless current_job
+          forbidden! unless can?(current_authenticated_job.user, :read_build, current_job)
+        end
+
         def current_job
           id = params[:id]
@@ -91,9 +97,28 @@ def current_job
           end
         end
-        def job_token_valid?(job)
-          token = (params[JOB_TOKEN_PARAM] || env[JOB_TOKEN_HEADER]).to_s
-          token && job.valid_token?(token)
+        # TODO: Replace this with `#current_authenticated_job from API::Helpers`
+        # after the feature flag `ci_authenticate_running_job_token_for_artifacts`
+        # is removed.
+        #
+        # For the time being, this needs to be overridden because the API
+        # GET api/v4/jobs/:id/artifacts
+        # needs to allow requests using token whose job is not running.
+        #
+        # https://gitlab.com/gitlab-org/gitlab/-/merge_requests/83713#note_942368526
+        def current_authenticated_job
+          strong_memoize(:current_authenticated_job) do
+            ::Ci::AuthJobFinder.new(token: job_token).execute
+          end
+        end
+
+        # The token used by runner to authenticate a request.
+        # In most cases, the runner uses the token belonging to the requested job.
+        # However, when requesting for job artifacts, the runner would use
+        # the token that belongs to downstream jobs that depend on the job that owns
+        # the artifacts.
+        def job_token
+          @job_token ||= (params[JOB_TOKEN_PARAM] || env[JOB_TOKEN_HEADER]).to_s
         end
         def job_forbidden!(job, reason)
@@ -120,6 +145,10 @@ def log_artifact_size(artifact)
         def get_runner_config_from_request
           { config: attributes_for_keys(%w(gpus), params.dig('info', 'config')) }
         end
+
+        def request_using_running_job_token?
+          current_job.present? && current_authenticated_job.present? && current_job != current_authenticated_job
+        end
       end
     end
   end

lib/api/ci/runner.rb

@@ -324,9 +324,14 @@ class Runner < ::API::Base
           optional :direct_download, default: false, type: Boolean, desc: %q(Perform direct download from remote storage instead of proxying artifacts)
         end
         get '/:id/artifacts', feature_category: :build_artifacts do
-          job = authenticate_job!(require_running: false)
-          present_carrierwave_file!(job.artifacts_file, supports_direct_download: params[:direct_download])
+          if Feature.enabled?(:ci_authenticate_running_job_token_for_artifacts, current_job&.project) &&
+            request_using_running_job_token?
+            authenticate_job_via_dependent_job!
+          else
+            authenticate_job!(require_running: false)
+          end
+          present_carrierwave_file!(current_job.artifacts_file, supports_direct_download: params[:direct_download])
         end
       end
     end

lib/api/entities/ci/job_request/dependency.rb

@@ -5,7 +5,16 @@ module Entities
     module Ci
       module JobRequest
         class Dependency < Grape::Entity
-          expose :id, :name, :token
+          expose :id, :name
+
+          expose :token do |job, options|
+            if ::Feature.enabled?(:ci_expose_running_job_token_for_artifacts, job.project)
+              options[:running_job]&.token
+            else
+              job.token
+            end
+          end
+
           expose :artifacts_file, using: Entities::Ci::JobArtifactFile, if: ->(job, _) { job.available_artifacts? }
         end
       end

lib/api/entities/ci/job_request/response.rb

@@ -28,8 +28,10 @@ class Response < Grape::Entity
           expose :artifacts, using: Entities::Ci::JobRequest::Artifacts
           expose :cache, using: Entities::Ci::JobRequest::Cache
           expose :credentials, using: Entities::Ci::JobRequest::Credentials
-          expose :all_dependencies, as: :dependencies, using: Entities::Ci::JobRequest::Dependency
           expose :features
+          expose :dependencies do |job, options|
+            Entities::Ci::JobRequest::Dependency.represent(job.all_dependencies, options.merge(running_job: job))
+          end
         end
       end
     end

spec/lib/api/entities/ci/job_request/dependency_spec.rb

@@ -3,8 +3,9 @@
 require 'spec_helper'
 RSpec.describe API::Entities::Ci::JobRequest::Dependency do
+  let(:running_job) { create(:ci_build, :artifacts) }
   let(:job) { create(:ci_build, :artifacts) }
-  let(:entity) { described_class.new(job) }
+  let(:entity) { described_class.new(job, { running_job: running_job }) }
   subject { entity.as_json }
@@ -16,8 +17,18 @@
     expect(subject[:name]).to eq(job.name)
   end
-  it 'returns the dependency token' do
-    expect(subject[:token]).to eq(job.token)
+  it 'returns the token belonging to the running job' do
+    expect(subject[:token]).to eq(running_job.token)
+  end
+
+  context 'when ci_expose_running_job_token_for_artifacts is disabled' do
+    before do
+      stub_feature_flags(ci_expose_running_job_token_for_artifacts: false)
+    end
+
+    it 'returns the token belonging to the dependency job' do
+      expect(subject[:token]).to eq(job.token)
+    end
   end
   it 'returns the dependency artifacts_file', :aggregate_failures do

spec/requests/api/ci/runner/jobs_artifacts_spec.rb

@@ -7,8 +7,20 @@
   include RedisHelpers
   include WorkhorseHelpers
+  let_it_be_with_reload(:parent_group) { create(:group) }
+  let_it_be_with_reload(:group) { create(:group, parent: parent_group) }
+  let_it_be_with_reload(:project) { create(:project, namespace: group, shared_runners_enabled: false) }
+
+  let_it_be(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') }
+  let_it_be(:runner) { create(:ci_runner, :project, projects: [project]) }
+  let_it_be(:user) { create(:user) }
+
   let(:registration_token) { 'abcdefg123456' }
+  before_all do
+    project.add_developer(user)
+  end
+
   before do
     stub_feature_flags(ci_enable_live_trace: true)
     stub_gitlab_calls
@@ -17,12 +29,6 @@
   end
   describe '/api/v4/jobs' do
-    let(:parent_group) { create(:group) }
-    let(:group) { create(:group, parent: parent_group) }
-    let(:project) { create(:project, namespace: group, shared_runners_enabled: false) }
-    let(:pipeline) { create(:ci_pipeline, project: project, ref: 'master') }
-    let(:runner) { create(:ci_runner, :project, projects: [project]) }
-    let(:user) { create(:user) }
     let(:job) do
       create(:ci_build, :artifacts, :extended_options,
              pipeline: pipeline, name: 'spinach', stage: 'test', stage_idx: 0)
@@ -817,25 +823,23 @@ def upload_artifacts(file, headers = {}, params = {})
         end
         context 'when job has artifacts' do
-          let(:job) { create(:ci_build) }
+          let(:job) { create(:ci_build, pipeline: pipeline, user: user) }
           let(:store) { JobArtifactUploader::Store::LOCAL }
           before do
             create(:ci_job_artifact, :archive, file_store: store, job: job)
           end
-          context 'when using job token' do
+          shared_examples 'successful artifact download' do
             context 'when artifacts are stored locally' do
               let(:download_headers) do
                 { 'Content-Transfer-Encoding' => 'binary',
                   'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip) }
               end
-              before do
+              it 'downloads artifacts' do
                 download_artifact
-              end
-              it 'download artifacts' do
                 expect(response).to have_gitlab_http_status(:ok)
                 expect(response.headers.to_h).to include download_headers
               end
@@ -843,26 +847,20 @@ def upload_artifacts(file, headers = {}, params = {})
             context 'when artifacts are stored remotely' do
               let(:store) { JobArtifactUploader::Store::REMOTE }
-              let!(:job) { create(:ci_build) }
               context 'when proxy download is being used' do
-                before do
+                it 'uses workhorse send-url' do
                   download_artifact(direct_download: false)
-                end
-                it 'uses workhorse send-url' do
                   expect(response).to have_gitlab_http_status(:ok)
-                  expect(response.headers.to_h).to include(
-                    'Gitlab-Workhorse-Send-Data' => /send-url:/)
+                  expect(response.headers.to_h).to include('Gitlab-Workhorse-Send-Data' => /send-url:/)
                 end
               end
               context 'when direct download is being used' do
-                before do
+                it 'receives redirect for downloading artifacts' do
                   download_artifact(direct_download: true)
-                end
-                it 'receive redirect for downloading artifacts' do
                   expect(response).to have_gitlab_http_status(:found)
                   expect(response.headers).to include('Location')
                 end
@@ -870,16 +868,151 @@ def upload_artifacts(file, headers = {}, params = {})
             end
           end
-          context 'when using runnners token' do
-            let(:token) { job.project.runners_token }
+          shared_examples 'forbidden request' do
+            it 'responds with forbidden' do
+              download_artifact
+
+              expect(response).to have_gitlab_http_status(:forbidden)
+            end
+          end
+
+          context 'when using job token' do
+            let(:token) { job.token }
+
+            it_behaves_like 'successful artifact download'
+
+            context 'when the job is no longer running' do
+              before do
+                job.success!
+              end
+
+              it_behaves_like 'successful artifact download'
+            end
+
+            context 'when feature flag is disabled' do
+              before do
+                stub_feature_flags(ci_authenticate_running_job_token_for_artifacts: false)
+              end
+
+              it_behaves_like 'successful artifact download'
+
+              context 'when the job is no longer running' do
+                before do
+                  job.success!
+                end
+
+                it_behaves_like 'successful artifact download'
+              end
+            end
+          end
+
+          context 'when using token belonging to the dependent job' do
+            let!(:dependent_job) { create(:ci_build, :running, :dependent, user: user, pipeline: pipeline) }
+            let!(:job) { dependent_job.all_dependencies.first }
+
+            let(:token) { dependent_job.token }
+
+            it_behaves_like 'successful artifact download'
+
+            context 'when the dependent job is no longer running' do
+              before do
+                dependent_job.success!
+              end
+
+              it_behaves_like 'forbidden request'
+            end
+
+            context 'when feature flag is disabled' do
+              before do
+                stub_feature_flags(ci_authenticate_running_job_token_for_artifacts: false)
+              end
+
+              it_behaves_like 'forbidden request'
+            end
+          end
+
+          context 'when using token belonging to another job created by another project member' do
+            let!(:ci_build) { create(:ci_build, :running, :dependent, user: user, pipeline: pipeline) }
+            let!(:job) { ci_build.all_dependencies.first }
+
+            let!(:another_dev) { create(:user) }
+
+            let(:token) { ci_build.token }
             before do
-              download_artifact
+              project.add_developer(another_dev)
+              ci_build.update!(user: another_dev)
             end
-            it 'responds with forbidden' do
-              expect(response).to have_gitlab_http_status(:forbidden)
+            it_behaves_like 'successful artifact download'
+
+            context 'when feature flag is disabled' do
+              before do
+                stub_feature_flags(ci_authenticate_running_job_token_for_artifacts: false)
+              end
+
+              it_behaves_like 'forbidden request'
+            end
+          end
+
+          context 'when using token belonging to a pending dependent job' do
+            let!(:ci_build) { create(:ci_build, :pending, :dependent, user: user, project: project, pipeline: pipeline) }
+            let!(:job) { ci_build.all_dependencies.first }
+
+            let(:token) { ci_build.token }
+
+            it_behaves_like 'forbidden request'
+          end
+
+          context 'when using a token from a cross pipeline build' do
+            let!(:ci_build) { create(:ci_build, :pending, :dependent, user: user, project: project, pipeline: pipeline) }
+            let!(:job) { ci_build.all_dependencies.first }
+
+            let!(:options) do
+              {
+                cross_dependencies: [
+                  {
+                    pipeline: pipeline.id,
+                    job: job.name,
+                    artifacts: true
+                  }
+                ]
+
+              }
             end
+
+            let!(:cross_pipeline) { create(:ci_pipeline, project: project, child_of: pipeline) }
+            let!(:cross_pipeline_build) { create(:ci_build, :running, project: project, user: user, options: options, pipeline: cross_pipeline) }
+
+            let(:token) { cross_pipeline_build.token }
+
+            before do
+              job.success!
+            end
+
+            it_behaves_like 'successful artifact download'
+          end
+
+          context 'when using a token from an unrelated project' do
+            let!(:ci_build) { create(:ci_build, :running, :dependent, user: user, project: project, pipeline: pipeline) }
+            let!(:job) { ci_build.all_dependencies.first }
+
+            let!(:unrelated_ci_build) { create(:ci_build, :running, user: create(:user)) }
+            let(:token) { unrelated_ci_build.token }
+
+            it_behaves_like 'forbidden request'
+          end
+
+          context 'when using runnners token' do
+            let(:token) { job.project.runners_token }
+
+            it_behaves_like 'forbidden request'
+          end
+
+          context 'when using an invalid token' do
+            let(:token) { 'invalid-token' }
+
+            it_behaves_like 'forbidden request'
           end
         end

spec/requests/api/ci/runner/jobs_request_post_spec.rb

@@ -496,15 +496,32 @@
               job2.success
             end
-            it 'returns dependent jobs' do
+            it 'returns dependent jobs with the token of the test job' do
               request_job
               expect(response).to have_gitlab_http_status(:created)
               expect(json_response['id']).to eq(test_job.id)
               expect(json_response['dependencies'].count).to eq(2)
               expect(json_response['dependencies']).to include(
-                { 'id' => job.id, 'name' => job.name, 'token' => job.token },
-                { 'id' => job2.id, 'name' => job2.name, 'token' => job2.token })
+                { 'id' => job.id, 'name' => job.name, 'token' => test_job.token },
+                { 'id' => job2.id, 'name' => job2.name, 'token' => test_job.token })
+            end
+
+            context 'when ci_expose_running_job_token_for_artifacts is disabled' do
+              before do
+                stub_feature_flags(ci_expose_running_job_token_for_artifacts: false)
+              end
+
+              it 'returns dependent jobs with the dependency job tokens' do
+                request_job
+
+                expect(response).to have_gitlab_http_status(:created)
+                expect(json_response['id']).to eq(test_job.id)
+                expect(json_response['dependencies'].count).to eq(2)
+                expect(json_response['dependencies']).to include(
+                  { 'id' => job.id, 'name' => job.name, 'token' => job.token },
+                  { 'id' => job2.id, 'name' => job2.name, 'token' => job2.token })
+              end
             end
             describe 'preloading job_artifacts_archive' do
@@ -526,14 +543,14 @@
               job.success
             end
-            it 'returns dependent jobs' do
+            it 'returns dependent jobs with the token of the test job' do
               request_job
               expect(response).to have_gitlab_http_status(:created)
               expect(json_response['id']).to eq(test_job.id)
               expect(json_response['dependencies'].count).to eq(1)
               expect(json_response['dependencies']).to include(
-                { 'id' => job.id, 'name' => job.name, 'token' => job.token,
+                { 'id' => job.id, 'name' => job.name, 'token' => test_job.token,
                   'artifacts_file' => { 'filename' => 'ci_build_artifacts.zip', 'size' => 107464 } })
             end
           end
@@ -552,13 +569,13 @@
               job2.success
             end
-            it 'returns dependent jobs' do
+            it 'returns dependent jobs with the token of the test job' do
               request_job
               expect(response).to have_gitlab_http_status(:created)
               expect(json_response['id']).to eq(test_job.id)
               expect(json_response['dependencies'].count).to eq(1)
-              expect(json_response['dependencies'][0]).to include('id' => job2.id, 'name' => job2.name, 'token' => job2.token)
+              expect(json_response['dependencies'][0]).to include('id' => job2.id, 'name' => job2.name, 'token' => test_job.token)
             end
           end