You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by sh...@apache.org on 2026/09/28 15:45:48 UTC
(unomi) branch UNOMI-973-tenant-file-endpoints updated: UNOMI-973: Refuse a scheme Camel cannot resolve and keep a failed context switch from costing the batch its routes
This is an automated email from the ASF dual-hosted git repository.
asf-gitbox-commits pushed a commit to branch UNOMI-973-tenant-file-endpoints
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/UNOMI-973-tenant-file-endpoints by this push:
new d80dff251 UNOMI-973: Refuse a scheme Camel cannot resolve and keep a failed context switch from costing the batch its routes
d80dff251 is described below
commit d80dff251af33738fad17d0bc175fdfcb7acd4f9
Author: Serge Huber <sh...@apache.org>
AuthorDate: Mon Sep 28 17:45:43 2026 +0200
UNOMI-973: Refuse a scheme Camel cannot resolve and keep a failed context switch from costing the batch its routes
The scheme is matched in the case it is allowed in. Camel looks a component up by the exact name, so FILE: was accepted, stored, and failed when its route was built.
Switching to the system subject and the tenant can fail before anything is saved. That failure is now kept where a failed save already was, so the other configurations keep their routes.
The tenant-less validate() is private, the endpoint check runs in the base saveConfiguration so no configuration type can skip it, and the tenant directory name rule is one shared predicate.
Tests cover moveExisting, tempPrefix, tenant ids that name no directory, a host authority, the order of the system and tenant contexts, and the export side of the REST validation. The REST test runs on JUnit 5 like the rest of its module, and the containment IT removes the files it creates.
Co-Authored-By: Claude Fable 5.1 <no...@anthropic.com>
---
.../apache/unomi/router/api/EndpointValidator.java | 45 +++--
.../processor/ImportConfigByFileNameProcessor.java | 7 +-
.../route/ProfileExportCollectRouteBuilder.java | 6 +-
.../route/ProfileImportFromSourceRouteBuilder.java | 6 +-
.../core/route/RouterAbstractRouteBuilder.java | 32 +++-
.../core/route/FileEndpointContainmentTest.java | 111 ++++++------
.../core/route/RefusedConfigurationStatusTest.java | 96 ++++++----
.../router/core/route/RouterTestFixtures.java | 80 +++++++++
extensions/router/router-rest/pom.xml | 11 --
.../rest/AbstractConfigurationServiceEndpoint.java | 24 ++-
.../rest/ExportConfigurationServiceEndPoint.java | 15 +-
.../rest/ImportConfigurationServiceEndPoint.java | 15 +-
.../rest/ConfigurationEndpointValidationTest.java | 197 +++++++++++++++------
.../itests/ProfileImportExportContainmentIT.java | 9 +
14 files changed, 452 insertions(+), 202 deletions(-)
diff --git a/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java
index 10ecf0cbb..19893377f 100644
--- a/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java
+++ b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java
@@ -36,7 +36,9 @@ import java.util.regex.Pattern;
/**
* Decides whether the endpoint URI carried by an import or export configuration may be used.
*
- * <p>Two rules apply. The scheme must belong to the configured allow-list. And a {@code file}
+ * <p>Two rules apply. The scheme must belong to the configured allow-list, in the case it is listed
+ * in: Camel looks a component up by the exact name the URI carries, so {@code FILE:} names no
+ * component, and an endpoint accepted under that name would fail when its route is built. And a {@code file}
* endpoint must resolve inside one of the base directories the deployment permits — the directory
* the URI names, and every path-bearing option it carries, since validating only the directory would
* leave {@code file:///permitted/?fileName=../../elsewhere} open.
@@ -127,8 +129,10 @@ public final class EndpointValidator {
private static final String PROPERTY_PLACEHOLDER_TOKEN = "{{";
/**
- * A tenant id used as a directory name. The same shape the one-shot import accepts, so a tenant
- * cannot be {@code ..} or a path of its own and leave the directory this class confines it to.
+ * A tenant id used as a directory name: it cannot be {@code ..} or a path of its own and leave the
+ * directory this class confines it to. The tenant service is stricter about the ids it creates (no
+ * single character, no leading or trailing separator), so every id it accepts is accepted here;
+ * this pattern only has to make the id safe as one path segment.
*/
private static final Pattern TENANT_ID = Pattern.compile("[A-Za-z0-9_-]+");
@@ -140,7 +144,19 @@ public final class EndpointValidator {
}
/**
- * Validates the endpoint URI of an import or export configuration.
+ * Whether a tenant id can be used as the name of that tenant's directory: a single path segment
+ * made of letters, digits, hyphens and underscores.
+ *
+ * @param tenantId the tenant id, which may be {@code null}
+ * @return {@code true} when the id is safe to use as one directory name
+ */
+ public static boolean isTenantDirectoryName(String tenantId) {
+ return tenantId != null && TENANT_ID.matcher(tenantId).matches();
+ }
+
+ /**
+ * Validates the endpoint URI against the directories it is given, as they stand. Private: the only
+ * way in is {@link #validateForTenant}, so that no caller can skip the tenant's confinement.
*
* @param endpointUri the endpoint URI, as configured
* @param allowedSchemes the comma-separated list of allowed schemes
@@ -148,7 +164,7 @@ public final class EndpointValidator {
* resolve into
* @return {@code null} when the endpoint may be used, otherwise the reason it is refused
*/
- public static String validate(String endpointUri, String allowedSchemes, String permittedBaseDirs) {
+ private static String validate(String endpointUri, String allowedSchemes, String permittedBaseDirs) {
if (isBlank(endpointUri)) {
return "no endpoint is configured";
}
@@ -159,7 +175,7 @@ public final class EndpointValidator {
}
String scheme = endpointUri.substring(0, schemeSeparator);
- if (!containsIgnoreCase(split(allowedSchemes), scheme)) {
+ if (!split(allowedSchemes).contains(scheme)) {
return "endpoint scheme '" + scheme + "' is not allowed";
}
if (endpointUri.contains(PROPERTY_PLACEHOLDER_TOKEN)) {
@@ -169,7 +185,7 @@ public final class EndpointValidator {
try {
String refusal = validateLocalWorkDirectory(endpointUri, permittedBaseDirs);
- if (refusal != null || !FILE_SCHEME.equalsIgnoreCase(scheme)) {
+ if (refusal != null || !FILE_SCHEME.equals(scheme)) {
return refusal;
}
return validateContainment(endpointUri, permittedBaseDirs);
@@ -182,8 +198,8 @@ public final class EndpointValidator {
}
/**
- * Validates an endpoint the way {@link #validate(String, String, String)} does, after confining a
- * {@code file} path to {@code {baseDir}/{tenantId}} for every configured base directory.
+ * Validates the endpoint URI of an import or export configuration, with a {@code file} path
+ * confined to {@code {baseDir}/{tenantId}} for every configured base directory.
*
* @param endpointUri the endpoint URI, as configured
* @param allowedSchemes the comma-separated list of allowed schemes
@@ -192,7 +208,7 @@ public final class EndpointValidator {
* @return {@code null} when the endpoint may be used, otherwise the reason it is refused
*/
public static String validateForTenant(String endpointUri, String allowedSchemes, String permittedBaseDirs, String tenantId) {
- if (tenantId == null || !TENANT_ID.matcher(tenantId).matches()) {
+ if (!isTenantDirectoryName(tenantId)) {
return "no tenant is set for this configuration";
}
return validate(endpointUri, allowedSchemes, scopeToTenant(permittedBaseDirs, tenantId));
@@ -527,15 +543,6 @@ public final class EndpointValidator {
return values;
}
- private static boolean containsIgnoreCase(List<String> values, String searched) {
- for (String value : values) {
- if (value.equalsIgnoreCase(searched)) {
- return true;
- }
- }
- return false;
- }
-
private static boolean isBlank(String value) {
return value == null || value.trim().isEmpty();
}
diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/processor/ImportConfigByFileNameProcessor.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/processor/ImportConfigByFileNameProcessor.java
index 0ac7d4499..f110b44cc 100644
--- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/processor/ImportConfigByFileNameProcessor.java
+++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/processor/ImportConfigByFileNameProcessor.java
@@ -21,6 +21,7 @@ import org.apache.camel.Processor;
import org.apache.camel.component.file.GenericFile;
import org.apache.unomi.api.services.ExecutionContextManager;
import org.apache.unomi.api.tenants.TenantService;
+import org.apache.unomi.router.api.EndpointValidator;
import org.apache.unomi.router.api.ImportConfiguration;
import org.apache.unomi.router.api.RouterConstants;
import org.apache.unomi.router.api.services.ImportExportConfigurationService;
@@ -170,12 +171,8 @@ public class ImportConfigByFileNameProcessor implements Processor {
* @return true if the tenant ID is valid, false otherwise
*/
private boolean isValidTenantId(String tenantId) {
- if (tenantId == null || tenantId.isEmpty()) {
- return false;
- }
-
// Only allow alphanumeric characters, hyphens, and underscores in tenant IDs
- return tenantId.matches("^[a-zA-Z0-9_-]+$");
+ return EndpointValidator.isTenantDirectoryName(tenantId);
}
/**
diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java
index f74cab573..bd652f3af 100644
--- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java
+++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileExportCollectRouteBuilder.java
@@ -141,11 +141,7 @@ public class ProfileExportCollectRouteBuilder extends RouterAbstractRouteBuilder
LOGGER.warn("Export configuration incomplete, route {} will be skipped!", exportConfiguration.getItemId());
}
}
- if (refused > 0) {
- LOGGER.warn("{} of {} export configuration(s) name a destination that is refused and run no route; each is "
- + "marked {} and logged above. After an upgrade, check that their files sit under "
- + "{baseDir}/{tenantId}.", refused, exportConfigurationList.size(), RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT);
- }
+ logRefused(LOGGER, refused, exportConfigurationList.size(), "export", "destination");
}
/**
diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java
index 688b43b59..f9919a670 100644
--- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java
+++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/ProfileImportFromSourceRouteBuilder.java
@@ -189,11 +189,7 @@ public class ProfileImportFromSourceRouteBuilder extends RouterAbstractRouteBuil
}
}
}
- if (refused > 0) {
- LOGGER.warn("{} of {} import configuration(s) name a source that is refused and run no route; each is "
- + "marked {} and logged above. After an upgrade, check that their files sit under "
- + "{baseDir}/{tenantId}.", refused, importConfigurationList.size(), RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT);
- }
+ logRefused(LOGGER, refused, importConfigurationList.size(), "import", "source");
}
/**
diff --git a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java
index 59a84fc39..af94e719f 100644
--- a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java
+++ b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/route/RouterAbstractRouteBuilder.java
@@ -176,19 +176,47 @@ public abstract class RouterAbstractRouteBuilder extends RouteBuilder {
* subject's permissions and switches to this tenant. When no context manager is available the mark
* is written as it stands, which is what the unit tests do. This is the only entry point: writing
* the mark outside the configuration's tenant is not something a route builder may do.
+ *
+ * <p>Switching context can fail by itself, before anything is saved: there is no system subject
+ * while the security service is still starting. That failure is kept here for the reason
+ * {@link #saveQuietly} keeps a failed save: the route is built or skipped as decided, and the
+ * other configurations of the batch keep theirs.
*/
protected <T extends ImportExportConfiguration> void recordEndpointOutcome(
T configuration, ImportExportConfigurationService<T> service, String refusal,
ExecutionContextManager executionContextManager) {
String tenantId = configuration.getTenantId();
if (executionContextManager != null && tenantId != null && !tenantId.isEmpty()) {
- executionContextManager.executeAsSystem(() -> executionContextManager.executeAsTenant(tenantId,
- () -> recordEndpointOutcome(configuration, service, refusal)));
+ try {
+ executionContextManager.executeAsSystem(() -> executionContextManager.executeAsTenant(tenantId,
+ () -> recordEndpointOutcome(configuration, service, refusal)));
+ } catch (RuntimeException e) {
+ LOGGER.error("Could not record the endpoint outcome on configuration {} as tenant {}; its route is "
+ + "built or skipped as decided, only the record of it is missing", configuration.getItemId(), tenantId, e);
+ }
} else {
recordEndpointOutcome(configuration, service, refusal);
}
}
+ /**
+ * Says, once per batch, how many configurations were refused, so that an upgrade that leaves files
+ * outside the tenant directories shows as one line rather than as a route that quietly never runs.
+ *
+ * @param logger the logger of the route builder that built the batch
+ * @param refused how many configurations of the batch were refused
+ * @param total how many configurations the batch holds
+ * @param kind {@code import} or {@code export}
+ * @param endpoint what the configuration names: a {@code source} or a {@code destination}
+ */
+ protected void logRefused(Logger logger, int refused, int total, String kind, String endpoint) {
+ if (refused > 0) {
+ logger.warn("{} of {} {} configuration(s) name a {} that is refused and run no route; each is "
+ + "marked {} and logged above. After an upgrade, check that their files sit under "
+ + "{baseDir}/{tenantId}.", refused, total, kind, endpoint, RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT);
+ }
+ }
+
/**
* Gets the appropriate endpoint URI based on configuration type and operation.
*
diff --git a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java
index 2a71b7052..8d328f04d 100644
--- a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java
+++ b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java
@@ -18,7 +18,6 @@ package org.apache.unomi.router.core.route;
import org.apache.camel.component.jackson.JacksonDataFormat;
import org.apache.camel.impl.DefaultCamelContext;
-import org.apache.unomi.api.services.ProfileService;
import org.apache.unomi.router.api.ExportConfiguration;
import org.apache.unomi.router.api.ImportConfiguration;
import org.apache.unomi.router.api.ProfileToImport;
@@ -36,12 +35,14 @@ import java.io.IOException;
import java.lang.reflect.Proxy;
import java.nio.file.Files;
import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
+import static org.apache.unomi.router.core.route.RouterTestFixtures.TENANT;
+import static org.apache.unomi.router.core.route.RouterTestFixtures.fileUri;
+import static org.apache.unomi.router.core.route.RouterTestFixtures.noOpProfileService;
+import static org.apache.unomi.router.core.route.RouterTestFixtures.recurrentExport;
+import static org.apache.unomi.router.core.route.RouterTestFixtures.recurrentImport;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
@@ -79,8 +80,6 @@ public class FileEndpointContainmentTest {
/** The shipped default. The containment rules must hold while {@code file} is an allowed scheme. */
private static final String DEFAULT_ALLOWED_ENDPOINTS = "file,ftp,sftp,ftps";
- private static final String TENANT = "acme";
-
private static final Map<String, String> NO_KAFKA = new HashMap<>();
@Rule
@@ -335,6 +334,22 @@ public class FileEndpointContainmentTest {
assertRouteRefused("donefilename-escape", "doneFileName carries a path and must be contained too");
}
+ @Test
+ public void exportRouteIsRefusedWhenMoveExistingOptionEscapesPermittedBaseDir() throws Exception {
+ addExportRoutes(recurrentExport("moveexisting-escape", fileUri(permittedExportDir,
+ "?fileName=profiles.csv&fileExist=Move&moveExisting=../" + arbitraryDir.getName())));
+
+ assertRouteRefused("moveexisting-escape", "moveExisting says where the file already there is moved to");
+ }
+
+ @Test
+ public void exportRouteIsRefusedWhenTempPrefixOptionEscapesPermittedBaseDir() throws Exception {
+ addExportRoutes(recurrentExport("tempprefix-escape", fileUri(permittedExportDir,
+ "?fileName=profiles.csv&tempPrefix=../" + arbitraryDir.getName() + "/")));
+
+ assertRouteRefused("tempprefix-escape", "tempPrefix is put in front of the name, directories included");
+ }
+
@Test
public void importRouteIsRefusedWhenPathBearingOptionIsWrappedInRaw() throws Exception {
addImportRoutes(recurrentImport("raw-escape",
@@ -519,6 +534,26 @@ public class FileEndpointContainmentTest {
assertRouteRefused("scheme-denied", "file is not in the configured scheme allow-list");
}
+ @Test
+ public void importRouteIsRefusedWhenSchemeIsNotInTheCaseItIsAllowedIn() throws Exception {
+ addImportRoutes(
+ recurrentImport("upper-case-scheme", "FILE://" + permittedImportDir.getAbsolutePath() + "?fileName=profiles.csv"),
+ recurrentImport("mixed-case-remote", "Ftp://ftp.example.com/profiles?fileName=profiles.csv"));
+
+ assertRouteRefused("upper-case-scheme",
+ "Camel looks a component up by the exact name, so FILE names none and the route could not be built");
+ assertRouteRefused("mixed-case-remote", "the same holds for a remote scheme");
+ }
+
+ @Test
+ public void importRouteIsRefusedWhenSourceNamesAHost() throws Exception {
+ addImportRoutes(recurrentImport("host-authority",
+ "file://localhost" + permittedImportDir.getAbsolutePath() + "?fileName=profiles.csv"));
+
+ assertRouteRefused("host-authority",
+ "Camel knows no authority: the host is the first segment of a path relative to the working directory");
+ }
+
@Test
public void importRouteIsRefusedWhenSchemeIsOnlyASubstringOfAnAllowedScheme() throws Exception {
addImportRoutes(recurrentImport("substring-scheme", "fil://" + permittedImportDir.getAbsolutePath() + "?fileName=profiles.csv"));
@@ -711,6 +746,23 @@ public class FileEndpointContainmentTest {
assertRouteRefused("traversing-tenant", "a tenant id that is a path of its own is not confined to a directory");
}
+ @Test
+ public void importRouteIsRefusedWhenTheTenantIdNamesNoDirectoryOfItsOwn() throws Exception {
+ // each of these would resolve to the shared base directory, or to its parent
+ String[] tenantIds = {".", "..", "", " "};
+ ImportConfiguration[] configurations = new ImportConfiguration[tenantIds.length];
+ for (int i = 0; i < tenantIds.length; i++) {
+ configurations[i] = recurrentImport("tenant-" + i, fileUri(importRoot, "?fileName=profiles.csv"));
+ configurations[i].setTenantId(tenantIds[i]);
+ }
+
+ addImportRoutes(configurations);
+
+ for (int i = 0; i < tenantIds.length; i++) {
+ assertRouteRefused("tenant-" + i, "the tenant id '" + tenantIds[i] + "' is not the name of a directory");
+ }
+ }
+
// ---------------------------------------------------------------------------------------------
// Fixtures
// ---------------------------------------------------------------------------------------------
@@ -731,34 +783,6 @@ public class FileEndpointContainmentTest {
camelContext.getRouteDefinition(routeId));
}
- private String fileUri(File directory, String suffix) {
- return "file://" + directory.getAbsolutePath() + suffix;
- }
-
- private ImportConfiguration recurrentImport(String itemId, String source) {
- ImportConfiguration configuration = new ImportConfiguration();
- configuration.setItemId(itemId);
- configuration.setTenantId(TENANT);
- configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT);
- configuration.setActive(true);
- configuration.getProperties().put("source", source);
- configuration.getProperties().put("mapping", Collections.singletonMap("0", 0));
- return configuration;
- }
-
- private ExportConfiguration recurrentExport(String itemId, String destination) {
- ExportConfiguration configuration = new ExportConfiguration();
- configuration.setItemId(itemId);
- configuration.setTenantId(TENANT);
- configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT);
- configuration.setActive(true);
- configuration.getProperties().put("destination", destination);
- configuration.getProperties().put("mapping", Collections.singletonMap("0", "firstName"));
- configuration.getProperties().put("segment", "exportSegment");
- configuration.getProperties().put("period", "1m");
- return configuration;
- }
-
private void addImportRoutes(ImportConfiguration... configurations) throws Exception {
addImportRoutes(DEFAULT_ALLOWED_ENDPOINTS, configurations);
}
@@ -811,23 +835,4 @@ public class FileEndpointContainmentTest {
new Class<?>[]{ImportExportConfigurationService.class},
(proxy, method, args) -> "save".equals(method.getName()) ? args[0] : null);
}
-
- /**
- * The route builders ask the profile service for the profile property types while they build.
- * Nothing in these tests depends on what it answers.
- */
- private static ProfileService noOpProfileService() {
- return (ProfileService) Proxy.newProxyInstance(
- ProfileService.class.getClassLoader(),
- new Class<?>[]{ProfileService.class},
- (proxy, method, args) -> {
- if (Collection.class.isAssignableFrom(method.getReturnType())) {
- return Collections.emptyList();
- }
- if (List.class.isAssignableFrom(method.getReturnType())) {
- return Collections.emptyList();
- }
- return null;
- });
- }
}
diff --git a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/RefusedConfigurationStatusTest.java b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/RefusedConfigurationStatusTest.java
index 7a54022ff..8185ea88b 100644
--- a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/RefusedConfigurationStatusTest.java
+++ b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/RefusedConfigurationStatusTest.java
@@ -19,7 +19,6 @@ package org.apache.unomi.router.core.route;
import org.apache.camel.component.jackson.JacksonDataFormat;
import org.apache.camel.impl.DefaultCamelContext;
import org.apache.unomi.api.services.ExecutionContextManager;
-import org.apache.unomi.api.services.ProfileService;
import org.apache.unomi.router.api.ExportConfiguration;
import org.apache.unomi.router.api.ImportConfiguration;
import org.apache.unomi.router.api.ProfileToImport;
@@ -41,6 +40,9 @@ import java.util.List;
import java.util.Map;
import java.util.function.Supplier;
+import static org.apache.unomi.router.core.route.RouterTestFixtures.TENANT;
+import static org.apache.unomi.router.core.route.RouterTestFixtures.fileUri;
+import static org.apache.unomi.router.core.route.RouterTestFixtures.noOpProfileService;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
@@ -72,8 +74,6 @@ public class RefusedConfigurationStatusTest {
private DefaultCamelContext camelContext;
- private static final String TENANT = "acme";
-
private File importRoot;
private File exportRoot;
private File permittedImportDir;
@@ -86,6 +86,12 @@ public class RefusedConfigurationStatusTest {
/** The tenant the fake context manager is currently running as, or {@code null} outside any. */
private String currentTenant;
+ /** Whether the fake context manager is running as the system subject. */
+ private boolean systemSubject;
+
+ /** Whether the subject the tenant context was built from may save, or {@code null} outside any. */
+ private Boolean tenantContextMaySave;
+
@Before
public void setUp() throws Exception {
importRoot = tmp.newFolder("permitted-import");
@@ -147,6 +153,36 @@ public class RefusedConfigurationStatusTest {
assertNull("the batch's own context must be restored afterwards", currentTenant);
}
+ @Test
+ public void aRefusedConfigurationIsRecordedWithThePermissionToSaveIt() throws Exception {
+ // The route refresh runs on a thread that has no subject. The tenant context takes the
+ // permissions of the subject it is built from, so the system subject has to be there first.
+ ImportConfiguration configuration = recurrentImport(fileUri(arbitraryDir, "?fileName=profiles.csv"));
+ importConfigurations.maySave = () -> tenantContextMaySave;
+
+ addImportRoutes(recordingContextManager(), configuration);
+
+ assertEquals("the tenant context must be entered from the system subject, not the other way round",
+ Boolean.TRUE, importConfigurations.maySaveAtLastSave);
+ }
+
+ @Test
+ public void aContextThatCannotBeSwitchedDoesNotCostTheBatchItsOtherRoutes() throws Exception {
+ ExecutionContextManager noSystemSubject = (ExecutionContextManager) Proxy.newProxyInstance(
+ ExecutionContextManager.class.getClassLoader(),
+ new Class<?>[]{ExecutionContextManager.class},
+ (proxy, method, args) -> {
+ throw new SecurityException("Failed to obtain system subject");
+ });
+
+ addImportRoutes(noSystemSubject, recurrentImport(fileUri(arbitraryDir, "?fileName=profiles.csv")),
+ inBoundsImport("in-bounds"));
+
+ assertNull("the refused configuration still gets no route", camelContext.getRouteDefinition("out-of-bounds"));
+ assertNotNull("failing to switch context must not cost the other configurations their routes",
+ camelContext.getRouteDefinition("in-bounds"));
+ }
+
@Test
public void aStoreThatCannotRecordTheRefusalDoesNotCostTheBatchItsOtherRoutes() throws Exception {
importConfigurations.unwritable = true;
@@ -238,10 +274,6 @@ public class RefusedConfigurationStatusTest {
// Fixtures
// ---------------------------------------------------------------------------------------------
- private String fileUri(File directory, String suffix) {
- return "file://" + directory.getAbsolutePath() + suffix;
- }
-
private ImportConfiguration inBoundsImport(String itemId) {
ImportConfiguration configuration = recurrentImport(fileUri(permittedImportDir, "?fileName=profiles.csv"));
configuration.setItemId(itemId);
@@ -249,27 +281,11 @@ public class RefusedConfigurationStatusTest {
}
private ImportConfiguration recurrentImport(String source) {
- ImportConfiguration configuration = new ImportConfiguration();
- configuration.setItemId("out-of-bounds");
- configuration.setTenantId(TENANT);
- configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT);
- configuration.setActive(true);
- configuration.getProperties().put("source", source);
- configuration.getProperties().put("mapping", Collections.singletonMap("0", 0));
- return configuration;
+ return RouterTestFixtures.recurrentImport("out-of-bounds", source);
}
private ExportConfiguration recurrentExport(String destination) {
- ExportConfiguration configuration = new ExportConfiguration();
- configuration.setItemId("out-of-bounds");
- configuration.setTenantId(TENANT);
- configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT);
- configuration.setActive(true);
- configuration.getProperties().put("destination", destination);
- configuration.getProperties().put("mapping", Collections.singletonMap("0", "firstName"));
- configuration.getProperties().put("segment", "exportSegment");
- configuration.getProperties().put("period", "1m");
- return configuration;
+ return RouterTestFixtures.recurrentExport("out-of-bounds", destination);
}
private void addImportRoutes(ImportConfiguration... configurations) throws Exception {
@@ -293,8 +309,9 @@ public class RefusedConfigurationStatusTest {
/**
* Runs an operation as the tenant it is asked to, and remembers which one that is for as long as
* the operation lasts, the way the real manager swaps the thread's context. {@code executeAsSystem}
- * only runs its operation: the real one installs the system subject, which this stand-in has no
- * need of, and the tenant switch is what the test records.
+ * installs the system subject for as long as its operation lasts, and {@code executeAsTenant}
+ * builds its context from the subject that is there when it is called, as the real ones do: a
+ * tenant context entered without the system subject carries no permission to save.
*/
private ExecutionContextManager recordingContextManager() {
return (ExecutionContextManager) Proxy.newProxyInstance(
@@ -302,17 +319,26 @@ public class RefusedConfigurationStatusTest {
new Class<?>[]{ExecutionContextManager.class},
(proxy, method, args) -> {
if ("executeAsSystem".equals(method.getName())) {
- return invokeContextOperation(args[0]);
+ boolean previous = systemSubject;
+ systemSubject = true;
+ try {
+ return invokeContextOperation(args[0]);
+ } finally {
+ systemSubject = previous;
+ }
}
if (!"executeAsTenant".equals(method.getName())) {
return null;
}
String previous = currentTenant;
+ Boolean previousMaySave = tenantContextMaySave;
currentTenant = (String) args[0];
+ tenantContextMaySave = systemSubject;
try {
return invokeContextOperation(args[1]);
} finally {
currentTenant = previous;
+ tenantContextMaySave = previousMaySave;
}
});
}
@@ -337,14 +363,6 @@ public class RefusedConfigurationStatusTest {
camelContext.addRoutes(builder);
}
- private static ProfileService noOpProfileService() {
- return (ProfileService) Proxy.newProxyInstance(
- ProfileService.class.getClassLoader(),
- new Class<?>[]{ProfileService.class},
- (proxy, method, args) -> java.util.Collection.class.isAssignableFrom(method.getReturnType())
- ? Collections.emptyList() : null);
- }
-
/**
* Stores what it is given, and remembers whether the last save asked for the running route to be
* refreshed — a refused configuration must not, or the refresh loops.
@@ -360,6 +378,11 @@ public class RefusedConfigurationStatusTest {
private String tenantAtLastSave;
+ /** Whether the current context may save; the real store refuses a save from one that may not. */
+ private Supplier<Boolean> maySave = () -> null;
+
+ private Boolean maySaveAtLastSave;
+
/** Stands in for a store that cannot be written to -- Elasticsearch unreachable at start-up. */
private boolean unwritable;
@@ -384,6 +407,7 @@ public class RefusedConfigurationStatusTest {
}
lastSaveAskedForARouteRefresh = updateRunningRoute;
tenantAtLastSave = currentTenant.get();
+ maySaveAtLastSave = maySave.get();
stored.put(itemIdOf(configuration), configuration);
return configuration;
}
diff --git a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/RouterTestFixtures.java b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/RouterTestFixtures.java
new file mode 100644
index 000000000..ff1ca1b8d
--- /dev/null
+++ b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/RouterTestFixtures.java
@@ -0,0 +1,80 @@
+/*
+ * 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.router.core.route;
+
+import org.apache.unomi.api.services.ProfileService;
+import org.apache.unomi.router.api.ExportConfiguration;
+import org.apache.unomi.router.api.ImportConfiguration;
+import org.apache.unomi.router.api.RouterConstants;
+
+import java.io.File;
+import java.lang.reflect.Proxy;
+import java.util.Collection;
+import java.util.Collections;
+
+/**
+ * What a recurrent configuration looks like, for the tests that build routes out of one. Kept in one
+ * place so that the tests of route construction and of the recorded status cannot come to disagree
+ * about it.
+ */
+final class RouterTestFixtures {
+
+ static final String TENANT = "acme";
+
+ private RouterTestFixtures() {
+ }
+
+ static String fileUri(File directory, String suffix) {
+ return "file://" + directory.getAbsolutePath() + suffix;
+ }
+
+ static ImportConfiguration recurrentImport(String itemId, String source) {
+ ImportConfiguration configuration = new ImportConfiguration();
+ configuration.setItemId(itemId);
+ configuration.setTenantId(TENANT);
+ configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT);
+ configuration.setActive(true);
+ configuration.getProperties().put("source", source);
+ configuration.getProperties().put("mapping", Collections.singletonMap("0", 0));
+ return configuration;
+ }
+
+ static ExportConfiguration recurrentExport(String itemId, String destination) {
+ ExportConfiguration configuration = new ExportConfiguration();
+ configuration.setItemId(itemId);
+ configuration.setTenantId(TENANT);
+ configuration.setConfigType(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT);
+ configuration.setActive(true);
+ configuration.getProperties().put("destination", destination);
+ configuration.getProperties().put("mapping", Collections.singletonMap("0", "firstName"));
+ configuration.getProperties().put("segment", "exportSegment");
+ configuration.getProperties().put("period", "1m");
+ return configuration;
+ }
+
+ /**
+ * The route builders ask the profile service for the profile property types while they build.
+ * Nothing in these tests depends on what it answers.
+ */
+ static ProfileService noOpProfileService() {
+ return (ProfileService) Proxy.newProxyInstance(
+ ProfileService.class.getClassLoader(),
+ new Class<?>[]{ProfileService.class},
+ (proxy, method, args) -> Collection.class.isAssignableFrom(method.getReturnType())
+ ? Collections.emptyList() : null);
+ }
+}
diff --git a/extensions/router/router-rest/pom.xml b/extensions/router/router-rest/pom.xml
index 5d9904848..ef2c4b848 100644
--- a/extensions/router/router-rest/pom.xml
+++ b/extensions/router/router-rest/pom.xml
@@ -118,17 +118,6 @@
<artifactId>mockito-core</artifactId>
<scope>test</scope>
</dependency>
- <dependency>
- <groupId>junit</groupId>
- <artifactId>junit</artifactId>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>org.junit.vintage</groupId>
- <artifactId>junit-vintage-engine</artifactId>
- <version>${junit-jupiter.version}</version>
- <scope>test</scope>
- </dependency>
</dependencies>
<build>
diff --git a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java
index 172e33f44..eb0548dee 100644
--- a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java
+++ b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/AbstractConfigurationServiceEndpoint.java
@@ -20,6 +20,7 @@ import org.apache.unomi.api.ExecutionContext;
import org.apache.unomi.api.services.ConfigSharingService;
import org.apache.unomi.api.services.ExecutionContextManager;
import org.apache.unomi.router.api.EndpointValidator;
+import org.apache.unomi.router.api.ImportExportConfiguration;
import org.apache.unomi.router.api.RouterConstants;
import org.apache.unomi.router.api.services.ImportExportConfigurationService;
@@ -33,7 +34,7 @@ import java.util.List;
*
* @param <T> configuration item type
*/
-public abstract class AbstractConfigurationServiceEndpoint<T> {
+public abstract class AbstractConfigurationServiceEndpoint<T extends ImportExportConfiguration> {
protected ImportExportConfigurationService<T> configurationService;
@@ -57,7 +58,7 @@ public abstract class AbstractConfigurationServiceEndpoint<T> {
* @param permittedBaseDirsProperty the shared property holding the base directories for this direction
* @param tenantId the tenant the configuration will be stored for
*/
- protected void refuseIfEndpointCannotBeHonoured(String endpointUri, String permittedBaseDirsProperty, String tenantId) {
+ private void refuseIfEndpointCannotBeHonoured(String endpointUri, String permittedBaseDirsProperty, String tenantId) {
String allowedSchemes = (String) configSharingService.getProperty(RouterConstants.CONFIG_ALLOWED_ENDPOINTS);
String permittedBaseDirs = (String) configSharingService.getProperty(permittedBaseDirsProperty);
if (allowedSchemes == null || permittedBaseDirs == null) {
@@ -87,7 +88,7 @@ public abstract class AbstractConfigurationServiceEndpoint<T> {
* tenant would let the check pass for a directory the caller does not own. A system caller may name
* the tenant. When the body names none, the current context supplies it.
*/
- protected String tenantToConfine(String configuredTenantId) {
+ private String tenantToConfine(String configuredTenantId) {
ExecutionContext context = executionContextManager == null ? null : executionContextManager.getCurrentContext();
if (context != null && !context.isSystem()) {
return context.getTenantId();
@@ -112,8 +113,15 @@ public abstract class AbstractConfigurationServiceEndpoint<T> {
return this.configurationService.getAll();
}
+ /** The property of the configuration that holds its endpoint URI. */
+ protected abstract String endpointProperty();
+
+ /** The shared property that holds the base directories permitted for this direction. */
+ protected abstract String permittedBaseDirsProperty();
+
/**
- * Creates or updates a router configuration.
+ * Creates or updates a router configuration. The endpoint of a recurrent configuration is validated
+ * here, before the store is reached, so that no configuration type can be saved without it.
*
* @param configuration the configuration to save
* @return the persisted configuration
@@ -123,7 +131,13 @@ public abstract class AbstractConfigurationServiceEndpoint<T> {
@Path("/")
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
- public abstract T saveConfiguration(T configuration);
+ public T saveConfiguration(T configuration) {
+ if (RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT.equals(configuration.getConfigType())) {
+ refuseIfEndpointCannotBeHonoured((String) configuration.getProperties().get(endpointProperty()),
+ permittedBaseDirsProperty(), tenantToConfine(configuration.getTenantId()));
+ }
+ return configurationService.save(configuration, true);
+ }
/**
* Returns the configuration with the given id.
diff --git a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ExportConfigurationServiceEndPoint.java b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ExportConfigurationServiceEndPoint.java
index 18326a720..db02500ec 100644
--- a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ExportConfigurationServiceEndPoint.java
+++ b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ExportConfigurationServiceEndPoint.java
@@ -136,12 +136,17 @@ public class ExportConfigurationServiceEndPoint extends AbstractConfigurationSer
*/
@Override
public ExportConfiguration saveConfiguration(ExportConfiguration exportConfiguration) {
- if (RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT.equals(exportConfiguration.getConfigType())) {
- refuseIfEndpointCannotBeHonoured((String) exportConfiguration.getProperties().get("destination"),
- RouterConstants.CONFIG_EXPORT_BASE_DIRS, tenantToConfine(exportConfiguration.getTenantId()));
- }
+ return super.saveConfiguration(exportConfiguration);
+ }
- return configurationService.save(exportConfiguration, true);
+ @Override
+ protected String endpointProperty() {
+ return "destination";
+ }
+
+ @Override
+ protected String permittedBaseDirsProperty() {
+ return RouterConstants.CONFIG_EXPORT_BASE_DIRS;
}
/**
diff --git a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ImportConfigurationServiceEndPoint.java b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ImportConfigurationServiceEndPoint.java
index 72c6515ce..2fba3797b 100644
--- a/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ImportConfigurationServiceEndPoint.java
+++ b/extensions/router/router-rest/src/main/java/org/apache/unomi/router/rest/ImportConfigurationServiceEndPoint.java
@@ -118,12 +118,17 @@ public class ImportConfigurationServiceEndPoint extends AbstractConfigurationSer
*/
@Override
public ImportConfiguration saveConfiguration(ImportConfiguration importConfiguration) {
- if (RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT.equals(importConfiguration.getConfigType())) {
- refuseIfEndpointCannotBeHonoured((String) importConfiguration.getProperties().get("source"),
- RouterConstants.CONFIG_IMPORT_BASE_DIRS, tenantToConfine(importConfiguration.getTenantId()));
- }
+ return super.saveConfiguration(importConfiguration);
+ }
+
+ @Override
+ protected String endpointProperty() {
+ return "source";
+ }
- return configurationService.save(importConfiguration, true);
+ @Override
+ protected String permittedBaseDirsProperty() {
+ return RouterConstants.CONFIG_IMPORT_BASE_DIRS;
}
/**
diff --git a/extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java b/extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java
index bcdbc66d0..32fa3fac3 100644
--- a/extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java
+++ b/extensions/router/router-rest/src/test/java/org/apache/unomi/router/rest/ConfigurationEndpointValidationTest.java
@@ -23,10 +23,9 @@ import org.apache.unomi.router.api.ExportConfiguration;
import org.apache.unomi.router.api.ImportConfiguration;
import org.apache.unomi.router.api.RouterConstants;
import org.apache.unomi.router.api.services.ImportExportConfigurationService;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
import javax.ws.rs.WebApplicationException;
import java.io.File;
@@ -38,12 +37,12 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
/**
* A configuration whose endpoint cannot be honoured must be refused when it is saved, not silently
@@ -60,8 +59,8 @@ import static org.junit.Assert.fail;
*/
public class ConfigurationEndpointValidationTest {
- @Rule
- public TemporaryFolder tmp = new TemporaryFolder();
+ @TempDir
+ File tmp;
private static final String TENANT = "acme";
@@ -77,15 +76,15 @@ public class ConfigurationEndpointValidationTest {
private ImportConfigurationServiceEndPoint importEndpoint;
private ExportConfigurationServiceEndPoint exportEndpoint;
- @Before
+ @BeforeEach
public void setUp() throws Exception {
- importRoot = tmp.newFolder("permitted-import");
+ importRoot = newFolder("permitted-import");
permittedImportDir = new File(importRoot, TENANT);
assertTrue(permittedImportDir.mkdirs());
- exportRoot = tmp.newFolder("permitted-export");
+ exportRoot = newFolder("permitted-export");
permittedExportDir = new File(exportRoot, TENANT);
assertTrue(permittedExportDir.mkdirs());
- arbitraryDir = tmp.newFolder("arbitrary");
+ arbitraryDir = newFolder("arbitrary");
InMemoryConfigSharingService configSharingService = new InMemoryConfigSharingService();
configSharingService.setProperty(RouterConstants.CONFIG_ALLOWED_ENDPOINTS, "file,ftp,sftp,ftps");
@@ -109,7 +108,8 @@ public class ConfigurationEndpointValidationTest {
recurrentImport(fileUri(permittedImportDir, "?fileName=profiles.csv")));
assertEquals("in-bounds", saved.getItemId());
- assertTrue("the configuration should have been stored", importConfigurations.contains("in-bounds"));
+ assertTrue(importConfigurations.contains("in-bounds"),
+ "the configuration should have been stored");
}
@Test
@@ -122,7 +122,8 @@ public class ConfigurationEndpointValidationTest {
ImportConfiguration configuration = recurrentImport(fileUri(permittedImportDir, "?fileName=profiles.csv"));
assertUnavailable(() -> starting.saveConfiguration(configuration));
- assertFalse("nothing is stored while the endpoint cannot be judged", importConfigurations.contains("in-bounds"));
+ assertFalse(importConfigurations.contains("in-bounds"),
+ "nothing is stored while the endpoint cannot be judged");
}
@Test
@@ -132,7 +133,8 @@ public class ConfigurationEndpointValidationTest {
+ "?fileName=profiles.csv&localWorkDirectory=" + arbitraryDir.getAbsolutePath());
assertRefused(() -> importEndpoint.saveConfiguration(configuration));
- assertFalse("a refused configuration must not be stored", importConfigurations.contains("in-bounds"));
+ assertFalse(importConfigurations.contains("in-bounds"),
+ "a refused configuration must not be stored");
}
@Test
@@ -158,7 +160,8 @@ public class ConfigurationEndpointValidationTest {
ExportConfiguration configuration = recurrentExport(fileUri(permittedExportDir, "?fileName=profiles.csv"));
assertUnavailable(() -> starting.saveConfiguration(configuration));
- assertFalse("nothing is stored while the endpoint cannot be judged", exportConfigurations.contains("in-bounds"));
+ assertFalse(exportConfigurations.contains("in-bounds"),
+ "nothing is stored while the endpoint cannot be judged");
}
@Test
@@ -166,7 +169,8 @@ public class ConfigurationEndpointValidationTest {
ImportConfiguration configuration = recurrentImport(fileUri(arbitraryDir, "?fileName=profiles.csv"));
assertRefused(() -> importEndpoint.saveConfiguration(configuration));
- assertFalse("a refused configuration must not be stored", importConfigurations.contains("in-bounds"));
+ assertFalse(importConfigurations.contains("in-bounds"),
+ "a refused configuration must not be stored");
}
@Test
@@ -182,7 +186,8 @@ public class ConfigurationEndpointValidationTest {
ImportConfiguration configuration = recurrentImport(fileUri(permittedImportDir, "?fileName=profiles%00.csv"));
assertRefused(() -> importEndpoint.saveConfiguration(configuration));
- assertFalse("a refused configuration must not be stored", importConfigurations.contains("in-bounds"));
+ assertFalse(importConfigurations.contains("in-bounds"),
+ "a refused configuration must not be stored");
}
@Test
@@ -192,7 +197,8 @@ public class ConfigurationEndpointValidationTest {
assertRefused(() -> importEndpoint.saveConfiguration(
recurrentImport(fileUri(otherTenant, "?fileName=profiles.csv"))));
- assertFalse("a refused configuration must not be stored", importConfigurations.contains("in-bounds"));
+ assertFalse(importConfigurations.contains("in-bounds"),
+ "a refused configuration must not be stored");
}
@Test
@@ -201,8 +207,8 @@ public class ConfigurationEndpointValidationTest {
configuration.setTenantId(null);
assertRefused(() -> importEndpoint.saveConfiguration(configuration));
- assertFalse("a configuration with no tenant must not fall back to the shared directory",
- importConfigurations.contains("in-bounds"));
+ assertFalse(importConfigurations.contains("in-bounds"),
+ "a configuration with no tenant must not fall back to the shared directory");
}
@Test
@@ -232,8 +238,8 @@ public class ConfigurationEndpointValidationTest {
configuration.setTenantId("beta");
assertRefused(() -> importEndpoint.saveConfiguration(configuration));
- assertFalse("acme may not have a configuration judged against beta's directory by naming beta",
- importConfigurations.contains("in-bounds"));
+ assertFalse(importConfigurations.contains("in-bounds"),
+ "acme may not have a configuration judged against beta's directory by naming beta");
}
@Test
@@ -249,8 +255,8 @@ public class ConfigurationEndpointValidationTest {
importEndpoint.saveConfiguration(configuration);
- assertTrue("the system may configure any tenant, judged against that tenant's directory",
- importConfigurations.contains("in-bounds"));
+ assertTrue(importConfigurations.contains("in-bounds"),
+ "the system may configure any tenant, judged against that tenant's directory");
}
@Test
@@ -261,16 +267,16 @@ public class ConfigurationEndpointValidationTest {
configuration.setTenantId("acme/../beta");
assertRefused(() -> importEndpoint.saveConfiguration(configuration));
- assertFalse("a tenant id that is a path must not be resolved against the shared base directory",
- importConfigurations.contains("in-bounds"));
+ assertFalse(importConfigurations.contains("in-bounds"),
+ "a tenant id that is a path must not be resolved against the shared base directory");
}
@Test
public void savingARecurrentImportWhoseSourceCarriesAPropertyPlaceholderIsRefused() {
assertRefused(() -> importEndpoint.saveConfiguration(
recurrentImport(fileUri(permittedImportDir, "/{{env:UNOMI_UNSET_FOR_TEST:../beta}}?fileName=profiles.csv"))));
- assertFalse("Camel expands a placeholder after validation, so the configuration cannot be stored",
- importConfigurations.contains("in-bounds"));
+ assertFalse(importConfigurations.contains("in-bounds"),
+ "Camel expands a placeholder after validation, so the configuration cannot be stored");
}
@Test
@@ -282,8 +288,8 @@ public class ConfigurationEndpointValidationTest {
importEndpoint.saveConfiguration(configuration);
- assertTrue("a oneshot import names no endpoint and must keep being stored",
- importConfigurations.contains("oneshot"));
+ assertTrue(importConfigurations.contains("oneshot"),
+ "a oneshot import names no endpoint and must keep being stored");
}
@Test
@@ -292,7 +298,8 @@ public class ConfigurationEndpointValidationTest {
recurrentExport(fileUri(permittedExportDir, "?fileName=profiles.csv")));
assertEquals("in-bounds", saved.getItemId());
- assertTrue("the configuration should have been stored", exportConfigurations.contains("in-bounds"));
+ assertTrue(exportConfigurations.contains("in-bounds"),
+ "the configuration should have been stored");
}
@Test
@@ -300,35 +307,123 @@ public class ConfigurationEndpointValidationTest {
ExportConfiguration configuration = recurrentExport(fileUri(arbitraryDir, "?fileName=profiles.csv"));
assertRefused(() -> exportEndpoint.saveConfiguration(configuration));
- assertFalse("a refused configuration must not be stored", exportConfigurations.contains("in-bounds"));
+ assertFalse(exportConfigurations.contains("in-bounds"),
+ "a refused configuration must not be stored");
+ }
+
+ @Test
+ public void savingARecurrentExportInAnotherTenantsDirectoryIsRefused() {
+ File otherTenant = new File(exportRoot, "beta");
+ assertTrue(otherTenant.mkdirs());
+
+ assertRefused(() -> exportEndpoint.saveConfiguration(
+ recurrentExport(fileUri(otherTenant, "?fileName=profiles.csv"))));
+ assertFalse(exportConfigurations.contains("in-bounds"), "a refused configuration must not be stored");
+ }
+
+ @Test
+ public void savingARecurrentExportIsJudgedAgainstTheExportDirectoriesNotTheImportOnes() {
+ assertRefused(() -> exportEndpoint.saveConfiguration(
+ recurrentExport(fileUri(permittedImportDir, "?fileName=profiles.csv"))));
+ assertFalse(exportConfigurations.contains("in-bounds"), "a refused configuration must not be stored");
+ }
+
+ @Test
+ public void savingARecurrentExportWithNoTenantIsRefused() {
+ ExportConfiguration configuration = recurrentExport(fileUri(permittedExportDir, "?fileName=profiles.csv"));
+ configuration.setTenantId(null);
+
+ assertRefused(() -> exportEndpoint.saveConfiguration(configuration));
+ assertFalse(exportConfigurations.contains("in-bounds"),
+ "a configuration with no tenant must not fall back to the shared directory");
+ }
+
+ @Test
+ public void savingARecurrentExportUsesTheCallerTenantWhenTheBodyNamesNone() {
+ callerIs(TENANT);
+ ExportConfiguration configuration = recurrentExport(fileUri(permittedExportDir, "?fileName=profiles.csv"));
+ configuration.setTenantId(null);
+
+ exportEndpoint.saveConfiguration(configuration);
+
+ assertTrue(exportConfigurations.contains("in-bounds"));
+ }
+
+ @Test
+ public void savingARecurrentExportIsJudgedAgainstTheCallerTenantWhateverTenantTheBodyNames() {
+ callerIs(TENANT);
+ File otherTenant = new File(exportRoot, "beta");
+ assertTrue(otherTenant.mkdirs());
+ ExportConfiguration configuration = recurrentExport(fileUri(otherTenant, "?fileName=profiles.csv"));
+ configuration.setTenantId("beta");
+
+ assertRefused(() -> exportEndpoint.saveConfiguration(configuration));
+ assertFalse(exportConfigurations.contains("in-bounds"),
+ "acme may not have a configuration judged against beta's directory by naming beta");
+ }
+
+ @Test
+ public void savingARecurrentExportAsTheSystemHonoursTheTenantTheBodyNames() {
+ callerIs(ExecutionContext.SYSTEM_TENANT);
+ File otherTenant = new File(exportRoot, "beta");
+ assertTrue(otherTenant.mkdirs());
+ ExportConfiguration configuration = recurrentExport(fileUri(otherTenant, "?fileName=profiles.csv"));
+ configuration.setTenantId("beta");
+
+ exportEndpoint.saveConfiguration(configuration);
+
+ assertTrue(exportConfigurations.contains("in-bounds"),
+ "the system may configure any tenant, judged against that tenant's directory");
+ }
+
+ @Test
+ public void savingARecurrentExportWhoseDestinationCarriesAPropertyPlaceholderIsRefused() {
+ assertRefused(() -> exportEndpoint.saveConfiguration(
+ recurrentExport(fileUri(permittedExportDir, "/{{env:UNOMI_UNSET_FOR_TEST:../beta}}?fileName=profiles.csv"))));
+ assertFalse(exportConfigurations.contains("in-bounds"),
+ "Camel expands a placeholder after validation, so the configuration cannot be stored");
+ }
+
+ @Test
+ public void savingARecurrentImportWhoseSchemeIsNotInTheCaseItIsAllowedInIsRefused() {
+ assertRefused(() -> importEndpoint.saveConfiguration(
+ recurrentImport("FILE://" + permittedImportDir.getAbsolutePath() + "?fileName=profiles.csv")));
+ assertFalse(importConfigurations.contains("in-bounds"),
+ "Camel has no component of that name, so the route of this configuration could never be built");
}
// ---------------------------------------------------------------------------------------------
// Fixtures
// ---------------------------------------------------------------------------------------------
+ private void assertUnavailable(Runnable save) {
+ WebApplicationException e = assertThrows(WebApplicationException.class, save::run,
+ "saving the configuration should not have been answered yet");
+ assertEquals(503, e.getResponse().getStatus(),
+ "settings that are not published yet make the service unavailable, not the configuration wrong");
+ }
+
/**
* A refused configuration answers {@code 400 Bad Request}, and says why: the caller has to be able
* to correct the endpoint from the answer alone.
*/
- private void assertUnavailable(Runnable save) {
- try {
- save.run();
- fail("saving the configuration should not have been answered yet");
- } catch (WebApplicationException e) {
- assertEquals("settings that are not published yet make the service unavailable, not the "
- + "configuration wrong", 503, e.getResponse().getStatus());
- }
+ private void assertRefused(Runnable save) {
+ WebApplicationException e = assertThrows(WebApplicationException.class, save::run,
+ "saving the configuration should have been refused");
+ assertEquals(400, e.getResponse().getStatus(), "a refused configuration is a bad request");
+ assertTrue(e.getMessage() != null && !e.getMessage().trim().isEmpty(), "the refusal must say why");
}
- private void assertRefused(Runnable save) {
- try {
- save.run();
- fail("saving the configuration should have been refused");
- } catch (WebApplicationException e) {
- assertEquals("a refused configuration is a bad request", 400, e.getResponse().getStatus());
- assertTrue("the refusal must say why", e.getMessage() != null && !e.getMessage().trim().isEmpty());
- }
+ private File newFolder(String name) {
+ File folder = new File(tmp, name);
+ assertTrue(folder.mkdirs());
+ return folder;
+ }
+
+ private void callerIs(String tenantId) {
+ ExecutionContextManager contexts = mock(ExecutionContextManager.class);
+ when(contexts.getCurrentContext()).thenReturn(new ExecutionContext(tenantId, null, null));
+ exportEndpoint.setExecutionContextManager(contexts);
}
private String fileUri(File directory, String suffix) {
diff --git a/itests/src/test/java/org/apache/unomi/itests/ProfileImportExportContainmentIT.java b/itests/src/test/java/org/apache/unomi/itests/ProfileImportExportContainmentIT.java
index 5d77a70c8..3c1f54a80 100644
--- a/itests/src/test/java/org/apache/unomi/itests/ProfileImportExportContainmentIT.java
+++ b/itests/src/test/java/org/apache/unomi/itests/ProfileImportExportContainmentIT.java
@@ -76,6 +76,15 @@ public class ProfileImportExportContainmentIT extends BaseIT {
exportConfigurationService.delete(createdExportConfigId);
createdExportConfigId = null;
}
+ // the suite shares one container: what a test put on its disk is there for the next one
+ File arbitraryDir = new File(ARBITRARY_DIR);
+ File[] leftOver = arbitraryDir.listFiles();
+ if (leftOver != null) {
+ for (File file : leftOver) {
+ file.delete();
+ }
+ }
+ arbitraryDir.delete();
}
// ---------------------------------------------------------------------------------------------