Backport ee-40781-os-to-ce

12 files changed, 147 insertions, 60 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 0cef4d335dd..452c6784bd4 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -188,6 +188,8 @@ class Project < ActiveRecord::Base
   has_many :todos
   has_many :notification_settings, as: :source, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent
 
+  has_many :internal_ids
+
   has_one :import_data, class_name: 'ProjectImportData', inverse_of: :project, autosave: true
   has_one :project_feature, inverse_of: :project
   has_one :statistics, class_name: 'ProjectStatistics'
@@ -290,7 +292,6 @@ class Project < ActiveRecord::Base
   scope :non_archived, -> { where(archived: false) }
   scope :for_milestones, ->(ids) { joins(:milestones).where('milestones.id' => ids).distinct }
   scope :with_push, -> { joins(:events).where('events.action = ?', Event::PUSHED) }
-
   scope :with_project_feature, -> { joins('LEFT JOIN project_features ON projects.id = project_features.project_id') }
   scope :with_statistics, -> { includes(:statistics) }
   scope :with_shared_runners, -> { where(shared_runners_enabled: true) }
diff --git a/app/models/upload.rb b/app/models/upload.rb
index cf71a7b76fc..e94a81c8770 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -1,4 +1,6 @@
 class Upload < ActiveRecord::Base
+  prepend EE::Upload
+
   # Upper limit for foreground checksum processing
   CHECKSUM_THRESHOLD = 100.megabytes
 
diff --git a/app/uploaders/lfs_object_uploader.rb b/app/uploaders/lfs_object_uploader.rb
index e7cce1bbb0a..eb521a22ebc 100644
--- a/app/uploaders/lfs_object_uploader.rb
+++ b/app/uploaders/lfs_object_uploader.rb
@@ -2,11 +2,6 @@ class LfsObjectUploader < GitlabUploader
   extend Workhorse::UploadPath
   include ObjectStorage::Concern
 
-  # LfsObject are in `tmp/upload` instead of `tmp/uploads`
-  def self.workhorse_upload_path
-    File.join(root, 'tmp/upload')
-  end
-
   storage_options Gitlab.config.lfs
 
   def filename
diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example
index 075ac022674..05299adfa93 100644
--- a/config/gitlab.yml.example
+++ b/config/gitlab.yml.example
@@ -182,19 +182,18 @@ production: &base
     # storage_path: public/
     # base_dir: uploads/-/system
     object_store:
-      enabled: true
-      remote_directory: uploads # Bucket name
-      # background_upload: false # Temporary option to limit automatic upload (Default: true)
-      # proxy_download: false # Passthrough all downloads via GitLab instead of using Redirects to Object Storage
-      connection:
-        provider: AWS
-        aws_access_key_id: AWS_ACCESS_KEY_ID
-        aws_secret_access_key: AWS_SECRET_ACCESS_KEY
-        region: eu-central-1
-        # Use the following options to configure an AWS compatible host
-        # host: 'localhost' # default: s3.amazonaws.com
-        # endpoint: 'http://127.0.0.1:9000' # default: nil
-        # path_style: true # Use 'host/bucket_name/object' instead of 'bucket_name.host/object'
+      enabled: false
+    # remote_directory: uploads # Bucket name
+    # background_upload: false # Temporary option to limit automatic upload (Default: true)
+    # proxy_download: false # Passthrough all downloads via GitLab instead of using Redirects to Object Storage
+    # connection:
+    #   provider: AWS
+    #   aws_access_key_id: AWS_ACCESS_KEY_ID
+    #   aws_secret_access_key: AWS_SECRET_ACCESS_KEY
+    #   region: eu-central-1
+    #   host: 'localhost' # default: s3.amazonaws.com
+    #   endpoint: 'http://127.0.0.1:9000' # default: nil
+    #   path_style: true # Use 'host/bucket_name/object' instead of 'bucket_name.host/object'
 
   ## GitLab Pages
   pages:
@@ -719,7 +718,6 @@ test:
         region: eu-central-1
   uploads:
     storage_path: tmp/tests/public
-    enabled: true
     object_store:
       enabled: false
       connection:
diff --git a/config/sidekiq_queues.yml b/config/sidekiq_queues.yml
index 1c9c1d67669..c811034b29d 100644
--- a/config/sidekiq_queues.yml
+++ b/config/sidekiq_queues.yml
@@ -64,7 +64,6 @@
   - [update_user_activity, 1]
   - [propagate_service_template, 1]
   - [background_migration, 1]
-  - [object_storage_upload, 1]
   - [gcp_cluster, 1]
   - [project_migrate_hashed_storage, 1]
   - [storage_migrator, 1]
