Merge branch 'ensure-that-build-token-is-always-running' into 'security-11-6'

[11.6] Ensure that build token is always running

See merge request gitlab/gitlabhq!2563

4 files changed, 36 insertions, 21 deletions

diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 5dbfbb85e9e..ecc0a0a90bc 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1356,7 +1356,17 @@ module API
       end
 
       class Dependency < Grape::Entity
-        expose :id, :name, :token
+        expose :id, :name
+        expose :token do |dependency, options|
+          # overrides the job's dependency authorization token
+          # with the token of the job that is being run
+          # this way we use the parent job auth token
+          #
+          # ideally we would change the runner implementation to
+          # use different token, but this would require upgrade of
+          # all runners which is impossible
+          options[:auth_token]
+        end
         expose :artifacts_file, using: JobArtifactFile, if: ->(job, _) { job.artifacts? }
       end
 
@@ -1384,7 +1394,10 @@ module API
         expose :artifacts, using: Artifacts
         expose :cache, using: Cache
         expose :credentials, using: Credentials
-        expose :dependencies, using: Dependency
+        expose :dependencies do |model|
+          Dependency.represent(model.dependencies,
+            options.merge(auth_token: model.token))
+        end
         expose :features
       end
     end
diff --git a/lib/api/helpers/runner.rb b/lib/api/helpers/runner.rb
index 45d0343bc89..1a296c8ddb2 100644
--- a/lib/api/helpers/runner.rb
+++ b/lib/api/helpers/runner.rb
@@ -36,26 +36,32 @@ module API
       def validate_job!(job)
         not_found! unless job
 
-        yield if block_given?
-
         project = job.project
-        forbidden!('Project has been deleted!') if project.nil? || project.pending_delete?
-        forbidden!('Job has been erased!') if job.erased?
+        job_forbidden!(job, 'Project has been deleted!') if project.nil? || project.pending_delete?
+        job_forbidden!(job, 'Job has been erased!') if job.erased?
+        job_forbidden!(job, 'Not running!') unless job.running?
       end
 
-      def authenticate_job!
-        job = Ci::Build.find_by_id(params[:id])
+      def authenticate_job_by_token!
+        token = (params[JOB_TOKEN_PARAM] || env[JOB_TOKEN_HEADER]).to_s
 
-        validate_job!(job) do
-          forbidden! unless job_token_valid?(job)
+        Ci::Build.find_by_token(token).tap do |job|
+          validate_job!(job)
         end
+      end
 
-        job
+      # we look for a job that has ID and token matching
+      def authenticate_job!
+        authenticate_job_by_token!.tap do |job|
+          job_forbidden!(job, 'Invalid Job ID!') unless job.id == params[:id]
+        end
       end
 
-      def job_token_valid?(job)
-        token = (params[JOB_TOKEN_PARAM] || env[JOB_TOKEN_HEADER]).to_s
-        token && job.valid_token?(token)
+      # we look for a job that has been shared via pipeline using the ID
+      def authenticate_pipeline_job!
+        job = authenticate_job_by_token!
+
+        job.pipeline.builds.find(params[:id])
       end
 
       def max_artifacts_size
diff --git a/lib/api/runner.rb b/lib/api/runner.rb
index c60d25b88cb..8e842b13f8c 100644
--- a/lib/api/runner.rb
+++ b/lib/api/runner.rb
@@ -144,7 +144,6 @@ module API
       end
       put '/:id' do
         job = authenticate_job!
-        job_forbidden!(job, 'Job is not running') unless job.running?
 
         job.trace.set(params[:trace]) if params[:trace]
 
@@ -172,7 +171,6 @@ module API
       end
       patch '/:id/trace' do
         job = authenticate_job!
-        job_forbidden!(job, 'Job is not running') unless job.running?
 
         error!('400 Missing header Content-Range', 400) unless request.headers.key?('Content-Range')
         content_range = request.headers['Content-Range']
@@ -215,8 +213,7 @@ module API
         require_gitlab_workhorse!
         Gitlab::Workhorse.verify_api_request!(headers)
 
-        job = authenticate_job!
-        forbidden!('Job is not running') unless job.running?
+        authenticate_job!
 
         if params[:filesize]
           file_size = params[:filesize].to_i
@@ -259,7 +256,6 @@ module API
         require_gitlab_workhorse!
 
         job = authenticate_job!
-        forbidden!('Job is not running!') unless job.running?
 
         artifacts = UploadedFile.from_params(params, :file, JobArtifactUploader.workhorse_local_upload_path)
         metadata = UploadedFile.from_params(params, :metadata, JobArtifactUploader.workhorse_local_upload_path)
@@ -306,7 +302,7 @@ module API
         optional :direct_download, default: false, type: Boolean, desc: %q(Perform direct download from remote storage instead of proxying artifacts)
       end
       get '/:id/artifacts' do
-        job = authenticate_job!
+        job = authenticate_pipeline_job!
 
         present_carrierwave_file!(job.artifacts_file, supports_direct_download: params[:direct_download])
       end
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 6eb5f9e2300..f52af8364d7 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -296,7 +296,7 @@ module Gitlab
       private
 
       def find_build_by_token(token)
-        ::Ci::Build.running.find_by_token(token)
+        ::Ci::Build.find_running_by_token(token)
       end
     end
   end