buzzfeed / sso

sso, aka S.S.Octopus, aka octoboi, is a single sign-on solution for securing internal services
MIT License
3.09k stars 186 forks source link

sso-proxy: add websocket support #245

Closed niksrc closed 5 years ago

niksrc commented 5 years ago

Problem

Add support for WebSockets

Solution

What is the proposed solution? Since we started building on go1.12 with native WebSocket proxy support, the only thing needed is to implement Hijacker on responseLogger to support WebSockets. Fixes #103

Notes

To proxy WebSocket successfully, an appropriate flush_interval is required. This pr is tested to proxy weave-scope with a flush_interval of 0.1s

codecov[bot] commented 5 years ago

Codecov Report

Merging #245 into master will decrease coverage by 0.06%. The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #245      +/-   ##
==========================================
- Coverage   62.22%   62.16%   -0.07%     
==========================================
  Files          50       50              
  Lines        4072     4076       +4     
==========================================
  Hits         2534     2534              
- Misses       1350     1354       +4     
  Partials      188      188
Impacted Files Coverage Δ
internal/proxy/logging_handler.go 14.75% <0%> (-1.04%) :arrow_down:
codecov[bot] commented 5 years ago

Codecov Report

Merging #245 into master will decrease coverage by 0.06%. The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #245      +/-   ##
==========================================
- Coverage   62.22%   62.16%   -0.07%     
==========================================
  Files          50       50              
  Lines        4072     4076       +4     
==========================================
  Hits         2534     2534              
- Misses       1350     1354       +4     
  Partials      188      188
Impacted Files Coverage Δ
internal/proxy/logging_handler.go 14.75% <0%> (-1.04%) :arrow_down:
Jusshersmith commented 5 years ago

Hi @niksrc,

Thank you for the contribution! We'll get this reviewed and responded to properly in the coming days (early next week most likely).

Thanks for your patience!

yacut commented 5 years ago

@Jusshersmith Any update on this?

Jusshersmith commented 5 years ago

I've completely failed to follow up on this, for which I am extremely sorry!

I am however now actively testing this change.

Jusshersmith commented 5 years ago

There are some followup additions I'd like to land (such as another test, and some documentation changes), though seeing as this is a fork and to expedite this pull request, I'll add these afterwards in a new pull request.

Thanks for your patience on this everyone, and thank you again for the contribution @niksrc!

yacut commented 5 years ago

Great work! When will it be released?

Jusshersmith commented 4 years ago

Hey @yacut. We're planning on publishing a release this coming Monday!