From 21ab30d20767283b4696c992644432a38fc23849 Mon Sep 17 00:00:00 2001 From: Thomas Jackson Date: Thu, 12 Sep 2019 16:12:29 -0700 Subject: [PATCH] Add option to enable groups for oidc connectors There's been some discussion in #1065 regarding what to do about refreshing groups. As it stands today dex doesn't update any of the claims on refresh (groups would just be another one). The main concern with enabling it is that group claims may change more frequently. While we continue to wait on the upstream refresh flows, this adds an option to enable the group claim. This is disabled by default (so no behavioral change) but enables those that are willing to have the delay in group claim change to use oidc IDPs. Workaround to #1065 --- Documentation/connectors/oidc.md | 7 +++++++ connector/oidc/oidc.go | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/Documentation/connectors/oidc.md b/Documentation/connectors/oidc.md index 55b7a96e..20eedd96 100644 --- a/Documentation/connectors/oidc.md +++ b/Documentation/connectors/oidc.md @@ -61,6 +61,13 @@ connectors: # This can be overridden with the below option # insecureSkipEmailVerified: true + # Groups claims (like the rest of oidc claims through dex) only refresh when the id token is refreshed + # meaning the regular refresh flow doesn't update the groups claim. As such by default the oidc connector + # doesn't allow groups claims. If you are okay with having potentially stale group claims you can use + # this option to enable groups claims through the oidc connector on a per-connector basis. + # This can be overridden with the below option + # insecureEnableGroups: true + # When enabled, the OpenID Connector will query the UserInfo endpoint for additional claims. UserInfo claims # take priority over claims returned by the IDToken. This option should be used when the IDToken doesn't contain # all the claims requested. diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 4a64df8b..f638aa6e 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -40,6 +40,9 @@ type Config struct { // Override the value of email_verifed to true in the returned claims InsecureSkipEmailVerified bool `json:"insecureSkipEmailVerified"` + // InsecureEnableGroups enables groups claims. This is disabled by default until https://github.com/dexidp/dex/issues/1065 is resolved + InsecureEnableGroups bool `json:"insecureEnableGroups"` + // GetUserInfo uses the userinfo endpoint to get additional claims for // the token. This is especially useful where upstreams return "thin" // id tokens @@ -132,6 +135,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e cancel: cancel, hostedDomains: c.HostedDomains, insecureSkipEmailVerified: c.InsecureSkipEmailVerified, + insecureEnableGroups: c.InsecureEnableGroups, getUserInfo: c.GetUserInfo, userIDKey: c.UserIDKey, userNameKey: c.UserNameKey, @@ -152,6 +156,7 @@ type oidcConnector struct { logger log.Logger hostedDomains []string insecureSkipEmailVerified bool + insecureEnableGroups bool getUserInfo bool userIDKey string userNameKey string @@ -274,6 +279,19 @@ func (c *oidcConnector) HandleCallback(s connector.Scopes, r *http.Request) (ide identity.UserID = userID } + if c.insecureEnableGroups { + vs, ok := claims["groups"].([]interface{}) + if ok { + for _, v := range vs { + if s, ok := v.(string); ok { + identity.Groups = append(identity.Groups, s) + } else { + return identity, errors.New("malformed \"groups\" claim") + } + } + } + } + return identity, nil }