Update docs/adfs/README.md and move to wiki

[1nbuc]

Description

• Added Powershell command to activate signing on message and aseertion on AD FS in docs/adfs/README.md
• Added Link to Microsoft Documentation with a list of all available authnContext options.

Checklist:

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

[cjbarth]

We don't want any example code to disable security features.

[1nbuc]

But it is very confusing since this is the exact configuration for AD FS and not working out of the box.
If possible it could also be mentioned how to adjust the ADFS configuration to work with it but for me there was no other oppertunity than setting wantAuthnResponseSignedto false

[srd90]

AFAIK @markstos has proposed to move all IdP implementation specific guides to wiki section (like e.g. https://github.com/node-saml/passport-saml/wiki/How-to-use-with-Auth0 ) so this could be good moment to move ADFS specific guide also.

@1nbuc about the change that you proposed

+      // otherwise it will thow because of invalid document signature
+      wantAuthnResponseSigned: false,

and especially about this code comment:

// otherwise it will thow because of invalid document signature

it would be better to tell why this happens. Reason why this happens is that your IdP is signing only assertion. I.e. it does not sign whole authnresponse. Starting from 4.0.0 passport-saml was changed to require by default response and assertion signatures and it is up to developer to configure passport-saml to be compatible with how IdP is signing OR to (re-)configure IdP side.

I am not an ADFS user but based on quick web search it seems that it might be possible to configure ADFS to sign only assertion, only message or both. See e.g.:

• Set-AdfsRelyingParty powershell command's -SamlResponseSignature switch documentation
  • https://learn.microsoft.com/en-us/powershell/module/adfs/set-adfsrelyingpartytrust?view=windowsserver2022-ps#-samlresponsesignature
• and some random message from Microsoft forums for an example of effects of aforementioned choices
  • https://learn.microsoft.com/en-us/answers/questions/483078/(adfs)(saml)(response)(signature)-adfs-doesnt-send#answer-483504

So if your example is stating that ”otherwise you get invalid signature error” it might not be the case for someone else and in some case someone's ADFS might be configured to sign both in which case - when that someone has just copy pasted your example without understanding - his/her configuration would be less secure than what it could have been.

Having said all that this is not ADFS specific problem at all. It is related to how well developers know authentication protocol that they are using to secure their site. They should be aware of signatures and other security related aspects before implementing SAML and there are far better sites for that documentation than passport-saml project.

fwiw, there has been quite a few issues relating this new behaviour:

• [BUG] When validating the SAML Response, it is assumed that the SAML Response ID attribute is the same as the SAML Assertion ID attribute node-saml#211
• [BUG] Invalid document signature after upgrading from 2.2.0 to 4.0.1 #816
  • fwiw, this issue's discussion contains hint how to configure Azure AD signing
• [BUG] After an upgrade from passport-saml to @node-saml/passport-saml, assertion validation stopped working with HTTP-POST binding with an error: SAML.validatePostResponseAsync, Invalid document signature #839

[srd90]

Forgot to mention that if someone's ADFS would have been configured to sign only message and that someone would have copy pasted proposed example configuration as-is then there would have been another issue about incorrect example configuration due signature validation error because assertion is not signed and passport-saml default for wantAssertionsSigned is true. I.e. IdP (ADFS) would be signing only message but copy pasted configuration would have effectively this configuration: wantAuthnResponseSigned = false and wantAssertionsSigned = true whereas in that particular case effective configuration should be wantAuthnResponseSigned = true and wantAssertionsSigned = false.

[1nbuc]

Honestly I didn't know about the possibility to change the signing behaviour, I added the powershell script to the docs and removed the insecure configuration.

[cjbarth]

@1nbuc , for record-keeping purposes, can you please adjust the description of this PR to follow the directions?

[cjbarth]

This document has been moved to the Wiki.

[codecov]

Codecov Report

Merging #840 (6f9a2b0) into master (91b1ba6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #840   +/-   ##
=======================================
  Coverage   65.10%   65.10%           
=======================================
  Files           4        4           
  Lines         149      149           
  Branches       37       37           
=======================================
  Hits           97       97           
  Misses         29       29           
  Partials       23       23           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more