From 13978f92deba3a965569efead768e5bb55eb780b Mon Sep 17 00:00:00 2001 From: Celian GARCIA Date: Thu, 21 May 2026 20:23:47 +0200 Subject: [PATCH] fix(#3019): support combining extra_jwt_issuers with custom claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Célian Garcia Signed-off-by: Celian GARCIA --- CHANGELOG.md | 2 + oauthproxy.go | 12 ++- pkg/apis/middleware/session.go | 49 --------- pkg/middleware/jwt_session_test.go | 163 ++++++++++++++++++++++++++++- providers/oidc.go | 24 +---- providers/provider_data.go | 50 +++++++++ providers/provider_default.go | 3 +- 7 files changed, 222 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 320ba697..d05767cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ ## Changes since v7.15.2 +- [#3436](https://github.com/oauth2-proxy/oauth2-proxy/pull/3436) fix(#3019): support combining extra_jwt_issuers with custom claims (@celian-garcia) + # V7.15.2 ## Release Highlights diff --git a/oauthproxy.go b/oauthproxy.go index e2357c8d..69c7c930 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -417,9 +417,15 @@ func buildSessionChain(opts *options.Options, provider providers.Provider, sessi sessionLoaders := make([]middlewareapi.TokenToSessionFunc, 0, len(verifiers)+1) sessionLoaders = append(sessionLoaders, provider.CreateSessionFromToken) - for _, verifier := range opts.GetJWTBearerVerifiers() { - sessionLoaders = append(sessionLoaders, - middlewareapi.CreateTokenToSessionFunc(verifier.Verify)) + // Reuse the main provider's bearer-session pipeline + // (`ProviderData.CreateTokenToSessionFunc` -> `buildSessionFromClaims`) + // for every `--extra-jwt-issuers` verifier so that the configured + // claim mappings (UserClaim, EmailClaim, GroupsClaim, + // AdditionalClaims, email_verified...) are honored consistently + // for tokens from any trusted issuer (see issues #1243 / #3019). + pd := provider.Data() + for _, verifier := range verifiers { + sessionLoaders = append(sessionLoaders, pd.CreateTokenToSessionFunc(verifier.Verify)) } chain = chain.Append(middleware.NewJwtSessionLoader(sessionLoaders, opts.BearerTokenLoginFallback)) diff --git a/pkg/apis/middleware/session.go b/pkg/apis/middleware/session.go index afa56e9d..aba8dbfe 100644 --- a/pkg/apis/middleware/session.go +++ b/pkg/apis/middleware/session.go @@ -2,11 +2,9 @@ package middleware import ( "context" - "fmt" "github.com/coreos/go-oidc/v3/oidc" sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/util/ptr" ) // TokenToSessionFunc takes a raw ID Token and converts it into a SessionState. @@ -15,50 +13,3 @@ type TokenToSessionFunc func(ctx context.Context, token string) (*sessionsapi.Se // VerifyFunc takes a raw bearer token and verifies it returning the converted // oidc.IDToken representation of the token. type VerifyFunc func(ctx context.Context, token string) (*oidc.IDToken, error) - -// CreateTokenToSessionFunc provides a handler that is a default implementation -// for converting a JWT into a session. -func CreateTokenToSessionFunc(verify VerifyFunc) TokenToSessionFunc { - return func(ctx context.Context, token string) (*sessionsapi.SessionState, error) { - var claims struct { - Subject string `json:"sub"` - Email string `json:"email"` - Verified *bool `json:"email_verified"` - PreferredUsername string `json:"preferred_username"` - Groups []string `json:"groups"` - } - - idToken, err := verify(ctx, token) - if err != nil { - return nil, err - } - - if err := idToken.Claims(&claims); err != nil { - return nil, fmt.Errorf("failed to parse bearer token claims: %v", err) - } - - if claims.Email == "" { - claims.Email = claims.Subject - } - - // Ensure email is verified - // If the email is not verified, return an error - // If the email_verified claim is missing, assume it is verified - if !ptr.Deref(claims.Verified, true) { - return nil, fmt.Errorf("email in id_token (%s) isn't verified", claims.Email) - } - - newSession := &sessionsapi.SessionState{ - Email: claims.Email, - User: claims.Subject, - Groups: claims.Groups, - PreferredUsername: claims.PreferredUsername, - AccessToken: token, - IDToken: token, - RefreshToken: "", - ExpiresOn: &idToken.Expiry, - } - - return newSession, nil - } -} diff --git a/pkg/middleware/jwt_session_test.go b/pkg/middleware/jwt_session_test.go index 7b724280..5b6ffb15 100644 --- a/pkg/middleware/jwt_session_test.go +++ b/pkg/middleware/jwt_session_test.go @@ -17,6 +17,7 @@ import ( "github.com/golang-jwt/jwt/v5" middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware" sessionsapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" + "github.com/oauth2-proxy/oauth2-proxy/v7/providers" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" k8serrors "k8s.io/apimachinery/pkg/util/errors" @@ -31,6 +32,36 @@ func (noOpKeySet) VerifySignature(_ context.Context, jwt string) (payload []byte return base64.RawURLEncoding.DecodeString(payloadString) } +// testTokenToSession returns a TokenToSessionFunc that exercises the real +// production claim-extraction pipeline. It builds a minimally-configured +// ProviderData and delegates to ProviderData.CreateTokenToSessionFunc, so +// these middleware tests run through the same code path used at runtime +// by both the main provider and the `--extra-jwt-issuers` chain. +// +// No ProfileURL is configured because the bearer flow passes an empty +// access token, which makes the ClaimExtractor short-circuit any profile +// lookup (see ProviderData.getAuthorizationHeader and +// claimExtractor.loadProfileClaims). +// +// The CreatedAt timestamp set by buildSessionFromClaims is cleared so that +// the resulting session can be compared with deterministic fixtures. +func testTokenToSession(verify middlewareapi.VerifyFunc) middlewareapi.TokenToSessionFunc { + pd := &providers.ProviderData{ + UserClaim: "sub", + EmailClaim: "email", + GroupsClaim: "groups", + } + loader := pd.CreateTokenToSessionFunc(verify) + return func(ctx context.Context, token string) (*sessionsapi.SessionState, error) { + ss, err := loader(ctx, token) + if err != nil { + return nil, err + } + ss.CreatedAt = nil + return ss, nil + } +} + var _ = Describe("JWT Session Suite", func() { /* token payload: { @@ -109,7 +140,7 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=` rw := httptest.NewRecorder() sessionLoaders := []middlewareapi.TokenToSessionFunc{ - middlewareapi.CreateTokenToSessionFunc(verifier), + testTokenToSession(verifier), } // Create the handler with a next handler that will capture the session @@ -179,7 +210,7 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=` rw := httptest.NewRecorder() sessionLoaders := []middlewareapi.TokenToSessionFunc{ - middlewareapi.CreateTokenToSessionFunc(verifier), + testTokenToSession(verifier), } // Create the handler with a next handler that will capture the session @@ -261,7 +292,7 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=` j = &jwtSessionLoader{ jwtRegex: regexp.MustCompile(jwtRegexFormat), sessionLoaders: []middlewareapi.TokenToSessionFunc{ - middlewareapi.CreateTokenToSessionFunc(verifier), + testTokenToSession(verifier), }, } }) @@ -477,6 +508,11 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=` }) Context("CreateTokenToSessionFunc", func() { + // These tests exercise the bearer-token -> SessionState pipeline + // through testTokenToSession, which builds a real + // providers.ProviderData and delegates to + // ProviderData.CreateTokenToSessionFunc (the same code path used + // at runtime by the main provider and `--extra-jwt-issuers`). ctx := context.Background() expiresFuture := time.Now().Add(time.Duration(5) * time.Minute) verified := true @@ -517,7 +553,7 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=` rawIDToken, err := jwt.NewWithClaims(jwt.SigningMethodRS256, in.idToken).SignedString(key) Expect(err).ToNot(HaveOccurred()) - session, err := middlewareapi.CreateTokenToSessionFunc(verifier)(ctx, rawIDToken) + session, err := testTokenToSession(verifier)(ctx, rawIDToken) if in.expectedErr != nil { Expect(err).To(MatchError(in.expectedErr)) Expect(session).To(BeNil()) @@ -584,4 +620,123 @@ Nnc3a3lGVWFCNUMxQnNJcnJMTWxka1dFaHluYmI4Ongtb2F1dGgtYmFzaWM=` }), ) }) + + // Regression coverage for issues #1243 / #3019: bearer tokens (in + // particular those validated through `--extra-jwt-issuers`) must honor + // the provider's custom `--oidc-groups-claim` and `--oidc-email-claim` + // configuration when building the session, otherwise allowed-group + // checks fail for any token that does not happen to carry the + // hard-coded `groups` / `email` claim names. + Context("CreateTokenToSessionFunc with custom claim mappings", func() { + ctx := context.Background() + expiresFuture := time.Now().Add(5 * time.Minute) + + // Token claims that intentionally use non-default claim names + // ("upn" for the email-like identifier, "roles" for groups), as + // emitted by e.g. Microsoft Entra ID / ADFS tokens. + type customClaims struct { + UPN string `json:"upn,omitempty"` + Roles []string `json:"roles,omitempty"` + jwt.RegisteredClaims + } + + verifier := func(ctx context.Context, token string) (*oidc.IDToken, error) { + oidcVerifier := oidc.NewVerifier( + "https://issuer.example.com", + noOpKeySet{}, + &oidc.Config{ClientID: "asdf1234"}, + ) + return oidcVerifier.Verify(ctx, token) + } + + // Builds a TokenToSessionFunc on top of a ProviderData with the + // requested custom claim mappings, mirroring how oauthproxy.go + // configures it at runtime for `--extra-jwt-issuers`. + buildLoader := func(emailClaim, groupsClaim string) middlewareapi.TokenToSessionFunc { + pd := &providers.ProviderData{ + UserClaim: "sub", + EmailClaim: emailClaim, + GroupsClaim: groupsClaim, + } + loader := pd.CreateTokenToSessionFunc(verifier) + return func(ctx context.Context, token string) (*sessionsapi.SessionState, error) { + ss, err := loader(ctx, token) + if err != nil { + return nil, err + } + ss.CreatedAt = nil + return ss, nil + } + } + + signedToken := func(c customClaims) string { + key, err := rsa.GenerateKey(rand.Reader, 2048) + Expect(err).ToNot(HaveOccurred()) + raw, err := jwt.NewWithClaims(jwt.SigningMethodRS256, c).SignedString(key) + Expect(err).ToNot(HaveOccurred()) + return raw + } + + baseClaims := func() customClaims { + return customClaims{ + RegisteredClaims: jwt.RegisteredClaims{ + Audience: jwt.ClaimStrings{"asdf1234"}, + ExpiresAt: jwt.NewNumericDate(expiresFuture), + IssuedAt: jwt.NewNumericDate(time.Now()), + Issuer: "https://issuer.example.com", + NotBefore: jwt.NewNumericDate(time.Time{}), + Subject: "1234567890", + }, + } + } + + It("populates Groups from the configured GroupsClaim (e.g. \"roles\")", func() { + claims := baseClaims() + claims.Roles = []string{"app-viewer", "app-admin"} + rawToken := signedToken(claims) + + session, err := buildLoader("email", "roles")(ctx, rawToken) + Expect(err).ToNot(HaveOccurred()) + Expect(session).ToNot(BeNil()) + Expect(session.Groups).To(Equal([]string{"app-viewer", "app-admin"})) + Expect(session.User).To(Equal("1234567890")) + }) + + It("populates Email from the configured EmailClaim (e.g. \"upn\")", func() { + claims := baseClaims() + claims.UPN = "alice@corp.example.com" + rawToken := signedToken(claims) + + session, err := buildLoader("upn", "groups")(ctx, rawToken) + Expect(err).ToNot(HaveOccurred()) + Expect(session).ToNot(BeNil()) + Expect(session.Email).To(Equal("alice@corp.example.com")) + Expect(session.User).To(Equal("1234567890")) + }) + + It("honors both EmailClaim and GroupsClaim simultaneously", func() { + claims := baseClaims() + claims.UPN = "alice@corp.example.com" + claims.Roles = []string{"app-viewer"} + rawToken := signedToken(claims) + + session, err := buildLoader("upn", "roles")(ctx, rawToken) + Expect(err).ToNot(HaveOccurred()) + Expect(session).ToNot(BeNil()) + Expect(session.Email).To(Equal("alice@corp.example.com")) + Expect(session.Groups).To(Equal([]string{"app-viewer"})) + }) + + It("leaves Groups empty when the GroupsClaim is absent from the token", func() { + claims := baseClaims() + claims.UPN = "alice@corp.example.com" + // no Roles set + rawToken := signedToken(claims) + + session, err := buildLoader("upn", "roles")(ctx, rawToken) + Expect(err).ToNot(HaveOccurred()) + Expect(session).ToNot(BeNil()) + Expect(session.Groups).To(BeEmpty()) + }) + }) }) diff --git a/providers/oidc.go b/providers/oidc.go index aa022f63..850934be 100644 --- a/providers/oidc.go +++ b/providers/oidc.go @@ -210,29 +210,7 @@ func (p *OIDCProvider) redeemRefreshToken(ctx context.Context, s *sessions.Sessi // CreateSessionFromToken converts Bearer IDTokens into sessions func (p *OIDCProvider) CreateSessionFromToken(ctx context.Context, token string) (*sessions.SessionState, error) { ctx = oidc.ClientContext(ctx, requests.DefaultHTTPClient) - idToken, err := p.Verifier.Verify(ctx, token) - if err != nil { - return nil, err - } - - ss, err := p.buildSessionFromClaims(token, "") - if err != nil { - return nil, err - } - - // Allow empty Email in Bearer case since we can't hit the ProfileURL - if ss.Email == "" { - ss.Email = ss.User - } - - ss.AccessToken = token - ss.IDToken = token - ss.RefreshToken = "" - - ss.CreatedAtNow() - ss.SetExpiresOn(idToken.Expiry) - - return ss, nil + return p.CreateTokenToSessionFunc(p.Verifier.Verify)(ctx, token) } // createSession takes an oauth2.Token and creates a SessionState from it. diff --git a/providers/provider_data.go b/providers/provider_data.go index 80bd77ae..bfad2398 100644 --- a/providers/provider_data.go +++ b/providers/provider_data.go @@ -11,6 +11,7 @@ import ( "strings" "github.com/coreos/go-oidc/v3/oidc" + middleware "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger" @@ -310,6 +311,55 @@ func (p *ProviderData) buildSessionFromClaims(rawIDToken, accessToken string) (* return ss, nil } +// finalizeBearerSession turns the claims of a verified bearer id_token into a +// fully populated SessionState by delegating claim extraction to +// buildSessionFromClaims and then setting the bearer-token specific fields. +// +// This is the single place that bridges "I have a verified id_token" to "I +// have a SessionState"; it is shared by every flow that turns a verified +// bearer JWT into a session (main OIDC provider, default provider, and the +// `--extra-jwt-issuers` chain), so the same configured claim mappings +// (UserClaim, EmailClaim, GroupsClaim, AdditionalClaims, email_verified...) +// are honored consistently. +func (p *ProviderData) finalizeBearerSession(idToken *oidc.IDToken, rawToken string) (*sessions.SessionState, error) { + ss, err := p.buildSessionFromClaims(rawToken, "") + if err != nil { + return nil, err + } + + // Allow empty Email in Bearer case since we can't hit the ProfileURL + if ss.Email == "" { + ss.Email = ss.User + } + + ss.AccessToken = rawToken + ss.IDToken = rawToken + ss.RefreshToken = "" + + ss.CreatedAtNow() + ss.SetExpiresOn(idToken.Expiry) + + return ss, nil +} + +// CreateTokenToSessionFunc returns a middleware.TokenToSessionFunc that +// verifies bearer JWTs with the given verifier and builds a SessionState +// using this provider's claim configuration. +// +// Callers (e.g. `--extra-jwt-issuers` in oauthproxy.go) use this to plug +// additional trusted issuers into the bearer-token chain without +// duplicating any claim-extraction logic: every issuer ends up going +// through buildSessionFromClaims, just like the main OIDC provider. +func (p *ProviderData) CreateTokenToSessionFunc(verify middleware.VerifyFunc) middleware.TokenToSessionFunc { + return func(ctx context.Context, token string) (*sessions.SessionState, error) { + idToken, err := verify(ctx, token) + if err != nil { + return nil, err + } + return p.finalizeBearerSession(idToken, token) + } +} + func (p *ProviderData) getClaimExtractor(rawIDToken, accessToken string) (util.ClaimExtractor, error) { profileURL := p.ProfileURL if p.SkipClaimsFromProfileURL { diff --git a/providers/provider_default.go b/providers/provider_default.go index dbd93c91..da4fd351 100644 --- a/providers/provider_default.go +++ b/providers/provider_default.go @@ -7,7 +7,6 @@ import ( "fmt" "net/url" - "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" ) @@ -148,7 +147,7 @@ func (p *ProviderData) RefreshSession(_ context.Context, _ *sessions.SessionStat // CreateSessionFromToken converts Bearer IDTokens into sessions func (p *ProviderData) CreateSessionFromToken(ctx context.Context, token string) (*sessions.SessionState, error) { if p.Verifier != nil { - return middleware.CreateTokenToSessionFunc(p.Verifier.Verify)(ctx, token) + return p.CreateTokenToSessionFunc(p.Verifier.Verify)(ctx, token) } return nil, ErrNotImplemented }