Skip to content

Commit

Permalink
fix: attrValueMapper fails to parse complex AttributeValue tags (#245)
Browse files Browse the repository at this point in the history
This fixes an issue where the `attrValueMapper` would fail to properly
map the value for complex `AttributeValue` tags. This handles the case
where the `AttributeValue` contains a nested `NameID` tag.

One such example is the `eduPersonTargetedID` that is used as an
identifier in [eduGAIN][1] which can return an Attribute of the form

```xml
<saml2:Attribute FriendlyName="eduPersonTargetedID" Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.10" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri">
  <saml2:AttributeValue>
    <saml2:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" NameQualifier="https://idp.example-university.fr/idp/shibboleth" SPNameQualifier="https://www.service-provider.com/shibboleth">a6c2c4d4-08b9-4ca7-8ff9-43d83e6e1d35</saml2:NameID>
  </saml2:AttributeValue>
</saml2:Attribute>
```

Note that in reality, the `AttributeValue` tags can be much more complex.
[The Assertions and Protocols for the OASIS Security Assertion Markup
Language (SAML) V2.0][2] uses the following schema for the `Attribute`
tag:

```xml
<element name="Attribute" type="saml:AttributeType"/>
<complexType name="AttributeType">
  <sequence>
    <element ref="saml:AttributeValue" minOccurs="0" maxOccurs="unbounded"/>
  </sequence>
  <attribute name="Name" type="string" use="required"/>
  <attribute name="NameFormat" type="anyURI" use="optional"/>
  <attribute name="FriendlyName" type="string" use="optional"/>
  <anyAttribute namespace="##other" processContents="lax"/>
</complexType>
```

and the following schema for the `AttributeValue`:

```xml
<element name="AttributeValue" type="anyType" nillable="true"/>
```

which means it can take any type.

As pointed in [3], it is customary to use `NameQualifier` and the
`SPNameQualifier` in addition to the actual value to create a unique
identifier for the platform. That is why the `AttributeValue` is mapped
to an object containing the attribute of the `NameID` tag as well as
the string value for `eduPersonTargetedID` that is stored in the `Value`
property.

[1]: https://wiki.geant.org/display/eduGAIN/Identifier+Attributes
[2]: https://docs.oasis-open.org/security/saml/v2.0/saml-core-2.0-os.pdf
[3]: #245 (comment)
  • Loading branch information
dfdeagle47 committed Mar 9, 2020
1 parent fb1bda0 commit 96a0edd
Show file tree
Hide file tree
Showing 3 changed files with 164 additions and 6 deletions.
20 changes: 14 additions & 6 deletions lib/passport-saml/saml.js
Original file line number Diff line number Diff line change
Expand Up @@ -1008,10 +1008,6 @@ SAML.prototype.processValidlySignedAssertion = function(xml, samlResponseXml, in
.map(attr => attr.Attribute)
);

var attrValueMapper = function(value) {
return typeof value === 'string' ? value : value._;
};

if (attributes) {
attributes.forEach(attribute => {
if(!Object.prototype.hasOwnProperty.call(attribute, 'AttributeValue')) {
Expand All @@ -1020,9 +1016,9 @@ SAML.prototype.processValidlySignedAssertion = function(xml, samlResponseXml, in
}
var value = attribute.AttributeValue;
if (value.length === 1) {
profile[attribute.$.Name] = attrValueMapper(value[0]);
profile[attribute.$.Name] = self.attributeValueMapper(value[0]);
} else {
profile[attribute.$.Name] = value.map(attrValueMapper);
profile[attribute.$.Name] = value.map(self.attributeValueMapper);
}
});
}
Expand Down Expand Up @@ -1305,4 +1301,16 @@ SAML.prototype.keyToPEM = function (key) {
return wrappedKey;
};

SAML.prototype.attributeValueMapper = function (value) {
if(typeof value === 'string') {
return value;
} else if (typeof value._ === 'string') {
return value._;
} else if (value.NameID && value.NameID[0]) {
return Object.assign({
Value: value.NameID[0]._
}, value.NameID[0].$ || {});
}
};

exports.SAML = SAML;
109 changes: 109 additions & 0 deletions test/static/attributes-values-sample.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
module.exports = [
{
$: {
FriendlyName: "mail",
Name: "urn:oid:0.9.2342.19200300.100.1.3",
NameFormat: "urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
},
AttributeValue: [
{
_: "example-user@example-university.edu",
$: {
"xmlns:xsd": "http://www.w3.org/2001/XMLSchema",
"xmlns:xsi": "http://www.w3.org/2001/XMLSchema-instance",
"xsi:type": "xsd:string"
}
}
]
},
{
$: {
FriendlyName: "eduPersonTargetedID",
Name: "urn:oid:1.3.6.1.4.1.5923.1.1.1.10",
NameFormat: "urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
},
AttributeValue: [
{
NameID: [
{
_: "SBJMMcDv00BWSefyNqumyK0A+Jb=",
$: {
Format: "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent",
NameQualifier:
"https://idp.example-university.edu/idp/shibboleth",
SPNameQualifier: "https://www.example-service-provider.com/entity"
}
}
]
}
]
},
{
$: {
FriendlyName: "eduPersonPrimaryAffiliation",
Name: "urn:oid:1.3.6.1.4.1.5923.1.1.1.5",
NameFormat: "urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
},
AttributeValue: [
{
_: "staff",
$: {
"xmlns:xsd": "http://www.w3.org/2001/XMLSchema",
"xmlns:xsi": "http://www.w3.org/2001/XMLSchema-instance",
"xsi:type": "xsd:string"
}
}
]
},
{
$: {
FriendlyName: "displayName",
Name: "urn:oid:2.16.840.1.113730.3.1.241",
NameFormat: "urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
},
AttributeValue: [
{
_: "Smith John",
$: {
"xmlns:xsd": "http://www.w3.org/2001/XMLSchema",
"xmlns:xsi": "http://www.w3.org/2001/XMLSchema-instance",
"xsi:type": "xsd:string"
}
}
]
},
{
$: {
FriendlyName: "givenName",
Name: "urn:oid:2.5.4.42",
NameFormat: "urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
},
AttributeValue: [
{
_: "John",
$: {
"xmlns:xsd": "http://www.w3.org/2001/XMLSchema",
"xmlns:xsi": "http://www.w3.org/2001/XMLSchema-instance",
"xsi:type": "xsd:string"
}
}
]
},
{
$: {
FriendlyName: "surname",
Name: "urn:oid:2.5.4.4",
NameFormat: "urn:oasis:names:tc:SAML:2.0:attrname-format:uri"
},
AttributeValue: [
{
_: "Smith",
$: {
"xmlns:xsd": "http://www.w3.org/2001/XMLSchema",
"xmlns:xsi": "http://www.w3.org/2001/XMLSchema-instance",
"xsi:type": "xsd:string"
}
}
]
}
];
41 changes: 41 additions & 0 deletions test/tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ var parseString = require( 'xml2js' ).parseString;
var SAML = require( '../lib/passport-saml/index.js' ).SAML;
var fs = require( 'fs' );
var sinon = require('sinon');
var attributes = require( './static/attributes-values-sample.js' );

// a certificate which is re-used by several tests
var TEST_CERT = 'MIIEFzCCAv+gAwIBAgIUFJsUjPM7AmWvNtEvULSHlTTMiLQwDQYJKoZIhvcNAQEFBQAwWDELMAkGA1UEBhMCVVMxETAPBgNVBAoMCFN1YnNwYWNlMRUwEwYDVQQLDAxPbmVMb2dpbiBJZFAxHzAdBgNVBAMMFk9uZUxvZ2luIEFjY291bnQgNDIzNDkwHhcNMTQwNTEzMTgwNjEyWhcNMTkwNTE0MTgwNjEyWjBYMQswCQYDVQQGEwJVUzERMA8GA1UECgwIU3Vic3BhY2UxFTATBgNVBAsMDE9uZUxvZ2luIElkUDEfMB0GA1UEAwwWT25lTG9naW4gQWNjb3VudCA0MjM0OTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAKrAzJdY9FzFLt5blArJfPzgi87EnFGlTfcV5T1TUDwLBlDkY/0ZGKnMOpf3D7ie2C4pPFOImOogcM5kpDDL7qxTXZ1ewXVyjBdMu29NG2C6NzWeQTUMUji01EcHkC8o+Pts8ANiNOYcjxEeyhEyzJKgEizblYzMMKzdrOET6QuqWo3C83K+5+5dsjDn1ooKGRwj3HvgsYcFrQl9NojgQFjoobwsiE/7A+OJhLpBcy/nSVgnoJaMfrO+JsnukZPztbntLvOl56+Vra0N8n5NAYhaSayPiv/ayhjVgjfXd1tjMVTOiDknUOwizZuJ1Y3QH94vUtBgp0WBpBSs/xMyTs8CAwEAAaOB2DCB1TAMBgNVHRMBAf8EAjAAMB0GA1UdDgQWBBRQO4WpM5fWwxib49WTuJkfYDbxODCBlQYDVR0jBIGNMIGKgBRQO4WpM5fWwxib49WTuJkfYDbxOKFcpFowWDELMAkGA1UEBhMCVVMxETAPBgNVBAoMCFN1YnNwYWNlMRUwEwYDVQQLDAxPbmVMb2dpbiBJZFAxHzAdBgNVBAMMFk9uZUxvZ2luIEFjY291bnQgNDIzNDmCFBSbFIzzOwJlrzbRL1C0h5U0zIi0MA4GA1UdDwEB/wQEAwIHgDANBgkqhkiG9w0BAQUFAAOCAQEACdDAAoaZFCEY5pmfwbKuKrXtO5iE8lWtiCPjCZEUuT6bXRNcqrdnuV/EAfX9WQoXjalPi0eM78zKmbvRGSTUHwWw49RHjFfeJUKvHNeNnFgTXDjEPNhMvh69kHm453lFRmB+kk6yjtXRZaQEwS8Uuo2Ot+krgNbl6oTBZJ0AHH1MtZECDloms1Km7zsK8wAi5i8TVIKkVr5b2VlhrLgFMvzZ5ViAxIMGB6w47yY4QGQB/5Q8ya9hBs9vkn+wubA+yr4j14JXZ7blVKDSTYva65Ea+PqHyrp+Wnmnbw2ObS7iWexiTy1jD3G0R2avDBFjM8Fj5DbfufsE1b0U10RTtg==';
Expand Down Expand Up @@ -2497,5 +2498,45 @@ describe( 'passport-saml /', function() {
});
});
});

