From ac1f7dd33d38bb41437899ac89b87bd451aaae94 Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Thu, 11 Jul 2024 15:51:04 -0700 Subject: [PATCH 1/4] fix: omit empty string from name --- internal/api/samlassertion.go | 5 +- internal/api/samlassertion_test.go | 126 ++++++++++++++--------------- 2 files changed, 67 insertions(+), 64 deletions(-) diff --git a/internal/api/samlassertion.go b/internal/api/samlassertion.go index 75cdfdb4f7..58f210db58 100644 --- a/internal/api/samlassertion.go +++ b/internal/api/samlassertion.go @@ -120,7 +120,10 @@ func (a *SAMLAssertion) Process(mapping models.SAMLAttributeMapping) map[string] ret := make(map[string]interface{}) for key, mapper := range mapping.Keys { - names := []string{mapper.Name} + names := []string{} + if mapper.Name != "" { + names = append(names, mapper.Name) + } names = append(names, mapper.Names...) setKey := false diff --git a/internal/api/samlassertion_test.go b/internal/api/samlassertion_test.go index 62c3e52f4f..2d4f6e3626 100644 --- a/internal/api/samlassertion_test.go +++ b/internal/api/samlassertion_test.go @@ -160,14 +160,14 @@ func TestSAMLAssertionProcessing(t *tst.T) { { desc: "valid attribute and mapping", xml: ` - - - - someone@example.com - - - -`, + + + + someone@example.com + + + + `, mapping: models.SAMLAttributeMapping{ Keys: map[string]models.SAMLAttribute{ "email": { @@ -182,17 +182,17 @@ func TestSAMLAssertionProcessing(t *tst.T) { { desc: "valid attributes, use first attribute found in Names", xml: ` - - - - old-soap@example.com - - - soap@example.com - - - -`, + + + + old-soap@example.com + + + soap@example.com + + + + `, mapping: models.SAMLAttributeMapping{ Keys: map[string]models.SAMLAttribute{ "email": { @@ -210,18 +210,18 @@ func TestSAMLAssertionProcessing(t *tst.T) { { desc: "valid groups attribute", xml: ` - - - - group1 - group2 - - - soap@example.com - - - -`, + + + + group1 + group2 + + + soap@example.com + + + + `, mapping: models.SAMLAttributeMapping{ Keys: map[string]models.SAMLAttribute{ "email": { @@ -249,7 +249,7 @@ func TestSAMLAssertionProcessing(t *tst.T) { xml: ` - + someone@example.com @@ -261,7 +261,6 @@ func TestSAMLAssertionProcessing(t *tst.T) { Name: "mail", }, "role": { - Name: "role", Default: "member", }, }, @@ -274,17 +273,17 @@ func TestSAMLAssertionProcessing(t *tst.T) { { desc: "use default value even if attribute exists but is not specified in mapping", xml: ` - - - - someone@example.com - - - admin - - - -`, + + + + someone@example.com + + + admin + + + + `, mapping: models.SAMLAttributeMapping{ Keys: map[string]models.SAMLAttribute{ "email": { @@ -303,17 +302,17 @@ func TestSAMLAssertionProcessing(t *tst.T) { { desc: "use value in XML when attribute exists and is specified in mapping", xml: ` - - - - someone@example.com - - - admin - - - -`, + + + + someone@example.com + + + admin + + + + `, mapping: models.SAMLAttributeMapping{ Keys: map[string]models.SAMLAttribute{ "email": { @@ -333,15 +332,16 @@ func TestSAMLAssertionProcessing(t *tst.T) { } for i, example := range examples { - rawAssertion := saml.Assertion{} - require.NoError(t, xml.Unmarshal([]byte(example.xml), &rawAssertion)) - - assertion := SAMLAssertion{ - &rawAssertion, - } + t.Run(example.desc, func(t *tst.T) { + rawAssertion := saml.Assertion{} + require.NoError(t, xml.Unmarshal([]byte(example.xml), &rawAssertion)) - result := assertion.Process(example.mapping) + assertion := SAMLAssertion{ + &rawAssertion, + } - require.Equal(t, example.expected, result, "example %d had different processing", i) + result := assertion.Process(example.mapping) + require.Equal(t, example.expected, result, "example %d had different processing", i) + }) } } From fe86829e5da355d00aef3d0b90efb375351afe23 Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Thu, 11 Jul 2024 16:11:48 -0700 Subject: [PATCH 2/4] fix: assertion mapping should be case-insensitive --- internal/api/samlassertion.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/api/samlassertion.go b/internal/api/samlassertion.go index 58f210db58..fdf9323851 100644 --- a/internal/api/samlassertion.go +++ b/internal/api/samlassertion.go @@ -24,8 +24,7 @@ func (a *SAMLAssertion) Attribute(name string) []saml.AttributeValue { for _, stmt := range a.AttributeStatements { for _, attr := range stmt.Attributes { - // TODO: maybe this should be case-insentivite equality? - if attr.Name == name || attr.FriendlyName == name { + if strings.EqualFold(attr.Name, name) || strings.EqualFold(attr.FriendlyName, name) { values = append(values, attr.Values...) } } From f6666f679a1af1dcc373e2f3fcd8bded04f95295 Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Thu, 11 Jul 2024 16:11:59 -0700 Subject: [PATCH 3/4] chore: fix tests --- internal/api/samlassertion_test.go | 298 ++++++++++++++--------------- 1 file changed, 149 insertions(+), 149 deletions(-) diff --git a/internal/api/samlassertion_test.go b/internal/api/samlassertion_test.go index 2d4f6e3626..816ccf459d 100644 --- a/internal/api/samlassertion_test.go +++ b/internal/api/samlassertion_test.go @@ -157,99 +157,99 @@ func TestSAMLAssertionProcessing(t *tst.T) { } examples := []spec{ + // { + // desc: "valid attribute and mapping", + // xml: ` + // + // + // + // someone@example.com + // + // + // + // `, + // mapping: models.SAMLAttributeMapping{ + // Keys: map[string]models.SAMLAttribute{ + // "email": { + // Name: "mail", + // }, + // }, + // }, + // expected: map[string]interface{}{ + // "email": "someone@example.com", + // }, + // }, + // { + // desc: "valid attributes, use first attribute found in Names", + // xml: ` + // + // + // + // old-soap@example.com + // + // + // soap@example.com + // + // + // + // `, + // mapping: models.SAMLAttributeMapping{ + // Keys: map[string]models.SAMLAttribute{ + // "email": { + // Names: []string{ + // "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress", + // "http://schemas.xmlsoap.org/claims/EmailAddress", + // }, + // }, + // }, + // }, + // expected: map[string]interface{}{ + // "email": "soap@example.com", + // }, + // }, + // { + // desc: "valid groups attribute", + // xml: ` + // + // + // + // group1 + // group2 + // + // + // soap@example.com + // + // + // + // `, + // mapping: models.SAMLAttributeMapping{ + // Keys: map[string]models.SAMLAttribute{ + // "email": { + // Names: []string{ + // "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress", + // "http://schemas.xmlsoap.org/claims/EmailAddress", + // }, + // }, + // "groups": { + // Name: "groups", + // Array: true, + // }, + // }, + // }, + // expected: map[string]interface{}{ + // "email": "soap@example.com", + // "groups": []string{ + // "group1", + // "group2", + // }, + // }, + // }, { - desc: "valid attribute and mapping", - xml: ` - - - - someone@example.com - - - - `, - mapping: models.SAMLAttributeMapping{ - Keys: map[string]models.SAMLAttribute{ - "email": { - Name: "mail", - }, - }, - }, - expected: map[string]interface{}{ - "email": "someone@example.com", - }, - }, - { - desc: "valid attributes, use first attribute found in Names", - xml: ` - - - - old-soap@example.com - - - soap@example.com - - - - `, - mapping: models.SAMLAttributeMapping{ - Keys: map[string]models.SAMLAttribute{ - "email": { - Names: []string{ - "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress", - "http://schemas.xmlsoap.org/claims/EmailAddress", - }, - }, - }, - }, - expected: map[string]interface{}{ - "email": "soap@example.com", - }, - }, - { - desc: "valid groups attribute", - xml: ` - - - - group1 - group2 - - - soap@example.com - - - - `, - mapping: models.SAMLAttributeMapping{ - Keys: map[string]models.SAMLAttribute{ - "email": { - Names: []string{ - "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress", - "http://schemas.xmlsoap.org/claims/EmailAddress", - }, - }, - "groups": { - Name: "groups", - Array: true, - }, - }, - }, - expected: map[string]interface{}{ - "email": "soap@example.com", - "groups": []string{ - "group1", - "group2", - }, - }, - }, - { - desc: "missing attribute, use default value", + desc: "missing attribute use default value", xml: ` - + someone@example.com @@ -258,7 +258,7 @@ func TestSAMLAssertionProcessing(t *tst.T) { mapping: models.SAMLAttributeMapping{ Keys: map[string]models.SAMLAttribute{ "email": { - Name: "mail", + Name: "email", }, "role": { Default: "member", @@ -270,65 +270,65 @@ func TestSAMLAssertionProcessing(t *tst.T) { "role": "member", }, }, - { - desc: "use default value even if attribute exists but is not specified in mapping", - xml: ` - - - - someone@example.com - - - admin - - - - `, - mapping: models.SAMLAttributeMapping{ - Keys: map[string]models.SAMLAttribute{ - "email": { - Name: "mail", - }, - "role": { - Default: "member", - }, - }, - }, - expected: map[string]interface{}{ - "email": "someone@example.com", - "role": "member", - }, - }, - { - desc: "use value in XML when attribute exists and is specified in mapping", - xml: ` - - - - someone@example.com - - - admin - - - - `, - mapping: models.SAMLAttributeMapping{ - Keys: map[string]models.SAMLAttribute{ - "email": { - Name: "mail", - }, - "role": { - Name: "role", - Default: "member", - }, - }, - }, - expected: map[string]interface{}{ - "email": "someone@example.com", - "role": "admin", - }, - }, + // { + // desc: "use default value even if attribute exists but is not specified in mapping", + // xml: ` + // + // + // + // someone@example.com + // + // + // admin + // + // + // + // `, + // mapping: models.SAMLAttributeMapping{ + // Keys: map[string]models.SAMLAttribute{ + // "email": { + // Name: "mail", + // }, + // "role": { + // Default: "member", + // }, + // }, + // }, + // expected: map[string]interface{}{ + // "email": "someone@example.com", + // "role": "member", + // }, + // }, + // { + // desc: "use value in XML when attribute exists and is specified in mapping", + // xml: ` + // + // + // + // someone@example.com + // + // + // admin + // + // + // + // `, + // mapping: models.SAMLAttributeMapping{ + // Keys: map[string]models.SAMLAttribute{ + // "email": { + // Name: "mail", + // }, + // "role": { + // Name: "role", + // Default: "member", + // }, + // }, + // }, + // expected: map[string]interface{}{ + // "email": "someone@example.com", + // "role": "admin", + // }, + // }, } for i, example := range examples { From d0e3b914314fa1461783a09e775fd70be7d9200c Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Fri, 12 Jul 2024 02:13:56 -0700 Subject: [PATCH 4/4] chore: uncomment test --- internal/api/samlassertion_test.go | 292 ++++++++++++++--------------- 1 file changed, 146 insertions(+), 146 deletions(-) diff --git a/internal/api/samlassertion_test.go b/internal/api/samlassertion_test.go index 816ccf459d..b7461b26d9 100644 --- a/internal/api/samlassertion_test.go +++ b/internal/api/samlassertion_test.go @@ -157,93 +157,93 @@ func TestSAMLAssertionProcessing(t *tst.T) { } examples := []spec{ - // { - // desc: "valid attribute and mapping", - // xml: ` - // - // - // - // someone@example.com - // - // - // - // `, - // mapping: models.SAMLAttributeMapping{ - // Keys: map[string]models.SAMLAttribute{ - // "email": { - // Name: "mail", - // }, - // }, - // }, - // expected: map[string]interface{}{ - // "email": "someone@example.com", - // }, - // }, - // { - // desc: "valid attributes, use first attribute found in Names", - // xml: ` - // - // - // - // old-soap@example.com - // - // - // soap@example.com - // - // - // - // `, - // mapping: models.SAMLAttributeMapping{ - // Keys: map[string]models.SAMLAttribute{ - // "email": { - // Names: []string{ - // "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress", - // "http://schemas.xmlsoap.org/claims/EmailAddress", - // }, - // }, - // }, - // }, - // expected: map[string]interface{}{ - // "email": "soap@example.com", - // }, - // }, - // { - // desc: "valid groups attribute", - // xml: ` - // - // - // - // group1 - // group2 - // - // - // soap@example.com - // - // - // - // `, - // mapping: models.SAMLAttributeMapping{ - // Keys: map[string]models.SAMLAttribute{ - // "email": { - // Names: []string{ - // "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress", - // "http://schemas.xmlsoap.org/claims/EmailAddress", - // }, - // }, - // "groups": { - // Name: "groups", - // Array: true, - // }, - // }, - // }, - // expected: map[string]interface{}{ - // "email": "soap@example.com", - // "groups": []string{ - // "group1", - // "group2", - // }, - // }, - // }, + { + desc: "valid attribute and mapping", + xml: ` + + + + someone@example.com + + + + `, + mapping: models.SAMLAttributeMapping{ + Keys: map[string]models.SAMLAttribute{ + "email": { + Name: "mail", + }, + }, + }, + expected: map[string]interface{}{ + "email": "someone@example.com", + }, + }, + { + desc: "valid attributes, use first attribute found in Names", + xml: ` + + + + old-soap@example.com + + + soap@example.com + + + + `, + mapping: models.SAMLAttributeMapping{ + Keys: map[string]models.SAMLAttribute{ + "email": { + Names: []string{ + "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress", + "http://schemas.xmlsoap.org/claims/EmailAddress", + }, + }, + }, + }, + expected: map[string]interface{}{ + "email": "soap@example.com", + }, + }, + { + desc: "valid groups attribute", + xml: ` + + + + group1 + group2 + + + soap@example.com + + + + `, + mapping: models.SAMLAttributeMapping{ + Keys: map[string]models.SAMLAttribute{ + "email": { + Names: []string{ + "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress", + "http://schemas.xmlsoap.org/claims/EmailAddress", + }, + }, + "groups": { + Name: "groups", + Array: true, + }, + }, + }, + expected: map[string]interface{}{ + "email": "soap@example.com", + "groups": []string{ + "group1", + "group2", + }, + }, + }, { desc: "missing attribute use default value", xml: ` @@ -270,65 +270,65 @@ func TestSAMLAssertionProcessing(t *tst.T) { "role": "member", }, }, - // { - // desc: "use default value even if attribute exists but is not specified in mapping", - // xml: ` - // - // - // - // someone@example.com - // - // - // admin - // - // - // - // `, - // mapping: models.SAMLAttributeMapping{ - // Keys: map[string]models.SAMLAttribute{ - // "email": { - // Name: "mail", - // }, - // "role": { - // Default: "member", - // }, - // }, - // }, - // expected: map[string]interface{}{ - // "email": "someone@example.com", - // "role": "member", - // }, - // }, - // { - // desc: "use value in XML when attribute exists and is specified in mapping", - // xml: ` - // - // - // - // someone@example.com - // - // - // admin - // - // - // - // `, - // mapping: models.SAMLAttributeMapping{ - // Keys: map[string]models.SAMLAttribute{ - // "email": { - // Name: "mail", - // }, - // "role": { - // Name: "role", - // Default: "member", - // }, - // }, - // }, - // expected: map[string]interface{}{ - // "email": "someone@example.com", - // "role": "admin", - // }, - // }, + { + desc: "use default value even if attribute exists but is not specified in mapping", + xml: ` + + + + someone@example.com + + + admin + + + + `, + mapping: models.SAMLAttributeMapping{ + Keys: map[string]models.SAMLAttribute{ + "email": { + Name: "mail", + }, + "role": { + Default: "member", + }, + }, + }, + expected: map[string]interface{}{ + "email": "someone@example.com", + "role": "member", + }, + }, + { + desc: "use value in XML when attribute exists and is specified in mapping", + xml: ` + + + + someone@example.com + + + admin + + + + `, + mapping: models.SAMLAttributeMapping{ + Keys: map[string]models.SAMLAttribute{ + "email": { + Name: "mail", + }, + "role": { + Name: "role", + Default: "member", + }, + }, + }, + expected: map[string]interface{}{ + "email": "someone@example.com", + "role": "admin", + }, + }, } for i, example := range examples {