Fix internal lfs_authenticate API for non-project repositories

This endpoint is used for LFS over SSH and had two problems:

1. We were generating the repository URL from `project` rather than
   `container`, which means we'd return the wrong URL for e.g. project
   wiki repositories.

   In practice this didn't break LFS, as we're not touching the
   repository itself and just associating the objects with the project
   record, which works the same with project wikis.

2. We did not check if LFS is enabled, which means we'd still return a
   URL for project snippets. This is less of a problem as the subsequent
   HTTP requests to the LFS controllers would still fail, but it's
   better to be consistent and abort early here.

Follow-up to https://gitlab.com/gitlab-org/gitlab/-/merge_requests/45874

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