Merge branch...

Merge branch '57353-git-push-fails-on-large-lfs-files-where-the-push-take-a-long-time' into 'master'

Provide expires_in in LFS authentication payload

Closes #57353

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

changelogs/unreleased/57353-git-push-fails-on-large-lfs-files-where-the-push-take-a-long-time.yml

+---
+title: Provide expires_in in LFS authentication payload
+merge_request: 25082
+author:
+type: fixed

lib/api/internal.rb

@@ -117,13 +117,7 @@ module API
           raise ActiveRecord::RecordNotFound.new("No key_id or user_id passed!")
         end
 
-        token_handler = Gitlab::LfsToken.new(actor)
-
-        {
-          username: token_handler.actor_name,
-          lfs_token: token_handler.token,
-          repository_http_path: project.http_url_to_repo
-        }
+        Gitlab::LfsToken.new(actor).authentication_payload(project.http_url_to_repo)
       end
       # rubocop: enable CodeReuse/ActiveRecord
 

lib/gitlab/lfs_token.rb

@@ -30,8 +30,8 @@ module Gitlab
         end
     end
 
-    def token(expire_time: DEFAULT_EXPIRE_TIME)
-      HMACToken.new(actor).token(expire_time)
+    def token
+      HMACToken.new(actor).token(DEFAULT_EXPIRE_TIME)
     end
 
     def token_valid?(token_to_check)
@@ -47,6 +47,15 @@ module Gitlab
       user? ? :lfs_token : :lfs_deploy_token
     end
 
+    def authentication_payload(repository_http_path)
+      {
+        username: actor_name,
+        lfs_token: token,
+        repository_http_path: repository_http_path,
+        expires_in: DEFAULT_EXPIRE_TIME
+      }
+    end
+
     private # rubocop:disable Lint/UselessAccessModifier
 
     class HMACToken

spec/lib/gitlab/lfs_token_spec.rb

@@ -4,10 +4,8 @@ require 'spec_helper'
 
 describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do
   describe '#token' do
-    shared_examples 'an LFS token generator' do
+    shared_examples 'a valid LFS token' do
       it 'returns a computed token' do
-        expect(Settings).to receive(:attr_encrypted_db_key_base).and_return('fbnbv6hdjweo53qka7kza8v8swxc413c05pb51qgtfte0bygh1p2e508468hfsn5ntmjcyiz7h1d92ashpet5pkdyejg7g8or3yryhuso4h8o5c73h429d9c3r6bjnet').twice
-
         token = lfs_token.token
 
         expect(token).not_to be_nil
@@ -20,11 +18,7 @@ describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do
       let(:actor) { create(:user, username: 'test_user_lfs_1') }
       let(:lfs_token) { described_class.new(actor) }
 
-      before do
-        allow(actor).to receive(:encrypted_password).and_return('$2a$04$ETfzVS5spE9Hexn9wh6NUenCHG1LyZ2YdciOYxieV1WLSa8DHqOFO')
-      end
-
-      it_behaves_like 'an LFS token generator'
+      it_behaves_like 'a valid LFS token'
 
       it 'returns the correct username' do
         expect(lfs_token.actor_name).to eq(actor.username)
@@ -40,11 +34,7 @@ describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do
       let(:actor) { create(:key, user: user) }
       let(:lfs_token) { described_class.new(actor) }
 
-      before do
-        allow(user).to receive(:encrypted_password).and_return('$2a$04$C1GAMKsOKouEbhKy2JQoe./3LwOfQAZc.hC8zW2u/wt8xgukvnlV.')
-      end
-
-      it_behaves_like 'an LFS token generator'
+      it_behaves_like 'a valid LFS token'
 
       it 'returns the correct username' do
         expect(lfs_token.actor_name).to eq(user.username)
@@ -65,7 +55,7 @@ describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do
         allow(actor).to receive(:id).and_return(actor_id)
       end
 
-      it_behaves_like 'an LFS token generator'
+      it_behaves_like 'a valid LFS token'
 
       it 'returns the correct username' do
         expect(lfs_token.actor_name).to eq("lfs+deploy-key-#{actor_id}")
@@ -87,10 +77,6 @@ describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do
     let(:actor) { create(:user, username: 'test_user_lfs_1') }
     let(:lfs_token) { described_class.new(actor) }
 
-    before do
-      allow(actor).to receive(:encrypted_password).and_return('$2a$04$ETfzVS5spE9Hexn9wh6NUenCHG1LyZ2YdciOYxieV1WLSa8DHqOFO')
-    end
-
     context 'for an HMAC token' do
       before do
         # We're not interested in testing LegacyRedisDeviseToken here
@@ -240,4 +226,18 @@ describe Gitlab::LfsToken, :clean_gitlab_redis_shared_state do
       end
     end
   end
+
+  describe '#authentication_payload' do
+    it 'returns a Hash designed for gitlab-shell' do
+      actor = create(:user)
+      lfs_token = described_class.new(actor)
+      repo_http_path = 'http://localhost/user/repo.git'
+      authentication_payload = lfs_token.authentication_payload(repo_http_path)
+
+      expect(authentication_payload[:username]).to eq(actor.username)
+      expect(authentication_payload[:repository_http_path]).to eq(repo_http_path)
+      expect(authentication_payload[:lfs_token]).to be_a String
+      expect(authentication_payload[:expires_in]).to eq(described_class::DEFAULT_EXPIRE_TIME)
+    end
+  end
 end

spec/requests/api/internal_spec.rb

@@ -167,6 +167,7 @@ describe API::Internal do
         expect(response).to have_gitlab_http_status(200)
         expect(json_response['username']).to eq(user.username)
         expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
+        expect(json_response['expires_in']).to eq(Gitlab::LfsToken::DEFAULT_EXPIRE_TIME)
         expect(Gitlab::LfsToken.new(key).token_valid?(json_response['lfs_token'])).to be_truthy
       end