Merge branch '198289-unable-to-enter-leave-admin-mode-on-read-only-instance' into 'master'

Allow admins to enter/leave admin mode on a read-only instance

Closes #198289

See merge request gitlab-org/gitlab!23552

lib/gitlab/middleware/read_only/controller.rb

@@ -24,8 +24,9 @@ module Gitlab
           'projects/compare' => %w{create}
         }.freeze
 
-        WHITELISTED_LOGOUT_ROUTES = {
-          'sessions' => %w{destroy}
+        WHITELISTED_SESSION_ROUTES = {
+          'sessions' => %w{destroy},
+          'admin/sessions' => %w{create destroy}
         }.freeze
 
         GRAPHQL_URL = '/api/graphql'
@@ -89,7 +90,7 @@ module Gitlab
 
         # Overridden in EE module
         def whitelisted_routes
-          grack_route? || internal_route? || lfs_route? || compare_git_revisions_route? || sidekiq_route? || logout_route? || graphql_query?
+          grack_route? || internal_route? || lfs_route? || compare_git_revisions_route? || sidekiq_route? || session_route? || graphql_query?
         end
 
         def grack_route?
@@ -122,11 +123,12 @@ module Gitlab
           WHITELISTED_GIT_LFS_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
         end
 
-        def logout_route?
+        def session_route?
           # Calling route_hash may be expensive. Only do it if we think there's a possible match
-          return false unless request.post? && request.path.end_with?('/users/sign_out')
+          return false unless request.post? && request.path.end_with?('/users/sign_out',
+            '/admin/session', '/admin/session/destroy')
 
-          WHITELISTED_LOGOUT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
+          WHITELISTED_SESSION_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
         end
 
         def sidekiq_route?

spec/features/admin/admin_mode_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Admin mode', :clean_gitlab_redis_shared_state, :do_not_mock_admin_mode do
+  include MobileHelpers
+  include StubENV
+
+  let(:admin) { create(:admin) }
+
+  before do
+    stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false')
+  end
+
+  context 'feature flag :user_mode_in_session is enabled', :request_store do
+    before do
+      sign_in(admin)
+    end
+
+    context 'when not in admin mode' do
+      it 'has no leave admin mode button' do
+        visit new_admin_session_path
+
+        page.within('.navbar-sub-nav') do
+          expect(page).not_to have_link(href: destroy_admin_session_path)
+        end
+      end
+
+      it 'can open pages not in admin scope' do
+        visit new_admin_session_path
+
+        page.within('.navbar-sub-nav') do
+          find_all('a', text: 'Projects').first.click
+        end
+
+        expect(page).to have_current_path(dashboard_projects_path)
+      end
+
+      it 'is necessary to provide credentials again before opening pages in admin scope' do
+        visit admin_application_settings_path # admin logged out because not in admin_mode
+
+        expect(page).to have_current_path(new_admin_session_path)
+      end
+
+      it 'can enter admin mode' do
+        visit new_admin_session_path
+
+        fill_in 'password', with: admin.password
+
+        click_button 'Enter Admin Mode'
+
+        expect(page).to have_current_path(admin_root_path)
+      end
+
+      context 'on a read-only instance' do
+        before do
+          allow(Gitlab::Database).to receive(:read_only?).and_return(true)
+        end
+
+        it 'can enter admin mode' do
+          visit new_admin_session_path
+
+          fill_in 'password', with: admin.password
+
+          click_button 'Enter Admin Mode'
+
+          expect(page).to have_current_path(admin_root_path)
+        end
+      end
+    end
+
+    context 'when in admin_mode' do
+      before do
+        gitlab_enable_admin_mode_sign_in(admin)
+      end
+
+      it 'contains link to leave admin mode' do
+        page.within('.navbar-sub-nav') do
+          expect(page).to have_link(href: destroy_admin_session_path)
+        end
+      end
+
+      it 'can leave admin mode using main dashboard link', :js do
+        page.within('.navbar-sub-nav') do
+          click_on 'Leave Admin Mode'
+
+          expect(page).to have_link(href: new_admin_session_path)
+        end
+      end
+
+      it 'can leave admin mode using dropdown menu on smaller screens', :js do
+        resize_screen_xs
+        visit root_dashboard_path
+
+        find('.header-more').click
+
+        page.within '.navbar-sub-nav' do
+          click_on 'Leave Admin Mode'
+
+          find('.header-more').click
+
+          expect(page).to have_link(href: new_admin_session_path)
+        end
+      end
+
+      it 'can open pages not in admin scope' do
+        page.within('.navbar-sub-nav') do
+          find_all('a', text: 'Projects').first.click
+
+          expect(page).to have_current_path(dashboard_projects_path)
+        end
+      end
+
+      context 'nav bar' do
+        it 'shows admin dashboard links on bigger screen' do
+          visit root_dashboard_path
+
+          page.within '.navbar' do
+            expect(page).to have_link(text: 'Admin Area', href: admin_root_path, visible: true)
+            expect(page).to have_link(text: 'Leave Admin Mode', href: destroy_admin_session_path, visible: true)
+          end
+        end
+
+        it 'relocates admin dashboard links to dropdown list on smaller screen', :js do
+          resize_screen_xs
+          visit root_dashboard_path
+
+          page.within '.navbar' do
+            expect(page).not_to have_link(text: 'Admin Area', href: admin_root_path, visible: true)
+            expect(page).not_to have_link(text: 'Leave Admin Mode', href: destroy_admin_session_path, visible: true)
+          end
+
+          find('.header-more').click
+
+          page.within '.navbar' do
+            expect(page).to have_link(text: 'Admin Area', href: admin_root_path, visible: true)
+            expect(page).to have_link(text: 'Leave Admin Mode', href: destroy_admin_session_path, visible: true)
+          end
+        end
+      end
+
+      context 'on a read-only instance' do
+        before do
+          allow(Gitlab::Database).to receive(:read_only?).and_return(true)
+        end
+
+        it 'can leave admin mode', :js do
+          page.within('.navbar-sub-nav') do
+            click_on 'Leave Admin Mode'
+
+            expect(page).to have_link(href: new_admin_session_path)
+          end
+        end
+      end
+    end
+  end
+
+  context 'feature flag :user_mode_in_session is disabled' do
+    before do
+      stub_feature_flags(user_mode_in_session: false)
+      sign_in(admin)
+    end
+
+    it 'shows no admin mode buttons in navbar' do
+      visit admin_root_path
+
+      page.within('.navbar-sub-nav') do
+        expect(page).not_to have_link(href: new_admin_session_path)
+        expect(page).not_to have_link(href: destroy_admin_session_path)
+      end
+    end
+  end
+end

spec/features/admin/admin_settings_spec.rb

@@ -5,7 +5,6 @@ require 'spec_helper'
 describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_mock_admin_mode do
   include StubENV
   include TermsHelper
-  include MobileHelpers
 
   let(:admin) { create(:admin) }
 
@@ -449,100 +448,6 @@ describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_moc
           expect(page).to have_link(text: 'Support', href: new_support_url)
         end
       end
-
-      it 'Shows admin dashboard links on bigger screen' do
-        visit root_dashboard_path
-
-        page.within '.navbar' do
-          expect(page).to have_link(text: 'Admin Area', href: admin_root_path, visible: true)
-          expect(page).to have_link(text: 'Leave Admin Mode', href: destroy_admin_session_path, visible: true)
-        end
-      end
-
-      it 'Relocates admin dashboard links to dropdown list on smaller screen', :js do
-        resize_screen_xs
-        visit root_dashboard_path
-
-        page.within '.navbar' do
-          expect(page).not_to have_link(text: 'Admin Area', href: admin_root_path, visible: true)
-          expect(page).not_to have_link(text: 'Leave Admin Mode', href: destroy_admin_session_path, visible: true)
-        end
-
-        find('.header-more').click
-
-        page.within '.navbar' do
-          expect(page).to have_link(text: 'Admin Area', href: admin_root_path, visible: true)
-          expect(page).to have_link(text: 'Leave Admin Mode', href: destroy_admin_session_path, visible: true)
-        end
-      end
-    end
-
-    context 'when in admin_mode' do
-      it 'contains link to leave admin mode' do
-        page.within('.navbar-sub-nav') do
-          expect(page).to have_link(href: destroy_admin_session_path)
-        end
-      end
-
-      it 'can leave admin mode using main dashboard link', :js do
-        page.within('.navbar-sub-nav') do
-          click_on 'Leave Admin Mode'
-
-          expect(page).to have_link(href: new_admin_session_path)
-        end
-      end
-
-      it 'can leave admin mode using dropdown menu on smaller screens', :js do
-        resize_screen_xs
-        visit root_dashboard_path
-
-        find('.header-more').click
-
-        page.within '.navbar-sub-nav' do
-          click_on 'Leave Admin Mode'
-
-          find('.header-more').click
-
-          expect(page).to have_link(href: new_admin_session_path)
-        end
-      end
-
-      it 'can open pages not in admin scope' do
-        page.within('.navbar-sub-nav') do
-          find_all('a', text: 'Projects').first.click
-
-          expect(page).to have_current_path(dashboard_projects_path)
-        end
-      end
-    end
-
-    context 'when not in admin mode' do
-      before do
-        page.within('.navbar-sub-nav') do
-          # Select first, link is also included in mobile view list
-          click_on 'Leave Admin Mode', match: :first
-        end
-      end
-
-      it 'has no leave admin mode button' do
-        page.within('.navbar-sub-nav') do
-          expect(page).not_to have_link(href: destroy_admin_session_path)
-        end
-      end
-
-      it 'is necessary to provide credentials again before opening admin settings' do
-        visit admin_application_settings_path # admin logged out because not in admin_mode
-
-        expect(page).to have_current_path(new_admin_session_path)
-      end
-
-      it 'can open pages not in admin scope' do
-        page.within('.navbar-sub-nav') do
-          find_all('a', text: 'Projects').first.click
-        end
-
-        expect(page).to have_current_path(dashboard_projects_path)
-      end
     end
   end
 
@@ -559,13 +464,6 @@ describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_moc
     it 'loads admin settings page without redirect for reauthentication' do
       expect(current_path).to eq admin_application_settings_path
     end
-
-    it 'shows no admin mode buttons in navbar' do
-      page.within('.navbar-sub-nav') do
-        expect(page).not_to have_link(href: new_admin_session_path)
-        expect(page).not_to have_link(href: destroy_admin_session_path)
-      end
-    end
   end
 
   def check_all_events