Repository: dexidp/dex
Pull request #2643 (2022-08-29 23:29:00)

feat(connector/authproxy): support multiple groups
<!--
Thank you for sending a pull request! Here are some tips for contributors:

1. Fill the description template below.
2. Sign a DCO (if you haven't already signed it).
3. Include appropriate tests (if necessary). Make sure that all CI checks passed.
4. If the Pull Request is a work in progress, make use of GitHub's "Draft PR" feature and mark it as such.
-->

#### Overview

Added support to use multiple values for group header on authproxy connector.

#### What this PR does / why we need it

<!--
- Please state in detail why we need this PR and what it solves.
- If your PR closes some of the existing issues, please add links to them here.
  Mentioned issues will be automatically closed.
  Usage: "Closes #<issue number>", or "Closes (paste link of issue)"
-->

If you use multiples values using comma-separated, the connector can't understand there are different groups.

#### Special notes for your reviewer
Definition to use comma-separated list [RFC2616 section 4.2](https://www.rfc-editor.org/rfc/rfc2616#section-4.2)
#### Does this PR introduce a user-facing change?

<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->

```
NONE
```



Patch:
diff --git a/connector/authproxy/authproxy.go b/connector/authproxy/authproxy.go
index 487a3f60..87154121 100644
--- a/connector/authproxy/authproxy.go
+++ b/connector/authproxy/authproxy.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+	"strings"
 
 	"github.com/dexidp/dex/connector"
 	"github.com/dexidp/dex/pkg/log"
@@ -69,7 +70,11 @@ func (m *callback) HandleCallback(s connector.Scopes, r *http.Request) (connecto
 	groups := m.groups
 	headerGroup := r.Header.Get(m.groupHeader)
 	if headerGroup != "" {
-		groups = append(groups, headerGroup)
+		splitheaderGroup := strings.Split(headerGroup, ",")
+		for i, v := range splitheaderGroup {
+			splitheaderGroup[i] = strings.TrimSpace(v)
+		}
+		groups = append(splitheaderGroup, groups...)
 	}
 	return connector.Identity{
 		UserID:        remoteUser, // TODO: figure out if this is a bad ID value.


Test patch:
diff --git a/connector/authproxy/authproxy_test.go b/connector/authproxy/authproxy_test.go
new file mode 100644
index 00000000..5d42530e
--- /dev/null
+++ b/connector/authproxy/authproxy_test.go
@@ -0,0 +1,134 @@
+package authproxy
+
+import (
+	"io"
+	"net/http"
+	"reflect"
+	"testing"
+
+	"github.com/sirupsen/logrus"
+
+	"github.com/dexidp/dex/connector"
+)
+
+const (
+	testEmail        = "testuser@example.com"
+	testGroup1       = "group1"
+	testGroup2       = "group2"
+	testGroup3       = "group 3"
+	testGroup4       = "group 4"
+	testStaticGroup1 = "static1"
+	testStaticGroup2 = "static 2"
+)
+
+var logger = &logrus.Logger{Out: io.Discard, Formatter: &logrus.TextFormatter{}}
+
+func TestUser(t *testing.T) {
+	config := Config{
+		UserHeader: "X-Remote-User",
+	}
+	conn := callback{userHeader: config.UserHeader, logger: logger, pathSuffix: "/test"}
+
+	req, err := http.NewRequest("GET", "/", nil)
+	expectNil(t, err)
+	req.Header = map[string][]string{
+		"X-Remote-User": {testEmail},
+	}
+
+	ident, err := conn.HandleCallback(connector.Scopes{OfflineAccess: true, Groups: true}, req)
+	expectNil(t, err)
+
+	expectEquals(t, ident.UserID, testEmail)
+	expectEquals(t, ident.Email, testEmail)
+	expectEquals(t, len(ident.Groups), 0)
+}
+
+func TestSingleGroup(t *testing.T) {
+	config := Config{
+		UserHeader:  "X-Remote-User",
+		GroupHeader: "X-Remote-Group",
+	}
+
+	conn := callback{userHeader: config.UserHeader, groupHeader: config.GroupHeader, logger: logger, pathSuffix: "/test"}
+
+	req, err := http.NewRequest("GET", "/", nil)
+	expectNil(t, err)
+	req.Header = map[string][]string{
+		"X-Remote-User":  {testEmail},
+		"X-Remote-Group": {testGroup1},
+	}
+
+	ident, err := conn.HandleCallback(connector.Scopes{OfflineAccess: true, Groups: true}, req)
+	expectNil(t, err)
+
+	expectEquals(t, ident.UserID, testEmail)
+	expectEquals(t, len(ident.Groups), 1)
+	expectEquals(t, ident.Groups[0], testGroup1)
+}
+
+func TestMultipleGroup(t *testing.T) {
+	config := Config{
+		UserHeader:  "X-Remote-User",
+		GroupHeader: "X-Remote-Group",
+	}
+
+	conn := callback{userHeader: config.UserHeader, groupHeader: config.GroupHeader, logger: logger, pathSuffix: "/test"}
+
+	req, err := http.NewRequest("GET", "/", nil)
+	expectNil(t, err)
+	req.Header = map[string][]string{
+		"X-Remote-User":  {testEmail},
+		"X-Remote-Group": {testGroup1 + ", " + testGroup2 + ", " + testGroup3 + ", " + testGroup4},
+	}
+
+	ident, err := conn.HandleCallback(connector.Scopes{OfflineAccess: true, Groups: true}, req)
+	expectNil(t, err)
+
+	expectEquals(t, ident.UserID, testEmail)
+	expectEquals(t, len(ident.Groups), 4)
+	expectEquals(t, ident.Groups[0], testGroup1)
+	expectEquals(t, ident.Groups[1], testGroup2)
+	expectEquals(t, ident.Groups[2], testGroup3)
+	expectEquals(t, ident.Groups[3], testGroup4)
+}
+
+func TestStaticGroup(t *testing.T) {
+	config := Config{
+		UserHeader:  "X-Remote-User",
+		GroupHeader: "X-Remote-Group",
+		Groups:      []string{"static1", "static 2"},
+	}
+
+	conn := callback{userHeader: config.UserHeader, groupHeader: config.GroupHeader, groups: config.Groups, logger: logger, pathSuffix: "/test"}
+
+	req, err := http.NewRequest("GET", "/", nil)
+	expectNil(t, err)
+	req.Header = map[string][]string{
+		"X-Remote-User":  {testEmail},
+		"X-Remote-Group": {testGroup1 + ", " + testGroup2 + ", " + testGroup3 + ", " + testGroup4},
+	}
+
+	ident, err := conn.HandleCallback(connector.Scopes{OfflineAccess: true, Groups: true}, req)
+	expectNil(t, err)
+
+	expectEquals(t, ident.UserID, testEmail)
+	expectEquals(t, len(ident.Groups), 6)
+	expectEquals(t, ident.Groups[0], testGroup1)
+	expectEquals(t, ident.Groups[1], testGroup2)
+	expectEquals(t, ident.Groups[2], testGroup3)
+	expectEquals(t, ident.Groups[3], testGroup4)
+	expectEquals(t, ident.Groups[4], testStaticGroup1)
+	expectEquals(t, ident.Groups[5], testStaticGroup2)
+}
+
+func expectNil(t *testing.T, a interface{}) {
+	if a != nil {
+		t.Errorf("Expected %+v to equal nil", a)
+	}
+}
+
+func expectEquals(t *testing.T, a interface{}, b interface{}) {
+	if !reflect.DeepEqual(a, b) {
+		t.Errorf("Expected %+v to equal %+v", a, b)
+	}
+}