Merge pull request #2298 from mposolda/master

KEYCLOAK-1928 Fix Saml with IBM JDK

common/src/main/java/org/keycloak/common/util/Environment.java

@@ -0,0 +1,27 @@
+/*
+ * Copyright 2016 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;
+
+/**
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+public class Environment {
+
+    public static final boolean IS_IBM_JAVA = System.getProperty("java.vendor").contains("IBM");
+
+}

common/src/main/java/org/keycloak/common/util/KerberosJdkProvider.java

@@ -56,7 +56,7 @@ public abstract class KerberosJdkProvider {
         return kerberosTicketToGSSCredential(kerberosTicket, GSSCredential.DEFAULT_LIFETIME, GSSCredential.INITIATE_ONLY);
     }
 
-    // Actually same on both JDKs
+    // Actually can use same on both JDKs
     public GSSCredential kerberosTicketToGSSCredential(KerberosTicket kerberosTicket, final int lifetime, final int usage) {
         try {
             final GSSManager gssManager = GSSManager.getInstance();
@@ -85,7 +85,7 @@ public abstract class KerberosJdkProvider {
 
 
     public static KerberosJdkProvider getProvider() {
-        if (KerberosSerializationUtils.JAVA_INFO.contains("IBM")) {
+        if (Environment.IS_IBM_JAVA) {
             return new IBMJDKProvider();
         } else {
             return new SunJDKProvider();

saml-core/src/main/java/org/keycloak/saml/common/parsers/AbstractParser.java

@@ -16,6 +16,7 @@
  */
 package org.keycloak.saml.common.parsers;
 
+import org.keycloak.common.util.Environment;
 import org.keycloak.saml.common.PicketLinkLogger;
 import org.keycloak.saml.common.PicketLinkLoggerFactory;
 import org.keycloak.saml.common.constants.GeneralConstants;
@@ -29,6 +30,8 @@ import javax.xml.stream.XMLInputFactory;
 import javax.xml.stream.XMLStreamException;
 import javax.xml.stream.events.Characters;
 import javax.xml.stream.events.XMLEvent;
+import javax.xml.stream.util.EventReaderDelegate;
+
 import java.io.InputStream;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
@@ -83,28 +86,10 @@ public abstract class AbstractParser implements ParserNamespaceSupport {
         if (configStream == null)
             throw logger.nullArgumentError("InputStream");
 
-        XMLInputFactory xmlInputFactory = getXMLInputFactory();
-
         XMLEventReader xmlEventReader = StaxParserUtil.getXMLEventReader(configStream);
 
         try {
-            xmlEventReader = xmlInputFactory.createFilteredReader(xmlEventReader, new EventFilter() {
+            xmlEventReader = filterWhitespaces(xmlEventReader);
-                public boolean accept(XMLEvent xmlEvent) {
-                    // We are going to disregard characters that are new line and whitespace
-                    if (xmlEvent.isCharacters()) {
-                        Characters chars = xmlEvent.asCharacters();
-                        String data = chars.getData();
-                        data = valid(data) ? data.trim() : null;
-                        return valid(data);
-                    } else {
-                        return xmlEvent.isStartElement() || xmlEvent.isEndElement();
-                    }
-                }
-
-                private boolean valid(String str) {
-                    return str != null && str.length() > 0;
-                }
-            });
         } catch (XMLStreamException e) {
             throw logger.parserException(e);
         }
@@ -137,4 +122,48 @@ public abstract class AbstractParser implements ParserNamespaceSupport {
         }
     }
 
+    protected XMLEventReader filterWhitespaces(XMLEventReader xmlEventReader) throws XMLStreamException {
+        XMLInputFactory xmlInputFactory = getXMLInputFactory();
+
+        xmlEventReader = xmlInputFactory.createFilteredReader(xmlEventReader, new EventFilter() {
+            public boolean accept(XMLEvent xmlEvent) {
+                // We are going to disregard characters that are new line and whitespace
+                if (xmlEvent.isCharacters()) {
+                    Characters chars = xmlEvent.asCharacters();
+                    String data = chars.getData();
+                    data = valid(data) ? data.trim() : null;
+                    return valid(data);
+                } else {
+                    return xmlEvent.isStartElement() || xmlEvent.isEndElement();
+                }
+            }
+
+            private boolean valid(String str) {
+                return str != null && str.length() > 0;
+            }
+
+        });
+
+        // Handle IBM JDK bug with Stax parsing when EventReader presented
+        if (Environment.IS_IBM_JAVA) {
+            final XMLEventReader origReader = xmlEventReader;
+
+            xmlEventReader = new EventReaderDelegate(origReader) {
+
+                @Override
+                public boolean hasNext() {
+                    boolean hasNext = super.hasNext();
+                    try {
+                        return hasNext && (origReader.peek() != null);
+                    } catch (XMLStreamException xse) {
+                        throw new IllegalStateException(xse);
+                    }
+                }
+
+            };
+        }
+
+        return xmlEventReader;
+    }
+
 }

saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/metadata/AbstractDescriptorParser.java