describe('attributeValueMapper', function(done) {
var samlObj;
beforeEach(function() {
samlObj = new SAML();
});

it('correctly maps the attribute values', function(done) {
var profile = {};

attributes.forEach(attribute => {
if(!Object.prototype.hasOwnProperty.call(attribute, 'AttributeValue')) {
// if attributes has no AttributeValue child, continue
return;
}
var value = attribute.AttributeValue;
if (value.length === 1) {
profile[attribute.$.Name] = samlObj.attributeValueMapper(value[0]);
} else {
profile[attribute.$.Name] = value.map(samlObj.attributeValueMapper);
}
});

profile.should.eql({
'urn:oid:0.9.2342.19200300.100.1.3': 'example-user@example-university.edu',
'urn:oid:1.3.6.1.4.1.5923.1.1.1.10': {
Format: 'urn:oasis:names:tc:SAML:2.0:nameid-format:persistent',
NameQualifier: 'https://idp.example-university.edu/idp/shibboleth',
SPNameQualifier: 'https://www.example-service-provider.com/entity',
Value: 'SBJMMcDv00BWSefyNqumyK0A+Jb='
},
'urn:oid:1.3.6.1.4.1.5923.1.1.1.5': 'staff',
'urn:oid:2.16.840.1.113730.3.1.241': 'Smith John',
'urn:oid:2.5.4.4': 'Smith',
'urn:oid:2.5.4.42': 'John'
});

done();
});
})
});
});

0 comments on commit 96a0edd

Please sign in to comment.