Skip to content

Commit

Permalink
Add option to enable groups for oidc connectors
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jacksontj committed Sep 13, 2019
1 parent 8427f0f commit 21ab30d
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 0 deletions.
7 changes: 7 additions & 0 deletions Documentation/connectors/oidc.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
18 changes: 18 additions & 0 deletions connector/oidc/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -152,6 +156,7 @@ type oidcConnector struct {
logger log.Logger
hostedDomains []string
insecureSkipEmailVerified bool
insecureEnableGroups bool
getUserInfo bool
userIDKey string
userNameKey string
Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit 21ab30d

Please sign in to comment.