Skip to content

Commit

Permalink
sso: prevent upstream switching (#299)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jusshersmith authored Jul 28, 2020
1 parent 0060ecd commit 56c9209
Show file tree
Hide file tree
Showing 4 changed files with 194 additions and 111 deletions.
5 changes: 5 additions & 0 deletions internal/pkg/logging/logging.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,11 @@ func (l *LogEntry) WithEndpoint(endpoint string) *LogEntry {
return l.withField("endpoint", endpoint)
}

// WithAuthorizedUpstream appends an `authorized_upstream` tag to a LogEntry.
func (l *LogEntry) WithAuthorizedUpstream(upstream string) *LogEntry {
return l.withField("authorized_upstream", upstream)
}

// WithError appends an `error` tag to a LogEntry. Useful for annotating non-Error log
// entries (e.g. Fatal messages) with an `error` object.
func (l *LogEntry) WithError(err error) *LogEntry {
Expand Down
7 changes: 4 additions & 3 deletions internal/pkg/sessions/session_state.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ type SessionState struct {
ValidDeadline time.Time `json:"valid_deadline"`
GracePeriodStart time.Time `json:"grace_period_start"`

Email string `json:"email"`
User string `json:"user"`
Groups []string `json:"groups"`
Email string `json:"email"`
User string `json:"user"`
Groups []string `json:"groups"`
AuthorizedUpstream string `json:"authorized_upstream"`
}

// LifetimePeriodExpired returns true if the lifetime has expired
Expand Down
29 changes: 26 additions & 3 deletions internal/proxy/oauthproxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ var SignatureHeaders = []string{

// Errors
var (
ErrLifetimeExpired = errors.New("user lifetime expired")
ErrUserNotAuthorized = errors.New("user not authorized")
ErrWrongIdentityProvider = errors.New("user authenticated with wrong identity provider")
ErrLifetimeExpired = errors.New("user lifetime expired")
ErrUserNotAuthorized = errors.New("user not authorized")
ErrWrongIdentityProvider = errors.New("user authenticated with wrong identity provider")
ErrUnauthorizedUpstreamRequested = errors.New("user session authorized with different upstream")
)

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

// We add the request host into the session to allow us to validate that each request has
// been authorized for the upstream it's requesting.
// e.g. if a request is authenticated while trying to reach 'foo' upstream, it should not
// automatically be seen as authorized with 'bar' upstream. Each upstream may set different
// validators, so the request should be reauthenticated.
session.AuthorizedUpstream = req.Host

// We store the session in a cookie and redirect the user back to the application
err = p.sessionStore.SaveSession(rw, req, session)
if err != nil {
Expand Down Expand Up @@ -655,6 +663,12 @@ func (p *OAuthProxy) Proxy(rw http.ResponseWriter, req *http.Request) {
// the user has a stale sesssion.
p.OAuthStart(rw, req, tags)
return
case ErrUnauthorizedUpstreamRequested:
// The users session has been authorised for use with a different upstream than the one
// that is being requested, so we trigger the start of the oauth flow.
// This exists primarily to implement some form of grace period while this additional session
// check is being introduced.
p.OAuthStart(rw, req, tags)
case sessions.ErrInvalidSession:
// The user session is invalid and we can't decode it.
// This can happen for a variety of reasons but the most common non-malicious
Expand Down Expand Up @@ -720,6 +734,15 @@ func (p *OAuthProxy) Authenticate(rw http.ResponseWriter, req *http.Request) (er
return ErrWrongIdentityProvider
}

// check that the user has been authorized against the requested upstream
// this is primarily to combat against a user authorizing with one upstream and attempting to use
// the session cookie for a different upstream.
if req.Host != session.AuthorizedUpstream {
logger.WithProxyHost(req.Host).WithAuthorizedUpstream(session.AuthorizedUpstream).WithUser(session.Email).Warn(
"session authorized against different upstream; restarting authentication")
return ErrUnauthorizedUpstreamRequested
}

// Lifetime period is the entire duration in which the session is valid.
// This should be set to something like 14 to 30 days.
if session.LifetimePeriodExpired() {
Expand Down
Loading

0 comments on commit 56c9209

Please sign in to comment.