Add missing tests

lib/api/helpers.rb

@@ -357,6 +357,7 @@ module API
 
     def private_token
       params[APIGuard::PRIVATE_TOKEN_PARAM] || env[APIGuard::PRIVATE_TOKEN_HEADER]
+    end
 
     def job_token_authentication?
       initial_current_user && initial_current_user == find_user_by_job_token

spec/requests/api/helpers_spec.rb

@@ -34,11 +34,13 @@ describe API::Helpers do
 
   def clear_env
     env.delete(API::APIGuard::PRIVATE_TOKEN_HEADER)
+    env.delete(API::APIGuard::JOB_TOKEN_HEADER)
     env.delete(API::Helpers::SUDO_HEADER)
   end
 
   def clear_param
     params.delete(API::APIGuard::PRIVATE_TOKEN_PARAM)
+    params.delete(API::APIGuard::JOB_TOKEN_PARAM)
     params.delete(API::Helpers::SUDO_PARAM)
   end
 
@@ -199,6 +201,38 @@ describe API::Helpers do
       end
     end
 
+    describe "when authenticating using a job token" do
+      let(:job) { create(:ci_build) }
+  
+      it "returns nil for an invalid token" do
+        env[API::APIGuard::JOB_TOKEN_HEADER] = 'invalid token'
+        allow_any_instance_of(self.class).to receive(:doorkeeper_guard){ false }
+
+        expect(current_user).to be_nil
+      end
+
+      it "returns nil for a user without access" do
+        env[API::APIGuard::JOB_TOKEN_HEADER] = job.token
+        allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(false)
+
+        expect(current_user).to be_nil
+      end
+
+      it "returns nil for a user with access, but route not allowed to be authenticated" do
+        env[API::APIGuard::JOB_TOKEN_HEADER] = job.token
+        allow_any_instance_of(Gitlab::UserAccess).to receive(:allowed?).and_return(true)
+
+        expect(current_user).to be_nil
+      end
+
+      it "authenticates as user when route is allowed" do
+        env[API::APIGuard::JOB_TOKEN_HEADER] = job.token
+        route_setting(:authentication) = { job_token_allowed: true }
+
+        expect(current_user).to eq(user)
+      end
+    end
+
     context 'sudo usage' do
       context 'with admin' do
         context 'with header' do

spec/requests/api/jobs_spec.rb

@@ -191,7 +191,20 @@ describe API::Jobs do
   end
 
   describe 'GET /projects/:id/jobs/:job_id/artifacts' do
-    context 'normal authenticatin' 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_http_status(200)
+        expect(response.headers).to include(download_headers)
+        expect(response.body).to match_file(job.artifacts_file.file.file)
+      end
+    end
+
+    context 'normal authentication' do
       before do
         stub_artifacts_object_storage
         job
@@ -203,16 +216,7 @@ describe API::Jobs do
           let(:job) { create(:ci_build, :artifacts, pipeline: pipeline) }
 
           context 'authorized user' 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_http_status(200)
-              expect(response.headers).to include(download_headers)
-              expect(response.body).to match_file(job.artifacts_file.file.file)
-            end
+            it_behaves_like 'downloads artifact'
           end
 
           context 'unauthorized user' do
@@ -238,22 +242,25 @@ describe API::Jobs do
       end
     end
 
-    context 'authorized by ci_job_token' do
-      let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: user) }
+    context 'authorized by job_token' do
+      let(:job) { create(:ci_build, :artifacts, pipeline: pipeline, user: api_user) }
 
-      let(:download_headers) do
-        { 'Content-Transfer-Encoding' => 'binary',
-          'Content-Disposition' => 'attachment; filename=ci_build_artifacts.zip' }
+      before do
+        get api("/projects/#{project.id}/jobs/#{job.id}/artifacts"), job_token: job.token
       end
       
-      before do
-        get api("/projects/#{project.id}/jobs/#{job.id}/artifacts"), ci_job_token: job.token
+      context 'user is developer' do
+        let(:api_user) { user }
+      
+        it_behaves_like 'downloads artifact'
       end
 
-      it 'returns specific job artifacts' do
-        expect(response).to have_http_status(200)
-        expect(response.headers).to include(download_headers)
-        expect(response.body).to match_file(job.artifacts_file.file.file)
+      context 'user is admin, but not member' do
+        let(:api_user) { create(:admin) }
+      
+        it 'does not allow to see that artfiact is present' do
+          expect(response).to have_http_status(404)
+        end
       end
     end
   end
@@ -364,6 +371,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_http_status(404)
+          end
+        end
+      end
     end
   end