From 4a0feaf589606fce80a3a10ae600dbd841227bef Mon Sep 17 00:00:00 2001
From: Stephan Renatus <srenatus@chef.io>
Date: Mon, 11 May 2020 11:42:26 +0200
Subject: [PATCH] connector/saml: add 'FilterGroups' setting

This should make AllowedGroups equivalent to an LDAP group filter:

When set to true, only the groups from AllowedGroups will be included in the
user's identity.

Signed-off-by: Stephan Renatus <srenatus@chef.io>
---
 Documentation/connectors/saml.md |  2 ++
 connector/saml/saml.go           |  7 +++++++
 connector/saml/saml_test.go      | 25 +++++++++++++++++++++++++
 3 files changed, 34 insertions(+)

diff --git a/Documentation/connectors/saml.md b/Documentation/connectors/saml.md
index c9af0995..5c542490 100644
--- a/Documentation/connectors/saml.md
+++ b/Documentation/connectors/saml.md
@@ -18,6 +18,8 @@ The connector doesn't support signed AuthnRequests or encrypted attributes.
 
 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.
 
+If `filterGroups` is set to true, any groups _not_ part of `allowedGroups` will be excluded.
+
 ## Configuration
 
 ```yaml
diff --git a/connector/saml/saml.go b/connector/saml/saml.go
index 1c1cb460..e0f3f933 100644
--- a/connector/saml/saml.go
+++ b/connector/saml/saml.go
@@ -101,6 +101,7 @@ type Config struct {
 	// used split the groups string.
 	GroupsDelim   string   `json:"groupsDelim"`
 	AllowedGroups []string `json:"allowedGroups"`
+	FilterGroups  bool     `json:"filterGroups"`
 	RedirectURI   string   `json:"redirectURI"`
 
 	// Requested format of the NameID. The NameID value is is mapped to the ID Token
@@ -165,6 +166,7 @@ func (c *Config) openConnector(logger log.Logger) (*provider, error) {
 		groupsAttr:    c.GroupsAttr,
 		groupsDelim:   c.GroupsDelim,
 		allowedGroups: c.AllowedGroups,
+		filterGroups:  c.FilterGroups,
 		redirectURI:   c.RedirectURI,
 		logger:        logger,
 
@@ -240,6 +242,7 @@ type provider struct {
 	groupsAttr    string
 	groupsDelim   string
 	allowedGroups []string
+	filterGroups  bool
 
 	redirectURI string
 
@@ -430,6 +433,10 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str
 		return ident, fmt.Errorf("user not a member of allowed groups")
 	}
 
+	if p.filterGroups {
+		ident.Groups = groupMatches
+	}
+
 	// Otherwise, we're good
 	return ident, nil
 }
diff --git a/connector/saml/saml_test.go b/connector/saml/saml_test.go
index 4d28e33a..aa07aae7 100644
--- a/connector/saml/saml_test.go
+++ b/connector/saml/saml_test.go
@@ -53,6 +53,7 @@ type responseTest struct {
 	emailAttr     string
 	groupsAttr    string
 	allowedGroups []string
+	filterGroups  bool
 
 	// Expected outcome of the test.
 	wantErr   bool
@@ -121,6 +122,29 @@ func TestGroupsWhitelist(t *testing.T) {
 	test.run(t)
 }
 
+func TestGroupsWhitelistWithFiltering(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"},
+		filterGroups:  true,
+		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" is filtered
+		},
+	}
+	test.run(t)
+}
+
 func TestGroupsWhitelistEmpty(t *testing.T) {
 	test := responseTest{
 		caFile:        "testdata/ca.crt",
@@ -388,6 +412,7 @@ func (r responseTest) run(t *testing.T) {
 		RedirectURI:   r.redirectURI,
 		EntityIssuer:  r.entityIssuer,
 		AllowedGroups: r.allowedGroups,
+		FilterGroups:  r.filterGroups,
 		// Never logging in, don't need this.
 		SSOURL: "http://foo.bar/",
 	}