diff --git a/DEVNOTES.adoc b/DEVNOTES.adoc
new file mode 100644
index 0000000..e972ee9
--- /dev/null
+++ b/DEVNOTES.adoc
@@ -0,0 +1,14 @@
+= Developer Notes
+
+* When building, there will be warnings about `Bundle edu.internet2.middleware.grouper.plugins:grouper-authentication-plugin:bundle:0.0.1-SNAPSHOT : Classes found in the wrong directory`. This can be safely ignored
+
+* During tests, there will be log messages like
++
+----
+2023-12-01T10:30:12,364: [main] WARN ConfigUtils.checkConfig(93) - [] - you are using the config key `external.authentication.saml.keyStoreAlias`; this should be changed to `external.authentication.saml.keystoreAlias`
+2023-12-01T10:30:12,368: [main] WARN ConfigUtils.checkConfig(93) - [] - you are using the config key `external.authentication.saml.keyStoreType`; this should be changed to `external.authentication.saml.keystoreType`
+----
++
+These can usually be safely ignored.
++
+* SPIs will not work properly if the jar is built with inline; this should be considered if this configuration is changed. Some of the included libraries/frameworks depend on this, notably opensaml.
\ No newline at end of file
diff --git a/pom.xml b/pom.xml
index ff9a9e8..7222f7e 100644
--- a/pom.xml
+++ b/pom.xml
@@ -34,7 +34,8 @@
<properties>
<maven.compiler.source>17</maven.compiler.source>
<maven.compiler.target>17</maven.compiler.target>
- <pac4j.version>5.7.1</pac4j.version>
+ <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
+ <pac4j.version>5.7.2</pac4j.version>
<javaee-pac4j.version>7.1.0</javaee-pac4j.version>
<grouper.version>4.7.0</grouper.version>
</properties>
@@ -69,7 +70,7 @@
</dependency>
<dependency>
<groupId>org.pac4j</groupId>
- <artifactId>pac4j-cas</artifactId>
+ <artifactId>pac4j-cas-clientv4</artifactId>
<version>${pac4j.version}</version>
</dependency>
<dependency>
@@ -79,7 +80,7 @@
</dependency>
<dependency>
<groupId>org.pac4j</groupId>
- <artifactId>pac4j-saml</artifactId>
+ <artifactId>pac4j-saml-opensamlv5</artifactId>
<version>${pac4j.version}</version>
<exclusions>
<exclusion>
@@ -165,9 +166,20 @@
<build>
<plugins>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-compiler-plugin</artifactId>
+ <version>3.11.0</version>
+ <configuration>
+ <compilerArgs>
+ <arg>-Xlint:unchecked</arg>
+ </compilerArgs>
+ </configuration>
+ </plugin>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-jar-plugin</artifactId>
+ <version>3.3.0</version>
<configuration>
<archive>
<index>true</index>
@@ -185,6 +197,7 @@
<plugin>
<groupId>org.apache.felix</groupId>
<artifactId>maven-bundle-plugin</artifactId>
+ <version>5.1.9</version>
<extensions>true</extensions>
<configuration>
<instructions>
@@ -192,7 +205,8 @@
<Bundle-Name>${project.artifactId}-bundle</Bundle-Name>
<Bundle-Version>${project.version}</Bundle-Version>
<Bundle-Activator>edu.internet2.middleware.grouper.authentication.plugin.GrouperAuthentication</Bundle-Activator>
- <Export-Package>edu.internet2.middleware.grouper.authentication.plugin.filter</Export-Package>
+ <!-- <Export-Package>edu.internet2.middleware.grouper.authentication.plugin.filter</Export-Package> -->
+ <Export-Package>!*</Export-Package>
<Private-Package>edu.internet2.middleware.grouper.authentication.plugin.*</Private-Package>
<Embed-Dependency>*;scope=compile|runtime;inline=false</Embed-Dependency>
<Embed-Transitive>true</Embed-Transitive>
diff --git a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/ConfigUtils.java b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/ConfigUtils.java
index 5b1361c..fb8c310 100644
--- a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/ConfigUtils.java
+++ b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/ConfigUtils.java
@@ -17,10 +17,12 @@
import java.lang.reflect.Type;
import java.time.Period;
import java.util.Arrays;
+import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
+import java.util.List;
public class ConfigUtils {
private static final Map<String, String> PROPERTY_RENAMES = new HashMap<>();
@@ -49,8 +51,9 @@ public static ConfigPropertiesCascadeBase getBestGrouperConfiguration() {
public static ConfigPropertiesCascadeBase getConfigPropertiesCascadeBase(String type) {
try {
- ServiceReference<ConfigPropertiesCascadeBase> serviceReference = (ServiceReference<ConfigPropertiesCascadeBase>) bundleContext.getServiceReferences(ConfigPropertiesCascadeBase.class, "(type=" + type + ")").toArray()[0];
- return bundleContext.getService(serviceReference);
+ List<ServiceReference<ConfigPropertiesCascadeBase>> serviceReferenceList = (List<ServiceReference<ConfigPropertiesCascadeBase>>) bundleContext.getServiceReferences(ConfigPropertiesCascadeBase.class, "(type=" + type + ")");
+ ServiceReference<?> serviceReference = serviceReferenceList.get(0);
+ return (ConfigPropertiesCascadeBase) bundleContext.getService(serviceReference);
} catch (InvalidSyntaxException e) {
throw new RuntimeException(e);
}
@@ -127,7 +130,11 @@ private static Method getSetter(Class clazz, String name) throws NoSuchMethodExc
private static Object getProperty(ConfigPropertiesCascadeBase configPropertiesCascadeBase, Type type, String propName) throws ClassNotFoundException {
if (Enum.class.isAssignableFrom((Class<?>) type)) {
- return Enum.valueOf((Class<Enum>)type, configPropertiesCascadeBase.propertyValueString(propName));
+ // there are a few properties that are enums (e.g., CAS protocol)
+ // TODO: can this be checked?
+ @SuppressWarnings("unchecked")
+ Enum<?> e = Enum.valueOf((Class) type, configPropertiesCascadeBase.propertyValueString(propName));
+ return e;
} else {
switch (type.getTypeName()) {
case "java.lang.String": {
@@ -154,14 +161,13 @@ private static Object getProperty(ConfigPropertiesCascadeBase configPropertiesCa
return Arrays.asList(configPropertiesCascadeBase.propertyValueString(propName).split(","));
}
case "java.util.Set": {
- Set set = new HashSet();
- for (String prop : configPropertiesCascadeBase.propertyValueString(propName).split(",")) {
- set.add(prop);
- }
+ //TODO: hopefully string is all that is needed, but might need to revisit
+ Set<String> set = new HashSet<>();
+ Collections.addAll(set, configPropertiesCascadeBase.propertyValueString(propName).split(","));
return set;
}
case "java.util.Map": {
- Map<String, String> map = new HashMap();
+ Map<String, String> map = new HashMap<>();
for (String pairs : configPropertiesCascadeBase.propertyValueString(propName).split(",")) {
String[] keyValue = pairs.split("=");
map.put(keyValue[0].trim(), keyValue[1].trim());
diff --git a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/ExternalAuthenticationServletContainerInitializer.java b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/ExternalAuthenticationServletContainerInitializer.java
index c3f081c..0cc0538 100644
--- a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/ExternalAuthenticationServletContainerInitializer.java
+++ b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/ExternalAuthenticationServletContainerInitializer.java
@@ -3,10 +3,6 @@
import edu.internet2.middleware.grouper.authentication.plugin.filter.CallbackFilterDecorator;
import edu.internet2.middleware.grouper.authentication.plugin.filter.SecurityFilterDecorator;
import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.framework.ServiceReference;
import javax.servlet.FilterRegistration;
import javax.servlet.ServletContainerInitializer;
@@ -15,17 +11,7 @@
import java.util.Set;
public class ExternalAuthenticationServletContainerInitializer implements ServletContainerInitializer {
- private final Log log;
-
- public ExternalAuthenticationServletContainerInitializer(BundleContext bundleContext) {
- try {
- //TODO: figure out why this is weird
- ServiceReference<LogFactory> logfactoryReference = (ServiceReference<LogFactory>) bundleContext.getAllServiceReferences("org.apache.commons.logging.LogFactory", null)[0];
- log = bundleContext.getService(logfactoryReference).getInstance(ExternalAuthenticationServletContainerInitializer.class);
- } catch (InvalidSyntaxException e) {
- throw new RuntimeException(e);
- }
- }
+ private static final Log log = GrouperAuthentication.getLogFactory().getInstance(ExternalAuthenticationServletContainerInitializer.class);
@Override
public void onStartup(Set<Class<?>> c, ServletContext ctx) throws ServletException {
diff --git a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/GrouperAuthentication.java b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/GrouperAuthentication.java
index 4033c99..df1f58e 100644
--- a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/GrouperAuthentication.java
+++ b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/GrouperAuthentication.java
@@ -15,13 +15,15 @@
public class GrouperAuthentication implements BundleActivator {
private Map<String, ServiceReference> referenceMap = new HashMap<>();
- private Map<String, ServiceRegistration> registrationMap = new HashMap<>();
+ private final 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
+ // TODO: check if this can be checked
+ @SuppressWarnings("unchecked")
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) {
@@ -40,7 +42,7 @@ public static LogFactory getLogFactory() {
@Override
public void start(BundleContext context) throws Exception {
- ExternalAuthenticationServletContainerInitializer externalAuthenticationServletContainerInitializer = new ExternalAuthenticationServletContainerInitializer(context);
+ ExternalAuthenticationServletContainerInitializer externalAuthenticationServletContainerInitializer = new ExternalAuthenticationServletContainerInitializer();
ServiceRegistration easciRegistration = context.registerService(ServletContainerInitializer.class, externalAuthenticationServletContainerInitializer, new Hashtable<>());
registrationMap.put(ExternalAuthenticationServletContainerInitializer.class.getCanonicalName(), easciRegistration);
}
diff --git a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/Pac4jConfigFactory.java b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/Pac4jConfigFactory.java
index 46fd5d4..1d119a8 100644
--- a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/Pac4jConfigFactory.java
+++ b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/Pac4jConfigFactory.java
@@ -5,30 +5,16 @@
import edu.internet2.middleware.grouperClient.config.ConfigPropertiesCascadeBase;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.FrameworkUtil;
-import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.framework.ServiceReference;
import org.pac4j.core.client.Client;
import org.pac4j.core.client.Clients;
import org.pac4j.core.config.Config;
import org.pac4j.core.config.ConfigFactory;
import org.pac4j.core.matching.matcher.PathMatcher;
+import java.lang.reflect.InvocationTargetException;
+
public class Pac4jConfigFactory implements ConfigFactory {
- // private static final Logger LOGGER = Logger.getLogger(Pac4jConfigFactory.class);
- private static final Log LOGGER;
- 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];
- LOGGER = bundleContext.getService(logfactoryReference).getInstance(ExternalAuthenticationServletContainerInitializer.class);
- } catch (InvalidSyntaxException e) {
- throw new RuntimeException(e);
- }
- }
+ private static final Log LOGGER = GrouperAuthentication.getLogFactory().getInstance(Pac4jConfigFactory.class);
@Override
public Config build(Object... parameters) {
@@ -61,14 +47,16 @@ public Config build(Object... parameters) {
config.addMatcher("securityExclusions", pathMatcher);
return config;
- } catch (IllegalAccessException|InstantiationException e) {
+ } catch (IllegalAccessException | InstantiationException | NoSuchMethodException | InvocationTargetException e) {
throw new RuntimeException("problem configuring pac4j", e);
} finally {
Thread.currentThread().setContextClassLoader(classLoader);
}
}
- private static Client getClient(String provider) throws IllegalAccessException, InstantiationException {
+ //TODO: can this be checked
+ @SuppressWarnings("unchecked")
+ private static Client getClient(String provider) throws IllegalAccessException, InstantiationException, NoSuchMethodException, InvocationTargetException {
Class<? extends ClientProvider> providerClass;
//TODO: might be a better way of doing this
try {
@@ -80,6 +68,6 @@ private static Client getClient(String provider) throws IllegalAccessException,
throw new RuntimeException(classNotFoundException);
}
}
- return providerClass.newInstance().getClient();
+ return providerClass.getDeclaredConstructor().newInstance().getClient();
}
}
diff --git a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/config/OidcClientProvider.java b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/config/OidcClientProvider.java
index 613f6dc..fea1eb8 100644
--- a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/config/OidcClientProvider.java
+++ b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/config/OidcClientProvider.java
@@ -1,32 +1,18 @@
package edu.internet2.middleware.grouper.authentication.plugin.config;
import edu.internet2.middleware.grouper.authentication.plugin.ConfigUtils;
-import edu.internet2.middleware.grouper.authentication.plugin.ExternalAuthenticationServletContainerInitializer;
import edu.internet2.middleware.grouper.authentication.plugin.GrouperAuthentication;
import edu.internet2.middleware.grouper.authentication.plugin.oidc.client.ClaimAsUsernameOidcClient;
import edu.internet2.middleware.grouper.authentication.plugin.oidc.config.ClaimAsUsernameOidcConfiguration;
import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.FrameworkUtil;
-import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.framework.ServiceReference;
import org.pac4j.core.client.Client;
import org.pac4j.oidc.client.OidcClient;
import org.pac4j.oidc.config.OidcConfiguration;
+import java.lang.reflect.InvocationTargetException;
+
public class OidcClientProvider implements ClientProvider {
- private static final Log LOGGER;
- 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];
- LOGGER = bundleContext.getService(logfactoryReference).getInstance(ExternalAuthenticationServletContainerInitializer.class);
- } catch (InvalidSyntaxException e) {
- throw new RuntimeException(e);
- }
- }
+ private static final Log LOGGER = GrouperAuthentication.getLogFactory().getInstance(OidcClientProvider.class);
@Override
public boolean supports(String type) {
@@ -39,9 +25,11 @@ public Client getClient() {
String implementation = ConfigUtils.getBestGrouperConfiguration().propertyValueString("external.authentication.mechanism.oidc.clientImplementation");
if (implementation != null && !implementation.isEmpty()) {
try {
- OidcConfiguration configuration = (OidcConfiguration) Class.forName(implementation).newInstance();
+ OidcConfiguration configuration = (OidcConfiguration) Class.forName(implementation).getDeclaredConstructor().newInstance();
client = new OidcClient(configuration);
- } catch (InstantiationException | IllegalAccessException | ClassNotFoundException e) {
+ } catch (InstantiationException | IllegalAccessException | ClassNotFoundException | NoSuchMethodException |
+ InvocationTargetException e) {
+ //TODO: check whether this is actually a good idea and/or approapriate. there are a few cases that might need to be handled differently (e.g., wrong classname)
LOGGER.warn("problem loading pac4j client implementation; using a default", e);
client = getClaimAsUsernameOidcClient();
}
diff --git a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/filter/ReinitializingTimer.java b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/filter/ReinitializingTimer.java
index 796686b..a7d0cc6 100644
--- a/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/filter/ReinitializingTimer.java
+++ b/src/main/java/edu/internet2/middleware/grouper/authentication/plugin/filter/ReinitializingTimer.java
@@ -3,32 +3,16 @@
import edu.internet2.middleware.grouper.authentication.plugin.ConfigUtils;
import edu.internet2.middleware.grouper.authentication.plugin.ExternalAuthenticationServletContainerInitializer;
import edu.internet2.middleware.grouper.authentication.plugin.GrouperAuthentication;
-import edu.internet2.middleware.grouper.authentication.plugin.Pac4jConfigFactory;
import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.FrameworkUtil;
-import org.osgi.framework.InvalidSyntaxException;
-import org.osgi.framework.ServiceReference;
import java.util.Map;
import java.util.TimerTask;
import java.util.regex.Pattern;
public class ReinitializingTimer extends TimerTask {
- private static final Log LOGGER;
- 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];
- LOGGER = bundleContext.getService(logfactoryReference).getInstance(ExternalAuthenticationServletContainerInitializer.class);
- } catch (InvalidSyntaxException e) {
- throw new RuntimeException(e);
- }
- }
+ private static final Log LOGGER = GrouperAuthentication.getLogFactory().getInstance(ExternalAuthenticationServletContainerInitializer.class);
- private Map config;
+ private Map<String, String> config;
private final Reinitializable initTarget;
public ReinitializingTimer(Reinitializable initTarget) {
@@ -45,7 +29,7 @@ protected static boolean areEqual(Map<String, String> first, Map<String, String>
@Override
public void run() {
- Map curConfig = ConfigUtils.getBestGrouperConfiguration().propertiesMap(Pattern.compile("^external\\.authentication\\.([^.]+)$"));
+ Map<String, String> curConfig = ConfigUtils.getBestGrouperConfiguration().propertiesMap(Pattern.compile("^external\\.authentication\\.([^.]+)$"));
if (!areEqual(config, curConfig)) {
config = curConfig;
initTarget.initDecorator();
diff --git a/src/test/java/edu/internet2/middleware/grouper/authentication/Pac4JConfigFactoryTest.java b/src/test/java/edu/internet2/middleware/grouper/authentication/Pac4JConfigFactoryTest.java
index 931cae1..94d0219 100644
--- a/src/test/java/edu/internet2/middleware/grouper/authentication/Pac4JConfigFactoryTest.java
+++ b/src/test/java/edu/internet2/middleware/grouper/authentication/Pac4JConfigFactoryTest.java
@@ -39,6 +39,7 @@
public class Pac4JConfigFactoryTest {
MockedStatic<FrameworkUtil> frameworkUtilMockedStatic;
+ @SuppressWarnings("unchecked")
@Before
public void setup() throws Exception {
this.frameworkUtilMockedStatic = Mockito.mockStatic(FrameworkUtil.class);