Closed Jusshersmith closed 3 years ago
Merging #299 into master will increase coverage by
0.03%
. The diff coverage is85.71%
.
@@ Coverage Diff @@
## master #299 +/- ##
==========================================
+ Coverage 61.94% 61.98% +0.03%
==========================================
Files 57 57
Lines 4638 4645 +7
==========================================
+ Hits 2873 2879 +6
- Misses 1553 1554 +1
Partials 212 212
Impacted Files | Coverage Δ | |
---|---|---|
internal/pkg/sessions/session_state.go | 85.00% <ø> (ø) |
|
internal/proxy/oauthproxy.go | 55.05% <85.71%> (+0.51%) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update 0060ecd...a94ea18. Read the comment docs.
Problem
_We only revalidate group membership when the session is refreshed or revalidated_, which means that if a user were to successfully authorize with upstream 'foo', then they can effectively skip group membership validation on a different upstream by making the request with a slightly altered version of the saved cookie from the 'foo' upstream (providing the session is still valid and hasn't expired).
Solution
Add a new
AuthorizedUpstream
value to the session which is used to compare the upstream the session has been validated against, to the requested upstream. TheAuthorizedUpstream
value is checked against the request host on each request. For the time being, when caught this check will re-trigger the start of the oauth flow, primarily to help introduce this additional check in a graceful manner.Example log line when triggered:
upstream-1.foo.io
being the original upstream the session was used against, andupstream-2.foo.io
being the newly requested upstream.Notes