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

return_url ignores url parameter from POST request #66

Open
dmolesUC opened this issue Mar 21, 2022 · 1 comment · May be fixed by #67
Open

return_url ignores url parameter from POST request #66

dmolesUC opened this issue Mar 21, 2022 · 1 comment · May be fixed by #67

Comments

@dmolesUC
Copy link

dmolesUC commented Mar 21, 2022

Steps to reproduce:

  1. Configure OmniAuth to only allow POST requests to /auth/:provider as discussed in the Resolving CVE-2015-9284 wiki page.
  2. Make a POST request to /auth/calnet, providing a url parameter in the request body:
curl -v 'http://localhost:3000/auth/calnet' \
  -H 'Content-Type: application/x-www-form-urlencoded' \
  -d 'url=https%3A%2F%2Fright.example.test%2F'

Expected:

  • Location header in response includes the provided URL https://right.example.test/

Actual:

  • Location header in response does not include any url parameter
Location: https://auth-test.berkeley.edu/cas/login?
  service=http%3A%2F%2Flocalhost%3A3000%2Fauth%2Fcalnet%2Fcallback%3F

Notes:

The issue is the assumption commented on line 211 here:

def return_url
# If the request already has a `url` parameter, then it will already be appended to the callback URL.
if request.params && request.params['url']
{}
else
{ url: request.referer }
end
end

This only works for GET requests; for POST requests the parameter exists, but was never in in request.query_string to be copied by OmniAuth into the callback URL (see #callback_url and #query_string in OmniAuth::Strategy).

The result is that both the provided url parameter and the Referer header (if provided) are ignored.

@dmolesUC
Copy link
Author

I have a pull request ready to go (#67), but all that said, I'm not sure how important this is except as a way of minimizing migration effort, since OmniAuth separately provides more or less the same functionality via the origin param.

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

Successfully merging a pull request may close this issue.

1 participant