2026-10-06 23:57:57 +00:00
6 changed files with 266 additions and 15 deletions
@@ -137,13 +137,13 @@ Without `schedulerName` the processes exist but nothing runs them; the QBit logs
 | Field | Default | Description |
 |---|---|---|
 | `backendName` | required | QQQ backend for the operational tables |
-| `opensearchUrl` | | `https://host[:port]`; preferred over `opensearchHost`, `opensearchPort`, `useSsl` |
+| `opensearchUrl` | | `https://host[:port]`; preferred over `opensearchHost`, `opensearchPort`, `useSsl`; credentials in the URL are rejected |
 | `opensearchHost`, `opensearchPort`, `useSsl` | `useSsl=false` | Legacy connection fields |
 | `opensearchIndexName` | required | Alias name owned by this QBit; lowercase, OpenSearch naming rules |
 | `authMode` | inferred | `NONE`, `BASIC` (inferred when credentials are set), `AWS_SIGV4` |
-| `opensearchUsername`, `opensearchPassword` | | BASIC credentials; `${env.X}` references recommended; refused over plain HTTP unless `allowPlaintextCredentials` |
+| `opensearchUsername`, `opensearchPassword` | | BASIC credentials; `${env.X}` references recommended (an empty value counts as missing); refused over plain HTTP unless `allowPlaintextCredentials` |
 | `awsRegion`, `awsServiceName`, `awsAssumeRoleArn` | region from `AWS_REGION`; `es` | AWS_SIGV4 settings |
-| `tls` | JVM defaults | `caCertificatePath` or `trustStorePath`/`trustStorePassword`/`trustStoreType`, `keyStorePath`/`keyStorePassword`/`keyStoreType` (mTLS), `hostnameVerification`, `insecureSkipVerify` (loopback only unless `allowInsecureInProduction`) |
+| `tls` | JVM defaults | `caCertificatePath` or `trustStorePath`/`trustStorePassword`/`trustStoreType`, `keyStorePath`/`keyStorePassword`/`keyStoreType` (mTLS), `hostnameVerification` (`false` logs a warning), `insecureSkipVerify` (loopback only unless `allowInsecureInProduction`) |
 | `connectTimeoutMillis`, `responseTimeoutMillis`, `maxConnections` | 5000, 60000, 30 | Transport settings |
 | `transportCustomizer` | | `QCodeReference` to an `OpenSearchTransportCustomizer` (bearer tokens, API keys, interceptors) |
 | `startupMode` | `FAIL_FAST` | `FAIL_FAST` fails `produce()` when the cluster is unreachable; `DEGRADED` boots and retries on first use |
@@ -166,6 +166,10 @@ public class QuickSearchQBitConfig implements QBitConfig
            {
               errors.add("opensearchUrl must include a host");
            }
