You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by jl...@apache.org on 2021/04/22 19:47:28 UTC
[tomee] 03/04: Improve the way we evalate EL
This is an automated email from the ASF dual-hosted git repository.
jlmonteiro pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tomee.git
commit a6ece224c5c779d32a3466600d84b0598938dc96
Author: Jean-Louis Monteiro <jl...@tomitribe.com>
AuthorDate: Thu Apr 22 21:46:27 2021 +0200
Improve the way we evalate EL
---
.../tomee/security/TomEEELInvocationHandler.java | 87 ++++++++++++++++------
.../security/TomEEELInvocationHandlerTest.java | 61 ++++++++++++---
2 files changed, 114 insertions(+), 34 deletions(-)
diff --git a/tomee/tomee-security/src/main/java/org/apache/tomee/security/TomEEELInvocationHandler.java b/tomee/tomee-security/src/main/java/org/apache/tomee/security/TomEEELInvocationHandler.java
index 3393788..c02ecb9 100644
--- a/tomee/tomee-security/src/main/java/org/apache/tomee/security/TomEEELInvocationHandler.java
+++ b/tomee/tomee-security/src/main/java/org/apache/tomee/security/TomEEELInvocationHandler.java
@@ -24,9 +24,13 @@ import java.lang.reflect.InvocationHandler;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
public class TomEEELInvocationHandler implements InvocationHandler {
+ private static final Pattern EL_EXPRESSION_PATTERN = Pattern.compile("^[#$]\\{(.+)}$");
+
private final Annotation annotation;
private final ELProcessor processor;
@@ -41,37 +45,75 @@ public class TomEEELInvocationHandler implements InvocationHandler {
// todo optimize and cache methods
// avoid stack overflow because of infinite loop (See bellow)
- // if method is already the expression one, just invoke it
+ // if method is already the expression one with a String return type, then invoke it and return the result
+ // so we can evaluate the EL and return the evaluated value
if (method.getName().endsWith("Expression") && method.getReturnType().equals(String.class)) {
return method.invoke(annotation, args);
}
- try {
- final Method expressionMethod = annotation.getClass().getDeclaredMethod(method.getName() + "Expression", method.getParameterTypes());
- final String expression = (String) expressionMethod.invoke(proxy, args);
+ // If return value is not a String or an array of string, there is another method with "Expression" at the end and a return type String
+ if (!method.getReturnType().equals(String.class)) {
+ try {
+ // try to find the equivalent expression method
+ final Method expressionMethod =
+ annotation.getClass()
+ .getDeclaredMethod(method.getName() + "Expression", method.getParameterTypes());
+
+ // great, let's get the EL value
+ // we could check the return value to make sure it's a string,
+ // but let's assume people writing annotation apis aren't stupid
+ final String expression = (String) expressionMethod.invoke(proxy, args);
+
+ // if there is an expression, it takes precedence over the static one
+ if (!expression.trim().isEmpty()) {
+ // make sure to pass in the return type of the initial method, otherwith it would be String all the time
+ return eval(processor, sanitizeExpression(expression), method.getReturnType());
+
+ } else {
+ return method.invoke(annotation, args);
+ }
+
+ } catch (final NoSuchMethodException e) {
+ // from spec it is required for a new String to have an equivalent
+ // if not, keep going with the initial method invocation
+ return method.invoke(annotation, args);
- // if there is an expression, it takes precedence over the static one
- if (!expression.isEmpty()) {
- return eval(expression, method.getReturnType()); // use the return type of the static method instead
+ } catch (final InvocationTargetException e) { // unwrap the invocation target exception so we get the actual error
+ throw e.getTargetException();
+ }
+ }
- } else {
- return method.invoke(annotation, args);
+ // if the return type is a String, we may always get an expression to evaluate.
+ // check if it's something we can evaluate
+ final String value = (String) method.invoke(annotation, args);
+ if (value != null && value.length() > 3) {
+ final String sanitizedExpression = sanitizeExpression(value);
+ if (!value.equals(sanitizedExpression)) {
+ return eval(processor, sanitizedExpression, method.getReturnType());
}
+ }
- } catch (final NoSuchMethodException e) { // expression equivalent not found for the given method
- return method.invoke(annotation, args);
+ return value;
+ }
- } catch (final InvocationTargetException e) { // unwrap the invocation target exception so we get the actual error
- throw e.getTargetException();
- }
+ // following should be abstracted into a wrapper of the ELProcessor utility class
+ public static boolean isExpression(final String rawExpression) {
+ final Matcher matcher = EL_EXPRESSION_PATTERN.matcher(rawExpression);
+ return matcher.matches();
}
- private Object eval(final String expression, final Class<?> expectedType) {
- // expression maybe #{expression} instead of ${expression}
- // the ELProcessor anyways wraps it with ${}
- final String sanitizedExpression = expression.replaceAll("^[#$]\\{(.+)}$", "$1");
+ public static String sanitizeExpression(final String rawExpression) {
+ final Matcher matcher = EL_EXPRESSION_PATTERN.matcher(rawExpression);
+
+ if (!matcher.matches()) {
+ return rawExpression;
+ }
+
+ return matcher.replaceAll("$1");
+ }
+ public static Object eval(final ELProcessor processor, final String sanitizedExpression, final Class<?> expectedType) {
// ELProcessor does not do a good job with enums, so try to be a bit better (not sure)
// otherwise, let the EL processor do its best to convert into the expected value
if (!isEnumOrArrayOfEnums(expectedType)) {
@@ -99,13 +141,12 @@ public class TomEEELInvocationHandler implements InvocationHandler {
}
// else not sure what else we can do but let the Object go
-
}
return value;
}
- private boolean isEnumOrArrayOfEnums(final Class type) {
+ private static boolean isEnumOrArrayOfEnums(final Class type) {
if (type.isEnum()) {
return true;
}
@@ -118,7 +159,7 @@ public class TomEEELInvocationHandler implements InvocationHandler {
return false;
}
- private <T /*extends Enum<T>*/> T of(final Class<T> type, final Object name) {
+ private static <T /*extends Enum<T>*/> T of(final Class<T> type, final Object name) {
try {
return (T) type.getDeclaredMethod("valueOf", String.class).invoke(null, name);
@@ -132,9 +173,7 @@ public class TomEEELInvocationHandler implements InvocationHandler {
public static <T extends Annotation> T of(final Class<T> annotationClass, final T annotation, final BeanManager beanManager) {
final ELProcessor elProcessor = new ELProcessor();
elProcessor.getELManager().addELResolver(beanManager.getELResolver());
- return (T) Proxy.newProxyInstance(annotation.getClass().getClassLoader(),
- new Class[]{annotationClass},
- new TomEEELInvocationHandler(annotation, elProcessor));
+ return of(annotationClass, annotation, elProcessor);
}
public static <T extends Annotation> T of(final Class<T> annotationClass, final T annotation, final ELProcessor elProcessor) {
diff --git a/tomee/tomee-security/src/test/java/org/apache/tomee/security/TomEEELInvocationHandlerTest.java b/tomee/tomee-security/src/test/java/org/apache/tomee/security/TomEEELInvocationHandlerTest.java
index 80db047..4e23efe 100644
--- a/tomee/tomee-security/src/test/java/org/apache/tomee/security/TomEEELInvocationHandlerTest.java
+++ b/tomee/tomee-security/src/test/java/org/apache/tomee/security/TomEEELInvocationHandlerTest.java
@@ -16,32 +16,44 @@
*/
package org.apache.tomee.security;
+import org.apache.tomee.security.identitystore.TomEEDatabaseIdentityStore;
import org.junit.Assert;
import org.junit.Test;
import javax.el.ELProcessor;
-import javax.enterprise.inject.Alternative;
+import javax.el.ELResolver;
import javax.enterprise.inject.Vetoed;
import javax.enterprise.inject.spi.BeanManager;
import javax.enterprise.inject.spi.CDI;
+import javax.inject.Named;
import javax.security.enterprise.identitystore.DatabaseIdentityStoreDefinition;
import javax.security.enterprise.identitystore.IdentityStore;
import javax.security.enterprise.identitystore.PasswordHash;
-import java.lang.reflect.InvocationHandler;
import java.util.Map;
+import java.util.Set;
+import java.util.stream.Stream;
+
+import static java.util.Arrays.stream;
+import static java.util.stream.Collectors.toMap;
+import static java.util.stream.Collectors.toSet;
+import static org.apache.tomee.security.identitystore.TomEEDatabaseIdentityStore.eval;
+import static org.apache.tomee.security.identitystore.TomEEDatabaseIdentityStore.toStream;
public class TomEEELInvocationHandlerTest extends AbstractTomEESecurityTest {
@Test
public void canCreateInvocationHandler() {
- final DatabaseIdentityStoreDefinition annotation =
- Color.class.getAnnotation(DatabaseIdentityStoreDefinition.class);
+ final DatabaseIdentityStoreDefinition annotation = Color.class.getAnnotation(DatabaseIdentityStoreDefinition.class);
final ELProcessor elProcessor = new ELProcessor();
- elProcessor.getELManager().addELResolver(bm().getELResolver());
+ final ELResolver elResolver = bm().getELResolver();
+ elProcessor.getELManager().addELResolver(elResolver);
+
+ // small trick because of the @Vetoed bellow - OWB won't pick it up
+ // so we will register one ourselves into the processor so it is resolved
+ elProcessor.defineBean("color", new Color());
- final InvocationHandler handler = new TomEEELInvocationHandler(annotation, elProcessor);
- final DatabaseIdentityStoreDefinition proxiedAnnotation = TomEEELInvocationHandler.of(DatabaseIdentityStoreDefinition.class, annotation, bm());
+ final DatabaseIdentityStoreDefinition proxiedAnnotation = TomEEELInvocationHandler.of(DatabaseIdentityStoreDefinition.class, annotation, elProcessor);
Assert.assertEquals("select password from caller where name = ?", proxiedAnnotation.callerQuery());
Assert.assertEquals(90, proxiedAnnotation.priority());
@@ -49,21 +61,50 @@ public class TomEEELInvocationHandlerTest extends AbstractTomEESecurityTest {
Assert.assertEquals("90", proxiedAnnotation.priorityExpression());
Assert.assertArrayEquals(new IdentityStore.ValidationType[] {IdentityStore.ValidationType.VALIDATE}, proxiedAnnotation.useFor());
+ Assert.assertEquals("select group_name from caller_groups where caller_name = ?", proxiedAnnotation.groupsQuery());
+ final String[] hashAlgorithmParameters = proxiedAnnotation.hashAlgorithmParameters();
+ Assert.assertArrayEquals(new String[]{
+ "Pbkdf2PasswordHash.Iterations=3072",
+ "${color.dyna}"
+ }, hashAlgorithmParameters);
+
+ final Set<String> evaluatedHashParameters = stream(hashAlgorithmParameters)
+ .flatMap(s -> toStream(eval(elProcessor, s, Object.class))).collect(toSet());
+
+ System.out.println(evaluatedHashParameters);
+
+ final Map<String, String> parametersMap = evaluatedHashParameters.stream()
+ .collect(toMap(s -> (String) s.substring(0, s.indexOf('=')),
+ s -> (String) eval(elProcessor, s.substring(s.indexOf('=') + 1), String.class)));
+
+ System.out.println(parametersMap);
}
private BeanManager bm() {
return CDI.current().getBeanManager();
}
-
@Vetoed // so we don't break the other tests with this
+ @Named // see expression language
@DatabaseIdentityStoreDefinition(dataSourceLookup = "jdbc/securityAPIDB",
callerQuery = "select password from caller where name = ?",
- groupsQuery = "select group_name from caller_groups where caller_name = ?",
+ groupsQuery = "${color.groupsQuery}",
hashAlgorithm = CleartextPasswordHash.class,
priority = 30,
priorityExpression = "90",
- useForExpression = "#{'VALIDATE'}")
+ useForExpression = "#{'VALIDATE'}",
+ hashAlgorithmParameters = {
+ "Pbkdf2PasswordHash.Iterations=3072",
+ "${color.dyna}"
+ })
public static class Color {
+
+ public String getGroupsQuery() {
+ return "select group_name from caller_groups where caller_name = ?";
+ }
+
+ public String[] getDyna() {
+ return new String[]{"Pbkdf2PasswordHash.Algorithm=PBKDF2WithHmacSHA512", "Pbkdf2PasswordHash.SaltSizeBytes=64"};
+ }
}
public static class CleartextPasswordHash implements PasswordHash {