Refactored handling of the `LfsToken` and added functionality to it to simplify external code.

5 files changed, 46 insertions, 21 deletions

diff --git a/app/helpers/lfs_helper.rb b/app/helpers/lfs_helper.rb
index 2f5709a7395..031e7e72909 100644
--- a/app/helpers/lfs_helper.rb
+++ b/app/helpers/lfs_helper.rb
@@ -25,9 +25,7 @@ module LfsHelper
   def lfs_download_access?
     return false unless project.lfs_enabled?
 
-    return true if project.public?
-    return true if ci?
-    return true if lfs_deploy_key?
+    return true if project.public? || ci? || lfs_deploy_key?
 
     (user && user.can?(:download_code, project))
   end
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 1b3388347a8..1f189d81d16 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -80,16 +80,18 @@ module API
         key = Key.find(params[:key_id])
         user = key.user
 
-        if user
-          token = Gitlab::LfsToken.new(user).generate
-          response = { username: user.username, lfs_token: token }
-        else
-          token = Gitlab::LfsToken.new(key).generate
-          response = { username: "lfs-deploy-key-#{key.id}", lfs_token: token }
-        end
+        token_handler =
+          if user
+            Gitlab::LfsToken.new(user)
+          else
+            Gitlab::LfsToken.new(key)
+          end
 
-        response[:repository_http_path] = project.http_url_to_repo
-        response
+        {
+          username: token_handler.actor_name,
+          lfs_token: token_handler.generate,
+          repository_http_path: project.http_url_to_repo
+        }
       end
 
       get "/merge_request_urls" do
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 1b0398d18ee..821c0ef87e9 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -117,15 +117,16 @@ module Gitlab
       end
 
       def lfs_token_check(login, password)
-        if login.include?('lfs-deploy-key')
-          key = DeployKey.find(login.gsub('lfs-deploy-key-', ''))
-          token = Gitlab::LfsToken.new(key).value
-          Result.new(key, :lfs_deploy_token) if key && token == password
-        else
-          user = User.by_login(login)
-          token = Gitlab::LfsToken.new(user).value
-          Result.new(user, :lfs_token) if user && token == password
-        end
+        actor =
+          if login.include?('lfs-deploy-key')
+            DeployKey.find(login.gsub('lfs-deploy-key-', ''))
+          else
+            User.by_login(login)
+          end
+
+        token_handler = Gitlab::LfsToken.new(actor)
+
+        Result.new(actor, token_handler.type) if actor && token_handler.value == password
       end
     end
   end
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index 63656f0b4f1..8f49deb4d03 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -22,6 +22,14 @@ module Gitlab
       end
     end
 
+    def type
+      actor.is_a?(User) ? :lfs_token : :lfs_deploy_token
+    end
+
+    def actor_name
+      actor.is_a?(User) ? actor.username : "lfs-deploy-key-#{actor.id}"
+    end
+
     private
 
     def redis_key
diff --git a/spec/lib/gitlab/lfs_token_spec.rb b/spec/lib/gitlab/lfs_token_spec.rb
index 1d2e4fd9566..f9812664e3b 100644
--- a/spec/lib/gitlab/lfs_token_spec.rb
+++ b/spec/lib/gitlab/lfs_token_spec.rb
@@ -23,6 +23,14 @@ describe Gitlab::LfsToken, lib: true do
       let(:handler) { described_class.new(actor) }
 
       it_behaves_like 'an LFS token generator'
+
+      it 'returns the correct username' do
+        expect(handler.actor_name).to eq(actor.username)
+      end
+
+      it 'returns the correct token type' do
+        expect(handler.type).to eq(:lfs_token)
+      end
     end
 
     context 'when the actor is a deploy key' do
@@ -30,6 +38,14 @@ describe Gitlab::LfsToken, lib: true do
       let(:handler) { described_class.new(actor) }
 
       it_behaves_like 'an LFS token generator'
+
+      it 'returns the correct username' do
+        expect(handler.actor_name).to eq("lfs-deploy-key-#{actor.id}")
+      end
+
+      it 'returns the correct token type' do
+        expect(handler.type).to eq(:lfs_deploy_token)
+      end
     end
   end
 end