Merge branch 'cat-geo-separate-signin' into 'master'

Update Devise sign_in path for Geo secondaries

See merge request gitlab-org/gitlab!69748

config/routes/user.rb

@@ -21,11 +21,35 @@ if Gitlab::Auth::Ldap::Config.sign_in_enabled?
   end
 end
 
-devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks,
-                                  registrations: :registrations,
-                                  passwords: :passwords,
-                                  sessions: :sessions,
-                                  confirmations: :confirmations }
+devise_controllers = { omniauth_callbacks: :omniauth_callbacks,
+                       registrations: :registrations,
+                       passwords: :passwords,
+                       sessions: :sessions,
+                       confirmations: :confirmations }
+
+if ::Gitlab.ee? && ::Gitlab::Geo.connected? && ::Gitlab::Geo.secondary?
+  devise_for :users, controllers: devise_controllers, path_names: { sign_in: 'auth/geo/sign_in',
+                                                                    sign_out: 'auth/geo/sign_out' }
+  # When using Geo, the other type of routes should be present as well, as browsers
+  # cache 302 redirects locally, and events like primary going offline or a failover
+  # can result in browsers requesting the other paths because of it.
+  as :user do
+    get '/users/sign_in', to: 'sessions#new'
+    post '/users/sign_in', to: 'sessions#create'
+    post '/users/sign_out', to: 'sessions#destroy'
+  end
+else
+  devise_for :users, controllers: devise_controllers
+
+  # We avoid drawing Geo routes for FOSS, but keep them in for EE
+  Gitlab.ee do
+    as :user do
+      get '/users/auth/geo/sign_in', to: 'sessions#new'
+      post '/users/auth/geo/sign_in', to: 'sessions#create'
+      post '/users/auth/geo/sign_out', to: 'sessions#destroy'
+    end
+  end
+end
 
 devise_scope :user do
   get '/users/almost_there' => 'confirmations#almost_there'

ee/lib/ee/gitlab/middleware/read_only/controller.rb

@@ -24,6 +24,10 @@ module EE
             'repositories/lfs_locks_api' => %w{verify create unlock}
           }.freeze
 
+          ALLOWLISTED_SIGN_OUT_ROUTES = {
+            'sessions' => %w{destroy}
+          }.freeze
+
           ALLOWLISTED_SIGN_IN_ROUTES = {
             'sessions' => %w{create},
             'oauth/tokens' => %w{create}
@@ -33,12 +37,11 @@ module EE
 
           # In addition to routes allowed in FOSS, allow geo node update route
           # and geo api route, on both Geo primary and secondary.
-          # If this is on a Geo secondary, also allow git write routes.
-          # If in maintenance mode, don't allow git write routes on Geo
-          # secondary either
+          # If this is on a Geo secondary, also allow git write routes and custom logout routes.
+          # If in maintenance mode, don't allow git write routes on Geo secondary either
           override :allowlisted_routes
           def allowlisted_routes
-            allowed = super || geo_node_update_route? || geo_api_route? || admin_settings_update?
+            allowed = super || geo_node_update_route? || geo_api_route? || geo_sign_out_route? || admin_settings_update?
 
             return true if allowed
             return sign_in_route? if ::Gitlab.maintenance_mode?
@@ -90,8 +93,15 @@ module EE
             end
           end
 
+          def geo_sign_out_route?
+            return unless request_path.start_with?('/users/auth/geo/sign_out')
+
+            ALLOWLISTED_SIGN_OUT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
+          end
+
           def sign_in_route?
-            return unless request.post? && request.path.start_with?('/users/sign_in', '/oauth/token')
+            return unless request.post? && request.path.start_with?('/users/sign_in', '/oauth/token',
+              '/users/auth/geo/sign_in')
 
             ALLOWLISTED_SIGN_IN_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
           end

ee/spec/routing/user_routing_spec.rb

+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe 'EE-specific user routing' do
+  describe 'devise_for users scope' do
+    it 'defines regular and Geo routes' do
+      [
+        ['/users/sign_in', 'GET', 'new'],
+        ['/users/auth/geo/sign_in', 'GET', 'new'],
+        ['/users/sign_in', 'POST', 'create'],
+        ['/users/auth/geo/sign_in', 'POST', 'create'],
+        ['/users/sign_out', 'POST', 'destroy'],
+        ['/users/auth/geo/sign_out', 'POST', 'destroy']
+      ].each do |path, method, action|
+        expect(Rails.application.routes.recognize_path(path, { method: method })).to include(
+          { controller: 'sessions', action: action }
+        )
+      end
+    end
+
+    shared_examples 'routes session paths' do |route_type|
+      it "handles #{route_type} named route helpers" do
+        Rails.application.reload_routes!
+
+        sign_in_path, sign_out_path = case route_type
+                                      when :regular then
+                                        ['/users/sign_in', '/users/sign_out']
+                                      when :geo then
+                                        ['/users/auth/geo/sign_in', '/users/auth/geo/sign_out']
+                                      end
+
+        expect(Gitlab::Routing.url_helpers.new_user_session_path).to eq(sign_in_path)
+        expect(Gitlab::Routing.url_helpers.destroy_user_session_path).to eq(sign_out_path)
+      end
+    end
+
+    context 'no database connection' do
+      before do
+        allow(Gitlab::Geo).to receive(:connected?).and_return(false)
+      end
+
+      it_behaves_like 'routes session paths', :regular
+    end
+
+    context 'Geo is disabled' do
+      before do
+        allow(Gitlab::Geo).to receive(:enabled?).and_return(false)
+      end
+
+      it_behaves_like 'routes session paths', :regular
+    end
+
+    context 'current node is a Geo primary' do
+      before do
+        allow(Gitlab::Geo).to receive(:secondary?).and_return(false)
+      end
+
+      it_behaves_like 'routes session paths', :regular
+    end
+
+    context 'current node is a Geo secondary' do
+      before do
+        allow(Gitlab::Geo).to receive(:secondary?).and_return(true)
+      end
+
+      it_behaves_like 'routes session paths', :geo
+    end
+  end
+end

ee/spec/support/shared_examples/lib/gitlab/middleware/maintenance_mode_gitlab_ee_instance_shared_examples.rb

@@ -37,6 +37,8 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance in main
       where(:description, :path) do
         'LFS request to batch'        | '/root/rouge.git/info/lfs/objects/batch'
         'to geo replication node api' | "/api/#{API::API.version}/geo_replication/designs/resync"
+        'Geo sign in'                 | '/users/auth/geo/sign_in'
+        'Geo sign out'                | '/users/auth/geo/sign_out'
       end
 
       with_them do
@@ -116,6 +118,7 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance in main
 
       where(:description, :path) do
         'sign in route'     | '/users/sign_in'
+        'sign out route'    | '/users/sign_out'
         'oauth token route' | '/oauth/token'
       end
 

workhorse/internal/upstream/routes.go

@@ -359,7 +359,7 @@ func configureRoutes(u *upstream) {
 		u.route("", "^/-/metrics$", defaultUpstream),
 
 		// Authentication routes
-		u.route("", "^/users/(sign_in|sign_out)$", defaultUpstream),
+		u.route("", "^/users/auth/geo/(sign_in|sign_out)$", defaultUpstream),
 		u.route("", "^/oauth/geo/(auth|callback|logout)$", defaultUpstream),
 
 		// Admin Area > Geo routes