Skip to content

Commit

Permalink
Add assertion attributes to child object on profile (passport-saml#54…
Browse files Browse the repository at this point in the history
…3) (#5)

* Fix: Conflicting profile properties between profile and attributes (#543)

* Add assertion attributes to child object on profile (#543)

This attributes are also mounted to profile directly in a non
conflicting way.

Co-authored-by: Shashank Singh Solanki <shashank.solanki@postman.com>
  • Loading branch information
kriss1897 and kriss1897 committed Jun 17, 2021
1 parent 6114dd2 commit b98c97b
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 6 deletions.
25 changes: 20 additions & 5 deletions src/saml.ts
Expand Up @@ -1167,18 +1167,33 @@ class SAML {
};

if (attributes) {
const profileAttributes: Record<string, unknown> = {};

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

const name = attribute.$.Name;
const value =
attribute.AttributeValue.length === 1
? attrValueMapper(attribute.AttributeValue[0])
: attribute.AttributeValue.map(attrValueMapper);

profileAttributes[name] = value;

// If any property is already present in profile and is also present
// in attributes, then skip the one from attributes. Handle this
// conflict gracefully without returning any error
if (Object.prototype.hasOwnProperty.call(profile, name)) {
return;
}

profile[name] = value;
});

profile.attributes = profileAttributes;
}
}

Expand Down
33 changes: 32 additions & 1 deletion test/tests.spec.ts
Expand Up @@ -1895,10 +1895,13 @@ describe("node-saml /", function () {
});
});
describe("validatePostRequest()", function () {
const signingKey: any = fs.readFileSync(__dirname + "/static/key.pem", "ascii");
const signingCert: any = fs.readFileSync(__dirname + "/static/cert.pem", "ascii");
let samlObj: SAML;

beforeEach(function () {
samlObj = new SAML({
cert: fs.readFileSync(__dirname + "/static/cert.pem", "ascii"),
cert: signingCert,
});
});

Expand Down Expand Up @@ -1972,7 +1975,35 @@ describe("node-saml /", function () {
sessionIndex: "1",
});
});

it("check conflicting profile fields with data from attributes", async () => {
const testSAMLObj = new SAML({ cert: signingCert, issuer: "okta" });
const xml =
'<Response xmlns="urn:oasis:names:tc:SAML:2.0:protocol" ID="response0">' +
'<saml2:Assertion xmlns:saml2="urn:oasis:names:tc:SAML:2.0:assertion" Version="2.0">' +
"<saml:Issuer>http://idp.example.com/metadata.php</saml:Issuer>" +
"<saml2:AttributeStatement>" +
'<saml2:Attribute Name="attributeName" ' +
'NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:unspecified">' +
'<saml2:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" ' +
'xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" ' +
'xsi:type="xs:string"/>' +
"</saml2:Attribute>" +
'<saml2:Attribute Name="issuer" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:basic">' +
'<saml2:AttributeValue xsi:type="xs:string">test</saml2:AttributeValue>' +
"</saml2:Attribute>" +
"</saml2:AttributeStatement>" +
"</saml2:Assertion>" +
"</Response>";
const signedXml = signXmlResponse(xml, { privateKey: signingKey });
const { profile } = await testSAMLObj.validatePostResponseAsync({
SAMLResponse: Buffer.from(signedXml).toString("base64"),
});

should(profile!.issuer).not.be.equal("test");
});
});

it("validatePostRequest errors for encrypted nameID with wrong decryptionPvk", async () => {
const samlObj = new SAML({
cert: fs.readFileSync(__dirname + "/static/cert.pem", "ascii"),
Expand Down

0 comments on commit b98c97b

Please sign in to comment.