From f3f1ad3992c039563188a84787b1e916e367d702 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 4 Sep 2017 14:09:03 +0200
Subject: [PATCH 01/14] Add API endpoint for downloading single job artifact

---
 lib/api/jobs.rb                | 19 ++++++++++++
 spec/requests/api/jobs_spec.rb | 57 ++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+)

diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb
index 5bab96398fd..41b3b28037c 100644
--- a/lib/api/jobs.rb
+++ b/lib/api/jobs.rb
@@ -85,6 +85,25 @@ module API
         present_artifacts!(build.artifacts_file)
       end
 
+      desc 'Download a specific file from artifacts archive' do
+        detail 'This feature was introduced in GitLab 10.0'
+      end
+      params do
+        requires :job_id, type: Integer, desc: 'The ID of a job'
+        requires :artifact_path, type: String, desc: 'Artifact path'
+      end
+      get ':id/jobs/:job_id/artifacts/*artifact_path', format: false do
+        authorize_read_builds!
+
+        build = get_build!(params[:job_id])
+        not_found! unless build.artifacts?
+
+        entry = build.artifacts_metadata_entry(params[:artifact_path])
+        not_found! unless entry.exists?
+
+        Gitlab::Workhorse.send_artifacts_entry(build, entry)
+      end
+
       desc 'Download the artifacts file from a job' do
         detail 'This feature was introduced in GitLab 8.10'
       end
diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index f56baf9663d..f90ee4016d0 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -188,6 +188,63 @@ describe API::Jobs do
     end
   end
 
+  describe 'GET /projects/:id/jobs/:job_id/artifacts/:artifact_path' do
+    context 'when job has artifacts' do
+      let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) }
+
+      let(:artifact) do
+        'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif'
+      end
+
+      context 'when user is not unauthorized' do
+        let(:api_user) { nil }
+
+        it 'does not return specific job artifacts' do
+          get_artifact_file(artifact)
+
+          expect(response).to have_http_status(401)
+        end
+      end
+
+      context 'when user is authorized' do
+        it 'returns a specific artifact file for a valid path' do
+          expect(Gitlab::Workhorse)
+            .to receive(:send_artifacts_entry)
+            .and_call_original
+
+          get_artifact_file(artifact)
+
+          expect(response).to have_http_status(200)
+          expect(response.body)
+            .to include 'Gitlab-Workhorse-Send-Data', 'artifacts-entry'
+          expect(response.headers)
+            .to include('Content-Type' => 'application/json')
+        end
+      end
+
+      context 'when request path is invalid' do
+        it 'does not find artifact file' do
+          get_artifact_file('invalid/path')
+
+          expect(response).to have_http_status(404)
+        end
+      end
+    end
+
+    context 'when job does not have artifacts' do
+      it 'does not return job artifact file' do
+        get_artifact_file('some/artifact')
+
+        expect(response).to have_http_status(404)
+      end
+    end
+
+    def get_artifact_file(artifact_path)
+      get api("/projects/#{project.id}/jobs/#{job.id}/" \
+              "artifacts/#{artifact_path}", api_user)
+    end
+  end
+
   describe 'GET /projects/:id/jobs/:job_id/artifacts' do
     before do
       get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user)

From 83600e94262c1fadae98f1e0709f2430aaea03b7 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Mon, 4 Sep 2017 14:10:42 +0200
Subject: [PATCH 02/14] Set project / pipeline before context in job API specs

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

diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index f90ee4016d0..00e761e8919 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -1,11 +1,11 @@
 require 'spec_helper'
 
 describe API::Jobs do
-  let!(:project) do
+  set(:project) do
     create(:project, :repository, public_builds: false)
   end
 
-  let!(:pipeline) do
+  set(:pipeline) do
     create(:ci_empty_pipeline, project: project,
                                sha: project.commit.id,
                                ref: project.default_branch)

From dfb8fcbb651812d209d2f42baf6c2bb0e851c861 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Sep 2017 11:16:49 +0200
Subject: [PATCH 03/14] Use API helper to send artifact file through Workhorse

---
 lib/api/helpers.rb             | 4 ++++
 lib/api/jobs.rb                | 2 +-
 spec/requests/api/jobs_spec.rb | 5 ++---
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 3d377fdb9eb..f9ce1165544 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -432,6 +432,10 @@ module API
       header(*Gitlab::Workhorse.send_git_archive(repository, ref: ref, format: format))
     end
 
+    def send_artifacts_entry(build, entry)
+      header(*Gitlab::Workhorse.send_artifacts_entry(build, entry))
+    end
+
     # The Grape Error Middleware only has access to env but no params. We workaround this by
     # defining a method that returns the right value.
     def define_params_for_grape_middleware
diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb
index 41b3b28037c..41c70a2dcb7 100644
--- a/lib/api/jobs.rb
+++ b/lib/api/jobs.rb
@@ -101,7 +101,7 @@ module API
         entry = build.artifacts_metadata_entry(params[:artifact_path])
         not_found! unless entry.exists?
 
-        Gitlab::Workhorse.send_artifacts_entry(build, entry)
+        send_artifacts_entry(build, entry)
       end
 
       desc 'Download the artifacts file from a job' do
diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index 00e761e8919..ba5cab3265a 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -215,10 +215,9 @@ describe API::Jobs do
           get_artifact_file(artifact)
 
           expect(response).to have_http_status(200)
