Enable custom structure for group claim with default name group (oaut…

…h2-proxy#839) * Allow complex structure for groups in group claim. * Remove unused constant * Update variable name * Fix linting * Use helper method * Log error if not possible to append group value * Add missing import * Use own logger * Fix imports * Remove Dockerfile for testing * Add Changelog entry * Use formatGroup helper method and update tests * Return string instead of string array * Remove groups variable * Return error in format method. * Reorder imports Co-authored-by: Nick Meves <[email protected]>
liquidinvestigations · Apr 6, 2022 · a5ffb1b · a5ffb1b
1 parent 94abd2e
commit a5ffb1b
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 14 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@
   - Sessions from v6.0.0 or later had a graceful conversion to SHA256 that resulted in no reauthentication
   - Upgrading from v5.1.1 or earlier will result in a reauthentication
 - [#616](https://github.com/oauth2-proxy/oauth2-proxy/pull/616) Ensure you have configured oauth2-proxy to use the `groups` scope. The user may be logged out initially as they may not currently have the `groups` claim however after going back through login process wil be authenticated.
+- [#839](https://github.com/oauth2-proxy/oauth2-proxy/pull/839) Enables complex data structures for group claim entries, which are output as Json by default.
 
 ## Breaking Changes
 

diff --git a/providers/oidc.go b/providers/oidc.go
@@ -2,14 +2,17 @@ package providers
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
+	"reflect"
 	"strings"
 	"time"
 
 	oidc "github.com/coreos/go-oidc"
 	"golang.org/x/oauth2"
 
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions"
+	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"
 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests"
 )
 
@@ -285,21 +288,36 @@ func (p *OIDCProvider) extractGroupsFromRawClaims(rawClaims map[string]interface
 	rawGroups, ok := rawClaims[p.GroupsClaim].([]interface{})
 	if rawGroups != nil && ok {
 		for _, rawGroup := range rawGroups {
-			group, ok := rawGroup.(string)
-			if ok {
-				groups = append(groups, group)
+			formattedGroup, err := formatGroup(rawGroup)
+			if err != nil {
+				logger.Errorf("unable to format group of type %s with error %s", reflect.TypeOf(rawGroup), err)
+				continue
 			}
+			groups = append(groups, formattedGroup)
 		}
 	}
 
 	return groups
+
+}
+
+func formatGroup(rawGroup interface{}) (string, error) {
+	group, ok := rawGroup.(string)
+	if !ok {
+		jsonGroup, err := json.Marshal(rawGroup)
+		if err != nil {
+			return "", err
+		}
+		group = string(jsonGroup)
+	}
+	return group, nil
 }
 
 type OIDCClaims struct {
 	rawClaims         map[string]interface{}
 	UserID            string
-	Subject           string `json:"sub"`
-	Verified          *bool  `json:"email_verified"`
-	PreferredUsername string `json:"preferred_username"`
-	Groups            []string
+	Subject           string   `json:"sub"`
+	Verified          *bool    `json:"email_verified"`
+	PreferredUsername string   `json:"preferred_username"`
+	Groups            []string `json:"-"`
 }
diff --git a/providers/oidc_test.go b/providers/oidc_test.go
@@ -29,12 +29,12 @@ const clientID = "https://test.myapp.com"
 const secret = "secret"
 
 type idTokenClaims struct {
-	Name        string   `json:"name,omitempty"`
-	Email       string   `json:"email,omitempty"`
-	Phone       string   `json:"phone_number,omitempty"`
-	Picture     string   `json:"picture,omitempty"`
-	Groups      []string `json:"groups,omitempty"`
-	OtherGroups []string `json:"other_groups,omitempty"`
+	Name        string      `json:"name,omitempty"`
+	Email       string      `json:"email,omitempty"`
+	Phone       string      `json:"phone_number,omitempty"`
+	Picture     string      `json:"picture,omitempty"`
+	Groups      interface{} `json:"groups,omitempty"`
+	OtherGroups interface{} `json:"other_groups,omitempty"`
 	jwt.StandardClaims
 }
 
@@ -64,6 +64,29 @@ var defaultIDToken idTokenClaims = idTokenClaims{
 	},
 }
 
+var customGroupClaimIDToken idTokenClaims = idTokenClaims{
+	"Jane Dobbs",
+	"[email protected]",
+	"+4798765432",
+	"http://mugbook.com/janed/me.jpg",
+	[]map[string]interface{}{
+		{
+			"groupId": "Admin Group Id",
+			"roles":   []string{"Admin"},
+		},
+	},
+	[]string{"test:c", "test:d"},
+	jwt.StandardClaims{
+		Audience:  "https://test.myapp.com",
+		ExpiresAt: time.Now().Add(time.Duration(5) * time.Minute).Unix(),
+		Id:        "id-some-id",
+		IssuedAt:  time.Now().Unix(),
+		Issuer:    "https://issuer.example.com",
+		NotBefore: 0,
+		Subject:   "123456789",
+	},
+}
+
 var minimalIDToken idTokenClaims = idTokenClaims{
 	"",
 	"",
@@ -283,7 +306,7 @@ func TestCreateSessionStateFromBearerToken(t *testing.T) {
 		GroupsClaim    string
 		ExpectedUser   string
 		ExpectedEmail  string
-		ExpectedGroups []string
+		ExpectedGroups interface{}
 	}{
 		"Default IDToken": {
 			IDToken:        defaultIDToken,
@@ -306,6 +329,13 @@ func TestCreateSessionStateFromBearerToken(t *testing.T) {
 			ExpectedEmail:  defaultIDToken.Email,
 			ExpectedGroups: []string{"test:c", "test:d"},
 		},
+		"Custom Groups Claim2": {
+			IDToken:        customGroupClaimIDToken,
+			GroupsClaim:    "groups",
+			ExpectedUser:   customGroupClaimIDToken.Subject,
+			ExpectedEmail:  customGroupClaimIDToken.Email,
+			ExpectedGroups: []string{"{\"groupId\":\"Admin Group Id\",\"roles\":[\"Admin\"]}"},
+		},
 	}
 	for testName, tc := range testCases {
 		t.Run(testName, func(t *testing.T) {
@@ -373,3 +403,31 @@ func TestOIDCProvider_findVerifiedIdToken(t *testing.T) {
 	assert.Equal(t, nil, err)
 	assert.Equal(t, true, verifiedIDToken == nil)
 }
+
+func Test_formatGroup(t *testing.T) {
+	testCases := map[string]struct {
+		RawGroup                    interface{}
+		ExpectedFormattedGroupValue string
+	}{
+		"String Group": {
+			RawGroup:                    "group",
+			ExpectedFormattedGroupValue: "group",
+		},
+		"Map Group": {
+			RawGroup:                    map[string]string{"id": "1", "name": "Test"},
+			ExpectedFormattedGroupValue: "{\"id\":\"1\",\"name\":\"Test\"}",
+		},
+		"List Group": {
+			RawGroup:                    []string{"First", "Second"},
+			ExpectedFormattedGroupValue: "[\"First\",\"Second\"]",
+		},
+	}
+
+	for testName, tc := range testCases {
+		t.Run(testName, func(t *testing.T) {
+			formattedGroup, err := formatGroup(tc.RawGroup)
+			assert.Nil(t, err)
+			assert.Equal(t, tc.ExpectedFormattedGroupValue, formattedGroup)
+		})
+	}
+}