Skip to content

Commit

Permalink
Fix redirecting with query parameters on Rails 7.2
Browse files Browse the repository at this point in the history
Action Controller instrumentation now reads ActionDispatch::Response#request
when the redirect URL has query parameters, and we weren't setting it.

Fixes #309
  • Loading branch information
janko committed Jul 24, 2024
1 parent 36191e0 commit 4f4bc90
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 10 deletions.
18 changes: 12 additions & 6 deletions lib/rodauth/rails/feature/instrumentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ def rails_instrument_request
ActiveSupport::Notifications.instrument("process_action.action_controller", raw_payload) do |payload|
result = catch(:halt) { yield }

response = ActionDispatch::Response.new(*(result || [404, {}, []]))
payload[:response] = response
payload[:status] = response.status
rails_response = build_rails_response(result || [404, {}, []])
payload[:response] = rails_response
payload[:status] = rails_response.status

throw :halt, result if result
rescue => error
Expand All @@ -66,13 +66,19 @@ def rails_instrument_redirection
ActiveSupport::Notifications.instrument("redirect_to.action_controller", request: rails_request) do |payload|
result = catch(:halt) { yield }

response = ActionDispatch::Response.new(*result)
payload[:status] = response.status
payload[:location] = response.filtered_location
rails_response = build_rails_response(result)
payload[:status] = rails_response.status
payload[:location] = rails_response.filtered_location

throw :halt, result
end
end

def build_rails_response(args)
response = ActionDispatch::Response.new(*args)
response.request = rails_request
response
end
end
end
end
Expand Down
15 changes: 11 additions & 4 deletions test/integration/redirect_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,23 +3,30 @@
class RedirectTest < IntegrationTest
test "requiring authentication from Rodauth app" do
visit "/auth1"
assert_equal current_path, "/login"
assert_equal "/login", current_path

register
visit "/auth1"

assert_equal current_path, "/auth1"
assert_equal "/auth1", current_path
assert_includes page.html, %(Authenticated as user@example.com)
end

test "requiring authentication from Rails controller" do
visit "/auth2"
assert_equal current_path, "/login"
assert_equal "/login", current_path

register
visit "/auth2"

assert_equal current_path, "/auth2"
assert_equal "/auth2", current_path
assert_includes page.html, %(Authenticated as user@example.com)
end

test "redirecting with query parameters" do
register
visit "/create-account?foo=bar"

assert_equal "http://www.example.com/?foo=bar", current_url
end
end
1 change: 1 addition & 0 deletions test/rails_app/app/misc/rodauth_main.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ class RodauthMain < Rodauth::Rails::Auth
after_login { remember_login }
before_create_account { rails_account.username }

already_logged_in { redirect rails_routes.root_path(rails_request.query_parameters) }
logout_redirect { rails_routes.root_path }
verify_account_redirect { login_redirect }
reset_password_redirect { login_path }
Expand Down

0 comments on commit 4f4bc90

Please sign in to comment.