Skip to content
This repository has been archived by the owner on Jan 24, 2019. It is now read-only.

Use X-Original-URI for redirect if used in as 401 error_page #247

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Use X-Original-URI for redirect if used in as 401 error_page #247

wants to merge 5 commits into from

Conversation

sjoerdmulder
Copy link
Contributor

Otherwise it always redirects to /oauth2

@rremer
Copy link
Contributor

rremer commented Jun 11, 2016

@sjoerdmulder you may want to fetch upstream and repush, I suspect your build is fine and travis was just broken. It should work now.

@semenko
Copy link
Contributor

semenko commented Jun 28, 2016

Hey, thanks! This is incredibly useful for nginx auth_request

I'll try to clean up some of its documentation for #226, which is blocking on this PR, and #173

@semenko
Copy link
Contributor

semenko commented Jul 4, 2016

@sjoerdmulder Could you pass along a sample nginx config that you're using for this? Redirecting with 401 error_page doesn't seem to set X-Original-URI for me.

@sjoerdmulder
Copy link
Contributor Author

@semenko You should pass it with proxy_set_header

location = /oauth2/auth {
   internal;
   proxy_set_header Host $host;
   proxy_set_header X-Real-IP $remote_addr;
   proxy_set_header X-Scheme $scheme;
   proxy_set_header X-Original-URI $request_uri;
   proxy_set_header Content-Length "";
   proxy_pass_request_body off;
   proxy_pass http://oauth2-proxy;
   proxy_http_version 1.1;
   proxy_set_header Connection "";
}
location /oauth2 {
   proxy_set_header Host $host;
   proxy_set_header X-Real-IP $remote_addr;
   proxy_set_header X-Scheme $scheme;
   proxy_set_header X-Original-URI $request_uri;
   proxy_pass http://oauth2-proxy;
   proxy_http_version 1.1;
   proxy_set_header Connection "";
}

location / {
    auth_request /oauth2/auth;
    error_page 401 = /oauth2;
    ... upstream
}

@reist
Copy link

reist commented Aug 4, 2016

Hi,
Would you be interested in adding the same check to OAuthStart, so that authentication that skips the sign-in step will redirect as well?

@sjoerdmulder
Copy link
Contributor Author

@reist Good one, i also added it to the OAuthStart section. Build is somehow failing with another unrelated message?

@tamsky
Copy link
Contributor

tamsky commented Aug 25, 2016

Build is fixed in #299 if you want to try with that.

@ashkulz
Copy link
Contributor

ashkulz commented Nov 16, 2016

@reist: when I added it to OAuthStart in #319, it resulted in an infinite loop -- so I did not include that change in that PR.

@ys
Copy link

ys commented Nov 21, 2016

Will this land soon?

fschaefer pushed a commit to Securepoint/oauth2_proxy-legacy that referenced this pull request Nov 25, 2016
This is useful in Nginx auth_request mode, if a 401 handler is
configured to redirect to the sign-in page. As the request URL
does not reflect the actual URL, the value is taken from the
header "X-Auth-Request-Redirect" instead. Based on bitly#247
AaronKalair pushed a commit to conversocial/oauth2_proxy that referenced this pull request Feb 1, 2017
This is useful in Nginx auth_request mode, if a 401 handler is
configured to redirect to the sign-in page. As the request URL
does not reflect the actual URL, the value is taken from the
header "X-Auth-Request-Redirect" instead. Based on bitly#247
@ashkulz
Copy link
Contributor

ashkulz commented Mar 29, 2017

#319 has been merged, so this can be closed (other than the OAuthStart part).

@sjoerdmulder
Copy link
Contributor Author

@ashkulz I have updated the PR again... now using the header that is suggested in #319
This PR still adds value since current master you get redirected to the / of the site your authenticated too instead of the target url that can be sent to the oauth-redirect

@ploxiln
Copy link
Contributor

ploxiln commented Apr 26, 2017

Current master redirects you to the original page you were trying to view, if you use oauth2_proxy in the original proxy mode, or if you use auth_request mode and keep the sign-in page/button enabled (and set the X-Auth-Request-Redirect header).

The problem that remains (in master) is when you use auth_request mode and disable the sign-in page/button, you'll end up at "/" instead of the page you were originally trying to view.

I think this proposed fix does not cover enough cases correctly. If the sign-in page is enabled, and the nginx config is not particularly sophisticated, you'll get the redirect loop that @ashkulz mentioned, because the sign-in page will post to the oauth-start page which will get its own URI in the redirect header (depending on how sophisticated the nginx config is), and when oauth completes, the callback page will end up redirecting back to the oauth-start URI. I think there are other cases like this as well.

Also, it looks like if the sign-in page/button is disabled, and proxy mode is used (instead of nginx auth_request), the redirect back to the originally desired page also does not work, and needs a different fix (getting the RequestURI() like SignInPage() does).

So, I think more than this is needed for --skip-provider-button to redirect back to the original page reliably and not break anything else.

@Beanow
Copy link

Beanow commented Oct 17, 2017

Shouldn't this be using the X-Forwarded-* headers to be compatible with other (less configurable than nginx) proxies?

madmod pushed a commit to daffinity/oauth2_proxy that referenced this pull request Dec 1, 2017
This is useful in Nginx auth_request mode, if a 401 handler is
configured to redirect to the sign-in page. As the request URL
does not reflect the actual URL, the value is taken from the
header "X-Auth-Request-Redirect" instead. Based on bitly#247
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

9 participants