Skip to content

Commit

Permalink
Add assertion attributes to child object on profile (#593)
Browse files Browse the repository at this point in the history
* 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 c7b7226 commit 2a1699b
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 7 deletions.
5 changes: 4 additions & 1 deletion .gitignore
Expand Up @@ -5,4 +5,7 @@ node_modules/
yarn-error.log
.DS_Store
.eslintcache
.dir-locals.el
.dir-locals.el

## Local VS code settings and debug profiles
.vscode
25 changes: 20 additions & 5 deletions src/node-saml/saml.ts
Expand Up @@ -1170,18 +1170,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
29 changes: 28 additions & 1 deletion test/node-saml/tests.spec.ts
Expand Up @@ -1904,10 +1904,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 @@ -1981,7 +1984,31 @@ 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>" +
'<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");
should(profile!.attributes).containEql({ issuer: "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 2a1699b

Please sign in to comment.