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

Add websocket support #201

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

Add websocket support #201

wants to merge 2 commits into from

Conversation

soellman
Copy link

This PR adds support for transparent websocket proxying within the HTTP and HTTPS upstreams.

This method uses gorilla/websocket for both the client and the upstream connections and just bridges the sockets when negotiation of both sides is successful. It also tries to close the connections properly after one side or another is done, although more precise error handling could be done there.

@apsey
Copy link

apsey commented Apr 2, 2016

I was having websockets issues with ipython terminal (terminado) when using grahamrhay's solution (#145). After reverting it and adding this implementation, it started working. Thanks!

@soellman
Copy link
Author

soellman commented Apr 5, 2016

I'm glad it worked for you!

@corlettb
Copy link

corlettb commented Apr 5, 2016

Had a slight problem with this and firefox. Tweaked it to work with:

corlettb@151f03f

@soellman
Copy link
Author

soellman commented Apr 7, 2016

I've updated the PR with better handling of the upgrade/connection headers - it should handle the firefox case now. Thanks!

@jhoblitt
Copy link
Contributor

👍 I've tested this as working with a websockets app (bokeh) that is broken by v2.0.1.

@jhoblitt
Copy link
Contributor

I've dropped a copy of my test build of this PR on s3.

https://s3.amazonaws.com/oauth2_proxy/oauth2_proxy-2.0.1.linux-amd64.go1.5.4.tar.gz
sha1sum is ea8f13932790fb1f1da2a54db9a7b168cf192615

@jml
Copy link

jml commented Jun 12, 2016

We're using this branch on our servers and it works great. Would welcome it being merged & released.

@soellman
Copy link
Author

@jehiah I'd be happy to help in any way get this merged. Please do let me know if there's anything I can do!

@jhoblitt
Copy link
Contributor

I am using my binary build above in production. There have been a few unexpected exits (systemd disables core dumps by default, so it is unknown if it was an abort or clean exit) but after changing systemd to always restart the proxy, the problem has not been observed again.

@teh
Copy link

teh commented Jun 18, 2016

Another data point: Tested this with Jupyter notebooks with the Github provider, no SSL, and it works for that case. Code looks OK too.

@gurvindersingh
Copy link

@jehiah any plans to merge this and add support for websockets ?

Thanks for the nice work :)

@nside
Copy link

nside commented Jun 24, 2016

+1

1 similar comment
@daeyun
Copy link

daeyun commented Jul 13, 2016

+1

@ploxiln ploxiln mentioned this pull request Jul 25, 2016
@mogthesprog
Copy link

Is there any hope of this getting merged into stable anytime soon? :)

@nneul
Copy link

nneul commented Sep 15, 2016

FYI: This may be of use to those using this patch in conjunction with apache to handle vhosts. (Would sure be nice to have vhost support in oauth2_proxy too!)

http://stackoverflow.com/questions/22002744/how-to-set-up-an-apache-proxy-for-meteor-sockjs-and-websocket

@FrederikNJS
Copy link

Any progress on this? I could really use Websocket support for a project I'm currently working on.

@FrederikNJS
Copy link

@jehiah Is there any plans to merge this? It looks like multiple people have already tested it.

@hgomez-sonarsource
Copy link

A very welcomed feature, could it be merged ?

@mathewpeterson
Copy link

Not that I don't think this should be merged but you can use the ngix auth_request directive in order to make websockets work.

knumor pushed a commit to UNINETTSigma2/goidc-proxy that referenced this pull request Mar 2, 2017
@weargoggles
Copy link

@jehiah Can you please provide feedback on what needs to be done to make this pull request acceptable?

@weargoggles
Copy link

In the meantime I've built binaries for the current version, with this patch, and published them here https://github.com/weargoggles/oauth2_proxy/releases/tag/v2.2-websocket

for _, header := range HandshakeHeaders {
delete(upstreamHeader, header)
}
upstreamHeader.Set("Host", r.Host)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this also needs X-Forwarded-Proto, otherwise a rails app behind the proxy would fail the handshake. Rails has a config.force_ssl setting and it affects ws/wss as well.

ConnectionHeaderValue = "Upgrade"
UpgradeHeaderValue = "websocket"

HandshakeHeaders = []string{ConnectionHeaderKey, UpgradeHeaderKey, WSVersionHeaderKey, WSKeyHeaderKey}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to add "sec-websocket-extensions" to this list to get it to work. (See https://github.com/gorilla/websocket/blob/a91eba7f97777409bc2c443f5534d41dd20c5720/client.go#L237)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find, I also needed this!

@mizzao mizzao mentioned this pull request Jan 5, 2018
@mizzao
Copy link

mizzao commented Jan 5, 2018

Are there any options that need to be turned on for this to work? I'm getting the following error when trying to use WS, though normal HTTP works:

'wss://example.com/sockjs/535/hz93j02u/websocket' failed: One or more reserved bits are on: reserved1 = 1, reserved2 = 0, reserved3 = 0

joejulian added a commit to joejulian/oauth2_proxy that referenced this pull request Mar 2, 2018
This was merged as commit 7f5702b
joejulian added a commit to joejulian/oauth2_proxy that referenced this pull request Mar 2, 2018
This was merged as commit 7f5702b
bertrandmartel added a commit to bertrandmartel/oauth2_proxy that referenced this pull request Oct 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.