Merge branch 'security-deploy_token_project_auth' into 'master'

Add check for project access on deploy token check

Closes #221

See merge request gitlab-org/security/gitlab!815

changelogs/unreleased/security-deploy_token_project_auth.yml

+---
+title: Project access is checked during deploy token authentication
+merge_request:
+author:
+type: security

lib/gitlab/auth.rb

@@ -220,6 +220,9 @@ module Gitlab
         return unless token && login
         return if login != token.username
 
+        # Registry access (with jwt) does not have access to project
+        return if project && !token.has_access_to?(project)
+
         scopes = abilities_for_scopes(token.scopes)
 
         if valid_scoped_token?(token, all_available_scopes)

spec/lib/gitlab/auth_spec.rb

@@ -551,7 +551,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
 
         it 'fails if token is not related to project' do
           another_deploy_token = create(:deploy_token)
-          expect(gl_auth.find_for_git_client(login, another_deploy_token.token, project: project, ip: 'ip'))
+          expect(gl_auth.find_for_git_client(another_deploy_token.username, another_deploy_token.token, project: project, ip: 'ip'))
             .to eq(auth_failure)
         end
 
@@ -576,6 +576,13 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
 
           expect(subject).to eq(auth_success)
         end
+
+        it 'fails if token is not related to group' do
+          another_deploy_token = create(:deploy_token, :group, read_repository: true)
+
+          expect(gl_auth.find_for_git_client(another_deploy_token.username, another_deploy_token.token, project: project_with_group, ip: 'ip'))
+            .to eq(auth_failure)
+        end
       end
 
       context 'when the deploy token has read_registry as a scope' do

spec/requests/lfs_http_spec.rb

@@ -549,12 +549,6 @@ RSpec.describe 'Git LFS API and storage' do
           project.lfs_objects << lfs_object
         end
 
-        context 'when Deploy Token is valid' do
-          let(:deploy_token) { create(:deploy_token, projects: [project]) }
-
-          it_behaves_like 'an authorized request', renew_authorization: false
-        end
-
         context 'when Deploy Token is not valid' do
           let(:deploy_token) { create(:deploy_token, projects: [project], read_repository: false) }
 
@@ -564,7 +558,14 @@ RSpec.describe 'Git LFS API and storage' do
         context 'when Deploy Token is not related to the project' do
           let(:deploy_token) { create(:deploy_token, projects: [other_project]) }
 
-          it_behaves_like 'LFS http 404 response'
+          it_behaves_like 'LFS http 401 response'
+        end
+
+        # TODO: We should fix this test case that causes flakyness by alternating the result of the above test cases.
+        context 'when Deploy Token is valid' do
+          let(:deploy_token) { create(:deploy_token, projects: [project]) }
+
+          it_behaves_like 'an authorized request', renew_authorization: false
         end
       end