From d857dbc9a30bea36edb9a6263c0f798244427f83 Mon Sep 17 00:00:00 2001 From: AFaust Date: Sun, 15 Sep 2019 20:25:25 +0200 Subject: [PATCH] Safe reflection via setter + unit test config elements --- pom.xml | 4 +- repository/pom.xml | 2 +- share/pom.xml | 59 +++++--- .../config/KeycloakAdapterConfigElement.java | 59 +++++--- .../KeycloakAuthenticationConfigElement.java | 2 +- .../share/config/KeycloakConfigConstants.java | 30 ++++ .../web/KeycloakAuthenticationFilter.java | 16 ++- .../config/KeycloakAdapterConfigTest.java | 129 ++++++++++++++++++ share/src/test/resources/addendum-config.xml | 36 +++++ share/src/test/resources/default-config.xml | 60 ++++++++ 10 files changed, 351 insertions(+), 46 deletions(-) create mode 100644 share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakConfigConstants.java create mode 100644 share/src/test/java/de/acosix/alfresco/keycloak/share/config/KeycloakAdapterConfigTest.java create mode 100644 share/src/test/resources/addendum-config.xml create mode 100644 share/src/test/resources/default-config.xml diff --git a/pom.xml b/pom.xml index 58b8c82..92e33ec 100644 --- a/pom.xml +++ b/pom.xml @@ -21,12 +21,12 @@ de.acosix.alfresco.maven de.acosix.alfresco.maven.project.parent-6.1.2 - 1.2.0 + 1.2.1-SNAPSHOT de.acosix.alfresco.keycloak de.acosix.alfresco.keycloak.parent - 1.0.0 + 1.1.0-SNAPSHOT pom Acosix Alfresco Keycloak - Parent diff --git a/repository/pom.xml b/repository/pom.xml index 2037b21..03436e6 100644 --- a/repository/pom.xml +++ b/repository/pom.xml @@ -21,7 +21,7 @@ de.acosix.alfresco.keycloak de.acosix.alfresco.keycloak.parent - 1.0.0 + 1.1.0-SNAPSHOT de.acosix.alfresco.keycloak.repo diff --git a/share/pom.xml b/share/pom.xml index 8e7cb48..07b5248 100644 --- a/share/pom.xml +++ b/share/pom.xml @@ -21,12 +21,26 @@ de.acosix.alfresco.keycloak de.acosix.alfresco.keycloak.parent - 1.0.0 + 1.1.0-SNAPSHOT de.acosix.alfresco.keycloak.share Acosix Alfresco Keycloak - Share Module + + + + + ${project.groupId} + de.acosix.alfresco.keycloak.repo + ${project.version} + installable + test + + + + + @@ -69,39 +83,50 @@ de.acosix.alfresco.utility de.acosix.alfresco.utility.core.share + + + org.slf4j + slf4j-log4j12 + + de.acosix.alfresco.utility de.acosix.alfresco.utility.core.share installable + + + org.slf4j + slf4j-log4j12 + + ${project.groupId} de.acosix.alfresco.keycloak.repo - ${project.version} installable - test + + + + junit + junit + + + + ch.qos.logback + logback-classic + + + + org.slf4j + jcl-over-slf4j - - - - de.thetaphi - forbiddenapis - - - **/KeycloakAdapterConfigElement.class - - - - - - net.alchim31.maven diff --git a/share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakAdapterConfigElement.java b/share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakAdapterConfigElement.java index 4ae883a..8a8d1f5 100644 --- a/share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakAdapterConfigElement.java +++ b/share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakAdapterConfigElement.java @@ -16,12 +16,15 @@ package de.acosix.alfresco.keycloak.share.config; import java.lang.reflect.Field; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Locale; import java.util.Map; import java.util.Set; @@ -29,6 +32,8 @@ import org.alfresco.error.AlfrescoRuntimeException; import org.alfresco.util.EqualsHelper; import org.alfresco.util.ParameterCheck; import org.keycloak.representations.adapters.config.AdapterConfig; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.extensions.config.ConfigElement; import com.fasterxml.jackson.annotation.JsonProperty; @@ -42,11 +47,13 @@ import de.acosix.alfresco.utility.share.config.ConfigValueHolder; public class KeycloakAdapterConfigElement extends BaseCustomConfigElement { - public static final String NAME = "keycloak-adapter-config"; + public static final String NAME = KeycloakConfigConstants.KEYCLOAK_ADAPTER_CONFIG_NAME; + + private static final Logger LOGGER = LoggerFactory.getLogger(KeycloakAdapterConfigElement.class); private static final long serialVersionUID = -7211927327179092723L; - private static final Map FIELD_BY_CONFIG_NAME; + private static final Map SETTER_BY_CONFIG_NAME; private static final Map> VALUE_TYPE_BY_CONFIG_NAME; @@ -54,7 +61,7 @@ public class KeycloakAdapterConfigElement extends BaseCustomConfigElement static { - final Map fieldByConfigName = new HashMap<>(); + final Map setterByConfigName = new HashMap<>(); final Map> valueTypeByConfigName = new HashMap<>(); final List configNames = new ArrayList<>(); @@ -81,17 +88,35 @@ public class KeycloakAdapterConfigElement extends BaseCustomConfigElement if (annotation != null) { final String configName = annotation.value(); - Class valueType = field.getType(); - if (valueType.isPrimitive()) - { - valueType = primitiveWrapperTypeMap.get(valueType); - } - if (supportedValueTypes.contains(valueType)) + final String fieldName = field.getName(); + final StringBuilder setterNameBuilder = new StringBuilder(3 + fieldName.length()); + setterNameBuilder.append("set"); + setterNameBuilder.append(fieldName.substring(0, 1).toUpperCase(Locale.ENGLISH)); + setterNameBuilder.append(fieldName.substring(1)); + final String setterName = setterNameBuilder.toString(); + + Class valueType = field.getType(); + try { - fieldByConfigName.put(configName, field); - valueTypeByConfigName.put(configName, valueType); - configNames.add(configName); + final Method setter = cls.getDeclaredMethod(setterName, valueType); + + if (valueType.isPrimitive()) + { + valueType = primitiveWrapperTypeMap.get(valueType); + } + + if (supportedValueTypes.contains(valueType)) + { + setterByConfigName.put(configName, setter); + valueTypeByConfigName.put(configName, valueType); + configNames.add(configName); + } + } + catch (final NoSuchMethodException nsme) + { + LOGGER.warn("Cannot support Keycloak adapter config field {} as no appropriate setter {} could be found in {}", + fieldName, setterName, cls); } } } @@ -99,7 +124,7 @@ public class KeycloakAdapterConfigElement extends BaseCustomConfigElement cls = cls.getSuperclass(); } - FIELD_BY_CONFIG_NAME = Collections.unmodifiableMap(fieldByConfigName); + SETTER_BY_CONFIG_NAME = Collections.unmodifiableMap(setterByConfigName); VALUE_TYPE_BY_CONFIG_NAME = Collections.unmodifiableMap(valueTypeByConfigName); CONFIG_NAMES = Collections.unmodifiableList(configNames); } @@ -281,18 +306,16 @@ public class KeycloakAdapterConfigElement extends BaseCustomConfigElement { for (final String configName : CONFIG_NAMES) { - final Field field = FIELD_BY_CONFIG_NAME.get(configName); + final Method setter = SETTER_BY_CONFIG_NAME.get(configName); final Object value = this.configValueByField.get(configName); if (value != null) { - // TODO Refactor towards use of setter to avoid setAccessible - field.setAccessible(true); - field.set(config, value); + setter.invoke(config, value); } } } - catch (final IllegalAccessException ex) + catch (final IllegalAccessException | InvocationTargetException ex) { throw new AlfrescoRuntimeException("Error building adapter configuration", ex); } diff --git a/share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakAuthenticationConfigElement.java b/share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakAuthenticationConfigElement.java index 258d36a..b89088c 100644 --- a/share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakAuthenticationConfigElement.java +++ b/share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakAuthenticationConfigElement.java @@ -29,7 +29,7 @@ public class KeycloakAuthenticationConfigElement extends BaseCustomConfigElement private static final long serialVersionUID = 8587583775593697136L; - public static final String NAME = "keycloak-auth-config"; + public static final String NAME = KeycloakConfigConstants.KEYCLOAK_AUTH_CONFIG_NAME; protected final ConfigValueHolder enhanceLoginForm = new ConfigValueHolder<>(); diff --git a/share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakConfigConstants.java b/share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakConfigConstants.java new file mode 100644 index 0000000..c77f53c --- /dev/null +++ b/share/src/main/java/de/acosix/alfresco/keycloak/share/config/KeycloakConfigConstants.java @@ -0,0 +1,30 @@ +/* + * Copyright 2019 Acosix GmbH + * + * 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 de.acosix.alfresco.keycloak.share.config; + +/** + * @author Axel Faust + */ +public interface KeycloakConfigConstants +{ + + String KEYCLOAK_CONFIG_SECTION_NAME = "Keycloak"; + + String KEYCLOAK_ADAPTER_CONFIG_NAME = "keycloak-adapter-config"; + + String KEYCLOAK_AUTH_CONFIG_NAME = "keycloak-auth-config"; + +} diff --git a/share/src/main/java/de/acosix/alfresco/keycloak/share/web/KeycloakAuthenticationFilter.java b/share/src/main/java/de/acosix/alfresco/keycloak/share/web/KeycloakAuthenticationFilter.java index 1b3f582..f16dac6 100644 --- a/share/src/main/java/de/acosix/alfresco/keycloak/share/web/KeycloakAuthenticationFilter.java +++ b/share/src/main/java/de/acosix/alfresco/keycloak/share/web/KeycloakAuthenticationFilter.java @@ -81,6 +81,7 @@ import org.springframework.extensions.webscripts.servlet.DependencyInjectedFilte import de.acosix.alfresco.keycloak.share.config.KeycloakAdapterConfigElement; import de.acosix.alfresco.keycloak.share.config.KeycloakAuthenticationConfigElement; +import de.acosix.alfresco.keycloak.share.config.KeycloakConfigConstants; import de.acosix.alfresco.keycloak.share.remote.BearerTokenAwareSlingshotAlfrescoConnector; /** @@ -229,8 +230,8 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I LOGGER.error("No remote configuration has been defined for the application"); } - final KeycloakAdapterConfigElement keycloakAdapterConfig = (KeycloakAdapterConfigElement) this.configService.getConfig("Keycloak") - .getConfigElement(KeycloakAdapterConfigElement.NAME); + final KeycloakAdapterConfigElement keycloakAdapterConfig = (KeycloakAdapterConfigElement) this.configService + .getConfig(KeycloakConfigConstants.KEYCLOAK_CONFIG_SECTION_NAME).getConfigElement(KeycloakAdapterConfigElement.NAME); if (keycloakAdapterConfig != null) { final AdapterConfig adapterConfiguration = keycloakAdapterConfig.buildAdapterConfiguration(); @@ -268,7 +269,7 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I } final KeycloakAuthenticationConfigElement keycloakAuthConfig = (KeycloakAuthenticationConfigElement) this.configService - .getConfig("Keycloak").getConfigElement(KeycloakAuthenticationConfigElement.NAME); + .getConfig(KeycloakConfigConstants.KEYCLOAK_CONFIG_SECTION_NAME).getConfigElement(KeycloakAuthenticationConfigElement.NAME); if (keycloakAuthConfig != null) { this.filterEnabled = Boolean.TRUE.equals(keycloakAuthConfig.getEnableSsoFilter()); @@ -405,7 +406,8 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I currentSession.getId()); final KeycloakAuthenticationConfigElement keycloakAuthConfig = (KeycloakAuthenticationConfigElement) this.configService - .getConfig("Keycloak").getConfigElement(KeycloakAuthenticationConfigElement.NAME); + .getConfig(KeycloakConfigConstants.KEYCLOAK_CONFIG_SECTION_NAME) + .getConfigElement(KeycloakAuthenticationConfigElement.NAME); final OIDCServletHttpFacade facade = new OIDCServletHttpFacade(req, res); final Integer bodyBufferLimit = keycloakAuthConfig.getBodyBufferLimit(); @@ -445,7 +447,7 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I LOGGER.debug("Processing Keycloak authentication on request to {}", req.getRequestURL()); final KeycloakAuthenticationConfigElement keycloakAuthConfig = (KeycloakAuthenticationConfigElement) this.configService - .getConfig("Keycloak").getConfigElement(KeycloakAuthenticationConfigElement.NAME); + .getConfig(KeycloakConfigConstants.KEYCLOAK_CONFIG_SECTION_NAME).getConfigElement(KeycloakAuthenticationConfigElement.NAME); final Integer bodyBufferLimit = keycloakAuthConfig.getBodyBufferLimit(); final Integer sslRedirectPort = keycloakAuthConfig.getSslRedirectPort(); @@ -579,7 +581,7 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I protected void prepareLoginFormEnhancement(final ServletContext context, final HttpServletRequest req, final HttpServletResponse res) { final KeycloakAuthenticationConfigElement keycloakAuthConfig = (KeycloakAuthenticationConfigElement) this.configService - .getConfig("Keycloak").getConfigElement(KeycloakAuthenticationConfigElement.NAME); + .getConfig(KeycloakConfigConstants.KEYCLOAK_CONFIG_SECTION_NAME).getConfigElement(KeycloakAuthenticationConfigElement.NAME); final Integer bodyBufferLimit = keycloakAuthConfig.getBodyBufferLimit(); final Integer sslRedirectPort = keycloakAuthConfig.getSslRedirectPort(); @@ -901,7 +903,7 @@ public class KeycloakAuthenticationFilter implements DependencyInjectedFilter, I final OIDCServletHttpFacade facade = new OIDCServletHttpFacade(req, res); final KeycloakAuthenticationConfigElement keycloakAuthConfig = (KeycloakAuthenticationConfigElement) this.configService - .getConfig("Keycloak").getConfigElement(KeycloakAuthenticationConfigElement.NAME); + .getConfig(KeycloakConfigConstants.KEYCLOAK_CONFIG_SECTION_NAME).getConfigElement(KeycloakAuthenticationConfigElement.NAME); final Integer bodyBufferLimit = keycloakAuthConfig.getBodyBufferLimit(); final OIDCFilterSessionStore tokenStore = new OIDCFilterSessionStore(req, facade, diff --git a/share/src/test/java/de/acosix/alfresco/keycloak/share/config/KeycloakAdapterConfigTest.java b/share/src/test/java/de/acosix/alfresco/keycloak/share/config/KeycloakAdapterConfigTest.java new file mode 100644 index 0000000..b7f421d --- /dev/null +++ b/share/src/test/java/de/acosix/alfresco/keycloak/share/config/KeycloakAdapterConfigTest.java @@ -0,0 +1,129 @@ +/* + * Copyright 2019 Acosix GmbH + * + * 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 de.acosix.alfresco.keycloak.share.config; + +import java.util.Arrays; +import java.util.Map; + +import org.junit.Assert; +import org.junit.Test; +import org.keycloak.representations.adapters.config.AdapterConfig; +import org.springframework.extensions.config.Config; +import org.springframework.extensions.config.ConfigSource; +import org.springframework.extensions.config.source.UrlConfigSource; +import org.springframework.extensions.config.xml.XMLConfigService; + +/** + * @author Axel Faust + */ +public class KeycloakAdapterConfigTest +{ + + @Test + public void loadDefaultConfig() + { + // default-config.xml copied from src/main/config into src/test/resoruces because default resource filtering will not copy into + // build / class path + final ConfigSource configSource = new UrlConfigSource(Arrays.asList("classpath:default-config.xml"), true); + final XMLConfigService configService = new XMLConfigService(configSource); + configService.initConfig(); + + final Config keycloakConfigSection = configService.getConfig(KeycloakConfigConstants.KEYCLOAK_CONFIG_SECTION_NAME); + + final KeycloakAuthenticationConfigElement keycloakAuthConfig = (KeycloakAuthenticationConfigElement) keycloakConfigSection + .getConfigElement(KeycloakAuthenticationConfigElement.NAME); + + Assert.assertTrue(keycloakAuthConfig.getEnhanceLoginForm()); + Assert.assertTrue(keycloakAuthConfig.getEnableSsoFilter()); + Assert.assertFalse(keycloakAuthConfig.getForceKeycloakSso()); + Assert.assertEquals(Integer.valueOf(8443), keycloakAuthConfig.getSslRedirectPort()); + Assert.assertEquals(Integer.valueOf(10485760), keycloakAuthConfig.getBodyBufferLimit()); + Assert.assertEquals(Integer.valueOf(1000), keycloakAuthConfig.getSessionMapperLimit()); + + final KeycloakAdapterConfigElement keycloakAdapterConfig = (KeycloakAdapterConfigElement) keycloakConfigSection + .getConfigElement(KeycloakAdapterConfigElement.NAME); + + Assert.assertEquals("http://localhost:8180/auth", keycloakAdapterConfig.getFieldValue("auth-server-url")); + Assert.assertEquals("alfresco", keycloakAdapterConfig.getFieldValue("realm")); + Assert.assertEquals("alfresco", keycloakAdapterConfig.getFieldValue("resource")); + Assert.assertEquals("none", keycloakAdapterConfig.getFieldValue("ssl-required")); + Assert.assertEquals(Boolean.FALSE, keycloakAdapterConfig.getFieldValue("public-client")); + + Assert.assertTrue(keycloakAdapterConfig.getFieldValue("credentials") instanceof Map); + final Map credentials = (Map) keycloakAdapterConfig.getFieldValue("credentials"); + Assert.assertEquals("secret", credentials.get("provider")); + + final AdapterConfig adapterConfig = keycloakAdapterConfig.buildAdapterConfiguration(); + Assert.assertEquals("http://localhost:8180/auth", adapterConfig.getAuthServerUrl()); + Assert.assertEquals("alfresco", adapterConfig.getRealm()); + Assert.assertEquals("alfresco", adapterConfig.getResource()); + Assert.assertEquals("none", adapterConfig.getSslRequired()); + Assert.assertFalse(adapterConfig.isPublicClient()); + + Assert.assertNotNull(adapterConfig.getCredentials()); + Assert.assertEquals("secret", adapterConfig.getCredentials().get("provider")); + } + + @Test + public void loadMergedConfig() + { + // default-config.xml copied from src/main/config into src/test/resoruces because default resource filtering will not copy into + // build / class path + final ConfigSource configSource = new UrlConfigSource( + Arrays.asList("classpath:default-config.xml", "classpath:addendum-config.xml"), true); + final XMLConfigService configService = new XMLConfigService(configSource); + configService.initConfig(); + + final Config keycloakConfigSection = configService.getConfig(KeycloakConfigConstants.KEYCLOAK_CONFIG_SECTION_NAME); + + final KeycloakAuthenticationConfigElement keycloakAuthConfig = (KeycloakAuthenticationConfigElement) keycloakConfigSection + .getConfigElement(KeycloakAuthenticationConfigElement.NAME); + + Assert.assertFalse(keycloakAuthConfig.getEnhanceLoginForm()); + Assert.assertFalse(keycloakAuthConfig.getEnableSsoFilter()); + Assert.assertFalse(keycloakAuthConfig.getForceKeycloakSso()); + Assert.assertEquals(Integer.valueOf(8443), keycloakAuthConfig.getSslRedirectPort()); + Assert.assertEquals(Integer.valueOf(10485760), keycloakAuthConfig.getBodyBufferLimit()); + Assert.assertEquals(Integer.valueOf(2000), keycloakAuthConfig.getSessionMapperLimit()); + + final KeycloakAdapterConfigElement keycloakAdapterConfig = (KeycloakAdapterConfigElement) keycloakConfigSection + .getConfigElement(KeycloakAdapterConfigElement.NAME); + + Assert.assertEquals("http://localhost:8080/auth", keycloakAdapterConfig.getFieldValue("auth-server-url")); + Assert.assertEquals("my-realm", keycloakAdapterConfig.getFieldValue("realm")); + Assert.assertEquals("alfresco", keycloakAdapterConfig.getFieldValue("resource")); + Assert.assertEquals("none", keycloakAdapterConfig.getFieldValue("ssl-required")); + Assert.assertEquals(Boolean.FALSE, keycloakAdapterConfig.getFieldValue("public-client")); + Assert.assertEquals(Boolean.TRUE, keycloakAdapterConfig.getFieldValue("always-refresh-token")); + Assert.assertEquals(Integer.valueOf(123), keycloakAdapterConfig.getFieldValue("connection-pool-size")); + + Assert.assertTrue(keycloakAdapterConfig.getFieldValue("credentials") instanceof Map); + final Map credentials = (Map) keycloakAdapterConfig.getFieldValue("credentials"); + Assert.assertEquals("differentSecret", credentials.get("provider")); + + final AdapterConfig adapterConfig = keycloakAdapterConfig.buildAdapterConfiguration(); + Assert.assertEquals("http://localhost:8080/auth", adapterConfig.getAuthServerUrl()); + Assert.assertEquals("my-realm", adapterConfig.getRealm()); + Assert.assertEquals("alfresco", adapterConfig.getResource()); + Assert.assertEquals("none", adapterConfig.getSslRequired()); + Assert.assertFalse(adapterConfig.isPublicClient()); + Assert.assertTrue(adapterConfig.isAlwaysRefreshToken()); + Assert.assertEquals(123, adapterConfig.getConnectionPoolSize()); + + Assert.assertNotNull(adapterConfig.getCredentials()); + Assert.assertEquals("differentSecret", adapterConfig.getCredentials().get("provider")); + } +} diff --git a/share/src/test/resources/addendum-config.xml b/share/src/test/resources/addendum-config.xml new file mode 100644 index 0000000..f523672 --- /dev/null +++ b/share/src/test/resources/addendum-config.xml @@ -0,0 +1,36 @@ + + + + + + + false + false + 2000 + + + http://localhost:8080/auth + true + 123 + my-realm + + differentSecret + + + + + \ No newline at end of file diff --git a/share/src/test/resources/default-config.xml b/share/src/test/resources/default-config.xml new file mode 100644 index 0000000..263d90a --- /dev/null +++ b/share/src/test/resources/default-config.xml @@ -0,0 +1,60 @@ + + + + + + + + + + + + + + + true + true + false + + 8443 + 10485760 + 1000 + + + http://localhost:8180/auth + alfresco + alfresco + none + + false + + secret + + + + + + + + + 60000 + + + + \ No newline at end of file