Skip to content

Commit 8291181

Browse files
shunkicashunkica
committed
revise idAttribute handling and add tests
1 parent a9afc1d commit 8291181

File tree

3 files changed

+181
-5
lines changed

3 files changed

+181
-5
lines changed

‎src/signed-xml.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -572,12 +572,15 @@ export class SignedXml {
572572
} else {
573573
const attr = utils.findAttr(elem, idAttr.localName, idAttr.namespaceUri);
574574
if (attr && uri === attr.value) {
575-
if (idAttr.namespaceUri !== undefined) {
575+
if (typeof idAttr.namespaceUri === "string") {
576576
// When namespaceUri is set, we look for attributes in that specific namespace
577577
ref.xpath = `//*[@*[local-name(.)='${idAttr.localName}' and namespace-uri(.)='${idAttr.namespaceUri}']='${uri}']`;
578-
} else {
579-
// When namespaceUri is explicitly set to undefined, we look only for attributes without a namespace
578+
} else if (idAttr.namespaceUri === null) {
579+
// When namespaceUri is explicitly set to null, we look only for attributes without a namespace
580580
ref.xpath = `//*[@*[local-name(.)='${idAttr.localName}' and namespace-uri(.)='']='${uri}']`;
581+
} else {
582+
// When namespaceUri is undefined, we look for attributes regardless of namespace
583+
ref.xpath = `//*[@*[local-name(.)='${idAttr.localName}']='${uri}']`;
581584
}
582585
break; // found the correct element, no need to check further
583586
}
@@ -613,8 +616,10 @@ export class SignedXml {
613616
if (typeof idAttr === "string") {
614617
tmp_elemXpath = `//*[@*[local-name(.)='${idAttr}']='${uri}']`;
615618
} else {
616-
if (idAttr.namespaceUri) {
619+
if (typeof idAttr.namespaceUri === "string") {
617620
tmp_elemXpath = `//*[@*[local-name(.)='${idAttr.localName}' and namespace-uri(.)='${idAttr.namespaceUri}']='${uri}']`;
621+
} else if (idAttr.namespaceUri === null) {
622+
tmp_elemXpath = `//*[@*[local-name(.)='${idAttr.localName}' and namespace-uri(.)='']='${uri}']`;
618623
} else {
619624
tmp_elemXpath = `//*[@*[local-name(.)='${idAttr.localName}']='${uri}']`;
620625
}

‎src/types.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@ export type ErrorFirstCallback<T> = (err: Error | null, result?: T) => void;
1616

1717
export type SignatureIdAttributeType =
1818
| string
19+
| { prefix?: undefined; localName: string; namespaceUri?: null }
1920
| { prefix: string; localName: string; namespaceUri: string };
21+
2022
export type VerificationIdAttributeType =
2123
| string
22-
| { localName: string; namespaceUri: string | null };
24+
| { localName: string; namespaceUri?: string | null };
2325
export type IdAttributeType = SignatureIdAttributeType | VerificationIdAttributeType;
2426

2527
/**

‎test/xmldsig-verifier.spec.ts‎

Lines changed: 169 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -387,6 +387,175 @@ describe("XmlDSigVerifier", function () {
387387
});
388388
expectInvalidResult(verifier.verifySignature(signedXml), "fail");
389389
});
390+
391+
describe("idAttributes property handling", function () {
392+
const xmlIdAttrNoNs = '<root><test customId="val">content</test></root>';
393+
const xmlIdAttrWithNs =
394+
'<root xmlns:ns="urn:test"><test ns:customId="val">content</test></root>';
395+
const xmlIdAttrOtherNs =
396+
'<root xmlns:ns="urn:other"><test ns:customId="val">content</test></root>';
397+
398+
it("should validate when idAttributes is a string (matches non-namespaced ID attribute)", function () {
399+
// Create signature for no-namespace XML
400+
const sig = new SignedXml({
401+
privateKey,
402+
idAttributes: ["customId"],
403+
canonicalizationAlgorithm: CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N,
404+
signatureAlgorithm: SIGNATURE_ALGORITHMS.RSA_SHA1,
405+
});
406+
sig.addReference({
407+
xpath: "//*[local-name(.)='test']",
408+
digestAlgorithm: HASH_ALGORITHMS.SHA1,
409+
transforms: [CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N],
410+
});
411+
sig.computeSignature(xmlIdAttrNoNs);
412+
const signedXml = sig.getSignedXml();
413+
414+
const verifier = new XmlDSigVerifier({
415+
keySelector: { publicCert },
416+
idAttributes: ["customId"],
417+
});
418+
expectValidResult(verifier.verifySignature(signedXml));
419+
});
420+
421+
it("should validate namespaced attribute when idAttributes is a string (matches namespaced ID attribute)", function () {
422+
const sig = new SignedXml({
423+
privateKey,
424+
idAttributes: ["customId"],
425+
canonicalizationAlgorithm: CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N,
426+
signatureAlgorithm: SIGNATURE_ALGORITHMS.RSA_SHA1,
427+
});
428+
sig.addReference({
429+
xpath: "//*[local-name(.)='test']",
430+
digestAlgorithm: HASH_ALGORITHMS.SHA1,
431+
transforms: [CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N],
432+
});
433+
sig.computeSignature(xmlIdAttrWithNs);
434+
const signedXml = sig.getSignedXml();
435+
436+
const verifier = new XmlDSigVerifier({
437+
keySelector: { publicCert },
438+
idAttributes: ["customId"],
439+
});
440+
expectValidResult(verifier.verifySignature(signedXml));
441+
});
442+
443+
it("should validate when `idAttributes` `namespaceUri` is `undefined` (matches namespaced ID attribute)", function () {
444+
const sig = new SignedXml({
445+
privateKey,
446+
idAttributes: [{ localName: "customId", namespaceUri: undefined }],
447+
canonicalizationAlgorithm: CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N,
448+
signatureAlgorithm: SIGNATURE_ALGORITHMS.RSA_SHA1,
449+
});
450+
sig.addReference({
451+
xpath: "//*[local-name(.)='test']",
452+
digestAlgorithm: HASH_ALGORITHMS.SHA1,
453+
transforms: [CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N],
454+
});
455+
sig.computeSignature(xmlIdAttrWithNs);
456+
const signedXml = sig.getSignedXml();
457+
458+
const verifier = new XmlDSigVerifier({
459+
keySelector: { publicCert },
460+
idAttributes: [{ localName: "customId", namespaceUri: undefined }],
461+
});
462+
expectValidResult(verifier.verifySignature(signedXml));
463+
});
464+
465+
it("should validate when `idAttributes` `namespaceUri` is `null` (matches non-namespaced ID attribute)", function () {
466+
const sig = new SignedXml({
467+
privateKey,
468+
idAttributes: [{ localName: "customId", namespaceUri: null }],
469+
canonicalizationAlgorithm: CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N,
470+
signatureAlgorithm: SIGNATURE_ALGORITHMS.RSA_SHA1,
471+
});
472+
sig.addReference({
473+
xpath: "//*[local-name(.)='test']",
474+
digestAlgorithm: HASH_ALGORITHMS.SHA1,
475+
transforms: [CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N],
476+
});
477+
sig.computeSignature(xmlIdAttrNoNs);
478+
const signedXml = sig.getSignedXml();
479+
480+
const verifier = new XmlDSigVerifier({
481+
keySelector: { publicCert },
482+
idAttributes: [{ localName: "customId", namespaceUri: null }],
483+
});
484+
expectValidResult(verifier.verifySignature(signedXml));
485+
});
486+
487+
it("should fail validation when `idAttributes` `namespaceUri` is `null` but ID attribute is namespaced (should be excluded)", function () {
488+
const sig = new SignedXml({
489+
privateKey,
490+
idAttributes: ["customId"], // Sign it loosely so it signs
491+
canonicalizationAlgorithm: CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N,
492+
signatureAlgorithm: SIGNATURE_ALGORITHMS.RSA_SHA1,
493+
});
494+
sig.addReference({
495+
xpath: "//*[local-name(.)='test']",
496+
digestAlgorithm: HASH_ALGORITHMS.SHA1,
497+
transforms: [CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N],
498+
});
499+
sig.computeSignature(xmlIdAttrWithNs);
500+
const signedXml = sig.getSignedXml();
501+
502+
// Verifier expects NO namespace, but XML has one
503+
const verifier = new XmlDSigVerifier({
504+
keySelector: { publicCert },
505+
idAttributes: [{ localName: "customId", namespaceUri: null }],
506+
throwOnError: false,
507+
});
508+
509+
// Should fail because it can't find the reference target with the strict criteria
510+
expectInvalidResult(verifier.verifySignature(signedXml), "verification failed");
511+
});
512+
513+
it("should validate when `idAttributes` `namespaceUri` is a string and matches the ID attribute's namespace", function () {
514+
const sig = new SignedXml({
515+
privateKey,
516+
idAttributes: [{ localName: "customId", namespaceUri: "urn:test" }],
517+
canonicalizationAlgorithm: CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N,
518+
signatureAlgorithm: SIGNATURE_ALGORITHMS.RSA_SHA1,
519+
});
520+
sig.addReference({
521+
xpath: "//*[local-name(.)='test']",
522+
digestAlgorithm: HASH_ALGORITHMS.SHA1,
523+
transforms: [CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N],
524+
});
525+
sig.computeSignature(xmlIdAttrWithNs);
526+
const signedXml = sig.getSignedXml();
527+
528+
const verifier = new XmlDSigVerifier({
529+
keySelector: { publicCert },
530+
idAttributes: [{ localName: "customId", namespaceUri: "urn:test" }],
531+
});
532+
expectValidResult(verifier.verifySignature(signedXml));
533+
});
534+
535+
it("should fail validation when `idAttributes` `namespaceUri` is a string but ID attribute has a different namespace (should be excluded)", function () {
536+
const sig = new SignedXml({
537+
privateKey,
538+
idAttributes: ["customId"], // Sign loosely
539+
canonicalizationAlgorithm: CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N,
540+
signatureAlgorithm: SIGNATURE_ALGORITHMS.RSA_SHA1,
541+
});
542+
sig.addReference({
543+
xpath: "//*[local-name(.)='test']",
544+
digestAlgorithm: HASH_ALGORITHMS.SHA1,
545+
transforms: [CANONICALIZATION_ALGORITHMS.EXCLUSIVE_C14N],
546+
});
547+
sig.computeSignature(xmlIdAttrOtherNs);
548+
const signedXml = sig.getSignedXml();
549+
550+
const verifier = new XmlDSigVerifier({
551+
keySelector: { publicCert },
552+
idAttributes: [{ localName: "customId", namespaceUri: "urn:test" }],
553+
throwOnError: false,
554+
});
555+
556+
expectInvalidResult(verifier.verifySignature(signedXml), "verification failed");
557+
});
558+
});
390559
});
391560

392561
describe("throwOnError option", function () {

0 commit comments

Comments
 (0)