Skip to content
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.

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

Resolve XML-encoded carriage returns during signature validation #576

Merged
merged 1 commit into from Apr 5, 2021
Merged

Resolve XML-encoded carriage returns during signature validation #576

merged 1 commit into from Apr 5, 2021

Conversation

mhassan1
Copy link
Contributor

@mhassan1 mhassan1 commented Mar 25, 2021

Description

See #575 for background. This PR duplicates #578 for master.

This adds XML normalization to validateSignatureForCert before sending it to xml-crypto. It uses DOMParser for normalization tasks such as replacement of XML-encoded entities (
) with their actual representations (carriage return).

Checklist:

  • Issue Addressed: [X]
  • Link to SAML spec: [ ]
  • Tests included? [X]
  • Documentation updated? N/A

@markstos
Copy link
Contributor

I reviewed the diff, LGTM.

@markstos markstos self-requested a review March 25, 2021 18:31
@markstos
Copy link
Contributor

Thanks for including some tests.

@markstos
Copy link
Contributor

@cjbarth This seems like a fix worth releasing on the 2.x branch.

@cjbarth
Copy link
Collaborator

cjbarth commented Mar 26, 2021

I will gladly release 2.x if another PR against that branch is landed. As for this, I'd like to let #574 land before landing this so that we don't cause merge conflicts for that huge PR. We'll still get this landed before 3.x.

@mhassan1, would you like to create this PR against the 2.x branch so we can do a release to the 2.x series while we are waiting for 3.x to get finished up?

@cjbarth cjbarth added the semver-minor This PR requires a semver-minor version bump label Mar 27, 2021
@mhassan1 mhassan1 changed the title resolve XML-encoded carriage returns during signature validation Resolve XML-encoded carriage returns during signature validation Mar 29, 2021
@mhassan1
Copy link
Contributor Author

@cjbarth PR for 2.x: #578

@cjbarth
Copy link
Collaborator

cjbarth commented Apr 5, 2021

@mhassan1 Could you please update this PR to resolve the merge conflicts and we'll get this landed for the 3.x release?

test/node-saml/test-signatures.spec.ts Outdated Show resolved Hide resolved
@cjbarth cjbarth added this to the 3.0.0 milestone Apr 5, 2021
@cjbarth cjbarth merged commit 5618b65 into node-saml:master Apr 5, 2021
3 checks passed
@cjbarth cjbarth mentioned this pull request May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-minor This PR requires a semver-minor version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants