Do not require API authentication if artifacts are public

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',
@@ -72,40 +72,6 @@ module API
       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
       params do
@@ -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)
+
+            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)
 
-          expect(response).to have_http_status(401)
+            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