Merge branch 'lfs-ssh-authentication' into 'master'

Fix internal lfs_authenticate API for non-project repositories

See merge request gitlab-org/gitlab!47404

app/models/concerns/has_repository.rb

@@ -109,6 +109,11 @@ module HasRepository
     Gitlab::RepositoryUrlBuilder.build(repository.full_path, protocol: :http)
   end
 
+  # Is overridden in EE::Project for Geo support
+  def lfs_http_url_to_repo(_operation = nil)
+    http_url_to_repo
+  end
+
   def web_url(only_path: nil)
     Gitlab::UrlBuilder.build(self, only_path: only_path)
   end

app/models/project.rb

@@ -1469,11 +1469,6 @@ class Project < ApplicationRecord
     services.public_send(hooks_scope).any? # rubocop:disable GitlabSecurity/PublicSend
   end
 
-  # Is overridden in EE
-  def lfs_http_url_to_repo(_)
-    http_url_to_repo
-  end
-
   def feature_usage
     super.presence || build_feature_usage
   end

changelogs/unreleased/lfs-ssh-authentication.yml

+---
+title: Fix internal lfs_authenticate API for non-project repositories
+merge_request: 47404
+author:
+type: fixed

ee/app/models/ee/project.rb

@@ -657,7 +657,7 @@ module EE
     end
 
     override :lfs_http_url_to_repo
-    def lfs_http_url_to_repo(operation)
+    def lfs_http_url_to_repo(operation = nil)
       return super unless ::Gitlab::Geo.secondary_with_primary?
       return super if operation == GIT_LFS_DOWNLOAD_OPERATION # download always comes from secondary
 

ee/lib/ee/api/internal/base.rb

@@ -11,8 +11,8 @@ module EE
             extend ::Gitlab::Utils::Override
 
             override :lfs_authentication_url
-            def lfs_authentication_url(project)
-              project.lfs_http_url_to_repo(params[:operation])
+            def lfs_authentication_url(container)
+              container.lfs_http_url_to_repo(params[:operation])
             end
 
             override :check_allowed

ee/spec/requests/api/internal/base_spec.rb

@@ -283,6 +283,7 @@ RSpec.describe API::Internal::Base do
 
     context 'for a secondary node' do
       before do
+        stub_lfs_setting(enabled: true)
         stub_current_geo_node(secondary_node)
         project.add_developer(user)
       end

lib/api/internal/base.rb

@@ -34,10 +34,10 @@ module API
           { status: success, message: message }.merge(extra_options).compact
         end
 
-        def lfs_authentication_url(project)
+        def lfs_authentication_url(container)
           # This is a separate method so that EE can alter its behaviour more
           # easily.
-          project.http_url_to_repo
+          container.lfs_http_url_to_repo
         end
 
         def check_allowed(params)
@@ -135,6 +135,8 @@ module API
         end
 
         post "/lfs_authenticate", feature_category: :source_code_management do
+          not_found! unless container&.lfs_enabled?
+
           status 200
 
           unless actor.key_or_user
@@ -145,7 +147,7 @@ module API
 
           Gitlab::LfsToken
             .new(actor.key_or_user)
-            .authentication_payload(lfs_authentication_url(project))
+            .authentication_payload(lfs_authentication_url(container))
         end
 
         #

spec/requests/api/internal/base_spec.rb

@@ -253,6 +253,7 @@ RSpec.describe API::Internal::Base do
 
   describe "POST /internal/lfs_authenticate" do
     before do
+      stub_lfs_setting(enabled: true)
       project.add_developer(user)
     end
 
@@ -293,6 +294,33 @@ RSpec.describe API::Internal::Base do
 
         expect(response).to have_gitlab_http_status(:not_found)
       end
+
+      it 'returns a 404 when LFS is disabled on the project' do
+        project.update!(lfs_enabled: false)
+        lfs_auth_user(user.id, project)
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+
+      context 'other repository types' do
+        it 'returns the correct information for a project wiki' do
+          wiki = create(:project_wiki, project: project)
+          lfs_auth_user(user.id, wiki)
+
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(json_response['username']).to eq(user.username)
+          expect(json_response['repository_http_path']).to eq(wiki.http_url_to_repo)
+          expect(json_response['expires_in']).to eq(Gitlab::LfsToken::DEFAULT_EXPIRE_TIME)
+          expect(Gitlab::LfsToken.new(user).token_valid?(json_response['lfs_token'])).to be_truthy
+        end
+
+        it 'returns a 404 when the container does not support LFS' do
+          snippet = create(:project_snippet)
+          lfs_auth_user(user.id, snippet)
+
+          expect(response).to have_gitlab_http_status(:not_found)
+        end
+      end
     end
 
     context 'deploy key' do