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

Accept existing xml prefixes to avoid adding to signature #171

Merged
merged 4 commits into from
Feb 25, 2019
Merged

Accept existing xml prefixes to avoid adding to signature #171

merged 4 commits into from
Feb 25, 2019

Conversation

roelandmoors
Copy link

@roelandmoors roelandmoors commented Feb 2, 2019

Not sure if this is the best way, but it does work.

This adds an extra option existingPrefixes.
I also added this to the README and there is a test.

Fixes #170

@roelandmoors
Copy link
Author

Shouldn't Travis use a more recent node version? 0.10 and 0.12 are really old and not supported anymore.

Copy link
Collaborator

@LoneRifle LoneRifle left a comment

Choose a reason for hiding this comment

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

lgtm - I would strongly prefer that you keep the old node versions just because this library is used mostly in enterprise environments, which tend to be very slow in taking up new releases.

@LoneRifle LoneRifle changed the title Fix for https://github.com/yaronn/xml-crypto/issues/170 Accept existing xml prefixes to avoid adding to signature Feb 7, 2019
@roelandmoors
Copy link
Author

If I make this work on the old node version, would you accept it? I'm happy to change it then.
I now this isn't the best solution, but I couldn't think of a better one with the current xmldom.

@LoneRifle
Copy link
Collaborator

I would - and I was planning to make the changes in your branch if not for life getting in the way. Feel free to go ahead of me.

@roelandmoors
Copy link
Author

It should be solved now. Thanks for maintaining this library!

@LoneRifle LoneRifle merged commit 4d72940 into node-saml:master Feb 25, 2019
markstos pushed a commit to node-saml/passport-saml that referenced this pull request Nov 18, 2019
@cjbarth cjbarth added the bug label Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Custom KeyInfo, avoid adding empty/unneeded namespaces
3 participants