From e46f1c48ed64143f466980c4a6a855bbfab405b3 Mon Sep 17 00:00:00 2001 From: Oded Ben Ozer Date: Thu, 27 Jul 2023 11:58:28 +0200 Subject: [PATCH 1/3] Composite claims in OIDC connector (#3) * Add the ability to composite new claims in the OIDC connector, based on upstream claims Signed-off-by: Oded Ben-Ozer --- connector/oidc/oidc.go | 37 ++++++++++++++++++++++ connector/oidc/oidc_test.go | 62 +++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+) diff --git a/connector/oidc/oidc.go b/connector/oidc/oidc.go index 14329c0040d..00e9dc48e7e 100644 --- a/connector/oidc/oidc.go +++ b/connector/oidc/oidc.go @@ -87,6 +87,21 @@ type Config struct { // Configurable key which contains the groups claims GroupsKey string `json:"groups"` // defaults to "groups" } `json:"claimMapping"` + + // List of new claim to generate based on concatinate existing claims + ClaimConcatenations []ClaimConcatenation `json:"claimConcatenations"` +} + +// List of groups claim elements to create by concatenating other claims +type ClaimConcatenation struct { + // List of claim to join together + ClaimList []string `json:"claimList"` + + // String to separate the claims + Delimiter string `json:"delimiter"` + + // String to place before the first claim + Prefix string `json:"prefix"` } // Domains that don't support basic auth. golang.org/x/oauth2 has an internal @@ -189,6 +204,7 @@ func (c *Config) Open(id string, logger log.Logger) (conn connector.Connector, e preferredUsernameKey: c.ClaimMapping.PreferredUsernameKey, emailKey: c.ClaimMapping.EmailKey, groupsKey: c.ClaimMapping.GroupsKey, + claimConcatenations: c.ClaimConcatenations, }, nil } @@ -216,6 +232,7 @@ type oidcConnector struct { preferredUsernameKey string emailKey string groupsKey string + claimConcatenations []ClaimConcatenation } func (c *oidcConnector) Close() error { @@ -416,6 +433,26 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I } } + for _, cc := range c.claimConcatenations { + newElement := "" + for _, clm := range cc.ClaimList { + // Non string claim value are ignored, concatenating them doesn't really make any sense + if claimValue, ok := claims[clm].(string); ok { + // Removing the delimiier string from the concatenated claim to ensure resulting claim structure + // is in full control of Dex operator + claimCleanedValue := strings.ReplaceAll(claimValue, cc.Delimiter, "") + if newElement == "" { + newElement = cc.Prefix + cc.Delimiter + claimCleanedValue + } else { + newElement = newElement + cc.Delimiter + claimCleanedValue + } + } + } + if newElement != "" { + groups = append(groups, newElement) + } + } + cd := connectorData{ RefreshToken: []byte(token.RefreshToken), } diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index 5c5208a60e7..2f19d9674f0 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -62,6 +62,7 @@ func TestHandleCallback(t *testing.T) { expectPreferredUsername string expectedEmailField string token map[string]interface{} + claimConcatenations []ClaimConcatenation }{ { name: "simpleCase", @@ -288,6 +289,66 @@ func TestHandleCallback(t *testing.T) { "email_verified": true, }, }, + { + name: "concatinateClaim", + userIDKey: "", // not configured + userNameKey: "", // not configured + expectUserID: "subvalue", + expectUserName: "namevalue", + expectGroups: []string{"group1", "gh::acme::pipeline-one", "tfe-acme-foobar", "bk-emailvalue"}, + expectedEmailField: "emailvalue", + claimConcatenations: []ClaimConcatenation{ + { + ClaimList: []string{ + "organization", + "pipeline", + }, + Delimiter: "::", + Prefix: "gh", + }, + { + ClaimList: []string{ + "non-existing1", + "non-existing2", + }, + Delimiter: "::", + Prefix: "tfe", + }, + { + ClaimList: []string{ + "organization", + "claim-with-delimiter", + }, + Delimiter: "-", + Prefix: "tfe", + }, + { + ClaimList: []string{ + "non-string-claim", + "non-string-claim2", + "email", + }, + Delimiter: "-", + Prefix: "bk", + }, + }, + + token: map[string]interface{}{ + "sub": "subvalue", + "name": "namevalue", + "groups": "group1", + "organization": "acme", + "pipeline": "pipeline-one", + "email": "emailvalue", + "email_verified": true, + "claim-with-delimiter": "foo-bar", + "non-string-claim": []string{ + "element1", + "element2", + }, + "non-string-claim2": 666, + }, + }, } for _, tc := range tests { @@ -319,6 +380,7 @@ func TestHandleCallback(t *testing.T) { InsecureEnableGroups: true, BasicAuthUnsupported: &basicAuth, OverrideClaimMapping: tc.overrideClaimMapping, + ClaimConcatenations: tc.claimConcatenations, } config.ClaimMapping.PreferredUsernameKey = tc.preferredUsernameKey config.ClaimMapping.EmailKey = tc.emailKey From 72c45a1e5d6163117588b92186849f2f5f20918f Mon Sep 17 00:00:00 2001 From: Oded Ben-Ozer Date: Sun, 6 Aug 2023 13:13:39 +0200 Subject: [PATCH 2/3] Document each test case --- connector/oidc/oidc_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index 2f19d9674f0..459e2ca2c52 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -298,7 +298,7 @@ func TestHandleCallback(t *testing.T) { expectGroups: []string{"group1", "gh::acme::pipeline-one", "tfe-acme-foobar", "bk-emailvalue"}, expectedEmailField: "emailvalue", claimConcatenations: []ClaimConcatenation{ - { + { // The basic functionality, should create "gh::acme::pipeline-one". ClaimList: []string{ "organization", "pipeline", @@ -306,7 +306,7 @@ func TestHandleCallback(t *testing.T) { Delimiter: "::", Prefix: "gh", }, - { + { // Non existing claims, should not generate any any new group claim. ClaimList: []string{ "non-existing1", "non-existing2", @@ -314,7 +314,9 @@ func TestHandleCallback(t *testing.T) { Delimiter: "::", Prefix: "tfe", }, - { + { // In this case the delimiter character("-") should be removed removed from "claim-with-delimiter" claim to ensure the resulting + // claim structure is in full control of the Dex operator and not the person creating a new pipeline. + // Should create "tfe-acme-foobar" and not "tfe-acme-foo-bar". ClaimList: []string{ "organization", "claim-with-delimiter", @@ -322,7 +324,7 @@ func TestHandleCallback(t *testing.T) { Delimiter: "-", Prefix: "tfe", }, - { + { // Ignore non string claims (like arrays), this should result in "bk-emailvalue". ClaimList: []string{ "non-string-claim", "non-string-claim2", From 6d801c13353eb36aef06a369fe7f05e04fef1f00 Mon Sep 17 00:00:00 2001 From: Oded Ben-Ozer Date: Sun, 6 Aug 2023 13:13:39 +0200 Subject: [PATCH 3/3] Document each test case Signed-off-by: Oded Ben-Ozer --- connector/oidc/oidc_test.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/connector/oidc/oidc_test.go b/connector/oidc/oidc_test.go index 2f19d9674f0..459e2ca2c52 100644 --- a/connector/oidc/oidc_test.go +++ b/connector/oidc/oidc_test.go @@ -298,7 +298,7 @@ func TestHandleCallback(t *testing.T) { expectGroups: []string{"group1", "gh::acme::pipeline-one", "tfe-acme-foobar", "bk-emailvalue"}, expectedEmailField: "emailvalue", claimConcatenations: []ClaimConcatenation{ - { + { // The basic functionality, should create "gh::acme::pipeline-one". ClaimList: []string{ "organization", "pipeline", @@ -306,7 +306,7 @@ func TestHandleCallback(t *testing.T) { Delimiter: "::", Prefix: "gh", }, - { + { // Non existing claims, should not generate any any new group claim. ClaimList: []string{ "non-existing1", "non-existing2", @@ -314,7 +314,9 @@ func TestHandleCallback(t *testing.T) { Delimiter: "::", Prefix: "tfe", }, - { + { // In this case the delimiter character("-") should be removed removed from "claim-with-delimiter" claim to ensure the resulting + // claim structure is in full control of the Dex operator and not the person creating a new pipeline. + // Should create "tfe-acme-foobar" and not "tfe-acme-foo-bar". ClaimList: []string{ "organization", "claim-with-delimiter", @@ -322,7 +324,7 @@ func TestHandleCallback(t *testing.T) { Delimiter: "-", Prefix: "tfe", }, - { + { // Ignore non string claims (like arrays), this should result in "bk-emailvalue". ClaimList: []string{ "non-string-claim", "non-string-claim2",