This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch UNOMI-972-credentials-profile-binding-privileged-rest in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 899ef37b1e23a306932618b0a18d113d7f2b8232 Author: Serge Huber <[email protected]> AuthorDate: Mon Aug 10 09:24:58 2026 +0200 UNOMI-972: require an explicit admin and health-check password at startup Reported issue 1. users.properties resolved the shipped karaf and health accounts via ${...:-karaf} / ${...:-health}, so a deployment that set nothing authenticated with a known password. Removing the fallback alone is not enough: an unset property expands to the empty string, which Karaf's PropertiesLoginModule still accepts, so the accounts would simply have accepted an empty password instead. bin/setenv and the Docker entrypoint therefore refuse to start without the passwords, and AuthenticationFilter rejects a blank Basic credential wherever one is consumed - the launchers cannot cover every way the JVM is started (notably karaf.bat, whose inability to halt startup is documented in setenv itself). Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- clear-elasticsearch.sh | 2 + clear-opensearch.sh | 2 + docker/README.md | 18 +- docker/src/main/docker/docker-compose-build-es.yml | 2 + docker/src/main/docker/docker-compose-build-os.yml | 2 + docker/src/main/docker/docker-compose-cluster.yml | 4 + docker/src/main/docker/docker-compose-es.yml | 3 + docker/src/main/docker/docker-compose-os.yml | 3 + docker/src/main/docker/entrypoint.sh | 41 ++ .../test/java/org/apache/unomi/itests/BaseIT.java | 6 + .../test/java/org/apache/unomi/itests/BasicIT.java | 29 +- .../org/apache/unomi/itests/HealthCheckIT.java | 11 +- .../java/org/apache/unomi/itests/TenantIT.java | 8 +- .../apache/unomi/itests/V2CompatibilityModeIT.java | 4 +- .../apache/unomi/itests/graphql/BaseGraphQLIT.java | 2 +- itests/src/test/resources/etc/users.properties | 4 +- package/src/main/resources/bin/setenv | 88 +++++ package/src/main/resources/bin/setenv.bat | 61 +++ .../main/resources/etc/custom.system.properties | 11 +- package/src/main/resources/etc/users.properties | 6 +- .../rest/authentication/AuthenticationFilter.java | 76 +++- .../AuthenticationFilterBlankPasswordTest.java | 243 ++++++++++++ .../config/ShippedAdminPasswordConfigTest.java | 428 +++++++++++++++++++++ setup-elasticsearch.sh | 22 +- setup-opensearch.sh | 26 +- setup-utils.sh | 27 ++ 26 files changed, 1077 insertions(+), 52 deletions(-) diff --git a/clear-elasticsearch.sh b/clear-elasticsearch.sh index 0d92e9da2..baa760647 100755 --- a/clear-elasticsearch.sh +++ b/clear-elasticsearch.sh @@ -57,6 +57,8 @@ unset UNOMI_ELASTICSEARCH_SSL_ENABLE unset UNOMI_ELASTICSEARCH_SSL_TRUST_ALL_CERTIFICATES # Also set by setup-elasticsearch.sh / setup-opensearch.sh unset UNOMI_DISTRIBUTION +unset UNOMI_ROOT_PASSWORD +unset UNOMI_HEALTHCHECK_PASSWORD unset _IS_SOURCED diff --git a/clear-opensearch.sh b/clear-opensearch.sh index 61b6c5a12..df4c9425b 100755 --- a/clear-opensearch.sh +++ b/clear-opensearch.sh @@ -58,6 +58,8 @@ unset UNOMI_OPENSEARCH_SSL_ENABLE unset UNOMI_OPENSEARCH_SSL_TRUST_ALL_CERTIFICATES # Also set by setup-opensearch.sh / setup-elasticsearch.sh unset UNOMI_DISTRIBUTION +unset UNOMI_ROOT_PASSWORD +unset UNOMI_HEALTHCHECK_PASSWORD unset _IS_SOURCED diff --git a/docker/README.md b/docker/README.md index c5ab6553d..a5dc49bde 100644 --- a/docker/README.md +++ b/docker/README.md @@ -30,9 +30,13 @@ required Unomi tarball. ## Launching docker-compose using Maven project -Unomi requires a search engine (ElasticSearch or OpenSearch) so it is recommended to run Unomi and the search engine using docker-compose: +Unomi requires a search engine (ElasticSearch or OpenSearch) so it is recommended to run Unomi and the search engine using docker-compose. + +Set admin and health passwords first (required; no known defaults are shipped): ``` +export UNOMI_ROOT_PASSWORD='choose-a-strong-password' +export UNOMI_HEALTHCHECK_PASSWORD='choose-a-strong-health-password' mvn docker:start ``` @@ -72,6 +76,8 @@ For Unomi (with ElasticSearch): ```bash docker pull apache/unomi:3.1.0-SNAPSHOT docker run -d --name unomi --net unomi -p 8181:8181 -p 9443:9443 -p 8102:8102 \ + -e UNOMI_ROOT_PASSWORD='choose-a-strong-password' \ + -e UNOMI_HEALTHCHECK_PASSWORD='choose-a-strong-health-password' \ -e UNOMI_ELASTICSEARCH_ADDRESSES=elasticsearch:9200 \ apache/unomi:3.1.0-SNAPSHOT ``` @@ -81,6 +87,8 @@ For Unomi (with OpenSearch): ```bash docker pull apache/unomi:3.1.0-SNAPSHOT docker run -d --name unomi --net unomi -p 8181:8181 -p 9443:9443 -p 8102:8102 \ + -e UNOMI_ROOT_PASSWORD='choose-a-strong-password' \ + -e UNOMI_HEALTHCHECK_PASSWORD='choose-a-strong-health-password' \ -e UNOMI_DISTRIBUTION=unomi-distribution-opensearch \ -e UNOMI_OPENSEARCH_ADDRESSES=opensearch:9200 \ -e UNOMI_OPENSEARCH_PASSWORD=${OPENSEARCH_ADMIN_PASSWORD} \ @@ -93,6 +101,8 @@ For ElasticSearch: ```bash docker run -d --name unomi -p 8181:8181 -p 9443:9443 -p 8102:8102 \ + -e UNOMI_ROOT_PASSWORD='choose-a-strong-password' \ + -e UNOMI_HEALTHCHECK_PASSWORD='choose-a-strong-health-password' \ -e UNOMI_ELASTICSEARCH_ADDRESSES=host.docker.internal:9200 \ apache/unomi:3.1.0-SNAPSHOT ``` @@ -101,6 +111,8 @@ For OpenSearch: ```bash docker run -d --name unomi -p 8181:8181 -p 9443:9443 -p 8102:8102 \ + -e UNOMI_ROOT_PASSWORD='choose-a-strong-password' \ + -e UNOMI_HEALTHCHECK_PASSWORD='choose-a-strong-health-password' \ -e UNOMI_DISTRIBUTION=unomi-distribution-opensearch \ -e UNOMI_OPENSEARCH_ADDRESSES=host.docker.internal:9200 \ -e UNOMI_OPENSEARCH_PASSWORD=${OPENSEARCH_ADMIN_PASSWORD} \ @@ -112,6 +124,8 @@ Note: Linux doesn't support the host.docker.internal DNS lookup method yet, it s ## Environment Variables ### Common Variables +- `UNOMI_ROOT_PASSWORD`: Required admin (`karaf`) password — no known default +- `UNOMI_HEALTHCHECK_PASSWORD`: Required health-check (`health`) password — no known default - `UNOMI_AUTO_START`: Boolean to specify if unomi auto start with karaf (defaults to `true`) - `UNOMI_DISTRIBUTION`: Specifies the Unomi Distribution Feature to use (`unomi-distribution-elasticsearch` or `unomi-distribution-opensearch`, defaults to `unomi-distribution-elasticsearch`) @@ -133,7 +147,7 @@ Multi-tenancy requires a tenant before client endpoints such as `/cxs/context.js ```bash curl -X POST http://localhost:8181/cxs/tenants \ - --user karaf:karaf \ + --user "karaf:${UNOMI_ROOT_PASSWORD}" \ -H "Content-Type: application/json" \ -d '{"requestedId":"default","properties":{"name":"Default Tenant"}}' ``` diff --git a/docker/src/main/docker/docker-compose-build-es.yml b/docker/src/main/docker/docker-compose-build-es.yml index 03f265a7b..34193ad30 100644 --- a/docker/src/main/docker/docker-compose-build-es.yml +++ b/docker/src/main/docker/docker-compose-build-es.yml @@ -39,6 +39,8 @@ services: - UNOMI_AUTO_START=true - UNOMI_DISTRIBUTION=unomi-distribution-elasticsearch - UNOMI_ELASTICSEARCH_ADDRESSES=elasticsearch:9200 + - UNOMI_ROOT_PASSWORD=${UNOMI_ROOT_PASSWORD:?Set UNOMI_ROOT_PASSWORD} + - UNOMI_HEALTHCHECK_PASSWORD=${UNOMI_HEALTHCHECK_PASSWORD:?Set UNOMI_HEALTHCHECK_PASSWORD} # Debug settings - KARAF_DEBUG=${DEBUG:-false} - KARAF_DEBUG_PORT=${DEBUG_PORT:-5005} diff --git a/docker/src/main/docker/docker-compose-build-os.yml b/docker/src/main/docker/docker-compose-build-os.yml index 97d38cfe4..f5c8592d5 100644 --- a/docker/src/main/docker/docker-compose-build-os.yml +++ b/docker/src/main/docker/docker-compose-build-os.yml @@ -100,6 +100,8 @@ services: - UNOMI_OPENSEARCH_ADDRESSES=opensearch-node1:9200 - UNOMI_OPENSEARCH_USERNAME=admin - UNOMI_OPENSEARCH_PASSWORD=${OPENSEARCH_INITIAL_ADMIN_PASSWORD} + - UNOMI_ROOT_PASSWORD=${UNOMI_ROOT_PASSWORD:?Set UNOMI_ROOT_PASSWORD} + - UNOMI_HEALTHCHECK_PASSWORD=${UNOMI_HEALTHCHECK_PASSWORD:?Set UNOMI_HEALTHCHECK_PASSWORD} # Debug settings - KARAF_DEBUG=${DEBUG:-false} - KARAF_DEBUG_PORT=${DEBUG_PORT:-5005} diff --git a/docker/src/main/docker/docker-compose-cluster.yml b/docker/src/main/docker/docker-compose-cluster.yml index 75aed4628..fa1a3069e 100644 --- a/docker/src/main/docker/docker-compose-cluster.yml +++ b/docker/src/main/docker/docker-compose-cluster.yml @@ -36,6 +36,8 @@ services: environment: - UNOMI_ELASTICSEARCH_ADDRESSES=elasticsearch:9200 - UNOMI_CLUSTER_NODEID=unomi-3-node-1 + - UNOMI_ROOT_PASSWORD=${UNOMI_ROOT_PASSWORD:?Set UNOMI_ROOT_PASSWORD} + - UNOMI_HEALTHCHECK_PASSWORD=${UNOMI_HEALTHCHECK_PASSWORD:?Set UNOMI_HEALTHCHECK_PASSWORD} ports: - 8181:8181 - 9443:9443 @@ -58,6 +60,8 @@ services: environment: - UNOMI_ELASTICSEARCH_ADDRESSES=elasticsearch:9200 - UNOMI_CLUSTER_NODEID=unomi-3-node-2 + - UNOMI_ROOT_PASSWORD=${UNOMI_ROOT_PASSWORD:?Set UNOMI_ROOT_PASSWORD} + - UNOMI_HEALTHCHECK_PASSWORD=${UNOMI_HEALTHCHECK_PASSWORD:?Set UNOMI_HEALTHCHECK_PASSWORD} ports: - 8182:8181 - 9444:9443 diff --git a/docker/src/main/docker/docker-compose-es.yml b/docker/src/main/docker/docker-compose-es.yml index edfada591..a3d294672 100644 --- a/docker/src/main/docker/docker-compose-es.yml +++ b/docker/src/main/docker/docker-compose-es.yml @@ -45,6 +45,9 @@ services: - UNOMI_AUTO_START=true - UNOMI_DISTRIBUTION=unomi-distribution-elasticsearch - UNOMI_ELASTICSEARCH_ADDRESSES=elasticsearch:9200 + # Required admin password (no known default is shipped). Override via .env / shell. + - UNOMI_ROOT_PASSWORD=${UNOMI_ROOT_PASSWORD:?Set UNOMI_ROOT_PASSWORD} + - UNOMI_HEALTHCHECK_PASSWORD=${UNOMI_HEALTHCHECK_PASSWORD:?Set UNOMI_HEALTHCHECK_PASSWORD} # Debug settings - KARAF_DEBUG=${DEBUG:-false} - KARAF_DEBUG_PORT=${DEBUG_PORT:-5005} diff --git a/docker/src/main/docker/docker-compose-os.yml b/docker/src/main/docker/docker-compose-os.yml index 579d3e2bc..a37babf5b 100644 --- a/docker/src/main/docker/docker-compose-os.yml +++ b/docker/src/main/docker/docker-compose-os.yml @@ -84,6 +84,9 @@ services: - UNOMI_OPENSEARCH_ADDRESSES=opensearch-node1:9200 - UNOMI_OPENSEARCH_USERNAME=admin - UNOMI_OPENSEARCH_PASSWORD=${OPENSEARCH_INITIAL_ADMIN_PASSWORD} + # Required admin password (no known default is shipped). Override via .env / shell. + - UNOMI_ROOT_PASSWORD=${UNOMI_ROOT_PASSWORD:?Set UNOMI_ROOT_PASSWORD} + - UNOMI_HEALTHCHECK_PASSWORD=${UNOMI_HEALTHCHECK_PASSWORD:?Set UNOMI_HEALTHCHECK_PASSWORD} # Debug settings - KARAF_DEBUG=${DEBUG:-false} - KARAF_DEBUG_PORT=${DEBUG_PORT:-5005} diff --git a/docker/src/main/docker/entrypoint.sh b/docker/src/main/docker/entrypoint.sh index 1cfc68456..e805537b4 100755 --- a/docker/src/main/docker/entrypoint.sh +++ b/docker/src/main/docker/entrypoint.sh @@ -34,6 +34,47 @@ export KARAF_OPTS="-Dunomi.autoStart=${UNOMI_AUTO_START} -Dunomi.distribution=${ echo "KARAF_OPTS: $KARAF_OPTS" +# Refuse to start without admin/health passwords. An unset password is not "no account": it +# expands to the empty string, which Karaf's PropertiesLoginModule accepts as a valid password. +# This is the gate for container launches: exiting here means the container fails to start rather +# than booting with an administrator account that accepts an empty password. +check_required_password() { + # $1 env var name, $2 property name, $3 skip flag name + eval _value=\"\${$1}\" + eval _skip=\"\${$3}\" + + [ -n "${_value}" ] && return 0 + + if [ "${_skip}" = "true" ]; then + cat >&2 <<EOF + +WARNING: $3=true but $1 is empty. + Unless $2 is supplied another way, the + account will have an EMPTY password that grants full administrator access. + +EOF + return 0 + fi + + cat >&2 <<EOF +ERROR: $1 is not set. + +Apache Unomi does not ship a known default password, and an unset value becomes an EMPTY +password that still authenticates. Pass it when starting the container, for example: + + docker run -e UNOMI_ROOT_PASSWORD='choose-a-strong-password' \\ + -e UNOMI_HEALTHCHECK_PASSWORD='choose-a-strong-health-password' ... + +Or with docker compose, export both UNOMI_ROOT_PASSWORD and UNOMI_HEALTHCHECK_PASSWORD first. +EOF + return 1 +} + +check_required_password UNOMI_ROOT_PASSWORD \ + org.apache.unomi.security.root.password UNOMI_SKIP_ROOT_PASSWORD_CHECK || exit 1 +check_required_password UNOMI_HEALTHCHECK_PASSWORD \ + org.apache.unomi.healthcheck.password UNOMI_SKIP_HEALTHCHECK_PASSWORD_CHECK || exit 1 + # Function to check cluster health for a specific node check_node_health() { local node_url="$1" diff --git a/itests/src/test/java/org/apache/unomi/itests/BaseIT.java b/itests/src/test/java/org/apache/unomi/itests/BaseIT.java index 5ec6385ba..1940d3c79 100644 --- a/itests/src/test/java/org/apache/unomi/itests/BaseIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/BaseIT.java @@ -132,7 +132,10 @@ public abstract class BaseIT extends KarafTestSupport { protected static final ContentType JSON_CONTENT_TYPE = ContentType.create("application/json"); protected static final String BASE_URL = "http://localhost";; protected static final String BASIC_AUTH_USER_NAME = "karaf"; + /** Explicit IT password — package no longer ships a known default ({@code UNOMI_ROOT_PASSWORD}). */ protected static final String BASIC_AUTH_PASSWORD = "karaf"; + protected static final String HEALTHCHECK_AUTH_USER_NAME = "health"; + protected static final String HEALTHCHECK_AUTH_PASSWORD = "health"; protected static final int REQUEST_TIMEOUT = 60000; protected static final int DEFAULT_TRYING_TIMEOUT = 1000; protected static final int DEFAULT_TRYING_TRIES = 10; @@ -671,6 +674,9 @@ public abstract class BaseIT extends KarafTestSupport { editConfigurationFilePut("etc/system.properties", SEARCH_ENGINE_PROPERTY, searchEngine), editConfigurationFilePut("etc/system.properties", PERSISTENCE_PROVIDER_PROPERTY, searchEngine), editConfigurationFilePut("etc/custom.system.properties", "org.apache.unomi.migration.tenant.id", TEST_TENANT_ID), + // Explicit test credentials (package no longer ships a known default password). + editConfigurationFilePut("etc/custom.system.properties", "org.apache.unomi.security.root.password", BASIC_AUTH_PASSWORD), + editConfigurationFilePut("etc/custom.system.properties", "org.apache.unomi.healthcheck.password", HEALTHCHECK_AUTH_PASSWORD), // Default scheduler.thread.poolSize (5) is sized for the near-instant in-memory unit-test // double, not a real ES/OS backend. Under real refresh/write latency, the checker, task // executions, and lease-renewal heartbeats (see scheduler.adoc) compete for the same small diff --git a/itests/src/test/java/org/apache/unomi/itests/BasicIT.java b/itests/src/test/java/org/apache/unomi/itests/BasicIT.java index bd650359b..ff4e12f06 100644 --- a/itests/src/test/java/org/apache/unomi/itests/BasicIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/BasicIT.java @@ -38,6 +38,8 @@ import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.ops4j.pax.exam.junit.PaxExam; + +import java.util.Base64; import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy; import org.ops4j.pax.exam.spi.reactors.PerSuite; import org.slf4j.Logger; @@ -248,15 +250,36 @@ public class BasicIT extends BaseIT { loginEventPropertiesVisitor2.put(LAST_NAME, LAST_NAME_VISITOR_2); loginEventPropertiesVisitor2.put(EMAIL, EMAIL_VISITOR_2); - // Create login event with VISITOR_2 ContextRequest contextRequestLoginVisitor2 = getContextRequestWithLoginEvent(sourceSite, loginEventPropertiesVisitor2, EMAIL_VISITOR_2, SESSION_ID_4); + + // Public API key must not switch identity / merge into another profile on a shared cookie. + HttpPost publicSwitchAttempt = new HttpPost(getFullUrl("/cxs/context.json")); + publicSwitchAttempt.addHeader("Cookie", requestResponsePageView1.getCookieHeaderValue()); + publicSwitchAttempt.addHeader("X-Unomi-Api-Key", testPublicKeyValue); + publicSwitchAttempt.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequestLoginVisitor2), + ContentType.create("application/json"))); + TestUtils.RequestResponse publicSwitchResponse = executeContextJSONRequest(publicSwitchAttempt, SESSION_ID_4); + Assert.assertEquals("Public login must not switch away from the cookie profile", + profileIdVisitor1, publicSwitchResponse.getContextResponse().getProfileId()); + + // Public login still runs copyProperties on the cookie profile; restore visitor1 before the trusted switch. + Profile restoredVisitor1 = profileService.load(profileIdVisitor1); + restoredVisitor1.setProperty(FIRST_NAME, FIRST_NAME_VISITOR_1); + restoredVisitor1.setProperty(LAST_NAME, LAST_NAME_VISITOR_1); + restoredVisitor1.setProperty(EMAIL, EMAIL_VISITOR_1); + profileService.save(restoredVisitor1); + keepTrying("Visitor1 properties not restored", () -> profileService.load(profileIdVisitor1), + p -> FIRST_NAME_VISITOR_1.equals(p.getProperty(FIRST_NAME)), DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + + // Trusted private key may switch the browsing profile to VISITOR_2. HttpPost requestLoginVisitor2 = new HttpPost(getFullUrl("/cxs/context.json")); requestLoginVisitor2.addHeader("Cookie", requestResponsePageView1.getCookieHeaderValue()); - requestLoginVisitor2.addHeader("X-Unomi-Api-Key", testPublicKeyValue); + requestLoginVisitor2.setHeader("Authorization", "Basic " + Base64.getEncoder().encodeToString( + (TEST_TENANT_ID + ":" + testPrivateKeyValue).getBytes())); requestLoginVisitor2.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequestLoginVisitor2), ContentType.create("application/json"))); - TestUtils.RequestResponse requestResponseLoginVisitor2 = executeContextJSONRequest(requestLoginVisitor2, SESSION_ID_4); + TestUtils.RequestResponse requestResponseLoginVisitor2 = executeContextJSONRequest(requestLoginVisitor2, SESSION_ID_4, -1, false); // We should have a new profile id so the session should have been moved from VISITOR_1 to VISITOR_2 String profileIdVisitor2 = requestResponseLoginVisitor2.getContextResponse().getProfileId(); Assert.assertNotEquals("Context profile id should not be the same", profileIdVisitor1, diff --git a/itests/src/test/java/org/apache/unomi/itests/HealthCheckIT.java b/itests/src/test/java/org/apache/unomi/itests/HealthCheckIT.java index 305c2f38e..f7179a7f4 100644 --- a/itests/src/test/java/org/apache/unomi/itests/HealthCheckIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/HealthCheckIT.java @@ -54,8 +54,6 @@ public class HealthCheckIT extends BaseIT { private final static Logger LOGGER = LoggerFactory.getLogger(HealthCheckIT.class); - protected static final String HEALTHCHECK_AUTH_USER_NAME = "health"; - protected static final String HEALTHCHECK_AUTH_PASSWORD = "health"; protected static final String HEALTHCHECK_ENDPOINT = "/health/check"; @Test @@ -132,6 +130,15 @@ public class HealthCheckIT extends BaseIT { } } + @Test + public void testHealthCheck_wrongPasswordRejected() throws Exception { + final HttpGet httpGet = new HttpGet(getFullUrl(HEALTHCHECK_ENDPOINT)); + try (CloseableHttpResponse response = executeHttpRequest( + httpGet, AuthType.CUSTOM_BASIC, HEALTHCHECK_AUTH_USER_NAME, "wrong-password")) { + Assert.assertEquals(401, response.getStatusLine().getStatusCode()); + } + } + protected <T> T get(final String url, TypeReference<T> typeReference) { CloseableHttpResponse response = null; try { diff --git a/itests/src/test/java/org/apache/unomi/itests/TenantIT.java b/itests/src/test/java/org/apache/unomi/itests/TenantIT.java index a9ef9a595..9cce84707 100644 --- a/itests/src/test/java/org/apache/unomi/itests/TenantIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/TenantIT.java @@ -190,7 +190,7 @@ public class TenantIT extends BaseIT { // Create test tenant for API key tests BasicCredentialsProvider adminCredsProvider = new BasicCredentialsProvider(); - adminCredsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials("karaf", "karaf")); + adminCredsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(BASIC_AUTH_USER_NAME, BASIC_AUTH_PASSWORD)); try (CloseableHttpClient adminClient = HttpClients.custom().setDefaultCredentialsProvider(adminCredsProvider).build()) { Map<String, Object> requestBody = new HashMap<>(); @@ -273,7 +273,7 @@ public class TenantIT extends BaseIT { // Test with JAAS auth (should succeed) — use a fresh request to avoid carrying X-Unomi-Api-Key from previous step BasicCredentialsProvider adminCredsProvider = new BasicCredentialsProvider(); - adminCredsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials("karaf", "karaf")); + adminCredsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(BASIC_AUTH_USER_NAME, BASIC_AUTH_PASSWORD)); try (CloseableHttpClient adminClient = HttpClients.custom().setDefaultCredentialsProvider(adminCredsProvider).build(); CloseableHttpResponse response = adminClient.execute(new HttpGet(getFullUrl("/context.json?sessionId=" + sessionId)))) { Assert.assertEquals("JAAS auth should grant access to public endpoints", 200, response.getStatusLine().getStatusCode()); @@ -325,7 +325,7 @@ public class TenantIT extends BaseIT { // Test with JAAS auth (should succeed) — use a fresh request to avoid carrying Authorization from previous step BasicCredentialsProvider adminCredsProvider = new BasicCredentialsProvider(); - adminCredsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials("karaf", "karaf")); + adminCredsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(BASIC_AUTH_USER_NAME, BASIC_AUTH_PASSWORD)); try (CloseableHttpClient adminClient = HttpClients.custom().setDefaultCredentialsProvider(adminCredsProvider).build(); CloseableHttpResponse response = adminClient.execute(new HttpGet(getFullUrl("/cxs/profiles/count")))) { Assert.assertEquals("JAAS auth should grant access to private endpoints", 200, response.getStatusLine().getStatusCode()); @@ -378,7 +378,7 @@ public class TenantIT extends BaseIT { // Test with JAAS authentication (should succeed) getRequest = new HttpGet(getFullUrl("/cxs/profiles/count")); - getRequest.setHeader("Authorization", "Basic " + Base64.getEncoder().encodeToString(("karaf:karaf").getBytes())); + getRequest.setHeader("Authorization", "Basic " + Base64.getEncoder().encodeToString((BASIC_AUTH_USER_NAME + ":" + BASIC_AUTH_PASSWORD).getBytes())); try (CloseableHttpResponse response = executeHttpRequest(getRequest, AuthType.JAAS_ADMIN)) { Assert.assertEquals("JAAS authentication should grant access to private endpoints", 200, response.getStatusLine().getStatusCode()); } diff --git a/itests/src/test/java/org/apache/unomi/itests/V2CompatibilityModeIT.java b/itests/src/test/java/org/apache/unomi/itests/V2CompatibilityModeIT.java index cce6c5484..f27089368 100644 --- a/itests/src/test/java/org/apache/unomi/itests/V2CompatibilityModeIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/V2CompatibilityModeIT.java @@ -225,7 +225,7 @@ public class V2CompatibilityModeIT extends BaseIT { request.setEntity(new StringEntity(getObjectMapper().writeValueAsString(contextRequest), ContentType.APPLICATION_JSON)); BasicCredentialsProvider credsProvider = new BasicCredentialsProvider(); - credsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials("karaf", "karaf")); + credsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(BASIC_AUTH_USER_NAME, BASIC_AUTH_PASSWORD)); RequestConfig requestConfig = RequestConfig.custom() .setAuthenticationEnabled(true) @@ -283,7 +283,7 @@ public class V2CompatibilityModeIT extends BaseIT { HttpGet getRequest = new HttpGet(getFullUrl("/cxs/profiles/" + TEST_PROFILE_ID)); BasicCredentialsProvider credsProvider = new BasicCredentialsProvider(); - credsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials("karaf", "karaf")); + credsProvider.setCredentials(AuthScope.ANY, new UsernamePasswordCredentials(BASIC_AUTH_USER_NAME, BASIC_AUTH_PASSWORD)); RequestConfig requestConfig = RequestConfig.custom() .setAuthenticationEnabled(true) diff --git a/itests/src/test/java/org/apache/unomi/itests/graphql/BaseGraphQLIT.java b/itests/src/test/java/org/apache/unomi/itests/graphql/BaseGraphQLIT.java index 1ff220109..34db0381e 100644 --- a/itests/src/test/java/org/apache/unomi/itests/graphql/BaseGraphQLIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/graphql/BaseGraphQLIT.java @@ -72,7 +72,7 @@ public abstract class BaseGraphQLIT extends BaseIT { } /** - * Performs a GraphQL POST request with JAAS admin authentication (karaf:karaf). + * Performs a GraphQL POST request with JAAS admin authentication ({@link #BASIC_AUTH_USER_NAME}/{@link #BASIC_AUTH_PASSWORD}). * This is equivalent to AuthType.JAAS_ADMIN. * * @param resource The resource path to the GraphQL query/mutation diff --git a/itests/src/test/resources/etc/users.properties b/itests/src/test/resources/etc/users.properties index 377fe6a16..538906c38 100644 --- a/itests/src/test/resources/etc/users.properties +++ b/itests/src/test/resources/etc/users.properties @@ -29,6 +29,6 @@ # and modifiable via the JAAS command group. These users reside in a JAAS domain # with the name "karaf". # -karaf = ${org.apache.unomi.security.root.password:-karaf},_g_:admingroup -health = ${org.apache.unomi.healthcheck.password:-health},health +karaf = ${org.apache.unomi.security.root.password},_g_:admingroup +health = ${org.apache.unomi.healthcheck.password},health _g_\:admingroup = group,admin,manager,viewer,systembundles,ssh,ROLE_UNOMI_ADMIN,ROLE_UNOMI_TENANT_USER,ROLE_UNOMI_TENANT_ADMIN diff --git a/package/src/main/resources/bin/setenv b/package/src/main/resources/bin/setenv index e48561de4..66fa7157d 100755 --- a/package/src/main/resources/bin/setenv +++ b/package/src/main/resources/bin/setenv @@ -50,6 +50,94 @@ MY_DIRNAME=`dirname $0` MY_KARAF_HOME=`cd "$MY_DIRNAME/.."; pwd` +# Fail fast when starting without admin/health passwords (no known defaults are shipped). +# +# An unset password does NOT resolve to "no account": ${env:...} with no value expands to the +# empty string, which Karaf's PropertiesLoginModule accepts as a valid (empty) password for the +# admin account. So "unset" and "blank" are the same failure, and both must be refused. +# +# This script is the gate for shell-launched Karaf; the Docker entrypoint covers containers. Any +# launcher that starts the JVM directly without sourcing this file (systemd units, Kubernetes +# command overrides, PaxExam) bypasses the check, so those deployments must set the passwords +# themselves. +_unomi_etc="${KARAF_ETC:-${MY_KARAF_HOME}/etc}" + +# Print the configured value of a property, searching custom.system.properties and the optional +# overlay it includes. Last assignment wins, comments ignored. Empty output means "not configured". +_unomi_configured_property() { + for _unomi_file in "${_unomi_etc}/custom.system.properties" "${_unomi_etc}/unomi.custom.system.properties"; do + [ -f "${_unomi_file}" ] || continue + sed -n "s|^[[:space:]]*$1[[:space:]]*=[[:space:]]*\(.*\)$|\1|p" "${_unomi_file}" + done | sed -e 's/[[:space:]]*$//' | grep -v '^$' | tail -n 1 +} + +# _unomi_require_password ENV_VAR_NAME PROPERTY_NAME SKIP_FLAG_NAME +# Returns 0 when a non-blank password is available, 1 when startup must be refused. +_unomi_require_password() { + eval _unomi_value=\"\${$1}\" + eval _unomi_skip=\"\${$3}\" + + [ -n "${_unomi_value}" ] && return 0 + + _unomi_configured=`_unomi_configured_property "$2"` + case "${_unomi_configured}" in + # An unresolved ${env:...} placeholder is not a configured password — it expands to empty. + *'${'*) _unomi_configured='' ;; + esac + [ -n "${_unomi_configured}" ] && return 0 + + if [ "${_unomi_skip}" = "true" ]; then + cat >&2 <<EOF + +WARNING: $3=true, but neither $1 nor + $2 is set to a non-empty value. + If nothing else supplies this password, the account will be created with an EMPTY + password that authenticates successfully, granting full administrator access. + +EOF + return 0 + fi + + cat >&2 <<EOF +ERROR: $1 is not set. + +Apache Unomi does not ship a known default password, and an unset value becomes an EMPTY +password that still authenticates. Set one before starting, for example: + + export UNOMI_ROOT_PASSWORD='choose-a-strong-password' + export UNOMI_HEALTHCHECK_PASSWORD='choose-a-strong-health-password' + ./bin/karaf + +Or set $2 in etc/custom.system.properties +(then $3=true only suppresses this early check). + +See getting started / configuration documentation for details. +EOF + return 1 +} + +# Only guard the scripts that actually start the server. bin/stop, bin/status, bin/client and +# bin/shell set KARAF_SCRIPT to their own name, so the outer case already excludes them. +# +# "bin/karaf stop" and "bin/karaf status" reuse KARAF_SCRIPT=karaf, so skip on the subcommand too: +# those are the only two that replace the main class (with Main.Stop / Main.Status) instead of +# booting the server. Everything else - including "karaf client", "karaf shell" and any unknown +# argument - falls through to org.apache.karaf.main.Main and starts a full server, so it must be +# guarded. +case "${KARAF_SCRIPT}" in + karaf|start|server) + case "$1" in + stop|status) ;; + *) + _unomi_require_password UNOMI_ROOT_PASSWORD \ + org.apache.unomi.security.root.password UNOMI_SKIP_ROOT_PASSWORD_CHECK || exit 1 + _unomi_require_password UNOMI_HEALTHCHECK_PASSWORD \ + org.apache.unomi.healthcheck.password UNOMI_SKIP_HEALTHCHECK_PASSWORD_CHECK || exit 1 + ;; + esac + ;; +esac + # In order to activate the Yourkit Profiler agent, uncomment one of the following lines depending on the operating # system and adjust the path to the location of the installation directory. # On MacOS: diff --git a/package/src/main/resources/bin/setenv.bat b/package/src/main/resources/bin/setenv.bat index f5db6f80b..5333a1090 100644 --- a/package/src/main/resources/bin/setenv.bat +++ b/package/src/main/resources/bin/setenv.bat @@ -63,3 +63,64 @@ rem SET KARAF_DEBUG set MY_DIRNAME=%~dp0% set MY_KARAF_HOME=%DIRNAME%.. + +rem Warn early when starting without admin/health passwords (no known defaults are shipped). +rem +rem An unset password does NOT mean "no account": it expands to the empty string, which Karaf's +rem PropertiesLoginModule accepts as a valid password for the admin account. +rem +rem KNOWN LIMITATION: karaf.bat invokes this file with "call" and does not test errorlevel +rem afterwards, so the "exit /b 1" below returns from this script but does NOT stop the launcher. +rem The error message is printed and startup continues. Windows operators must therefore act on the +rem message; there is no way to fail closed from here without also killing the operator's console +rem (plain "exit 1" would terminate the whole cmd.exe session, including an interactive prompt). +rem +rem karaf.bat sets KARAF_SCRIPT with the quotes included in the value (SET KARAF_SCRIPT="karaf.bat"), +rem so strip them before comparing - otherwise every comparison below silently fails to match. +set _UNOMI_KARAF_SCRIPT=%KARAF_SCRIPT:"=% +if /I "%_UNOMI_KARAF_SCRIPT%"=="karaf.bat" goto checkPasswords +if /I "%_UNOMI_KARAF_SCRIPT%"=="start.bat" goto checkPasswords +if /I "%_UNOMI_KARAF_SCRIPT%"=="karaf" goto checkPasswords +if /I "%_UNOMI_KARAF_SCRIPT%"=="start" goto checkPasswords +goto afterPasswordChecks + +:checkPasswords +if not "%UNOMI_ROOT_PASSWORD%"=="" goto afterRootPasswordCheck +if /I "%UNOMI_SKIP_ROOT_PASSWORD_CHECK%"=="true" goto afterRootPasswordCheck +echo ERROR: UNOMI_ROOT_PASSWORD is not set. +echo. +echo Apache Unomi does not ship a known default admin password, and an unset value becomes +echo an EMPTY password that still authenticates. Set one before starting, for example: +echo. +echo set UNOMI_ROOT_PASSWORD=choose-a-strong-password +echo set UNOMI_HEALTHCHECK_PASSWORD=choose-a-strong-health-password +echo bin\karaf.bat +echo. +echo Or set org.apache.unomi.security.root.password in etc\custom.system.properties +echo and set UNOMI_SKIP_ROOT_PASSWORD_CHECK=true. +echo. +echo WARNING: startup continues anyway on Windows - see the note at the top of this file. +set _UNOMI_KARAF_SCRIPT= +exit /b 1 + +:afterRootPasswordCheck +if not "%UNOMI_HEALTHCHECK_PASSWORD%"=="" goto afterPasswordChecks +if /I "%UNOMI_SKIP_HEALTHCHECK_PASSWORD_CHECK%"=="true" goto afterPasswordChecks +echo ERROR: UNOMI_HEALTHCHECK_PASSWORD is not set. +echo. +echo Apache Unomi does not ship a known default health-check password, and an unset value +echo becomes an EMPTY password that still authenticates. Set one before starting, for example: +echo. +echo set UNOMI_ROOT_PASSWORD=choose-a-strong-password +echo set UNOMI_HEALTHCHECK_PASSWORD=choose-a-strong-health-password +echo bin\karaf.bat +echo. +echo Or set org.apache.unomi.healthcheck.password in etc\custom.system.properties +echo and set UNOMI_SKIP_HEALTHCHECK_PASSWORD_CHECK=true. +echo. +echo WARNING: startup continues anyway on Windows - see the note at the top of this file. +set _UNOMI_KARAF_SCRIPT= +exit /b 1 + +:afterPasswordChecks +set _UNOMI_KARAF_SCRIPT= diff --git a/package/src/main/resources/etc/custom.system.properties b/package/src/main/resources/etc/custom.system.properties index fa98f7979..4dd435a78 100644 --- a/package/src/main/resources/etc/custom.system.properties +++ b/package/src/main/resources/etc/custom.system.properties @@ -22,7 +22,13 @@ ${optionals}=unomi.custom.system.properties ####################################################################################################################### ## Security settings ## ####################################################################################################################### -org.apache.unomi.security.root.password=${env:UNOMI_ROOT_PASSWORD:-karaf} +# Required: set UNOMI_ROOT_PASSWORD and UNOMI_HEALTHCHECK_PASSWORD before start +# (bin/setenv and the Docker entrypoint refuse to start if either is unset). +# There are no shipped known defaults. +# Escape hatches: UNOMI_SKIP_ROOT_PASSWORD_CHECK=true / UNOMI_SKIP_HEALTHCHECK_PASSWORD_CHECK=true +# if you set the matching property another way. +org.apache.unomi.security.root.password=${env:UNOMI_ROOT_PASSWORD} +org.apache.unomi.healthcheck.password=${env:UNOMI_HEALTHCHECK_PASSWORD} # These parameters control the list of classes that are allowed or forbidden when executing expressions. org.apache.unomi.scripting.allow=${env:UNOMI_ALLOW_SCRIPTING_CLASSES:-org.apache.unomi.api.Event,org.apache.unomi.api.Profile,org.apache.unomi.api.Session,org.apache.unomi.api.Item,org.apache.unomi.api.CustomItem,java.lang.Object,java.util.Map,java.util.HashMap,java.lang.Integer,org.mvel2.*,java.lang.String} @@ -302,7 +308,8 @@ org.apache.unomi.profile.cookie.name=${env:UNOMI_PROFILE_COOKIE_NAME:-context-pr # This setting controls the maximum age of the profile cookie. By default it is set to a year. org.apache.unomi.profile.cookie.maxAgeInSeconds=${env:UNOMI_PROFILE_COOKIE_MAXAGEINSECONDS:-31536000} # This setting controls if the cookie should be flagged as HttpOnly or not. -org.apache.unomi.profile.cookie.httpOnly=${env:UNOMI_PROFILE_COOKIE_HTTPONLY:-false} +# Default true so browser JavaScript cannot read the profile bearer cookie. +org.apache.unomi.profile.cookie.httpOnly=${env:UNOMI_PROFILE_COOKIE_HTTPONLY:-true} #Allowed profile download formats, actually only csv (horizontal and vertical), json, text and yaml are allowed. org.apache.unomi.profile.download.formats=${env:UNOMI_PROFILE_DOWNLOAD_FORMATS:-csv,yaml,json,text} # This setting allow for request size (Content-length) protection. Checking that the requests do not exceed the limit. diff --git a/package/src/main/resources/etc/users.properties b/package/src/main/resources/etc/users.properties index bffdc5bf3..c7064f536 100644 --- a/package/src/main/resources/etc/users.properties +++ b/package/src/main/resources/etc/users.properties @@ -29,6 +29,8 @@ # and modifiable via the JAAS command group. These users reside in a JAAS domain # with the name "karaf". # -karaf = ${org.apache.unomi.security.root.password:-karaf},_g_:admingroup -health = ${org.apache.unomi.healthcheck.password:-health},health +# Passwords come from org.apache.unomi.security.root.password / healthcheck.password (see custom.system.properties). +# No known defaults are shipped; set UNOMI_ROOT_PASSWORD and UNOMI_HEALTHCHECK_PASSWORD before start. +karaf = ${org.apache.unomi.security.root.password},_g_:admingroup +health = ${org.apache.unomi.healthcheck.password},health _g_\:admingroup = group,admin,manager,viewer,systembundles,ssh,ROLE_UNOMI_ADMIN,ROLE_UNOMI_TENANT_ADMIN,ROLE_UNOMI_TENANT_USER diff --git a/rest/src/main/java/org/apache/unomi/rest/authentication/AuthenticationFilter.java b/rest/src/main/java/org/apache/unomi/rest/authentication/AuthenticationFilter.java index 3c59c1cf6..0e990ffaa 100644 --- a/rest/src/main/java/org/apache/unomi/rest/authentication/AuthenticationFilter.java +++ b/rest/src/main/java/org/apache/unomi/rest/authentication/AuthenticationFilter.java @@ -101,17 +101,33 @@ public class AuthenticationFilter implements ContainerRequestFilter { TenantService tenantService, SecurityService securityService, ExecutionContextManager executionContextManager) { + this(restAuthenticationConfig, tenantService, securityService, executionContextManager, buildJaasFilter()); + } + + /** + * Test seam: lets a test supply a stub JAAS filter so it can assert that a credential was + * refused <em>before</em> it reached JAAS. Asserting on the response status alone proves + * nothing here — every refusal path in this class ends in the same 401. + */ + AuthenticationFilter(RestAuthenticationConfig restAuthenticationConfig, + TenantService tenantService, + SecurityService securityService, + ExecutionContextManager executionContextManager, + JAASAuthenticationFilter jaasAuthenticationFilter) { this.restAuthenticationConfig = restAuthenticationConfig; this.tenantService = tenantService; this.securityService = securityService; this.executionContextManager = executionContextManager; + this.jaasAuthenticationFilter = jaasAuthenticationFilter; + } - // Build wrapped jaas filter - jaasAuthenticationFilter = new JAASAuthenticationFilter(); - jaasAuthenticationFilter.setRoleClassifier(ROLE_CLASSIFIER); - jaasAuthenticationFilter.setRoleClassifierType(ROLE_CLASSIFIER_TYPE); - jaasAuthenticationFilter.setContextName(CONTEXT_NAME); - jaasAuthenticationFilter.setRealmName(REALM_NAME); + private static JAASAuthenticationFilter buildJaasFilter() { + JAASAuthenticationFilter jaasFilter = new JAASAuthenticationFilter(); + jaasFilter.setRoleClassifier(ROLE_CLASSIFIER); + jaasFilter.setRoleClassifierType(ROLE_CLASSIFIER_TYPE); + jaasFilter.setContextName(CONTEXT_NAME); + jaasFilter.setRealmName(REALM_NAME); + return jaasFilter; } @Override @@ -133,6 +149,9 @@ public class AuthenticationFilter implements ContainerRequestFilter { unauthorized(requestContext); return; } + if (rejectBlankBasicAuthPassword(requestContext, authHeader)) { + return; + } try { jaasAuthenticationFilter.filter(requestContext); @@ -192,6 +211,9 @@ public class AuthenticationFilter implements ContainerRequestFilter { // For all other cases, try tenant private key first, then fall back to JAAS String authHeader = requestContext.getHeaderString(HttpHeaders.AUTHORIZATION); if (authHeader != null && authHeader.startsWith(BASIC_AUTH_PREFIX)) { + if (rejectBlankBasicAuthPassword(requestContext, authHeader)) { + return; + } // Try tenant private key authentication first String[] credentials = extractBasicAuthCredentials(authHeader); if (credentials != null && credentials.length == 2) { @@ -299,6 +321,9 @@ public class AuthenticationFilter implements ContainerRequestFilter { // For private endpoints, require system administrator authentication (like V2) String authHeader = requestContext.getHeaderString(HttpHeaders.AUTHORIZATION); if (authHeader != null && authHeader.startsWith(BASIC_AUTH_PREFIX)) { + if (rejectBlankBasicAuthPassword(requestContext, authHeader)) { + return; + } try { jaasAuthenticationFilter.filter(requestContext); // JAASAuthenticationFilter handles credential failures internally (calls abortWith + returns normally). @@ -352,6 +377,45 @@ public class AuthenticationFilter implements ContainerRequestFilter { unauthorized(requestContext); } + /** + * Rejects the request when the Basic credential about to be used carries an empty password. + * <p> + * Defence in depth against a blank administrator password (UNOMI-972). If + * {@code org.apache.unomi.security.root.password} is unset, Karaf resolves it to the empty + * string and the shipped JAAS account authenticates with an empty password. {@code bin/karaf} + * and the Docker entrypoint refuse to start in that state, but they cannot cover every launcher + * (notably {@code karaf.bat}), so an empty credential is never accepted over REST either. + * <p> + * Deliberately called at each point where a Basic credential is actually consumed rather than + * once at the top of {@link #filter}: anonymous traffic on public paths — and every path in V2 + * compatibility mode — ignores {@code Authorization} entirely, so rejecting up front would turn + * a stray or stale Basic header (a cached browser credential, an injecting proxy) into a 401 on + * a request that is supposed to succeed without authentication. + * + * @return {@code true} when the request has been aborted and the caller must return + */ + private boolean rejectBlankBasicAuthPassword(ContainerRequestContext requestContext, String authHeader) { + if (!hasBlankBasicAuthPassword(authHeader)) { + return false; + } + logger.warn("Rejecting Basic authentication with an empty password"); + unauthorized(requestContext); + return true; + } + + /** + * Whether a Basic {@code Authorization} header carries an empty password. A missing, malformed + * or non-Basic header is not treated as blank here — those are rejected by the normal + * authentication paths instead. + */ + boolean hasBlankBasicAuthPassword(String authHeader) { + if (authHeader == null || !authHeader.startsWith(BASIC_AUTH_PREFIX)) { + return false; + } + String[] credentials = extractBasicAuthCredentials(authHeader); + return credentials != null && credentials.length == 2 && credentials[1].isEmpty(); + } + private String[] extractBasicAuthCredentials(String authHeader) { try { String base64Credentials = authHeader.substring(BASIC_AUTH_PREFIX.length()).trim(); diff --git a/rest/src/test/java/org/apache/unomi/rest/authentication/AuthenticationFilterBlankPasswordTest.java b/rest/src/test/java/org/apache/unomi/rest/authentication/AuthenticationFilterBlankPasswordTest.java new file mode 100644 index 000000000..a219885c3 --- /dev/null +++ b/rest/src/test/java/org/apache/unomi/rest/authentication/AuthenticationFilterBlankPasswordTest.java @@ -0,0 +1,243 @@ +/* + * 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.rest.authentication; + +import org.apache.cxf.jaxrs.security.JAASAuthenticationFilter; +import org.apache.unomi.api.security.SecurityService; +import org.apache.unomi.api.services.ExecutionContextManager; +import org.apache.unomi.api.tenants.ApiKey; +import org.apache.unomi.api.tenants.TenantService; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import javax.ws.rs.container.ContainerRequestContext; +import javax.ws.rs.core.HttpHeaders; +import javax.ws.rs.core.Response; +import javax.ws.rs.core.UriInfo; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.Collections; +import java.util.regex.Pattern; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +/** + * A blank {@code UNOMI_ROOT_PASSWORD} leaves the shipped JAAS administrator with an empty password + * that {@code PropertiesLoginModule} accepts. {@code bin/karaf} and the Docker entrypoint refuse to + * start in that state, but they cannot cover every launcher, so the REST layer must never accept an + * empty credential either. + * <p> + * Covers both the predicate and its wiring into {@link AuthenticationFilter#filter}: the check is + * applied where a Basic credential is consumed, and must NOT reject anonymous traffic on public + * paths that ignores {@code Authorization} altogether. + */ +class AuthenticationFilterBlankPasswordTest { + + private RestAuthenticationConfig restAuthenticationConfig; + private TenantService tenantService; + private JAASAuthenticationFilter jaasAuthenticationFilter; + private AuthenticationFilter filter; + + @BeforeEach + void setUp() { + restAuthenticationConfig = mock(RestAuthenticationConfig.class); + tenantService = mock(TenantService.class); + // Stubbed so the tests below can assert the credential never reached JAAS. Every refusal + // path in the filter answers 401, so the status alone cannot tell "refused for a blank + // password" apart from "JAAS rejected it" — only this can. + jaasAuthenticationFilter = mock(JAASAuthenticationFilter.class); + filter = new AuthenticationFilter( + restAuthenticationConfig, + tenantService, + mock(SecurityService.class), + mock(ExecutionContextManager.class), + jaasAuthenticationFilter); + } + + @Test + void emptyPasswordIsRejected() { + assertTrue(filter.hasBlankBasicAuthPassword(basic("karaf:"))); + } + + @Test + void emptyUserAndPasswordIsRejected() { + assertTrue(filter.hasBlankBasicAuthPassword(basic(":"))); + } + + @Test + void realPasswordIsAccepted() { + assertFalse(filter.hasBlankBasicAuthPassword(basic("karaf:a-strong-password"))); + } + + /** + * A password consisting of spaces is a real (if terrible) password, not the blank-resolution + * failure this guard exists for — leave it to the realm. + */ + @Test + void whitespacePasswordIsNotTreatedAsBlank() { + assertFalse(filter.hasBlankBasicAuthPassword(basic("karaf: "))); + } + + @Test + void missingOrNonBasicHeadersAreLeftToTheNormalPaths() { + assertFalse(filter.hasBlankBasicAuthPassword(null)); + assertFalse(filter.hasBlankBasicAuthPassword("Bearer some-token")); + } + + @Test + void malformedHeaderIsLeftToTheNormalPaths() { + assertFalse(filter.hasBlankBasicAuthPassword("Basic not-base64!!")); + // No colon at all: cannot be split into user and password. + assertFalse(filter.hasBlankBasicAuthPassword(basic("karaf"))); + } + + // ------------------------------------------------------------------ filter() wiring + + /** + * The predicate being correct is not enough: if the call site were dropped or moved after an + * earlier {@code return}, only this test would notice. + */ + @Test + void filterRejectsBlankPasswordOnAnAuthenticatedPath() throws IOException { + ContainerRequestContext requestContext = request("tenants", basic("karaf:")); + + filter.filter(requestContext); + + assertUnauthorizedWithoutReachingJaas(requestContext); + } + + /** Control: a non-blank credential on the same path must still be handed to JAAS to judge. */ + @Test + void filterPassesNonBlankPasswordToJaasOnAnAuthenticatedPath() throws IOException { + ContainerRequestContext requestContext = request("tenants", basic("karaf:a-strong-password")); + + filter.filter(requestContext); + + verify(jaasAuthenticationFilter).filter(requestContext); + } + + /** + * V2 compatibility mode routes every request through {@link AuthenticationFilter}'s own + * private-endpoint branch, which consumes the Basic credential at a third, separate call site. + * Without this test that call site is unreachable from the suite: the other tests leave + * {@code isV2CompatibilityModeEnabled()} at the unstubbed Mockito {@code false}, so deleting + * the guard there would leave every test green. + */ + @Test + void filterRejectsBlankPasswordOnAPrivatePathInV2CompatibilityMode() throws IOException { + when(restAuthenticationConfig.isV2CompatibilityModeEnabled()).thenReturn(true); + when(restAuthenticationConfig.getPublicPathPatterns()).thenReturn(Collections.emptyList()); + ContainerRequestContext requestContext = request("profiles", basic("karaf:")); + + filter.filter(requestContext); + + assertUnauthorizedWithoutReachingJaas(requestContext); + } + + /** Control for the V2 branch: a non-blank credential must still reach JAAS there too. */ + @Test + void filterPassesNonBlankPasswordToJaasOnAPrivatePathInV2CompatibilityMode() throws IOException { + when(restAuthenticationConfig.isV2CompatibilityModeEnabled()).thenReturn(true); + when(restAuthenticationConfig.getPublicPathPatterns()).thenReturn(Collections.emptyList()); + ContainerRequestContext requestContext = request("profiles", basic("karaf:a-strong-password")); + + filter.filter(requestContext); + + verify(jaasAuthenticationFilter).filter(requestContext); + } + + /** + * A public path in V2 compatibility mode authenticates by default tenant, ignoring + * {@code Authorization} entirely — so a stray blank Basic header must not turn it into a 401. + */ + @Test + void filterDoesNotRejectAStrayBlankBasicHeaderOnAPublicPathInV2CompatibilityMode() throws IOException { + when(restAuthenticationConfig.isV2CompatibilityModeEnabled()).thenReturn(true); + when(restAuthenticationConfig.getPublicPathPatterns()) + .thenReturn(Collections.singletonList(Pattern.compile("POST context\\.json"))); + when(restAuthenticationConfig.getV2CompatibilityDefaultTenantId()).thenReturn("default"); + ContainerRequestContext requestContext = request("context.json", basic("someone:")); + + filter.filter(requestContext); + + verify(tenantService).getTenant("default"); + } + + private void assertUnauthorizedWithoutReachingJaas(ContainerRequestContext requestContext) throws IOException { + ArgumentCaptor<Response> aborted = ArgumentCaptor.forClass(Response.class); + verify(requestContext).abortWith(aborted.capture()); + assertEquals(Response.Status.UNAUTHORIZED.getStatusCode(), aborted.getValue().getStatus()); + verify(jaasAuthenticationFilter, never()).filter(any()); + } + + /** + * Regression guard for the placement bug: the check used to run at the top of {@code filter()}, + * so anonymous traffic carrying a stray Basic header — a stale cached browser credential, an + * injecting proxy — was rejected before the public-path branch could authenticate it by API key. + * <p> + * Asserted by observing that the public-path branch is still reached (the API key is looked up) + * rather than by asserting no abort: a public path whose API key does not resolve legitimately + * falls through and is refused for that unrelated reason, and authenticating one successfully + * needs a live CXF exchange, which is out of scope for a unit test. + */ + @Test + void filterConsultsThePublicPathBranchDespiteAStrayBlankBasicHeader() throws IOException { + when(restAuthenticationConfig.getPublicPathPatterns()) + .thenReturn(Collections.singletonList(Pattern.compile("POST context\\.json"))); + ContainerRequestContext requestContext = request("context.json", basic("someone:")); + + filter.filter(requestContext); + + verify(tenantService).getTenantByApiKey(any(), eq(ApiKey.ApiKeyType.PUBLIC)); + } + + /** A public path with no Authorization header at all must equally reach the API-key lookup. */ + @Test + void filterConsultsThePublicPathBranchForAnonymousRequests() throws IOException { + when(restAuthenticationConfig.getPublicPathPatterns()) + .thenReturn(Collections.singletonList(Pattern.compile("POST context\\.json"))); + ContainerRequestContext requestContext = request("context.json", null); + + filter.filter(requestContext); + + verify(tenantService).getTenantByApiKey(any(), eq(ApiKey.ApiKeyType.PUBLIC)); + } + + private ContainerRequestContext request(String path, String authHeader) { + ContainerRequestContext requestContext = mock(ContainerRequestContext.class); + UriInfo uriInfo = mock(UriInfo.class); + when(uriInfo.getPath()).thenReturn(path); + when(requestContext.getUriInfo()).thenReturn(uriInfo); + when(requestContext.getMethod()).thenReturn("POST"); + when(requestContext.getHeaderString(HttpHeaders.AUTHORIZATION)).thenReturn(authHeader); + return requestContext; + } + + private static String basic(String credentials) { + return "Basic " + Base64.getEncoder().encodeToString(credentials.getBytes(StandardCharsets.UTF_8)); + } +} diff --git a/rest/src/test/java/org/apache/unomi/rest/config/ShippedAdminPasswordConfigTest.java b/rest/src/test/java/org/apache/unomi/rest/config/ShippedAdminPasswordConfigTest.java new file mode 100644 index 000000000..b44e9fea0 --- /dev/null +++ b/rest/src/test/java/org/apache/unomi/rest/config/ShippedAdminPasswordConfigTest.java @@ -0,0 +1,428 @@ +/* + * 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.rest.config; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.concurrent.TimeUnit; +import java.util.stream.Collectors; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +/** + * Ensures the distribution cannot start with a known or blank admin/health password. + * <p> + * The launcher guards are <em>executed</em> here rather than grepped: a check whose text is present + * but whose condition never matches would otherwise pass silently, which is exactly how the Windows + * {@code KARAF_SCRIPT} quoting bug survived review. + */ +class ShippedAdminPasswordConfigTest { + + private static final String ROOT_PASSWORD_PROPERTY = "org.apache.unomi.security.root.password"; + private static final String HEALTHCHECK_PASSWORD_PROPERTY = "org.apache.unomi.healthcheck.password"; + + /** + * The two files this test executes. They double as the fingerprint of the repository root (see + * {@link #locateRepoRoot()}): a directory containing both is the Unomi checkout, not some nested + * copy or a same-named directory further up the filesystem. + */ + private static final String SETENV_PATH = "package/src/main/resources/bin/setenv"; + private static final String ENTRYPOINT_PATH = "docker/src/main/docker/entrypoint.sh"; + + /** First line of the guard to copy out of entrypoint.sh. */ + private static final String ENTRYPOINT_GUARD_START = "check_required_password()"; + /** Last line of the guard; the slice is meaningless unless this is actually reached. */ + private static final String ENTRYPOINT_GUARD_END = "UNOMI_SKIP_HEALTHCHECK_PASSWORD_CHECK || exit 1"; + + /** Default stub launcher name, mirroring {@code bin/karaf}. */ + private static final String DEFAULT_KARAF_SCRIPT = "karaf"; + + private static final Path REPO_ROOT = locateRepoRoot(); + + // ---------------------------------------------------------------- shipped configuration + + @Test + void usersProperties_hasNoKnownDefaultPasswordFallback() throws Exception { + String content = Files.readString(repoFile("package/src/main/resources/etc/users.properties")); + + assertFalse(content.contains(":-karaf"), "users.properties must not default the karaf password to 'karaf'"); + assertFalse(content.contains(":-health"), "users.properties must not default the health password to 'health'"); + assertTrue(content.contains("${" + ROOT_PASSWORD_PROPERTY + "}")); + assertTrue(content.contains("${" + HEALTHCHECK_PASSWORD_PROPERTY + "}")); + } + + @Test + void customSystemProperties_requiresRootPasswordEnvWithoutKnownDefault() throws Exception { + List<String> securityLines = Files.readAllLines(repoFile("package/src/main/resources/etc/custom.system.properties")) + .stream() + .filter(line -> line.contains(ROOT_PASSWORD_PROPERTY) || line.contains(HEALTHCHECK_PASSWORD_PROPERTY)) + .collect(Collectors.toList()); + + assertFalse(securityLines.isEmpty()); + for (String line : securityLines) { + assertFalse(line.contains(":-karaf"), "root password must not fall back to karaf: " + line); + assertFalse(line.contains(":-health"), "health password must not fall back to health: " + line); + } + } + + @Test + void profileCookieHttpOnly_defaultsToTrue() throws Exception { + String webCfg = Files.readString(repoFile("web-servlets/src/main/resources/org.apache.unomi.web.cfg")); + assertTrue(webCfg.matches("(?s).*profileIdCookieHttpOnly=\\$\\{[^}]*:-true}.*"), + "profileId cookie HttpOnly should default to true"); + + String systemProps = Files.readString(repoFile("package/src/main/resources/etc/custom.system.properties")); + assertTrue(systemProps.matches("(?s).*org\\.apache\\.unomi\\.profile\\.cookie\\.httpOnly=\\$\\{[^}]*:-true}.*"), + "custom.system.properties must default profile cookie HttpOnly to true"); + } + + // ---------------------------------------------------------------- bin/setenv, executed + + @Test + void setenv_refusesToStartWhenPasswordsMissing(@TempDir Path karafHome) throws Exception { + assumeTrue(hasPosixShell(), "requires a POSIX shell"); + installFakeKarafHome(karafHome, DEFAULT_KARAF_SCRIPT, "${env:UNOMI_ROOT_PASSWORD}", "${env:UNOMI_HEALTHCHECK_PASSWORD}"); + + Result missing = runLauncher(karafHome, DEFAULT_KARAF_SCRIPT, new HashMap<>()); + assertEquals(1, missing.exitCode, "setenv must refuse to start without passwords:\n" + missing.output); + assertFalse(missing.output.contains("LAUNCHED"), "the launcher must not be reached"); + assertTrue(missing.output.contains("UNOMI_ROOT_PASSWORD is not set"), missing.output); + } + + /** + * The outer {@code case "${KARAF_SCRIPT}"} in bin/setenv lists every shipped script that boots a + * server: {@code bin/karaf}, {@code bin/start} and {@code bin/karaf server} all reach + * {@code org.apache.karaf.main.Main}. Only {@code karaf} used to be exercised here, so narrowing + * that list to a single entry would have gone unnoticed - now each entry has its own test case. + */ + @ParameterizedTest(name = "KARAF_SCRIPT={0}") + @ValueSource(strings = {"karaf", "start", "server"}) + void setenv_refusesToStartForEveryServerStartingScript(String karafScript, @TempDir Path tempDir) throws Exception { + assumeTrue(hasPosixShell(), "requires a POSIX shell"); + Path karafHome = Files.createDirectories(tempDir.resolve(karafScript)); + installFakeKarafHome(karafHome, karafScript, "${env:UNOMI_ROOT_PASSWORD}", "${env:UNOMI_HEALTHCHECK_PASSWORD}"); + + Result missing = runLauncher(karafHome, karafScript, new HashMap<>()); + assertEquals(1, missing.exitCode, + "bin/" + karafScript + " starts a server, so it must refuse to run without passwords:\n" + missing.output); + assertFalse(missing.output.contains("LAUNCHED"), "the launcher must not be reached for KARAF_SCRIPT=" + karafScript); + assertTrue(missing.output.contains("UNOMI_ROOT_PASSWORD is not set"), missing.output); + } + + /** + * The dedicated {@code bin/stop}, {@code bin/status}, {@code bin/client} and {@code bin/shell} + * scripts set {@code KARAF_SCRIPT} to their own name. None of them starts a server - they talk to + * an already running one - so the outer case must keep excluding them, otherwise stopping an + * instance would require the passwords used to start it. + */ + @ParameterizedTest(name = "KARAF_SCRIPT={0}") + @ValueSource(strings = {"stop", "status", "client", "shell"}) + void setenv_doesNotGuardScriptsThatOnlyTalkToARunningServer(String karafScript, @TempDir Path tempDir) throws Exception { + assumeTrue(hasPosixShell(), "requires a POSIX shell"); + Path karafHome = Files.createDirectories(tempDir.resolve(karafScript)); + installFakeKarafHome(karafHome, karafScript, "${env:UNOMI_ROOT_PASSWORD}", "${env:UNOMI_HEALTHCHECK_PASSWORD}"); + + Result result = runLauncher(karafHome, karafScript, new HashMap<>()); + assertEquals(0, result.exitCode, "bin/" + karafScript + " must not require the passwords:\n" + result.output); + assertTrue(result.output.contains("LAUNCHED"), result.output); + } + + @Test + void setenv_startsWhenPasswordsProvided(@TempDir Path karafHome) throws Exception { + assumeTrue(hasPosixShell(), "requires a POSIX shell"); + installFakeKarafHome(karafHome, DEFAULT_KARAF_SCRIPT, "${env:UNOMI_ROOT_PASSWORD}", "${env:UNOMI_HEALTHCHECK_PASSWORD}"); + + Map<String, String> env = new HashMap<>(); + env.put("UNOMI_ROOT_PASSWORD", "a-strong-password"); + env.put("UNOMI_HEALTHCHECK_PASSWORD", "a-strong-health-password"); + + Result provided = runLauncher(karafHome, DEFAULT_KARAF_SCRIPT, env); + assertEquals(0, provided.exitCode, provided.output); + assertTrue(provided.output.contains("LAUNCHED"), provided.output); + } + + /** + * The escape hatch only suppresses the environment-variable check. Configuring the property + * directly is a supported way to start, and must not be reported as an error. + */ + @Test + void setenv_acceptsPasswordsConfiguredInPropertiesFile(@TempDir Path karafHome) throws Exception { + assumeTrue(hasPosixShell(), "requires a POSIX shell"); + installFakeKarafHome(karafHome, DEFAULT_KARAF_SCRIPT, "configured-root", "configured-health"); + + Result configured = runLauncher(karafHome, DEFAULT_KARAF_SCRIPT, new HashMap<>()); + assertEquals(0, configured.exitCode, configured.output); + assertTrue(configured.output.contains("LAUNCHED"), configured.output); + } + + /** + * Claiming the password is set elsewhere, while leaving it blank everywhere, is the dangerous + * case: it must warn rather than pass silently. + */ + @Test + void setenv_warnsWhenSkipFlagHidesABlankPassword(@TempDir Path karafHome) throws Exception { + assumeTrue(hasPosixShell(), "requires a POSIX shell"); + installFakeKarafHome(karafHome, DEFAULT_KARAF_SCRIPT, "${env:UNOMI_ROOT_PASSWORD}", "${env:UNOMI_HEALTHCHECK_PASSWORD}"); + + Map<String, String> env = new HashMap<>(); + env.put("UNOMI_SKIP_ROOT_PASSWORD_CHECK", "true"); + env.put("UNOMI_SKIP_HEALTHCHECK_PASSWORD_CHECK", "true"); + + Result skipped = runLauncher(karafHome, DEFAULT_KARAF_SCRIPT, env); + assertEquals(0, skipped.exitCode, skipped.output); + assertTrue(skipped.output.contains("WARNING"), "a bypassed check must still warn:\n" + skipped.output); + } + + /** + * {@code bin/karaf stop} and {@code bin/karaf status} keep {@code KARAF_SCRIPT=karaf} but replace + * the main class with {@code Main.Stop} / {@code Main.Status}: no server is started, so no + * password is needed. Requiring one would make an instance impossible to shut down cleanly from a + * shell that no longer has the startup environment. + */ + @ParameterizedTest(name = "bin/karaf {0}") + @ValueSource(strings = {"stop", "status"}) + void setenv_doesNotBlockSubcommandsThatDoNotStartAServer(String subcommand, @TempDir Path tempDir) throws Exception { + assumeTrue(hasPosixShell(), "requires a POSIX shell"); + Path karafHome = Files.createDirectories(tempDir.resolve(subcommand)); + installFakeKarafHome(karafHome, DEFAULT_KARAF_SCRIPT, "${env:UNOMI_ROOT_PASSWORD}", "${env:UNOMI_HEALTHCHECK_PASSWORD}"); + + Result result = runLauncher(karafHome, DEFAULT_KARAF_SCRIPT, new HashMap<>(), subcommand); + assertEquals(0, result.exitCode, "'karaf " + subcommand + "' must not require the passwords:\n" + result.output); + assertTrue(result.output.contains("LAUNCHED"), result.output); + } + + /** + * Regression guard for a real bug: the inner skip list once read {@code stop|status|client|shell}, + * which silently disabled the password check for {@code bin/karaf client} and + * {@code bin/karaf shell}. Those subcommands are <em>not</em> the {@code bin/client} / + * {@code bin/shell} remote consoles - the karaf script does not special-case them, so they fall + * through to {@code org.apache.karaf.main.Main} and boot a complete server, admin account + * included. The same is true of any argument the script does not recognise. Only {@code stop} and + * {@code status} may skip the check; everything else here must still be refused. + */ + @ParameterizedTest(name = "bin/karaf {0}") + @ValueSource(strings = {"client", "shell", "console", "--an-argument-karaf-does-not-know"}) + void setenv_stillGuardsSubcommandsThatBootAFullServer(String subcommand, @TempDir Path tempDir) throws Exception { + assumeTrue(hasPosixShell(), "requires a POSIX shell"); + Path karafHome = Files.createDirectories(tempDir.resolve(subcommand.replace("-", "_"))); + installFakeKarafHome(karafHome, DEFAULT_KARAF_SCRIPT, "${env:UNOMI_ROOT_PASSWORD}", "${env:UNOMI_HEALTHCHECK_PASSWORD}"); + + Result result = runLauncher(karafHome, DEFAULT_KARAF_SCRIPT, new HashMap<>(), subcommand); + assertEquals(1, result.exitCode, + "'karaf " + subcommand + "' starts a full server, so it must refuse to run without passwords:\n" + result.output); + assertFalse(result.output.contains("LAUNCHED"), "the launcher must not be reached for 'karaf " + subcommand + "'"); + assertTrue(result.output.contains("UNOMI_ROOT_PASSWORD is not set"), result.output); + } + + // ---------------------------------------------------------------- docker entrypoint, executed + + @Test + void entrypoint_refusesToStartWhenPasswordsMissing(@TempDir Path workDir) throws Exception { + assumeTrue(hasPosixShell(), "requires a POSIX shell"); + Path guard = extractEntrypointPasswordGuard(workDir); + + assertEquals(1, runShell(guard, workDir, new HashMap<>()).exitCode, + "the Docker entrypoint must exit non-zero without passwords"); + + Map<String, String> env = new HashMap<>(); + env.put("UNOMI_ROOT_PASSWORD", "a-strong-password"); + env.put("UNOMI_HEALTHCHECK_PASSWORD", "a-strong-health-password"); + Result provided = runShell(guard, workDir, env); + assertEquals(0, provided.exitCode, provided.output); + assertTrue(provided.output.contains("GUARD-PASSED"), provided.output); + } + + // ---------------------------------------------------------------- setenv.bat regression guard + + /** + * {@code karaf.bat} sets {@code KARAF_SCRIPT} with the quotes included in the value + * ({@code SET KARAF_SCRIPT="karaf.bat"}), so comparing {@code "%KARAF_SCRIPT%"} against + * {@code "karaf.bat"} never matches and the whole check is skipped. The quotes must be stripped + * before comparing. This cannot be executed on a POSIX CI machine, so assert the shape instead. + */ + @Test + void setenvBat_stripsQuotesBeforeComparingKarafScript() throws Exception { + String content = Files.readString(repoFile("package/src/main/resources/bin/setenv.bat")); + + assertTrue(content.contains("%KARAF_SCRIPT:\"=%"), + "setenv.bat must strip the quotes karaf.bat embeds in KARAF_SCRIPT"); + assertFalse(content.contains("\"%KARAF_SCRIPT%\"==\"karaf.bat\""), + "comparing the raw KARAF_SCRIPT against karaf.bat never matches"); + assertTrue(content.contains("UNOMI_ROOT_PASSWORD") && content.contains("UNOMI_HEALTHCHECK_PASSWORD"), + "setenv.bat must check both passwords"); + } + + // ---------------------------------------------------------------- helpers + + /** + * Finds the repository root by walking up from the working directory until an ancestor holds + * <em>both</em> shipped launcher scripts this test executes. Matching on a single relative path + * would let a nested checkout (or any unrelated directory that happens to contain a + * {@code package/} tree) win, and the test would then silently assert against the wrong sources. + */ + private static Path locateRepoRoot() { + Path start = Paths.get("").toAbsolutePath().normalize(); + for (Path candidate = start; candidate != null; candidate = candidate.getParent()) { + if (Files.isRegularFile(candidate.resolve(SETENV_PATH)) + && Files.isRegularFile(candidate.resolve(ENTRYPOINT_PATH))) { + return candidate; + } + } + throw new IllegalStateException("Could not locate the Unomi repository root from " + start + + " (looked for an ancestor containing both " + SETENV_PATH + " and " + ENTRYPOINT_PATH + ")"); + } + + /** + * Resolves a repository-relative path against the detected repository root, so the test works + * regardless of which module directory the build runs it from. + */ + private static Path repoFile(String relativePath) { + Path resolved = REPO_ROOT.resolve(relativePath); + if (!Files.exists(resolved)) { + throw new IllegalStateException("Could not locate " + relativePath + " under repository root " + REPO_ROOT); + } + return resolved; + } + + private static boolean hasPosixShell() { + return !System.getProperty("os.name", "").toLowerCase().contains("win"); + } + + /** + * Builds a throwaway Karaf layout containing the real {@code bin/setenv} plus a stub launcher + * that mimics how the shipped scripts source it. {@code karafScript} names the stub and the value + * it exports as {@code KARAF_SCRIPT}, so tests can reproduce {@code bin/karaf}, {@code bin/start}, + * {@code bin/stop}, ... rather than only ever exercising {@code karaf}. + */ + private static void installFakeKarafHome(Path karafHome, String karafScript, String rootPassword, String healthPassword) + throws IOException { + Path bin = Files.createDirectories(karafHome.resolve("bin")); + Path etc = Files.createDirectories(karafHome.resolve("etc")); + + Files.copy(repoFile(SETENV_PATH), bin.resolve("setenv")); + Files.write(etc.resolve("custom.system.properties"), + (ROOT_PASSWORD_PROPERTY + "=" + rootPassword + "\n" + + HEALTHCHECK_PASSWORD_PROPERTY + "=" + healthPassword + "\n").getBytes(StandardCharsets.UTF_8)); + + // Mirrors apache-karaf/bin/<script>: export KARAF_SCRIPT, then source setenv at top level. + Path launcher = bin.resolve(karafScript); + Files.write(launcher, ("#!/bin/sh\n" + + "KARAF_SCRIPT=\"" + karafScript + "\"\n" + + "export KARAF_SCRIPT\n" + + ". \"$(dirname \"$0\")/setenv\"\n" + + "echo LAUNCHED\n").getBytes(StandardCharsets.UTF_8)); + launcher.toFile().setExecutable(true); + } + + /** + * Copies the entrypoint's password guard into a standalone script, so the test does not have to + * run the rest of the container bootstrap (which needs a search engine). + */ + private static Path extractEntrypointPasswordGuard(Path workDir) throws IOException { + List<String> lines = Files.readAllLines(repoFile(ENTRYPOINT_PATH)); + StringBuilder guard = new StringBuilder("#!/bin/sh\n"); + boolean capturing = false; + boolean terminated = false; + for (String line : lines) { + if (line.startsWith(ENTRYPOINT_GUARD_START)) { + capturing = true; + } + if (capturing) { + guard.append(line).append('\n'); + } + if (capturing && line.contains(ENTRYPOINT_GUARD_END)) { + terminated = true; + break; + } + } + assertTrue(capturing, "entrypoint.sh must define the guard; start marker '" + ENTRYPOINT_GUARD_START + + "' was not found in " + ENTRYPOINT_PATH); + // Without this, a reformatted entrypoint.sh would make the slice run to end-of-file: the test + // would still "pass" while executing something entirely different from the guard. + assertTrue(terminated, "the guard slice never reached its terminating line '" + ENTRYPOINT_GUARD_END + + "' in " + ENTRYPOINT_PATH + " - update the marker, this test is no longer testing the guard"); + guard.append("echo GUARD-PASSED\n"); + + Path script = workDir.resolve("entrypoint-guard.sh"); + Files.write(script, guard.toString().getBytes(StandardCharsets.UTF_8)); + return script; + } + + private static Result runLauncher(Path karafHome, String karafScript, Map<String, String> env, String... args) throws Exception { + String[] command = new String[args.length + 2]; + command[0] = "/bin/sh"; + command[1] = karafHome.resolve("bin").resolve(karafScript).toString(); + System.arraycopy(args, 0, command, 2, args.length); + return run(new ProcessBuilder(command).directory(karafHome.toFile()), env); + } + + private static Result runShell(Path script, Path workDir, Map<String, String> env) throws Exception { + return run(new ProcessBuilder("/bin/sh", script.toString()).directory(workDir.toFile()), env); + } + + private static Result run(ProcessBuilder builder, Map<String, String> env) throws Exception { + Map<String, String> environment = builder.environment(); + + // Inherit the JVM's environment - bin/setenv shells out to dirname, sed, grep and tail, which + // do not live under /usr/bin:/bin on every platform (NixOS, minimal images), and a hardcoded + // PATH turns that into a confusing failure instead of a working test. + // + // But strip every UNOMI_* variable first: an ambient UNOMI_ROOT_PASSWORD on a developer's + // machine would otherwise satisfy the "missing password" cases and make them pass by accident, + // which is the one property these tests cannot afford to lose. KARAF_* goes too, because + // KARAF_ETC would redirect the script at the developer's own etc/ directory. + List<String> inherited = new ArrayList<>(environment.keySet()); + for (String name : inherited) { + if (name.startsWith("UNOMI_") || name.startsWith("KARAF_")) { + environment.remove(name); + } + } + environment.putIfAbsent("PATH", "/usr/bin:/bin"); + environment.putAll(env); + builder.redirectErrorStream(true); + + Process process = builder.start(); + String output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8); + assertTrue(process.waitFor(60, TimeUnit.SECONDS), "script did not terminate"); + return new Result(process.exitValue(), output); + } + + private static final class Result { + private final int exitCode; + private final String output; + + private Result(int exitCode, String output) { + this.exitCode = exitCode; + this.output = output; + } + } +} diff --git a/setup-elasticsearch.sh b/setup-elasticsearch.sh index 8c103668f..f794801ce 100755 --- a/setup-elasticsearch.sh +++ b/setup-elasticsearch.sh @@ -56,17 +56,15 @@ _setup_elasticsearch() { return 1 fi - # Load only the Elasticsearch password from .env.local if it exists - # This ensures we don't load the OpenSearch password - if ! load_password_from_env_local "${SCRIPT_DIR}" "UNOMI_ELASTICSEARCH_PASSWORD"; then - # If not found in .env.local, check if it's already set in environment - if [ -z "${UNOMI_ELASTICSEARCH_PASSWORD}" ]; then - echo "Note: UNOMI_ELASTICSEARCH_PASSWORD not found in .env.local or environment" - fi + # Load required passwords from .env.local (or existing environment). + # Engine password is Elasticsearch-only so we do not pull OpenSearch's password. + if ! require_password "${SCRIPT_DIR}" "UNOMI_ELASTICSEARCH_PASSWORD"; then + return 1 fi - - # Check if password is set - if ! check_password "UNOMI_ELASTICSEARCH_PASSWORD"; then + if ! require_password "${SCRIPT_DIR}" "UNOMI_ROOT_PASSWORD"; then + return 1 + fi + if ! require_password "${SCRIPT_DIR}" "UNOMI_HEALTHCHECK_PASSWORD"; then return 1 fi @@ -76,7 +74,6 @@ _setup_elasticsearch() { export UNOMI_ELASTICSEARCH_USERNAME=elastic export UNOMI_ELASTICSEARCH_SSL_ENABLE=true export UNOMI_ELASTICSEARCH_SSL_TRUST_ALL_CERTIFICATES=true - # Password is already set from .env.local or environment # Set the distribution to use Elasticsearch (default, but explicit for clarity) export UNOMI_DISTRIBUTION=unomi-distribution-elasticsearch @@ -87,6 +84,9 @@ _setup_elasticsearch() { echo " Username: ${UNOMI_ELASTICSEARCH_USERNAME} (overridden from default: empty)" echo " SSL Enabled: ${UNOMI_ELASTICSEARCH_SSL_ENABLE} (overridden from default: false)" echo " Trust All Certificates: ${UNOMI_ELASTICSEARCH_SSL_TRUST_ALL_CERTIFICATES} (overridden from default: false)" + echo " Elasticsearch password: (set from .env.local or environment)" + echo " Root password: (set from .env.local or environment)" + echo " Healthcheck password: (set from .env.local or environment)" return 0 } diff --git a/setup-opensearch.sh b/setup-opensearch.sh index 565dcc4e9..995ad1b2a 100755 --- a/setup-opensearch.sh +++ b/setup-opensearch.sh @@ -56,30 +56,26 @@ _setup_opensearch() { return 1 fi - # Load only the OpenSearch password from .env.local if it exists - # This ensures we don't load the Elasticsearch password - if ! load_password_from_env_local "${SCRIPT_DIR}" "UNOMI_OPENSEARCH_PASSWORD"; then - # If not found in .env.local, check if it's already set in environment - if [ -z "${UNOMI_OPENSEARCH_PASSWORD}" ]; then - echo "Note: UNOMI_OPENSEARCH_PASSWORD not found in .env.local or environment" - fi + # Load required passwords from .env.local (or existing environment). + # Engine password is OpenSearch-only so we do not pull Elasticsearch's password. + if ! require_password "${SCRIPT_DIR}" "UNOMI_OPENSEARCH_PASSWORD"; then + return 1 fi - - # Check if password is set - if ! check_password "UNOMI_OPENSEARCH_PASSWORD"; then + if ! require_password "${SCRIPT_DIR}" "UNOMI_ROOT_PASSWORD"; then + return 1 + fi + if ! require_password "${SCRIPT_DIR}" "UNOMI_HEALTHCHECK_PASSWORD"; then return 1 fi - - # Set OpenSearch 3 password (only override needed - defaults are appropriate for OpenSearch 3) - # Password is already set from .env.local or environment, just ensure it's exported - export UNOMI_OPENSEARCH_PASSWORD # Set the distribution to use OpenSearch export UNOMI_DISTRIBUTION=unomi-distribution-opensearch echo "OpenSearch 3 environment variables configured." echo " Distribution: ${UNOMI_DISTRIBUTION}" - echo " Password: (set from .env.local or environment)" + echo " OpenSearch password: (set from .env.local or environment)" + echo " Root password: (set from .env.local or environment)" + echo " Healthcheck password: (set from .env.local or environment)" echo " Note: Using Unomi defaults for other OpenSearch settings (cluster, addresses, username, SSL)" return 0 diff --git a/setup-utils.sh b/setup-utils.sh index 6d8f04338..fd82fb0c2 100755 --- a/setup-utils.sh +++ b/setup-utils.sh @@ -139,6 +139,33 @@ check_password() { return 0 } +# Load a password from .env.local (if present) and require it to be set. +# Usage: require_password SCRIPT_DIR PASSWORD_VAR_NAME +# Returns: 0 if set, 1 if missing +require_password() { + local script_dir="$1" + local password_var="$2" + + if ! load_password_from_env_local "${script_dir}" "${password_var}"; then + if [ -n "${ZSH_VERSION}" ]; then + local password_value="${(P)password_var}" + else + local password_value="${!password_var}" + fi + if [ -z "${password_value}" ]; then + echo "Note: ${password_var} not found in .env.local or environment" + fi + fi + + if ! check_password "${password_var}"; then + return 1 + fi + + # Ensure the variable is exported into the current shell + eval "export ${password_var}" + return 0 +} + # Clear the opposite search engine's environment variables # Usage: clear_opposite SCRIPT_DIR OPPOSITE_TYPE # OPPOSITE_TYPE should be "opensearch" or "elasticsearch"
