diff --git a/common/src/main/java/org/keycloak/common/util/Environment.java b/common/src/main/java/org/keycloak/common/util/Environment.java new file mode 100644 index 0000000000..94a4d453fb --- /dev/null +++ b/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 Marek Posolda + */ +public class Environment { + + public static final boolean IS_IBM_JAVA = System.getProperty("java.vendor").contains("IBM"); + +} diff --git a/common/src/main/java/org/keycloak/common/util/KerberosJdkProvider.java b/common/src/main/java/org/keycloak/common/util/KerberosJdkProvider.java index 00a5f12885..ffd42f7bbc 100644 --- a/common/src/main/java/org/keycloak/common/util/KerberosJdkProvider.java +++ b/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(); diff --git a/saml-core/src/main/java/org/keycloak/saml/common/parsers/AbstractParser.java b/saml-core/src/main/java/org/keycloak/saml/common/parsers/AbstractParser.java index 612ead9b34..5cb19deb04 100755 --- a/saml-core/src/main/java/org/keycloak/saml/common/parsers/AbstractParser.java +++ b/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() { - 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; - } - }); + xmlEventReader = filterWhitespaces(xmlEventReader); } 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; + } + } \ No newline at end of file diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/metadata/AbstractDescriptorParser.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/metadata/AbstractDescriptorParser.java index 6b9db93812..a58006ce66 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/parsers/saml/metadata/AbstractDescriptorParser.java +++ b/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() { - 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; + return filterWhitespaces(xmlEventReader); } catch (XMLStreamException e) { throw new ParsingException(e); } diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java index 11eba29b99..32d64260cf 100755 --- a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java +++ b/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); } diff --git a/services/src/test/java/org/keycloak/test/broker/saml/SAMLParsingTest.java b/services/src/test/java/org/keycloak/test/broker/saml/SAMLParsingTest.java new file mode 100644 index 0000000000..d26895ae4a --- /dev/null +++ b/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 Marek Posolda + */ +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); + } +} diff --git a/testsuite/integration/src/test/java/org/keycloak/testsuite/saml/SamlPicketlinkSPTest.java b/testsuite/integration/src/test/java/org/keycloak/testsuite/saml/SamlPicketlinkSPTest.java index 2c13fdd9ed..6ce33ec491 100755 --- a/testsuite/integration/src/test/java/org/keycloak/testsuite/saml/SamlPicketlinkSPTest.java +++ b/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";