Enable custom structure for group claim with default name group (#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 <nick.meves@greenhouse.io>
This commit is contained in:
		
							parent
							
								
									cc6b808c17
								
							
						
					
					
						commit
						65016c8da1
					
				|  | @ -11,6 +11,7 @@ | ||||||
|   - Sessions from v6.0.0 or later had a graceful conversion to SHA256 that resulted in no reauthentication |   - 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 |   - 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. | - [#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 | ## Breaking Changes | ||||||
| 
 | 
 | ||||||
|  |  | ||||||
|  | @ -2,7 +2,9 @@ package providers | ||||||
| 
 | 
 | ||||||
| import ( | import ( | ||||||
| 	"context" | 	"context" | ||||||
|  | 	"encoding/json" | ||||||
| 	"fmt" | 	"fmt" | ||||||
|  | 	"reflect" | ||||||
| 	"strings" | 	"strings" | ||||||
| 	"time" | 	"time" | ||||||
| 
 | 
 | ||||||
|  | @ -10,6 +12,7 @@ import ( | ||||||
| 	"golang.org/x/oauth2" | 	"golang.org/x/oauth2" | ||||||
| 
 | 
 | ||||||
| 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/sessions" | 	"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" | 	"github.com/oauth2-proxy/oauth2-proxy/v7/pkg/requests" | ||||||
| ) | ) | ||||||
| 
 | 
 | ||||||
|  | @ -285,14 +288,29 @@ func (p *OIDCProvider) extractGroupsFromRawClaims(rawClaims map[string]interface | ||||||
| 	rawGroups, ok := rawClaims[p.GroupsClaim].([]interface{}) | 	rawGroups, ok := rawClaims[p.GroupsClaim].([]interface{}) | ||||||
| 	if rawGroups != nil && ok { | 	if rawGroups != nil && ok { | ||||||
| 		for _, rawGroup := range rawGroups { | 		for _, rawGroup := range rawGroups { | ||||||
| 			group, ok := rawGroup.(string) | 			formattedGroup, err := formatGroup(rawGroup) | ||||||
| 			if ok { | 			if err != nil { | ||||||
| 				groups = append(groups, group) | 				logger.Errorf("unable to format group of type %s with error %s", reflect.TypeOf(rawGroup), err) | ||||||
|  | 				continue | ||||||
| 			} | 			} | ||||||
|  | 			groups = append(groups, formattedGroup) | ||||||
| 		} | 		} | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	return groups | 	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 { | type OIDCClaims struct { | ||||||
|  | @ -301,5 +319,5 @@ type OIDCClaims struct { | ||||||
| 	Subject           string   `json:"sub"` | 	Subject           string   `json:"sub"` | ||||||
| 	Verified          *bool    `json:"email_verified"` | 	Verified          *bool    `json:"email_verified"` | ||||||
| 	PreferredUsername string   `json:"preferred_username"` | 	PreferredUsername string   `json:"preferred_username"` | ||||||
| 	Groups            []string | 	Groups            []string `json:"-"` | ||||||
| } | } | ||||||
|  |  | ||||||
|  | @ -33,8 +33,8 @@ type idTokenClaims struct { | ||||||
| 	Email       string      `json:"email,omitempty"` | 	Email       string      `json:"email,omitempty"` | ||||||
| 	Phone       string      `json:"phone_number,omitempty"` | 	Phone       string      `json:"phone_number,omitempty"` | ||||||
| 	Picture     string      `json:"picture,omitempty"` | 	Picture     string      `json:"picture,omitempty"` | ||||||
| 	Groups      []string `json:"groups,omitempty"` | 	Groups      interface{} `json:"groups,omitempty"` | ||||||
| 	OtherGroups []string `json:"other_groups,omitempty"` | 	OtherGroups interface{} `json:"other_groups,omitempty"` | ||||||
| 	jwt.StandardClaims | 	jwt.StandardClaims | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | @ -64,6 +64,29 @@ var defaultIDToken idTokenClaims = idTokenClaims{ | ||||||
| 	}, | 	}, | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
|  | var customGroupClaimIDToken idTokenClaims = idTokenClaims{ | ||||||
|  | 	"Jane Dobbs", | ||||||
|  | 	"janed@me.com", | ||||||
|  | 	"+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{ | var minimalIDToken idTokenClaims = idTokenClaims{ | ||||||
| 	"", | 	"", | ||||||
| 	"", | 	"", | ||||||
|  | @ -283,7 +306,7 @@ func TestCreateSessionStateFromBearerToken(t *testing.T) { | ||||||
| 		GroupsClaim    string | 		GroupsClaim    string | ||||||
| 		ExpectedUser   string | 		ExpectedUser   string | ||||||
| 		ExpectedEmail  string | 		ExpectedEmail  string | ||||||
| 		ExpectedGroups []string | 		ExpectedGroups interface{} | ||||||
| 	}{ | 	}{ | ||||||
| 		"Default IDToken": { | 		"Default IDToken": { | ||||||
| 			IDToken:        defaultIDToken, | 			IDToken:        defaultIDToken, | ||||||
|  | @ -306,6 +329,13 @@ func TestCreateSessionStateFromBearerToken(t *testing.T) { | ||||||
| 			ExpectedEmail:  defaultIDToken.Email, | 			ExpectedEmail:  defaultIDToken.Email, | ||||||
| 			ExpectedGroups: []string{"test:c", "test:d"}, | 			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 { | 	for testName, tc := range testCases { | ||||||
| 		t.Run(testName, func(t *testing.T) { | 		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, nil, err) | ||||||
| 	assert.Equal(t, true, verifiedIDToken == nil) | 	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) | ||||||
|  | 		}) | ||||||
|  | 	} | ||||||
|  | } | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue