Redis sessions

[sporkmonger]

Problem

Session state can become quite large and can easily exceed the maximum cookie size. I previously tried to fix it with #150 but that proved to be a bad solution prone to failures.

Solution

This introduces optional support for a Redis backend for sessions. Setting a SESSION_REDIS_CONNECTION environment variable to e.g. redis://primary.sso-sessions.example-cloud.com:6379 will switch the session backend from cookie sessions to redis sessions. The default remains cookie sessions.

Notes

This also avoids the issue of large amounts of session state being retransmitted on every request. Session state is encrypted using the existing cipher mechanism and session keys. As discussed in #158, I recommend renaming the cipher key configuration name to be more generic, but this PR doesn't make that change. This PR does not use the ticket system mentioned in #158, and instead opts for the simpler approach of encrypting using the session secret held on the auth server. I have tested this PR in conjunction with #118 and it works well.

[codecov[bot]]

Codecov Report

Merging #224 into master will decrease coverage by 2.61%. The diff coverage is 20%.

@@            Coverage Diff             @@
##           master     #224      +/-   ##
==========================================
- Coverage   62.23%   59.62%   -2.62%     
==========================================
  Files          50       51       +1     
  Lines        4067     4225     +158     
==========================================
- Hits         2531     2519      -12     
- Misses       1349     1510     +161     
- Partials      187      196       +9

Impacted Files	Coverage Δ
internal/proxy/options.go	83.59% <ø> (ø)	:arrow_up:
internal/proxy/proxy.go	22.72% <0%> (-1.67%)	:arrow_down:
internal/auth/options.go	52.89% <0%> (-25.68%)	:arrow_down:
internal/proxy/oauthproxy.go	41.27% <0%> (-9.71%)	:arrow_down:
internal/pkg/sessions/redis_store.go	29.72% <29.72%> (ø)
internal/auth/configuration.go	49.26% <43.75%> (-0.47%)	:arrow_down:
internal/auth/mux.go	71.18% <50%> (-3.82%)	:arrow_down:
internal/pkg/sessions/session_state.go	48.57% <0%> (-36.43%)	:arrow_down:
... and 8 more

[jphines]

@sporkmonger this is much cleaner than I expected, which is fantastic!

I'll take a look at this today/this weekend, provide some review, and kick the tires.

Thank you for your work on this effort, it is a fantastic set of functionality. I (we) greatly appreciate your contributions!

[sporkmonger]

Sure thing. I had some trouble figuring out how to mock the Redis client, which is why there's a bit of a coverage drop I believe.

[sporkmonger]

@jphines Any updates?

[sylr]

@sporkmonger it seems there currently is a conflict, could you resolve it ?

@jphines Do you have time to review this ?

[jphines]

My apologizes, I've failed to prioritize review here appropriately. I've asked @Jusshersmith to review these changes in light of my inability to find the time to do so.

[sporkmonger]

Yeah, I'll try to address the conflict shortly.

[Jusshersmith]

This looks great @sporkmonger, and thank you for your patience! I've left some comments to start with!

[sylr]

Any updates ?

[sylr]

Guys ?

[sporkmonger]

It's taken long enough that... we ended up moving from Azure AD to Okta in the meantime.

[sylr]

😭

[Jusshersmith]

Thank you for all of your great work on this @sporkmonger! This seems like it could still be an extremely valuable addition, and would be a shame for all of this work to go to waste. I don't think we at BuzzFeed necessarily have the capacity to push it over the finish line in the immediate future, but I think we can keep this open for a while still to allow someone to pick it up if possible -- providing you're happy with that @sporkmonger.

[sporkmonger]

Totally. My bandwidth for stuff like this goes up and down a lot, but the next time I've got some spare cycles I'll try to get this addressed. If someone else wants to pick it up and get it over the finish line, I'm equally OK with that too.