From 2e8c5b6956af88ba444ec84b86815b1138640722 Mon Sep 17 00:00:00 2001 From: Shashank Singh Solanki Date: Fri, 21 May 2021 19:23:00 +0530 Subject: [PATCH 1/3] Add assertion attributes to child object on profile (passport-saml#543) This attributes are also mounted to profile directly in a non conflicting way. --- src/saml.ts | 25 ++++++++++++++++++++----- test/tests.spec.ts | 33 ++++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/saml.ts b/src/saml.ts index fa8de39a..4dbe9978 100644 --- a/src/saml.ts +++ b/src/saml.ts @@ -1167,18 +1167,33 @@ class SAML { }; if (attributes) { + const profileAttributes: Record = {}; + 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; } } diff --git a/test/tests.spec.ts b/test/tests.spec.ts index bedac6f4..d0422bff 100644 --- a/test/tests.spec.ts +++ b/test/tests.spec.ts @@ -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, }); }); @@ -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 = + '' + + '' + + "http://idp.example.com/metadata.php" + + "" + + '' + + '' + + "" + + '' + + 'test' + + "" + + "" + + "" + + ""; + 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"), From 9b1a4116c9dfe572f27cfca0be1f1c377fed31d8 Mon Sep 17 00:00:00 2001 From: Shashank Singh Solanki Date: Wed, 26 May 2021 23:51:32 +0530 Subject: [PATCH 2/3] Fix: Clear and re-install npm modules after npm update to overcome npm-cli bug --- .github/workflows/workflow.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index f33c49bc..6df46d09 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -24,7 +24,11 @@ jobs: - run: npm ci - run: npm test - run: npm ci - - run: npm --depth 9999 update + - run: npm update + # Remove the node_modules and re-install with updated package-lock.json + # because npm update command breaks some of the npm-cli packages + - run: rm -rf node_modules + - run: npm ci - run: npm test env: CI: true From fe43cf2dc4226b225f1ae3eb790b7d0172089d1d Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Thu, 17 Jun 2021 11:11:13 -0400 Subject: [PATCH 3/3] Revert "Fix: Clear and re-install npm modules after npm update to overcome npm-cli bug" This reverts commit 9b1a4116c9dfe572f27cfca0be1f1c377fed31d8. --- .github/workflows/workflow.yml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/workflow.yml b/.github/workflows/workflow.yml index 6df46d09..f33c49bc 100644 --- a/.github/workflows/workflow.yml +++ b/.github/workflows/workflow.yml @@ -24,11 +24,7 @@ jobs: - run: npm ci - run: npm test - run: npm ci - - run: npm update - # Remove the node_modules and re-install with updated package-lock.json - # because npm update command breaks some of the npm-cli packages - - run: rm -rf node_modules - - run: npm ci + - run: npm --depth 9999 update - run: npm test env: CI: true