Skip to content

match saml sso issuer exactly instead of by prefix#3281

Open
dxbjavid wants to merge 1 commit into
apache:mainfrom
dxbjavid:saml-sso-issuer-exact-match
Open

match saml sso issuer exactly instead of by prefix#3281
dxbjavid wants to merge 1 commit into
apache:mainfrom
dxbjavid:saml-sso-issuer-exact-match

Conversation

@dxbjavid

@dxbjavid dxbjavid commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

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.

// 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())) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coheigea could you please take a look? I believe you have changed exact much to prefix here [1], thank you

[1] 9253b78

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants