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

Redirect Insecure Requests with the X-Forwarded-Proto header #556

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

Redirect Insecure Requests with the X-Forwarded-Proto header #556

wants to merge 3 commits into from

Conversation

tomasbasham
Copy link

@tomasbasham tomasbasham commented Mar 2, 2018

The X-Forwarded-Proto header is the standard way to identify over what protocol a request has been made by a client to a proxy or load balancer. Some load balancers (including Google Cloud Load Balancer - one which I use) set this header before forwarding the request.

I have setup oauth2_proxy to sit behind the GCLB (that performs TLS termination) however currently when I hit the load balancer over HTTP it does not redirect to HTTPS before initiating the OAuth dance causing the authentication to fail due to the mismatch of protocol. To prevent this I have had to sit nginx in front of oauth2_proxy just to do the HTTPS redirection. The current infrastructure look like the following:

GCLB -> nginx -> oauth2_proxy -> app

This PR introduces the ability for the oauth2_proxy to handle the redirection by respecting the X-Forwarded-Proto header removing the need for an extra proxy (nginx or similar).

someburner added a commit to someburner/oauth2_proxy that referenced this pull request Mar 22, 2018
@tomasbasham tomasbasham changed the title Respect the X-Forwarded-Proto header Rediret Respect the X-Forwarded-Proto header May 22, 2018
@tomasbasham tomasbasham changed the title Rediret Respect the X-Forwarded-Proto header Redirect Insecure Requests with the X-Forwarded-Proto header May 22, 2018
@bravoecho
Copy link

bravoecho commented May 22, 2018

Hi, it would be lovely if anyone could help with the dep ensure failing on Travis. We can't replicate locally.

We would then greatly appreciate if this PR was considered for review and merging because at the moment we are using a fork, and if the functionality doesn't get into upstream we will be forced to revert to an needlessly complicated nginx setup sidecar in our deployment.

Thank you!! 🙂

@tomasbasham
Copy link
Author

@jehiah Is there any chance you can take a look at this? We are relying on this feature and ideally would not want to revert to having an nginx proxy in the middle.

Also if you have any idea why dep ensure is failing on travis it would be appreciated. Seems to be a common issue on other PRs

@ploxiln
Copy link
Contributor

ploxiln commented May 30, 2018

re: dep ensure: see #574 and #595 - the inotify situation is confusing. your local dep may have cached the inotify import metadata which enables it to continue working. On completely fresh systems it doesn't quite work.

@tomasbasham
Copy link
Author

Thanks @ploxiln. I guess there is noting I can do until either of these gets merged save me having to duplicate the effort.

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.

3 participants