KEYCLOAK-7852 Use original NameId value in logout requests

2019-07-02 13:13:58 +02:00 · 2019-07-02 13:13:58 +02:00 · ca4e14fbfa
commit ca4e14fbfa
parent cc8cfd4269
13 changed files with 469 additions and 29 deletions
--- a/adapters/saml/core-public/src/main/java/org/keycloak/adapters/saml/SamlPrincipal.java
+++ b/adapters/saml/core-public/src/main/java/org/keycloak/adapters/saml/SamlPrincipal.java
@ -20,7 +20,9 @@ package org.keycloak.adapters.saml;
 import org.keycloak.common.util.MultivaluedHashMap;
 import org.keycloak.dom.saml.v2.assertion.AssertionType;

+import org.keycloak.dom.saml.v2.assertion.NameIDType;
 import java.io.Serializable;
+import java.net.URI;
 import java.security.Principal;
 import java.util.Collections;
 import java.util.List;
@ -81,6 +83,27 @@ public class SamlPrincipal implements Serializable, Principal {
        return nameIDFormat;
    }

+    /**
+     * Subject nameID format
+     *
+     * @return
+     */
+    public NameIDType getNameID() {
+        if (assertion != null
+          && assertion.getSubject() != null
+          && assertion.getSubject().getSubType() != null
+          && assertion.getSubject().getSubType().getBaseID() instanceof NameIDType) {
+            return (NameIDType) assertion.getSubject().getSubType().getBaseID();
+        }
+
+        NameIDType res = new NameIDType();
+        res.setValue(getSamlSubject());
+        if (getNameIDFormat() != null) {
+            res.setFormat(URI.create(getNameIDFormat()));
+        }
+        return res;
+    }
+
    @Override
    public String getName() {
        return name;
--- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/webbrowsersso/WebBrowserSsoAuthenticationHandler.java
+++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/webbrowsersso/WebBrowserSsoAuthenticationHandler.java
@ -108,7 +108,7 @@ public class WebBrowserSsoAuthenticationHandler extends AbstractSamlAuthenticati
                .assertionExpiration(30)
                .issuer(deployment.getEntityID())
                .sessionIndex(account.getSessionIndex())
-                .userPrincipal(account.getPrincipal().getSamlSubject(), account.getPrincipal().getNameIDFormat())
+                .nameId(account.getPrincipal().getNameID())
                .destination(deployment.getIDP().getSingleLogoutService().getRequestBindingUrl());
        BaseSAML2BindingBuilder binding = new BaseSAML2BindingBuilder();
        if (deployment.getIDP().getSingleLogoutService().signRequest()) {
--- a/common/pom.xml
+++ b/common/pom.xml
@ -63,6 +63,11 @@
            <artifactId>junit</artifactId>
            <scope>test</scope>
        </dependency>
+        <dependency>
+            <groupId>org.hamcrest</groupId>
+            <artifactId>hamcrest-all</artifactId>
+            <scope>test</scope>
+        </dependency>
    </dependencies>
    <build>
        <resources>
--- a/common/src/main/java/org/keycloak/common/util/StringSerialization.java
+++ b/common/src/main/java/org/keycloak/common/util/StringSerialization.java
@ -0,0 +1,112 @@
+package org.keycloak.common.util;
+
+import java.net.URI;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Objects;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Utilities to serialize objects to string. Type safety is not guaranteed here and is responsibility of the caller.
+ * @author hmlnarik
+ */
+public class StringSerialization {
+
+    // Since there is still need to support JDK 7, we have to work without functional interfaces
+    private static enum DeSerializer {
+        OBJECT {
+            @Override public String serialize(Object o)   { return o.toString(); };
+            @Override public Object deserialize(String s) { return s; };
+        },
+        URI {
+            @Override public String serialize(Object o)   { return o.toString(); };
+            @Override public Object deserialize(String s) { return java.net.URI.create(s); };
+        },
+        ;
+
+        /** Serialize value which is guaranteed to be non-null */
+        public abstract String serialize(Object o);
+        public abstract Object deserialize(String s);
+    }
+
+    private static final Map<Class<?>, DeSerializer> WELL_KNOWN_DESERIALIZERS = new LinkedHashMap<>();
+    private static final String SEPARATOR = ";";
+    private static final Pattern ESCAPE_PATTERN = Pattern.compile(SEPARATOR);
+    private static final Pattern UNESCAPE_PATTERN = Pattern.compile(SEPARATOR + SEPARATOR);
+    private static final Pattern VALUE_PATTERN = Pattern.compile("([NV])" +
+      "(" +
+        "(?:[^" + SEPARATOR + "]|" + SEPARATOR + SEPARATOR + ")*?" +
+      ")($|" + SEPARATOR + "(?!" + SEPARATOR + "))",
+      Pattern.DOTALL
+    );
+
+    static {
+        WELL_KNOWN_DESERIALIZERS.put(URI.class, DeSerializer.URI);
+        WELL_KNOWN_DESERIALIZERS.put(String.class, DeSerializer.OBJECT);
+    }
+
+    /**
+     * Serialize given objects as strings separated by {@link #SEPARATOR} according to the {@link #WELL_KNOWN_SERIALIZERS}.
+     * @param toSerialize
+     * @return
+     */
+    public static String serialize(Object... toSerialize) {
+        StringBuilder sb = new StringBuilder();
+
+        for (int i = 0; i < toSerialize.length; i ++) {
+            Object o = toSerialize[i];
+            String stringO = getStringFrom(o);
+            String escapedStringO = ESCAPE_PATTERN.matcher(stringO).replaceAll(SEPARATOR + SEPARATOR);
+            sb.append(escapedStringO);
+
+            if (i < toSerialize.length - 1) {
+                sb.append(SEPARATOR);
+            }
+        }
+
+        return sb.toString();
+    }
+
+    public static Deserializer deserialize(String what) {
+        return new Deserializer(what);
+    }
+
+    private static String getStringFrom(Object o) {
+        if (o == null) {
+            return "N";
+        }
+
+        Class<?> c = o.getClass();
+        DeSerializer f = WELL_KNOWN_DESERIALIZERS.get(c);
+        return "V" + (f == null ? o : f.serialize(o));
+    }
+
+    private static <T> T getObjectFrom(String escapedString, Class<T> clazz) {
+        DeSerializer f = WELL_KNOWN_DESERIALIZERS.get(clazz);
+        Object res = f == null ? escapedString : f.deserialize(escapedString);
+        return clazz.cast(res);
+    }
+
+    public static class Deserializer {
+
+        private final Matcher valueMatcher;
+
+        public Deserializer(String what) {
+            this.valueMatcher = VALUE_PATTERN.matcher(what);
+        }
+
+        public <T> T next(Class<T> clazz) {
+            if (! this.valueMatcher.find()) {
+                return null;
+            }
+            String valueOrNull = this.valueMatcher.group(1);
+            if (valueOrNull == null || Objects.equals(valueOrNull, "N")) {
+                return null;
+            }
+            String escapedStringO = this.valueMatcher.group(2);
+            String unescapedStringO = UNESCAPE_PATTERN.matcher(escapedStringO).replaceAll(SEPARATOR);
+            return getObjectFrom(unescapedStringO, clazz);
+        }
+    }
+}
--- a/common/src/test/java/org/keycloak/common/util/StringSerializationTest.java
+++ b/common/src/test/java/org/keycloak/common/util/StringSerializationTest.java
@ -0,0 +1,89 @@
+/*
+ * Copyright 2019 Red Hat, Inc. and/or its affiliates
+ * and other contributors as indicated by the @author tags.
+ * 
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * 
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * 
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.keycloak.common.util;
+
+import org.keycloak.common.util.StringSerialization.Deserializer;
+import java.net.URI;
+import org.junit.Test;
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.nullValue;
+import static org.junit.Assert.assertThat;
+
+/**
+ *
+ * @author hmlnarik
+ */
+public class StringSerializationTest {
+
+    @Test
+    public void testString() {
+        String a = "aa";
+        String b = "a:\na";
+        String c = null;
+        String d = "a::a";
+        String e = "::";
+
+        String serialized = StringSerialization.serialize(a, b, c, d, e);
+        Deserializer deserializer = StringSerialization.deserialize(serialized);
+
+        assertThat(deserializer.next(String.class), is(a));
+        assertThat(deserializer.next(String.class), is(b));
+        assertThat(deserializer.next(String.class), is(c));
+        assertThat(deserializer.next(String.class), is(d));
+        assertThat(deserializer.next(String.class), is(e));
+        assertThat(deserializer.next(String.class), nullValue());
+    }
+
+    @Test
+    public void testStringWithSeparators() {
+        String a = ";;";
+        String b = "a;a";
+        String c = null;
+        String d = "a;;a";
+        String e = ";;";
+
+        String serialized = StringSerialization.serialize(a, b, c, d, e);
+        Deserializer deserializer = StringSerialization.deserialize(serialized);
+
+        assertThat(deserializer.next(String.class), is(a));
+        assertThat(deserializer.next(String.class), is(b));
+        assertThat(deserializer.next(String.class), is(c));
+        assertThat(deserializer.next(String.class), is(d));
+        assertThat(deserializer.next(String.class), is(e));
+        assertThat(deserializer.next(String.class), nullValue());
+    }
+
+    @Test
+    public void testStringUri() {
+        String a = ";;";
+        String b = "a;a";
+        String c = null;
+        URI d = URI.create("http://my.domain.com");
+        String e = ";;";
+
+        String serialized = StringSerialization.serialize(a, b, c, d, e);
+        Deserializer deserializer = StringSerialization.deserialize(serialized);
+
+        assertThat(deserializer.next(String.class), is(a));
+        assertThat(deserializer.next(String.class), is(b));
+        assertThat(deserializer.next(String.class), is(c));
+        assertThat(deserializer.next(URI.class), is(d));
+        assertThat(deserializer.next(String.class), is(e));
+        assertThat(deserializer.next(String.class), nullValue());
+    }
+
+}
--- a/saml-core-api/src/main/java/org/keycloak/dom/saml/v2/assertion/NameIDType.java
+++ b/saml-core-api/src/main/java/org/keycloak/dom/saml/v2/assertion/NameIDType.java
@ -16,6 +16,8 @@
 */
 package org.keycloak.dom.saml.v2.assertion;

+import org.keycloak.common.util.StringSerialization;
+import org.keycloak.common.util.StringSerialization.Deserializer;
 import java.net.URI;

 /**
@ -69,4 +71,25 @@ public class NameIDType extends BaseIDAbstractType {
    public void setSPProvidedID(String sPProvidedID) {
        this.sPProvidedID = sPProvidedID;
    }
+
+    public String serializeAsString() {
+        return StringSerialization.serialize(
+          getNameQualifier(),
+          getSPNameQualifier(),
+          value,
+          format,
+          sPProvidedID
+        );
+    }
+
+    public static NameIDType deserializeFromString(String s) {
+        NameIDType res = new NameIDType();
+        Deserializer d = StringSerialization.deserialize(s);
+        res.setNameQualifier(d.next(String.class));
+        res.setSPNameQualifier(d.next(String.class));
+        res.setValue(d.next(String.class));
+        res.setFormat(d.next(URI.class));
+        res.setSPProvidedID(d.next(String.class));
+        return res;
+    }
 }
--- a/saml-core/src/main/java/org/keycloak/saml/SAML2LogoutRequestBuilder.java
+++ b/saml-core/src/main/java/org/keycloak/saml/SAML2LogoutRequestBuilder.java
@ -36,8 +36,7 @@ import org.keycloak.dom.saml.v2.protocol.ExtensionsType;
 * @version $Revision: 1 $
 */
 public class SAML2LogoutRequestBuilder implements SamlProtocolExtensionsAwareBuilder<SAML2LogoutRequestBuilder> {
-    protected String userPrincipal;
-    protected String userPrincipalFormat;
+    protected NameIDType nameId;
    protected String sessionIndex;
    protected long assertionExpiration;
    protected String destination;
@ -72,10 +71,26 @@ public class SAML2LogoutRequestBuilder implements SamlProtocolExtensionsAwareBui
        return this;
    }

+    /**
+     *
+     * @param userPrincipal
+     * @param userPrincipalFormat
+     * @return
+     * @deprecated Prefer {@link #nameId(org.keycloak.dom.saml.v2.assertion.NameIDType)}
+     */
+    @Deprecated
+    public SAML2LogoutRequestBuilder userPrincipal(String userPrincipal, String userPrincipalFormat) {
+        NameIDType nid = new NameIDType();
+        nid.setValue(userPrincipal);
+        if (userPrincipalFormat != null) {
+            nid.setFormat(URI.create(userPrincipalFormat));
+        }
+        
+        return nameId(nid);
+    }

-    public SAML2LogoutRequestBuilder userPrincipal(String nameID, String nameIDformat) {
-        this.userPrincipal = nameID;
-        this.userPrincipalFormat = nameIDformat;
+    public SAML2LogoutRequestBuilder nameId(NameIDType nameId) {
+        this.nameId = nameId;
        return this;
    }

@ -92,14 +107,7 @@ public class SAML2LogoutRequestBuilder implements SamlProtocolExtensionsAwareBui
    private LogoutRequestType createLogoutRequest() throws ConfigurationException {
        LogoutRequestType lort = SAML2Request.createLogoutRequest(issuer);

-        NameIDType nameID = new NameIDType();
-        nameID.setValue(userPrincipal);
-        //Deal with NameID Format
-        String nameIDFormat = userPrincipalFormat;
-        if (nameIDFormat != null) {
-            nameID.setFormat(URI.create(nameIDFormat));
-        }
-        lort.setNameID(nameID);
+        lort.setNameID(nameId);

        if (issuer != null) {
            NameIDType issuerID = new NameIDType();
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
@ -104,8 +104,11 @@ import org.w3c.dom.NodeList;
 public class SAMLEndpoint {
    protected static final Logger logger = Logger.getLogger(SAMLEndpoint.class);
    public static final String SAML_FEDERATED_SESSION_INDEX = "SAML_FEDERATED_SESSION_INDEX";
+    @Deprecated // in favor of SAML_FEDERATED_SUBJECT_NAMEID
    public static final String SAML_FEDERATED_SUBJECT = "SAML_FEDERATED_SUBJECT";
+    @Deprecated // in favor of SAML_FEDERATED_SUBJECT_NAMEID
    public static final String SAML_FEDERATED_SUBJECT_NAMEFORMAT = "SAML_FEDERATED_SUBJECT_NAMEFORMAT";
+    public static final String SAML_FEDERATED_SUBJECT_NAMEID = "SAML_FEDERATED_SUBJECT_NAME_ID";
    public static final String SAML_LOGIN_RESPONSE = "SAML_LOGIN_RESPONSE";
    public static final String SAML_ASSERTION = "SAML_ASSERTION";
    public static final String SAML_IDP_INITIATED_CLIENT_ID = "SAML_IDP_INITIATED_CLIENT_ID";
--- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
+++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
@ -129,8 +129,7 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider<SAMLIdentityP
        SubjectType subject = assertion.getSubject();
        SubjectType.STSubType subType = subject.getSubType();
        NameIDType subjectNameID = (NameIDType) subType.getBaseID();
-        authSession.setUserSessionNote(SAMLEndpoint.SAML_FEDERATED_SUBJECT, subjectNameID.getValue());
-        if (subjectNameID.getFormat() != null) authSession.setUserSessionNote(SAMLEndpoint.SAML_FEDERATED_SUBJECT_NAMEFORMAT, subjectNameID.getFormat().toString());
+        authSession.setUserSessionNote(SAMLEndpoint.SAML_FEDERATED_SUBJECT_NAMEID, subjectNameID.serializeAsString());
        AuthnStatementType authn =  (AuthnStatementType)context.getContextData().get(SAMLEndpoint.SAML_AUTHN_STATEMENT);
        if (authn != null && authn.getSessionIndex() != null) {
            authSession.setUserSessionNote(SAMLEndpoint.SAML_FEDERATED_SESSION_INDEX, authn.getSessionIndex());
@ -191,7 +190,7 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider<SAMLIdentityP
                .assertionExpiration(realm.getAccessCodeLifespan())
                .issuer(getEntityId(uriInfo, realm))
                .sessionIndex(userSession.getNote(SAMLEndpoint.SAML_FEDERATED_SESSION_INDEX))
-                .userPrincipal(userSession.getNote(SAMLEndpoint.SAML_FEDERATED_SUBJECT), userSession.getNote(SAMLEndpoint.SAML_FEDERATED_SUBJECT_NAMEFORMAT))
+                .nameId(NameIDType.deserializeFromString(userSession.getNote(SAMLEndpoint.SAML_FEDERATED_SUBJECT_NAMEID)))
                .destination(singleLogoutServiceUrl);
        return logoutBuilder;
    }
--- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClientBuilder.java
+++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/SamlClientBuilder.java
@ -17,6 +17,7 @@
 package org.keycloak.testsuite.util;

 import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
+import org.keycloak.testsuite.page.AbstractPage;
 import org.keycloak.testsuite.util.SamlClient.Binding;
 import org.keycloak.testsuite.util.SamlClient.DoNotFollowRedirectStep;
 import org.keycloak.testsuite.util.SamlClient.ResultExtractor;
@ -194,6 +195,10 @@ public class SamlClientBuilder {
        return this;
    }

+    public SamlClientBuilder navigateTo(AbstractPage page) {
+        return navigateTo(page.buildUri());
+    }
+
    public SamlClientBuilder navigateTo(URI httpGetUri) {
        steps.add((client, currentURI, currentResponse, context) -> new HttpGet(httpGetUri));
        return this;
--- a/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/CreateLogoutRequestStepBuilder.java
+++ b/testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/saml/CreateLogoutRequestStepBuilder.java
@ -97,13 +97,8 @@ public class CreateLogoutRequestStepBuilder extends SamlDocumentStepBuilder<Logo
        SAML2LogoutRequestBuilder builder = new SAML2LogoutRequestBuilder()
          .destination(authServerSamlUrl.toString())
          .issuer(issuer)
-          .sessionIndex(sessionIndex());
-
-        final NameIDType nameIdValue = nameId();
-
-        if (nameIdValue != null) {
-            builder = builder.userPrincipal(nameIdValue.getValue(), nameIdValue.getFormat() == null ? null : nameIdValue.getFormat().toString());
-        }
+          .sessionIndex(sessionIndex())
+          .nameId(nameId());

        String documentAsString = DocumentUtil.getDocumentAsString(builder.buildDocument());
        String transformed = getTransformer().transform(documentAsString);
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLLogoutAdapterTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/SAMLLogoutAdapterTest.java
@ -0,0 +1,111 @@
+package org.keycloak.testsuite.adapter.servlet;
+
+import org.keycloak.dom.saml.v2.SAML2Object;
+import org.keycloak.dom.saml.v2.assertion.AssertionType;
+import org.keycloak.dom.saml.v2.assertion.NameIDType;
+import org.keycloak.dom.saml.v2.protocol.LogoutRequestType;
+import org.keycloak.dom.saml.v2.protocol.ResponseType;
+import org.keycloak.representations.idm.RealmRepresentation;
+import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
+import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
+import org.keycloak.testsuite.adapter.AbstractServletsAdapterTest;
+import org.keycloak.testsuite.adapter.page.EmployeeServlet;
+import org.keycloak.testsuite.arquillian.annotation.AppServerContainer;
+import org.keycloak.testsuite.util.SamlClient.Binding;
+import org.keycloak.testsuite.util.SamlClientBuilder;
+import org.keycloak.testsuite.utils.arquillian.ContainerConstants;
+import org.keycloak.testsuite.utils.io.IOUtil;
+import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
+import org.hamcrest.Matchers;
+import org.jboss.arquillian.container.test.api.Deployment;
+import org.jboss.arquillian.graphene.page.Page;
+import org.jboss.shrinkwrap.api.spec.WebArchive;
+import org.junit.Test;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
+import static org.junit.Assert.assertThat;
+import static org.keycloak.testsuite.adapter.AbstractServletsAdapterTest.samlServletDeployment;
+import static org.keycloak.testsuite.util.Matchers.isSamlResponse;
+
+/**
+ *
+ * @author hmlnarik
+ */
+@AppServerContainer(ContainerConstants.APP_SERVER_UNDERTOW)
+@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY)
+@AppServerContainer(ContainerConstants.APP_SERVER_WILDFLY_DEPRECATED)
+@AppServerContainer(ContainerConstants.APP_SERVER_EAP)
+@AppServerContainer(ContainerConstants.APP_SERVER_EAP6)
+@AppServerContainer(ContainerConstants.APP_SERVER_EAP71)
+@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT7)
+@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT8)
+@AppServerContainer(ContainerConstants.APP_SERVER_TOMCAT9)
+public class SAMLLogoutAdapterTest extends AbstractServletsAdapterTest {
+
+    private static final String SP_PROVIDED_ID = "spProvidedId";
+    private static final String SP_NAME_QUALIFIER = "spNameQualifier";
+    private static final String NAME_QUALIFIER = "nameQualifier";
+
+    @Deployment(name = EmployeeServlet.DEPLOYMENT_NAME)
+    protected static WebArchive employee() {
+        return samlServletDeployment(EmployeeServlet.DEPLOYMENT_NAME, SendUsernameServlet.class);
+    }
+
+    @Page
+    private EmployeeServlet employeeServletPage;
+
+    private final AtomicReference<NameIDType> nameIdRef = new AtomicReference<>();
+
+    @Override
+    public void addAdapterTestRealms(List<RealmRepresentation> testRealms) {
+        testRealms.add(IOUtil.loadRealm("/adapter-test/keycloak-saml/testsaml.json"));
+    }
+
+    private SAML2Object extractNameId(SAML2Object so) {
+        assertThat(so, isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS));
+        ResponseType loginResp1 = (ResponseType) so;
+        final AssertionType firstAssertion = loginResp1.getAssertions().get(0).getAssertion();
+        assertThat(firstAssertion, org.hamcrest.Matchers.notNullValue());
+        assertThat(firstAssertion.getSubject().getSubType().getBaseID(), instanceOf(NameIDType.class));
+
+        NameIDType nameId = (NameIDType) firstAssertion.getSubject().getSubType().getBaseID();
+
+        nameIdRef.set(nameId);
+
+        return so;
+    }
+
+    @Test
+    public void employeeTest() {
+        SAMLDocumentHolder b = new SamlClientBuilder()
+          .navigateTo(employeeServletPage)
+          .processSamlResponse(Binding.POST)
+            .build()
+          .login().user(bburkeUser).build()
+          .processSamlResponse(Binding.POST)
+            .targetAttributeSamlResponse()
+            .transformObject(this::extractNameId)
+            .transformObject((SAML2Object o) -> {
+              assertThat(o, isSamlResponse(JBossSAMLURIConstants.STATUS_SUCCESS));
+              ResponseType rt = (ResponseType) o;
+              NameIDType t = (NameIDType) rt.getAssertions().get(0).getAssertion().getSubject().getSubType().getBaseID();
+              t.setNameQualifier(NAME_QUALIFIER);
+              t.setSPNameQualifier(SP_NAME_QUALIFIER);
+              t.setSPProvidedID(SP_PROVIDED_ID);
+              return o;
+            }).build()
+          .navigateTo(employeeServletPage.getUriBuilder().clone().queryParam("GLO", "true").build())
+          .getSamlResponse(Binding.POST);
+        
+        assertThat(b.getSamlObject(), instanceOf(LogoutRequestType.class));
+        LogoutRequestType lr = (LogoutRequestType) b.getSamlObject();
+        NameIDType logoutRequestNameID = lr.getNameID();
+        assertThat(logoutRequestNameID.getFormat(), is(nameIdRef.get().getFormat()));
+        assertThat(logoutRequestNameID.getValue(), is(nameIdRef.get().getValue()));
+        assertThat(logoutRequestNameID.getNameQualifier(), is(NAME_QUALIFIER));
+        assertThat(logoutRequestNameID.getSPProvidedID(), is(SP_PROVIDED_ID));
+        assertThat(logoutRequestNameID.getSPNameQualifier(), is(SP_NAME_QUALIFIER));
+    }
+
+}
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/LogoutTest.java
@ -71,6 +71,14 @@ import static org.keycloak.testsuite.util.SamlClient.Binding.*;
 */
 public class LogoutTest extends AbstractSamlTest {

+    private static final String SP_PROVIDED_ID = "spProvidedId";
+    private static final String SP_NAME_QUALIFIER = "spNameQualifier";
+    private static final String NAME_QUALIFIER = "nameQualifier";
+
+    private static final String BROKER_SIGN_ON_SERVICE_URL = "http://saml.idp/saml";
+    private static final String BROKER_LOGOUT_SERVICE_URL = "http://saml.idp/SLO/saml";
+    private static final String BROKER_SERVICE_ID = "http://saml.idp/saml";
+
    private ClientRepresentation salesRep;
    private ClientRepresentation sales2Rep;

@ -315,8 +323,8 @@ public class LogoutTest extends AbstractSamlTest {
          .providerId(SAMLIdentityProviderFactory.PROVIDER_ID)
          .alias(SAML_BROKER_ALIAS)
          .displayName("SAML")
-          .setAttribute(SAMLIdentityProviderConfig.SINGLE_SIGN_ON_SERVICE_URL, "http://saml.idp/saml")
-          .setAttribute(SAMLIdentityProviderConfig.SINGLE_LOGOUT_SERVICE_URL, "http://saml.idp/saml")
+          .setAttribute(SAMLIdentityProviderConfig.SINGLE_SIGN_ON_SERVICE_URL, BROKER_SIGN_ON_SERVICE_URL)
+          .setAttribute(SAMLIdentityProviderConfig.SINGLE_LOGOUT_SERVICE_URL, BROKER_LOGOUT_SERVICE_URL)
          .setAttribute(SAMLIdentityProviderConfig.NAME_ID_POLICY_FORMAT, "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress")
          .setAttribute(SAMLIdentityProviderConfig.POST_BINDING_RESPONSE, "false")
          .setAttribute(SAMLIdentityProviderConfig.POST_BINDING_AUTHN_REQUEST, "false")
@ -328,10 +336,10 @@ public class LogoutTest extends AbstractSamlTest {
    private SAML2Object createAuthnResponse(SAML2Object so) {
        AuthnRequestType req = (AuthnRequestType) so;
        try {
-            return new SAML2LoginResponseBuilder()
+            final ResponseType res = new SAML2LoginResponseBuilder()
              .requestID(req.getID())
              .destination(req.getAssertionConsumerServiceURL().toString())
-              .issuer("http://saml.idp/saml")
+              .issuer(BROKER_SERVICE_ID)
              .assertionExpiration(1000000)
              .subjectExpiration(1000000)
              .requestIssuer(getAuthServerRealmBase(REALM_NAME).toString())
@ -339,6 +347,13 @@ public class LogoutTest extends AbstractSamlTest {
              .authMethod(JBossSAMLURIConstants.AC_UNSPECIFIED.get())
              .sessionIndex("idp:" + UUID.randomUUID())
              .buildModel();
+
+            NameIDType nameId = (NameIDType) res.getAssertions().get(0).getAssertion().getSubject().getSubType().getBaseID();
+            nameId.setNameQualifier(NAME_QUALIFIER);
+            nameId.setSPNameQualifier(SP_NAME_QUALIFIER);
+            nameId.setSPProvidedID(SP_PROVIDED_ID);
+
+            return res;
        } catch (ConfigurationException | ProcessingException ex) {
            throw new RuntimeException(ex);
        }
@ -350,7 +365,7 @@ public class LogoutTest extends AbstractSamlTest {
            return new SAML2LogoutResponseBuilder()
              .logoutRequestID(req.getID())
              .destination(getSamlBrokerUrl(REALM_NAME).toString())
-              .issuer("http://saml.idp/saml")
+              .issuer(BROKER_SERVICE_ID)
              .buildModel();
        } catch (ConfigurationException ex) {
            throw new RuntimeException(ex);
@ -409,4 +424,56 @@ public class LogoutTest extends AbstractSamlTest {
        }
    }

+    @Test
+    public void testLogoutPropagatesToSamlIdentityProviderNameIdPreserved() throws IOException {
+        final RealmResource realm = adminClient.realm(REALM_NAME);
+
+        try (
+          Closeable sales = ClientAttributeUpdater.forClient(adminClient, REALM_NAME, SAML_CLIENT_ID_SALES_POST)
+          .setFrontchannelLogout(true)
+          .removeAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_POST_ATTRIBUTE)
+          .setAttribute(SamlProtocol.SAML_SINGLE_LOGOUT_SERVICE_URL_REDIRECT_ATTRIBUTE, "http://url")
+          .update();
+
+          Closeable idp = new IdentityProviderCreator(realm, addIdentityProvider())
+          ) {
+            SAMLDocumentHolder samlResponse = new SamlClientBuilder()
+              .authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, POST).build()
+
+              // Virtually perform login at IdP (return artificial SAML response)
+              .login().idp(SAML_BROKER_ALIAS).build()
+              .processSamlResponse(REDIRECT)
+                .transformObject(this::createAuthnResponse)
+                .targetAttributeSamlResponse()
+                .targetUri(getSamlBrokerUrl(REALM_NAME))
+                .build()
+              .updateProfile().username("a").email("a@b.c").firstName("A").lastName("B").build()
+              .followOneRedirect()
+
+              // Now returning back to the app
+              .processSamlResponse(POST)
+                .transformObject(this::extractNameIdAndSessionIndexAndTerminate)
+                .build()
+
+              // ----- Logout phase ------
+
+              // Logout initiated from the app
+              .logoutRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, REDIRECT)
+                .nameId(nameIdRef::get)
+                .sessionIndex(sessionIndexRef::get)
+                .build()
+
+              .getSamlResponse(REDIRECT);
+
+            assertThat(samlResponse.getSamlObject(), isSamlLogoutRequest(BROKER_LOGOUT_SERVICE_URL));
+            LogoutRequestType lr = (LogoutRequestType) samlResponse.getSamlObject();
+            NameIDType logoutRequestNameID = lr.getNameID();
+            assertThat(logoutRequestNameID.getFormat(), is(JBossSAMLURIConstants.NAMEID_FORMAT_EMAIL.getUri()));
+            assertThat(logoutRequestNameID.getValue(), is("a@b.c"));
+            assertThat(logoutRequestNameID.getNameQualifier(), is(NAME_QUALIFIER));
+            assertThat(logoutRequestNameID.getSPProvidedID(), is(SP_PROVIDED_ID));
+            assertThat(logoutRequestNameID.getSPNameQualifier(), is(SP_NAME_QUALIFIER));
+        }
+    }
+
 }