-          expect(response.body)
-            .to include 'Gitlab-Workhorse-Send-Data', 'artifacts-entry'
           expect(response.headers)
-            .to include('Content-Type' => 'application/json')
+            .to include('Content-Type' => 'application/json',
+                        'Gitlab-Workhorse-Send-Data' => /artifacts-entry/)
         end
       end
 

From c53f319f883064e6c5e77f9b48a235b23b0f4363 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Sep 2017 12:17:56 +0200
Subject: [PATCH 04/14] Extract a class that represents artifacts file path

---
 .../ci/build/artifacts/metadata/entry.rb      | 242 +++++++++---------
 lib/gitlab/ci/build/artifacts/path.rb         |  51 ++++
 .../gitlab/ci/build/artifacts/path_spec.rb    |  64 +++++
 3 files changed, 236 insertions(+), 121 deletions(-)
 create mode 100644 lib/gitlab/ci/build/artifacts/path.rb
 create mode 100644 spec/lib/gitlab/ci/build/artifacts/path_spec.rb

diff --git a/lib/gitlab/ci/build/artifacts/metadata/entry.rb b/lib/gitlab/ci/build/artifacts/metadata/entry.rb
index 2e073334abc..321ddfd8c29 100644
--- a/lib/gitlab/ci/build/artifacts/metadata/entry.rb
+++ b/lib/gitlab/ci/build/artifacts/metadata/entry.rb
@@ -1,128 +1,128 @@
 module Gitlab
-  module Ci::Build::Artifacts
-    class Metadata
-      ##
-      # Class that represents an entry (path and metadata) to a file or
-      # directory in GitLab CI Build Artifacts binary file / archive
-      #
-      # This is IO-operations safe class, that does similar job to
-      # Ruby's Pathname but without the risk of accessing filesystem.
-      #
-      # This class is working only with UTF-8 encoded paths.
-      #
-      class Entry
-        attr_reader :path, :entries
-        attr_accessor :name
+  module Ci
+    module Build
+      module Artifacts
+        class Metadata
+          ##
+          # Class that represents an entry (path and metadata) to a file or
+          # directory in GitLab CI Build Artifacts binary file / archive
+          #
+          # This is IO-operations safe class, that does similar job to
+          # Ruby's Pathname but without the risk of accessing filesystem.
+          #
+          # This class is working only with UTF-8 encoded paths.
+          #
+          class Entry
+            attr_reader :entries
+            attr_accessor :name
 
-        def initialize(path, entries)
-          @path = path.dup.force_encoding('UTF-8')
-          @entries = entries
+            def initialize(path, entries)
+              @entries = entries
+              @path = Artifacts::Path.new(path)
+            end
 
