From 10e71a7bb5665002ee5fe29fcd1b716815a7c934 Mon Sep 17 00:00:00 2001 From: Trong Huu Nguyen Date: Thu, 11 Jan 2024 10:27:45 +0100 Subject: [PATCH] feat(handler/reverseproxy): remove x-wonderwall headers The use of these headers in upstreams may be risky, espeically if Wonderwall is accidentally misconfigured or disabled, or requests are performed directly to the upstream circumventing Wonderwall. We should prefer using a signed token or similar that can be verified by the upstreams. --- pkg/handler/reverseproxy.go | 38 ----------------------------------- pkg/middleware/context.go | 40 ------------------------------------- pkg/session/data.go | 16 ++++++--------- pkg/session/session.go | 15 -------------- pkg/session/store_test.go | 2 -- 5 files changed, 6 insertions(+), 105 deletions(-) diff --git a/pkg/handler/reverseproxy.go b/pkg/handler/reverseproxy.go index bac13a2..849c0d3 100644 --- a/pkg/handler/reverseproxy.go +++ b/pkg/handler/reverseproxy.go @@ -67,31 +67,6 @@ func NewReverseProxy(upstream *urllib.URL, preserveInboundHostHeader bool) *Reve if ok { r.Out.Header.Set("authorization", "Bearer "+accessToken) } - - r.Out.Header.Del("X-Wonderwall-Acr") - r.Out.Header.Del("X-Wonderwall-Amr") - r.Out.Header.Del("X-Wonderwall-Auth-Time") - r.Out.Header.Del("X-Wonderwall-Sid") - - sessAcr, ok := mw.AcrFrom(r.In.Context()) - if ok && sessAcr != "" { - r.Out.Header.Set("X-Wonderwall-Acr", sessAcr) - } - - amr, ok := mw.AmrFrom(r.In.Context()) - if ok && amr != "" { - r.Out.Header.Set("X-Wonderwall-Amr", amr) - } - - authTime, ok := mw.AuthTimeFrom(r.In.Context()) - if ok && authTime != "" { - r.Out.Header.Set("X-Wonderwall-Auth-Time", authTime) - } - - sid, ok := mw.SessionIDFrom(r.In.Context()) - if ok && sid != "" { - r.Out.Header.Set("X-Wonderwall-Sid", sid) - } }, Transport: server.DefaultTransport(), } @@ -123,21 +98,8 @@ func (rp *ReverseProxy) Handler(src ReverseProxySource, w http.ResponseWriter, r ctx := r.Context() if sess != nil { - if sessAcr := sess.Acr(); sessAcr != "" { - ctx = mw.WithAcr(ctx, sessAcr) - } - - if amr := sess.Amr(); amr != "" { - ctx = mw.WithAmr(ctx, amr) - } - - if authTime := sess.AuthTime(); authTime != "" { - ctx = mw.WithAuthTime(ctx, authTime) - } - if sid := sess.ExternalSessionID(); sid != "" { logger = logger.WithField("sid", sid) - ctx = mw.WithSessionID(ctx, sid) } } diff --git a/pkg/middleware/context.go b/pkg/middleware/context.go index cf28053..e0a136a 100644 --- a/pkg/middleware/context.go +++ b/pkg/middleware/context.go @@ -11,10 +11,6 @@ type contextKey string const ( ctxAccessToken = contextKey("AccessToken") - ctxAcr = contextKey("Acr") - ctxAmr = contextKey("Amr") - ctxAuthTime = contextKey("AuthTime") - ctxSessionID = contextKey("SessionID") ctxIngress = contextKey("Ingress") ctxPath = contextKey("Path") ) @@ -28,42 +24,6 @@ func WithAccessToken(ctx context.Context, accessToken string) context.Context { return context.WithValue(ctx, ctxAccessToken, accessToken) } -func AcrFrom(ctx context.Context) (string, bool) { - acr, ok := ctx.Value(ctxAcr).(string) - return acr, ok -} - -func WithAcr(ctx context.Context, acr string) context.Context { - return context.WithValue(ctx, ctxAcr, acr) -} - -func AmrFrom(ctx context.Context) (string, bool) { - amr, ok := ctx.Value(ctxAmr).(string) - return amr, ok -} - -func WithAmr(ctx context.Context, amr string) context.Context { - return context.WithValue(ctx, ctxAmr, amr) -} - -func AuthTimeFrom(ctx context.Context) (string, bool) { - authTime, ok := ctx.Value(ctxAuthTime).(string) - return authTime, ok -} - -func WithAuthTime(ctx context.Context, authTime string) context.Context { - return context.WithValue(ctx, ctxAuthTime, authTime) -} - -func SessionIDFrom(ctx context.Context) (string, bool) { - sessionID, ok := ctx.Value(ctxSessionID).(string) - return sessionID, ok -} - -func WithSessionID(ctx context.Context, sessionID string) context.Context { - return context.WithValue(ctx, ctxSessionID, sessionID) -} - func IngressFrom(ctx context.Context) (ingress.Ingress, bool) { i, ok := ctx.Value(ctxIngress).(ingress.Ingress) return i, ok diff --git a/pkg/session/data.go b/pkg/session/data.go index de24f3e..f41e731 100644 --- a/pkg/session/data.go +++ b/pkg/session/data.go @@ -49,14 +49,12 @@ func (in *EncryptedData) Decrypt(crypter crypto.Crypter) (*Data, error) { } type Data struct { - ExternalSessionID string `json:"external_session_id"` - AccessToken string `json:"access_token"` - IDToken string `json:"id_token"` - RefreshToken string `json:"refresh_token"` - Acr string `json:"acr"` - Amr string `json:"amr"` - AuthTime time.Time `json:"auth_time"` - Metadata Metadata `json:"metadata"` + ExternalSessionID string `json:"external_session_id"` + AccessToken string `json:"access_token"` + IDToken string `json:"id_token"` + RefreshToken string `json:"refresh_token"` + Acr string `json:"acr"` + Metadata Metadata `json:"metadata"` } func NewData(externalSessionID string, tokens *openid.Tokens, metadata *Metadata) *Data { @@ -66,8 +64,6 @@ func NewData(externalSessionID string, tokens *openid.Tokens, metadata *Metadata IDToken: tokens.IDToken.GetSerialized(), RefreshToken: tokens.RefreshToken, Acr: tokens.IDToken.GetAcrClaim(), - Amr: tokens.IDToken.GetAmrClaim(), - AuthTime: tokens.IDToken.GetAuthTimeClaim(), } if metadata != nil { diff --git a/pkg/session/session.go b/pkg/session/session.go index 006adee..b35a673 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "net/http" - "strconv" "time" "github.com/nais/wonderwall/pkg/cookie" @@ -69,20 +68,6 @@ func (in *Session) Acr() string { return "" } -func (in *Session) Amr() string { - if in.data != nil { - return in.data.Amr - } - return "" -} - -func (in *Session) AuthTime() string { - if in.data != nil && !in.data.AuthTime.IsZero() { - return strconv.FormatInt(in.data.AuthTime.Unix(), 10) - } - return "" -} - func (in *Session) ExternalSessionID() string { if in.data != nil { return in.data.ExternalSessionID diff --git a/pkg/session/store_test.go b/pkg/session/store_test.go index 86b7a4f..2cf9c00 100644 --- a/pkg/session/store_test.go +++ b/pkg/session/store_test.go @@ -20,8 +20,6 @@ func decryptedEqual(t *testing.T, expected, actual *session.Data) { assert.Equal(t, expected.IDToken, actual.IDToken) assert.Equal(t, expected.ExternalSessionID, actual.ExternalSessionID) assert.Equal(t, expected.Acr, actual.Acr) - assert.Equal(t, expected.Amr, actual.Amr) - assert.WithinDuration(t, expected.AuthTime, actual.AuthTime, 0) assert.WithinDuration(t, expected.Metadata.Session.CreatedAt, actual.Metadata.Session.CreatedAt, 0) assert.WithinDuration(t, expected.Metadata.Session.EndsAt, actual.Metadata.Session.EndsAt, 0) assert.WithinDuration(t, expected.Metadata.Tokens.ExpireAt, actual.Metadata.Tokens.ExpireAt, 0)