Handle LFS token creation and retrieval in the same method, and in the same Redis connection.lfs-ssh-authorization-fix

Reset expiry time of token, if token is retrieved again before it expires.

7 files changed, 21 insertions, 24 deletions

diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 090d04544da..9a5d1ece070 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -90,7 +90,7 @@ module API
 
         {
           username: token_handler.actor_name,
-          lfs_token: token_handler.generate,
+          lfs_token: token_handler.token,
           repository_http_path: project.http_url_to_repo
         }
       end
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 7c0f2115d43..aca5d0020cf 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -124,7 +124,7 @@ module Gitlab
             read_authentication_abilities
           end
 
-        Result.new(actor, nil, token_handler.type, authentication_abilities) if Devise.secure_compare(token_handler.value, password)
+        Result.new(actor, nil, token_handler.type, authentication_abilities) if Devise.secure_compare(token_handler.token, password)
       end
 
       def build_access_token_check(login, password)
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index f31444b2b07..7b3bbcf6a32 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -17,21 +17,18 @@ module Gitlab
         end
     end
 
-    def generate
-      return value if value
-
-      token = Devise.friendly_token(TOKEN_LENGTH)
-
+    def token
       Gitlab::Redis.with do |redis|
-        redis.set(redis_key, token, ex: EXPIRY_TIME)
-      end
+        token = redis.get(redis_key)
 
-      token
-    end
+        if token
+          redis.expire(redis_key, EXPIRY_TIME)
+        else
+          token = Devise.friendly_token(TOKEN_LENGTH)
+          redis.set(redis_key, token, ex: EXPIRY_TIME)
+        end
 
-    def value
-      Gitlab::Redis.with do |redis|
-        redis.get(redis_key)
+        token
       end
     end
 
diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index 745fbc0df45..c9d64e99f88 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -64,7 +64,7 @@ describe Gitlab::Auth, lib: true do
     it 'recognizes user lfs tokens' do
       user = create(:user)
       ip = 'ip'
-      token = Gitlab::LfsToken.new(user).generate
+      token = Gitlab::LfsToken.new(user).token
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
       expect(gl_auth.find_for_git_client(user.username, token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, nil, :lfs_token, full_authentication_abilities))
@@ -73,7 +73,7 @@ describe Gitlab::Auth, lib: true do
     it 'recognizes deploy key lfs tokens' do
       key = create(:deploy_key)
       ip = 'ip'
-      token = Gitlab::LfsToken.new(key).generate
+      token = Gitlab::LfsToken.new(key).token
 
       expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: "lfs+deploy-key-#{key.id}")
       expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities))
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
index 9f04f67e0a8..e9c1163e22a 100644
--- a/spec/lib/gitlab/lfs_token_spec.rb
+++ b/spec/lib/gitlab/lfs_token_spec.rb
@@ -1,10 +1,10 @@
 require 'spec_helper'
 
 describe Gitlab::LfsToken, lib: true do
-  describe '#generate and #value' do
+  describe '#token' do
     shared_examples 'an LFS token generator' do
       it 'returns a randomly generated token' do
-        token = handler.generate
+        token = handler.token
 
         expect(token).not_to be_nil
         expect(token).to be_a String
@@ -12,9 +12,9 @@ describe Gitlab::LfsToken, lib: true do
       end
 
       it 'returns the correct token based on the key' do
-        token = handler.generate
+        token = handler.token
 
-        expect(handler.value).to eq(token)
+        expect(handler.token).to eq(token)
       end
     end
 
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 46e8e6f1169..f0f590b0331 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -111,7 +111,7 @@ describe API::API, api: true  do
 
         expect(response).to have_http_status(200)
         expect(json_response['username']).to eq(user.username)
-        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value)
+        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).token)
 
         expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
       end
@@ -131,7 +131,7 @@ describe API::API, api: true  do
 
         expect(response).to have_http_status(200)
         expect(json_response['username']).to eq("lfs+deploy-key-#{key.id}")
-        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value)
+        expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).token)
         expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
       end
     end
diff --git a/spec/requests/lfs_http_spec.rb b/spec/requests/lfs_http_spec.rb
index a84be1b7c9e..dbdf83a0dff 100644
--- a/spec/requests/lfs_http_spec.rb
+++ b/spec/requests/lfs_http_spec.rb
@@ -1133,11 +1133,11 @@ describe 'Git LFS API and storage' do
   end
 
   def authorize_deploy_key
-    ActionController::HttpAuthentication::Basic.encode_credentials("lfs+deploy-key-#{key.id}", Gitlab::LfsToken.new(key).generate)
+    ActionController::HttpAuthentication::Basic.encode_credentials("lfs+deploy-key-#{key.id}", Gitlab::LfsToken.new(key).token)
   end
 
   def authorize_user_key
-    ActionController::HttpAuthentication::Basic.encode_credentials(user.username, Gitlab::LfsToken.new(user).generate)
+    ActionController::HttpAuthentication::Basic.encode_credentials(user.username, Gitlab::LfsToken.new(user).token)
   end
 
   def fork_project(project, user, object = nil)