Closed bseddon closed 3 years ago
Just some random thoughts;
I think everything after the July 1st commits shouldn't be part of this PR. That's a change on it's own and personally I don't like the idea of people importing their own namespaces into another man's project. Might as well create a fork then.
My requirement is to be able to add a timestamp as a <SignatureProperty>. In issue #94 you comment that there is no support for <SignatureProperties>. While this PR does not include a generic mechanism to add properties, it does make use of addObject() to create function called addTimestamp to add a specific property. The technique used could be adapted by users to add any other named property and value.
To add a timestamp, pass an id (a new parameter) to the XMLSecurityDSig constructor:
then call the new function before signing:
This call might be placed before the security key is created in the example in the readme.
Other changes
The code has also been modified to add support for detached signature files.
Most if not all uses of namespace strings have been replaced by references to constants that already exist.
In validateReference() the original code recovers the original Xml content by removing the signature. This works OK when signing the Xml content. However removing the signature node causes a problem when trying to canonicalize content within the <Signature> - such as <SignatureProperties>. This is because $domNode->C14N() will always return an empty string if it is called on a node that is no attached to a document.
Instead of removing <Signature> in validateReference(), when the original Xml content is needed the DOM document is cloned in processRefNode() and <Signature> is removed from the clone. As a result, nodes within <Signature> in sigNode can still be canonicalized.
No attempt has been made to run the tests to see it changes will cause regressions in those cases. For this and other reasons you may be reluctant to accept the PR. Even so, because the changes have been made it seems polite to share.