From 0d0f0262be3c52dcf7c0d3a4c2f070bad09a6eeb Mon Sep 17 00:00:00 2001 From: Paul Bourhis Date: Fri, 20 Feb 2026 14:08:12 +0100 Subject: [PATCH] feat: enhance OIDC support with additional claims and fallback profile URL handling --- docs/docs/configuration/alpha_config.md | 14 +- docs/docs/configuration/alpha_config.md.tmpl | 10 +- .../configuration/providers/ms_entra_id.md | 47 +++++++ pkg/apis/options/providers.go | 10 +- pkg/providers/util/claim_extractor.go | 62 ++++++--- pkg/providers/util/claim_extractor_test.go | 122 +++++++++++++++++- pkg/validation/providers.go | 15 +++ pkg/validation/providers_test.go | 32 +++++ pkg/validation/sessions.go | 18 ++- pkg/validation/sessions_test.go | 29 ++++- providers/provider_data.go | 25 ++-- providers/provider_data_test.go | 120 +++++++++++++---- providers/providers.go | 21 ++- 13 files changed, 438 insertions(+), 87 deletions(-) diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index 52af0358..7ffa8897 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -153,9 +153,17 @@ injectResponseHeaders: ``` **Value sources:** -* `claimSource` - `claim` (session claims either from id token or from profile URL) +* `claimSource` - `claim` (session claims from standard session fields or from `oidcConfig.additionalClaims`) * `secretSource` - `value` (base64), `fromFile` (file path) +For `oidcConfig.additionalClaims`, claim values are resolved in this order: +1. ID token +2. OIDC `userinfo_endpoint` (when available via discovery) +3. configured `profileURL` as fallback + +Only allowlisted additional claims are persisted in session `extraClaims`. +Sensitive token claim names `access_token`, `id_token`, and `refresh_token` are rejected in `additionalClaims`. + **Request option:** `preserveRequestValue: true` retains existing header values **Incompatibility:** Remove legacy flags `pass-user-headers`, `set-xauthrequest` @@ -488,7 +496,7 @@ character. | `userIDClaim` | _string_ | UserIDClaim indicates which claim contains the user ID
default set to 'email' | | `audienceClaims` | _[]string_ | AudienceClaim allows to define any claim that is verified against the client id
By default `aud` claim is used for verification. | | `extraAudiences` | _[]string_ | ExtraAudiences is a list of additional audiences that are allowed
to pass verification in addition to the client id. | -| `additionalClaims` | _[]string_ | AdditionalClaims is a list of additional claim names to pull from the ID token
or profile URL and store in the session for use with ClaimSource. | +| `additionalClaims` | _[]string_ | AdditionalClaims is an allowlist of additional claim names to pull from the ID token,
then OIDC userinfo endpoint, then fallback profileURL, and store in the session for ClaimSource use.
Sensitive token claim names (`access_token`, `id_token`, `refresh_token`) are not allowed. | ### Provider @@ -520,7 +528,7 @@ Provider holds all configuration for a single provider | `loginURLParameters` | _[[]LoginURLParameter](#loginurlparameter)_ | LoginURLParameters defines the parameters that can be passed from the start URL to the IdP login URL | | `authRequestResponseMode` | _string_ | AuthRequestResponseMode defines the response mode to request during authorization request | | `redeemURL` | _string_ | RedeemURL is the token redemption endpoint | -| `profileURL` | _string_ | ProfileURL is the profile access endpoint | +| `profileURL` | _string_ | ProfileURL is the profile access endpoint.
When OIDC discovery provides `userinfo_endpoint`, that endpoint is primary and profileURL is used as fallback. | | `skipClaimsFromProfileURL` | _bool_ | SkipClaimsFromProfileURL allows to skip request to Profile URL for resolving claims not present in id_token
default set to 'false' | | `resource` | _string_ | ProtectedResource is the resource that is protected (Azure AD and ADFS only) | | `validateURL` | _string_ | ValidateURL is the access token validation endpoint | diff --git a/docs/docs/configuration/alpha_config.md.tmpl b/docs/docs/configuration/alpha_config.md.tmpl index 081657c4..baa0c57d 100644 --- a/docs/docs/configuration/alpha_config.md.tmpl +++ b/docs/docs/configuration/alpha_config.md.tmpl @@ -153,9 +153,17 @@ injectResponseHeaders: ``` **Value sources:** -* `claimSource` - `claim` (session claims either from id token or from profile URL) +* `claimSource` - `claim` (session claims from standard session fields or from `oidcConfig.additionalClaims`) * `secretSource` - `value` (base64), `fromFile` (file path) +For `oidcConfig.additionalClaims`, claim values are resolved in this order: +1. ID token +2. OIDC `userinfo_endpoint` (when available via discovery) +3. configured `profileURL` as fallback + +Only allowlisted additional claims are persisted in session `extraClaims`. +Sensitive token claim names `access_token`, `id_token`, and `refresh_token` are rejected in `additionalClaims`. + **Request option:** `preserveRequestValue: true` retains existing header values **Incompatibility:** Remove legacy flags `pass-user-headers`, `set-xauthrequest` diff --git a/docs/docs/configuration/providers/ms_entra_id.md b/docs/docs/configuration/providers/ms_entra_id.md index b9b9e1f8..46fc46e1 100644 --- a/docs/docs/configuration/providers/ms_entra_id.md +++ b/docs/docs/configuration/providers/ms_entra_id.md @@ -104,6 +104,53 @@ For personal microsoft accounts, required scope is `openid profile email`. See: [Overview of permissions and consent in the Microsoft identity platform](https://learn.microsoft.com/en-us/entra/identity-platform/permissions-consent-overview). +### Additional claims for `auth_request` response headers + +You can expose extra identity fields via `claimSource` when they are explicitly allowlisted in `oidcConfig.additionalClaims`. + +Claim lookup order is: +1. ID token +2. OIDC `userinfo_endpoint` (from discovery) +3. configured `profileURL` fallback (for example Microsoft Graph `/me`) + +For Entra ID specifically: +- `jobTitle` is typically available from Microsoft Graph `/me`, not in the ID token. +- Display name may be `name` in ID token, while Graph returns `displayName`. + +Minimal alpha-config example (ID token `name`, Graph `/me` fallback for `displayName` and `jobTitle`): + +```yaml +providers: + - id: entra + provider: entra-id + clientID: "${OAUTH_CLIENT_ID}" + clientSecret: "${OAUTH_CLIENT_SECRET}" + oidcConfig: + issuerURL: "https://login.microsoftonline.com//v2.0" + additionalClaims: + - name + - displayName + - jobTitle + # If discovery returns userinfo_endpoint, this profileURL is used as fallback + # when claims are missing or userinfo cannot satisfy them. + profileURL: "https://graph.microsoft.com/v1.0/me?$select=displayName,jobTitle" + scope: "openid profile email User.Read" + +injectResponseHeaders: + - name: X-Auth-Request-Name + values: + - claimSource: + claim: name + - name: X-Auth-Request-DisplayName + values: + - claimSource: + claim: displayName + - name: X-Auth-Request-JobTitle + values: + - claimSource: + claim: jobTitle +``` + ### Multi-tenant apps To authenticate apps from multiple tenants (including personal Microsoft accounts), set the common OIDC issuer url and disable verification: ```toml diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go index 9afa4a2d..4c009a78 100644 --- a/pkg/apis/options/providers.go +++ b/pkg/apis/options/providers.go @@ -118,7 +118,9 @@ type Provider struct { AuthRequestResponseMode string `yaml:"authRequestResponseMode,omitempty"` // RedeemURL is the token redemption endpoint RedeemURL string `yaml:"redeemURL,omitempty"` - // ProfileURL is the profile access endpoint + // ProfileURL is the profile access endpoint. + // When OIDC discovery is enabled and userinfo_endpoint is discovered, that endpoint is used as the primary profile source. + // In that case, a configured ProfileURL is used as a fallback profile source. ProfileURL string `yaml:"profileURL,omitempty"` // SkipClaimsFromProfileURL allows to skip request to Profile URL for resolving claims not present in id_token // default set to 'false' @@ -318,8 +320,10 @@ type OIDCOptions struct { // ExtraAudiences is a list of additional audiences that are allowed // to pass verification in addition to the client id. ExtraAudiences []string `yaml:"extraAudiences,omitempty"` - // AdditionalClaims defines additional claims to pull from the ID token or - // profile URL and store in the session for claimSource usage. + // AdditionalClaims defines an allowlist of additional claim names to pull from the ID token, + // then from discovered userinfo endpoint, then from fallback profile URL (if configured). + // Only these claims are stored in the session for claimSource usage. + // Sensitive token claim names (`access_token`, `id_token`, `refresh_token`) are not allowed. AdditionalClaims []string `yaml:"additionalClaims,omitempty"` } diff --git a/pkg/providers/util/claim_extractor.go b/pkg/providers/util/claim_extractor.go index 9ab7a8c8..0322a338 100644 --- a/pkg/providers/util/claim_extractor.go +++ b/pkg/providers/util/claim_extractor.go @@ -27,8 +27,9 @@ type ClaimExtractor interface { // NewClaimExtractor constructs a new ClaimExtractor from the raw ID Token. // If needed, it will use the profile URL to look up a claim if it isn't present -// within the ID Token. -func NewClaimExtractor(ctx context.Context, idToken string, profileURL *url.URL, profileRequestHeaders http.Header) (ClaimExtractor, error) { +// within the ID Token. A fallback profile URL can also be configured for use when +// claims are not found in, or cannot be fetched from, the primary profile URL. +func NewClaimExtractor(ctx context.Context, idToken string, profileURL, fallbackProfileURL *url.URL, profileRequestHeaders http.Header) (ClaimExtractor, error) { payload, err := parseJWT(idToken) if err != nil { return nil, fmt.Errorf("failed to parse ID Token: %v", err) @@ -40,20 +41,23 @@ func NewClaimExtractor(ctx context.Context, idToken string, profileURL *url.URL, } return &claimExtractor{ - ctx: ctx, - profileURL: profileURL, - requestHeaders: profileRequestHeaders, - tokenClaims: tokenClaims, + ctx: ctx, + profileURL: profileURL, + fallbackProfileURL: fallbackProfileURL, + requestHeaders: profileRequestHeaders, + tokenClaims: tokenClaims, }, nil } // claimExtractor implements the ClaimExtractor interface type claimExtractor struct { - profileURL *url.URL - ctx context.Context - requestHeaders map[string][]string - tokenClaims *simplejson.Json - profileClaims *simplejson.Json + profileURL *url.URL + fallbackProfileURL *url.URL + ctx context.Context + requestHeaders map[string][]string + tokenClaims *simplejson.Json + profileClaims *simplejson.Json + fallbackClaims *simplejson.Json } // GetClaim will return the value claim if it exists. @@ -69,25 +73,43 @@ func (c *claimExtractor) GetClaim(claim string) (interface{}, bool, error) { } if c.profileClaims == nil { - profileClaims, err := c.loadProfileClaims() - if err != nil { - return nil, false, fmt.Errorf("failed to fetch claims from profile URL: %v", err) + profileClaims, profileErr := c.loadClaims(c.profileURL) + if profileErr == nil { + c.profileClaims = profileClaims + } else if c.fallbackProfileURL != nil && c.fallbackProfileURL.String() != "" { + c.profileClaims = simplejson.New() + } else { + return nil, false, fmt.Errorf("failed to fetch claims from profile URL: %v", profileErr) } - - c.profileClaims = profileClaims } if value := getClaimFrom(claim, c.profileClaims); value != nil { return value, true, nil } + if c.fallbackProfileURL == nil || c.fallbackProfileURL.String() == "" { + return nil, false, nil + } + + if c.fallbackClaims == nil { + fallbackClaims, err := c.loadClaims(c.fallbackProfileURL) + if err != nil { + return nil, false, fmt.Errorf("failed to fetch claims from fallback profile URL: %v", err) + } + c.fallbackClaims = fallbackClaims + } + + if value := getClaimFrom(claim, c.fallbackClaims); value != nil { + return value, true, nil + } + return nil, false, nil } -// loadProfileClaims will fetch the profileURL using the provided headers as +// loadClaims will fetch the profileURL using the provided headers as // authentication. -func (c *claimExtractor) loadProfileClaims() (*simplejson.Json, error) { - if c.profileURL == nil || c.profileURL.String() == "" || c.requestHeaders == nil { +func (c *claimExtractor) loadClaims(profileURL *url.URL) (*simplejson.Json, error) { + if profileURL == nil || profileURL.String() == "" || c.requestHeaders == nil { // When no profileURL is set, we return a non-empty map so that // we don't attempt to populate the profile claims again. // If there are no headers, the request would be unauthorized so we also skip @@ -95,7 +117,7 @@ func (c *claimExtractor) loadProfileClaims() (*simplejson.Json, error) { return simplejson.New(), nil } - builder := requests.New(c.profileURL.String()). + builder := requests.New(profileURL.String()). WithContext(c.ctx). WithHeaders(c.requestHeaders). Do() diff --git a/pkg/providers/util/claim_extractor_test.go b/pkg/providers/util/claim_extractor_test.go index 4ce4606f..330b0101 100644 --- a/pkg/providers/util/claim_extractor_test.go +++ b/pkg/providers/util/claim_extractor_test.go @@ -74,7 +74,7 @@ var _ = Describe("Claim Extractor Suite", func() { DescribeTable("NewClaimExtractor", func(in newClaimExtractorTableInput) { - _, err := NewClaimExtractor(context.Background(), in.idToken, nil, nil) + _, err := NewClaimExtractor(context.Background(), in.idToken, nil, nil, nil) if in.expectedError != nil { Expect(err).To(MatchError(in.expectedError)) } else { @@ -257,6 +257,18 @@ var _ = Describe("Claim Extractor Suite", func() { expectedValue: []interface{}{"nestedClaimContainingHypenGroup1", "nestedClaimContainingHypenGroup2"}, expectedError: nil, }), + Entry("does not support indexed JSON path claims", getClaimTableInput{ + testClaimExtractorOpts: testClaimExtractorOpts{ + idTokenPayload: basicIDTokenPayload, + setProfileURL: true, + profileRequestHeaders: newAuthorizedHeader(), + profileRequestHandler: requiresAuthProfileHandler, + }, + claim: "groups.0", + expectExists: false, + expectedValue: nil, + expectedError: nil, + }), ) }) @@ -310,6 +322,81 @@ var _ = Describe("Claim Extractor Suite", func() { Expect(value).To(BeNil()) }) + It("GetClaim should use fallback profile URL when primary profile URL request fails", func() { + primaryRequestHandler := func(rw http.ResponseWriter, _ *http.Request) { + rw.WriteHeader(500) + rw.Write([]byte("boom")) + } + + fallbackRequestHandler := func(rw http.ResponseWriter, req *http.Request) { + if !hasAuthorizedHeader(req.Header) { + rw.WriteHeader(403) + rw.Write([]byte("Unauthorized")) + return + } + + rw.Write([]byte(`{"displayName":"Graph Jane"}`)) + } + + claimExtractor, serverClose, err := newTestClaimExtractor(testClaimExtractorOpts{ + idTokenPayload: "{}", + setProfileURL: true, + profileRequestHeaders: newAuthorizedHeader(), + profileRequestHandler: primaryRequestHandler, + setFallbackProfileURL: true, + fallbackRequestHandler: fallbackRequestHandler, + }) + Expect(err).ToNot(HaveOccurred()) + if serverClose != nil { + defer serverClose() + } + + value, exists, err := claimExtractor.GetClaim("displayName") + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeTrue()) + Expect(value).To(Equal("Graph Jane")) + }) + + It("GetClaim should keep primary profile URL precedence over fallback", func() { + primaryRequestHandler := func(rw http.ResponseWriter, req *http.Request) { + if !hasAuthorizedHeader(req.Header) { + rw.WriteHeader(403) + rw.Write([]byte("Unauthorized")) + return + } + + rw.Write([]byte(`{"displayName":"UserInfo Jane"}`)) + } + + fallbackRequestHandler := func(rw http.ResponseWriter, req *http.Request) { + if !hasAuthorizedHeader(req.Header) { + rw.WriteHeader(403) + rw.Write([]byte("Unauthorized")) + return + } + + rw.Write([]byte(`{"displayName":"Graph Jane"}`)) + } + + claimExtractor, serverClose, err := newTestClaimExtractor(testClaimExtractorOpts{ + idTokenPayload: "{}", + setProfileURL: true, + profileRequestHeaders: newAuthorizedHeader(), + profileRequestHandler: primaryRequestHandler, + setFallbackProfileURL: true, + fallbackRequestHandler: fallbackRequestHandler, + }) + Expect(err).ToNot(HaveOccurred()) + if serverClose != nil { + defer serverClose() + } + + value, exists, err := claimExtractor.GetClaim("displayName") + Expect(err).ToNot(HaveOccurred()) + Expect(exists).To(BeTrue()) + Expect(value).To(Equal("UserInfo Jane")) + }) + type getClaimIntoTableInput struct { testClaimExtractorOpts into interface{} @@ -552,27 +639,48 @@ var _ = Describe("Claim Extractor Suite", func() { // ****************************************** type testClaimExtractorOpts struct { - idTokenPayload string - setProfileURL bool - profileRequestHeaders http.Header - profileRequestHandler http.HandlerFunc + idTokenPayload string + setProfileURL bool + profileRequestHeaders http.Header + profileRequestHandler http.HandlerFunc + setFallbackProfileURL bool + fallbackRequestHandler http.HandlerFunc } func newTestClaimExtractor(in testClaimExtractorOpts) (ClaimExtractor, func(), error) { var profileURL *url.URL + var fallbackProfileURL *url.URL var closeServer func() + cleanup := []func(){} if in.setProfileURL { server := httptest.NewServer(http.HandlerFunc(in.profileRequestHandler)) - closeServer = server.Close + cleanup = append(cleanup, server.Close) var err error profileURL, err = url.Parse("http://" + server.Listener.Addr().String() + profilePath) Expect(err).ToNot(HaveOccurred()) } + if in.setFallbackProfileURL { + server := httptest.NewServer(http.HandlerFunc(in.fallbackRequestHandler)) + cleanup = append(cleanup, server.Close) + + var err error + fallbackProfileURL, err = url.Parse("http://" + server.Listener.Addr().String() + profilePath) + Expect(err).ToNot(HaveOccurred()) + } + + if len(cleanup) > 0 { + closeServer = func() { + for _, c := range cleanup { + c() + } + } + } + rawIDToken := createJWTFromPayload(in.idTokenPayload) - claimExtractor, err := NewClaimExtractor(context.Background(), rawIDToken, profileURL, in.profileRequestHeaders) + claimExtractor, err := NewClaimExtractor(context.Background(), rawIDToken, profileURL, fallbackProfileURL, in.profileRequestHeaders) return claimExtractor, closeServer, err } diff --git a/pkg/validation/providers.go b/pkg/validation/providers.go index 9e62e98a..29243cca 100644 --- a/pkg/validation/providers.go +++ b/pkg/validation/providers.go @@ -59,6 +59,21 @@ func validateProvider(provider options.Provider, providerIDs map[string]struct{} msgs = append(msgs, validateEntraConfig(provider)...) } + msgs = append(msgs, validateAdditionalClaims(provider)...) + + return msgs +} + +func validateAdditionalClaims(provider options.Provider) []string { + msgs := []string{} + + for _, claim := range provider.OIDCConfig.AdditionalClaims { + if isSensitiveTokenClaim(claim) { + msgs = append(msgs, + fmt.Sprintf("provider %q has invalid oidcConfig.additionalClaims entry %q: sensitive token claims are not allowed", provider.ID, claim)) + } + } + return msgs } diff --git a/pkg/validation/providers_test.go b/pkg/validation/providers_test.go index 065eb305..88eb9b2f 100644 --- a/pkg/validation/providers_test.go +++ b/pkg/validation/providers_test.go @@ -34,6 +34,8 @@ var _ = Describe("Providers", func() { emptyIDMsg := "provider has empty id: ids are required for all providers" duplicateProviderIDMsg := "multiple providers found with id ProviderID: provider ids must be unique" skipButtonAndMultipleProvidersMsg := "SkipProviderButton and multiple providers are mutually exclusive" + invalidAdditionalClaimAccessToken := "provider \"ProviderID\" has invalid oidcConfig.additionalClaims entry \"access_token\": sensitive token claims are not allowed" + invalidAdditionalClaimRefreshToken := "provider \"ProviderID\" has invalid oidcConfig.additionalClaims entry \"refresh_token\": sensitive token claims are not allowed" DescribeTable("validateProviders", func(o *validateProvidersTableInput) { @@ -79,5 +81,35 @@ var _ = Describe("Providers", func() { }, errStrings: []string{skipButtonAndMultipleProvidersMsg}, }), + Entry("with sensitive additional claim access_token", &validateProvidersTableInput{ + options: &options.Options{ + Providers: options.Providers{ + { + ID: "ProviderID", + ClientID: "ClientID", + ClientSecret: "ClientSecret", + OIDCConfig: options.OIDCOptions{ + AdditionalClaims: []string{"access_token"}, + }, + }, + }, + }, + errStrings: []string{invalidAdditionalClaimAccessToken}, + }), + Entry("with sensitive additional claim refresh_token", &validateProvidersTableInput{ + options: &options.Options{ + Providers: options.Providers{ + { + ID: "ProviderID", + ClientID: "ClientID", + ClientSecret: "ClientSecret", + OIDCConfig: options.OIDCOptions{ + AdditionalClaims: []string{"refresh_token"}, + }, + }, + }, + }, + errStrings: []string{invalidAdditionalClaimRefreshToken}, + }), ) }) diff --git a/pkg/validation/sessions.go b/pkg/validation/sessions.go index 96ea6d4f..6a6569fa 100644 --- a/pkg/validation/sessions.go +++ b/pkg/validation/sessions.go @@ -4,6 +4,7 @@ import ( "context" "encoding/base64" "fmt" + "strings" "time" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" @@ -20,13 +21,9 @@ func validateSessionCookieMinimal(o *options.Options) []string { for _, header := range append(o.InjectRequestHeaders, o.InjectResponseHeaders...) { for _, value := range header.Values { if value.ClaimSource != nil { - if value.ClaimSource.Claim == "access_token" { + if isSensitiveTokenClaim(value.ClaimSource.Claim) { msgs = append(msgs, - fmt.Sprintf("access_token claim for header %q requires oauth tokens in sessions. session_cookie_minimal cannot be set", header.Name)) - } - if value.ClaimSource.Claim == "id_token" { - msgs = append(msgs, - fmt.Sprintf("id_token claim for header %q requires oauth tokens in sessions. session_cookie_minimal cannot be set", header.Name)) + fmt.Sprintf("%s claim for header %q requires oauth tokens in sessions. session_cookie_minimal cannot be set", value.ClaimSource.Claim, header.Name)) } } } @@ -39,6 +36,15 @@ func validateSessionCookieMinimal(o *options.Options) []string { return msgs } +func isSensitiveTokenClaim(claim string) bool { + switch strings.TrimSpace(claim) { + case "access_token", "id_token", "refresh_token": + return true + default: + return false + } +} + // validateRedisSessionStore builds a Redis Client from the options and // attempts to connect, Set, Get and Del a random health check key func validateRedisSessionStore(o *options.Options) []string { diff --git a/pkg/validation/sessions_test.go b/pkg/validation/sessions_test.go index cb54c571..03bf7fe5 100644 --- a/pkg/validation/sessions_test.go +++ b/pkg/validation/sessions_test.go @@ -12,9 +12,10 @@ import ( var _ = Describe("Sessions", func() { const ( - idTokenConflictMsg = "id_token claim for header \"X-ID-Token\" requires oauth tokens in sessions. session_cookie_minimal cannot be set" - accessTokenConflictMsg = "access_token claim for header \"X-Access-Token\" requires oauth tokens in sessions. session_cookie_minimal cannot be set" - cookieRefreshMsg = "cookie_refresh > 0 requires oauth tokens in sessions. session_cookie_minimal cannot be set" + idTokenConflictMsg = "id_token claim for header \"X-ID-Token\" requires oauth tokens in sessions. session_cookie_minimal cannot be set" + accessTokenConflictMsg = "access_token claim for header \"X-Access-Token\" requires oauth tokens in sessions. session_cookie_minimal cannot be set" + refreshTokenConflictMsg = "refresh_token claim for header \"X-Refresh-Token\" requires oauth tokens in sessions. session_cookie_minimal cannot be set" + cookieRefreshMsg = "cookie_refresh > 0 requires oauth tokens in sessions. session_cookie_minimal cannot be set" ) type cookieMinimalTableInput struct { @@ -134,6 +135,28 @@ var _ = Describe("Sessions", func() { }, errStrings: []string{accessTokenConflictMsg}, }), + Entry("Request Header refresh_token conflict", &cookieMinimalTableInput{ + opts: &options.Options{ + Session: options.SessionOptions{ + Cookie: options.CookieStoreOptions{ + Minimal: true, + }, + }, + InjectRequestHeaders: []options.Header{ + { + Name: "X-Refresh-Token", + Values: []options.HeaderValue{ + { + ClaimSource: &options.ClaimSource{ + Claim: "refresh_token", + }, + }, + }, + }, + }, + }, + errStrings: []string{refreshTokenConflictMsg}, + }), Entry("CookieRefresh conflict", &cookieMinimalTableInput{ opts: &options.Options{ Cookie: options.Cookie{ diff --git a/providers/provider_data.go b/providers/provider_data.go index a9cbb408..354c6c77 100644 --- a/providers/provider_data.go +++ b/providers/provider_data.go @@ -27,16 +27,17 @@ const ( // ProviderData contains information required to configure all implementations // of OAuth2 providers type ProviderData struct { - ProviderName string - LoginURL *url.URL - RedeemURL *url.URL - ProfileURL *url.URL - ProtectedResource *url.URL - ValidateURL *url.URL - ClientID string - ClientSecret string - ClientSecretFile string - Scope string + ProviderName string + LoginURL *url.URL + RedeemURL *url.URL + ProfileURL *url.URL + ProfileURLFallback *url.URL + ProtectedResource *url.URL + ValidateURL *url.URL + ClientID string + ClientSecret string + ClientSecretFile string + Scope string // The response mode requested from the provider or empty for default ("query") AuthRequestResponseMode string // The picked CodeChallenge Method or empty if none. @@ -307,11 +308,13 @@ func (p *ProviderData) buildSessionFromClaims(rawIDToken, accessToken string) (* func (p *ProviderData) getClaimExtractor(rawIDToken, accessToken string) (util.ClaimExtractor, error) { profileURL := p.ProfileURL + profileURLFallback := p.ProfileURLFallback if p.SkipClaimsFromProfileURL { profileURL = &url.URL{} + profileURLFallback = &url.URL{} } - extractor, err := util.NewClaimExtractor(context.TODO(), rawIDToken, profileURL, p.getAuthorizationHeader(accessToken)) + extractor, err := util.NewClaimExtractor(context.TODO(), rawIDToken, profileURL, profileURLFallback, p.getAuthorizationHeader(accessToken)) if err != nil { return nil, fmt.Errorf("could not initialise claim extractor: %v", err) } diff --git a/providers/provider_data_test.go b/providers/provider_data_test.go index 9873ecdc..426bf856 100644 --- a/providers/provider_data_test.go +++ b/providers/provider_data_test.go @@ -126,16 +126,16 @@ var ( ) type idTokenClaims struct { - Name string `json:"preferred_username,omitempty"` - Email string `json:"email,omitempty"` - Phone string `json:"phone_number,omitempty"` - Picture string `json:"picture,omitempty"` - Groups interface{} `json:"groups,omitempty"` - Roles interface{} `json:"roles,omitempty"` + Name string `json:"preferred_username,omitempty"` + Email string `json:"email,omitempty"` + Phone string `json:"phone_number,omitempty"` + Picture string `json:"picture,omitempty"` + Groups interface{} `json:"groups,omitempty"` + Roles interface{} `json:"roles,omitempty"` DisplayName string `json:"displayName,omitempty"` JobTitle string `json:"jobTitle,omitempty"` - Verified *bool `json:"email_verified,omitempty"` - Nonce string `json:"nonce,omitempty"` + Verified *bool `json:"email_verified,omitempty"` + Nonce string `json:"nonce,omitempty"` jwt.RegisteredClaims } @@ -243,17 +243,21 @@ func TestProviderData_verifyIDToken(t *testing.T) { func TestProviderData_buildSessionFromClaims(t *testing.T) { testCases := map[string]struct { - IDToken idTokenClaims - AllowUnverified bool - UserClaim string - EmailClaim string - GroupsClaim string - SkipClaimsFromProfileURL bool - SetProfileURL bool - AdditionalClaims []string - ExpectedError error - ExpectedSession *sessions.SessionState - ExpectProfileURLCalled bool + IDToken idTokenClaims + AllowUnverified bool + UserClaim string + EmailClaim string + GroupsClaim string + SkipClaimsFromProfileURL bool + SetProfileURL bool + SetProfileURLFallback bool + ProfileURLPayload string + ProfileURLFallbackPayload string + AdditionalClaims []string + ExpectedError error + ExpectedSession *sessions.SessionState + ExpectProfileURLCalled bool + ExpectProfileURLFallbackCalled bool }{ "Standard": { IDToken: defaultIDToken, @@ -423,11 +427,11 @@ func TestProviderData_buildSessionFromClaims(t *testing.T) { }, }, "Extra Claims": { - IDToken: defaultIDToken, - AllowUnverified: true, - EmailClaim: "email", - GroupsClaim: "groups", - UserClaim: "sub", + IDToken: defaultIDToken, + AllowUnverified: true, + EmailClaim: "email", + GroupsClaim: "groups", + UserClaim: "sub", AdditionalClaims: []string{"picture", "roles"}, ExpectedSession: &sessions.SessionState{ User: "123456789", @@ -461,33 +465,93 @@ func TestProviderData_buildSessionFromClaims(t *testing.T) { "Request claims from ProfileURL": { IDToken: minimalIDToken, SetProfileURL: true, + ProfileURLPayload: "{}", ExpectProfileURLCalled: true, ExpectedSession: &sessions.SessionState{}, }, "Skip claims request to ProfileURL": { IDToken: minimalIDToken, SetProfileURL: true, + ProfileURLPayload: "{}", SkipClaimsFromProfileURL: true, ExpectedSession: &sessions.SessionState{}, }, + "Extra Claims from fallback ProfileURL": { + IDToken: minimalIDToken, + SetProfileURL: true, + ProfileURLPayload: "{}", + SetProfileURLFallback: true, + ProfileURLFallbackPayload: `{"displayName":"Graph Jane","jobTitle":"Principal Consultant"}`, + AdditionalClaims: []string{"displayName", "jobTitle"}, + ExpectedSession: &sessions.SessionState{ + ExtraClaims: map[string][]string{ + "displayName": {"Graph Jane"}, + "jobTitle": {"Principal Consultant"}, + }, + }, + ExpectProfileURLCalled: true, + ExpectProfileURLFallbackCalled: true, + }, + "Extra Claims prefer ID token over ProfileURL and fallback": { + IDToken: displayNameAndJobTitleIDToken, + AllowUnverified: true, + EmailClaim: "email", + GroupsClaim: "groups", + UserClaim: "sub", + SetProfileURL: true, + ProfileURLPayload: `{"displayName":"UserInfo Jane"}`, + SetProfileURLFallback: true, + ProfileURLFallbackPayload: `{"displayName":"Graph Jane"}`, + AdditionalClaims: []string{"displayName"}, + ExpectedSession: &sessions.SessionState{ + User: "123456789", + Email: "janed@me.com", + Groups: []string{"test:a", "test:b"}, + PreferredUsername: "Jane Dobbs", + ExtraClaims: map[string][]string{ + "displayName": {"Jane D."}, + }, + }, + ExpectProfileURLCalled: false, + ExpectProfileURLFallbackCalled: false, + }, } for testName, tc := range testCases { t.Run(testName, func(t *testing.T) { g := NewWithT(t) var ( - profileURL *url.URL - profileURLCalled bool + profileURL *url.URL + profileURLFallback *url.URL + profileURLCalled bool + profileURLFallbackCalled bool ) if tc.SetProfileURL { profileURLSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { profileURLCalled = true - w.Write([]byte("{}")) + payload := tc.ProfileURLPayload + if payload == "" { + payload = "{}" + } + w.Write([]byte(payload)) })) defer profileURLSrv.Close() profileURL, _ = url.Parse(profileURLSrv.URL) } + if tc.SetProfileURLFallback { + profileURLFallbackSrv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + profileURLFallbackCalled = true + payload := tc.ProfileURLFallbackPayload + if payload == "" { + payload = "{}" + } + w.Write([]byte(payload)) + })) + defer profileURLFallbackSrv.Close() + profileURLFallback, _ = url.Parse(profileURLFallbackSrv.URL) + } + verificationOptions := internaloidc.IDTokenVerificationOptions{ AudienceClaims: []string{"aud"}, ClientID: oidcClientID, @@ -499,6 +563,7 @@ func TestProviderData_buildSessionFromClaims(t *testing.T) { &oidc.Config{ClientID: oidcClientID}, ), verificationOptions), ProfileURL: profileURL, + ProfileURLFallback: profileURLFallback, getAuthorizationHeaderFunc: func(s string) http.Header { return http.Header{} }, } provider.AllowUnverifiedEmail = tc.AllowUnverified @@ -519,6 +584,7 @@ func TestProviderData_buildSessionFromClaims(t *testing.T) { g.Expect(ss).To(Equal(tc.ExpectedSession)) } g.Expect(profileURLCalled).To(Equal(tc.ExpectProfileURLCalled)) + g.Expect(profileURLFallbackCalled).To(Equal(tc.ExpectProfileURLFallbackCalled)) }) } } diff --git a/providers/providers.go b/providers/providers.go index 69409de8..aa504275 100644 --- a/providers/providers.go +++ b/providers/providers.go @@ -86,6 +86,9 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData, AuthRequestResponseMode: providerConfig.AuthRequestResponseMode, } + configuredProfileURL := providerConfig.ProfileURL + profileURLFallback := "" + needsVerifier, err := providerRequiresOIDCProviderVerifier(providerConfig.Type) if err != nil { return nil, err @@ -113,7 +116,12 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData, pkce := pv.Provider().PKCE() providerConfig.LoginURL = endpoints.AuthURL providerConfig.RedeemURL = endpoints.TokenURL - providerConfig.ProfileURL = endpoints.UserInfoURL + if endpoints.UserInfoURL != "" { + providerConfig.ProfileURL = endpoints.UserInfoURL + if configuredProfileURL != "" && configuredProfileURL != endpoints.UserInfoURL { + profileURLFallback = configuredProfileURL + } + } providerConfig.OIDCConfig.JwksURL = endpoints.JWKsURL p.SupportedCodeChallengeMethods = pkce.CodeChallengeAlgs } @@ -124,11 +132,12 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData, dst **url.URL raw string }{ - "login": {dst: &p.LoginURL, raw: providerConfig.LoginURL}, - "redeem": {dst: &p.RedeemURL, raw: providerConfig.RedeemURL}, - "profile": {dst: &p.ProfileURL, raw: providerConfig.ProfileURL}, - "validate": {dst: &p.ValidateURL, raw: providerConfig.ValidateURL}, - "resource": {dst: &p.ProtectedResource, raw: providerConfig.ProtectedResource}, + "login": {dst: &p.LoginURL, raw: providerConfig.LoginURL}, + "redeem": {dst: &p.RedeemURL, raw: providerConfig.RedeemURL}, + "profile": {dst: &p.ProfileURL, raw: providerConfig.ProfileURL}, + "profile fallback": {dst: &p.ProfileURLFallback, raw: profileURLFallback}, + "validate": {dst: &p.ValidateURL, raw: providerConfig.ValidateURL}, + "resource": {dst: &p.ProtectedResource, raw: providerConfig.ProtectedResource}, } { var err error *u.dst, err = url.Parse(u.raw)