18F / identity-idp

Login.gov Core App: Identity Provider (IdP)
https://secure.login.gov/
Other
524 stars 112 forks source link

Ignore unknown authncontext (#11362) #11396

Closed Sgtpluck closed 1 week ago

Sgtpluck commented 1 week ago

This PR is reverting a revert (I was worried it was related to a problem with the sample SAML staging app, but looks like it is not the cause). The original PR comment is copied below

🎫 Ticket

Link to the relevant ticket: Ignore unknown authentication contexts Link to 1-pager

πŸ›  Summary of changes

This change updates our SAML integration to allow and ignore unknown authentication context class reference (ACR) values that are sent via the request, as long as there is at least one valid AuthnContext value that we can assert. (OIDC already allowed arbitrary values to be passed in.)

SAML spec line 1820

If Comparison is set to "exact" or omitted, then the resulting authentication context in the authentication statement MUST be the exact match of at least one of the authentication contexts specified.

This change also:

Open questions: I am currently handling the case of a partner passing in only unknown ACR values as invalid.

However, we do allow SAML partners to pass in no ACR values, in which case we use their service provider defaults, so an argument could be made that if we get only unknown ACR values, we should default to the SP's defaults.

My thinking is that if they are purposefully sending ACR values, that should override any defaults we have -- if they are only sending one, unknown ACR values that is more likely a mistake. Am open to arguments in favor/against this approach!

πŸ“œ Testing Plan

Provide a checklist of steps to confirm the changes.