From 18f49e015fa4f4cbcb28fea36cd563b9d9a4577d Mon Sep 17 00:00:00 2001 From: Egor Mikhailov Date: Tue, 9 Jan 2024 21:41:46 +0300 Subject: [PATCH] auth: add new optional `OIDCDisableUserInfo` setting for OIDC auth provider (#19566) Add new optional `OIDCDisableUserInfo` setting for OIDC auth provider which disables a request to the identity provider to get OIDC UserInfo. This option is helpful when your identity provider doesn't send any additional claims from the UserInfo endpoint, such as Microsoft AD FS OIDC Provider: > The AD FS UserInfo endpoint always returns the subject claim as specified in the > OpenID standards. AD FS doesn't support additional claims requested via the > UserInfo endpoint Fixes #19318 --- .changelog/19566.txt | 3 +++ api/acl.go | 2 ++ command/acl_auth_method.go | 1 + internal/testing/apitests/acl_test.go | 2 ++ lib/auth/oidc/provider_test.go | 1 + nomad/acl_endpoint.go | 8 +++++--- nomad/mock/acl.go | 1 + nomad/structs/acl.go | 4 ++++ nomad/structs/acl_test.go | 5 +++++ website/content/api-docs/acl/auth-methods.mdx | 10 ++++++++++ 10 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 .changelog/19566.txt diff --git a/.changelog/19566.txt b/.changelog/19566.txt new file mode 100644 index 000000000..788d90514 --- /dev/null +++ b/.changelog/19566.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth: Added new optional OIDCDisableUserInfo setting for OIDC auth provider +``` diff --git a/api/acl.go b/api/acl.go index c393505fa..64d1a22af 100644 --- a/api/acl.go +++ b/api/acl.go @@ -826,6 +826,8 @@ type ACLAuthMethodConfig struct { OIDCClientID string // The OAuth Client Secret configured with the OIDC provider OIDCClientSecret string + // Disable claims from the OIDC UserInfo endpoint + OIDCDisableUserInfo bool // List of OIDC scopes OIDCScopes []string // List of auth claims that are valid for login diff --git a/command/acl_auth_method.go b/command/acl_auth_method.go index 0178974a5..50ca27aea 100644 --- a/command/acl_auth_method.go +++ b/command/acl_auth_method.go @@ -93,6 +93,7 @@ func formatAuthMethodConfig(config *api.ACLAuthMethodConfig) string { fmt.Sprintf("OIDC Discovery URL|%s", config.OIDCDiscoveryURL), fmt.Sprintf("OIDC Client ID|%s", config.OIDCClientID), fmt.Sprintf("OIDC Client Secret|%s", config.OIDCClientSecret), + fmt.Sprintf("OIDC Disable UserInfo|%t", config.OIDCDisableUserInfo), fmt.Sprintf("OIDC Scopes|%s", strings.Join(config.OIDCScopes, ",")), fmt.Sprintf("Bound audiences|%s", strings.Join(config.BoundAudiences, ",")), fmt.Sprintf("Bound issuer|%s", strings.Join(config.BoundIssuer, ",")), diff --git a/internal/testing/apitests/acl_test.go b/internal/testing/apitests/acl_test.go index 36889e612..b4969e3cf 100644 --- a/internal/testing/apitests/acl_test.go +++ b/internal/testing/apitests/acl_test.go @@ -37,6 +37,7 @@ func TestACLOIDC_GetAuthURL(t *testing.T) { OIDCDiscoveryURL: oidcTestProvider.Addr(), OIDCClientID: "mock", OIDCClientSecret: "verysecretsecret", + OIDCDisableUserInfo: false, BoundAudiences: []string{"mock"}, AllowedRedirectURIs: []string{"http://127.0.0.1:4649/oidc/callback"}, DiscoveryCaPem: []string{oidcTestProvider.CACert()}, @@ -96,6 +97,7 @@ func TestACLOIDC_CompleteAuth(t *testing.T) { OIDCDiscoveryURL: oidcTestProvider.Addr(), OIDCClientID: "mock", OIDCClientSecret: "verysecretsecret", + OIDCDisableUserInfo: false, BoundAudiences: []string{"mock"}, AllowedRedirectURIs: []string{"http://127.0.0.1:4649/oidc/callback"}, DiscoveryCaPem: []string{oidcTestProvider.CACert()}, diff --git a/lib/auth/oidc/provider_test.go b/lib/auth/oidc/provider_test.go index fa89222d2..3c1418a33 100644 --- a/lib/auth/oidc/provider_test.go +++ b/lib/auth/oidc/provider_test.go @@ -35,6 +35,7 @@ func TestProviderCache(t *testing.T) { OIDCDiscoveryURL: oidcTestProvider.Addr(), OIDCClientID: "alice", OIDCClientSecret: "ssshhhh", + OIDCDisableUserInfo: false, AllowedRedirectURIs: []string{"http://example.com"}, DiscoveryCaPem: []string{oidcTestProvider.CACert()}, SigningAlgs: []string{string(tpAlg)}, diff --git a/nomad/acl_endpoint.go b/nomad/acl_endpoint.go index 63e7e0781..302ccd918 100644 --- a/nomad/acl_endpoint.go +++ b/nomad/acl_endpoint.go @@ -2747,9 +2747,11 @@ func (a *ACL) OIDCCompleteAuth( } var userClaims map[string]interface{} - if userTokenSource := oidcToken.StaticTokenSource(); userTokenSource != nil { - if err := oidcProvider.UserInfo(ctx, userTokenSource, idTokenClaims["sub"].(string), &userClaims); err != nil { - return fmt.Errorf("failed to retrieve the user info claims: %v", err) + if !authMethod.Config.OIDCDisableUserInfo { + if userTokenSource := oidcToken.StaticTokenSource(); userTokenSource != nil { + if err := oidcProvider.UserInfo(ctx, userTokenSource, idTokenClaims["sub"].(string), &userClaims); err != nil { + return fmt.Errorf("failed to retrieve the user info claims: %v", err) + } } } diff --git a/nomad/mock/acl.go b/nomad/mock/acl.go index e86701fbd..dde6174b6 100644 --- a/nomad/mock/acl.go +++ b/nomad/mock/acl.go @@ -275,6 +275,7 @@ func ACLOIDCAuthMethod() *structs.ACLAuthMethod { OIDCDiscoveryURL: "http://example.com", OIDCClientID: "mock", OIDCClientSecret: "very secret secret", + OIDCDisableUserInfo: false, OIDCScopes: []string{"groups"}, BoundAudiences: []string{"sales", "engineering"}, AllowedRedirectURIs: []string{"foo", "bar"}, diff --git a/nomad/structs/acl.go b/nomad/structs/acl.go index bd96a3dbe..d1cedf696 100644 --- a/nomad/structs/acl.go +++ b/nomad/structs/acl.go @@ -784,6 +784,7 @@ func (a *ACLAuthMethod) SetHash() []byte { _, _ = hash.Write([]byte(a.Config.OIDCDiscoveryURL)) _, _ = hash.Write([]byte(a.Config.OIDCClientID)) _, _ = hash.Write([]byte(a.Config.OIDCClientSecret)) + _, _ = hash.Write([]byte(strconv.FormatBool(a.Config.OIDCDisableUserInfo))) _, _ = hash.Write([]byte(a.Config.ExpirationLeeway.String())) _, _ = hash.Write([]byte(a.Config.NotBeforeLeeway.String())) _, _ = hash.Write([]byte(a.Config.ClockSkewLeeway.String())) @@ -990,6 +991,9 @@ type ACLAuthMethodConfig struct { // The OAuth Client Secret configured with the OIDC provider OIDCClientSecret string + // Disable claims from the OIDC UserInfo endpoint + OIDCDisableUserInfo bool + // List of OIDC scopes OIDCScopes []string diff --git a/nomad/structs/acl_test.go b/nomad/structs/acl_test.go index 0f0fb81d3..8c1abbd40 100644 --- a/nomad/structs/acl_test.go +++ b/nomad/structs/acl_test.go @@ -1098,6 +1098,7 @@ func TestACLAuthMethod_Stub(t *testing.T) { OIDCDiscoveryURL: "http://example.com", OIDCClientID: "mock", OIDCClientSecret: "very secret secret", + OIDCDisableUserInfo: false, BoundAudiences: []string{"audience1", "audience2"}, AllowedRedirectURIs: []string{"foo", "bar"}, DiscoveryCaPem: []string{"foo"}, @@ -1138,6 +1139,7 @@ func TestACLAuthMethod_Equal(t *testing.T) { OIDCDiscoveryURL: "http://example.com", OIDCClientID: "mock", OIDCClientSecret: "very secret secret", + OIDCDisableUserInfo: false, BoundAudiences: []string{"audience1", "audience2"}, AllowedRedirectURIs: []string{"foo", "bar"}, DiscoveryCaPem: []string{"foo"}, @@ -1190,6 +1192,7 @@ func TestACLAuthMethod_Copy(t *testing.T) { OIDCDiscoveryURL: "http://example.com", OIDCClientID: "mock", OIDCClientSecret: "very secret secret", + OIDCDisableUserInfo: false, BoundAudiences: []string{"audience1", "audience2"}, AllowedRedirectURIs: []string{"foo", "bar"}, DiscoveryCaPem: []string{"foo"}, @@ -1282,6 +1285,7 @@ func TestACLAuthMethod_Merge(t *testing.T) { OIDCDiscoveryURL: "http://example.com", OIDCClientID: "mock", OIDCClientSecret: "very secret secret", + OIDCDisableUserInfo: false, BoundAudiences: []string{"audience1", "audience2"}, AllowedRedirectURIs: []string{"foo", "bar"}, DiscoveryCaPem: []string{"foo"}, @@ -1309,6 +1313,7 @@ func TestACLAuthMethodConfig_Copy(t *testing.T) { OIDCDiscoveryURL: "http://example.com", OIDCClientID: "mock", OIDCClientSecret: "very secret secret", + OIDCDisableUserInfo: false, OIDCScopes: []string{"groups"}, BoundAudiences: []string{"audience1", "audience2"}, AllowedRedirectURIs: []string{"foo", "bar"}, diff --git a/website/content/api-docs/acl/auth-methods.mdx b/website/content/api-docs/acl/auth-methods.mdx index e81c3652c..df67044cf 100644 --- a/website/content/api-docs/acl/auth-methods.mdx +++ b/website/content/api-docs/acl/auth-methods.mdx @@ -63,6 +63,11 @@ The table below shows this endpoint's support for - `OIDCClientSecret` `(string: )` - The OAuth client secret configured with your OIDC provider. + - `OIDCDisableUserInfo` `(bool: false)` - When set to `true`, Nomad will not make + a request to the identity provider to get OIDC UserInfo. You may wish to set this + if your identity provider doesn't send any additional claims from the UserInfo + endpoint. + - `OIDCScopes` `(array)` - List of OIDC scopes. - `BoundAudiences` `(array)` - List of aud claims that are valid for @@ -228,6 +233,11 @@ queries](/nomad/api-docs#blocking-queries) and [required ACLs](/nomad/api-docs#a - `OIDCClientSecret` `(string: "")` - The OAuth client secret configured with your OIDC provider. + - `OIDCDisableUserInfo` `(bool: false)` - When set to `true`, Nomad will not make + a request to the identity provider to get OIDC UserInfo. You may wish to set this + if your identity provider doesn't send any additional claims from the UserInfo + endpoint. + - `OIDCScopes` `(array)` - List of OIDC scopes. - `BoundAudiences` `(array)` - List of aud claims that are valid for