Skip to content

Commit

Permalink
fix: omit empty string from name & use case-insensitive equality for …
Browse files Browse the repository at this point in the history 
…comparing SAML attributes (supabase#1654)

## What kind of change does this PR introduce?
* Fixes an issue where the SAML mapping is incorrect when a `Default` is
specified in the mapping and the `FriendlyName` in the SAML Assertion is
an empty string
* Use case-insensitive equality for mapping SAML attributes
* Tested with a trial okta account

## What is the current behavior?
* During the SAML assertion process, the `SAMLAttributeMapping` struct
defaults the `Name` to an empty string. When combined with a SAML
assertion that has an empty string set in the `FriendlyName`, this
causes an incorrect mapping when a default mapping is provided. (see
[test
case](https://github.com/supabase/auth/pull/1654/files#diff-e708a1335880a92063e8b45f3a06eb96931ab0ee46b94cdf6bf5f566640f7a3cR247-R272)
* Use case-sensitive equality for mapping SAML attributes

Please link any relevant issues here.

## What is the new behavior?

Feel free to include screenshots if it includes visual changes.

## Additional context

Add any other context or screenshots.
  • Loading branch information
@kangmingtay @LashaJini
kangmingtay authored and LashaJini committed Nov 13, 2024
1 parent bf70492 commit 245b929
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 68 deletions.
8 changes: 5 additions & 3 deletions internal/api/samlassertion.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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...)
}
}
Expand Down Expand Up @@ -120,7 +119,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
Expand Down
130 changes: 65 additions & 65 deletions internal/api/samlassertion_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,14 +160,14 @@ func TestSAMLAssertionProcessing(t *tst.T) {
{
desc: "valid attribute and mapping",
xml: `<?xml version="1.0" encoding="UTF-8"?>
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsd="http://www.w3.org/2001/XMLSchema" ID="_72591c79da230cac1457d0ea0f2771ab" IssueInstant="2022-08-11T14:53:38.260Z" Version="2.0">
<saml2:AttributeStatement>
<saml2:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">someone@example.com</saml2:AttributeValue>
</saml2:Attribute>
</saml2:AttributeStatement>
</saml2:Assertion>
`,
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsd="http://www.w3.org/2001/XMLSchema" ID="_72591c79da230cac1457d0ea0f2771ab" IssueInstant="2022-08-11T14:53:38.260Z" Version="2.0">
<saml2:AttributeStatement>
<saml2:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">someone@example.com</saml2:AttributeValue>
</saml2:Attribute>
</saml2:AttributeStatement>
</saml2:Assertion>
`,
mapping: models.SAMLAttributeMapping{
Keys: map[string]models.SAMLAttribute{
"email": {
Expand All @@ -182,17 +182,17 @@ func TestSAMLAssertionProcessing(t *tst.T) {
{
desc: "valid attributes, use first attribute found in Names",
xml: `<?xml version="1.0" encoding="UTF-8"?>
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsd="http://www.w3.org/2001/XMLSchema" ID="_72591c79da230cac1457d0ea0f2771ab" IssueInstant="2022-08-11T14:53:38.260Z" Version="2.0">
<saml2:AttributeStatement>
<saml2:Attribute Name="http://schemas.xmlsoap.org/claims/EmailAddress" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">old-soap@example.com</saml2:AttributeValue>
</saml2:Attribute>
<saml2:Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">soap@example.com</saml2:AttributeValue>
</saml2:Attribute>
</saml2:AttributeStatement>
</saml2:Assertion>
`,
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsd="http://www.w3.org/2001/XMLSchema" ID="_72591c79da230cac1457d0ea0f2771ab" IssueInstant="2022-08-11T14:53:38.260Z" Version="2.0">
<saml2:AttributeStatement>
<saml2:Attribute Name="http://schemas.xmlsoap.org/claims/EmailAddress" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">old-soap@example.com</saml2:AttributeValue>
</saml2:Attribute>
<saml2:Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">soap@example.com</saml2:AttributeValue>
</saml2:Attribute>
</saml2:AttributeStatement>
</saml2:Assertion>
`,
mapping: models.SAMLAttributeMapping{
Keys: map[string]models.SAMLAttribute{
"email": {
Expand All @@ -210,18 +210,18 @@ func TestSAMLAssertionProcessing(t *tst.T) {
{
desc: "valid groups attribute",
xml: `<?xml version="1.0" encoding="UTF-8"?>
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsd="http://www.w3.org/2001/XMLSchema" ID="_72591c79da230cac1457d0ea0f2771ab" IssueInstant="2022-08-11T14:53:38.260Z" Version="2.0">
<saml2:AttributeStatement>
<saml2:Attribute Name="http://whatever.com/groups" FriendlyName="groups" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:string">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">group1</saml2:AttributeValue>
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">group2</saml2:AttributeValue>
</saml2:Attribute>
<saml2:Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">soap@example.com</saml2:AttributeValue>
</saml2:Attribute>
</saml2:AttributeStatement>
</saml2:Assertion>
`,
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsd="http://www.w3.org/2001/XMLSchema" ID="_72591c79da230cac1457d0ea0f2771ab" IssueInstant="2022-08-11T14:53:38.260Z" Version="2.0">
<saml2:AttributeStatement>
<saml2:Attribute Name="http://whatever.com/groups" FriendlyName="groups" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:string">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">group1</saml2:AttributeValue>
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">group2</saml2:AttributeValue>
</saml2:Attribute>
<saml2:Attribute Name="http://schemas.xmlsoap.org/ws/2005/05/identity/claims/emailaddress" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">soap@example.com</saml2:AttributeValue>
</saml2:Attribute>
</saml2:AttributeStatement>
</saml2:Assertion>
`,
mapping: models.SAMLAttributeMapping{
Keys: map[string]models.SAMLAttribute{
"email": {
Expand All @@ -245,11 +245,11 @@ func TestSAMLAssertionProcessing(t *tst.T) {
},
},
{
desc: "missing attribute, use default value",
desc: "missing attribute use default value",
xml: `<?xml version="1.0" encoding="UTF-8"?>
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsd="http://www.w3.org/2001/XMLSchema" ID="_72591c79da230cac1457d0ea0f2771ab" IssueInstant="2022-08-11T14:53:38.260Z" Version="2.0">
<saml2:AttributeStatement>
<saml2:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:Attribute Name="email" FriendlyName="" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">someone@example.com</saml2:AttributeValue>
</saml2:Attribute>
</saml2:AttributeStatement>
Expand All @@ -258,10 +258,9 @@ func TestSAMLAssertionProcessing(t *tst.T) {
mapping: models.SAMLAttributeMapping{
Keys: map[string]models.SAMLAttribute{
"email": {
Name: "mail",
Name: "email",
},
"role": {
Name: "role",
Default: "member",
},
},
Expand All @@ -274,17 +273,17 @@ func TestSAMLAssertionProcessing(t *tst.T) {
{
desc: "use default value even if attribute exists but is not specified in mapping",
xml: `<?xml version="1.0" encoding="UTF-8"?>
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsd="http://www.w3.org/2001/XMLSchema" ID="_72591c79da230cac1457d0ea0f2771ab" IssueInstant="2022-08-11T14:53:38.260Z" Version="2.0">
<saml2:AttributeStatement>
<saml2:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">someone@example.com</saml2:AttributeValue>
</saml2:Attribute>
<saml2:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" FriendlyName="role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">admin</saml2:AttributeValue>
</saml2:Attribute>
</saml2:AttributeStatement>
</saml2:Assertion>
`,
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsd="http://www.w3.org/2001/XMLSchema" ID="_72591c79da230cac1457d0ea0f2771ab" IssueInstant="2022-08-11T14:53:38.260Z" Version="2.0">
<saml2:AttributeStatement>
<saml2:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">someone@example.com</saml2:AttributeValue>
</saml2:Attribute>
<saml2:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" FriendlyName="role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">admin</saml2:AttributeValue>
</saml2:Attribute>
</saml2:AttributeStatement>
</saml2:Assertion>
`,
mapping: models.SAMLAttributeMapping{
Keys: map[string]models.SAMLAttribute{
"email": {
Expand All @@ -303,17 +302,17 @@ func TestSAMLAssertionProcessing(t *tst.T) {
{
desc: "use value in XML when attribute exists and is specified in mapping",
xml: `<?xml version="1.0" encoding="UTF-8"?>
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsd="http://www.w3.org/2001/XMLSchema" ID="_72591c79da230cac1457d0ea0f2771ab" IssueInstant="2022-08-11T14:53:38.260Z" Version="2.0">
<saml2:AttributeStatement>
<saml2:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">someone@example.com</saml2:AttributeValue>
</saml2:Attribute>
<saml2:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" FriendlyName="role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">admin</saml2:AttributeValue>
</saml2:Attribute>
</saml2:AttributeStatement>
</saml2:Assertion>
`,
<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:xsd="http://www.w3.org/2001/XMLSchema" ID="_72591c79da230cac1457d0ea0f2771ab" IssueInstant="2022-08-11T14:53:38.260Z" Version="2.0">
<saml2:AttributeStatement>
<saml2:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" FriendlyName="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">someone@example.com</saml2:AttributeValue>
</saml2:Attribute>
<saml2:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" FriendlyName="role" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
<saml2:AttributeValue xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xsd:string">admin</saml2:AttributeValue>
</saml2:Attribute>
</saml2:AttributeStatement>
</saml2:Assertion>
`,
mapping: models.SAMLAttributeMapping{
Keys: map[string]models.SAMLAttribute{
"email": {
Expand All @@ -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)
})
}
}

0 comments on commit 245b929

Please sign in to comment.