heartcombo / devise

Flexible authentication solution for Rails with Warden.
http://blog.plataformatec.com.br/tag/devise/
MIT License
24.01k stars 5.55k forks source link

Failure App incorrectly adds url query parameters to PATH_INFO #5704

Open benjaminwood opened 3 months ago

benjaminwood commented 3 months ago

Environment

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!