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

[BUG] Wrong readme example for authnContext for ADFS #642

Closed
jyenduri-uptycs opened this issue Sep 21, 2021 · 4 comments · Fixed by #647
Closed

[BUG] Wrong readme example for authnContext for ADFS #642

jyenduri-uptycs opened this issue Sep 21, 2021 · 4 comments · Fixed by #647
Labels
documentation Request for or contribution to documentation pr-welcome

Comments

@jyenduri-uptycs
Copy link
Contributor

jyenduri-uptycs commented Sep 21, 2021

To Reproduce

I am trying out ADFS based on the example authContext is a string, but looks like it is only working if we give it as an array

{
entryPoint: 'https://ad.example.net/adfs/ls/',
issuer: 'https://your-app.example.net/login/callback',
callbackUrl: 'https://your-app.example.net/login/callback',
cert: 'MIICizCCAfQCCQCY8tKaMc0BMjANBgkqh ... W==',
authnContext: 'http://schemas.microsoft.com/ws/2008/06/identity/authenticationmethod/windows',
identifierFormat: null
}

Working example

{
entryPoint: 'https://ad.example.net/adfs/ls/',
issuer: 'https://your-app.example.net/login/callback',
callbackUrl: 'https://your-app.example.net/login/callback',
cert: 'MIICizCCAfQCCQCY8tKaMc0BMjANBgkqh ... W==',
authnContext: ['http://schemas.microsoft.com/ws/2008/06/identity/authenticationmethod/windows'],
identifierFormat: null
}

Expected behavior
We need to update the README and examples

Environment

  • Node.js version: v12.14.0
  • passport-saml version: 3.1.2
@cjbarth cjbarth added documentation Request for or contribution to documentation pr-welcome and removed bug labels Sep 21, 2021
@jyenduri-uptycs
Copy link
Contributor Author

jyenduri-uptycs commented Sep 21, 2021

@cjbarth Here is the Pull request #647

@jyenduri-uptycs
Copy link
Contributor Author

@cjbarth Can you please look at this pull request #647

@cjbarth cjbarth linked a pull request Sep 22, 2021 that will close this issue
@ctaschereau
Copy link

Agreed. This appears to be an undocumented breaking change with the version 2.x since we had working code passing only a string as an authncontext before...

@cjbarth
Copy link
Collaborator

cjbarth commented Sep 30, 2021

I found the commit that created the break. It was my commit, and I overlooked updating the documentation. Thanks for catching this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Request for or contribution to documentation pr-welcome
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants