spring-projects / spring-projects/spring-security

Make name resolution configurable in OpenSamlLogoutRequestValidator

Open
#12,128 4 comments 2 reactions 1 assignee View on GitHub

@jzheaux is already working on this.

Since Jan 10, 2023.

in: saml2 type: enhancement
Dominant language
Java
Stars
9.6k
Forks
6.3k
Avg merge
2d 11h
Merged PRs (30d)
52

Description

Describe the bug
Spring SAML considers NameID to hold username, populates Saml2AuthenticatedPrincipal#name with NameID value and later in Single Logout flow again populates/validates NameID value using Principal Name. This behaviour breaks our current integrations.

Username can be released by IdP in one of Assertion’s Attribute element instead. NameID, if present (as it is even optional, as also discussed in https://github.com/spring-projects/spring-security/issues/11463), can be of different Format, holding different kind of values.

One example for all: According to SAML spec, NameID Format urn:oasis:names:tc:SAML:2.0:nameid-format:transient indicates that the content of the element is an identifier with transient semantics and SHOULD be treated as an opaque and temporary value by the relying party. This NameID element is also sent by IdP in LogoutRequest (causing validation against Principal Name in OpenSamlLogoutRequestValidator#validateNameId to fail) and is expected to be present in LogoutRequest sent from relying party (where it is populated with Principal Name in OpenSamlLogoutRequestResolver, making the IdP refuse the request).

Our workarounds:

  1. Login: Custom responseAuthenticationConverter to retrieve username from Attribute + storing NameID element for later use.
  2. Outbound LogoutRequest: Overriding NameID element with correct one stored during login.
  3. Inbound LogoutRequest: There is no easy or clean way to work around this, because OpenSamlLogoutRequestValidator is not much configurable.

To Reproduce
Reproduced when integrating with Shibboleth IdP, which uses transient NameID and an Attribute to release username.

Expected behavior
Whole NameID element could be stored in DefaultSaml2AuthenticatedPrincipal (similar as session indexes are stored already) during login. It would be used to construct NameID element in outbound LogoutRequest, and it’s value would be used for validation when handling inbound LogoutRequest.

Ideally could you consider also to configure where to retrieve the username from in the first place (NameID element versus providing a Name of an Assertion Attribute)?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.