diff --git a/db/migrate/20170601163708_add_artifacts_store_to_ci_build.rb b/db/migrate/20170601163708_add_artifacts_store_to_ci_build.rb
index deba890a478..e82109190a7 100644
--- a/db/migrate/20170601163708_add_artifacts_store_to_ci_build.rb
+++ b/db/migrate/20170601163708_add_artifacts_store_to_ci_build.rb
@@ -3,15 +3,8 @@ class AddArtifactsStoreToCiBuild < ActiveRecord::Migration
 
   DOWNTIME = false
 
-  disable_ddl_transaction!
-
-  def up
-    add_column_with_default(:ci_builds, :artifacts_file_store, :integer, default: 1)
-    add_column_with_default(:ci_builds, :artifacts_metadata_store, :integer, default: 1)
-  end
-
-  def down
-    remove_column(:ci_builds, :artifacts_file_store)
-    remove_column(:ci_builds, :artifacts_metadata_store)
+  def change
+    add_column(:ci_builds, :artifacts_file_store, :integer)
+    add_column(:ci_builds, :artifacts_metadata_store, :integer)
   end
 end
diff --git a/ee/spec/requests/api/jobs_spec.rb b/ee/spec/requests/api/jobs_spec.rb
new file mode 100644
index 00000000000..9f8b502867d
--- /dev/null
+++ b/ee/spec/requests/api/jobs_spec.rb
@@ -0,0 +1,72 @@
+require 'spec_helper'
+
+describe API::Jobs do
+  set(:project) do
+    create(:project, :repository, public_builds: false)
+  end
+
+  set(:pipeline) do
+    create(:ci_empty_pipeline, project: project,
+                               sha: project.commit.id,
+                               ref: project.default_branch)
+  end
+
+  let!(:job) { create(:ci_build, :success, pipeline: pipeline) }
+
+  let(:user) { create(:user) }
+  let(:api_user) { user }
+  let(:reporter) { create(:project_member, :reporter, project: project).user }
+  let(:cross_project_pipeline_enabled) { true }
+
+  before do
+    stub_licensed_features(cross_project_pipelines: cross_project_pipeline_enabled)
+    project.add_developer(user)
+  end
+
+  describe 'GET /projects/:id/jobs/:job_id/artifacts' do
+    shared_examples 'downloads artifact' do
+      let(:download_headers) do
+        { 'Content-Transfer-Encoding' => 'binary',
+          'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' }
+      end
+
+      it 'returns specific job artifacts' do
+        expect(response).to have_gitlab_http_status(200)
+        expect(response.headers).to include(download_headers)
+        expect(response.body).to match_file(job.artifacts_file.file.file)
+      end
+    end
+
+    context 'authorized by job_token' do
+      let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) }
+
+      before do
+        get api("/projects/#{project.id}/jobs/#{job.id}/artifacts"), job_token: job.token
+      end
+
+      context 'user is developer' do
+        let(:api_user) { user }
+
+        it_behaves_like 'downloads artifact'
+      end
+
+      context 'when anonymous user is accessing private artifacts' do
+        let(:api_user) { nil }
+
+        it 'hides artifacts and rejects request' do
+          expect(project).to be_private
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+
+      context 'feature is disabled for EES' do
+        let(:api_user) { user }
+        let(:cross_project_pipeline_enabled) { false }
+
+        it 'disallows access to the artifacts' do
+          expect(response).to have_gitlab_http_status(404)
+        end
+      end
+    end
+  end
+end
diff --git a/lib/gitlab/ci/trace/http_io.rb b/lib/gitlab/ci/trace/http_io.rb
index 5256f7999c1..ac4308f4e2c 100644
--- a/lib/gitlab/ci/trace/http_io.rb
+++ b/lib/gitlab/ci/trace/http_io.rb
@@ -37,6 +37,10 @@ module Gitlab
         end
 
         def path
+          nil
+        end
+
+        def url
           @uri.to_s
         end
 
diff --git a/spec/controllers/concerns/send_file_upload_spec.rb b/spec/controllers/concerns/send_file_upload_spec.rb
index da4e955f6a3..f4c99ea4064 100644
--- a/spec/controllers/concerns/send_file_upload_spec.rb
+++ b/spec/controllers/concerns/send_file_upload_spec.rb
@@ -64,10 +64,12 @@ describe SendFileUpload do
         end
 
         it 'sends a file' do
-          subject
+          headers = double
+          expect(headers).to receive(:store).with(Gitlab::Workhorse::SEND_DATA_HEADER, /^send-url:/)
+          expect(controller).to receive(:headers) { headers }
+          expect(controller).to receive(:head).with(:ok)
 
-          is_expected.to start_with(Gitlab::Workhorse::SEND_DATA_HEADER)
-          is_expected.to end_with(/^send-url:/)
+          subject
         end
       end
 
