Merge pull request #1544 from kenperkins/saml-groups

Adding support for allowed groups in SAML Connector

Author: bonifaido

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 )

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
 }
 

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/",
 	}