@@ -35,28 +35,8 @@ import javax.xml.stream.events.XMLEvent;
 public abstract class AbstractDescriptorParser extends AbstractParser {
 
     protected XMLEventReader filterWhiteSpaceCharacters(XMLEventReader xmlEventReader) throws ParsingException {
-
-        XMLInputFactory xmlInputFactory = getXMLInputFactory();
-
         try {
-            xmlEventReader = xmlInputFactory.createFilteredReader(xmlEventReader, new EventFilter() {
+            return filterWhitespaces(xmlEventReader);
-                public boolean accept(XMLEvent xmlEvent) {
-                    // We are going to disregard characters that are new line and whitespace
-                    if (xmlEvent.isCharacters()) {
-                        Characters chars = xmlEvent.asCharacters();
-                        String data = chars.getData();
-                        data = valid(data) ? data.trim() : null;
-                        return valid(data);
-                    } else {
-                        return xmlEvent.isStartElement() || xmlEvent.isEndElement();
-                    }
-                }
-
-                private boolean valid(String str) {
-                    return str != null && str.length() > 0;
-                }
-            });
-            return xmlEventReader;
         } catch (XMLStreamException e) {
             throw new ParsingException(e);
         }

services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java

@@ -425,7 +425,6 @@ public class SAMLEndpoint {
         @Override
         protected SAMLDocumentHolder extractResponseDocument(String response) {
             byte[] samlBytes = PostBindingUtil.base64Decode(response);
-            String xml = new String(samlBytes);
             return SAMLRequestParser.parseResponseDocument(samlBytes);
         }
 

services/src/test/java/org/keycloak/test/broker/saml/SAMLParsingTest.java

@@ -0,0 +1,41 @@
+/*
+ * Copyright 2016 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.test.broker.saml;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.keycloak.saml.SAMLRequestParser;
+import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder;
+import org.keycloak.saml.processing.web.util.PostBindingUtil;
+
+/**
+ * This was failing on IBM JDK
+ * 
+ * @author <a href="mailto:mposolda@redhat.com">Marek Posolda</a>
+ */
+public class SAMLParsingTest {
+
+    private static final String SAML_RESPONSE = "PHNhbWxwOkxvZ291dFJlc3BvbnNlIHhtbG5zOnNhbWxwPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6cHJvdG9jb2wiIHhtbG5zPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiBEZXN0aW5hdGlvbj0iaHR0cDovL2xvY2FsaG9zdDo4MDgxL2F1dGgvcmVhbG1zL3JlYWxtLXdpdGgtYnJva2VyL2Jyb2tlci9rYy1zYW1sLWlkcC1iYXNpYy9lbmRwb2ludCIgSUQ9IklEXzlhMTcxZDIzLWM0MTctNDJmNS05YmNhLWMwOTMxMjNmZDY4YyIgSW5SZXNwb25zZVRvPSJJRF9iYzczMDcxMS0yMDM3LTQzZjMtYWQ3Ni03YmMzMzg0MmZiODciIElzc3VlSW5zdGFudD0iMjAxNi0wMi0yOVQxMjowMDoxNC4wNDRaIiBWZXJzaW9uPSIyLjAiPjxzYW1sOklzc3VlciB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIj5odHRwOi8vbG9jYWxob3N0OjgwODIvYXV0aC9yZWFsbXMvcmVhbG0td2l0aC1zYW1sLWlkcC1iYXNpYzwvc2FtbDpJc3N1ZXI+PHNhbWxwOlN0YXR1cz48c2FtbHA6U3RhdHVzQ29kZSBWYWx1ZT0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnN0YXR1czpTdWNjZXNzIi8+PC9zYW1scDpTdGF0dXM+PC9zYW1scDpMb2dvdXRSZXNwb25zZT4=";
+
+    @Test
+    public void parseTest() throws Exception {
+        byte[] samlBytes = PostBindingUtil.base64Decode(SAML_RESPONSE);
+        SAMLDocumentHolder holder = SAMLRequestParser.parseResponseDocument(samlBytes);
+        Assert.assertNotNull(holder);
+    }
+}

testsuite/integration/src/test/java/org/keycloak/testsuite/saml/SamlPicketlinkSPTest.java

@@ -22,8 +22,13 @@ import org.junit.Assert;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.RuleChain;
+import org.junit.rules.TestRule;
+import org.junit.runner.Description;
+import org.junit.runners.model.Statement;
 import org.keycloak.admin.client.Keycloak;
 import org.keycloak.admin.client.resource.RealmResource;
+import org.keycloak.common.util.Environment;
 import org.keycloak.models.ClientModel;
 import org.keycloak.models.Constants;
 import org.keycloak.models.ProtocolMapperModel;
@@ -66,7 +71,28 @@ import static org.junit.Assert.assertEquals;
  */
 public class SamlPicketlinkSPTest {
 
-    @ClassRule
+    // This test is ignored in IBM JDK due to the IBM JDK bug, which is handled in Keycloak SP ( org.keycloak.saml.common.parsers.AbstractParser ) but not in Picketlink SP
+    public static TestRule ignoreIBMJDK = new TestRule() {
+
+        @Override
+        public Statement apply(final Statement base, final Description description) {
+            return new Statement() {
+
+                @Override
+                public void evaluate() throws Throwable {
+                    if (Environment.IS_IBM_JAVA) {
+                        System.err.println("Ignore " + description.getDisplayName() + " because executing on IBM JDK");
+                    } else {
+                        base.evaluate();
+                    }
+                }
+
+            };
+        }
+
+    };
+
+
     public static SamlKeycloakRule keycloakRule = new SamlKeycloakRule() {
         @Override
         public void initWars() {
@@ -97,6 +123,12 @@ public class SamlPicketlinkSPTest {
         }
     };
 
+    @ClassRule
+    public static TestRule chain = RuleChain
+            .outerRule(ignoreIBMJDK)
+            .around(keycloakRule);
+
+
     public static class SamlSPFacade extends HttpServlet {
         public static String samlResponse;
         public static String RELAY_STATE = "http://test.com/foo/bar";