Project

General

Profile

Actions

Defect #41930

closed

Redirection after signing in fails when the back_url includes a port number

Added by Kenta Kumojima about 2 months ago. Updated 3 days ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
Accounts / authentication
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

Description

If URI contains a port, I cannot redirect to back_url after login.

If back_uri contains port, ApplicationController#validate_back_url raises Addressable::URI::InvalidURIError and returns false.
So, hidden field of back_url is not rendered at '/login'.
The validation method should remove the port before the host.

       [:scheme, :host, :port].each do |component|                                                                                                                                                       
         if uri.send(component).present? && uri.send(component) != request.send(component)
           return false
         end

         uri.send(:"#{component}=", nil) # => raises InvalidURIError
       end

Files

fix_redirect_to_back_url_with_port.patch (2.14 KB) fix_redirect_to_back_url_with_port.patch Kenta Kumojima, 2024-12-03 16:29
41930-v2.patch (2.09 KB) 41930-v2.patch Go MAEDA, 2025-01-28 02:44

Related issues

Related to Redmine - Defect #31831: Back url parse in validationClosedMarius BÄ‚LTEANU

Actions
Actions #1

Updated by Go MAEDA 5 days ago

Actions #2

Updated by Go MAEDA 5 days ago

  • Status changed from New to Confirmed
  • Target version set to Candidate for next minor release

As reported, I have confirmed the issue where passing a URL containing a port number, such as http://localhost:3000/admin, to the ApplicationController#validate_back_url method results in false being returned.

Inside the validate_back_url method, there is a process that removes the protocol, hostname, and port number from the provided URL. Specifically, the following code is responsible for this:

uri = Addressable::URI.parse(back_url)
[:scheme, :host, :port].each do |component|
  if uri.send(component).present? && uri.send(component) != request.send(component)
    return false
  end

  uri.send(:"#{component}=", nil)
end

In this code, the protocol, hostname, and port number are removed sequentially from the Addressable::URI object created from the given URL. However, when processing in this order, an exception occurs when attempting to remove the hostname. This happens because removing the hostname leaves the URL in a state where it has a port number but no hostname, resulting in the following exception:

Addressable::URI::InvalidURIError: Hostname not supplied: '/admin'

As suggested in the proposed patch, this issue can be resolved by removing the port number before removing the hostname. This adjustment ensures that the URL remains valid during the process.

Actions #3

Updated by Go MAEDA 4 days ago

Here is another solution.

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index a3eaec4bb..9056e1603 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -511,11 +511,10 @@ class ApplicationController < ActionController::Base
         if uri.send(component).present? && uri.send(component) != request.send(component)
           return false
         end
-
-        uri.send(:"#{component}=", nil)
       end
-      # Always ignore basic user:password in the URL
-      uri.userinfo = nil
+
+      # Remove the scheme, user:password, host, and port from the URL
+      uri.omit!(:scheme, :userinfo, :host, :port)
     rescue Addressable::URI::InvalidURIError
       return false
     end
Actions #4

Updated by Go MAEDA 4 days ago

  • Target version changed from Candidate for next minor release to 6.0.3

Setting the target version to 6.0.3.

Actions #5

Updated by Go MAEDA 4 days ago

Here is a new patch. I have updated the code to use the omit! method, as I suggested in #note-3, to remove unnecessary components from the URI object. By using omit! to remove multiple components at once, we no longer need to worry about the order in which the components are removed.

Additionally, the current code has a bug where it throws an exception when a URL contains Basic authentication information (e.g., http://user:password@redmine.example.net) during the uri.send(:"host=") step. This patch also fixes that issue.

Actions #6

Updated by Go MAEDA 4 days ago

  • Status changed from Confirmed to Resolved
  • Assignee set to Go MAEDA
  • Resolution set to Fixed

Committed the patch in r23465. Thank you for your contribution.

Actions #7

Updated by Go MAEDA 4 days ago

  • Subject changed from Cannot redirect to back_url including port after login to Redirection after signing in fails when the back_url includes a port number
Actions #8

Updated by Go MAEDA 3 days ago

  • Status changed from Resolved to Closed

Merged the fix into the 6.0-stable branch in r23467.

Actions #9

Updated by Go MAEDA 3 days ago

Replaced uri.omit!(:scheme, :userinfo, :host, :port) with uri.omit!(:scheme, :authority) in r23469 (trunk) and r23470 (6.0-stable).

Actions

Also available in: Atom PDF