From 3b1bdb216adb14dff916693e26a935b3320900cd Mon Sep 17 00:00:00 2001 From: vramik Date: Mon, 13 Jan 2020 13:58:20 +0100 Subject: [PATCH] KEYCLOAK-11486 Add support for system property or env variable in AllowedClockSkew in keycloak-saml subsystem --- .../subsystem/saml/as7/AllowedClockSkew.java | 8 +++---- ...systemParsingAllowedClockSkewTestCase.java | 23 +++++++++---------- .../saml/config/parsers/IdpParser.java | 2 +- .../saml/extension/AllowedClockSkew.java | 9 ++++---- ...systemParsingAllowedClockSkewTestCase.java | 23 +++++++++---------- .../saml/common/util/StaxParserUtil.java | 18 +++++++++++++++ .../adapter/AbstractServletsAdapterTest.java | 2 +- 7 files changed, 51 insertions(+), 34 deletions(-) diff --git a/adapters/saml/as7-eap6/subsystem/src/main/java/org/keycloak/subsystem/saml/as7/AllowedClockSkew.java b/adapters/saml/as7-eap6/subsystem/src/main/java/org/keycloak/subsystem/saml/as7/AllowedClockSkew.java index 8231eeeebf..997c75e2b6 100644 --- a/adapters/saml/as7-eap6/subsystem/src/main/java/org/keycloak/subsystem/saml/as7/AllowedClockSkew.java +++ b/adapters/saml/as7-eap6/subsystem/src/main/java/org/keycloak/subsystem/saml/as7/AllowedClockSkew.java @@ -32,8 +32,8 @@ abstract public class AllowedClockSkew { static final SimpleAttributeDefinition ALLOWED_CLOCK_SKEW_VALUE = new SimpleAttributeDefinitionBuilder(Constants.Model.ALLOWED_CLOCK_SKEW_VALUE, ModelType.INT, false) .setXmlName(Constants.XML.ALLOWED_CLOCK_SKEW) - .setAllowExpression(false) - .setValidator(new IntRangeValidator(1, Integer.MAX_VALUE, true, false)) + .setAllowExpression(true) + .setValidator(new IntRangeValidator(1, Integer.MAX_VALUE, true, true)) .build(); static private enum AllowedClockSkewUnits {MINUTES, SECONDS, MILLISECONDS, MICROSECONDS, NANOSECONDS}; @@ -41,9 +41,9 @@ abstract public class AllowedClockSkew { static final SimpleAttributeDefinition ALLOWED_CLOCK_SKEW_UNIT = new SimpleAttributeDefinitionBuilder(Constants.Model.ALLOWED_CLOCK_SKEW_UNIT, ModelType.STRING, true) .setXmlName(Constants.XML.ALLOWED_CLOCK_SKEW_UNIT) - .setAllowExpression(false) + .setAllowExpression(true) .setDefaultValue(new ModelNode(AllowedClockSkewUnits.SECONDS.name())) - .setValidator(EnumValidator.create(AllowedClockSkewUnits.class, true, false)) + .setValidator(EnumValidator.create(AllowedClockSkewUnits.class, true, true)) .build(); static final SimpleAttributeDefinition[] ATTRIBUTES = {ALLOWED_CLOCK_SKEW_UNIT, ALLOWED_CLOCK_SKEW_VALUE}; diff --git a/adapters/saml/as7-eap6/subsystem/src/test/java/org/keycloak/subsystem/saml/as7/SubsystemParsingAllowedClockSkewTestCase.java b/adapters/saml/as7-eap6/subsystem/src/test/java/org/keycloak/subsystem/saml/as7/SubsystemParsingAllowedClockSkewTestCase.java index 5177f35c40..068a2f5b2b 100755 --- a/adapters/saml/as7-eap6/subsystem/src/test/java/org/keycloak/subsystem/saml/as7/SubsystemParsingAllowedClockSkewTestCase.java +++ b/adapters/saml/as7-eap6/subsystem/src/test/java/org/keycloak/subsystem/saml/as7/SubsystemParsingAllowedClockSkewTestCase.java @@ -184,16 +184,15 @@ public class SubsystemParsingAllowedClockSkewTestCase extends AbstractSubsystemB testSubsystem("30", "invalid-unit"); } - // For the moment no expressions allowed as the rest of the subsystem doesn't resolve expressions - //@Test - //public void testExpression() throws Exception { - // System.setProperty("test.prop.SKEW_TIME", "30"); - // System.setProperty("test.prop.SKEW_UNIT", "MILLISECONDS"); - // try { - // testSubsystem("${test.prop.SKEW_TIME}", "${test.prop.SKEW_UNIT}", 30, "MILLISECONDS"); - // } finally { - // System.clearProperty("test.prop.SKEW_TIME"); - // System.clearProperty("test.prop.SKEW_UNIT"); - // } - //} + @Test + public void testExpression() throws Exception { + System.setProperty("test.prop.SKEW_TIME", "30"); + System.setProperty("test.prop.SKEW_UNIT", "MILLISECONDS"); + try { + testSubsystem("${test.prop.SKEW_TIME}", "${test.prop.SKEW_UNIT}", 30, "MILLISECONDS"); + } finally { + System.clearProperty("test.prop.SKEW_TIME"); + System.clearProperty("test.prop.SKEW_UNIT"); + } + } } diff --git a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/IdpParser.java b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/IdpParser.java index ae051180b3..3ccfa37496 100644 --- a/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/IdpParser.java +++ b/adapters/saml/core/src/main/java/org/keycloak/adapters/saml/config/parsers/IdpParser.java @@ -78,7 +78,7 @@ public class IdpParser extends AbstractKeycloakSamlAdapterV1Parser { String timeUnitString = StaxParserUtil.getAttributeValueRP(elementDetail, KeycloakSamlAdapterV1QNames.ATTR_UNIT); target.setAllowedClockSkewUnit(timeUnitString == null ? TimeUnit.SECONDS : TimeUnit.valueOf(timeUnitString)); StaxParserUtil.advance(xmlEventReader); - target.setAllowedClockSkew(Integer.parseInt(StaxParserUtil.getElementText(xmlEventReader))); + target.setAllowedClockSkew(Integer.parseInt(StaxParserUtil.getElementTextRP(xmlEventReader))); break; } } diff --git a/adapters/saml/wildfly/wildfly-subsystem/src/main/java/org/keycloak/subsystem/adapter/saml/extension/AllowedClockSkew.java b/adapters/saml/wildfly/wildfly-subsystem/src/main/java/org/keycloak/subsystem/adapter/saml/extension/AllowedClockSkew.java index d557078e28..455fe113eb 100644 --- a/adapters/saml/wildfly/wildfly-subsystem/src/main/java/org/keycloak/subsystem/adapter/saml/extension/AllowedClockSkew.java +++ b/adapters/saml/wildfly/wildfly-subsystem/src/main/java/org/keycloak/subsystem/adapter/saml/extension/AllowedClockSkew.java @@ -16,6 +16,7 @@ */ package org.keycloak.subsystem.adapter.saml.extension; +import java.util.EnumSet; import org.jboss.as.controller.SimpleAttributeDefinition; import org.jboss.as.controller.SimpleAttributeDefinitionBuilder; import org.jboss.as.controller.operations.validation.EnumValidator; @@ -32,8 +33,8 @@ abstract public class AllowedClockSkew { static final SimpleAttributeDefinition ALLOWED_CLOCK_SKEW_VALUE = new SimpleAttributeDefinitionBuilder(Constants.Model.ALLOWED_CLOCK_SKEW_VALUE, ModelType.INT, false) .setXmlName(Constants.XML.ALLOWED_CLOCK_SKEW) - .setAllowExpression(false) - .setValidator(new IntRangeValidator(1, Integer.MAX_VALUE, true, false)) + .setAllowExpression(true) + .setValidator(new IntRangeValidator(1, Integer.MAX_VALUE, true, true)) .build(); static private enum AllowedClockSkewUnits {MINUTES, SECONDS, MILLISECONDS, MICROSECONDS, NANOSECONDS}; @@ -41,12 +42,12 @@ abstract public class AllowedClockSkew { static final SimpleAttributeDefinition ALLOWED_CLOCK_SKEW_UNIT = new SimpleAttributeDefinitionBuilder(Constants.Model.ALLOWED_CLOCK_SKEW_UNIT, ModelType.STRING, true) .setXmlName(Constants.XML.ALLOWED_CLOCK_SKEW_UNIT) - .setAllowExpression(false) + .setAllowExpression(true) .setDefaultValue(new ModelNode(AllowedClockSkewUnits.SECONDS.name())) .setAllowedValues(AllowedClockSkewUnits.MINUTES.name(), AllowedClockSkewUnits.SECONDS.name(), AllowedClockSkewUnits.MILLISECONDS.name(), AllowedClockSkewUnits.MICROSECONDS.name(), AllowedClockSkewUnits.NANOSECONDS.name()) - .setValidator(EnumValidator.create(AllowedClockSkewUnits.class, true, false)) + .setValidator(EnumValidator.create(AllowedClockSkewUnits.class, EnumSet.allOf(AllowedClockSkewUnits.class))) .build(); static final SimpleAttributeDefinition[] ATTRIBUTES = {ALLOWED_CLOCK_SKEW_UNIT, ALLOWED_CLOCK_SKEW_VALUE}; diff --git a/adapters/saml/wildfly/wildfly-subsystem/src/test/java/org/keycloak/subsystem/adapter/saml/extension/SubsystemParsingAllowedClockSkewTestCase.java b/adapters/saml/wildfly/wildfly-subsystem/src/test/java/org/keycloak/subsystem/adapter/saml/extension/SubsystemParsingAllowedClockSkewTestCase.java index c4adfd272c..d7e17c2de6 100755 --- a/adapters/saml/wildfly/wildfly-subsystem/src/test/java/org/keycloak/subsystem/adapter/saml/extension/SubsystemParsingAllowedClockSkewTestCase.java +++ b/adapters/saml/wildfly/wildfly-subsystem/src/test/java/org/keycloak/subsystem/adapter/saml/extension/SubsystemParsingAllowedClockSkewTestCase.java @@ -223,16 +223,15 @@ public class SubsystemParsingAllowedClockSkewTestCase extends AbstractSubsystemB testSubsystem("30", "invalid-unit"); } - // For the moment no expressions allowed as the rest of the subsystem doesn't resolve expressions - //@Test - //public void testExpression() throws Exception { - // System.setProperty("test.prop.SKEW_TIME", "30"); - // System.setProperty("test.prop.SKEW_UNIT", "MILLISECONDS"); - // try { - // testSubsystem("${test.prop.SKEW_TIME}", "${test.prop.SKEW_UNIT}", 30, "MILLISECONDS"); - // } finally { - // System.clearProperty("test.prop.SKEW_TIME"); - // System.clearProperty("test.prop.SKEW_UNIT"); - // } - //} + @Test + public void testExpression() throws Exception { + System.setProperty("test.prop.SKEW_TIME", "30"); + System.setProperty("test.prop.SKEW_UNIT", "MILLISECONDS"); + try { + testSubsystem("${test.prop.SKEW_TIME}", "${test.prop.SKEW_UNIT}", 30, "MILLISECONDS"); + } finally { + System.clearProperty("test.prop.SKEW_TIME"); + System.clearProperty("test.prop.SKEW_UNIT"); + } + } } diff --git a/saml-core/src/main/java/org/keycloak/saml/common/util/StaxParserUtil.java b/saml-core/src/main/java/org/keycloak/saml/common/util/StaxParserUtil.java index 857d7ca9d0..e4bd59d632 100755 --- a/saml-core/src/main/java/org/keycloak/saml/common/util/StaxParserUtil.java +++ b/saml-core/src/main/java/org/keycloak/saml/common/util/StaxParserUtil.java @@ -494,6 +494,24 @@ public class StaxParserUtil { return str; } + /** + * Get the element text, replacing every occurrence of ${..} by corresponding system property value + * + * @param xmlEventReader + * + * @return A trimmed string value with all property references replaced if any. + * If there are no valid references the input string will be returned + * + * @throws ParsingException + */ + public static String getElementTextRP(XMLEventReader xmlEventReader) throws ParsingException { + try { + return trim(StringPropertyReplacer.replaceProperties(xmlEventReader.getElementText())); + } catch (XMLStreamException e) { + throw logger.parserException(e); + } + } + /** * Get the XML event reader * diff --git a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java index c2a3bd210c..cc11e4f539 100644 --- a/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java +++ b/testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/adapter/AbstractServletsAdapterTest.java @@ -134,7 +134,7 @@ public abstract class AbstractServletsAdapterTest extends AbstractAdapterTest { if (clockSkewSec != null) { String keycloakSamlXMLContent = IOUtils.toString(keycloakSAMLConfig.openStream(), Charset.forName("UTF-8")) - .replace("%CLOCK_SKEW%", String.valueOf(clockSkewSec)); + .replace("%CLOCK_SKEW%", "${allowed.clock.skew:" + String.valueOf(clockSkewSec) + "}"); deployment.addAsWebInfResource(new StringAsset(keycloakSamlXMLContent), "keycloak-saml.xml"); } else { deployment.addAsWebInfResource(keycloakSAMLConfig, "keycloak-saml.xml");