-          if path.include?("\0")
-            raise ArgumentError, 'Path contains zero byte character!'
+            delegate :empty?, to: :children
+
+            def directory?
+              blank_node? || @path.directory?
+            end
+
+            def file?
+              !directory?
+            end
+
+            def blob
+              return unless file?
+
+              @blob ||= Blob.decorate(::Ci::ArtifactBlob.new(self), nil)
+            end
+
+            def has_parent?
+              nodes > 0
+            end
+
+            def parent
+              return nil unless has_parent?
+              self.class.new(@path.to_s.chomp(basename), @entries)
+            end
+
+            def basename
+              (directory? && !blank_node?) ? name + '/' : name
+            end
+
+            def name
+              @name || @path.name
+            end
+
+            def children
+              return [] unless directory?
+              return @children if @children
+
+              child_pattern = %r{^#{Regexp.escape(@path.to_s)}[^/]+/?$}
+              @children = select_entries { |path| path =~ child_pattern }
+            end
+
+            def directories(opts = {})
+              return [] unless directory?
+              dirs = children.select(&:directory?)
+              return dirs unless has_parent? && opts[:parent]
+
+              dotted_parent = parent
+              dotted_parent.name = '..'
+              dirs.prepend(dotted_parent)
+            end
+
+            def files
+              return [] unless directory?
+              children.select(&:file?)
+            end
+
+            def metadata
+              @entries[@path.to_s] || {}
+            end
+
+            def nodes
+              @path.nodes + (file? ? 1 : 0)
+            end
+
+            def blank_node?
+              @path.to_s.empty? # "" is considered to be './'
+            end
+
+            def exists?
+              blank_node? || @entries.include?(@path.to_s)
+            end
+
+            def total_size
+              descendant_pattern = %r{^#{Regexp.escape(@path.to_s)}}
+              entries.sum do |path, entry|
+                (entry[:size] if path =~ descendant_pattern).to_i
+              end
+            end
+
+            def path
+              @path.to_s
+            end
+
+            def to_s
+              @path.to_s
+            end
+
+            def ==(other)
+              path == other.path && @entries == other.entries
+            end
+
+            def inspect
+              "#{self.class.name}: #{self.to_s}"
+            end
+
+            private
+
+            def select_entries
+              selected = @entries.select { |path, _metadata| yield path }
+              selected.map { |path, _metadata| self.class.new(path, @entries) }
+            end
           end
-
-          unless path.valid_encoding?
-            raise ArgumentError, 'Path contains non-UTF-8 byte sequence!'
-          end
-        end
-
-        delegate :empty?, to: :children
-
-        def directory?
-          blank_node? || @path.end_with?('/')
-        end
-
-        def file?
-          !directory?
-        end
-
-        def blob
-          return unless file?
-
-          @blob ||= Blob.decorate(::Ci::ArtifactBlob.new(self), nil)
-        end
-
-        def has_parent?
-          nodes > 0
-        end
-
-        def parent
-          return nil unless has_parent?
-          self.class.new(@path.chomp(basename), @entries)
-        end
-
-        def basename
-          (directory? && !blank_node?) ? name + '/' : name
-        end
-
-        def name
-          @name || @path.split('/').last.to_s
-        end
-
-        def children
-          return [] unless directory?
-          return @children if @children
-
-          child_pattern = %r{^#{Regexp.escape(@path)}[^/]+/?$}
-          @children = select_entries { |path| path =~ child_pattern }
-        end
-
-        def directories(opts = {})
-          return [] unless directory?
-          dirs = children.select(&:directory?)
-          return dirs unless has_parent? && opts[:parent]
-
-          dotted_parent = parent
-          dotted_parent.name = '..'
-          dirs.prepend(dotted_parent)
-        end
-
-        def files
-          return [] unless directory?
-          children.select(&:file?)
-        end
-
-        def metadata
-          @entries[@path] || {}
-        end
-
-        def nodes
-          @path.count('/') + (file? ? 1 : 0)
-        end
-
-        def blank_node?
-          @path.empty? # "" is considered to be './'
-        end
-
-        def exists?
-          blank_node? || @entries.include?(@path)
-        end
-
-        def total_size
-          descendant_pattern = %r{^#{Regexp.escape(@path)}}
-          entries.sum do |path, entry|
-            (entry[:size] if path =~ descendant_pattern).to_i
-          end
-        end
-
-        def to_s
-          @path
-        end
-
-        def ==(other)
-          @path == other.path && @entries == other.entries
-        end
-
-        def inspect
-          "#{self.class.name}: #{@path}"
-        end
-
-        private
-
-        def select_entries
-          selected = @entries.select { |path, _metadata| yield path }
-          selected.map { |path, _metadata| self.class.new(path, @entries) }
         end
       end
     end
diff --git a/lib/gitlab/ci/build/artifacts/path.rb b/lib/gitlab/ci/build/artifacts/path.rb
new file mode 100644
index 00000000000..9cd9b36c5f8
--- /dev/null
+++ b/lib/gitlab/ci/build/artifacts/path.rb
@@ -0,0 +1,51 @@
+module Gitlab
+  module Ci
+    module Build
+      module Artifacts
+        class Path
+          def initialize(path)
+            @path = path.dup.force_encoding('UTF-8')
+          end
+
+          def valid?
+            nonzero? && utf8?
+          end
+
+          def directory?
+            @path.end_with?('/')
+          end
+
+          def name
+            @path.split('/').last.to_s
+          end
+
+          def nodes
+            @path.count('/')
+          end
+
+          def to_s
+            @path.tap do |path|
+              unless nonzero?
+                raise ArgumentError, 'Path contains zero byte character!'
+              end
+
+              unless utf8?
+                raise ArgumentError, 'Path contains non-UTF-8 byte sequence!'
+              end
+            end
+          end
+
+          private
+
+          def nonzero?
+            @path.exclude?("\0")
+          end
+
+          def utf8?
+            @path.valid_encoding?
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/build/artifacts/path_spec.rb b/spec/lib/gitlab/ci/build/artifacts/path_spec.rb
new file mode 100644
index 00000000000..7bd6a2ead25
--- /dev/null
+++ b/spec/lib/gitlab/ci/build/artifacts/path_spec.rb
@@ -0,0 +1,64 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Build::Artifacts::Path do
+  describe '#valid?' do
+    context 'when path contains a zero character' do
+      it 'is not valid' do
+        expect(described_class.new("something/\255")).not_to be_valid
+      end
+    end
+
+    context 'when path is not utf8 string' do
+      it 'is not valid' do
+        expect(described_class.new("something/\0")).not_to be_valid
+      end
+    end
+
+    context 'when path is valid' do
+      it 'is valid' do
+        expect(described_class.new("some/file/path")).to be_valid
+      end
+    end
+  end
+
+  describe '#directory?' do
+    context 'when path ends with a directory indicator' do
+      it 'is a directory' do
+        expect(described_class.new("some/file/dir/")).to be_directory
+      end
+    end
+
+    context 'when path does not end with a directory indicator' do
+      it 'is not a directory' do
+        expect(described_class.new("some/file")).not_to be_directory
+      end
+    end
+  end
+
+  describe '#name' do
+    it 'returns a base name' do
+      expect(described_class.new("some/file").name).to eq 'file'
+    end
+  end
+
+  describe '#nodes' do
+    it 'returns number of path nodes' do
+      expect(described_class.new("some/dir/file").nodes).to eq 2
+    end
+  end
+
+  describe '#to_s' do
+    context 'when path is valid' do
+      it 'returns a string representation of a path' do
+        expect(described_class.new('some/path').to_s).to eq 'some/path'
+      end
+    end
+
+    context 'when path is invalid' do
+      it 'raises an error' do
+        expect { described_class.new("invalid/\0").to_s }
+          .to raise_error ArgumentError
+      end
+    end
+  end
+end

From 52e52f4a172ae5aa54eac4a22d98610ec5aea1b0 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Sep 2017 12:20:09 +0200
Subject: [PATCH 05/14] Make it explicit that workhorse needs artifact path

---
 lib/gitlab/workhorse.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb
index e5ad9b5a40c..7a94af2f8f1 100644
--- a/lib/gitlab/workhorse.rb
+++ b/lib/gitlab/workhorse.rb
@@ -121,10 +121,10 @@ module Gitlab
         ]
       end
 
-      def send_artifacts_entry(build, entry)
+      def send_artifacts_entry(build, path)
         params = {
           'Archive' => build.artifacts_file.path,
-          'Entry' => Base64.encode64(entry.path)
+          'Entry' => Base64.encode64(path.to_s)
         }
 
         [

From 3b874414c06156767117b7aa7ae705c7342d887c Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Sep 2017 12:27:40 +0200
Subject: [PATCH 06/14] Do not use artifacts metadata to access single artifact

---
 app/controllers/projects/artifacts_controller.rb | 12 +++++++-----
 lib/api/jobs.rb                                  |  7 ++++---
 spec/requests/api/jobs_spec.rb                   |  8 --------
 3 files changed, 11 insertions(+), 16 deletions(-)

diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb
index f637a9a803b..eb010923466 100644
--- a/app/controllers/projects/artifacts_controller.rb
+++ b/app/controllers/projects/artifacts_controller.rb
@@ -7,7 +7,7 @@ class Projects::ArtifactsController < Projects::ApplicationController
   before_action :authorize_update_build!, only: [:keep]
   before_action :extract_ref_name_and_path
   before_action :validate_artifacts!
-  before_action :set_path_and_entry, only: [:file, :raw]
+  before_action :entry, only: [:file]
 
   def download
     if artifacts_file.file_storage?
@@ -41,7 +41,10 @@ class Projects::ArtifactsController < Projects::ApplicationController
   end
 
   def raw
-    send_artifacts_entry(build, @entry)
+    path = Gitlab::Ci::Build::Artifacts::Path
+      .new(params[:path])
+
+    send_artifacts_entry(build, path)
   end
 
   def keep
@@ -93,9 +96,8 @@ class Projects::ArtifactsController < Projects::ApplicationController
     @artifacts_file ||= build.artifacts_file
   end
 
-  def set_path_and_entry
-    @path = params[:path]
-    @entry = build.artifacts_metadata_entry(@path)
+  def entry
+    @entry = build.artifacts_metadata_entry(params[:path])
 
     render_404 unless @entry.exists?
   end
diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb
index 41c70a2dcb7..3d71d6bb062 100644
--- a/lib/api/jobs.rb
+++ b/lib/api/jobs.rb
@@ -98,10 +98,11 @@ module API
         build = get_build!(params[:job_id])
         not_found! unless build.artifacts?
 
-        entry = build.artifacts_metadata_entry(params[:artifact_path])
-        not_found! unless entry.exists?
+        path = Gitlab::Ci::Build::Artifacts::Path
+          .new(params[:artifact_path])
+        not_found! unless path.valid?
 
-        send_artifacts_entry(build, entry)
+        send_artifacts_entry(build, path)
       end
 
       desc 'Download the artifacts file from a job' do
diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index ba5cab3265a..9a113096951 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -220,14 +220,6 @@ describe API::Jobs do
                         'Gitlab-Workhorse-Send-Data' => /artifacts-entry/)
         end
       end
-
-      context 'when request path is invalid' do
-        it 'does not find artifact file' do
-          get_artifact_file('invalid/path')
-
-          expect(response).to have_http_status(404)
-        end
-      end
     end
 
     context 'when job does not have artifacts' do

From d4154ef30f52b30054e8e5d9bbf172d8700e8049 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Tue, 5 Sep 2017 13:22:15 +0200
Subject: [PATCH 07/14] Do not require API authentication if artifacts are
 public

---
 lib/api/jobs.rb                | 79 ++++++++++++++++++----------------
 spec/requests/api/jobs_spec.rb | 45 ++++++++++++++++---
 2 files changed, 81 insertions(+), 43 deletions(-)

diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb
index 3d71d6bb062..9e2af071e0a 100644
--- a/lib/api/jobs.rb
+++ b/lib/api/jobs.rb
@@ -2,12 +2,12 @@ module API
   class Jobs < Grape::API
     include PaginationParams
 
-    before { authenticate! }
-
     params do
       requires :id, type: String, desc: 'The ID of a project'
     end
     resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
+      before { authenticate! }
+
       helpers do
         params :optional_scope do
           optional :scope, types: [String, Array[String]], desc: 'The scope of builds to show',
@@ -71,40 +71,6 @@ module API
         present build, with: Entities::Job
       end
 
-      desc 'Download the artifacts file from a job' do
-        detail 'This feature was introduced in GitLab 8.5'
-      end
-      params do
-        requires :job_id, type: Integer, desc: 'The ID of a job'
-      end
-      get ':id/jobs/:job_id/artifacts' do
-        authorize_read_builds!
-
-        build = get_build!(params[:job_id])
-
-        present_artifacts!(build.artifacts_file)
-      end
-
-      desc 'Download a specific file from artifacts archive' do
-        detail 'This feature was introduced in GitLab 10.0'
-      end
-      params do
-        requires :job_id, type: Integer, desc: 'The ID of a job'
-        requires :artifact_path, type: String, desc: 'Artifact path'
-      end
-      get ':id/jobs/:job_id/artifacts/*artifact_path', format: false do
-        authorize_read_builds!
-
-        build = get_build!(params[:job_id])
-        not_found! unless build.artifacts?
-
-        path = Gitlab::Ci::Build::Artifacts::Path
-          .new(params[:artifact_path])
-        not_found! unless path.valid?
-
-        send_artifacts_entry(build, path)
-      end
-
       desc 'Download the artifacts file from a job' do
         detail 'This feature was introduced in GitLab 8.10'
       end
@@ -235,6 +201,47 @@ module API
       end
     end
 
+    params do
+      requires :id, type: String, desc: 'The ID of a project'
+    end
+    resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
+      before { authenticate_non_get! }
+
+      desc 'Download the artifacts file from a job' do
+        detail 'This feature was introduced in GitLab 8.5'
+      end
+      params do
+        requires :job_id, type: Integer, desc: 'The ID of a job'
+      end
+      get ':id/jobs/:job_id/artifacts' do
+        authorize_read_builds!
+
+        build = get_build!(params[:job_id])
+
+        present_artifacts!(build.artifacts_file)
+      end
+
+      desc 'Download a specific file from artifacts archive' do
+        detail 'This feature was introduced in GitLab 10.0'
+      end
+      params do
+        requires :job_id, type: Integer, desc: 'The ID of a job'
+        requires :artifact_path, type: String, desc: 'Artifact path'
+      end
+      get ':id/jobs/:job_id/artifacts/*artifact_path', format: false do
+        authorize_read_builds!
+
+        build = get_build!(params[:job_id])
+        not_found! unless build.artifacts?
+
+        path = Gitlab::Ci::Build::Artifacts::Path
+          .new(params[:artifact_path])
+        not_found! unless path.valid?
+
+        send_artifacts_entry(build, path)
+      end
+    end
+
     helpers do
       def find_build(id)
         user_project.builds.find_by(id: id.to_i)
diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index 9a113096951..dd2aed38412 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -196,13 +196,43 @@ describe API::Jobs do
         'other_artifacts_0.1.2/another-subdirectory/banana_sample.gif'
       end
 
-      context 'when user is not unauthorized' do
+      context 'when user is anonymous' do
         let(:api_user) { nil }
 
-        it 'does not return specific job artifacts' do
-          get_artifact_file(artifact)
+        context 'when project is public' do
+          it 'allows to access artifacts' do
+            project.update_column(:visibility_level,
+                                  Gitlab::VisibilityLevel::PUBLIC)
+            project.update_column(:public_builds, true)
 
-          expect(response).to have_http_status(401)
+            get_artifact_file(artifact)
+
+            expect(response).to have_http_status(200)
+          end
+        end
+
+        context 'when project is public with builds access disabled' do
+          it 'rejects access to artifacts' do
+            project.update_column(:visibility_level,
+                                  Gitlab::VisibilityLevel::PUBLIC)
+            project.update_column(:public_builds, false)
+
+            get_artifact_file(artifact)
+
+            expect(response).to have_http_status(403)
+          end
+        end
+
+        context 'when project is private' do
+          it 'rejects access and hides existence of artifacts' do
+            project.update_column(:visibility_level,
+                                  Gitlab::VisibilityLevel::PRIVATE)
+            project.update_column(:public_builds, true)
+
+            get_artifact_file(artifact)
+
+            expect(response).to have_http_status(404)
+          end
         end
       end
 
@@ -257,11 +287,12 @@ describe API::Jobs do
         end
       end
 
-      context 'unauthorized user' do
+      context 'when anonymous user is accessing private artifacts' do
         let(:api_user) { nil }
 
-        it 'does not return specific job artifacts' do
-          expect(response).to have_http_status(401)
+        it 'hides artifacts and rejects request' do
+          expect(project).to be_private
+          expect(response).to have_http_status(404)
         end
       end
     end

From 80b3dcc777c72ee1853b4228baf6310ccd5e44d8 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Sep 2017 11:20:12 +0200
Subject: [PATCH 08/14] Extract job artifacts API code to a separate file

---
 lib/api/api.rb                 |   1 +
 lib/api/helpers.rb             |  12 ++++
 lib/api/job_artifacts.rb       |  80 +++++++++++++++++++++++++
 lib/api/jobs.rb                | 105 ++-------------------------------
 spec/requests/api/jobs_spec.rb |   5 +-
 5 files changed, 102 insertions(+), 101 deletions(-)
 create mode 100644 lib/api/job_artifacts.rb

diff --git a/lib/api/api.rb b/lib/api/api.rb
index 94df543853b..1405a5d0f0e 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -108,6 +108,7 @@ module API
     mount ::API::Internal
     mount ::API::Issues
     mount ::API::Jobs
+    mount ::API::JobArtifacts
     mount ::API::Keys
     mount ::API::Labels
     mount ::API::Lint
diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index f9ce1165544..6fa1527461f 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -128,6 +128,10 @@ module API
       merge_request
     end
 
+    def find_build!(id)
+      user_project.builds.find(id.to_i)
+    end
+
     def authenticate!
       unauthorized! unless current_user && can?(initial_current_user, :access_api)
     end
@@ -160,6 +164,14 @@ module API
       authorize! :admin_project, user_project
     end
 
+    def authorize_read_builds!
+      authorize! :read_build, user_project
+    end
+
+    def authorize_update_builds!
+      authorize! :update_build, user_project
+    end
+
     def require_gitlab_workhorse!
       unless env['HTTP_GITLAB_WORKHORSE'].present?
         forbidden!('Request should be executed via GitLab Workhorse')
diff --git a/lib/api/job_artifacts.rb b/lib/api/job_artifacts.rb
new file mode 100644
index 00000000000..e0c5449e65e
--- /dev/null
+++ b/lib/api/job_artifacts.rb
@@ -0,0 +1,80 @@
+module API
+  class JobArtifacts < Grape::API
+    before { authenticate_non_get! }
+
+    params do
+      requires :id, type: String, desc: 'The ID of a project'
+    end
+    resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
+      desc 'Download the artifacts file from a job' do
+        detail 'This feature was introduced in GitLab 8.10'
+      end
+      params do
+        requires :ref_name, type: String, desc: 'The ref from repository'
+        requires :job,      type: String, desc: 'The name for the job'
+      end
+      get ':id/jobs/artifacts/:ref_name/download',
+        requirements: { ref_name: /.+/ } do
+        authorize_read_builds!
+
+        builds = user_project.latest_successful_builds_for(params[:ref_name])
+        latest_build = builds.find_by!(name: params[:job])
+
+        present_artifacts!(latest_build.artifacts_file)
+      end
+
+      desc 'Download the artifacts file from a job' do
+        detail 'This feature was introduced in GitLab 8.5'
+      end
+      params do
+        requires :job_id, type: Integer, desc: 'The ID of a job'
+      end
+      get ':id/jobs/:job_id/artifacts' do
+        authorize_read_builds!
+
+        build = find_build!(params[:job_id])
+
+        present_artifacts!(build.artifacts_file)
+      end
+
+      desc 'Download a specific file from artifacts archive' do
+        detail 'This feature was introduced in GitLab 10.0'
+      end
+      params do
+        requires :job_id, type: Integer, desc: 'The ID of a job'
+        requires :artifact_path, type: String, desc: 'Artifact path'
+      end
+      get ':id/jobs/:job_id/artifacts/*artifact_path', format: false do
+        authorize_read_builds!
+
+        build = find_build!(params[:job_id])
+        not_found! unless build.artifacts?
+
+        path = Gitlab::Ci::Build::Artifacts::Path
+          .new(params[:artifact_path])
+        not_found! unless path.valid?
+
+        send_artifacts_entry(build, path)
+      end
+
+      desc 'Keep the artifacts to prevent them from being deleted' do
+        success Entities::Job
+      end
+      params do
+        requires :job_id, type: Integer, desc: 'The ID of a job'
+      end
+      post ':id/jobs/:job_id/artifacts/keep' do
+        authorize_update_builds!
+
+        build = find_build!(params[:job_id])
+        authorize!(:update_build, build)
+        return not_found!(build) unless build.artifacts?
+
+        build.keep_artifacts!
+
+        status 200
+        present build, with: Entities::Job
+      end
+    end
+  end
+end
diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb
index 9e2af071e0a..19161aabd37 100644
--- a/lib/api/jobs.rb
+++ b/lib/api/jobs.rb
@@ -66,28 +66,11 @@ module API
       get ':id/jobs/:job_id' do
         authorize_read_builds!
 
-        build = get_build!(params[:job_id])
+        build = find_build!(params[:job_id])
 
         present build, with: Entities::Job
       end
 
-      desc 'Download the artifacts file from a job' do
-        detail 'This feature was introduced in GitLab 8.10'
-      end
-      params do
-        requires :ref_name, type: String, desc: 'The ref from repository'
-        requires :job,      type: String, desc: 'The name for the job'
-      end
-      get ':id/jobs/artifacts/:ref_name/download',
-        requirements: { ref_name: /.+/ } do
-        authorize_read_builds!
-
-        builds = user_project.latest_successful_builds_for(params[:ref_name])
-        latest_build = builds.find_by!(name: params[:job])
-
-        present_artifacts!(latest_build.artifacts_file)
-      end
-
       # TODO: We should use `present_file!` and leave this implementation for backward compatibility (when build trace
       #       is saved in the DB instead of file). But before that, we need to consider how to replace the value of
       #       `runners_token` with some mask (like `xxxxxx`) when sending trace file directly by workhorse.
@@ -98,7 +81,7 @@ module API
       get ':id/jobs/:job_id/trace' do
         authorize_read_builds!
 
-        build = get_build!(params[:job_id])
+        build = find_build!(params[:job_id])
 
         header 'Content-Disposition', "infile; filename=\"#{build.id}.log\""
         content_type 'text/plain'
@@ -117,7 +100,7 @@ module API
       post ':id/jobs/:job_id/cancel' do
         authorize_update_builds!
 
-        build = get_build!(params[:job_id])
+        build = find_build!(params[:job_id])
         authorize!(:update_build, build)
 
         build.cancel
@@ -134,7 +117,7 @@ module API
       post ':id/jobs/:job_id/retry' do
         authorize_update_builds!
 
-        build = get_build!(params[:job_id])
+        build = find_build!(params[:job_id])
         authorize!(:update_build, build)
         return forbidden!('Job is not retryable') unless build.retryable?
 
@@ -152,7 +135,7 @@ module API
       post ':id/jobs/:job_id/erase' do
         authorize_update_builds!
 
-        build = get_build!(params[:job_id])
+        build = find_build!(params[:job_id])
         authorize!(:update_build, build)
         return forbidden!('Job is not erasable!') unless build.erasable?
 
@@ -160,25 +143,6 @@ module API
         present build, with: Entities::Job
       end
 
-      desc 'Keep the artifacts to prevent them from being deleted' do
-        success Entities::Job
-      end
-      params do
-        requires :job_id, type: Integer, desc: 'The ID of a job'
-      end
-      post ':id/jobs/:job_id/artifacts/keep' do
-        authorize_update_builds!
-
-        build = get_build!(params[:job_id])
-        authorize!(:update_build, build)
-        return not_found!(build) unless build.artifacts?
-
-        build.keep_artifacts!
-
-        status 200
-        present build, with: Entities::Job
-      end
-
       desc 'Trigger a manual job' do
         success Entities::Job
         detail 'This feature was added in GitLab 8.11'
@@ -189,7 +153,7 @@ module API
       post ":id/jobs/:job_id/play" do
         authorize_read_builds!
 
-        build = get_build!(params[:job_id])
+        build = find_build!(params[:job_id])
 
         authorize!(:update_build, build)
         bad_request!("Unplayable Job") unless build.playable?
@@ -201,56 +165,7 @@ module API
       end
     end
 
-    params do
-      requires :id, type: String, desc: 'The ID of a project'
-    end
-    resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
-      before { authenticate_non_get! }
-
-      desc 'Download the artifacts file from a job' do
-        detail 'This feature was introduced in GitLab 8.5'
-      end
-      params do
-        requires :job_id, type: Integer, desc: 'The ID of a job'
-      end
-      get ':id/jobs/:job_id/artifacts' do
-        authorize_read_builds!
-
-        build = get_build!(params[:job_id])
-
-        present_artifacts!(build.artifacts_file)
-      end
-
-      desc 'Download a specific file from artifacts archive' do
-        detail 'This feature was introduced in GitLab 10.0'
-      end
-      params do
-        requires :job_id, type: Integer, desc: 'The ID of a job'
-        requires :artifact_path, type: String, desc: 'Artifact path'
-      end
-      get ':id/jobs/:job_id/artifacts/*artifact_path', format: false do
-        authorize_read_builds!
-
-        build = get_build!(params[:job_id])
-        not_found! unless build.artifacts?
-
-        path = Gitlab::Ci::Build::Artifacts::Path
-          .new(params[:artifact_path])
-        not_found! unless path.valid?
-
-        send_artifacts_entry(build, path)
-      end
-    end
-
     helpers do
-      def find_build(id)
-        user_project.builds.find_by(id: id.to_i)
-      end
-
-      def get_build!(id)
-        find_build(id) || not_found!
-      end
-
       def filter_builds(builds, scope)
         return builds if scope.nil? || scope.empty?
 
@@ -261,14 +176,6 @@ module API
 
         builds.where(status: available_statuses && scope)
       end
-
-      def authorize_read_builds!
-        authorize! :read_build, user_project
-      end
-
-      def authorize_update_builds!
-        authorize! :update_build, user_project
-      end
     end
   end
 end
diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index dd2aed38412..2d7cc1a1798 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -321,8 +321,9 @@ describe API::Jobs do
         get_for_ref
       end
 
-      it 'gives 401' do
-        expect(response).to have_http_status(401)
+      it 'does not find a resource in a private project' do
+        expect(project).to be_private
+        expect(response).to have_http_status(404)
       end
     end
 

From c922fb4b68e3d57bf5328b6825040f1871a48b66 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Sep 2017 11:31:08 +0200
Subject: [PATCH 09/14] Respond with a bad request if artifact path is invalid

---
 lib/api/helpers.rb       | 2 +-
 lib/api/job_artifacts.rb | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb
index 6fa1527461f..e646c63467a 100644
--- a/lib/api/helpers.rb
+++ b/lib/api/helpers.rb
@@ -222,7 +222,7 @@ module API
 
     def bad_request!(attribute)
       message = ["400 (Bad request)"]
-      message << "\"" + attribute.to_s + "\" not given"
+      message << "\"" + attribute.to_s + "\" not given" if attribute
       render_api_error!(message.join(' '), 400)
     end
 
diff --git a/lib/api/job_artifacts.rb b/lib/api/job_artifacts.rb
index e0c5449e65e..2a8fa7659bf 100644
--- a/lib/api/job_artifacts.rb
+++ b/lib/api/job_artifacts.rb
@@ -52,7 +52,7 @@ module API
 
         path = Gitlab::Ci::Build::Artifacts::Path
           .new(params[:artifact_path])
-        not_found! unless path.valid?
+        bad_request! unless path.valid?
 
         send_artifacts_entry(build, path)
       end

From f1bb89271dad5310575b73632b0f074fd5c46a21 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Sep 2017 11:32:21 +0200
Subject: [PATCH 10/14] Add changelog entry for new artifacts API endpoint

---
 .../feature-gb-download-single-job-artifact-using-api.yml    | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 changelogs/unreleased/feature-gb-download-single-job-artifact-using-api.yml

diff --git a/changelogs/unreleased/feature-gb-download-single-job-artifact-using-api.yml b/changelogs/unreleased/feature-gb-download-single-job-artifact-using-api.yml
new file mode 100644
index 00000000000..920679ca166
--- /dev/null
+++ b/changelogs/unreleased/feature-gb-download-single-job-artifact-using-api.yml
@@ -0,0 +1,5 @@
+---
+title: Make it possible to download a single job artifact file using the API
+merge_request: 14027
+author:
+type: added

From 49e9e0b2a0ea0c1b8b6b472d320a63067bbbf79c Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Sep 2017 11:40:20 +0200
Subject: [PATCH 11/14] Add basic docs about new artifact access API endpoint

---
 doc/api/jobs.md | 38 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/doc/api/jobs.md b/doc/api/jobs.md
index 297115e94ac..d60c7c12881 100644
--- a/doc/api/jobs.md
+++ b/doc/api/jobs.md
@@ -320,11 +320,11 @@ Response:
 
 [ce-2893]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/2893
 
-## Download the artifacts file
+## Download the artifacts archive
 
 > [Introduced][ce-5347] in GitLab 8.10.
 
-Download the artifacts file from the given reference name and job provided the
+Download the artifacts archive from the given reference name and job provided the
 job finished successfully.
 
 ```
@@ -354,6 +354,40 @@ Example response:
 
 [ce-5347]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5347
 
+## Download a single artifact file
+
+> Introduced in GitLab 10.0
+
+Download a single artifact file from within the job's artifacts archive.
+
+Only a single file is going to be extracted from the archive and streamed to a client.
+
+```
+GET /projects/:id/jobs/:job_id/artifacts/*artifact_path
+```
+
+Parameters
+
+| Attribute       | Type    | Required | Description               |
+|-----------------|---------|----------|-------------------------- |
+| `id`            | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user       |
+| `job_id  `      | integer | yes      | The unique job identifier |
+| `artifact_path` | string  | yes      | Path to a file inside the artifacts archive |
+
+Example request:
+
+```
+curl --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" "https://gitlab.example.com/api/v4/projects/1/jobs/5/artifacts/some/release/file.pdf"
+```
+
+Example response:
+
+| Status    | Description                          |
+|-----------|--------------------------------------|
+| 200       | Sends a single artifact file         |
+| 400       | Invalid path provided                |
+| 404       | Build not found or no file/artifacts |
+
 ## Get a trace file
 
 Get a trace of a specific job of a project

From b03d9e506d88cbfa13d34e12e0595dbb92f46e7d Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Sep 2017 11:42:14 +0200
Subject: [PATCH 12/14] Remove unneeded string interpolation from entry class

---
 lib/gitlab/ci/build/artifacts/metadata/entry.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/gitlab/ci/build/artifacts/metadata/entry.rb b/lib/gitlab/ci/build/artifacts/metadata/entry.rb
index 321ddfd8c29..22941d48edf 100644
--- a/lib/gitlab/ci/build/artifacts/metadata/entry.rb
+++ b/lib/gitlab/ci/build/artifacts/metadata/entry.rb
@@ -113,7 +113,7 @@ module Gitlab
             end
 
             def inspect
-              "#{self.class.name}: #{self.to_s}"
+              "#{self.class.name}: #{self}"
             end
 
             private

From deaa7f54e016b6ae1051c38abb95586451f470c1 Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Sep 2017 14:27:42 +0200
Subject: [PATCH 13/14] Fix artifacts controller specs

---
 spec/controllers/projects/artifacts_controller_spec.rb | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb
index d2c613a2423..caa63e7bd22 100644
--- a/spec/controllers/projects/artifacts_controller_spec.rb
+++ b/spec/controllers/projects/artifacts_controller_spec.rb
@@ -81,14 +81,6 @@ describe Projects::ArtifactsController do
         expect(params['Entry']).to eq(Base64.encode64('ci_artifacts.txt'))
       end
     end
-
-    context 'when the file does not exist' do
-      it 'responds Not Found' do
-        get :raw, namespace_id: project.namespace, project_id: project, job_id: job, path: 'unknown'
-
-        expect(response).to be_not_found
-      end
-    end
   end
 
   describe 'GET latest_succeeded' do

From ec7a12da818898b278a3e47a9b96ccebafbe5b4c Mon Sep 17 00:00:00 2001
From: Grzegorz Bizon <grzesiek.bizon@gmail.com>
Date: Wed, 6 Sep 2017 14:45:28 +0200
Subject: [PATCH 14/14] Revert moving authorization hook in jobs API

---
 lib/api/jobs.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/api/jobs.rb b/lib/api/jobs.rb
index 19161aabd37..3c1c412ba42 100644
--- a/lib/api/jobs.rb
+++ b/lib/api/jobs.rb
@@ -2,12 +2,12 @@ module API
   class Jobs < Grape::API
     include PaginationParams
 
+    before { authenticate! }
+
     params do
       requires :id, type: String, desc: 'The ID of a project'
     end
     resource :projects, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
-      before { authenticate! }
-
       helpers do
         params :optional_scope do
           optional :scope, types: [String, Array[String]], desc: 'The scope of builds to show',