From bc940cb3904f2b8c681ded2ca2c8ecf4db9fabe0 Mon Sep 17 00:00:00 2001 From: Craig <3979063+craig8@users.noreply.github.com> Date: Fri, 10 Jul 2026 11:01:06 -0700 Subject: [PATCH 1/2] fix: honor configured broker manager credential instead of hardcoded system/manager GridOpticsServer read goss.system.manager and goss.system.manager.password via getProperty() but discarded the results, always connecting to the broker as the hardcoded system/manager. The configured credential is now stored and used at createConnection(). Absent or empty config falls back to system/manager via shared DEFAULT_SYSTEM_MANAGER_USER/PASSWORD constants, so deployments that never set these properties are unchanged. Connect-failure log raised to warn so a bad configured credential is visible without debug logging. Behavior change: a deployment that set these properties (previously ignored) now has them take effect. Tests cover configured-honored, default-when-absent, and default-when-empty. --- .../core/server/impl/GridOpticsServer.java | 22 +++-- .../GridOpticsServerBrokerCredentialTest.java | 96 +++++++++++++++++++ 2 files changed, 112 insertions(+), 6 deletions(-) create mode 100644 pnnl.goss.core/test/pnnl/goss/core/server/impl/test/GridOpticsServerBrokerCredentialTest.java diff --git a/pnnl.goss.core/src/pnnl/goss/core/server/impl/GridOpticsServer.java b/pnnl.goss.core/src/pnnl/goss/core/server/impl/GridOpticsServer.java index fe1657e2..cb0978b4 100644 --- a/pnnl.goss.core/src/pnnl/goss/core/server/impl/GridOpticsServer.java +++ b/pnnl.goss.core/src/pnnl/goss/core/server/impl/GridOpticsServer.java @@ -127,6 +127,9 @@ public class GridOpticsServer implements ServerControl { private static final String PROP_SYSTEM_MANAGER = "goss.system.manager"; private static final String PROP_SYSTEM_MANAGER_PASSWORD = "goss.system.manager.password"; + private static final String DEFAULT_SYSTEM_MANAGER_USER = "system"; + private static final String DEFAULT_SYSTEM_MANAGER_PASSWORD = "manager"; + private BrokerService broker; private Connection connection; private Session session; @@ -147,6 +150,13 @@ public class GridOpticsServer implements ServerControl { // Topic to listen on for receiving requests private String requestQueue = null; + // The broker connection principal used at createConnection(...). Defaults to + // system/manager (GADP-014 / issue #42) so a deployment that never sets + // goss.system.manager[.password] in pnnl.goss.core.server.cfg keeps today's + // behavior unchanged. + private String systemManagerUser = DEFAULT_SYSTEM_MANAGER_USER; + private String systemManagerPassword = DEFAULT_SYSTEM_MANAGER_PASSWORD; + // SSL Parameters private boolean sslEnabled = false; private String sslClientKeyStore = null; @@ -223,10 +233,10 @@ public synchronized void updated(Map properties) throws SystemEx if (properties != null) { - getProperty((String) properties.get(PROP_SYSTEM_MANAGER), - "system"); - getProperty((String) properties.get(PROP_SYSTEM_MANAGER_PASSWORD), - "manager"); + systemManagerUser = getProperty((String) properties.get(PROP_SYSTEM_MANAGER), + DEFAULT_SYSTEM_MANAGER_USER); + systemManagerPassword = getProperty((String) properties.get(PROP_SYSTEM_MANAGER_PASSWORD), + DEFAULT_SYSTEM_MANAGER_PASSWORD); shouldStartBroker = Boolean.parseBoolean( getProperty((String) properties.get(PROP_START_BROKER), "true")); @@ -449,10 +459,10 @@ public void start(Map properties) { connectionFactory = new ActiveMQConnectionFactory(openwireTransport); } - connection = connectionFactory.createConnection("system", "manager"); + connection = connectionFactory.createConnection(systemManagerUser, systemManagerPassword); connection.start(); } catch (Exception e) { - log.debug("Error Connecting to ActiveMQ", e); + log.warn("Error Connecting to ActiveMQ", e); if (shouldStartBroker) { try { if (broker != null) { diff --git a/pnnl.goss.core/test/pnnl/goss/core/server/impl/test/GridOpticsServerBrokerCredentialTest.java b/pnnl.goss.core/test/pnnl/goss/core/server/impl/test/GridOpticsServerBrokerCredentialTest.java new file mode 100644 index 00000000..7ed6f2bd --- /dev/null +++ b/pnnl.goss.core/test/pnnl/goss/core/server/impl/test/GridOpticsServerBrokerCredentialTest.java @@ -0,0 +1,96 @@ +package pnnl.goss.core.server.impl.test; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.lang.reflect.Field; +import java.util.HashMap; +import java.util.Map; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import pnnl.goss.core.server.impl.GridOpticsServer; + +/** + * GADP-014 / issue #42: the configured broker system-manager credential + * (goss.system.manager / goss.system.manager.password) was read via + * getProperty(...) but the returned values were discarded, so + * createConnection(...) always used the literal "system"/"manager" regardless + * of configuration. + * + * These tests assert the fix at the field level that createConnection(...) + * reads from: systemManagerUser / systemManagerPassword. GridOpticsServer has + * no OSGi container in a unit test, so updated(Map) is invoked directly (the + * same method @Activate's start(Map) delegates to before createConnection), and + * the resulting private fields are read via reflection since the class exposes + * no getters for them. + */ +public class GridOpticsServerBrokerCredentialTest { + + private static final String PROP_SYSTEM_MANAGER = "goss.system.manager"; + private static final String PROP_SYSTEM_MANAGER_PASSWORD = "goss.system.manager.password"; + + private String readSystemManagerUser(GridOpticsServer server) throws Exception { + return readPrivateField(server, "systemManagerUser"); + } + + private String readSystemManagerPassword(GridOpticsServer server) throws Exception { + return readPrivateField(server, "systemManagerPassword"); + } + + private String readPrivateField(GridOpticsServer server, String fieldName) throws Exception { + Field field = GridOpticsServer.class.getDeclaredField(fieldName); + field.setAccessible(true); + return (String) field.get(server); + } + + @Test + @DisplayName("Configured goss.system.manager / .password are honored when set") + public void configuredCredentialIsHonoredWhenSet() throws Exception { + GridOpticsServer server = new GridOpticsServer(); + Map properties = new HashMap<>(); + properties.put(PROP_SYSTEM_MANAGER, "configuredUser"); + properties.put(PROP_SYSTEM_MANAGER_PASSWORD, "configuredPassword"); + + server.updated(properties); + + assertThat(readSystemManagerUser(server)) + .as("the configured goss.system.manager value must reach the connection principal") + .isEqualTo("configuredUser"); + assertThat(readSystemManagerPassword(server)) + .as("the configured goss.system.manager.password value must reach the connection principal") + .isEqualTo("configuredPassword"); + } + + @Test + @DisplayName("system/manager defaults are used when the credential properties are absent") + public void defaultsToSystemManagerWhenPropertiesAbsent() throws Exception { + GridOpticsServer server = new GridOpticsServer(); + Map properties = new HashMap<>(); + // Deliberately omit PROP_SYSTEM_MANAGER / PROP_SYSTEM_MANAGER_PASSWORD: + // a deployment that never sets these must see unchanged behavior. + + server.updated(properties); + + assertThat(readSystemManagerUser(server)) + .as("absent goss.system.manager must default to 'system', preserving prior behavior") + .isEqualTo("system"); + assertThat(readSystemManagerPassword(server)) + .as("absent goss.system.manager.password must default to 'manager', preserving prior behavior") + .isEqualTo("manager"); + } + + @Test + @DisplayName("empty-string credential properties fall back to the system/manager default") + public void defaultsToSystemManagerWhenPropertiesEmpty() throws Exception { + GridOpticsServer server = new GridOpticsServer(); + Map properties = new HashMap<>(); + properties.put(PROP_SYSTEM_MANAGER, ""); + properties.put(PROP_SYSTEM_MANAGER_PASSWORD, ""); + + server.updated(properties); + + assertThat(readSystemManagerUser(server)).isEqualTo("system"); + assertThat(readSystemManagerPassword(server)).isEqualTo("manager"); + } +} From 7706b5c0d44ae4be8e418065979f85a5372d4b80 Mon Sep 17 00:00:00 2001 From: Craig <3979063+craig8@users.noreply.github.com> Date: Fri, 10 Jul 2026 11:01:06 -0700 Subject: [PATCH 2/2] fix: correct broker manager password typo in runner config (managera to manager) pnnl.goss.core.server.cfg had goss.system.manager.password = managera, which did not match the realm account (system=manager). This was masked while the server hardcoded the credential; once the server honors the configured value, the typo would break broker auth. Verified manager against the realm account in propertyfile.cfg, GossAuthorizingRealm, and SystemBasedRealm. --- pnnl.goss.core.runner/conf/pnnl.goss.core.server.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pnnl.goss.core.runner/conf/pnnl.goss.core.server.cfg b/pnnl.goss.core.runner/conf/pnnl.goss.core.server.cfg index ae9b0ee9..aca1668f 100644 --- a/pnnl.goss.core.runner/conf/pnnl.goss.core.server.cfg +++ b/pnnl.goss.core.runner/conf/pnnl.goss.core.server.cfg @@ -6,7 +6,7 @@ # This user is equivelant to root. Must have '*' # permissions on the message bus. goss.system.manager = system -goss.system.manager.password = managera +goss.system.manager.password = manager # The following are used for the core-client connection. goss.openwire.uri = tcp://${activemq.host}:${openwire.port}