match saml sso issuer exactly instead of by prefix#3281
Open
dxbjavid wants to merge 1 commit into
Open
Conversation
reta
reviewed
Jul 5, 2026
| // Issuer value must match (be contained in) Issuer IDP | ||
| if (enforceKnownIssuer && (issuer.getValue() == null || !issuerIDP.startsWith(issuer.getValue()))) { | ||
| // Issuer value must match the configured Issuer IDP | ||
| if (enforceKnownIssuer && !issuerIDP.equals(issuer.getValue())) { |
Member
Contributor
Author
There was a problem hiding this comment.
That's the commit, yes. As far as I can tell the reason for the prefix check is that validateIssuer compares the response Issuer against requestState.getIdpServiceAddress() (set in AbstractRequestAssertionConsumerHandler), which is the IdP's SSO endpoint URL rather than its entityID, and the entityID is often a prefix of that address. The trouble is that a bare startsWith accepts any prefix at all, down to a single character, so with enforceKnownIssuer on the issuer isn't really pinned to anything. If exact matching against the service address is too strict for existing deployments, a separate expected issuer property might be the safer route, but happy to rework it whichever way you and Colm prefer.
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.
validateIssuer in the SAML 2.0 SSO response validator checks the received issuer against the configured issuer IDP with issuerIDP.startsWith(issuer.getValue()), so any value that is only a prefix of the expected issuer satisfies the enforceKnownIssuer check, right down to a single character. That means the known-issuer control does not really pin the issuer, and where a service provider trusts more than one identity provider it weakens the binding between an assertion and the IdP it is meant to have come from. Compare the issuer to the configured value exactly instead, which is how SAML entityIDs are meant to be matched.