Merge branch '46869-deploy-tokens-failed-to-clone-lfs-repository' into 'master'

Resolve "Deploy Tokens failed to clone LFS repository"

Closes #46869

See merge request gitlab-org/gitlab-ce!20729

app/controllers/concerns/lfs_request.rb

@@ -71,7 +71,22 @@ module LfsRequest
   def lfs_download_access?
     return false unless project.lfs_enabled?
 
-    ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code?
+    ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code? || deploy_token_can_download_code?
+  end
+
+  def deploy_token_can_download_code?
+    deploy_token_present? &&
+      deploy_token.project == project &&
+      deploy_token.active? &&
+      deploy_token.read_repository?
+  end
+
+  def deploy_token_present?
+    user && user.is_a?(DeployToken)
+  end
+
+  def deploy_token
+    user
   end
 
   def lfs_upload_access?
@@ -86,7 +101,7 @@ module LfsRequest
   end
 
   def user_can_download_code?
-    has_authentication_ability?(:download_code) && can?(user, :download_code, project)
+    has_authentication_ability?(:download_code) && can?(user, :download_code, project) && !deploy_token_present?
   end
 
   def build_can_download_code?

app/models/deploy_token.rb

@@ -27,7 +27,7 @@ class DeployToken < ActiveRecord::Base
   end
 
   def active?
-    !revoked
+    !revoked && expires_at > Date.today
   end
 
   def scopes
@@ -58,6 +58,10 @@ class DeployToken < ActiveRecord::Base
     write_attribute(:expires_at, value.presence || Forever.date)
   end
 
+  def admin?
+    false
+  end
+
   private
 
   def ensure_at_least_one_scope

changelogs/unreleased/46869-deploy-tokens-failed-to-clone-lfs-repository.yml

+---
+title: Allow cloning LFS repositories through DeployTokens
+merge_request: 20729
+author:
+type: other

spec/models/deploy_token_spec.rb

@@ -62,11 +62,18 @@ describe DeployToken do
       end
     end
 
-    context "when it hasn't been revoked" do
+    context "when it hasn't been revoked and is not expired" do
       it 'should return true' do
         expect(deploy_token.active?).to be_truthy
       end
     end
+
+    context "when it hasn't been revoked and is expired" do
+      it 'should return true' do
+        deploy_token.update_attribute(:expires_at, Date.today - 5.days)
+        expect(deploy_token.active?).to be_falsy
+      end
+    end
   end
 
   describe '#username' do

spec/requests/lfs_http_spec.rb

@@ -575,6 +575,40 @@ describe 'Git LFS API and storage' do
         end
       end
 
+      context 'when using Deploy Tokens' do
+        let(:project) { create(:project, :repository) }
+        let(:authorization) { authorize_deploy_token }
+        let(:update_user_permissions) { nil }
+        let(:role) { nil }
+        let(:update_lfs_permissions) 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 requests'
+        end
+
+        context 'when Deploy Token is not valid' do
+          let(:deploy_token) { create(:deploy_token, projects: [project], read_repository: false) }
+
+          it 'responds with access denied' do
+            expect(response).to have_gitlab_http_status(401)
+          end
+        end
+
+        context 'when Deploy Token is not related to the project' do
+          let(:another_project) { create(:project, :repository) }
+          let(:deploy_token) { create(:deploy_token, projects: [another_project]) }
+
+          it 'responds with access forbidden' do
+            # We render 404, to prevent data leakage about existence of the project
+            expect(response).to have_gitlab_http_status(404)
+          end
+        end
+      end
+
       context 'when build is authorized as' do
         let(:authorization) { authorize_ci_project }
 
@@ -1381,6 +1415,10 @@ describe 'Git LFS API and storage' do
     ActionController::HttpAuthentication::Basic.encode_credentials(user.username, Gitlab::LfsToken.new(user).token)
   end
 
+  def authorize_deploy_token
+    ActionController::HttpAuthentication::Basic.encode_credentials(deploy_token.username, deploy_token.token)
+  end
+
   def post_lfs_json(url, body = nil, headers = nil)
     post(url, body.try(:to_json), (headers || {}).merge('Content-Type' => LfsRequest::CONTENT_TYPE))
   end