Update dependencies - add new option flags, clean up code #264
Conversation
Clever#109 This PR is simply to update dependencies. Primarily it is to update xml-encryption, to get this other update included: auth0/node-xml-encryption#28 This is to avoid https://snyk.io/vuln/npm:ejs:20161130
Seems like index.js is supposed to build the coffeescript file, but that never happens with Meteor...
In some cases we get multiple authentication statements, as they are not being considered, we will just continue as normal and still use just the first one.
For cases where we have more than 1 authnStatements for whatever reason, we want to log both of them to see why that is happening.
… auth statements. Added a sample xml file based on the good assertion xml file.
…er-master-2-9-2022 # Conflicts: # Makefile # index.js # package.json Resolved all of them in favor of the original package.
…nerabilities Update saml2.coffee
Bumps [@xmldom/xmldom](https://github.com/xmldom/xmldom) from 0.7.5 to 0.8.2. - [Release notes](https://github.com/xmldom/xmldom/releases) - [Changelog](https://github.com/xmldom/xmldom/blob/master/CHANGELOG.md) - [Commits](xmldom/xmldom@0.7.5...0.8.2) --- updated-dependencies: - dependency-name: "@xmldom/xmldom" dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]>
Bumps [mocha](https://github.com/mochajs/mocha) from 8.4.0 to 10.0.0. - [Release notes](https://github.com/mochajs/mocha/releases) - [Changelog](https://github.com/mochajs/mocha/blob/master/CHANGELOG.md) - [Commits](mochajs/mocha@v8.4.0...v10.0.0) --- updated-dependencies: - dependency-name: mocha dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
…dom/xmldom-0.8.2 Bump @xmldom/xmldom from 0.7.5 to 0.8.2
Bumps [xmlbuilder2](https://github.com/oozcitak/xmlbuilder2) from 2.4.1 to 3.0.2. - [Release notes](https://github.com/oozcitak/xmlbuilder2/releases) - [Changelog](https://github.com/oozcitak/xmlbuilder2/blob/master/CHANGELOG.md) - [Commits](oozcitak/xmlbuilder2@v2.4.1...v3.0.2) --- updated-dependencies: - dependency-name: xmlbuilder2 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
…builder2-3.0.2 Bump xmlbuilder2 from 2.4.1 to 3.0.2
…ha-10.0.0 Bump mocha from 8.4.0 to 10.0.0
Change npmignore, package.json
Bumps [xml-crypto](https://github.com/yaronn/xml-crypto) from 2.1.4 to 3.0.0. - [Release notes](https://github.com/yaronn/xml-crypto/releases) - [Commits](node-saml/xml-crypto@v2.1.4...v3.0.0) --- updated-dependencies: - dependency-name: xml-crypto dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <[email protected]>
…-10-17-2022 # Conflicts: # package.json Manually resolved conflicts, kept saml2js designation for the name of the package as it's being used through github.
…ml-crypto-3.0.0' into dario-master-10-17-2022 # Conflicts: # package.json
…r-10-17-2022 # Conflicts: # .npmignore # README.md # lib/saml2.coffee # package.json Resolved conflicts manually in favor of newer libraries and new code from the Clever/master branch.
…ne that deals with the tests. Added 2 new options for the post and redirect assert - notonorafter_required - defaults to false (current behavior) - notbefore_required - defaults to false (current behavior) Added 4 new test xml files to test the new options that check for the two tags in the Conditions element. One of the tests checks if the Conditions tag is defined, but one or more of the two new options are set to true - in that case the Conditions tag is required by default.
…s, defaults to false. Renamed flags to be more consistent with previous options present. Updated tests and organized them a bit better for easier reading. Added 3 new test files for the new flag.
|
As is, this PR is very hard to merge in. The large changes stem from five different goals:
To bring forks to parity, we should focus on one area at a time. If you can cherry pick commits that focus on feature differences, we can see if they're appropriately tested and maintainable. |
|
@mcab as this library wasn't maintained for a while it's kind of hard to separate from older commits - wouldn't the fact that all the unit tests pass be enough to conclude that there were no changes in logic that were unintended ? I can add more tests if you have specific concerns, maybe some parts weren't being tested properly previously. |
No, because it has new functionality that changes how this library was used before, and how it's used by you now. These include the new features:
If you wish to contribute these features to Clever's branch, then it makes sense to cut commits and PRs for each separate one. As a whole, it's very hard to classify what this single branch should be considered. Fundamentally, there are some function signature changes, which should be considered a new major version. I believe that if it's working well for you, there's nothing wrong with maintaining a fork. All these commits can be dropped or reconsidered, given their purposes. Dependency drift:
Tools that are unused upstream: Changes that affect publishing the package: Formatting changes that should be standardized in tooling: |
|
The optional ignore signature and multiple authn statements do affect functionality - they were originally PR's/Fixes suggested on this fork - but were never merged as there weren't tests for them. For some reason I remember the authn part was throwing an error that it shouldn't have been in some cases - so that was an intentional addition. They might be working together with the ignore signature addition to properly parse responses. #74 @mgduk 's comment The two new parameters for the NotBefore/NotOnOrAfter and OneTimeUse default to false so they shouldn't affect the functionality. As far as the dependency drift commits - I think I can get those to be the latest committed to this main repo - with the only change being that I think lodash works better than underscore - maybe limit the import of it to just the functions used to keep it lightweight. The tools would be removed and changes affecting the publishing would be reverted to the current master branch. Then the formatting changes although manual in nature bring a bit more readability to the library - would be nice if they were adopted and maintained - coffeescript is hard 😅 Let me know if these sound alright, I can try making another 2 branches and PRs with these changes in mind. |
|
Hey, I am gladly accepting PRs (again) on the revived fork |
This PR contains a lot of changes