Move snippet feature flag check to controllers

At the moment, class `GitAccessSnippet` ensures that
the feature flag :version_snippets is enabled in order
to allow/disallow snippet git actions.

Nevertheless, there are git operations, like when
we update the snippet info, that we want to perform
always, no matter the feature flag status.

We need to move the check outside the access class into
the controllers and the internal API.

app/controllers/repositories/git_http_controller.rb

@@ -12,6 +12,8 @@ module Repositories
     rescue_from Gitlab::GitAccess::ProjectCreationError, with: :render_422_with_exception
     rescue_from Gitlab::GitAccess::TimeoutError, with: :render_503_with_exception
 
+    before_action :snippet_request_allowed?
+
     # GET /foo/bar.git/info/refs?service=git-upload-pack (git pull)
     # GET /foo/bar.git/info/refs?service=git-receive-pack (git push)
     def info_refs
@@ -116,6 +118,12 @@ module Repositories
     def log_user_activity
       Users::ActivityService.new(user).execute
     end
+
+    def snippet_request_allowed?
+      if repo_type.snippet? && Feature.disabled?(:version_snippets, user)
+        render plain: 'The project you were looking for could not be found.', status: :not_found
+      end
+    end
   end
 end
 

lib/api/internal/base.rb

@@ -108,6 +108,10 @@ module API
         #   check_ip - optional, only in EE version, may limit access to
         #     group resources based on its IP restrictions
         post "/allowed" do
+          if repo_type.snippet? && Feature.disabled?(:version_snippets, actor.user)
+            break response_with_status(code: 404, success: false, message: 'The project you were looking for could not be found.')
+          end
+
           # It was moved to a separate method so that EE can alter its behaviour more
           # easily.
           check_allowed(params)

lib/gitlab/git_access_snippet.rb

@@ -8,7 +8,6 @@ module Gitlab
       authentication_mechanism: 'The authentication mechanism is not supported.',
       read_snippet: 'You are not allowed to read this snippet.',
       update_snippet: 'You are not allowed to update this snippet.',
-      project_not_found: 'The project you were looking for could not be found.',
       snippet_not_found: 'The snippet you were looking for could not be found.',
       repository_not_found: 'The snippet repository you were looking for could not be found.'
     }.freeze
@@ -31,10 +30,6 @@ module Gitlab
         raise ForbiddenError, ERROR_MESSAGES[:authentication_mechanism]
       end
 
-      unless Feature.enabled?(:version_snippets, user)
-        raise NotFoundError, ERROR_MESSAGES[:project_not_found]
-      end
-
       check_snippet_accessibility!
 
       super

spec/controllers/repositories/git_http_controller_spec.rb

@@ -135,6 +135,38 @@ describe Repositories::GitHttpController do
     end
   end
 
+  shared_examples 'snippet feature flag disabled behavior' do
+    before do
+      stub_feature_flags(version_snippets: false)
+
+      request.headers.merge! auth_env(user.username, user.password, nil)
+    end
+
+    describe 'GET #info_refs' do
+      let(:params) { container_params.merge(service: 'git-upload-pack') }
+
+      it 'returns 404' do
+        get :info_refs, params: params
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+
+    describe 'POST #git_upload_pack' do
+      before do
+        allow(controller).to receive(:authenticate_user).and_return(true)
+        allow(controller).to receive(:verify_workhorse_api!).and_return(true)
+        allow(controller).to receive(:access_check).and_return(nil)
+      end
+
+      it 'returns 404' do
+        post :git_upload_pack, params: params
+
+        expect(response).to have_gitlab_http_status(:not_found)
+      end
+    end
+  end
+
   context 'when repository container is a project' do
     it_behaves_like 'info_refs behavior' do
       let(:user) { project.owner }
@@ -158,6 +190,9 @@ describe Repositories::GitHttpController do
       let(:expected_class) { Gitlab::GitAccessSnippet }
       let(:expected_object) { personal_snippet }
     end
+    it_behaves_like 'snippet feature flag disabled behavior' do
+      let(:user) { personal_snippet.author }
+    end
   end
 
   context 'when repository container is a project snippet' do
@@ -172,5 +207,8 @@ describe Repositories::GitHttpController do
       let(:expected_class) { Gitlab::GitAccessSnippet }
       let(:expected_object) { project_snippet }
     end
+    it_behaves_like 'snippet feature flag disabled behavior' do
+      let(:user) { project_snippet.author }
+    end
   end
 end

spec/lib/gitlab/git_access_snippet_spec.rb

@@ -31,12 +31,15 @@ describe Gitlab::GitAccessSnippet do
   end
 
   describe 'when feature flag :version_snippets is disabled' do
+    let(:user) { snippet.author }
+
     before do
       stub_feature_flags(version_snippets: false)
     end
 
-    it 'does not allow push and pull access' do
-      expect { pull_access_check }.to raise_project_not_found
+    it 'allows push and pull access' do
+      expect { pull_access_check }.not_to raise_error
+      expect { push_access_check }.not_to raise_error
     end
   end
 

spec/requests/api/internal/base_spec.rb

@@ -315,6 +315,18 @@ describe API::Internal::Base do
         end
       end
 
+      shared_examples 'snippets with disabled feature flag' do
+        context 'when feature flag :version_snippets is disabled' do
+          it 'returns 404' do
+            stub_feature_flags(version_snippets: false)
+
+            subject
+
+            expect(response).to have_gitlab_http_status(:not_found)
+          end
+        end
+      end
+
       context 'git push with personal snippet' do
         it 'responds with success' do
           push(key, personal_snippet)
@@ -325,6 +337,10 @@ describe API::Internal::Base do
           expect(json_response["gl_repository"]).to eq("snippet-#{personal_snippet.id}")
           expect(user.reload.last_activity_on).to be_nil
         end
+
+        it_behaves_like 'snippets with disabled feature flag' do
+          subject { push(key, personal_snippet) }
+        end
       end
 
       context 'git pull with personal snippet' do
@@ -337,6 +353,10 @@ describe API::Internal::Base do
           expect(json_response["gl_repository"]).to eq("snippet-#{personal_snippet.id}")
           expect(user.reload.last_activity_on).to eql(Date.today)
         end
+
+        it_behaves_like 'snippets with disabled feature flag' do
+          subject { pull(key, personal_snippet) }
+        end
       end
 
       context 'git push with project snippet' do
@@ -349,6 +369,10 @@ describe API::Internal::Base do
           expect(json_response["gl_repository"]).to eq("snippet-#{project_snippet.id}")
           expect(user.reload.last_activity_on).to be_nil
         end
+
+        it_behaves_like 'snippets with disabled feature flag' do
+          subject { push(key, project_snippet) }
+        end
       end
 
       context 'git pull with project snippet' do
@@ -361,6 +385,10 @@ describe API::Internal::Base do
           expect(json_response["gl_repository"]).to eq("snippet-#{project_snippet.id}")
           expect(user.reload.last_activity_on).to eql(Date.today)
         end
+
+        it_behaves_like 'snippets with disabled feature flag' do
+          subject { pull(key, project_snippet) }
+        end
       end
 
       context "git pull" do