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 validateInResponseTo null check #596

Merged
merged 1 commit into from May 21, 2021
Merged

Conversation

jamiees2
Copy link
Contributor

Description

When validateInResponseTo isn't set to true, we end up setting it to false here:

validateInResponseTo: ctorOptions.validateInResponseTo ?? false,

This value was then checked if it's not null here, which is always true. The intention was likely to have this check check if the value was true, like everywhere else this value is used. This changes the gate to just check if the value is true.

This is a small enough change so I won't default to writing a test for this edge behavior - an extra delete call to the cache provider isn't a big deal, but I found this when debugging our custom cache provider.

Checklist:

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

@cjbarth
Copy link
Collaborator

cjbarth commented May 20, 2021

This looks good to me. Can you please create a matching PR against the node-saml project too so they stay in sync while we dis-entangle them?

@jamiees2
Copy link
Contributor Author

Made node-saml/node-saml#4 :)

@cjbarth cjbarth merged commit bf41764 into node-saml:master May 21, 2021
5 checks passed
@jamiees2 jamiees2 deleted the patch-1 branch May 21, 2021 21:24
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

2 participants