From ff11b70448bf9640443df2cbd70ce64200095876 Mon Sep 17 00:00:00 2001 From: Jack Conradson Date: Wed, 5 Feb 2025 08:57:12 -0800 Subject: [PATCH] Update policy parser to allow static methods for entitlement creation (#121706) This updates the PolicyParser to allow static methods to have an ExternalEntitlement annotation. This removes a limitation where constructors cannot properly support type-erasure with different types of data structures for internal entitlement generation and external entitlement generation (for example List from the parser and List from an internal builder). We continue to enforce that only one constructor/method may be annotated with ExternalEntitlement per Entitlement class. --- .../runtime/policy/ExternalEntitlement.java | 2 +- .../runtime/policy/PolicyParser.java | 34 ++++++- .../policy/entitlements/FileEntitlement.java | 4 +- .../runtime/policy/FileAccessTreeTests.java | 2 +- .../runtime/policy/PolicyParserTests.java | 89 ++++++++++++++++++- 5 files changed, 122 insertions(+), 9 deletions(-) diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/ExternalEntitlement.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/ExternalEntitlement.java index b58e0d2fb87e7..fef7b5d11aff0 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/ExternalEntitlement.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/ExternalEntitlement.java @@ -22,7 +22,7 @@ * using this annotation is considered parseable as part of a policy file * for entitlements. */ -@Target(ElementType.CONSTRUCTOR) +@Target({ ElementType.CONSTRUCTOR, ElementType.METHOD }) @Retention(RetentionPolicy.RUNTIME) public @interface ExternalEntitlement { diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyParser.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyParser.java index 992728b68186e..2d3468165a590 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyParser.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/PolicyParser.java @@ -27,6 +27,8 @@ import java.io.UncheckedIOException; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -147,6 +149,7 @@ protected Entitlement parseEntitlement(String scopeName, String entitlementType) } Constructor entitlementConstructor = null; + Method entitlementMethod = null; ExternalEntitlement entitlementMetadata = null; for (var ctor : entitlementClass.getConstructors()) { var metadata = ctor.getAnnotation(ExternalEntitlement.class); @@ -161,8 +164,27 @@ protected Entitlement parseEntitlement(String scopeName, String entitlementType) entitlementConstructor = ctor; entitlementMetadata = metadata; } - } + for (var method : entitlementClass.getMethods()) { + var metadata = method.getAnnotation(ExternalEntitlement.class); + if (metadata != null) { + if (Modifier.isStatic(method.getModifiers()) == false) { + throw new IllegalStateException( + "entitlement class [" + entitlementClass.getName() + "] has non-static method annotated with ExternalEntitlement" + ); + } + if (entitlementMetadata != null) { + throw new IllegalStateException( + "entitlement class [" + + entitlementClass.getName() + + "] has more than one constructor and/or method annotated with ExternalEntitlement" + ); + } + entitlementMethod = method; + entitlementMetadata = metadata; + } + } + if (entitlementMetadata == null) { throw newPolicyParserException(scopeName, "unknown entitlement type [" + entitlementType + "]"); } @@ -171,7 +193,9 @@ protected Entitlement parseEntitlement(String scopeName, String entitlementType) throw newPolicyParserException("entitlement type [" + entitlementType + "] is allowed only on modules"); } - Class[] parameterTypes = entitlementConstructor.getParameterTypes(); + Class[] parameterTypes = entitlementConstructor != null + ? entitlementConstructor.getParameterTypes() + : entitlementMethod.getParameterTypes(); String[] parametersNames = entitlementMetadata.parameterNames(); if (parameterTypes.length != 0 || parametersNames.length != 0) { @@ -204,7 +228,11 @@ protected Entitlement parseEntitlement(String scopeName, String entitlementType) } try { - return (Entitlement) entitlementConstructor.newInstance(parameterValues); + if (entitlementConstructor != null) { + return (Entitlement) entitlementConstructor.newInstance(parameterValues); + } else { + return (Entitlement) entitlementMethod.invoke(null, parameterValues); + } } catch (InvocationTargetException | InstantiationException | IllegalAccessException e) { if (e.getCause() instanceof PolicyValidationException piae) { throw newPolicyParserException(startLocation, scopeName, entitlementType, piae); diff --git a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FileEntitlement.java b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FileEntitlement.java index f3a0ee1758a04..5a2492b1b2313 100644 --- a/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FileEntitlement.java +++ b/libs/entitlement/src/main/java/org/elasticsearch/entitlement/runtime/policy/entitlements/FileEntitlement.java @@ -43,7 +43,7 @@ private static Mode parseMode(String mode) { } @ExternalEntitlement(parameterNames = { "path", "mode" }, esModulesOnly = false) - public FileEntitlement(String path, String mode) { - this(path, parseMode(mode)); + public static FileEntitlement create(String path, String mode) { + return new FileEntitlement(path, parseMode(mode)); } } diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java index c133cf0f1242e..28fec6da73897 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/FileAccessTreeTests.java @@ -85,6 +85,6 @@ public void testNormalizePath() { FileEntitlement entitlement(String path, String mode) { Path p = path(path); - return new FileEntitlement(p.toString(), mode); + return FileEntitlement.create(p.toString(), mode); } } diff --git a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java index 53cd5ee8aae08..85bffda369f34 100644 --- a/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java +++ b/libs/entitlement/src/test/java/org/elasticsearch/entitlement/runtime/policy/PolicyParserTests.java @@ -40,6 +40,35 @@ public ManyConstructorsEntitlement(String s) {} public ManyConstructorsEntitlement(int i) {} } + public static class ManyMethodsEntitlement implements Entitlement { + @ExternalEntitlement + public static ManyMethodsEntitlement create(String s) { + return new ManyMethodsEntitlement(); + } + + @ExternalEntitlement + public static ManyMethodsEntitlement create(int i) { + return new ManyMethodsEntitlement(); + } + } + + public static class ConstructorAndMethodEntitlement implements Entitlement { + @ExternalEntitlement + public static ConstructorAndMethodEntitlement create(String s) { + return new ConstructorAndMethodEntitlement(s); + } + + @ExternalEntitlement + public ConstructorAndMethodEntitlement(String s) {} + } + + public static class NonStaticMethodEntitlement implements Entitlement { + @ExternalEntitlement + public NonStaticMethodEntitlement create() { + return new NonStaticMethodEntitlement(); + } + } + public void testGetEntitlementTypeName() { assertEquals("create_class_loader", PolicyParser.getEntitlementTypeName(CreateClassLoaderEntitlement.class)); @@ -55,7 +84,7 @@ public void testPolicyBuilder() throws IOException { .parsePolicy(); Policy expected = new Policy( "test-policy.yaml", - List.of(new Scope("entitlement-module-name", List.of(new FileEntitlement("test/path/to/file", "read_write")))) + List.of(new Scope("entitlement-module-name", List.of(FileEntitlement.create("test/path/to/file", "read_write")))) ); assertEquals(expected, parsedPolicy); } @@ -65,7 +94,7 @@ public void testPolicyBuilderOnExternalPlugin() throws IOException { .parsePolicy(); Policy expected = new Policy( "test-policy.yaml", - List.of(new Scope("entitlement-module-name", List.of(new FileEntitlement("test/path/to/file", "read_write")))) + List.of(new Scope("entitlement-module-name", List.of(FileEntitlement.create("test/path/to/file", "read_write")))) ); assertEquals(expected, parsedPolicy); } @@ -174,4 +203,60 @@ public void testMultipleConstructorsAnnotated() throws IOException { ) ); } + + public void testMultipleMethodsAnnotated() throws IOException { + var parser = new PolicyParser(new ByteArrayInputStream(""" + entitlement-module-name: + - many_methods + """.getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", true, Map.of("many_methods", ManyMethodsEntitlement.class)); + + var e = expectThrows(IllegalStateException.class, parser::parsePolicy); + assertThat( + e.getMessage(), + equalTo( + "entitlement class " + + "[org.elasticsearch.entitlement.runtime.policy.PolicyParserTests$ManyMethodsEntitlement]" + + " has more than one constructor and/or method annotated with ExternalEntitlement" + ) + ); + } + + public void testConstructorAndMethodAnnotated() throws IOException { + var parser = new PolicyParser( + new ByteArrayInputStream(""" + entitlement-module-name: + - constructor_and_method + """.getBytes(StandardCharsets.UTF_8)), + "test-policy.yaml", + true, + Map.of("constructor_and_method", ConstructorAndMethodEntitlement.class) + ); + + var e = expectThrows(IllegalStateException.class, parser::parsePolicy); + assertThat( + e.getMessage(), + equalTo( + "entitlement class " + + "[org.elasticsearch.entitlement.runtime.policy.PolicyParserTests$ConstructorAndMethodEntitlement]" + + " has more than one constructor and/or method annotated with ExternalEntitlement" + ) + ); + } + + public void testNonStaticMethodAnnotated() throws IOException { + var parser = new PolicyParser(new ByteArrayInputStream(""" + entitlement-module-name: + - non_static + """.getBytes(StandardCharsets.UTF_8)), "test-policy.yaml", true, Map.of("non_static", NonStaticMethodEntitlement.class)); + + var e = expectThrows(IllegalStateException.class, parser::parsePolicy); + assertThat( + e.getMessage(), + equalTo( + "entitlement class " + + "[org.elasticsearch.entitlement.runtime.policy.PolicyParserTests$NonStaticMethodEntitlement]" + + " has non-static method annotated with ExternalEntitlement" + ) + ); + } }