Fix: Conflicting profile properties between profile and attributes (n…

…ode-saml#543)
kriss1897 · May 15, 2021 · d17e55a · d17e55a
1 parent 2d2cc0a
commit d17e55a
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 4 deletions.
diff --git a/.gitignore b/.gitignore
@@ -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
diff --git a/src/node-saml/saml.ts b/src/node-saml/saml.ts
@@ -1175,11 +1175,20 @@ class SAML {
             // if attributes has no AttributeValue child, continue
             return;
           }
+          const name = attribute.$.Name;
           const value = attribute.AttributeValue;
+
+          // 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;
+          }
+
           if (value.length === 1) {
-            profile[attribute.$.Name] = attrValueMapper(value[0]);
+            profile[name] = attrValueMapper(value[0]);
           } else {
-            profile[attribute.$.Name] = value.map(attrValueMapper);
+            profile[name] = value.map(attrValueMapper);
           }
         });
       }

diff --git a/test/node-saml/tests.spec.ts b/test/node-saml/tests.spec.ts
@@ -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,
       });
     });
 
@@ -1981,7 +1984,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"),