Fix the redirect to custom home page URL and move it to RootController

Signed-off-by: Rémy Coutable <remy@rymai.me>

app/controllers/application_controller.rb

@@ -72,14 +72,6 @@ def authenticate_user_from_private_token!
     end
   end
 
-  def authenticate_user!(*args)
-    if redirect_to_home_page_url?
-      return redirect_to current_application_settings.home_page_url
-    end
-
-    super(*args)
-  end
-
   def log_exception(exception)
     application_trace = ActionDispatch::ExceptionWrapper.new(env, exception).application_trace
     application_trace.map!{ |t| "  #{t}\n" }
@@ -287,19 +279,6 @@ def skip_two_factor?
     session[:skip_tfa] && session[:skip_tfa] > Time.current
   end
 
-  def redirect_to_home_page_url?
-    # If user is not signed-in and tries to access root_path - redirect him to landing page
-    # Don't redirect to the default URL to prevent endless redirections
-    return false unless current_application_settings.home_page_url.present?
-
-    home_page_url = current_application_settings.home_page_url.chomp('/')
-    root_urls = [Gitlab.config.gitlab['url'].chomp('/'), root_url.chomp('/')]
-
-    return false if root_urls.include?(home_page_url)
-
-    current_user.nil? && root_path == request.path
-  end
-
   # U2F (universal 2nd factor) devices need a unique identifier for the application
   # to perform authentication.
   # https://developers.yubico.com/U2F/App_ID.html

app/controllers/root_controller.rb

@@ -8,7 +8,9 @@
 # `DashboardController#show`, which is the default.
 class RootController < Dashboard::ProjectsController
   skip_before_action :authenticate_user!, only: [:index]
-  before_action :redirect_to_custom_dashboard, only: [:index]
+
+  before_action :redirect_unlogged_user, if: -> { current_user.nil? }
+  before_action :redirect_logged_user, if: -> { current_user.present? }
 
   def index
     super
@@ -16,23 +18,38 @@ def index
 
   private
 
-  def redirect_to_custom_dashboard
-    return redirect_to new_user_session_path unless current_user
+  def redirect_unlogged_user
+    if redirect_to_home_page_url?
+      redirect_to(current_application_settings.home_page_url)
+    else
+      redirect_to(new_user_session_path)
+    end
+  end
 
+  def redirect_logged_user
     case current_user.dashboard
     when 'stars'
       flash.keep
-      redirect_to starred_dashboard_projects_path
+      redirect_to(starred_dashboard_projects_path)
     when 'project_activity'
-      redirect_to activity_dashboard_path
+      redirect_to(activity_dashboard_path)
     when 'starred_project_activity'
-      redirect_to activity_dashboard_path(filter: 'starred')
+      redirect_to(activity_dashboard_path(filter: 'starred'))
     when 'groups'
-      redirect_to dashboard_groups_path
+      redirect_to(dashboard_groups_path)
     when 'todos'
-      redirect_to dashboard_todos_path
-    else
-      return
+      redirect_to(dashboard_todos_path)
     end
   end
+
+  def redirect_to_home_page_url?
+    # If user is not signed-in and tries to access root_path - redirect him to landing page
+    # Don't redirect to the default URL to prevent endless redirections
+    return false unless current_application_settings.home_page_url.present?
+
+    home_page_url = current_application_settings.home_page_url.chomp('/')
+    root_urls = [Gitlab.config.gitlab['url'].chomp('/'), root_url.chomp('/')]
+
+    root_urls.exclude?(home_page_url)
+  end
 end

changelogs/unreleased/28609-fix-redirect-to-home-page-url.yml

+---
+title: Fix the redirect to custom home page URL
+merge_request: 9518
+author:

spec/controllers/root_controller_spec.rb

@@ -2,6 +2,26 @@
 
 describe RootController do
   describe 'GET index' do
+    context 'when user is not logged in' do
+      it 'redirects to the sign-in page' do
+        get :index
+
+        expect(response).to redirect_to(new_user_session_path)
+      end
+
+      context 'when a custom home page URL is defined' do
+        before do
+          stub_application_setting(home_page_url: 'https://gitlab.com')
+        end
+
+        it 'redirects the user to the custom home page URL' do
+          get :index
+
+          expect(response).to redirect_to('https://gitlab.com')
+        end
+      end
+    end
+
     context 'with a user' do
       let(:user) { create(:user) }
 
@@ -12,55 +32,60 @@
 
       context 'who has customized their dashboard setting for starred projects' do
         before do
-          user.update_attribute(:dashboard, 'stars')
+          user.dashboard = 'stars'
         end
 
         it 'redirects to their specified dashboard' do
           get :index
+
           expect(response).to redirect_to starred_dashboard_projects_path
         end
       end
 
       context 'who has customized their dashboard setting for project activities' do
         before do
-          user.update_attribute(:dashboard, 'project_activity')
+          user.dashboard = 'project_activity'
         end
 
         it 'redirects to the activity list' do
           get :index
+
           expect(response).to redirect_to activity_dashboard_path
         end
       end
 
       context 'who has customized their dashboard setting for starred project activities' do
         before do
-          user.update_attribute(:dashboard, 'starred_project_activity')
+          user.dashboard = 'starred_project_activity'
         end
 
         it 'redirects to the activity list' do
           get :index
+
           expect(response).to redirect_to activity_dashboard_path(filter: 'starred')
         end
       end
 
       context 'who has customized their dashboard setting for groups' do
         before do
-          user.update_attribute(:dashboard, 'groups')
+          user.dashboard = 'groups'
         end
 
         it 'redirects to their group list' do
           get :index
+
           expect(response).to redirect_to dashboard_groups_path
         end
       end
 
       context 'who has customized their dashboard setting for todos' do
         before do
-          user.update_attribute(:dashboard, 'todos')
+          user.dashboard = 'todos'
         end
 
         it 'redirects to their todo list' do
           get :index
+
           expect(response).to redirect_to dashboard_todos_path
         end
       end
@@ -68,6 +93,7 @@
       context 'who uses the default dashboard setting' do
         it 'renders the default dashboard' do
           get :index
+
           expect(response).to render_template 'dashboard/projects/index'
         end
       end