From 285c1f162e19d9dd718e55dfde192ec285444a55 Mon Sep 17 00:00:00 2001 From: Ken Perkins Date: Tue, 10 Sep 2019 06:13:50 -0700 Subject: [PATCH] connector/saml: Adding group filtering - 4 new tests - Doc changes to use the group filtering --- Documentation/connectors/saml.md | 8 +++ connector/saml/saml.go | 59 ++++++++++++----- connector/saml/saml_test.go | 110 ++++++++++++++++++++++++++++--- 3 files changed, 150 insertions(+), 27 deletions(-) diff --git a/Documentation/connectors/saml.md b/Documentation/connectors/saml.md index 62cf6a7ff5..c9af0995e6 100644 --- a/Documentation/connectors/saml.md +++ b/Documentation/connectors/saml.md @@ -14,6 +14,10 @@ __The connector doesn't support refresh tokens__ since the SAML 2.0 protocol doe The connector doesn't support signed AuthnRequests or encrypted attributes. +## Group Filtering + +The SAML Connector supports providing a whitelist of SAML Groups to filter access based on, and when the `groupsattr` is set with a scope including groups, Dex will check for membership based on configured groups in the `allowedGroups` config setting for the SAML connector. + ## Configuration ```yaml @@ -44,6 +48,10 @@ connectors: emailAttr: email groupsAttr: groups # optional + # List of groups to filter access based on membership + # allowedGroups + # - Admins + # CA's can also be provided inline as a base64'd blob. # # caData: ( RAW base64'd PEM encoded CA ) diff --git a/connector/saml/saml.go b/connector/saml/saml.go index 4dac2aca6f..09414ea384 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -15,6 +15,7 @@ import ( "github.com/beevik/etree" "github.com/dexidp/dex/connector" + "github.com/dexidp/dex/pkg/groups" "github.com/dexidp/dex/pkg/log" dsig "github.com/russellhaering/goxmldsig" "github.com/russellhaering/goxmldsig/etreeutils" @@ -97,9 +98,9 @@ type Config struct { // If GroupsDelim is supplied the connector assumes groups are returned as a // single string instead of multiple attribute values. This delimiter will be // used split the groups string. - GroupsDelim string `json:"groupsDelim"` - - RedirectURI string `json:"redirectURI"` + GroupsDelim string `json:"groupsDelim"` + AllowedGroups []string `json:"allowedGroups"` + RedirectURI string `json:"redirectURI"` // Requested format of the NameID. The NameID value is is mapped to the ID Token // 'sub' claim. @@ -154,16 +155,17 @@ func (c *Config) openConnector(logger log.Logger) (*provider, error) { } p := &provider{ - entityIssuer: c.EntityIssuer, - ssoIssuer: c.SSOIssuer, - ssoURL: c.SSOURL, - now: time.Now, - usernameAttr: c.UsernameAttr, - emailAttr: c.EmailAttr, - groupsAttr: c.GroupsAttr, - groupsDelim: c.GroupsDelim, - redirectURI: c.RedirectURI, - logger: logger, + entityIssuer: c.EntityIssuer, + ssoIssuer: c.SSOIssuer, + ssoURL: c.SSOURL, + now: time.Now, + usernameAttr: c.UsernameAttr, + emailAttr: c.EmailAttr, + groupsAttr: c.GroupsAttr, + groupsDelim: c.GroupsDelim, + allowedGroups: c.AllowedGroups, + redirectURI: c.RedirectURI, + logger: logger, nameIDPolicyFormat: c.NameIDPolicyFormat, } @@ -232,10 +234,11 @@ type provider struct { validator *dsig.ValidationContext // Attribute mappings - usernameAttr string - emailAttr string - groupsAttr string - groupsDelim string + usernameAttr string + emailAttr string + groupsAttr string + groupsDelim string + allowedGroups []string redirectURI string @@ -388,11 +391,16 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str return ident, fmt.Errorf("no attribute with name %q: %s", p.usernameAttr, attributes.names()) } - if !s.Groups || p.groupsAttr == "" { + if len(p.allowedGroups) == 0 && (!s.Groups || p.groupsAttr == "") { // Groups not requested or not configured. We're done. return ident, nil } + if len(p.allowedGroups) > 0 && (!s.Groups || p.groupsAttr == "") { + // allowedGroups set but no groups or groupsAttr. Disallowing. + return ident, fmt.Errorf("User not a member of allowed groups") + } + // Grab the groups. if p.groupsDelim != "" { groupsStr, ok := attributes.get(p.groupsAttr) @@ -408,6 +416,21 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str } ident.Groups = groups } + + if len(p.allowedGroups) == 0 { + // No allowed groups set, just return the ident + return ident, nil + } + + // Look for membership in one of the allowed groups + groupMatches := groups.Filter(ident.Groups, p.allowedGroups) + + if len(groupMatches) == 0 { + // No group membership matches found, disallowing + return ident, fmt.Errorf("User not a member of allowed groups") + } + + // Otherwise, we're good return ident, nil } diff --git a/connector/saml/saml_test.go b/connector/saml/saml_test.go index d9aaf3f49b..4d28e33a5a 100644 --- a/connector/saml/saml_test.go +++ b/connector/saml/saml_test.go @@ -49,9 +49,10 @@ type responseTest struct { entityIssuer string // Attribute customization. - usernameAttr string - emailAttr string - groupsAttr string + usernameAttr string + emailAttr string + groupsAttr string + allowedGroups []string // Expected outcome of the test. wantErr bool @@ -98,6 +99,96 @@ func TestGroups(t *testing.T) { test.run(t) } +func TestGroupsWhitelist(t *testing.T) { + test := responseTest{ + caFile: "testdata/ca.crt", + respFile: "testdata/good-resp.xml", + now: "2017-04-04T04:34:59.330Z", + usernameAttr: "Name", + emailAttr: "email", + groupsAttr: "groups", + allowedGroups: []string{"Admins"}, + inResponseTo: "6zmm5mguyebwvajyf2sdwwcw6m", + redirectURI: "http://127.0.0.1:5556/dex/callback", + wantIdent: connector.Identity{ + UserID: "eric.chiang+okta@coreos.com", + Username: "Eric", + Email: "eric.chiang+okta@coreos.com", + EmailVerified: true, + Groups: []string{"Admins", "Everyone"}, + }, + } + test.run(t) +} + +func TestGroupsWhitelistEmpty(t *testing.T) { + test := responseTest{ + caFile: "testdata/ca.crt", + respFile: "testdata/good-resp.xml", + now: "2017-04-04T04:34:59.330Z", + usernameAttr: "Name", + emailAttr: "email", + groupsAttr: "groups", + allowedGroups: []string{}, + inResponseTo: "6zmm5mguyebwvajyf2sdwwcw6m", + redirectURI: "http://127.0.0.1:5556/dex/callback", + wantIdent: connector.Identity{ + UserID: "eric.chiang+okta@coreos.com", + Username: "Eric", + Email: "eric.chiang+okta@coreos.com", + EmailVerified: true, + Groups: []string{"Admins", "Everyone"}, + }, + } + test.run(t) +} + +func TestGroupsWhitelistDisallowed(t *testing.T) { + test := responseTest{ + wantErr: true, + caFile: "testdata/ca.crt", + respFile: "testdata/good-resp.xml", + now: "2017-04-04T04:34:59.330Z", + usernameAttr: "Name", + emailAttr: "email", + groupsAttr: "groups", + allowedGroups: []string{"Nope"}, + inResponseTo: "6zmm5mguyebwvajyf2sdwwcw6m", + redirectURI: "http://127.0.0.1:5556/dex/callback", + wantIdent: connector.Identity{ + UserID: "eric.chiang+okta@coreos.com", + Username: "Eric", + Email: "eric.chiang+okta@coreos.com", + EmailVerified: true, + Groups: []string{"Admins", "Everyone"}, + }, + } + test.run(t) +} + +func TestGroupsWhitelistDisallowedNoGroupsOnIdent(t *testing.T) { + test := responseTest{ + wantErr: true, + caFile: "testdata/ca.crt", + respFile: "testdata/good-resp.xml", + now: "2017-04-04T04:34:59.330Z", + usernameAttr: "Name", + emailAttr: "email", + groupsAttr: "groups", + allowedGroups: []string{"Nope"}, + inResponseTo: "6zmm5mguyebwvajyf2sdwwcw6m", + redirectURI: "http://127.0.0.1:5556/dex/callback", + wantIdent: connector.Identity{ + UserID: "eric.chiang+okta@coreos.com", + Username: "Eric", + Email: "eric.chiang+okta@coreos.com", + EmailVerified: true, + Groups: []string{}, + }, + } + test.run(t) +} + // TestOkta tests against an actual response from Okta. func TestOkta(t *testing.T) { test := responseTest{ @@ -290,12 +381,13 @@ func loadCert(ca string) (*x509.Certificate, error) { func (r responseTest) run(t *testing.T) { c := Config{ - CA: r.caFile, - UsernameAttr: r.usernameAttr, - EmailAttr: r.emailAttr, - GroupsAttr: r.groupsAttr, - RedirectURI: r.redirectURI, - EntityIssuer: r.entityIssuer, + CA: r.caFile, + UsernameAttr: r.usernameAttr, + EmailAttr: r.emailAttr, + GroupsAttr: r.groupsAttr, + RedirectURI: r.redirectURI, + EntityIssuer: r.entityIssuer, + AllowedGroups: r.allowedGroups, // Never logging in, don't need this. SSOURL: "http://foo.bar/", }