Skip to content

Commit 56c9209

Browse files
authored
sso: prevent upstream switching (#299)
1 parent 0060ecd commit 56c9209

File tree

4 files changed

+194
-111
lines changed

4 files changed

+194
-111
lines changed

internal/pkg/logging/logging.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,11 @@ func (l *LogEntry) WithEndpoint(endpoint string) *LogEntry {
144144
return l.withField("endpoint", endpoint)
145145
}
146146

147+
// WithAuthorizedUpstream appends an `authorized_upstream` tag to a LogEntry.
148+
func (l *LogEntry) WithAuthorizedUpstream(upstream string) *LogEntry {
149+
return l.withField("authorized_upstream", upstream)
150+
}
151+
147152
// WithError appends an `error` tag to a LogEntry. Useful for annotating non-Error log
148153
// entries (e.g. Fatal messages) with an `error` object.
149154
func (l *LogEntry) WithError(err error) *LogEntry {

internal/pkg/sessions/session_state.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,10 @@ type SessionState struct {
2525
ValidDeadline time.Time `json:"valid_deadline"`
2626
GracePeriodStart time.Time `json:"grace_period_start"`
2727

28-
Email string `json:"email"`
29-
User string `json:"user"`
30-
Groups []string `json:"groups"`
28+
Email string `json:"email"`
29+
User string `json:"user"`
30+
Groups []string `json:"groups"`
31+
AuthorizedUpstream string `json:"authorized_upstream"`
3132
}
3233

3334
// LifetimePeriodExpired returns true if the lifetime has expired

internal/proxy/oauthproxy.go

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,10 @@ var SignatureHeaders = []string{
4040

4141
// Errors
4242
var (
43-
ErrLifetimeExpired = errors.New("user lifetime expired")
44-
ErrUserNotAuthorized = errors.New("user not authorized")
45-
ErrWrongIdentityProvider = errors.New("user authenticated with wrong identity provider")
43+
ErrLifetimeExpired = errors.New("user lifetime expired")
44+
ErrUserNotAuthorized = errors.New("user not authorized")
45+
ErrWrongIdentityProvider = errors.New("user authenticated with wrong identity provider")
46+
ErrUnauthorizedUpstreamRequested = errors.New("user session authorized with different upstream")
4647
)
4748

4849
type ErrOAuthProxyMisconfigured struct {
@@ -591,6 +592,13 @@ func (p *OAuthProxy) OAuthCallback(rw http.ResponseWriter, req *http.Request) {
591592
logger.WithRemoteAddress(remoteAddr).WithUser(session.Email).WithInGroups(session.Groups).Info(
592593
fmt.Sprintf("oauth callback: user validated "))
593594

595+
// We add the request host into the session to allow us to validate that each request has
596+
// been authorized for the upstream it's requesting.
597+
// e.g. if a request is authenticated while trying to reach 'foo' upstream, it should not
598+
// automatically be seen as authorized with 'bar' upstream. Each upstream may set different
599+
// validators, so the request should be reauthenticated.
600+
session.AuthorizedUpstream = req.Host
601+
594602
// We store the session in a cookie and redirect the user back to the application
595603
err = p.sessionStore.SaveSession(rw, req, session)
596604
if err != nil {
@@ -655,6 +663,12 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) {
655663
// the user has a stale sesssion.
656664
p.OAuthStart(rw, req, tags)
657665
return
666+
case ErrUnauthorizedUpstreamRequested:
667+
// The users session has been authorised for use with a different upstream than the one
668+
// that is being requested, so we trigger the start of the oauth flow.
669+
// This exists primarily to implement some form of grace period while this additional session
670+
// check is being introduced.
671+
p.OAuthStart(rw, req, tags)
658672
case sessions.ErrInvalidSession:
659673
// The user session is invalid and we can't decode it.
660674
// This can happen for a variety of reasons but the most common non-malicious
@@ -720,6 +734,15 @@ func (p *OAuthProxy) Authenticate(rw http.ResponseWriter, req *http.Request) (er
720734
return ErrWrongIdentityProvider
721735
}
722736

737+
// check that the user has been authorized against the requested upstream
738+
// this is primarily to combat against a user authorizing with one upstream and attempting to use
739+
// the session cookie for a different upstream.
740+
if req.Host != session.AuthorizedUpstream {
741+
logger.WithProxyHost(req.Host).WithAuthorizedUpstream(session.AuthorizedUpstream).WithUser(session.Email).Warn(
742+
"session authorized against different upstream; restarting authentication")
743+
return ErrUnauthorizedUpstreamRequested
744+
}
745+
723746
// Lifetime period is the entire duration in which the session is valid.
724747
// This should be set to something like 14 to 30 days.
725748
if session.LifetimePeriodExpired() {

0 commit comments

Comments
 (0)