Test against public projects

app/controllers/projects/artifacts_controller.rb

@@ -29,7 +29,7 @@ class Projects::ArtifactsController < Projects::ApplicationController
     blob = @entry.blob
     conditionally_expand_blob(blob)
 
-    if blob.external_link?
+    if blob.external_link?(build)
       redirect_to blob.external_url(@project, build)
     else
       respond_to do |format|

app/models/ci/artifact_blob.rb

@@ -34,7 +34,7 @@ module Ci
     end
 
     def external_url(project, job)
-      return unless external_link?
+      return unless external_link?(job)
 
       components = project.full_path_components
       components << "-/jobs/#{job.id}/artifacts/file/#{path}"
@@ -43,10 +43,11 @@ module Ci
       "#{pages_config.protocol}://#{components[0]}.#{pages_config.host}/#{artifact_path}"
     end
 
-    def external_link?
+    def external_link?(job)
       pages_config.enabled &&
         pages_config.artifacts_server &&
-        EXTENTIONS_SERVED_BY_PAGES.include?(File.extname(name))
+        EXTENTIONS_SERVED_BY_PAGES.include?(File.extname(name)) &&
+        job.project.public?
     end
 
     private

app/views/projects/artifacts/_tree_file.html.haml

 - blob = file.blob
 - path_to_file = file_project_job_artifacts_path(@project, @build, path: file.path)
-- external_link = blob.external_link? && @project.public?
+- external_link = blob.external_link?(@build)
 
 %tr.tree-item.js-artifact-tree-row{ data: { link: path_to_file, external_link: "#{external_link}" } }
   %td.tree-item-file-name

spec/controllers/projects/artifacts_controller_spec.rb

@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Projects::ArtifactsController do
   set(:user) { create(:user) }
-  set(:project) { create(:project, :repository) }
+  set(:project) { create(:project, :repository, :public) }
 
   let(:pipeline) do
     create(:ci_pipeline,
@@ -91,6 +91,25 @@ describe Projects::ArtifactsController do
         end
       end
     end
+
+    context 'when the project is private' do
+      let(:private_project) { create(:project, :repository, :private) }
+      let(:pipeline) { create(:ci_pipeline, project: private_project) }
+      let(:job) { create(:ci_build, :success, :artifacts, pipeline: pipeline) }
+
+      before do
+        private_project.add_developer(user)
+
+        allow(Gitlab.config.pages).to receive(:artifacts_server).and_return(true)
+      end
+
+      it 'does not redirect the request' do
+        get :file, namespace_id: private_project.namespace, project_id: private_project, job_id: job, path: 'ci_artifacts.txt'
+
+        expect(response).to have_http_status(:ok)
+        expect(response).to render_template('projects/artifacts/file')
+      end
+    end
   end
 
   describe 'GET raw' do

spec/features/projects/artifacts/browse_spec.rb

@@ -28,11 +28,12 @@ feature 'Browse artifact', :js do
     before do
       allow(Gitlab.config.pages).to receive(:enabled).and_return(true)
       allow(Gitlab.config.pages).to receive(:artifacts_server).and_return(true)
-
-      visit browse_url
     end
 
+    context 'when the project is public' do
       it "shows external link icon and styles" do
+        visit browse_url
+
         link = first('.tree-item-file-external-link')
 
         expect(page).to have_link('doc_sample.txt', href: file_project_job_artifacts_path(project, job, path: txt_entry.blob.path))
@@ -42,4 +43,25 @@ feature 'Browse artifact', :js do
         expect(page).to have_selector('.js-artifact-tree-external-icon')
       end
     end
+
+    context 'when the project is private' do
+      let!(:private_project) { create(:project, :private) }
+      let(:pipeline) { create(:ci_empty_pipeline, project: private_project) }
+      let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) }
+      let(:user) { create(:user) }
+
+      before do
+        private_project.add_developer(user)
+
+        sign_in(user)
+      end
+
+      it 'shows internal link styles' do
+        visit browse_project_job_artifacts_path(private_project, job, 'other_artifacts_0.1.2')
+
+        expect(page).to have_link('doc_sample.txt')
+        expect(page).not_to have_selector('.js-artifact-tree-external-icon')
+      end
+    end
+  end
 end