From 442bb1c16cc4ba1c7a127d0a10e6b39d2c448b35 Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Tue, 18 Jul 2023 00:20:35 -0400 Subject: [PATCH 1/2] Enforce eslint `prefer-template` --- .eslintrc.json | 1 + src/c14n-canonicalization.ts | 12 +-- src/exclusive-canonicalization.ts | 14 ++- src/signed-xml.ts | 168 +++++++++++------------------- test/signature-unit-tests.spec.ts | 37 +++---- test/utils-tests.spec.ts | 5 +- 6 files changed, 88 insertions(+), 149 deletions(-) diff --git a/.eslintrc.json b/.eslintrc.json index 18ed0784..2e455300 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -28,6 +28,7 @@ "eqeqeq": ["error", "smart"], "no-var": "error", "prefer-const": "error", + "prefer-template": "error", "deprecation/deprecation": "warn", "@typescript-eslint/no-non-null-assertion": "error", "@typescript-eslint/no-unused-vars": "warn", diff --git a/src/c14n-canonicalization.ts b/src/c14n-canonicalization.ts index 03406145..550c6584 100644 --- a/src/c14n-canonicalization.ts +++ b/src/c14n-canonicalization.ts @@ -230,13 +230,11 @@ export class C14nCanonicalization implements CanonicalizationOrTransformationAlg } } - return ( - (isAfterDocument ? "\n" : "") + - "" + - (isBeforeDocument ? "\n" : "") - ); + const afterDocument = isAfterDocument ? "\n" : ""; + const beforeDocument = isBeforeDocument ? "\n" : ""; + const encodedText = utils.encodeSpecialCharactersInText(node.data); + + return `${afterDocument}${beforeDocument}`; } /** diff --git a/src/exclusive-canonicalization.ts b/src/exclusive-canonicalization.ts index 4f76c83a..b5a48acc 100644 --- a/src/exclusive-canonicalization.ts +++ b/src/exclusive-canonicalization.ts @@ -242,13 +242,11 @@ export class ExclusiveCanonicalization implements CanonicalizationOrTransformati } } - return ( - (isAfterDocument ? "\n" : "") + - "" + - (isBeforeDocument ? "\n" : "") - ); + const afterDocument = isAfterDocument ? "\n" : ""; + const beforeDocument = isBeforeDocument ? "\n" : ""; + const encodedText = utils.encodeSpecialCharactersInText(node.data); + + return `${afterDocument}${beforeDocument}`; } /** @@ -293,7 +291,7 @@ export class ExclusiveCanonicalization implements CanonicalizationOrTransformati if (prefix === ancestorNamespace.prefix) { node.setAttributeNS( "http://www.w3.org/2000/xmlns/", - "xmlns:" + prefix, + `xmlns:${prefix}`, ancestorNamespace.namespaceURI ); } diff --git a/src/signed-xml.ts b/src/signed-xml.ts index 13bf3d08..4253c2ea 100644 --- a/src/signed-xml.ts +++ b/src/signed-xml.ts @@ -180,7 +180,7 @@ export class SignedXml { return null; } - prefix = prefix ? prefix + ":" : ""; + prefix = prefix ? `${prefix}:` : ""; let x509Certs = ""; if (Buffer.isBuffer(publicCert)) { @@ -268,7 +268,7 @@ export class SignedXml { this.validateSignatureValue(doc, (err: Error | null, isValidSignature?: boolean) => { if (err) { this.validationErrors.push( - "invalid signature: the signature value " + this.signatureValue + " is incorrect" + `invalid signature: the signature value ${this.signatureValue} is incorrect` ); callback(err); } else { @@ -342,7 +342,7 @@ export class SignedXml { const res = signer.verifySignature(signedInfoCanon, key, this.signatureValue); if (res === false) { this.validationErrors.push( - "invalid signature: the signature value " + this.signatureValue + " is incorrect" + `invalid signature: the signature value ${this.signatureValue} is incorrect` ); } return res; @@ -367,7 +367,7 @@ export class SignedXml { if (algo) { return new algo(); } else { - throw new Error("signature algorithm '" + name + "' is not supported"); + throw new Error(`signature algorithm '${name}' is not supported`); } } @@ -376,7 +376,7 @@ export class SignedXml { if (algo) { return new algo(); } else { - throw new Error("canonicalization algorithm '" + name + "' is not supported"); + throw new Error(`canonicalization algorithm '${name}' is not supported`); } } @@ -385,7 +385,7 @@ export class SignedXml { if (algo) { return new algo(); } else { - throw new Error("hash algorithm '" + name + "' is not supported"); + throw new Error(`hash algorithm '${name}' is not supported`); } } @@ -425,9 +425,7 @@ export class SignedXml { // Note, we are using the last found element from the loop above if (!utils.isArrayHasLength(elem)) { this.validationErrors.push( - "invalid signature: the signature references an element with uri " + - ref.uri + - " but could not find such element in the xml" + `invalid signature: the signature references an element with uri ${ref.uri} but could not find such element in the xml` ); return false; } @@ -439,12 +437,7 @@ export class SignedXml { if (!utils.validateDigestValue(digest, ref.digestValue)) { this.validationErrors.push( - "invalid signature: for uri " + - ref.uri + - " calculated digest is " + - digest + - " but the xml to validate supplies digest " + - ref.digestValue + `invalid signature: for uri ${ref.uri} calculated digest is ${digest} but the xml to validate supplies digest ${ref.digestValue}` ); return false; @@ -525,23 +518,23 @@ export class SignedXml { loadReference(ref) { let nodes = utils.findChilds(ref, "DigestMethod"); if (nodes.length === 0) { - throw new Error("could not find DigestMethod in reference " + ref.toString()); + throw new Error(`could not find DigestMethod in reference ${ref.toString()}`); } const digestAlgoNode = nodes[0]; const attr = utils.findAttr(digestAlgoNode, "Algorithm"); if (!attr) { - throw new Error("could not find Algorithm attribute in node " + digestAlgoNode.toString()); + throw new Error(`could not find Algorithm attribute in node ${digestAlgoNode.toString()}`); } const digestAlgo = attr.value; nodes = utils.findChilds(ref, "DigestValue"); if (nodes.length === 0) { - throw new Error("could not find DigestValue node in reference " + ref.toString()); + throw new Error(`could not find DigestValue node in reference ${ref.toString()}`); } const firstChild = nodes[0].firstChild; if (!firstChild || !("data" in firstChild)) { - throw new Error("could not find the value of DigestValue in " + nodes[0].toString()); + throw new Error(`could not find the value of DigestValue in ${nodes[0].toString()}`); } const digestValue = firstChild.data; @@ -720,10 +713,9 @@ export class SignedXml { if (validActions.indexOf(location.action) === -1) { const err = new Error( - "location.action option has an invalid action: " + - location.action + - ", must be any of the following values: " + - validActions.join(", ") + `location.action option has an invalid action: ${ + location.action + }, must be any of the following values: ${validActions.join(", ")}` ); if (!callback) { throw err; @@ -735,38 +727,37 @@ export class SignedXml { // automatic insertion of `:` if (prefix) { - xmlNsAttr += ":" + prefix; - currentPrefix = prefix + ":"; + xmlNsAttr += `:${prefix}`; + currentPrefix = `${prefix}:`; } else { currentPrefix = ""; } Object.keys(attrs).forEach(function (name) { if (name !== "xmlns" && name !== xmlNsAttr) { - signatureAttrs.push(name + '="' + attrs[name] + '"'); + signatureAttrs.push(`${name}="${attrs[name]}"`); } }); // add the xml namespace attribute - signatureAttrs.push(xmlNsAttr + '="http://www.w3.org/2000/09/xmldsig#"'); + signatureAttrs.push(`${xmlNsAttr}="http://www.w3.org/2000/09/xmldsig#"`); - let signatureXml = "<" + currentPrefix + "Signature " + signatureAttrs.join(" ") + ">"; + let signatureXml = `<${currentPrefix}Signature ${signatureAttrs.join(" ")}>`; signatureXml += this.createSignedInfo(doc, prefix); signatureXml += this.getKeyInfo(prefix); - signatureXml += ""; + signatureXml += ``; this.originalXmlWithIds = doc.toString(); let existingPrefixesString = ""; Object.keys(existingPrefixes).forEach(function (key) { - existingPrefixesString += "xmlns:" + key + '="' + existingPrefixes[key] + '" '; + existingPrefixesString += `xmlns:${key}="${existingPrefixes[key]}" `; }); // A trick to remove the namespaces that already exist in the xml // This only works if the prefix and namespace match with those in the xml - const dummySignatureWrapper = - "" + signatureXml + ""; + const dummySignatureWrapper = `${signatureXml}`; const nodeXml = new Dom().parseFromString(dummySignatureWrapper); // Because we are using a dummy wrapper hack described above, we know there will be a `firstChild` @@ -777,7 +768,7 @@ export class SignedXml { if (!xpath.isNodeLike(referenceNode)) { const err2 = new Error( - "the following xpath cannot be used because it was not found: " + location.reference + `the following xpath cannot be used because it was not found: ${location.reference}` ); if (!callback) { throw err2; @@ -843,27 +834,21 @@ export class SignedXml { } getKeyInfo(prefix) { - let res = ""; - let currentPrefix; - - currentPrefix = prefix || ""; - currentPrefix = currentPrefix ? currentPrefix + ":" : currentPrefix; + const currentPrefix = prefix ? `${prefix}:` : ""; let keyInfoAttrs = ""; if (this.keyInfoAttributes) { Object.keys(this.keyInfoAttributes).forEach((name) => { - keyInfoAttrs += " " + name + '="' + this.keyInfoAttributes[name] + '"'; + keyInfoAttrs += ` ${name}="${this.keyInfoAttributes[name]}"`; }); } + const keyInfoContent = this.getKeyInfoContent({ publicCert: this.publicCert, prefix }); - if (keyInfoAttrs !== "" || keyInfoContent != null) { - res += "<" + currentPrefix + "KeyInfo" + keyInfoAttrs + ">"; - res += keyInfoContent; - res += ""; - return res; - } else { - return ""; + if (keyInfoAttrs || keyInfoContent) { + return `<${currentPrefix}KeyInfo${keyInfoAttrs}>${keyInfoContent}`; } + + return ""; } /** @@ -874,38 +859,35 @@ export class SignedXml { let res = ""; prefix = prefix || ""; - prefix = prefix ? prefix + ":" : prefix; + prefix = prefix ? `${prefix}:` : prefix; for (const ref of this.references) { const nodes = xpath.selectWithResolver(ref.xpath ?? "", doc, this.namespaceResolver); if (!utils.isArrayHasLength(nodes)) { throw new Error( - "the following xpath cannot be signed because it was not found: " + ref.xpath + `the following xpath cannot be signed because it was not found: ${ref.xpath}` ); } for (const node of nodes) { if (ref.isEmptyUri) { - res += "<" + prefix + 'Reference URI="">'; + res += `<${prefix}Reference URI="">`; } else { const id = this.ensureHasId(node); ref.uri = id; - res += "<" + prefix + 'Reference URI="#' + id + '">'; + res += `<${prefix}Reference URI="#${id}">`; } - res += "<" + prefix + "Transforms>"; + res += `<${prefix}Transforms>`; for (const trans of ref.transforms || []) { const transform = this.findCanonicalizationAlgorithm(trans); - res += "<" + prefix + 'Transform Algorithm="' + transform.getAlgorithmName() + '"'; + res += `<${prefix}Transform Algorithm="${transform.getAlgorithmName()}"`; if (utils.isArrayHasLength(ref.inclusiveNamespacesPrefixList)) { res += ">"; - res += - ''; - res += ""; + res += ``; + res += ``; } else { res += " />"; } @@ -915,24 +897,10 @@ export class SignedXml { const digestAlgorithm = this.findHashAlgorithm(ref.digestAlgorithm); res += - "" + - "<" + - prefix + - 'DigestMethod Algorithm="' + - digestAlgorithm.getAlgorithmName() + - '" />' + - "<" + - prefix + - "DigestValue>" + - digestAlgorithm.getHash(canonXml) + - "" + - ""; + `` + + `<${prefix}DigestMethod Algorithm="${digestAlgorithm.getAlgorithmName()}" />` + + `<${prefix}DigestValue>${digestAlgorithm.getHash(canonXml)}` + + ``; } } @@ -990,7 +958,7 @@ export class SignedXml { } //add the attribute - const id = "_" + this.id++; + const id = `_${this.id++}`; if (this.idMode === "wssecurity") { node.setAttributeNS( @@ -1020,31 +988,23 @@ export class SignedXml { let currentPrefix; currentPrefix = prefix || ""; - currentPrefix = currentPrefix ? currentPrefix + ":" : currentPrefix; - - let res = "<" + currentPrefix + "SignedInfo>"; - res += - "<" + - currentPrefix + - 'CanonicalizationMethod Algorithm="' + - transform.getAlgorithmName() + - '"'; + currentPrefix = currentPrefix ? `${currentPrefix}:` : currentPrefix; + + let res = `<${currentPrefix}SignedInfo>`; + res += `<${currentPrefix}CanonicalizationMethod Algorithm="${transform.getAlgorithmName()}"`; if (utils.isArrayHasLength(this.inclusiveNamespacesPrefixList)) { res += ">"; - res += - ''; - res += ""; + res += ``; + res += ``; } else { res += " />"; } - res += "<" + currentPrefix + 'SignatureMethod Algorithm="' + algo.getAlgorithmName() + '" />'; + res += `<${currentPrefix}SignatureMethod Algorithm="${algo.getAlgorithmName()}" />`; res += this.createReferences(doc, prefix); - res += ""; + res += ``; return res; } @@ -1056,26 +1016,16 @@ export class SignedXml { let xmlNsAttr = "xmlns"; if (prefix) { - xmlNsAttr += ":" + prefix; + xmlNsAttr += `:${prefix}`; prefix += ":"; } else { prefix = ""; } - const signatureValueXml = - "<" + prefix + "SignatureValue>" + this.signatureValue + ""; + const signatureValueXml = `<${prefix}SignatureValue>${this.signatureValue}`; //the canonicalization requires to get a valid xml node. //we need to wrap the info in a dummy signature since it contains the default namespace. - const dummySignatureWrapper = - "<" + - prefix + - "Signature " + - xmlNsAttr + - '="http://www.w3.org/2000/09/xmldsig#">' + - signatureValueXml + - ""; + const dummySignatureWrapper = `<${prefix}Signature ${xmlNsAttr}="http://www.w3.org/2000/09/xmldsig#">${signatureValueXml}`; const doc = new Dom().parseFromString(dummySignatureWrapper); diff --git a/test/signature-unit-tests.spec.ts b/test/signature-unit-tests.spec.ts index 6ed2918a..0ba8c49e 100644 --- a/test/signature-unit-tests.spec.ts +++ b/test/signature-unit-tests.spec.ts @@ -77,10 +77,10 @@ describe("Signature unit tests", function () { for (let i = 0; i < sig.references.length; i++) { // @ts-expect-error FIXME const ref = sig.references[i]; - const expectedUri = "#_" + i; + const expectedUri = `#_${i}`; expect( ref.uri, - "wrong uri for index " + i + ". expected: " + expectedUri + " actual: " + ref.uri + `wrong uri for index ${i}. expected: ${expectedUri} actual: ${ref.uri}` ).to.equal(expectedUri); expect(ref.transforms.length).to.equal(1); expect(ref.transforms[0]).to.equal("http://www.w3.org/2001/10/xml-exc-c14n#"); @@ -96,10 +96,7 @@ describe("Signature unit tests", function () { } function verifyDoesNotDuplicateIdAttributes(mode, prefix) { - const xml = - ""; + const xml = ``; const sig = new SignedXml({ idMode: mode }); sig.privateKey = fs.readFileSync("./test/static/client.pem"); sig.addReference({ xpath: "//*[local-name(.)='x']" }); @@ -117,7 +114,7 @@ describe("Signature unit tests", function () { } const node = xpath.select(xpathArg, doc); // @ts-expect-error FIXME - expect(node.length, "xpath " + xpathArg + " not found").to.equal(1); + expect(node.length, `xpath ${xpathArg} not found`).to.equal(1); } function verifyAddsId(mode, nsMode) { @@ -135,10 +132,7 @@ describe("Signature unit tests", function () { const op = nsMode === "equal" ? "=" : "!="; - const xpathArg = - "//*[local-name(.)='{elem}' and '_{id}' = @*[local-name(.)='Id' and namespace-uri(.)" + - op + - "'http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd']]"; + const xpathArg = `//*[local-name(.)='{elem}' and '_{id}' = @*[local-name(.)='Id' and namespace-uri(.)${op}'http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd']]`; //verify each of the signed nodes now has an "Id" attribute with the right value nodeExists(doc, xpathArg.replace("{id}", "0").replace("{elem}", "x")); @@ -168,13 +162,12 @@ describe("Signature unit tests", function () { const doc = new Dom().parseFromString(signedXml); const signatureNode = doc.documentElement; - expect( - attrs.Id, - 'Id attribute is not equal to the expected value: "' + attrs.Id + '"' - ).to.equal(signatureNode.getAttribute("Id")); + expect(attrs.Id, `Id attribute is not equal to the expected value: "${attrs.Id}"`).to.equal( + signatureNode.getAttribute("Id") + ); expect( attrs.data, - 'data attribute is not equal to the expected value: "' + attrs.data + '"' + `data attribute is not equal to the expected value: "${attrs.data}"` ).to.equal(signatureNode.getAttribute("data")); expect(attrs.xmlns, "xmlns attribute can not be overridden").not.to.equal( signatureNode.getAttribute("xmlns") @@ -889,7 +882,7 @@ describe("Signature unit tests", function () { const doc = new Dom().parseFromString(signedXml); const URI = xpath.select1("//*[local-name(.)='Reference']/@URI", doc); // @ts-expect-error FIXME - expect(URI.value, "uri should be empty but instead was " + URI.value).to.equal(""); + expect(URI.value, `uri should be empty but instead was ${URI.value}`).to.equal(""); }); it("signer appends signature to a non-existing reference node", function () { @@ -916,12 +909,10 @@ describe("Signature unit tests", function () { it("signer adds existing prefixes", function () { function getKeyInfoContentWithAssertionId({ assertionId }) { return ( - ' ' + - '' + - assertionId + - "" + - "" + ` ` + + `${assertionId}` + + `` ); } diff --git a/test/utils-tests.spec.ts b/test/utils-tests.spec.ts index 355475bc..ab89a2f1 100644 --- a/test/utils-tests.spec.ts +++ b/test/utils-tests.spec.ts @@ -8,8 +8,9 @@ describe("Utils tests", function () { const normalizedPem = fs.readFileSync("./test/static/client_public.pem", "latin1"); const pemAsArray = normalizedPem.trim().split("\n"); const base64String = pemAsArray.slice(1, -1).join(""); - const nonNormalizedPem = - pemAsArray[0] + "\n" + base64String + "\n" + pemAsArray[pemAsArray.length - 1]; + const nonNormalizedPem = `${pemAsArray[0]}\n${base64String}\n${ + pemAsArray[pemAsArray.length - 1] + }`; // @ts-expect-error FIXME expect(utils.derToPem(nonNormalizedPem)).to.equal(normalizedPem); From a8064e3fd9a6b2aeaa82260d51f57fc0deed143a Mon Sep 17 00:00:00 2001 From: Chris Barth Date: Tue, 18 Jul 2023 00:23:50 -0400 Subject: [PATCH 2/2] lint --- src/signed-xml.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/signed-xml.ts b/src/signed-xml.ts index 4253c2ea..43af106d 100644 --- a/src/signed-xml.ts +++ b/src/signed-xml.ts @@ -842,7 +842,7 @@ export class SignedXml { keyInfoAttrs += ` ${name}="${this.keyInfoAttributes[name]}"`; }); } - + const keyInfoContent = this.getKeyInfoContent({ publicCert: this.publicCert, prefix }); if (keyInfoAttrs || keyInfoContent) { return `<${currentPrefix}KeyInfo${keyInfoAttrs}>${keyInfoContent}`;