diff --git a/spec/features/projects/import_export/test_project_export.tar.gz b/spec/features/projects/import_export/test_project_export.tar.gz
index 0cc68aff494..ecb7651acad 100644
--- a/spec/features/projects/import_export/test_project_export.tar.gz
+++ b/spec/features/projects/import_export/test_project_export.tar.gz
diff --git a/spec/lib/gitlab/ci/trace/http_io_spec.rb b/spec/lib/gitlab/ci/trace/http_io_spec.rb
index b839ef7ce36..5474e2f518c 100644
--- a/spec/lib/gitlab/ci/trace/http_io_spec.rb
+++ b/spec/lib/gitlab/ci/trace/http_io_spec.rb
@@ -7,26 +7,6 @@ describe Gitlab::Ci::Trace::HttpIO do
   let(:url) { remote_trace_url }
   let(:size) { remote_trace_size }
 
-  describe 'Interchangeability between IO and HttpIO' do
-    EXCEPT_METHODS = %i[read_nonblock raw raw! cooked cooked! getch echo= echo?
-                        winsize winsize= iflush oflush ioflush beep goto cursor cursor= pressed?
-                        getpass write_nonblock stat pathconf wait_readable wait_writable getbyte <<
-                        wait lines bytes chars codepoints getc readpartial set_encoding printf print
-                        putc puts readlines gets each each_byte each_char each_codepoint to_io reopen
-                        syswrite to_i fileno sysread fdatasync fsync sync= sync lineno= lineno readchar
-                        ungetbyte readbyte ungetc nonblock= nread rewind pos= eof close_on_exec?
-                        close_on_exec= closed? close_read close_write isatty tty? binmode? sysseek
-                        advise ioctl fcntl pid external_encoding internal_encoding autoclose? autoclose=
-                        posix_fileno nonblock? ready? noecho nonblock].freeze
-
-    it 'HttpIO covers core interfaces in IO' do
-      expected_interfaces = ::IO.instance_methods(false)
-      expected_interfaces -= EXCEPT_METHODS
-
-      expect(expected_interfaces - described_class.instance_methods).to be_empty
-    end
-  end
-
   describe '#close' do
     subject { http_io.close }
 
@@ -48,6 +28,12 @@ describe Gitlab::Ci::Trace::HttpIO do
   describe '#path' do
     subject { http_io.path }
 
+    it { is_expected.to be_nil }
+  end
+
+  describe '#url' do
+    subject { http_io.url }
+
     it { is_expected.to eq(url) }
   end
 
diff --git a/spec/requests/api/jobs_spec.rb b/spec/requests/api/jobs_spec.rb
index 62ed9fd00a1..81335e72350 100644
--- a/spec/requests/api/jobs_spec.rb
+++ b/spec/requests/api/jobs_spec.rb
@@ -21,6 +21,7 @@ describe API::Jobs do
   let(:guest) { create(:project_member, :guest, project: project).user }
 
   before do
+    stub_licensed_features(cross_project_pipelines: true)
     project.add_developer(user)
   end
 
@@ -316,11 +317,6 @@ describe API::Jobs do
       end
     end
 
-    before do
-      stub_artifacts_object_storage
-      get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user)
-    end
-
     context 'normal authentication' do
       context 'job with artifacts' do
         context 'when artifacts are stored locally' do
@@ -344,8 +340,10 @@ describe API::Jobs do
         end
 
         context 'when artifacts are stored remotely' do
+          let(:proxy_download) { false }
+
           before do
-            stub_artifacts_object_storage
+            stub_artifacts_object_storage(proxy_download: proxy_download)
           end
 
           let(:job) { create(:ci_build, pipeline: pipeline) }
@@ -357,6 +355,20 @@ describe API::Jobs do
             get api("/projects/#{project.id}/jobs/#{job.id}/artifacts", api_user)
           end
 
+          context 'when proxy download is enabled' do
+            let(:proxy_download) { true }
+
+            it 'responds with the workhorse send-url' do
+              expect(response.headers[Gitlab::Workhorse::SEND_DATA_HEADER]).to start_with("send-url:")
+            end
+          end
+
+          context 'when proxy download is disabled' do
+            it 'returns location redirect' do
+              expect(response).to have_gitlab_http_status(302)
+            end
+          end
+
           context 'authorized user' do
             it 'returns the file remote URL' do
               expect(response).to redirect_to(artifact.file.url)
@@ -495,6 +507,29 @@ describe API::Jobs do
 
         it_behaves_like 'a valid file'
       end
+
+      context 'when using job_token to authenticate' do
+        before do
+          pipeline.reload
+          pipeline.update(ref: 'master',
+                          sha: project.commit('master').sha)
+
+          get api("/projects/#{project.id}/jobs/artifacts/master/download"), job: job.name, job_token: job.token
+        end
+
+        context 'when user is reporter' do
+          it_behaves_like 'a valid file'
+        end
+
+        context 'when user is admin, but not member' do
+          let(:api_user) { create(:admin) }
+          let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) }
+
+          it 'does not allow to see that artfiact is present' do
+            expect(response).to have_gitlab_http_status(404)
+          end
+        end
+      end
     end
   end