Skip to content

Respect idAttribute when generating signatures. #508

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.

Sign up for GitHub

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

Open
tkalmar wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Respect idAttribute when generating signatures. #508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3510993
Respect idAttribute when generating signatures.
Jul 12, 2025
26d9b91
Merge branch 'master' into master
cjbarth Oct 27, 2025
193a6a6
Add namespaced id support
Nov 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { NamespacedId } from "./types";

/** Well known namespaced ids */
export const KNOWN_NAMESPACED_IDS: { [key: string]: NamespacedId } = {
/** WS-Security */
wssecurity: {
prefix: "wsu",
localName: "Id",
nameSpaceURI:
"http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd",
},
/** Xml */
xml: {
prefix: "xml",
localName: "id",
nameSpaceURI: "http://www.w3.org/XML/1998/namespace",
},
Comment on lines +13 to +17
Copy link

@coderabbitai coderabbitai bot Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

LGTM! Supports the xml:id use case.

The xml entry correctly models the xml:id attribute, addressing the customer requirement mentioned in the PR comments where they need to generate signatures with xml:id instead of Id.

Verify that the xml constant is actually used in the codebase:

🏁 Script executed:

#!/bin/bash
# Search for usage of KNOWN_NAMESPACED_IDS["xml"]
rg -n 'KNOWN_NAMESPACED_IDS\["xml"\]' --type ts
rg -n "KNOWN_NAMESPACED_IDS\['xml'\]" --type ts

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Search for broader KNOWN_NAMESPACED_IDS usage patterns
echo "=== Search for KNOWN_NAMESPACED_IDS usage ==="
rg -n 'KNOWN_NAMESPACED_IDS' --type ts -C 2

echo ""
echo "=== View the constants.ts file to see the structure ==="
fd 'constants.ts' --type f | xargs head -30

Length of output: 1317

🏁 Script executed:

#!/bin/bash
# Search for idMode usage to understand if xml is supposed to be supported
echo "=== Search for idMode string patterns ==="
rg -n 'idMode\s*===|idMode\s*=\s*["\']' --type ts -C 1

echo ""
echo "=== Check full idMode logic in signed-xml.ts ==="
rg -n 'idMode' src/signed-xml.ts -C 3

Length of output: 110

🏁 Script executed:

#!/bin/bash
# Search for idMode usage with simpler patterns
echo "=== Search for idMode assignments and checks ==="
rg -n 'idMode' src/signed-xml.ts -B 2 -A 2

echo ""
echo "=== Search for any xml mode or xml:id references ==="
rg -n 'xml:id|xml.*mode|mode.*xml' --type ts

Length of output: 815

Add implementation logic to complete the xml:id support feature.

The xml constant has been added to KNOWN_NAMESPACED_IDS, but the feature is incomplete and currently dead code. To make this functional:

  1. Update the idMode type at src/signed-xml.ts:33 to include "xml":

    idMode?: "wssecurity" | "xml";

  2. Add the corresponding handler in the constructor (after line 162):

    if (idMode === "xml") {
  this.idAttributes.unshift(KNOWN_NAMESPACED_IDS["xml"]);
}

Without these changes, the xml constant cannot be used and the customer's xml:id use case won't function.

🤖 Prompt for AI Agents 
In src/constants.ts around lines 13-17 and in src/signed-xml.ts update the
idMode type and constructor to enable xml:id handling: add "xml" to the idMode
union at or near line 33 (idMode?: "wssecurity" | "xml";) and in the constructor
after line 162, detect idMode === "xml" and unshift KNOWN_NAMESPACED_IDS["xml"]
onto this.idAttributes so the xml namespaced id is actually used; this wires the
new KNOWN_NAMESPACED_IDS entry into runtime behavior.

};
44 changes: 22 additions & 22 deletions src/signed-xml.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type {
GetKeyInfoContentArgs,
HashAlgorithm,
HashAlgorithmType,
NamespacedId,
ObjectAttributes,
Reference,
SignatureAlgorithm,
Expand All @@ -26,10 +27,11 @@ import * as execC14n from "./exclusive-canonicalization";
import * as hashAlgorithms from "./hash-algorithms";
import * as signatureAlgorithms from "./signature-algorithms";
import * as utils from "./utils";
import { KNOWN_NAMESPACED_IDS } from "./constants";

export class SignedXml {
idMode?: "wssecurity";
idAttributes: string[];
idAttributes: (string | NamespacedId)[];
/**
* A {@link Buffer} or pem encoded {@link String} containing your private key
*/
Expand Down Expand Up @@ -155,6 +157,10 @@ export class SignedXml {
if (idAttribute) {
this.idAttributes.unshift(idAttribute);
}
if (idMode === "wssecurity") {
this.idAttributes.unshift(KNOWN_NAMESPACED_IDS["wssecurity"]);
}

this.privateKey = privateKey;
this.publicCert = publicCert;
this.signatureAlgorithm = signatureAlgorithm ?? this.signatureAlgorithm;
Expand Down Expand Up @@ -503,7 +509,10 @@ export class SignedXml {
const uri = ref.uri?.[0] === "#" ? ref.uri.substring(1) : ref.uri;

for (const attr of this.idAttributes) {
const elemId = elem.getAttribute(attr);
const elemId =
typeof attr === "string"
? elem.getAttribute(attr)
: elem.getAttribute(`${attr.prefix}:${attr.localName}`);
Comment on lines +512 to +515
Copy link

@coderabbitai coderabbitai bot Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Consider using getAttributeNS for namespaced attributes.

The code constructs a prefixed attribute name (${attr.prefix}:${attr.localName}) for NamespacedId attributes, but DOM's getAttributeNS(namespaceURI, localName) is the standard method for reading namespaced attributes. This could lead to issues if the prefix varies or is absent.

Apply this pattern for reading namespaced attributes:

 const elemId =
   typeof attr === "string"
     ? elem.getAttribute(attr)
-    : elem.getAttribute(`${attr.prefix}:${attr.localName}`);
+    : elem.getAttributeNS(attr.nameSpaceURI, attr.localName);

This ensures correct attribute resolution regardless of prefix usage in the document.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/signed-xml.ts around lines 512 to 515, the code builds a prefixed name to
read a namespaced attribute which will break if the document uses a different or
no prefix; change the branch that handles non-string attr to call
elem.getAttributeNS(attr.namespaceURI, attr.localName) instead of
elem.getAttribute(`${attr.prefix}:${attr.localName}`), and if getAttributeNS
returns null/undefined fall back to the prefixed lookup only as a last resort;
also guard for the case attr.namespaceURI may be missing (then use the old
getAttribute behavior).

if (uri === elemId) {
ref.xpath = `//*[@*[local-name(.)='${attr}']='${uri}']`;
Copy link

@coderabbitai coderabbitai bot Nov 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Fix XPath construction for NamespacedId.

When attr is a NamespacedId object, the template literal ${attr} will stringify it as "[object Object]", producing an invalid XPath. Extract the localName for proper XPath construction.

Apply this fix:

+        const attrName = typeof attr === "string" ? attr : attr.localName;
-        if (uri === elemId) {
-          ref.xpath = `//*[@*[local-name(.)='${attr}']='${uri}']`;
+        if (uri === elemId) {
+          ref.xpath = `//*[@*[local-name(.)='${attrName}']='${uri}']`;
           break; // found the correct element, no need to check further
         }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In src/signed-xml.ts around line 517, the XPath construction uses `${attr}`
which stringifies a NamespacedId to "[object Object]"; change it to detect when
attr is a NamespacedId (e.g. an object with a localName property) and use its
localName instead of the object in the template literal so the xpath becomes
`//*[@*[local-name(.)='${localName}']='${uri}']`; ensure the code handles both
string and NamespacedId inputs before assigning ref.xpath.

break; // found the correct element, no need to check further
Expand Down Expand Up @@ -1297,18 +1306,10 @@ export class SignedXml {
private ensureHasId(node) {
let attr;

if (this.idMode === "wssecurity") {
attr = utils.findAttr(
node,
"Id",
"http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd",
);
} else {
this.idAttributes.some((idAttribute) => {
attr = utils.findAttr(node, idAttribute);
return !!attr; // This will break the loop as soon as a truthy attr is found.
});
}
this.idAttributes.some((idAttribute) => {
attr = utils.findAttr(node, idAttribute);
return !!attr; // This will break the loop as soon as a truthy attr is found.
});

if (attr) {
return attr.value;
Expand All @@ -1317,21 +1318,20 @@ export class SignedXml {
//add the attribute
const id = `_${this.id++}`;

if (this.idMode === "wssecurity") {
if (typeof this.idAttributes[0] === "string") {
node.setAttribute(this.idAttributes[0], id);
} else {
node.setAttributeNS(
"http://www.w3.org/2000/xmlns/",
"xmlns:wsu",
"http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd",
`xmlns:${this.idAttributes[0].prefix}`,
this.idAttributes[0].nameSpaceURI,
);
node.setAttributeNS(
"http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd",
"wsu:Id",
this.idAttributes[0].nameSpaceURI,
`${this.idAttributes[0].prefix}:${this.idAttributes[0].localName}`,
id,
);
} else {
node.setAttribute("Id", id);
}

return id;
}

Expand Down
14 changes: 13 additions & 1 deletion src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,24 @@ export interface ObjectAttributes {
[key: string]: string | undefined;
}

/**
* Namespaced id attribute.
*/
export interface NamespacedId {
/** Namespace prefix */
prefix: string;
/** Attribute local name */
localName: string;
/** Namespace URI */
nameSpaceURI: string;
}

/**
* Options for the SignedXml constructor.
*/
export interface SignedXmlOptions {
idMode?: "wssecurity";
idAttribute?: string;
idAttribute?: string | NamespacedId;
privateKey?: crypto.KeyLike;
publicCert?: crypto.KeyLike;
signatureAlgorithm?: SignatureAlgorithmType;
Expand Down
13 changes: 10 additions & 3 deletions src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as xpath from "xpath";
import type { NamespacePrefix } from "./types";
import type { NamespacedId, NamespacePrefix } from "./types";
import * as isDomNode from "@xmldom/is-dom-node";

export function isArrayHasLength(array: unknown): array is unknown[] {
Expand All @@ -17,10 +17,17 @@ function attrEqualsImplicitly(attr: Attr, localName: string, namespace?: string,
);
}

export function findAttr(element: Element, localName: string, namespace?: string) {
export function findAttr(element: Element, id: string | NamespacedId) {
for (let i = 0; i < element.attributes.length; i++) {
const attr = element.attributes[i];

let localName: string;
let namespace: string | undefined;
if (typeof id === "string") {
localName = id;
} else {
localName = id.localName;
namespace = id.nameSpaceURI;
}
if (
attrEqualsExplicitly(attr, localName, namespace) ||
attrEqualsImplicitly(attr, localName, namespace, element)
Expand Down
12 changes: 9 additions & 3 deletions test/signature-unit-tests.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,9 @@ describe("Signature unit tests", function () {
expect(node.length, `xpath ${xpathArg} not found`).to.equal(1);
}

function verifyAddsId(mode, nsMode) {
function verifyAddsId(mode, nsMode, idAttribute: string | undefined = undefined) {
const xml = '<root><x xmlns="ns"></x><y attr="value"></y><z><w></w></z></root>';
const sig = new SignedXml({ idMode: mode });
const sig = new SignedXml({ idMode: mode, idAttribute });
sig.privateKey = fs.readFileSync("./test/static/client.pem");

sig.addReference({
Expand All @@ -114,14 +114,20 @@ 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(.)='${
idAttribute || "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"));
nodeExists(doc, xpathArg.replace("{id}", "1").replace("{elem}", "y"));
nodeExists(doc, xpathArg.replace("{id}", "2").replace("{elem}", "w"));
}

it("signer adds increasing different id attributes to elements with custom idAttribute", function () {
verifyAddsId(null, "different", "myIdAttribute");
});

it("signer adds increasing different id attributes to elements", function () {
verifyAddsId(null, "different");
});
Expand Down