Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow retrieving username from SAML Assertion Attributes #12136

Open
palakova opened this issue Nov 3, 2022 · 3 comments
Open

Allow retrieving username from SAML Assertion Attributes #12136

palakova opened this issue Nov 3, 2022 · 3 comments
Assignees
Labels
in: saml2 An issue in SAML2 modules type: enhancement A general enhancement

Comments

@palakova
Copy link
Contributor

palakova commented Nov 3, 2022

Expected Behavior

Username (in the sense of Principal Name) can be released by IdP AuthnRequest's Assertion in NameID element or one of the Attribute element instead. NameID, if present (as it is even optional, as also discussed in #11463), can be of different Format, holding different kind of values.

It would be nice to have a more convenient way to configure where to retrieve the username from - NameID element versus providing a Name of an Assertion Attribute that is expected to hold the username.

Current Behavior

Spring SAML considers only NameID element to hold username, and populates Saml2AuthenticatedPrincipal#name with NameID value in OpenSaml4AuthenticationProvider.

Context

Example for integration with Shibboleth: IdP releases NameID Format urn:oasis:names:tc:SAML:2.0:nameid-format:transient (according to SAML spec, this 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). Username is released in one of Assertion Attribute instead.

Workaround: custom responseAuthenticationConverter to retrieve username from Attribute

@palakova palakova added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Nov 3, 2022
@marcusdacoregio marcusdacoregio added in: saml2 An issue in SAML2 modules and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 4, 2022
@marcusdacoregio marcusdacoregio added this to the 6.1.0-M1 milestone Nov 4, 2022
@jzheaux jzheaux self-assigned this Jan 10, 2023
@marcusdacoregio marcusdacoregio modified the milestones: 6.1.0-M1, 6.1.0-M2 Jan 16, 2023
@jzheaux jzheaux removed this from the 6.1.0-M2 milestone Mar 20, 2023
@AkashB23
Copy link

AkashB23 commented Dec 1, 2023

@jzheaux in which version will the changes be available for adoption ?

@jzheaux
Copy link
Contributor

jzheaux commented Jan 23, 2024

Sorry for the delay in my response, @AkashB23.

Because an application can configure a response authentication converter already, it hasn't been as high of a priority, though I'm happy to help you with a PR if you are interested.

I think what would likely make the most sense is moving the default response authentication converter support into a separate class and then adding a setter to it like so (pseudocode):

class ResponseAuthenticationConverter implements Converter<ResponseToken, AbstractAuthenticationToken> {
    ...

    public void setPrincipalNameConverter(Converter<AssertionToken, String> principalNameConverter) {
        ...
    }
}

@AkashB23
Copy link

Hi @jzheaux
Yes, I can provide my own ResponseAuthenticationConverter to get the email from whatever attribute I want but as I mentioned in [https://github.com//issues/14199]

even before the control is passed to converter, an assertion without NameId policy from Idp will fail at https://github.com/spring-projects/spring-security/blob/515e8216b18a6b7757f69a949014ede74c9676aa/saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/OpenSaml4AuthenticationProvider.java#L649

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: saml2 An issue in SAML2 modules type: enhancement A general enhancement
Projects
Status: No status
Development

No branches or pull requests

4 participants