This is an automated email from the ASF dual-hosted git repository.
sergehuber pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/master by this push:
new 668a15fbf UNOMI-976: Restrict Groovy action upload to system
administrators (#852)
668a15fbf is described below
commit 668a15fbfca583f47809519c9dd52a358b6294ef
Author: Serge Huber <[email protected]>
AuthorDate: Tue Sep 22 16:48:51 2026 +0200
UNOMI-976: Restrict Groovy action upload to system administrators (#852)
Co-authored-by: Claude Opus 5 (1M context) <[email protected]>
---
.../karaf-kar/src/main/feature/feature.xml | 2 +
extensions/groovy-actions/rest/pom.xml | 10 ++
.../groovy/actions/rest/GroovyActionsEndPoint.java | 5 +
.../rest/GroovyActionsEndPointRoleTest.java | 37 +++++++
.../services/impl/GroovyActionsServiceImpl.java | 23 ++++-
.../impl/GroovyActionsServiceImplTest.java | 49 +++++++++
.../cxs/actions/fieldInitializerAction.groovy | 27 +++++
.../test/java/org/apache/unomi/itests/AllITs.java | 1 +
.../apache/unomi/itests/CorePersistenceITs.java | 1 +
.../GroovyActionsEndpointRoleSecurityIT.java | 113 +++++++++++++++++++++
10 files changed, 265 insertions(+), 3 deletions(-)
diff --git a/extensions/groovy-actions/karaf-kar/src/main/feature/feature.xml
b/extensions/groovy-actions/karaf-kar/src/main/feature/feature.xml
index 5d527b1e8..c16201eda 100644
--- a/extensions/groovy-actions/karaf-kar/src/main/feature/feature.xml
+++ b/extensions/groovy-actions/karaf-kar/src/main/feature/feature.xml
@@ -20,6 +20,8 @@
<details>${project.description}</details>
<feature>wrap</feature>
<feature>unomi-services</feature>
+ <!-- REST endpoints use @RequiresRole from
org.apache.unomi.rest.security -->
+ <feature>unomi-rest-api</feature>
<configfile
finalname="/etc/org.apache.unomi.groovy.actions.cfg">mvn:org.apache.unomi/unomi-groovy-actions-services/${project.version}/cfg/groovyactionscfg</configfile>
<bundle
start="false">mvn:org.apache.unomi/unomi-groovy-actions-services/${project.version}</bundle>
<bundle
start="false">mvn:org.apache.unomi/unomi-groovy-actions-rest/${project.version}</bundle>
diff --git a/extensions/groovy-actions/rest/pom.xml
b/extensions/groovy-actions/rest/pom.xml
index 3bedcc985..b6aef5cd3 100644
--- a/extensions/groovy-actions/rest/pom.xml
+++ b/extensions/groovy-actions/rest/pom.xml
@@ -57,6 +57,11 @@
<artifactId>unomi-groovy-actions-services</artifactId>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-rest</artifactId>
+ <scope>provided</scope>
+ </dependency>
<dependency>
<groupId>org.osgi</groupId>
@@ -106,6 +111,11 @@
<artifactId>slf4j-api</artifactId>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter</artifactId>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
<plugins>
diff --git
a/extensions/groovy-actions/rest/src/main/java/org/apache/unomi/groovy/actions/rest/GroovyActionsEndPoint.java
b/extensions/groovy-actions/rest/src/main/java/org/apache/unomi/groovy/actions/rest/GroovyActionsEndPoint.java
index 1f41cf6f3..635dfe3d4 100644
---
a/extensions/groovy-actions/rest/src/main/java/org/apache/unomi/groovy/actions/rest/GroovyActionsEndPoint.java
+++
b/extensions/groovy-actions/rest/src/main/java/org/apache/unomi/groovy/actions/rest/GroovyActionsEndPoint.java
@@ -21,7 +21,9 @@ import org.apache.commons.io.IOUtils;
import org.apache.cxf.jaxrs.ext.multipart.Attachment;
import org.apache.cxf.jaxrs.ext.multipart.Multipart;
import org.apache.cxf.rs.security.cors.CrossOriginResourceSharing;
+import org.apache.unomi.api.security.UnomiRoles;
import org.apache.unomi.groovy.actions.services.GroovyActionsService;
+import org.apache.unomi.rest.security.RequiresRole;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;
import org.slf4j.Logger;
@@ -35,6 +37,7 @@ import java.io.IOException;
@Produces(MediaType.APPLICATION_JSON + ";charset=UTF-8")
@CrossOriginResourceSharing(allowAllOrigins = true, allowCredentials = true)
@Path("/groovyActions")
+@RequiresRole(UnomiRoles.ADMINISTRATOR)
@Component(service = GroovyActionsEndPoint.class, property =
"osgi.jaxrs.resource=true")
public class GroovyActionsEndPoint {
@@ -55,6 +58,7 @@ public class GroovyActionsEndPoint {
* Uploads a Groovy action script and registers a matching action type.
* <p>
* The multipart field {@code file} must be a {@code .groovy} file; the
action id is derived from the filename.
+ * Restricted to system administrators (JAAS); tenant administrators
cannot upload scripts.
*
* @param file the Groovy script upload
* @return an empty success response
@@ -79,6 +83,7 @@ public class GroovyActionsEndPoint {
/**
* Deletes the Groovy action and its action type entry.
+ * Restricted to system administrators (JAAS).
*
* @param actionId the action identifier
* @api.status 204 empty Action deleted.
diff --git
a/extensions/groovy-actions/rest/src/test/java/org/apache/unomi/groovy/actions/rest/GroovyActionsEndPointRoleTest.java
b/extensions/groovy-actions/rest/src/test/java/org/apache/unomi/groovy/actions/rest/GroovyActionsEndPointRoleTest.java
new file mode 100644
index 000000000..92ffea28d
--- /dev/null
+++
b/extensions/groovy-actions/rest/src/test/java/org/apache/unomi/groovy/actions/rest/GroovyActionsEndPointRoleTest.java
@@ -0,0 +1,37 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.unomi.groovy.actions.rest;
+
+import org.apache.unomi.api.security.UnomiRoles;
+import org.apache.unomi.rest.security.RequiresRole;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+/**
+ * Regression: Groovy upload/delete must stay restricted to system
administrators.
+ */
+class GroovyActionsEndPointRoleTest {
+
+ @Test
+ void endpointRequiresSystemAdministratorRole() {
+ RequiresRole requiresRole =
GroovyActionsEndPoint.class.getAnnotation(RequiresRole.class);
+ assertNotNull(requiresRole, "GroovyActionsEndPoint must declare
@RequiresRole");
+ assertArrayEquals(new String[]{UnomiRoles.ADMINISTRATOR},
requiresRole.value());
+ }
+}
diff --git
a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java
b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java
index 40c02a7ae..09307e038 100644
---
a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java
+++
b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java
@@ -301,7 +301,7 @@ public class GroovyActionsServiceImpl extends
AbstractMultiTypeCachingService im
// Extract Action annotation and register the ActionType
try {
synchronized(compilationLock) {
- Action actionAnnotation =
compilationShell.parse(groovyCodeSource).getClass().getMethod("execute").getAnnotation(Action.class);
+ Action actionAnnotation =
compileToClass(groovyCodeSource).getMethod("execute").getAnnotation(Action.class);
if (actionAnnotation != null) {
contextManager.executeAsSystem(() -> {
saveActionType(actionAnnotation);
@@ -407,7 +407,7 @@ public class GroovyActionsServiceImpl extends
AbstractMultiTypeCachingService im
try {
GroovyCodeSource groovyCodeSource = new
GroovyCodeSource(script, actionName, "/groovy/script");
synchronized(compilationLock) {
-
compilationShell.parse(groovyCodeSource).getClass().getMethod("execute");
+ compileToClass(groovyCodeSource).getMethod("execute");
}
// Note: We don't extract or save the ActionType here
} catch (NoSuchMethodException e) {
@@ -471,16 +471,33 @@ public class GroovyActionsServiceImpl extends
AbstractMultiTypeCachingService im
}
}
+
/**
* Thread-safe script compilation using synchronized shared shell.
*/
private Class<? extends Script> compileScript(String actionName, String
scriptContent) {
GroovyCodeSource codeSource = new GroovyCodeSource(scriptContent,
actionName, "/groovy/script");
synchronized(compilationLock) {
- return compilationShell.parse(codeSource).getClass();
+ return compileToClass(codeSource);
}
}
+
+ /**
+ * Compiles a script to its Class without instantiating it.
+ * <p>
+ * Deliberately {@code parseClass} and not {@code GroovyShell#parse}:
{@code parse} returns a
+ * {@code Script} <em>instance</em>, and constructing that instance runs
the script's field
+ * initializers. An uploaded script carrying a Groovy {@code @Field}
initializer would therefore
+ * execute at upload/compile time, before any rule ever dispatches it.
Every caller here only
+ * needs the compiled Class (to read the {@code @Action} annotation or
check for {@code execute}),
+ * so nothing needs to be instantiated until the action is actually run.
+ */
+ @SuppressWarnings("unchecked")
+ private Class<? extends Script> compileToClass(GroovyCodeSource
codeSource) {
+ return (Class<? extends Script>)
compilationShell.getClassLoader().parseClass(codeSource, false);
+ }
+
/**
* Compiles a script and creates metadata with timing information.
*/
diff --git
a/extensions/groovy-actions/services/src/test/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImplTest.java
b/extensions/groovy-actions/services/src/test/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImplTest.java
index 94536a296..557ff0649 100644
---
a/extensions/groovy-actions/services/src/test/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImplTest.java
+++
b/extensions/groovy-actions/services/src/test/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImplTest.java
@@ -16,6 +16,7 @@
*/
package org.apache.unomi.groovy.actions.services.impl;
+import groovy.lang.GroovyShell;
import groovy.lang.Script;
import org.apache.unomi.api.Event;
import org.apache.unomi.api.ExecutionContext;
@@ -47,6 +48,7 @@ import org.osgi.framework.wiring.BundleWiring;
import java.net.URL;
import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.*;
@@ -59,6 +61,8 @@ import static org.mockito.Mockito.when;
*/
public class GroovyActionsServiceImplTest {
+ private static final String FIELD_INITIALIZER_MARKER =
"unomi.test.groovyFieldInitializerRan";
+
private GroovyActionsServiceImpl groovyActionsService;
private TenantService tenantService;
private PersistenceService persistenceService;
@@ -172,6 +176,51 @@ public class GroovyActionsServiceImplTest {
});
}
+ /**
+ * Saving an action must compile it without instantiating it. A Groovy
{@code @Field} initializer
+ * runs at instantiation, so an implementation that instantiates while
saving would run whatever
+ * the script's author put in that initializer at save time, before any
rule dispatches the action.
+ * <p>
+ * The positive control matters as much as the assertion. The same script
is first run through a
+ * plain {@link GroovyShell}, which must set the marker; without that step
a script that silently
+ * failed to set it would make the real assertion pass while proving
nothing.
+ */
+ @Test
+ public void testSaveCompilesWithoutInstantiating() throws Exception {
+ String groovyScript = loadGroovyScript(
+ "/META-INF/cxs/actions/fieldInitializerAction.groovy",
+ "Could not find the field-initializer test Groovy action file");
+ System.clearProperty(FIELD_INITIALIZER_MARKER);
+ try {
+ // Positive control: instantiating the script does run the @Field
initializer.
+ new GroovyShell().parse(stripActionAnnotation(groovyScript));
+ assertEquals("positive control failed: the @Field initializer did
not run even via "
+ + "GroovyShell#parse, so the assertion below would
prove nothing",
+ "true", System.getProperty(FIELD_INITIALIZER_MARKER));
+ System.clearProperty(FIELD_INITIALIZER_MARKER);
+
+ // The assertion: saving the very same script must not instantiate
it.
+ contextManager.executeAsTenant(TENANT_1, () -> {
+ groovyActionsService.save("fieldInitializerAction",
groovyScript);
+ });
+
+ assertNull("Saving a Groovy action must compile it without
instantiating it, so a @Field "
+ + "initializer must not run at save time",
+ System.getProperty(FIELD_INITIALIZER_MARKER));
+ } finally {
+ System.clearProperty(FIELD_INITIALIZER_MARKER);
+ }
+ }
+
+ /**
+ * Drops the {@code @Action} line so the positive control compiles under a
bare {@link GroovyShell},
+ * which has neither the service's ImportCustomizer nor its script base
class. The {@code @Field}
+ * initializer - the only part under test - is untouched.
+ */
+ private static String stripActionAnnotation(String script) {
+ return script.replaceAll("(?m)^@Action\\(.*\\)$", "");
+ }
+
@Test
public void testRemoveGroovyAction() throws Exception {
// First save an action
diff --git
a/extensions/groovy-actions/services/src/test/resources/META-INF/cxs/actions/fieldInitializerAction.groovy
b/extensions/groovy-actions/services/src/test/resources/META-INF/cxs/actions/fieldInitializerAction.groovy
new file mode 100644
index 000000000..530b36e4f
--- /dev/null
+++
b/extensions/groovy-actions/services/src/test/resources/META-INF/cxs/actions/fieldInitializerAction.groovy
@@ -0,0 +1,27 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+import groovy.transform.Field
+
+// A Groovy @Field initializer runs when the script class is *instantiated*,
not when it is
+// compiled. Saving this action must compile it without instantiating it, so
this marker must not
+// be set afterwards. Setting a system property keeps the probe inert.
+@Field def sideEffect = {
System.setProperty("unomi.test.groovyFieldInitializerRan", "true") }()
+
+@Action(id = "fieldInitializerAction", actionExecutor =
"groovy:fieldInitializerAction")
+def execute() {
+ return EventService.NO_CHANGE
+}
diff --git a/itests/src/test/java/org/apache/unomi/itests/AllITs.java
b/itests/src/test/java/org/apache/unomi/itests/AllITs.java
index 89609d825..6a6881caf 100644
--- a/itests/src/test/java/org/apache/unomi/itests/AllITs.java
+++ b/itests/src/test/java/org/apache/unomi/itests/AllITs.java
@@ -61,6 +61,7 @@ import org.junit.runners.Suite.SuiteClasses;
RuleServiceIT.class,
PrivacyServiceIT.class,
GroovyActionsServiceIT.class,
+ GroovyActionsEndpointRoleSecurityIT.class,
GraphQLEventIT.class,
GraphQLListIT.class,
GraphQLProfileIT.class,
diff --git
a/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java
b/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java
index c279eeef7..a00e0199b 100644
--- a/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java
+++ b/itests/src/test/java/org/apache/unomi/itests/CorePersistenceITs.java
@@ -62,6 +62,7 @@ import org.junit.runners.Suite.SuiteClasses;
RuleServiceIT.class,
PrivacyServiceIT.class,
GroovyActionsServiceIT.class,
+ GroovyActionsEndpointRoleSecurityIT.class,
GraphQLEventIT.class,
GraphQLListIT.class,
GraphQLProfileIT.class,
diff --git
a/itests/src/test/java/org/apache/unomi/itests/GroovyActionsEndpointRoleSecurityIT.java
b/itests/src/test/java/org/apache/unomi/itests/GroovyActionsEndpointRoleSecurityIT.java
new file mode 100644
index 000000000..5802faf26
--- /dev/null
+++
b/itests/src/test/java/org/apache/unomi/itests/GroovyActionsEndpointRoleSecurityIT.java
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.unomi.itests;
+
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpDelete;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.ByteArrayEntity;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
+import org.ops4j.pax.exam.spi.reactors.PerSuite;
+
+import java.nio.charset.StandardCharsets;
+
+/**
+ * HTTP-level checks that system-admin-only REST endpoints reject tenant
private keys
+ * (including multipart upload / oneshot paths).
+ */
+@RunWith(PaxExam.class)
+@ExamReactorStrategy(PerSuite.class)
+public class GroovyActionsEndpointRoleSecurityIT extends BaseIT {
+
+
+
+
+
+ @Test
+ public void groovyActions_requiresSystemAdministrator() throws Exception {
+ String path =
getFullUrl("/cxs/groovyActions/rest-role-security-it-missing-action");
+
+ try (CloseableHttpResponse tenantAdmin = executeHttpRequest(new
HttpDelete(path), AuthType.PRIVATE_KEY)) {
+ Assert.assertEquals("Tenant private key must not delete groovy
actions",
+ 403, tenantAdmin.getStatusLine().getStatusCode());
+ }
+
+ try (CloseableHttpResponse jaasAdmin = executeHttpRequest(new
HttpDelete(path), AuthType.JAAS_ADMIN)) {
+ int status = jaasAdmin.getStatusLine().getStatusCode();
+ Assert.assertTrue("JAAS admin delete should be allowed (got " +
status + ")",
+ status == 200 || status == 204 || status == 404);
+ }
+ }
+
+ @Test
+ public void groovyActions_upload_requiresSystemAdministrator() throws
Exception {
+ String script = "// GroovyActionsEndpointRoleSecurityIT probe\nvoid
execute() {}\n";
+ HttpPost upload = multipartPost(getFullUrl("/cxs/groovyActions/"),
+ "----UnomiGroovyBoundary",
+ filePart("file", "RestRoleSecurityITProbe.groovy",
"text/plain", script));
+
+ try (CloseableHttpResponse tenantAdmin = executeHttpRequest(upload,
AuthType.PRIVATE_KEY)) {
+ Assert.assertEquals("Tenant private key must not upload groovy
actions",
+ 403, tenantAdmin.getStatusLine().getStatusCode());
+ }
+
+ HttpPost uploadJaas = multipartPost(getFullUrl("/cxs/groovyActions/"),
+ "----UnomiGroovyBoundaryJaas",
+ filePart("file", "RestRoleSecurityITProbe.groovy",
"text/plain", script));
+ try (CloseableHttpResponse jaasAdmin = executeHttpRequest(uploadJaas,
AuthType.JAAS_ADMIN)) {
+ Assert.assertEquals("JAAS admin should be allowed to upload groovy
actions",
+ 200, jaasAdmin.getStatusLine().getStatusCode());
+ }
+
+ try (CloseableHttpResponse cleanup = executeHttpRequest(
+ new
HttpDelete(getFullUrl("/cxs/groovyActions/RestRoleSecurityITProbe")),
AuthType.JAAS_ADMIN)) {
+ int status = cleanup.getStatusLine().getStatusCode();
+ Assert.assertTrue(status == 200 || status == 204 || status == 404);
+ }
+ }
+
+ private static HttpPost multipartPost(String url, String boundary,
String... parts) {
+ HttpPost post = new HttpPost(url);
+ StringBuilder body = new StringBuilder();
+ for (String part : parts) {
+ body.append("--").append(boundary).append("\r\n").append(part);
+ }
+ body.append("--").append(boundary).append("--\r\n");
+ post.setHeader("Content-Type", "multipart/form-data; boundary=" +
boundary);
+ post.setEntity(new
ByteArrayEntity(body.toString().getBytes(StandardCharsets.UTF_8)));
+ return post;
+ }
+
+ private static String part(String name, String contentType, String value) {
+ return "Content-Disposition: form-data; name=\"" + name + "\"\r\n"
+ + "Content-Type: " + contentType + "\r\n\r\n"
+ + value + "\r\n";
+ }
+
+ private static String filePart(String name, String filename, String
contentType, String value) {
+ return "Content-Disposition: form-data; name=\"" + name + "\";
filename=\"" + filename + "\"\r\n"
+ + "Content-Type: " + contentType + "\r\n\r\n"
+ + value + "\r\n";
+ }
+}
