Skip to content

Commit

Permalink
Address issues raised in review:
Browse files Browse the repository at this point in the history
Improve naming

Signed-off-by: Oded Ben-Ozer <obenozer@wayfair.com>
  • Loading branch information
Oded-B committed Oct 25, 2023
1 parent 033717a commit 1154259
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 17 deletions.
18 changes: 9 additions & 9 deletions connector/oidc/oidc.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,16 @@ type Config struct {
GroupsKey string `json:"groups"` // defaults to "groups"
} `json:"claimMapping"`

// ClaimModifications holds all claim modifications options
ClaimModifications struct {
NewGroupsFromClaims []NewGroupsFromClaims `json:"newGroupsFromClaims"`
// ClaimMutations holds all claim mutations options
ClaimMutations struct {
NewGroupFromClaims []NewGroupFromClaims `json:"newGroupFromClaims"`
} `json:"claimModifications"`
}

// NewGroupFromClaims creates a new group from a list of claims and appends it to the list of existing groups.
type NewGroupsFromClaims struct {
type NewGroupFromClaims struct {
// List of claim to join together
ClaimList []string `json:"claimList"`
Claims []string `json:"claims"`

// String to separate the claims
Delimiter string `json:"delimiter"`
Expand Down Expand Up @@ -210,7 +210,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,
newGroupsFromClaims: c.ClaimModifications.NewGroupsFromClaims,
newGroupFromClaims: c.ClaimMutations.NewGroupFromClaims,
}, nil
}

Expand Down Expand Up @@ -238,7 +238,7 @@ type oidcConnector struct {
preferredUsernameKey string
emailKey string
groupsKey string
newGroupsFromClaims []NewGroupsFromClaims
newGroupFromClaims []NewGroupFromClaims
}

func (c *oidcConnector) Close() error {
Expand Down Expand Up @@ -450,11 +450,11 @@ func (c *oidcConnector) createIdentity(ctx context.Context, identity connector.I
}
}

for _, config := range c.newGroupsFromClaims {
for _, config := range c.newGroupFromClaims {
newGroupSegments := []string{
config.Prefix,
}
for _, claimName := range config.ClaimList {
for _, claimName := range config.Claims {
claimValue, ok := claims[claimName].(string)
if !ok { // Non string claim value are ignored, concatenating them doesn't really make any sense
continue
Expand Down
16 changes: 8 additions & 8 deletions connector/oidc/oidc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestHandleCallback(t *testing.T) {
expectPreferredUsername string
expectedEmailField string
token map[string]interface{}
newGroupsFromClaims []NewGroupsFromClaims
newGroupFromClaims []NewGroupFromClaims
}{
{
name: "simpleCase",
Expand Down Expand Up @@ -297,17 +297,17 @@ func TestHandleCallback(t *testing.T) {
expectUserName: "namevalue",
expectGroups: []string{"group1", "gh::acme::pipeline-one", "clr_delim-acme-foobar", "keep_delim-acme-foo-bar", "bk-emailvalue"},
expectedEmailField: "emailvalue",
newGroupsFromClaims: []NewGroupsFromClaims{
newGroupFromClaims: []NewGroupFromClaims{
{ // The basic functionality, should create "gh::acme::pipeline-one".
ClaimList: []string{
Claims: []string{
"organization",
"pipeline",
},
Delimiter: "::",
Prefix: "gh",
},
{ // Non existing claims, should not generate any any new group claim.
ClaimList: []string{
Claims: []string{
"non-existing1",
"non-existing2",
},
Expand All @@ -317,7 +317,7 @@ func TestHandleCallback(t *testing.T) {
{ // 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 "clr_delim-acme-foobar" and not "tfe-acme-foo-bar".
ClaimList: []string{
Claims: []string{
"organization",
"claim-with-delimiter",
},
Expand All @@ -327,7 +327,7 @@ func TestHandleCallback(t *testing.T) {
},
{ // In this case the delimiter character("-") should be NOT removed from "claim-with-delimiter" claim.
// Should create "keep_delim-acme-foo-bar".
ClaimList: []string{
Claims: []string{
"organization",
"claim-with-delimiter",
},
Expand All @@ -336,7 +336,7 @@ func TestHandleCallback(t *testing.T) {
Prefix: "keep_delim",
},
{ // Ignore non string claims (like arrays), this should result in "bk-emailvalue".
ClaimList: []string{
Claims: []string{
"non-string-claim",
"non-string-claim2",
"email",
Expand Down Expand Up @@ -397,7 +397,7 @@ func TestHandleCallback(t *testing.T) {
config.ClaimMapping.PreferredUsernameKey = tc.preferredUsernameKey
config.ClaimMapping.EmailKey = tc.emailKey
config.ClaimMapping.GroupsKey = tc.groupsKey
config.ClaimModifications.NewGroupsFromClaims = tc.newGroupsFromClaims
config.ClaimMutations.NewGroupFromClaims = tc.newGroupFromClaims

conn, err := newConnector(config)
if err != nil {
Expand Down

0 comments on commit 1154259

Please sign in to comment.