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 <[email protected]>
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 <[email protected]>
---
.../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();
}
//
---------------------------------------------------------------------------------------------
