Skip to content

SAML 2.0 LogoutRequest should contain session indexes #10613

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

Closed
Olbix opened this issue Dec 17, 2021 · 2 comments
Closed

SAML 2.0 LogoutRequest should contain session indexes #10613

Olbix opened this issue Dec 17, 2021 · 2 comments
Assignees
Labels
in: saml2 An issue in SAML2 modules type: enhancement A general enhancement
Milestone

Comments

@Olbix
Copy link

Olbix commented Dec 17, 2021

Expected Behavior

Currently "LogoutRequest" is missing "SessionIndex" attributes, some IdPs might have a problem with accepting such requests. It would be great if we could have it implemented as it was in the currently deprecated SAML Library. IMHO, OpenSamlLogoutRequestResolver should have implemented similar logic to https://github.com/spring-projects/spring-security-saml/blob/main/core/src/main/java/org/springframework/security/saml/websso/SingleLogoutProfileImpl.java#L110

Current Behavior

Saml2LogoutRequestResolver constructs "LogoutRequest" with usage of OpenSamlLogoutRequestResolver which does not add SessionIndex to "LogoutRequest" , so that IdP returns "urn:oasis:names:tc:SAML:2.0:status:Requester" response code, and SLO is not conducted in proper way

Version

5.6

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Dec 17, 2021
@marcusdacoregio marcusdacoregio added in: saml2 An issue in SAML2 modules type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 22, 2021
@jzheaux
Copy link
Contributor

jzheaux commented Jan 3, 2022

@Olbix, thanks for the suggestion.

I think session indexes will need to be remembered in the Saml2AuthenticatedPrincipal at login time so that they can be retrieved when formulating a LogoutRequest.

Are you able to provide a PR that adds this feature?

@jzheaux jzheaux added this to the 5.7.x milestone Jan 3, 2022
@boukewoudstra
Copy link

Just sharing a workaround for the time being using a custom Saml2LogoutRequestResolver:

    // Customize the saml2 logout request resolver in order to add the sessionIndex which is needed for single logout
    @Bean
    public Saml2LogoutRequestResolver logoutRequestResolver(RelyingPartyRegistrationRepository relyingPartyRepository) {
        OpenSaml4LogoutRequestResolver logoutRequestResolver = new OpenSaml4LogoutRequestResolver(new DefaultRelyingPartyRegistrationResolver(relyingPartyRepository));
        logoutRequestResolver.setParametersConsumer((parameters) -> {
            Saml2AuthenticatedPrincipal principal = (Saml2AuthenticatedPrincipal) parameters.getAuthentication().getPrincipal();
            LogoutRequest logoutRequest = parameters.getLogoutRequest();
            SessionIndex sessionIndex = new MySessionIndex("urn:oasis:names:tc:SAML:2.0:protocol", "SessionIndex", "saml2p");
            sessionIndex.setValue(principal.getFirstAttribute("sessionIndex"));
            logoutRequest.getSessionIndexes().add(sessionIndex);
        });
        return logoutRequestResolver;
    }

In combination with a custom SessionIndex since it it a protected class:

public class MySessionIndex extends XSStringImpl implements SessionIndex {

    public MySessionIndex(String namespaceURI, String elementLocalName, String namespacePrefix) {
        super(namespaceURI, elementLocalName, namespacePrefix);
    }
}

and a custom authenticationProvider:

    @Bean
    public OpenSaml4AuthenticationProvider authenticationProvider(){
        OpenSaml4AuthenticationProvider authenticationProvider = new OpenSaml4AuthenticationProvider();
        authenticationProvider.setResponseAuthenticationConverter(responseToken -> {
            Saml2Authentication saml2Authentication = OpenSaml4AuthenticationProvider
                    .createDefaultResponseAuthenticationConverter()
                    .convert(responseToken);

            Assertion assertion = CollectionUtils.firstElement(responseToken.getResponse().getAssertions());
            AuthnStatement statement = CollectionUtils.firstElement(assertion.getAuthnStatements());
            String sessionIndex = statement.getSessionIndex();
            var customAuthorities = AuthorityUtils.createAuthorityList("ROLE_USER", "ROLE_TEST");

            DefaultSaml2AuthenticatedPrincipal principal = (DefaultSaml2AuthenticatedPrincipal) saml2Authentication.getPrincipal();
            principal.getAttributes().put("sessionIndex", List.of(sessionIndex));
            return new Saml2Authentication(principal, saml2Authentication.getSaml2Response(), customAuthorities);
        });
        return authenticationProvider;
    }

and wiring it all together:

      http
            .saml2Login(saml2 -> {
                saml2.authenticationManager(new ProviderManager(this.getApplicationContext().getBean(OpenSaml4AuthenticationProvider.class)));
            })
            .saml2Logout(saml2 -> {
                saml2.logoutRequest(logout -> {
                   logout.logoutRequestResolver(this.getApplicationContext().getBean(Saml2LogoutRequestResolver.class));
                });
            })

jzheaux added a commit to jzheaux/spring-security that referenced this issue Jan 26, 2022
jzheaux added a commit that referenced this issue Jan 28, 2022
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
None yet
Development

No branches or pull requests

5 participants