Merge branch '29155-fix-devise-401-responses' into 'master'

Avoid Devise "401 Unauthorized" responses

Closes #29155

See merge request gitlab-org/gitlab!16519

changelogs/unreleased/29155-fix-devise-401-responses.yml

+---
+title: Avoid Devise "401 Unauthorized" responses
+merge_request: 16519
+author:
+type: fixed

config/initializers/8_devise.rb

@@ -214,11 +214,9 @@ Devise.setup do |config|
   # If you want to use other strategies, that are not supported by Devise, or
   # change the failure app, you can configure them inside the config.warden block.
   #
-  # config.warden do |manager|
-  #   manager.failure_app = Gitlab::DeviseFailure
-  #   manager.intercept_401 = false
-  #   manager.default_strategies(scope: :user).unshift :some_external_strategy
-  # end
+  config.warden do |manager|
+    manager.failure_app = Gitlab::DeviseFailure
+  end
 
   if Gitlab::Auth::LDAP::Config.enabled?
     Gitlab::Auth::LDAP::Config.providers.each do |provider|

lib/gitlab/devise_failure.rb

+# frozen_string_literal: true
+
+module Gitlab
+  class DeviseFailure < Devise::FailureApp
+    # If the request format is not known, send a redirect instead of a 401
+    # response, since this is the outcome we're most likely to want
+    def http_auth?
+      return super unless Feature.enabled?(:devise_redirect_unknown_formats, default_enabled: true)
+
+      request_format && super
+    end
+  end
+end

spec/controllers/application_controller_spec.rb

@@ -171,16 +171,40 @@ describe ApplicationController do
   end
 
   describe '#route_not_found' do
+    controller(described_class) do
+      def index
+        route_not_found
+      end
+    end
+
     it 'renders 404 if authenticated' do
-      allow(controller).to receive(:current_user).and_return(user)
-      expect(controller).to receive(:not_found)
-      controller.send(:route_not_found)
+      sign_in(user)
+
+      get :index
+
+      expect(response).to have_gitlab_http_status(404)
     end
 
-    it 'does redirect to login page via authenticate_user! if not authenticated' do
-      allow(controller).to receive(:current_user).and_return(nil)
-      expect(controller).to receive(:authenticate_user!)
-      controller.send(:route_not_found)
+    it 'redirects to login page via authenticate_user! if not authenticated' do
+      get :index
+
+      expect(response).to redirect_to new_user_session_path
+    end
+
+    context 'request format is unknown' do
+      it 'redirects if unauthenticated' do
+        get :index, format: 'unknown'
+
+        expect(response).to redirect_to new_user_session_path
+      end
+
+      it 'returns a 401 if the feature flag is disabled' do
+        stub_feature_flags(devise_redirect_unknown_formats: false)
+
+        get :index, format: 'unknown'
+
+        expect(response).to have_gitlab_http_status(401)
+      end
     end
   end