Mark getKeyInfo() private as it has no public consumers

[cjbarth]

No description provided.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (741240f) 73.36% compared to head (196a541) 73.36%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #412   +/-   ##
=======================================
  Coverage   73.36%   73.36%           
=======================================
  Files           9        9           
  Lines         901      901           
  Branches      239      239           
=======================================
  Hits          661      661           
  Misses        142      142           
  Partials       98       98           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[GauriSpears]

Hey! Why have you done this? I used my own redefined getKeyInfo to make the signature xades-compliant by adding Object section:

sig.getKeyInfoOld = sig.getKeyInfo;
sig.getKeyInfo = function (prefix) {
      return this.getKeyInfoOld(prefix) +
          '<ds:Object>'+
            '<xades:QualifyingProperties ........
            '</xades:QualifyingProperties>'+
          '</ds:Object>';
}

So how should I do this now?

[cjbarth]

Thank you for bringing this to our attention. You're the first case I'm aware of that is using this function like this. What is more, there is no test suite covering this at all. Many have found leveraging getKeyInfoContent() is a suitable abstraction for this problem. Would that work in your case?

I know that getKeyInfo() returns the <KeyInfo /> node already wrapped, and it looks like you're just appending the <ds:Object /> node as a sibling of <KeyInfo />, so it may not work, but I don't work with XAdES-compliance regularly (I assume this is the spec, please correct me if I'm wrong).

If not, please, let's continue this discussion and get this change reverted with a test suite to make sure it doesn't break again. Or, please feel free to make a PR that enables XAdES-compliance in our codebase so that you don't need custom code.

It seems from a quick Google that we probably need to modify computeSignature() to support this additional element instead of overloading getKeyInfo() as you've done here. Maybe something like this:

    let signatureXml = `<${currentPrefix}Signature ${signatureAttrs.join(" ")}>`;

    signatureXml += this.createSignedInfo(doc, prefix);
    signatureXml += this.getKeyInfo(prefix);
    const xadesSignedProperties = this.getXadesSignedProperties();
    const xadesUnsignedProperties = this.getXadesInsignedProperties();
    if (xadesSignedProperties != null || xadesUnsignedProperties != null) {
        signatureXml += `<${currentPrefix}Object><QualifyingProperties>`
        if (xadesSignedProperties != null) {
            signatureXml += `<SignedProperties>${xadesSignedProperties}</SignedProperties>`
        }
        if (xadesUnsignedProperties != null) {
            signatureXml += `<UnsignedProperties>${xadesUnsignedProperties}</UnsignedProperties>`
        }
        signatureXml += `</QualifyingProperties></${currentPrefix}Object>`
    }
    signatureXml += `</${currentPrefix}Signature>`;

If something like that looks right, please put up a PR with some tests and we'll get it merged and released as 5.1.

Again, thank you for bringing this to everyone's attention and for your support of this open-source project!

[GauriSpears]

Thanks for your kind attitude)
Indeed, getKeyInfoContent wouldn't help because we need to put Object after KeyInfo, not inside it. Actually to satisfy my usecase we need to:
1). add Object node after KeyInfo
and
2). allow Reference to link nodes inside Signature itself (I need to add reference to Signature/Object/QualifyingProperties/SignedProperties).
You are absolutely right, redefining getKeyInfo was rather a dirty hack than a proper way. I'll prepare a PR.

[GauriSpears]

I have finally prepared a PR: #444