Merged
Conversation
changelog: User-Facing Improvements, Integration Experience, Allowing and ignoring unknown authn_context values
zachmargolis
approved these changes
Oct 24, 2024
kevinsmaster5
approved these changes
Oct 24, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
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.
make runmainIdP withmake runsign inAuthn context Unauthorized authentication contexterrormake runsign in