Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failure App incorrectly adds url query parameters to PATH_INFO #5704

Open
benjaminwood opened this issue Aug 8, 2024 · 0 comments
Open

Failure App incorrectly adds url query parameters to PATH_INFO #5704

benjaminwood opened this issue Aug 8, 2024 · 0 comments

Comments

@benjaminwood
Copy link

benjaminwood commented Aug 8, 2024

Environment

  • Ruby 3.0.3p157
  • Rails 7.0.6
  • Devise 4.9.4

Current behavior

If authentication fails when submitting to a URL that includes URL parameters, the failure app mutates PATH_INFO (spec reference) in the request.env to the path + query parameters.

This is because warden sets the attempted_path to the request fullpath (which includes query parameters) here. That value is accessed the failure app here. The failure app later mutates the request env using the value here.

One side effect of this is that valid form authenticity tokens will be rejected when the failure app submits the recall action because rails takes the path into account when validating the token. IE: it's correct/valid when the form is originally submitted, but invalid when the failure app calls the recall action (defaults to :new). This results in a authenticity token error.

Expected behavior

The default failure app should not include query parameters when mutating PATH_INFO in the request.env. It is my belief that PATH_INFO should only contain the path.

I'm creating this issue to get some feedback. I believe the solution may be as simple as stripping query parameters here. I'm happy to open a PR to do that if anyone can confirm that my assertion regarding the expected behavior is correct.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant