Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: Conflicting profile properties between profile and attributes #593

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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
13 changes: 11 additions & 2 deletions src/node-saml/saml.ts
Expand Up @@ -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)) {
cjbarth marked this conversation as resolved.
Show resolved Hide resolved
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);
}
});
}
Expand Down
33 changes: 32 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,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