Refactored `user` to `actor` to better follow `GitHttpClientController`, and added better tests for `lfs_token`.better-lfs-ssh-tests

3 files changed, 49 insertions, 6 deletions

diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb
index 662d38b10a5..3c10e318014 100644
--- a/app/controllers/projects/git_http_controller.rb
+++ b/app/controllers/projects/git_http_controller.rb
@@ -59,7 +59,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
 
   def render_ok
     set_workhorse_internal_api_content_type
-    render json: Gitlab::Workhorse.git_http_ok(repository, user)
+    render json: Gitlab::Workhorse.git_http_ok(repository, actor)
   end
 
   def render_http_not_allowed
@@ -67,7 +67,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def render_denied
-    if user && user.can?(:read_project, project)
+    if actor.is_a?(User) && can?(actor, :read_project, project)
       render plain: 'Access denied', status: :forbidden
     else
       # Do not leak information about project existence
@@ -78,7 +78,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   def upload_pack_allowed?
     return false unless Gitlab.config.gitlab_shell.upload_pack
 
-    if user
+    if actor
       access_check.allowed?
     else
       ci? || project.public?
@@ -86,7 +86,7 @@ class Projects::GitHttpController < Projects::GitHttpClientController
   end
 
   def access
-    @access ||= Gitlab::GitAccess.new(user, project, 'http', authentication_abilities: authentication_abilities)
+    @access ||= Gitlab::GitAccess.new(actor, project, 'http', authentication_abilities: authentication_abilities)
   end
 
   def access_check
diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb
index d089a2f9b0b..b43ccfc8b76 100644
--- a/lib/gitlab/lfs_token.rb
+++ b/lib/gitlab/lfs_token.rb
@@ -38,11 +38,11 @@ module Gitlab
     end
 
     def type
-      actor.is_a?(User) ? :lfs_token : :lfs_deploy_token
+      user? ? :lfs_token : :lfs_deploy_token
     end
 
     def actor_name
-      actor.is_a?(User) ? actor.username : "lfs+deploy-key-#{actor.id}"
+      user? ? actor.username : "lfs+deploy-key-#{actor.id}"
     end
 
     private
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 74516686921..7e0c4cecc79 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -213,6 +213,49 @@ describe 'Git HTTP requests', lib: true do
               end
             end
 
+            describe 'lfs_token' do
+              context 'when lfs_token is provided' do
+                let(:lfs_token) { Gitlab::LfsToken.new(user).generate }
+                let(:env) { { user: user.username, password: lfs_token } }
+
+                it 'accepts clone attempt' do
+                  clone_get(path, env)
+
+                  expect(response).to have_http_status(200)
+                end
+
+                it 'accepts push attempt' do
+                  upload(path, env) do |response|
+                    expect(response).to have_http_status(200)
+                  end
+                end
+              end
+
+              context 'when lfs_deploy_token provided' do
+                let(:deploy_key) { create(:deploy_key) }
+                let(:env) { { user: "lfs+deploy-key-#{deploy_key.id}", password: Gitlab::LfsToken.new(deploy_key).generate } }
+
+                it 'rejects clone attempt if deploy_key not in project' do
+                  clone_get(path, env)
+
+                  expect(response).to have_http_status(404)
+                end
+
+                it 'accepts clone attempt' do
+                  deploy_key.projects << project
+                  clone_get(path, env)
+
+                  expect(response).to have_http_status(200)
+                end
+
+                it 'rejects push attempt' do
+                  upload(path, env) do |response|
+                    expect(response).to have_http_status(401)
+                  end
+                end
+              end
+            end
+
             context 'when user has 2FA enabled' do
               let(:user) { create(:user, :two_factor) }
               let(:access_token) { create(:personal_access_token, user: user) }