+            if(uri.getRawUserInfo() != null)
+            {
+               errors.add("opensearchUrl must not contain credentials; use opensearchUsername and opensearchPassword");
+            }
         }
         catch(Exception e)
         {
@@ -29,9 +29,12 @@ import com.fasterxml.jackson.annotation.JsonIgnore;
 ** supply keyStorePath for client-certificate (mTLS) authentication.
 ** insecureSkipVerify disables certificate and hostname checks and is refused
 ** for non-loopback hosts unless allowInsecureInProduction is also set.
+ ** hostnameVerification=false keeps certificate checks but skips the host name
+ ** check; it is allowed for any host and logs a warning.
 **
 ** Path and password values may be ${env.X} / ${prop.X} references; they are
- ** interpreted when the transport is built, never stored resolved.
+ ** interpreted when the transport is built, never stored resolved. A path that
+ ** resolves to an empty value is treated as missing.
 *******************************************************************************/
 public class QuickSearchTlsConfig implements Serializable
 {
@@ -23,6 +23,7 @@ import java.security.KeyStore;
 import java.security.cert.Certificate;
 import java.security.cert.CertificateFactory;
 import java.util.Collection;
+import java.util.regex.Pattern;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLContext;
 import com.fasterxml.jackson.databind.ObjectMapper;
@@ -78,6 +79,12 @@ public class OpenSearchTransportFactory

   private static final String USER_AGENT = "qbit-quick-search/" + QuickSearchQBitProducer.getVersion();

+   ///////////////////////////////////////////////////////////////////////////
+   // Only a lone ${env.X} or ${prop.X} is echoed in errors: a ?? chain may //
+   // hold a literal fallback secret between its references.                //
+   ///////////////////////////////////////////////////////////////////////////
+   private static final Pattern SINGLE_REFERENCE = Pattern.compile("^\\$\\{(env|prop)\\.[A-Za-z0-9_.-]+\\}$");
+


   /*******************************************************************************
@@ -132,12 +139,8 @@ public class OpenSearchTransportFactory
      BasicCredentialsProvider credentialsProvider = null;
      if(config.getEffectiveAuthMode() == QuickSearchAuthMode.BASIC)
      {
-         String username = interpreter.interpret(config.getOpensearchUsername());
-         String password = interpreter.interpret(config.getOpensearchPassword());
-         if(username == null || password == null)
-         {
-            throw (new QException("BASIC authentication is configured but the username or password resolved to null"));
-         }
+         String username = requireResolved(interpreter, config.getOpensearchUsername(), "opensearchUsername");
+         String password = requireResolved(interpreter, config.getOpensearchPassword(), "opensearchPassword");
         credentialsProvider = new BasicCredentialsProvider();
         credentialsProvider.setCredentials(new AuthScope(httpHost), new UsernamePasswordCredentials(username, password.toCharArray()));
      }
@@ -262,14 +265,14 @@ public class OpenSearchTransportFactory
      }
      else if(StringUtils.hasContent(tls.getCaCertificatePath()))
      {
-         builder.loadTrustMaterial(loadPemTrustStore(interpreter.interpret(tls.getCaCertificatePath())), null);
+         builder.loadTrustMaterial(loadPemTrustStore(requireResolved(interpreter, tls.getCaCertificatePath(), "tls.caCertificatePath")), null);
         custom = true;
      }
      else if(StringUtils.hasContent(tls.getTrustStorePath()))
      {
         String   password   = interpreter.interpret(tls.getTrustStorePassword());
         KeyStore trustStore = KeyStore.getInstance(StringUtils.hasContent(tls.getTrustStoreType()) ? tls.getTrustStoreType() : KeyStore.getDefaultType());
-         try(InputStream inputStream = new FileInputStream(interpreter.interpret(tls.getTrustStorePath())))
+         try(InputStream inputStream = new FileInputStream(requireResolved(interpreter, tls.getTrustStorePath(), "tls.trustStorePath")))
         {
            trustStore.load(inputStream, password == null ? null : password.toCharArray());
         }
@@ -282,7 +285,7 @@ public class OpenSearchTransportFactory
         String   password = interpreter.interpret(tls.getKeyStorePassword());
         char[]   chars    = password == null ? new char[0] : password.toCharArray();
         KeyStore keyStore = KeyStore.getInstance(StringUtils.hasContent(tls.getKeyStoreType()) ? tls.getKeyStoreType() : KeyStore.getDefaultType());
-         try(InputStream inputStream = new FileInputStream(interpreter.interpret(tls.getKeyStorePath())))
+         try(InputStream inputStream = new FileInputStream(requireResolved(interpreter, tls.getKeyStorePath(), "tls.keyStorePath")))
         {
            keyStore.load(inputStream, chars);
         }
@@ -296,19 +299,53 @@ public class OpenSearchTransportFactory


   /*******************************************************************************
-    **
+    ** The no-op verifier when insecureSkipVerify (warned in buildSslContext) or
+    ** hostnameVerification=false is set, else null for the default verifier.
    *******************************************************************************/
   static HostnameVerifier buildHostnameVerifier(QuickSearchTlsConfig tls)
   {
-      if(tls != null && (Boolean.TRUE.equals(tls.getInsecureSkipVerify()) || Boolean.FALSE.equals(tls.getHostnameVerification())))
+      if(tls == null)
+      {
+         return (null);
+      }
+
+      if(Boolean.TRUE.equals(tls.getInsecureSkipVerify()))
      {
         return (NoopHostnameVerifier.INSTANCE);
      }
+
+      if(Boolean.FALSE.equals(tls.getHostnameVerification()))
+      {
+         LOG.warn("tls.hostnameVerification is disabled: OpenSearch server certificates are NOT checked against the host name");
+         return (NoopHostnameVerifier.INSTANCE);
+      }
+
      return (null);
   }



+   /*******************************************************************************
+    ** Interpret a value that must resolve to content; null, empty and blank all
+    ** count as missing. The error names the field and, for a single ${env.X} or
+    ** ${prop.X} reference, the reference itself; never a resolved value and
+    ** never any part of a ?? fallback chain.
+    *******************************************************************************/
+   static String requireResolved(QMetaDataVariableInterpreter interpreter, String value, String fieldName) throws QException
+   {
+      String resolved = interpreter.interpret(value);
+      if(StringUtils.hasContent(resolved))
+      {
+         return (resolved);
+      }
+
+      String  trimmed     = value == null ? "" : value.trim();
+      boolean isReference = SINGLE_REFERENCE.matcher(trimmed).matches();
+      throw (new QException(fieldName + " is missing or empty" + (isReference ? " after resolving " + trimmed : "")));
+   }
+
+
+
   /*******************************************************************************
    ** Load one or more PEM certificates into an in-memory trust store.
    *******************************************************************************/
@@ -108,6 +108,18 @@ class QuickSearchQBitConfigValidationTest



+   @Test
+   void testUrl_withUserInfo_isRejectedWithoutEchoingIt()
+   {
+      List<String> errors = validate(baseConfig().withOpensearchHost(null).withOpensearchPort(null).withOpensearchUrl("https://admin:s3cret@search.example.com"));
+      assertThat(errors).anyMatch(e -> e.contains("opensearchUrl must not contain credentials"));
+      assertThat(errors).noneMatch(e -> e.contains("s3cret") || e.contains("admin"));
+
+      assertThat(validate(baseConfig().withOpensearchUrl("https://admin@search.example.com"))).anyMatch(e -> e.contains("opensearchUrl must not contain credentials"));
+   }
+
+
+
   @Test
   void testAuthMode_inferredFromCredentials()
   {
@@ -0,0 +1,195 @@
+/*
+ * Copyright 2024 Kingsrook, LLC
+ *
+ * Licensed 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 com.kingsrook.qbits.quicksearch.opensearch;
+
+
+import com.kingsrook.qqq.backend.core.exceptions.QException;
+import com.kingsrook.qqq.backend.core.logging.QCollectingLogger;
+import com.kingsrook.qqq.backend.core.logging.QLogger;
+import com.kingsrook.qbits.quicksearch.QuickSearchAuthMode;
+import com.kingsrook.qbits.quicksearch.QuickSearchQBitConfig;
+import com.kingsrook.qbits.quicksearch.QuickSearchTlsConfig;
+import org.apache.hc.client5.http.ssl.NoopHostnameVerifier;
+import org.apache.logging.log4j.Level;
+import org.opensearch.client.transport.OpenSearchTransport;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+
+/*******************************************************************************
+ ** Unit tests for OpenSearchTransportFactory: secret and path references that
+ ** resolve to nothing, and the hostname-verification warning.
+ *******************************************************************************/
+class OpenSearchTransportFactoryTest
+{
+   private static final String EMPTY_PROPERTY    = "quickSearch.test.emptyValue";
+   private static final String USER_PROPERTY     = "quickSearch.test.userValue";
+   private static final String PASSWORD_PROPERTY = "quickSearch.test.passwordValue";
+   private static final String UNSET_ENV         = "${env.QUICK_SEARCH_TEST_UNSET_VARIABLE_15}";
+
+
+
+   /*******************************************************************************
+    **
+    *******************************************************************************/
+   @AfterEach
+   void afterEach()
+   {
+      System.clearProperty(EMPTY_PROPERTY);
+      System.clearProperty(USER_PROPERTY);
+      System.clearProperty(PASSWORD_PROPERTY);
+      QLogger.deactivateCollectingLoggerForClass(OpenSearchTransportFactory.class);
+   }
+
+
+
+   /*******************************************************************************
+    **
+    *******************************************************************************/
+   private QuickSearchQBitConfig baseConfig()
+   {
+      return (new QuickSearchQBitConfig()
+         .withOpensearchUrl("https://localhost:9200")
+         .withOpensearchIndexName("app-search"));
+   }
+
+
+
+   @Test
+   void testBasic_emptyPasswordReference_namesFieldNotSecrets()
+   {
+      System.setProperty(USER_PROPERTY, "secret-user-value");
+      System.setProperty(EMPTY_PROPERTY, "");
+
+      QuickSearchQBitConfig config = baseConfig()
+         .withAuthMode(QuickSearchAuthMode.BASIC)
+         .withOpensearchUsername("${prop." + USER_PROPERTY + "}")
+         .withOpensearchPassword("${prop." + EMPTY_PROPERTY + "}");
+
+      assertThatThrownBy(() -> OpenSearchTransportFactory.build(config))
+         .isInstanceOf(QException.class)
+         .hasMessageContaining("opensearchPassword")
+         .hasMessageContaining("${prop." + EMPTY_PROPERTY + "}")
+         .hasMessageNotContaining("secret-user-value");
+   }
+
+
+
+   @Test
+   void testBasic_blankOrUnsetReference_isTreatedAsMissing()
+   {
+      System.setProperty(EMPTY_PROPERTY, "   ");
+
+      QuickSearchQBitConfig blankPassword = baseConfig()
+         .withAuthMode(QuickSearchAuthMode.BASIC)
+         .withOpensearchUsername("admin")
+         .withOpensearchPassword("${prop." + EMPTY_PROPERTY + "}");
+      assertThatThrownBy(() -> OpenSearchTransportFactory.build(blankPassword))
+         .isInstanceOf(QException.class)
+         .hasMessageContaining("opensearchPassword");
+
+      QuickSearchQBitConfig unsetUsername = baseConfig()
+         .withAuthMode(QuickSearchAuthMode.BASIC)
+         .withOpensearchUsername(UNSET_ENV)
+         .withOpensearchPassword("not-in-the-message");
+      assertThatThrownBy(() -> OpenSearchTransportFactory.build(unsetUsername))
+         .isInstanceOf(QException.class)
+         .hasMessageContaining("opensearchUsername")
+         .hasMessageContaining(UNSET_ENV)
+         .hasMessageNotContaining("not-in-the-message");
+   }
+
+
+
+   @Test
+   void testBasic_unresolvedFallbackChain_neverEchoesTheLiteral()
+   {
+      QuickSearchQBitConfig config = baseConfig()
+         .withAuthMode(QuickSearchAuthMode.BASIC)
+         .withOpensearchUsername("admin")
+         .withOpensearchPassword(UNSET_ENV + "??fallback-secret-literal??${env.QUICK_SEARCH_TEST_UNSET_VARIABLE_15B}");
+
+      assertThatThrownBy(() -> OpenSearchTransportFactory.build(config))
+         .isInstanceOf(QException.class)
+         .hasMessageContaining("opensearchPassword")
+         .hasMessageNotContaining("fallback-secret-literal");
+   }
+
+
+
+   @Test
+   void testBasic_resolvedReferences_buildTransport() throws Exception
+   {
+      System.setProperty(USER_PROPERTY, "admin");
+      System.setProperty(PASSWORD_PROPERTY, "admin-password");
+
+      QuickSearchQBitConfig config = baseConfig()
+         .withAuthMode(QuickSearchAuthMode.BASIC)
+         .withOpensearchUsername("${prop." + USER_PROPERTY + "}")
+         .withOpensearchPassword("${prop." + PASSWORD_PROPERTY + "}");
+
+      try(OpenSearchTransport transport = OpenSearchTransportFactory.build(config))
+      {
+         assertThat(transport).isNotNull();
+      }
+   }
+
+
+
+   @Test
+   void testTls_unresolvedPaths_nameTheField()
+   {
+      System.setProperty(EMPTY_PROPERTY, "");
+
+      assertPathError(new QuickSearchTlsConfig().withCaCertificatePath(UNSET_ENV), "tls.caCertificatePath", UNSET_ENV);
+      assertPathError(new QuickSearchTlsConfig().withTrustStorePath(UNSET_ENV), "tls.trustStorePath", UNSET_ENV);
+      assertPathError(new QuickSearchTlsConfig().withKeyStorePath("${prop." + EMPTY_PROPERTY + "}"), "tls.keyStorePath", "${prop." + EMPTY_PROPERTY + "}");
+   }
+
+
+
+   /*******************************************************************************
+    **
+    *******************************************************************************/
+   private void assertPathError(QuickSearchTlsConfig tls, String fieldName, String reference)
+   {
+      assertThatThrownBy(() -> OpenSearchTransportFactory.build(baseConfig().withTls(tls)))
+         .isInstanceOf(QException.class)
+         .hasMessageContaining(fieldName)
+         .hasMessageContaining(reference)
+         .hasMessageNotContaining(": null");
+   }
+
+
+
+   @Test
+   void testHostnameVerificationDisabled_logsWarning()
+   {
+      QCollectingLogger collectingLogger = QLogger.activateCollectingLoggerForClass(OpenSearchTransportFactory.class);
+
+      assertThat(OpenSearchTransportFactory.buildHostnameVerifier(new QuickSearchTlsConfig().withHostnameVerification(false))).isSameAs(NoopHostnameVerifier.INSTANCE);
+      assertThat(collectingLogger.getCollectedMessages()).anyMatch(m -> Level.WARN.equals(m.getLevel()) && m.getMessage().contains("tls.hostnameVerification"));
+
+      collectingLogger.clear();
+      assertThat(OpenSearchTransportFactory.buildHostnameVerifier(new QuickSearchTlsConfig())).isNull();
+      assertThat(OpenSearchTransportFactory.buildHostnameVerifier(null)).isNull();
+      assertThat(collectingLogger.getCollectedMessages()).isEmpty();
+   }
+
+}