diff --git a/CHANGELOG.md b/CHANGELOG.md index 4542945f..53c8b5ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ We improved our supply chain security by added additional checks to prevent pote ## Changes since v7.14.2 - [#3183](https://github.com/oauth2-proxy/oauth2-proxy/pull/3183) fix: allow URL parameters to configure username, password and max idle connection timeout if the matching configuration is empty. +- [#3340](https://github.com/oauth2-proxy/oauth2-proxy/issues/3340) feat: allow injecting additional OIDC claims via `oidcConfig.additionalClaims` # V7.14.2 @@ -78,7 +79,7 @@ We improved our supply chain security by added additional checks to prevent pote ## Important Notes -This release introduces a breaking change for Alpha Config users and moves us significantly +This release introduces a breaking change for Alpha Config users and moves us significantly closer to removing legacy configuration parameters, making the codebase of OAuth2 Proxy more future proof and extensible. @@ -155,7 +156,7 @@ the project for future maintainability and future improvements like structured l - [#3292](https://github.com/oauth2-proxy/oauth2-proxy/pull/3292) chore(deps): upgrade gomod and bump to golang v1.25.5 (@tuunit) - [#3304](https://github.com/oauth2-proxy/oauth2-proxy/pull/3304) fix: added conditional so default is not always set and env vars are honored fixes 3303 (@pixeldrew) - [#3264](https://github.com/oauth2-proxy/oauth2-proxy/pull/3264) fix: more aggressively truncate logged access_token (@MartinNowak / @tuunit) -- [#3267](https://github.com/oauth2-proxy/oauth2-proxy/pull/3267) fix: Session refresh handling in OIDC provider (@gysel) +- [#3267](https://github.com/oauth2-proxy/oauth2-proxy/pull/3267) fix: Session refresh handling in OIDC provider (@gysel) - [#3290](https://github.com/oauth2-proxy/oauth2-proxy/pull/3290) fix: WebSocket proxy to respect PassHostHeader setting (@UnsignedLong) # V7.13.0 diff --git a/docs/docs/configuration/alpha_config.md b/docs/docs/configuration/alpha_config.md index b4a75582..7ffa8897 100644 --- a/docs/docs/configuration/alpha_config.md +++ b/docs/docs/configuration/alpha_config.md @@ -152,10 +152,18 @@ injectResponseHeaders: value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk # base64 encoded password ``` -**Value sources:** -* `claimSource` - `claim` (session claims either from id token or from profile URL) +**Value sources:** +* `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` @@ -279,7 +287,7 @@ ClaimSource allows loading a header value from a claim within the session | Field | Type | Description | | ----- | ---- | ----------- | -| `claim` | _string_ | Claim is the name of the claim in the session that the value should be
loaded from. Available claims: `access_token` `id_token` `created_at`
`expires_on` `refresh_token` `email` `user` `groups` `preferred_username`. | +| `claim` | _string_ | Claim is the name of the claim in the session that the value should be
loaded from. Available claims: `access_token` `id_token` `created_at`
`expires_on` `refresh_token` `email` `user` `groups` `preferred_username`,
plus any custom claims configured in `oidcConfig.additionalClaims`. | | `prefix` | _string_ | Prefix is an optional prefix that will be prepended to the value of the
claim if it is non-empty. | | `basicAuthPassword` | _[SecretSource](#secretsource)_ | BasicAuthPassword converts this claim into a basic auth header.
Note the value of claim will become the basic auth username and the
basicAuthPassword will be used as the password value. | @@ -488,6 +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 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 @@ -519,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/docs/versioned_docs/version-7.14.x/configuration/alpha_config.md b/docs/versioned_docs/version-7.14.x/configuration/alpha_config.md index b4a75582..52af0358 100644 --- a/docs/versioned_docs/version-7.14.x/configuration/alpha_config.md +++ b/docs/versioned_docs/version-7.14.x/configuration/alpha_config.md @@ -152,7 +152,7 @@ injectResponseHeaders: value: c3VwZXItc2VjcmV0LXBhc3N3b3Jk # base64 encoded password ``` -**Value sources:** +**Value sources:** * `claimSource` - `claim` (session claims either from id token or from profile URL) * `secretSource` - `value` (base64), `fromFile` (file path) @@ -279,7 +279,7 @@ ClaimSource allows loading a header value from a claim within the session | Field | Type | Description | | ----- | ---- | ----------- | -| `claim` | _string_ | Claim is the name of the claim in the session that the value should be
loaded from. Available claims: `access_token` `id_token` `created_at`
`expires_on` `refresh_token` `email` `user` `groups` `preferred_username`. | +| `claim` | _string_ | Claim is the name of the claim in the session that the value should be
loaded from. Available claims: `access_token` `id_token` `created_at`
`expires_on` `refresh_token` `email` `user` `groups` `preferred_username`,
plus any custom claims configured in `oidcConfig.additionalClaims`. | | `prefix` | _string_ | Prefix is an optional prefix that will be prepended to the value of the
claim if it is non-empty. | | `basicAuthPassword` | _[SecretSource](#secretsource)_ | BasicAuthPassword converts this claim into a basic auth header.
Note the value of claim will become the basic auth username and the
basicAuthPassword will be used as the password value. | @@ -488,6 +488,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. | ### Provider diff --git a/pkg/apis/options/header.go b/pkg/apis/options/header.go index d9509de0..4ba98ce5 100644 --- a/pkg/apis/options/header.go +++ b/pkg/apis/options/header.go @@ -50,7 +50,8 @@ type HeaderValue struct { type ClaimSource struct { // Claim is the name of the claim in the session that the value should be // loaded from. Available claims: `access_token` `id_token` `created_at` - // `expires_on` `refresh_token` `email` `user` `groups` `preferred_username`. + // `expires_on` `refresh_token` `email` `user` `groups` `preferred_username`, + // plus any custom claims configured in `oidcConfig.additionalClaims`. Claim string `yaml:"claim,omitempty"` // Prefix is an optional prefix that will be prepended to the value of the diff --git a/pkg/apis/options/providers.go b/pkg/apis/options/providers.go index 94bdb592..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,6 +320,11 @@ 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 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"` } type LoginGovOptions struct { diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go index a1f807ab..7d6673d3 100644 --- a/pkg/apis/sessions/session_state.go +++ b/pkg/apis/sessions/session_state.go @@ -27,6 +27,7 @@ type SessionState struct { User string `msgpack:"u,omitempty"` Groups []string `msgpack:"g,omitempty"` PreferredUsername string `msgpack:"pu,omitempty"` + ExtraClaims map[string][]string `msgpack:"ec,omitempty"` // Internal helpers, not serialized Clock func() time.Time `msgpack:"-"` // override for time.Now, for testing @@ -156,7 +157,14 @@ func (s *SessionState) GetClaim(claim string) []string { case "preferred_username": return []string{s.PreferredUsername} default: - return []string{} + if s.ExtraClaims == nil { + return []string{} + } + values, ok := s.ExtraClaims[claim] + if !ok { + return []string{} + } + return append([]string(nil), values...) } } diff --git a/pkg/apis/sessions/session_state_test.go b/pkg/apis/sessions/session_state_test.go index 87b97614..31804f96 100644 --- a/pkg/apis/sessions/session_state_test.go +++ b/pkg/apis/sessions/session_state_test.go @@ -155,6 +155,26 @@ func TestAge(t *testing.T) { assert.Equal(t, time.Hour, ss.Age().Round(time.Minute)) } +func TestGetClaimAdditionalClaims(t *testing.T) { + ss := &SessionState{ + ExtraClaims: map[string][]string{ + "displayName": []string{"Jane D."}, + "jobTitle": []string{"Principal Consultant"}, + }, + } + + assert.Equal(t, []string{"Jane D."}, ss.GetClaim("displayName")) + assert.Equal(t, []string{"Principal Consultant"}, ss.GetClaim("jobTitle")) + assert.Equal(t, []string{}, ss.GetClaim("missing")) + + result := ss.GetClaim("displayName") + result[0] = "Mutated" + assert.Equal(t, []string{"Jane D."}, ss.GetClaim("displayName")) + + var nilSession *SessionState + assert.Equal(t, []string{}, nilSession.GetClaim("displayName")) +} + // TestEncodeAndDecodeSessionState encodes & decodes various session states // and confirms the operation is 1:1 func TestEncodeAndDecodeSessionState(t *testing.T) { diff --git a/pkg/middleware/headers_test.go b/pkg/middleware/headers_test.go index c3c402f0..dcc96370 100644 --- a/pkg/middleware/headers_test.go +++ b/pkg/middleware/headers_test.go @@ -180,6 +180,37 @@ var _ = Describe("Headers Suite", func() { }, expectedErr: "", }), + Entry("with configured additional claims in response headers", headersTableInput{ + headers: []options.Header{ + { + Name: "X-User-Display-Name", + Values: []options.HeaderValue{{ + ClaimSource: &options.ClaimSource{Claim: "displayName"}, + }}, + }, + { + Name: "X-User-Job-Title", + Values: []options.HeaderValue{{ + ClaimSource: &options.ClaimSource{Claim: "jobTitle"}, + }}, + }, + }, + initialHeaders: http.Header{ + "Foo": []string{"bar", "baz"}, + }, + session: &sessionsapi.SessionState{ + ExtraClaims: map[string][]string{ + "displayName": []string{"Jane D."}, + "jobTitle": []string{"Principal Consultant"}, + }, + }, + expectedHeaders: http.Header{ + "Foo": []string{"bar,baz"}, + "X-User-Display-Name": []string{"Jane D."}, + "X-User-Job-Title": []string{"Principal Consultant"}, + }, + expectedErr: "", + }), Entry("with an invalid basicAuthPassword claim valued header", headersTableInput{ headers: []options.Header{ { 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 95de5c50..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. @@ -51,6 +52,7 @@ type ProviderData struct { GroupsClaim string Verifier internaloidc.IDTokenVerifier SkipClaimsFromProfileURL bool + AdditionalClaims []string // Universal Group authorization data structure // any provider can set to consume @@ -268,6 +270,23 @@ func (p *ProviderData) buildSessionFromClaims(rawIDToken, accessToken string) (* } } + if len(p.AdditionalClaims) > 0 { + for _, claim := range p.AdditionalClaims { + var values []string + exists, err := extractor.GetClaimInto(claim, &values) + if err != nil { + return nil, err + } + if !exists || len(values) == 0 { + continue + } + if ss.ExtraClaims == nil { + ss.ExtraClaims = map[string][]string{} + } + ss.ExtraClaims[claim] = append([]string(nil), values...) + } + } + // `email_verified` must be present and explicitly set to `false` to be // considered unverified. verifyEmail := (p.EmailClaim == options.OIDCEmailClaim) && !p.AllowUnverifiedEmail @@ -289,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 044a77b1..426bf856 100644 --- a/providers/provider_data_test.go +++ b/providers/provider_data_test.go @@ -95,6 +95,20 @@ var ( RegisteredClaims: registeredClaims, } + displayNameAndJobTitleIDToken = idTokenClaims{ + Name: "Jane Dobbs", + Email: "janed@me.com", + Phone: "+4798765432", + Picture: "http://mugbook.com/janed/me.jpg", + Groups: []string{"test:a", "test:b"}, + Roles: []string{"test:c", "test:d"}, + DisplayName: "Jane D.", + JobTitle: "Principal Consultant", + Verified: &verified, + Nonce: encryption.HashNonce([]byte(oidcNonce)), + RegisteredClaims: registeredClaims, + } + unverifiedIDToken = idTokenClaims{ Name: "Mystery Man", Email: "unverified@email.com", @@ -112,14 +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"` - Verified *bool `json:"email_verified,omitempty"` - Nonce string `json:"nonce,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"` jwt.RegisteredClaims } @@ -227,16 +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 - 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, @@ -405,36 +426,132 @@ func TestProviderData_buildSessionFromClaims(t *testing.T) { PreferredUsername: "Jane Dobbs", }, }, + "Extra Claims": { + IDToken: defaultIDToken, + AllowUnverified: true, + EmailClaim: "email", + GroupsClaim: "groups", + UserClaim: "sub", + AdditionalClaims: []string{"picture", "roles"}, + ExpectedSession: &sessions.SessionState{ + User: "123456789", + Email: "janed@me.com", + Groups: []string{"test:a", "test:b"}, + PreferredUsername: "Jane Dobbs", + ExtraClaims: map[string][]string{ + "picture": {"http://mugbook.com/janed/me.jpg"}, + "roles": {"test:c", "test:d"}, + }, + }, + }, + "Extra Claims (displayName and jobTitle)": { + IDToken: displayNameAndJobTitleIDToken, + AllowUnverified: true, + EmailClaim: "email", + GroupsClaim: "groups", + UserClaim: "sub", + AdditionalClaims: []string{"displayName", "jobTitle"}, + 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."}, + "jobTitle": {"Principal Consultant"}, + }, + }, + }, "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, @@ -446,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 @@ -453,6 +571,7 @@ func TestProviderData_buildSessionFromClaims(t *testing.T) { provider.EmailClaim = tc.EmailClaim provider.GroupsClaim = tc.GroupsClaim provider.SkipClaimsFromProfileURL = tc.SkipClaimsFromProfileURL + provider.AdditionalClaims = tc.AdditionalClaims rawIDToken, err := newSignedTestIDToken(tc.IDToken) g.Expect(err).ToNot(HaveOccurred()) @@ -465,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 6af51ecf..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) @@ -148,6 +157,7 @@ func newProviderDataFromConfig(providerConfig options.Provider) (*ProviderData, p.EmailClaim = providerConfig.OIDCConfig.EmailClaim p.GroupsClaim = providerConfig.OIDCConfig.GroupsClaim p.SkipClaimsFromProfileURL = ptr.Deref(providerConfig.SkipClaimsFromProfileURL, options.DefaultSkipClaimsFromProfileURL) + p.AdditionalClaims = append([]string(nil), providerConfig.OIDCConfig.AdditionalClaims...) // Set PKCE enabled or disabled based on discovery and force options p.CodeChallengeMethod = parseCodeChallengeMethod(providerConfig)