Fix after login redirect not ending up on configured URL by default

We used request.url and a back_url for the home path to redirect users
if they did not end up trying to access on a deep link.

In case you access / and get redirected by require_login, you want users
to end up on the after_login_default_redirect_url

Likewise, if you access /login without a back_url, you want the same
behavior

Deep linking logins are unchanged

https://community.openproject.org/work_packages/74756

app/controllers/concerns/accounts/current_user.rb

@@ -165,7 +165,7 @@ module Accounts::CurrentUser
           # but ONLY for html requests to avoid double-resetting sessions
           reset_session
 
-          redirect_to main_app.signin_path(back_url: login_back_url)
+          redirect_to main_app.signin_path(signin_params)
         end
 
         format.any(:xml, :js, :json, :turbo_stream) do
@@ -184,4 +184,16 @@ module Accounts::CurrentUser
 
     render_403 unless current_user.admin?
   end
+
+  def signin_params
+    back_url = login_back_url
+
+    # Do not pass home path as a back_url
+    # as we want after_login_default_redirect_url to take effect
+    if back_url == home_url
+      {}
+    else
+      { back_url: }
+    end
+  end
 end

app/forms/settings/authentication_settings_form.rb

@@ -97,7 +97,8 @@ module Settings
 
       f.text_field(
         name: :after_login_default_redirect_url,
-        caption: helpers.t(:setting_after_login_default_redirect_url_text_html),
+        caption: helpers.t(:setting_after_login_default_redirect_url_example_html,
+                           example_code: helpers.content_tag(:code, "/my/page")),
         input_width: :large
       )
 

app/helpers/application_helper.rb

@@ -340,14 +340,7 @@ module ApplicationHelper
   end
 
   def back_url_to_current_page
-    back_url = params[:back_url] if params.present?
-    if back_url.present?
-      back_url = back_url.to_s
-    elsif request.get? && params.present?
-      back_url = request.url
-    end
-
-    back_url
+    params[:back_url].presence&.to_s
   end
 
   def check_all_links(form_id = nil, &)

config/locales/en.yml

@@ -5181,10 +5181,10 @@ en:
     <br/>
     Example: <code>/my/page</code>
   setting_after_login_default_redirect_url: "After login redirect"
-  setting_after_login_default_redirect_url_text_html: >
+  setting_after_login_default_redirect_url_example_html: >
     Set a default path to redirect users after login, if no back link was provided. Redirects to home page if not set.
     <br/>
-    Example: <code>/my/page</code>
+    Example: %{example_code}
   setting_apiv3_cors_title: "Cross-Origin Resource Sharing (CORS)"
   setting_apiv3_cors_enabled: "Enable CORS"
   setting_apiv3_cors_origins: "API V3 Cross-Origin Resource Sharing (CORS) allowed origins"

spec/controllers/application_controller_spec.rb

@@ -221,4 +221,30 @@ RSpec.describe ApplicationController do
       end
     end
   end
+
+  describe "#require_login redirect target", with_settings: { login_required: true } do
+    before do
+      allow(controller).to receive(:current_user).and_return(User.anonymous)
+    end
+
+    context "when back_url points to home" do
+      it "redirects to signin without back_url" do
+        allow(controller).to receive(:login_back_url).and_return(controller.home_url)
+
+        get :index
+
+        expect(response).to redirect_to(signin_path)
+      end
+    end
+
+    context "when back_url points to another page" do
+      it "redirects to signin with back_url" do
+        allow(controller).to receive(:login_back_url).and_return("http://test.host/projects")
+
+        get :index
+
+        expect(response).to redirect_to(signin_path(back_url: "http://test.host/projects"))
+      end
+    end
+  end
 end

spec/helpers/application_helper_spec.rb

@@ -290,6 +290,30 @@ RSpec.describe ApplicationHelper do
     end
   end
 
+  describe "#back_url_to_current_page" do
+    context "when back_url param is provided" do
+      it "returns the provided back_url" do
+        allow(helper).to receive(:params).and_return(ActionController::Parameters.new(back_url: "/work_packages"))
+
+        expect(helper.back_url_to_current_page).to eq("/work_packages")
+      end
+    end
+
+    context "when back_url param is missing" do
+      it "returns nil" do
+        allow(helper)
+          .to(
+            receive_messages(
+              params: ActionController::Parameters.new,
+              request: instance_double(ActionDispatch::Request, get?: true, url: "http://test.host/")
+            )
+          )
+
+        expect(helper.back_url_to_current_page).to be_nil
+      end
+    end
+  end
+
   describe "#link_to_content_update" do
     let(:options) { { controller: "work_packages", action: "show", id: 10 } }