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

fix: derive SamlConfig from SAMLOptions #515

Merged
merged 3 commits into from
Dec 15, 2020
Merged

Conversation

midgleyc
Copy link
Contributor

Strategy and MultiSamlStrategy accept an "options" parameter with three fields which are used locally by the strategy, and remaining fields which are passed on to the SAML constructor.

There was an issue where certain fields (e.g. privateKey) could not be passed to the Strategy constructor but had to be passed on to the SAML constructor. This fixes that more thoroughly than #497, also fixing the type of authnContext, the limited string parameters, and adding xmlSignatureTransforms, digestAlgorithm and disableRequestACSUrl.

Checklist:

  • Issue Addressed: [x]
  • Link to SAML spec: [ ]
  • Tests included? [ ]
  • Documentation updated? [ ]

@cjbarth
Copy link
Collaborator

cjbarth commented Dec 15, 2020

@gugu Since you were involved in a lot of the TypeScript conversions, I'd like you to have a look at this too. It seems good to me, but 4 eyes can't hurt :)

@gugu
Copy link
Contributor

gugu commented Dec 15, 2020

Looks good

@cjbarth cjbarth merged commit 29d997f into node-saml:master Dec 15, 2020
@gugu gugu added the bug label Jan 7, 2021
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants