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 6a012cd67 UNOMI-973: Refuse Camel property placeholders in file
endpoints
6a012cd67 is described below
commit 6a012cd6708febb2b91b583b4adc1aff2d438b9e
Author: Serge Huber <[email protected]>
AuthorDate: Sun Sep 27 11:15:56 2026 +0200
UNOMI-973: Refuse Camel property placeholders in file endpoints
Camel expands {{...}} placeholders over the whole endpoint URI before it
parses the endpoint, and the built-in env: and sys: functions accept an
inline default that is used verbatim when the variable is not set. So
file:///{baseDir}/{tenantId}/{{env:UNSET:../other}} passed containment
(the first parent segment is fused to the token and never seen by the
path walk) while Camel opened {baseDir}/other. EndpointValidator now
refuses any endpoint that carries the placeholder token, whatever its
scheme, and the manual says so.
Review follow-ups in the same change:
- the untenanted recordEndpointOutcome overload is private, so a route
builder cannot write the mark outside the configuration's tenant, and
the status a mark overwrites is logged since it is not restored;
- each route build logs one summary line when configurations were
refused, so a mass refusal after an upgrade is visible without reading
every line;
- the 3.0-to-3.1 migration guide explains the new {baseDir}/{tenantId}
layout and what to do with existing configurations;
- Javadoc on the new public setters, and the RAW{...} and '+' notes in
EndpointValidator now describe Camel 2.23.1 accurately;
- tests for placeholders, $simple{...}, a tenant id that is a path, a
non-system caller naming another tenant, a system caller naming one,
and the mark being written as the configuration's own tenant.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
---
.../apache/unomi/router/api/EndpointValidator.java | 26 +++++++-
.../router/core/context/RouterCamelContext.java | 10 +++
.../route/ProfileExportCollectRouteBuilder.java | 12 ++++
.../route/ProfileImportFromSourceRouteBuilder.java | 7 +++
.../core/route/RouterAbstractRouteBuilder.java | 12 +++-
.../core/route/FileEndpointContainmentTest.java | 73 ++++++++++++++++++++++
.../core/route/RefusedConfigurationStatusTest.java | 58 +++++++++++++++++
.../rest/ExportConfigurationServiceEndPoint.java | 10 +++
.../rest/ConfigurationEndpointValidationTest.java | 53 ++++++++++++++++
.../asciidoc/migrations/migrate-3.0-to-3.1.adoc | 17 +++++
.../src/main/asciidoc/profile-import-export.adoc | 3 +-
11 files changed, 274 insertions(+), 7 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 04ef984e7..10ecf0cbb 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
@@ -52,6 +52,13 @@ import java.util.regex.Pattern;
* instead of being cut in two, and the File Language expressions a
path-bearing option may carry are
* accounted for rather than taken literally.
*
+ * <p>One thing Camel does to a URI cannot be accounted for. A property
placeholder ({@code {{...}}})
+ * is expanded over the whole URI before the endpoint is parsed, and the
built-in {@code env:} and
+ * {@code sys:} functions take an inline default that is used verbatim when
the variable is not set:
+ * {@code {{env:UNSET:../elsewhere}}} is a parent segment that no path walk
sees, since the
+ * {@code ..} is fused to the token it sits in. An endpoint that carries the
placeholder token is
+ * therefore refused, whatever its scheme.
+ *
* <p>A path the file system cannot make sense of, and a path whose existing
part cannot be resolved,
* are both refusals: nothing is thrown out of this class, because one
malformed endpoint must not
* cost a deployment the routes of every other configuration.
@@ -113,6 +120,12 @@ public final class EndpointValidator {
/** Stands in for a token that expands to a name: one path component,
never a parent segment. */
private static final String NAME_PLACEHOLDER = "_";
+ /**
+ * Opens a Camel property placeholder. Camel expands these over the whole
URI before it parses the
+ * endpoint, so a placeholder can rewrite any part of a URI after this
class has looked at it.
+ */
+ 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.
@@ -149,6 +162,10 @@ public final class EndpointValidator {
if (!containsIgnoreCase(split(allowedSchemes), scheme)) {
return "endpoint scheme '" + scheme + "' is not allowed";
}
+ if (endpointUri.contains(PROPERTY_PLACEHOLDER_TOKEN)) {
+ return "endpoint '" + endpointUri + "' carries a property
placeholder, which Camel expands "
+ + "before the endpoint is resolved, so it cannot be
validated";
+ }
try {
String refusal = validateLocalWorkDirectory(endpointUri,
permittedBaseDirs);
@@ -391,8 +408,9 @@ public final class EndpointValidator {
}
/**
- * {@code RAW(...)} and {@code RAW{...}} tell Camel not to decode a value;
the path it wraps is used
- * as it stands.
+ * {@code RAW(...)} tells Camel not to decode a value; the path it wraps
is used as it stands.
+ * {@code RAW{...}} is the same marker in later Camel versions; the
version in use does not know
+ * it, so honouring it here is stricter than Camel, never looser, and
stays right across an upgrade.
*/
private static String stripRaw(String value) {
if (value.startsWith("RAW(") && value.endsWith(")")) {
@@ -458,7 +476,9 @@ public final class EndpointValidator {
* Decodes the percent-encoding of a URI, so that containment is decided
on the path the file system
* will see. Unlike form decoding, {@code +} is left alone: it is a valid
character in a file name.
* Characters that are not escaped keep their own encoding, so a path that
mixes an escape with a
- * non-ASCII name is not corrupted into a different path.
+ * non-ASCII name is not corrupted into a different path. Camel
additionally folds {@code +} to a
+ * space in a value that is not {@code RAW()}; that turns one name into
another within the same
+ * directory, never into a parent segment, so it is not reproduced here.
*/
private static String decode(String value) {
if (value.indexOf('%') < 0) {
diff --git
a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/context/RouterCamelContext.java
b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/context/RouterCamelContext.java
index fcfebb6d8..5911de36b 100644
---
a/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/context/RouterCamelContext.java
+++
b/extensions/router/router-core/src/main/java/org/apache/unomi/router/core/context/RouterCamelContext.java
@@ -490,10 +490,20 @@ public class RouterCamelContext implements
IRouterCamelContext {
this.securityService = securityService;
}
+ /**
+ * Sets the comma-separated base directories an import {@code file}
endpoint may resolve into.
+ *
+ * @param permittedImportBaseDirs the permitted import base directories
+ */
public void setPermittedImportBaseDirs(String permittedImportBaseDirs) {
this.permittedImportBaseDirs = permittedImportBaseDirs;
}
+ /**
+ * Sets the comma-separated base directories an export {@code file}
endpoint may resolve into.
+ *
+ * @param permittedExportBaseDirs the permitted export base directories
+ */
public void setPermittedExportBaseDirs(String permittedExportBaseDirs) {
this.permittedExportBaseDirs = permittedExportBaseDirs;
}
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 27480bbea..f74cab573 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
@@ -101,6 +101,7 @@ public class ProfileExportCollectRouteBuilder extends
RouterAbstractRouteBuilder
collectProfileBean.setExecutionContextManager(executionContextManager);
//Loop on multiple export configuration
+ int refused = 0;
for (final ExportConfiguration exportConfiguration :
exportConfigurationList) {
if
(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT.equals(exportConfiguration.getConfigType())
&&
exportConfiguration.getProperties() != null &&
exportConfiguration.getProperties().size() > 0) {
@@ -130,6 +131,7 @@ public class ProfileExportCollectRouteBuilder extends
RouterAbstractRouteBuilder
prDef.to((String)
getEndpointURI(RouterConstants.DIRECTION_FROM,
RouterConstants.DIRECT_EXPORT_DEPOSIT_BUFFER));
}
} else {
+ refused++;
LOGGER.error("Destination endpoint is refused ({}),
route {} will be skipped.", refusal, exportConfiguration.getItemId());
}
} else {
@@ -139,6 +141,11 @@ 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);
+ }
}
/**
@@ -151,6 +158,11 @@ public class ProfileExportCollectRouteBuilder extends
RouterAbstractRouteBuilder
this.permittedBaseDirs = permittedExportBaseDirs;
}
+ /**
+ * Sets the service a refused configuration is recorded with.
+ *
+ * @param exportConfigurationService the export configuration service
+ */
public void
setExportConfigurationService(ImportExportConfigurationService<ExportConfiguration>
exportConfigurationService) {
this.exportConfigurationService = exportConfigurationService;
}
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 c31713256..688b43b59 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
@@ -119,6 +119,7 @@ public class ProfileImportFromSourceRouteBuilder extends
RouterAbstractRouteBuil
}
//Loop on multiple import configuration
+ int refused = 0;
for (final ImportConfiguration importConfiguration :
importConfigurationList) {
if
(RouterConstants.IMPORT_EXPORT_CONFIG_TYPE_RECURRENT.equals(importConfiguration.getConfigType())
&&
importConfiguration.getProperties() != null &&
importConfiguration.getProperties().size() > 0) {
@@ -183,10 +184,16 @@ public class ProfileImportFromSourceRouteBuilder extends
RouterAbstractRouteBuil
prDef.to((String)
getEndpointURI(RouterConstants.DIRECTION_FROM,
RouterConstants.DIRECT_IMPORT_DEPOSIT_BUFFER));
}
} else {
+ refused++;
LOGGER.error("Source endpoint is refused ({}), route {}
will be skipped.", refusal, importConfiguration.getItemId());
}
}
}
+ 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);
+ }
}
/**
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 125c83419..e84d3a6d7 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
@@ -119,7 +119,8 @@ public abstract class RouterAbstractRouteBuilder extends
RouteBuilder {
*
* <p>The other way round matters just as much: restoring the permitted
directories must bring the
* configuration back on its own, without anyone having to touch it. Only
the status this method
- * sets is cleared, so the record of a run that genuinely failed survives.
+ * sets is cleared, so the record of a run that genuinely failed survives.
The status the mark
+ * replaces is not restored afterwards, so it is logged when it is
overwritten.
*
* <p>The configuration is saved without asking for its running route to
be refreshed: the refresh
* would rebuild the route, refuse it again and save it again, without end.
@@ -128,9 +129,13 @@ public abstract class RouterAbstractRouteBuilder extends
RouteBuilder {
* @param service the service holding that kind of configuration
* @param refusal the reason the endpoint was refused, or {@code
null} if it can be honoured
*/
- protected <T extends ImportExportConfiguration> void recordEndpointOutcome(
+ private <T extends ImportExportConfiguration> void recordEndpointOutcome(
T configuration, ImportExportConfigurationService<T> service,
String refusal) {
if (refusal != null) {
+ if
(!RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT.equals(configuration.getStatus()))
{
+ LOGGER.warn("Configuration {} is marked {} (its status was
{}): {}", configuration.getItemId(),
+ RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT,
configuration.getStatus(), refusal);
+ }
configuration.setStatus(RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT);
saveQuietly(configuration, service);
} else if
(RouterConstants.CONFIG_STATUS_INVALID_ENDPOINT.equals(configuration.getStatus()))
{
@@ -163,7 +168,8 @@ public abstract class RouterAbstractRouteBuilder extends
RouteBuilder {
* <p>Startup builds every tenant's routes in one pass, and the
configuration service refuses a save
* whose current tenant is not the configuration's. Writing the mark from
the wrong tenant would
* either fail or store it in another tenant's index. When no context
manager is available the mark
- * is written as it stands, which is what the unit tests do.
+ * 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.
*/
protected <T extends ImportExportConfiguration> void recordEndpointOutcome(
T configuration, ImportExportConfigurationService<T> service,
String refusal,
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 cdc6a5ddc..2a71b7052 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
@@ -638,6 +638,79 @@ public class FileEndpointContainmentTest {
assertRouteBuilt("well-formed", "one malformed configuration must not
cost the deployment its other routes");
}
+ //
---------------------------------------------------------------------------------------------
+ // Property placeholders — expanded by Camel after validation, over the
whole URI
+ //
---------------------------------------------------------------------------------------------
+
+ @Test
+ public void
importRouteIsRefusedWhenSourceDirectoryCarriesAPropertyPlaceholder() throws
Exception {
+ // Camel expands {{env:NAME:default}} before it parses the endpoint,
and uses the default
+ // verbatim when NAME is not set: what reaches the file system is
acme/../beta. To a path walk
+ // the first parent segment is fused to the token and is not a parent
segment at all.
+ addImportRoutes(recurrentImport("placeholder-directory",
+ fileUri(permittedImportDir,
"/{{env:UNOMI_UNSET_FOR_TEST:../beta}}?fileName=profiles.csv")));
+
+ assertRouteRefused("placeholder-directory",
+ "a placeholder is expanded after validation, so nothing about
the path can be trusted");
+ }
+
+ @Test
+ public void
importRouteIsRefusedWhenAPathBearingOptionCarriesAPropertyPlaceholder() throws
Exception {
+ addImportRoutes(recurrentImport("placeholder-option",
+ fileUri(permittedImportDir,
"?fileName=profiles.csv&move={{sys:unomi.unset.for.test:../../beta}}")));
+
+ assertRouteRefused("placeholder-option", "the placeholder's default is
a parent segment the path walk never sees");
+ }
+
+ @Test
+ public void
importRouteIsRefusedWhenARemoteSourceCarriesAPropertyPlaceholder() throws
Exception {
+ // starts inside the tenant's directory, so that only the placeholder
takes it out of there
+ addImportRoutes(recurrentImport("placeholder-remote",
"sftp://sftp.example.com/profiles?fileName=profiles.csv";
+ + "&localWorkDirectory=" +
permittedImportDir.getAbsolutePath() +
"/{{env:UNOMI_UNSET_FOR_TEST:../../elsewhere}}"));
+
+ assertRouteRefused("placeholder-remote", "a placeholder can put the
staging directory anywhere, whatever the scheme");
+ }
+
+ @Test
+ public void
exportRouteIsRefusedWhenDestinationCarriesAPropertyPlaceholder() throws
Exception {
+ addExportRoutes(recurrentExport("placeholder-destination",
+ fileUri(permittedExportDir,
"?fileName={{env:UNOMI_UNSET_FOR_TEST:../../beta/profiles.csv}}")));
+
+ assertRouteRefused("placeholder-destination", "an export destination
is expanded by Camel the same way");
+ }
+
+ //
---------------------------------------------------------------------------------------------
+ // The other expression syntax, and a tenant id that is a path
+ //
---------------------------------------------------------------------------------------------
+
+ @Test
+ public void
importRouteIsRefusedWhenMoveOptionUsesTheAlternateExpressionSyntaxToLeavePermittedBaseDir()
throws Exception {
+ addImportRoutes(recurrentImport("simple-syntax",
+ fileUri(permittedImportDir,
"?fileName=profiles.csv&move=$simple{file:parent}/../../elsewhere")));
+
+ assertRouteRefused("simple-syntax", "$simple{...} is the same File
Language expression as ${...}");
+ }
+
+ @Test
+ public void
importRouteIsBuiltWhenMoveOptionUsesTheAlternateExpressionSyntaxAndStaysInside()
throws Exception {
+ addImportRoutes(recurrentImport("simple-syntax-inside",
+ fileUri(permittedImportDir,
"?fileName=profiles.csv&move=.done/$simple{file:name.noext}-$simple{date:now:yyyyMMdd}.csv")));
+
+ assertRouteBuilt("simple-syntax-inside", "name and date tokens
contribute one component each, whatever the syntax");
+ }
+
+ @Test
+ public void importRouteIsRefusedWhenTheTenantIdIsNotASingleDirectoryName()
throws Exception {
+ File otherTenant = new File(importRoot, "beta");
+ assertTrue(otherTenant.mkdirs());
+ ImportConfiguration configuration =
recurrentImport("traversing-tenant", fileUri(otherTenant,
"?fileName=profiles.csv"));
+ configuration.setTenantId("acme/../beta");
+
+ addImportRoutes(configuration);
+
+ assertRouteRefused("traversing-tenant", "a tenant id that is a path of
its own is not confined to a directory");
+ }
+
//
---------------------------------------------------------------------------------------------
// Fixtures
//
---------------------------------------------------------------------------------------------
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 e6a24b59c..2df5c1cf7 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
@@ -18,6 +18,7 @@ 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;
@@ -38,6 +39,7 @@ import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.function.Supplier;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -81,6 +83,9 @@ public class RefusedConfigurationStatusTest {
private RecordingConfigurationService<ImportConfiguration>
importConfigurations;
private RecordingConfigurationService<ExportConfiguration>
exportConfigurations;
+ /** The tenant the fake context manager is currently running as, or {@code
null} outside any. */
+ private String currentTenant;
+
@Before
public void setUp() throws Exception {
importRoot = tmp.newFolder("permitted-import");
@@ -126,6 +131,22 @@ public class RefusedConfigurationStatusTest {
exportConfigurations.contains("out-of-bounds"));
}
+ @Test
+ public void aRefusedConfigurationIsRecordedAsItsOwnTenant() throws
Exception {
+ // Start-up builds every tenant's routes in one pass; the store
refuses a save whose current
+ // tenant is not the configuration's, so the mark has to be written as
that tenant.
+ ImportConfiguration configuration =
recurrentImport(fileUri(arbitraryDir, "?fileName=profiles.csv"));
+ configuration.setTenantId("beta");
+ importConfigurations.currentTenant = () -> currentTenant;
+
+ addImportRoutes(recordingContextManager(), configuration);
+
+ assertTrue("the refusal should have been recorded",
importConfigurations.contains("out-of-bounds"));
+ assertEquals("the mark must be written as the configuration's tenant,
whatever the batch runs as",
+ "beta", importConfigurations.tenantAtLastSave);
+ assertNull("the batch's own context must be restored afterwards",
currentTenant);
+ }
+
@Test
public void
aStoreThatCannotRecordTheRefusalDoesNotCostTheBatchItsOtherRoutes() throws
Exception {
importConfigurations.unwritable = true;
@@ -252,6 +273,10 @@ public class RefusedConfigurationStatusTest {
}
private void addImportRoutes(ImportConfiguration... configurations) throws
Exception {
+ addImportRoutes(null, configurations);
+ }
+
+ private void addImportRoutes(ExecutionContextManager contextManager,
ImportConfiguration... configurations) throws Exception {
ProfileImportFromSourceRouteBuilder builder =
new ProfileImportFromSourceRouteBuilder(NO_KAFKA,
RouterConstants.CONFIG_TYPE_NOBROKER);
builder.setImportConfigurationList(java.util.Arrays.asList(configurations));
@@ -260,10 +285,37 @@ public class RefusedConfigurationStatusTest {
builder.setJacksonDataFormat(new
JacksonDataFormat(ProfileToImport.class));
builder.setAllowedEndpoints(DEFAULT_ALLOWED_ENDPOINTS);
builder.setPermittedImportBaseDirs(importRoot.getAbsolutePath());
+ builder.setExecutionContextManager(contextManager);
builder.setContext(camelContext);
camelContext.addRoutes(builder);
}
+ /**
+ * 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.
+ */
+ private ExecutionContextManager recordingContextManager() {
+ return (ExecutionContextManager) Proxy.newProxyInstance(
+ ExecutionContextManager.class.getClassLoader(),
+ new Class<?>[]{ExecutionContextManager.class},
+ (proxy, method, args) -> {
+ if (!"executeAsTenant".equals(method.getName())) {
+ return null;
+ }
+ String previous = currentTenant;
+ currentTenant = (String) args[0];
+ try {
+ if (args[1] instanceof Runnable) {
+ ((Runnable) args[1]).run();
+ return null;
+ }
+ return ((Supplier<?>) args[1]).get();
+ } finally {
+ currentTenant = previous;
+ }
+ });
+ }
+
private void addExportRoutes(ExportConfiguration... configurations) throws
Exception {
ProfileExportCollectRouteBuilder builder =
new ProfileExportCollectRouteBuilder(NO_KAFKA,
RouterConstants.CONFIG_TYPE_NOBROKER);
@@ -294,6 +346,11 @@ public class RefusedConfigurationStatusTest {
private boolean lastSaveAskedForARouteRefresh;
+ /** Where the store reads the current tenant from; the real one
refuses a save from another. */
+ private Supplier<String> currentTenant = () -> null;
+
+ private String tenantAtLastSave;
+
/** Stands in for a store that cannot be written to -- Elasticsearch
unreachable at start-up. */
private boolean unwritable;
@@ -317,6 +374,7 @@ public class RefusedConfigurationStatusTest {
throw new IllegalStateException("the store is unreachable");
}
lastSaveAskedForARouteRefresh = updateRunningRoute;
+ tenantAtLastSave = currentTenant.get();
stored.put(itemIdOf(configuration), configuration);
return configuration;
}
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 01e909fba..18326a720 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
@@ -73,11 +73,21 @@ public class ExportConfigurationServiceEndPoint extends
AbstractConfigurationSer
configurationService = exportConfigurationService;
}
+ /**
+ * Sets the service the router publishes its allowed schemes and base
directories through.
+ *
+ * @param configSharingService the configuration sharing service
+ */
@Reference
public void setConfigSharingService(ConfigSharingService
configSharingService) {
this.configSharingService = configSharingService;
}
+ /**
+ * Sets the manager that tells which tenant, or the system, is calling.
+ *
+ * @param executionContextManager the execution context manager
+ */
@Reference
public void setExecutionContextManager(ExecutionContextManager
executionContextManager) {
this.executionContextManager = executionContextManager;
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 812bbb1c2..bcdbc66d0 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
@@ -220,6 +220,59 @@ public class ConfigurationEndpointValidationTest {
assertTrue(importConfigurations.contains("in-bounds"));
}
+ @Test
+ public void
savingARecurrentImportIsJudgedAgainstTheCallerTenantWhateverTenantTheBodyNames()
{
+ ExecutionContextManager contexts = mock(ExecutionContextManager.class);
+ when(contexts.getCurrentContext()).thenReturn(new
ExecutionContext(TENANT, null, null));
+ importEndpoint.setExecutionContextManager(contexts);
+ File otherTenant = new File(importRoot, "beta");
+ assertTrue(otherTenant.mkdirs());
+
+ ImportConfiguration configuration =
recurrentImport(fileUri(otherTenant, "?fileName=profiles.csv"));
+ 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"));
+ }
+
+ @Test
+ public void
savingARecurrentImportAsTheSystemHonoursTheTenantTheBodyNames() {
+ ExecutionContextManager contexts = mock(ExecutionContextManager.class);
+ when(contexts.getCurrentContext()).thenReturn(new
ExecutionContext(ExecutionContext.SYSTEM_TENANT, null, null));
+ importEndpoint.setExecutionContextManager(contexts);
+ File otherTenant = new File(importRoot, "beta");
+ assertTrue(otherTenant.mkdirs());
+
+ ImportConfiguration configuration =
recurrentImport(fileUri(otherTenant, "?fileName=profiles.csv"));
+ configuration.setTenantId("beta");
+
+ importEndpoint.saveConfiguration(configuration);
+
+ assertTrue("the system may configure any tenant, judged against that
tenant's directory",
+ importConfigurations.contains("in-bounds"));
+ }
+
+ @Test
+ public void savingARecurrentImportWhoseTenantIdIsAPathIsRefused() {
+ File otherTenant = new File(importRoot, "beta");
+ assertTrue(otherTenant.mkdirs());
+ ImportConfiguration configuration =
recurrentImport(fileUri(otherTenant, "?fileName=profiles.csv"));
+ 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"));
+ }
+
+ @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"));
+ }
+
@Test
public void savingAOneshotImportThatCarriesNoSourceStoresIt() {
ImportConfiguration configuration = new ImportConfiguration();
diff --git a/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc
b/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc
index ad78c4149..5ab6d1d6d 100644
--- a/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc
+++ b/manual/src/main/asciidoc/migrations/migrate-3.0-to-3.1.adoc
@@ -276,6 +276,22 @@ When migrating to 3.1, you need to understand that:
- Each tenant operates in complete isolation with their own data space
- Tenant context must be established for every API operation
+==== Recurrent import and export files move under a tenant directory
+
+A recurrent import source or export destination that uses the `file` scheme
must now resolve inside the
+deployment's base directory for that direction
(`org.apache.unomi.router.config.import.baseDir` and
+`org.apache.unomi.router.config.export.baseDir`, defaulting to
`${karaf.data}/router/import/` and
+`${karaf.data}/router/export/`), under the subdirectory named after the
configuration's tenant
+(`{baseDir}/{tenantId}`). This applies to the endpoint directory, to its
path-bearing options (`fileName`,
+`move`, `moveFailed`, ...) and to `localWorkDirectory` on `ftp`, `sftp` and
`ftps` endpoints.
+
+Existing configurations are not migrated. After the upgrade, a configuration
whose endpoint resolves anywhere
+else builds no route and is marked `INVALID_ENDPOINT`; one error line is
logged per configuration, followed
+by a summary count at the end of each route build. To keep such a
configuration running, either move its
+files under `{baseDir}/{tenantId}` and update its `source` or `destination`
accordingly, or point the two
+base-directory properties at the directories already in use. The mark clears
on its own once the endpoint is
+accepted again. The "Profile import and export" chapter describes the rule in
full.
+
==== Migration Steps
1. **Understand Multi-Tenancy Impact**
@@ -394,6 +410,7 @@ Before starting the migration, please ensure that:
- You have a plan to update client applications to tenant API keys (or
temporary <<_v2_compatibility_mode,single-tenant compatibility mode>> only if
coming from 2.x)
- You have reviewed the <<_client_facing_hardening_3_1,client-facing
hardening>> notes (cookie-only public profile binding, HttpOnly default,
required passwords, privileged REST roles)
- You know how to obtain plaintext API keys after upgrade (regenerate via
`/cxs/tenants/{id}/apikeys`; create responses expose masked keys only)
+- Your recurrent import and export `file` endpoints resolve under
`{baseDir}/{tenantId}`, or the router's base-directory properties point at the
directories already in use (see above)
=== Migration Process
diff --git a/manual/src/main/asciidoc/profile-import-export.adoc
b/manual/src/main/asciidoc/profile-import-export.adoc
index d97f2f7a1..cdb92cb89 100644
--- a/manual/src/main/asciidoc/profile-import-export.adoc
+++ b/manual/src/main/asciidoc/profile-import-export.adoc
@@ -242,7 +242,8 @@ scheme is refused unless it resolves inside one of these
base directories, under
tenant (`{baseDir}/{tenantId}`), at any depth, and so is any of its options
that names a path (`fileName`, `move`,
`moveFailed`, ...). The same applies to `localWorkDirectory`, whatever the
scheme, since that one names a local
directory even on a remote endpoint. Each property accepts a comma-separated
list. A tenant cannot point a file
-endpoint at another tenant's directory.
+endpoint at another tenant's directory. An endpoint that carries a Camel
property placeholder (`{{...}}`) is refused
+whatever its scheme, since Camel expands it after the endpoint has been
validated.
#Base directories a file endpoint may resolve into
org.apache.unomi.router.config.import.baseDir=${karaf.data}/router/import/
