Skip to content

Commit

Permalink
[NOTASK]
Browse files Browse the repository at this point in the history 
refactor (code format)
use accessors for setting configuration
check configuration and warn if using the wrong properties and rewrite as needed
rework logging configuration to make it easier to grab in plugin
updated tests
  • Loading branch information
Jj! committed Nov 29, 2023
1 parent 0363afc commit 78f8655
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 27 deletions.
74 changes: 53 additions & 21 deletions src/main/java/edu/internet2/middleware/grouper/authentication/plugin/ConfigUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import edu.internet2.middleware.grouperClient.config.ConfigPropertiesCascadeBase;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.logging.Log;
import org.osgi.framework.BundleContext;
import org.osgi.framework.FrameworkUtil;
import org.osgi.framework.InvalidSyntaxException;
Expand All @@ -20,13 +21,19 @@
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
import java.util.function.Function;
import java.util.stream.Collectors;

public class ConfigUtils {
final static ResourceLoader resourceLoader = new DefaultResourceLoader();
private static final Map<String, String> PROPERTY_RENAMES = new HashMap<>();
static {
PROPERTY_RENAMES.put("external.authentication.saml.keyStoreAlias", "external.authentication.saml.keystoreAlias");
PROPERTY_RENAMES.put("external.authentication.saml.keyStoreType", "external.authentication.saml.keystoreType");
}

private final static ResourceLoader resourceLoader = new DefaultResourceLoader();

final static BundleContext bundleContext = FrameworkUtil.getBundle(GrouperAuthentication.class).getBundleContext();
private final static BundleContext bundleContext = FrameworkUtil.getBundle(GrouperAuthentication.class).getBundleContext();

private final static Log LOG = GrouperAuthentication.getLogFactory().getInstance(ConfigUtils.class);

public static ConfigPropertiesCascadeBase getBestGrouperConfiguration() {
if (isGrouperUi()) {
Expand All @@ -50,29 +57,51 @@ public static ConfigPropertiesCascadeBase getConfigPropertiesCascadeBase(String
}

public static void setProperties(BaseClientConfiguration configuration, String authMechanism) {
checkConfig();
ConfigPropertiesCascadeBase grouperConfig = getBestGrouperConfiguration();

Class<?> clazz = configuration.getClass();
for (String name : grouperConfig.propertyNames()) {
if (name.startsWith("external.authentication." + authMechanism)) {
String fieldName = name.substring(name.lastIndexOf('.') + 1);
try {
Method method = getSetter(clazz, getMethodNameFromFieldName(fieldName));
method.invoke(configuration, getProperty(grouperConfig, method.getParameterTypes()[0], name));
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException |
ClassNotFoundException e) {
try {
Field field = getField(clazz, fieldName);
field.setAccessible(true);
field.set(configuration, getProperty(grouperConfig, field.getType(), name));
} catch (NoSuchFieldException | IllegalAccessException | ClassNotFoundException ex) {
throw new RuntimeException("could not set " + fieldName, ex);
}
}
for (String name : grouperConfig.propertyNames()
.stream()
.filter( p -> p.startsWith("external.authentication." + authMechanism))
.toList()) {
// map name to realname if needed (e.g., changing case)
String realName = ConfigUtils.propertyNameRename(name);
String fieldName = realName.substring(name.lastIndexOf('.') + 1);
try {
Method method = getSetter(clazz, getMethodNameFromFieldName(fieldName));
method.invoke(configuration, getProperty(grouperConfig, method.getParameterTypes()[0], name));
} catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException |
ClassNotFoundException e) {
throw new RuntimeException("could not set " + fieldName, e);
}
}
}

/**
* method to check configuration from various states
*/
private static void checkConfig() {
ConfigPropertiesCascadeBase grouperConfig = getBestGrouperConfiguration();

// check renames
for (Map.Entry<String, String> rename: PROPERTY_RENAMES.entrySet()) {
if (grouperConfig.containsKey(rename.getKey())) {
LOG.warn("you are using the config key `" + rename.getKey() + "`; this should be changed to `" + rename.getValue() + "`");
}
}
}

/**
* method to rewrite property names in the case of deprecation, etc
*
* @param propertyName
* @return
*/
private static String propertyNameRename(String propertyName) {
return PROPERTY_RENAMES.getOrDefault(propertyName, propertyName);
}

private static String getMethodNameFromFieldName(String fieldName) {
return "set" + StringUtils.capitalize(fieldName);
}
Expand All @@ -90,7 +119,10 @@ private static Field getField(Class clazz, String name) throws NoSuchFieldExcept

private static Method getSetter(Class clazz, String name) throws NoSuchMethodException {
//TODO: this is dangerous. currently there are no overloaded methods, but there could be in the future. need to decide best way to handle this (parameter type precedence?)
return Arrays.stream(clazz.getMethods()).filter(m -> m.getName().equals(name)).findFirst().orElseThrow(NoSuchMethodException::new);
return Arrays.stream(clazz.getMethods())
.filter(m -> m.getName().equals(name))
.findFirst()
.orElseThrow(NoSuchMethodException::new);
}

private static Object getProperty(ConfigPropertiesCascadeBase configPropertiesCascadeBase, Type type, String propName) throws ClassNotFoundException {
Expand Down
24 changes: 24 additions & 0 deletions ...in/java/edu/internet2/middleware/grouper/authentication/plugin/GrouperAuthentication.java
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package edu.internet2.middleware.grouper.authentication.plugin;

import org.apache.commons.logging.LogFactory;
import org.osgi.framework.BundleActivator;
import org.osgi.framework.BundleContext;
import org.osgi.framework.FrameworkUtil;
import org.osgi.framework.InvalidSyntaxException;
import org.osgi.framework.ServiceReference;
import org.osgi.framework.ServiceRegistration;

Expand All @@ -14,6 +17,27 @@ public class GrouperAuthentication implements BundleActivator {
private Map<String, ServiceReference> referenceMap = new HashMap<>();
private Map<String, ServiceRegistration> registrationMap = new HashMap<>();

private static final LogFactory LOG_FACTORY;
static {
try {
BundleContext bundleContext = FrameworkUtil.getBundle(GrouperAuthentication.class).getBundleContext();
//TODO: figure out why this is weird
ServiceReference<LogFactory> logfactoryReference = (ServiceReference<LogFactory>) bundleContext.getAllServiceReferences("org.apache.commons.logging.LogFactory", null)[0];
bundleContext.getServiceReference("org.apache.commons.logging.LogFactory");
if (bundleContext.getService(logfactoryReference) != null) {
LOG_FACTORY = bundleContext.getService(logfactoryReference);
} else {
LOG_FACTORY = LogFactory.getFactory();
}
} catch (InvalidSyntaxException e) {
throw new RuntimeException(e);
}
}

public static LogFactory getLogFactory() {
return LOG_FACTORY;
}

@Override
public void start(BundleContext context) throws Exception {
ExternalAuthenticationServletContainerInitializer externalAuthenticationServletContainerInitializer = new ExternalAuthenticationServletContainerInitializer(context);
Expand Down
11 changes: 5 additions & 6 deletions src/test/java/edu/internet2/middleware/grouper/authentication/Pac4JConfigFactoryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,8 @@
import edu.internet2.middleware.grouper.authentication.plugin.GrouperAuthentication;
import edu.internet2.middleware.grouper.authentication.plugin.Pac4jConfigFactory;
import edu.internet2.middleware.grouper.cfg.GrouperHibernateConfig;
import edu.internet2.middleware.grouper.ui.util.GrouperUiConfig;
import edu.internet2.middleware.grouper.ui.util.GrouperUiConfigInApi;
import edu.internet2.middleware.grouperClient.config.ConfigPropertiesCascadeBase;
import junit.framework.TestCase;
import junit.textui.TestRunner;
import org.apache.commons.logging.LogFactory;
import org.junit.After;
import org.junit.Assert;
Expand Down Expand Up @@ -181,7 +178,7 @@ public void testPac4JConfigFactorSAML() {
properties.put("external.authentication.saml.wantsResponsesSigned","false");
properties.put("external.authentication.saml.allSignatureValidationDisabled","true");
properties.put("external.authentication.saml.keystoreAlias","fred");
properties.put("external.authentication.saml.keyStoreType","text");
properties.put("external.authentication.saml.keystoreType","text");
properties.put("external.authentication.saml.assertionConsumerServiceIndex","5");
properties.put("external.authentication.saml.attributeConsumingServiceIndex","2");
properties.put("external.authentication.saml.providerName","paul");
Expand Down Expand Up @@ -227,8 +224,8 @@ public void testPac4JConfigFactorSAML() {
Assert.assertEquals(Boolean.parseBoolean(properties.get("external.authentication.saml.wantsAssertionsSigned")), configuration.isWantsAssertionsSigned());
Assert.assertEquals(Boolean.parseBoolean(properties.get("external.authentication.saml.wantsResponsesSigned")), configuration.isWantsResponsesSigned());
Assert.assertEquals(Boolean.parseBoolean(properties.get("external.authentication.saml.allSignatureValidationDisabled")), configuration.isAllSignatureValidationDisabled());
Assert.assertEquals(properties.get("external.authentication.saml.keyStoreAlias"), configuration.getKeyStoreAlias());
Assert.assertEquals(properties.get("external.authentication.saml.keyStoreType"), configuration.getKeyStoreType());
Assert.assertEquals(properties.get("external.authentication.saml.keystoreAlias"), configuration.getKeyStoreAlias());
Assert.assertEquals(properties.get("external.authentication.saml.keystoreType"), configuration.getKeyStoreType());
Assert.assertEquals(Integer.parseInt(properties.get("external.authentication.saml.assertionConsumerServiceIndex")), configuration.getAssertionConsumerServiceIndex());
Assert.assertEquals(Integer.parseInt(properties.get("external.authentication.saml.attributeConsumingServiceIndex")), configuration.getAttributeConsumingServiceIndex());
Assert.assertEquals(properties.get("external.authentication.saml.providerName"), configuration.getProviderName());
Expand Down Expand Up @@ -359,6 +356,7 @@ public void testConfigTestRename() {
Map<String, String> overrides = grouperConfig.propertiesOverrideMap();
overrides.put("external.authentication.provider","saml");
overrides.put("external.authentication.saml.keyStoreAlias","fred");
overrides.put("external.authentication.saml.keyStoreType","keystoretype");

Pac4jConfigFactory pac4jConfigFactory = new Pac4jConfigFactory();
Config config = pac4jConfigFactory.build();
Expand All @@ -368,5 +366,6 @@ public void testConfigTestRename() {
SAML2Configuration configuration = ((SAML2Client) config.getClients().getClients().get(0)).getConfiguration();

Assert.assertEquals(overrides.get("external.authentication.saml.keyStoreAlias"), configuration.getKeyStoreAlias());
Assert.assertEquals(overrides.get("external.authentication.saml.keyStoreType"), configuration.getKeyStoreType());
}
}

0 comments on commit 78f8655

Please sign in to comment.