From 8d81a4a2e43eda86f5d02d2dc10d97fe6d1ddbf0 Mon Sep 17 00:00:00 2001
From: Hynek Mlnarik <hmlnarik@redhat.com>
Date: Tue, 25 Jul 2017 15:37:57 +0200
Subject: [PATCH] KEYCLOAK-5236

---
 .../AbstractSamlAuthenticationHandler.java    |  6 ++--
 .../AbstractSAMLServletsAdapterTest.java      | 36 +++++++++++++++++++
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java
index 08ce4a988b..2b40a424e8 100644
--- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java
+++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/profile/AbstractSamlAuthenticationHandler.java
@@ -407,8 +407,8 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
 
         SubjectType subject = assertion.getSubject();
         SubjectType.STSubType subType = subject.getSubType();
-        NameIDType subjectNameID = (NameIDType) subType.getBaseID();
-        String principalName = subjectNameID.getValue();
+        NameIDType subjectNameID = subType == null ? null : (NameIDType) subType.getBaseID();
+        String principalName = subjectNameID == null ? null : subjectNameID.getValue();
 
         final Set<String> roles = new HashSet<>();
         MultivaluedHashMap<String, String> attributes = new MultivaluedHashMap<>();
@@ -473,7 +473,7 @@ public abstract class AbstractSamlAuthenticationHandler implements SamlAuthentic
         }
 
 
-        URI nameFormat = subjectNameID.getFormat();
+        URI nameFormat = subjectNameID == null ? null : subjectNameID.getFormat();
         String nameFormatString = nameFormat == null ? JBossSAMLURIConstants.NAMEID_FORMAT_UNSPECIFIED.get() : nameFormat.toString();
         final SamlPrincipal principal = new SamlPrincipal(assertion, principalName, principalName, nameFormatString, attributes, friendlyAttributes);
         String index = authn == null ? null : authn.getSessionIndex();
diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java
index 77f6281056..c3a40e5393 100644
--- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java
+++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/servlet/AbstractSAMLServletsAdapterTest.java
@@ -96,6 +96,12 @@ import java.security.PublicKey;
 import java.util.*;
 import java.util.stream.Collectors;
 
+import javax.xml.xpath.XPath;
+import javax.xml.xpath.XPathConstants;
+import javax.xml.xpath.XPathExpression;
+import javax.xml.xpath.XPathFactory;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
 import static org.hamcrest.Matchers.*;
 import static org.junit.Assert.*;
 import static org.keycloak.representations.idm.CredentialRepresentation.PASSWORD;
@@ -1056,6 +1062,36 @@ public abstract class AbstractSAMLServletsAdapterTest extends AbstractServletsAd
           });
     }
 
+    @Test
+    public void testNameIDUnset() throws Exception {
+        new SamlClientBuilder()
+          .navigateTo(employee2ServletPage.toString())
+          .processSamlResponse(Binding.POST).build()
+          .login().user(bburkeUser).build()
+          .processSamlResponse(Binding.POST)
+            .transformDocument(responseDoc -> {
+                XPathFactory xPathfactory = XPathFactory.newInstance();
+                XPath xpath = xPathfactory.newXPath();
+                XPathExpression expr = xpath.compile("//*[local-name()='NameID']");
+
+                NodeList nodeList = (NodeList) expr.evaluate(responseDoc, XPathConstants.NODESET);
+                assertThat(nodeList.getLength(), is(1));
+
+                final Node nameIdNode = nodeList.item(0);
+                nameIdNode.getParentNode().removeChild(nameIdNode);
+
+                return responseDoc;
+            })
+            .build()
+
+          .navigateTo(employee2ServletPage.toString())
+
+          .execute(r -> {
+              assertThat(r, statusCodeIsHC(Response.Status.OK));
+              assertThat(r, bodyHC(allOf(containsString("principal="), not(containsString("500")))));
+          });
+    }
+
     // KEYCLOAK-4329
     @Test
     public void testEmptyKeyInfoElement() {