Ensure that build token is only used when running

1. We provide an updated interface to ensure that,
2. We authenticate build dependendencies by build that is being run,

4 files changed, 36 insertions, 21 deletions

diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index cff05643f3b..b7e06d2977e 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1347,7 +1347,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
 
@@ -1375,7 +1385,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 2f15f3a7d76..3be2c281977 100644
--- a/lib/api/runner.rb
+++ b/lib/api/runner.rb
@@ -146,7 +146,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]
 
@@ -174,7 +173,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']
@@ -217,8 +215,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
@@ -261,7 +258,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)
@@ -308,7 +304,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