Repository: dexidp/dex
Pull request #2639 (2022-08-24 13:54:35)

fix: Fallback when group claim is a string instead of an array of strings
#### Overview
When using the OIDC provider, in combination with Microsoft ADFS using OpenID, groups can be filtered on Microsoft's side. Whenever the result of this filtering consists of only one group, the response back to Dex will be a string instead of an array of strings.

**OIDC response examples:**
_Response with multiple groups:_
```json
{
  "aud": "microsoft:identityserver:XXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXX",
  "iss": "http://domain.acme.com/adfs/services/trust",
  "iat": 1661346019,
  "exp": 1661349619,
  "email": "myemailaddress@acme.com",
  "given_name": "Henk",
  "family_name": "Vries, De",
  "unique_name": "Vries, Henk de",
  "winaccountname": "henkie",
  "groupsid": [
    "K8s_P_Group1",
    "K8s_P_Group2"
  ],
  "group": [
    "K8s_P_Group1",
    "K8s_P_Group2"
  ],
  "apptype": "Confidential",
  "appid": "XXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXX",
  "authmethod": "urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport",
  "auth_time": "2022-08-24T12:17:06.060Z",
  "ver": "1.0",
  "scp": "openid"
}
```

 _Response with one group:_
```json
{
  "aud": "microsoft:identityserver:XXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXX",
  "iss": "http://domain.acme.com/adfs/services/trust",
  "iat": 1661346019,
  "exp": 1661349619,
  "email": "myemailaddress@acme.com",
  "given_name": "Henk",
  "family_name": "Vries, De",
  "unique_name": "Vries, Henk de",
  "winaccountname": "henkie",
  "groupsid": "K8s_P_Group1",
  "group": "K8s_P_Group1",
  "apptype": "Confidential",
  "appid": "XXXXXXX-XXXX-XXXX-XXXX-XXXXXXXXX",
  "authmethod": "urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport",
  "auth_time": "2022-08-24T12:17:06.060Z",
  "ver": "1.0",
  "scp": "openid"
}
```
#### What this PR does / why we need it
This PR adds a fallback to the OIDC connector to add support for a singular group received as a string instead of an array of strings.

```release-note
NONE
```

Co-authored-by: Michiel van Pouderoijen <michiel@pouderoijen.nl>
Signed-off-by: Joost Buskermolen <joost@buskervezel.nl>

nabokihms: Related https://github.com/dexidp/dex/pull/2562
JoooostB: @nabokihms I moved the fallback outside of the if statement for custom `groupsKey` to make sure it's executed when the default `groupsKey` is used. As requested I also created a testcase containing a singular group as a string.

Patch:
diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go
index b4e67799..e345dca0 100644
--- a/connector/oidc/oidc.go
+++ b/connector/oidc/oidc.go
@@ -351,6 +351,11 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I
 			vs, found = claims[groupsKey].([]interface{})
 		}
 
+		// Fallback when claims[groupsKey] is a string instead of an array of strings.
+		if g, b := claims[groupsKey].(string); b {
+			groups = []string{g}
+		}
+
 		if found {
 			for _, v := range vs {
 				if s, ok := v.(string); ok {


Test patch:
diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go
index d8b30b39..d94af79d 100644
--- a/connector/oidc/oidc_test.go
+++ b/connector/oidc/oidc_test.go
@@ -271,6 +271,22 @@ func TestHandleCallback(t *testing.T) {
 				"cognito:groups": []string{"group3", "group4"},
 			},
 		},
+		{
+			name:               "singularGroupResponseAsString",
+			userIDKey:          "", // not configured
+			userNameKey:        "", // not configured
+			expectUserID:       "subvalue",
+			expectUserName:     "namevalue",
+			expectGroups:       []string{"group1"},
+			expectedEmailField: "emailvalue",
+			token: map[string]interface{}{
+				"sub":            "subvalue",
+				"name":           "namevalue",
+				"groups":         "group1",
+				"email":          "emailvalue",
+				"email_verified": true,
+			},
+		},
 	}
 
 	for _, tc := range tests {