diff --git a/pnnl.goss.core/src/pnnl/goss/core/security/impl/Activator.java b/pnnl.goss.core/src/pnnl/goss/core/security/impl/Activator.java index 73f0c293..133a0e25 100644 --- a/pnnl.goss.core/src/pnnl/goss/core/security/impl/Activator.java +++ b/pnnl.goss.core/src/pnnl/goss/core/security/impl/Activator.java @@ -115,11 +115,34 @@ public void realmRemoved(ServiceReference ref) { } } + /** + * Writes the collected realms onto the SecurityManager. + * + * Shiro's RealmSecurityManager.setRealms() rejects an empty collection with + * IllegalArgumentException, so removing the LAST realm used to throw out of + * realmRemoved() and DS logged "The realmRemoved method has thrown an + * exception" on every teardown. Shiro offers no way to install an empty realm + * set, so the guard declines the write and logs it. + * + * Declining does not widen the authorization surface. The realm reference is + * AT_LEAST_ONE, so losing the last realm makes this component unsatisfied and + * DS deactivates it; the SecurityManager service is unpublished with it, and + * GridOpticsServer's mandatory reference on that service goes with it. The + * throwing version left exactly the same stale realm set behind, because it + * threw before assigning, so this is strictly the same state without the + * spurious error. + */ private synchronized void registerAllRealms() { Set realms = new HashSet<>(); for (GossRealm r : realmMap.values()) { realms.add((Realm) r); } + if (realms.isEmpty()) { + log.warn("No GossRealm services remain; leaving the previously registered realm set in place. " + + "Shiro rejects an empty realm set, and this component is about to be deactivated by DS " + + "because its realm reference is AT_LEAST_ONE."); + return; + } setRealms(realms); log.info("Registered {} realms with SecurityManager: {}", realms.size(), realms.stream().map(r -> r.getClass().getSimpleName()).toList()); diff --git a/pnnl.goss.core/src/pnnl/goss/core/security/system/SystemBasedRealm.java b/pnnl.goss.core/src/pnnl/goss/core/security/system/SystemBasedRealm.java index 1aa6da87..1df5d27b 100644 --- a/pnnl.goss.core/src/pnnl/goss/core/security/system/SystemBasedRealm.java +++ b/pnnl.goss.core/src/pnnl/goss/core/security/system/SystemBasedRealm.java @@ -19,6 +19,9 @@ import org.osgi.service.component.annotations.ConfigurationPolicy; import org.osgi.service.component.annotations.Modified; import org.osgi.service.component.annotations.Reference; +import org.osgi.service.component.annotations.ReferenceCardinality; +import org.osgi.service.component.annotations.ReferencePolicy; +import org.osgi.service.component.annotations.ReferencePolicyOption; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -47,6 +50,20 @@ * AT_LEAST_ONE realm guard let GridOpticsServer connect as system/manager * before this realm was wired. * + * LIFECYCLE (GOSS-025): the account map is loaded from {@link #activate}, which + * Declarative Services is guaranteed to call. It used to be loaded only from + * Shiro's {@code onInit()} hook, which is unreachable in this deployment: + * {@code onInit()} is protected and called only from + * {@code Initializable.init()}, normally driven by + * {@code LifecycleUtils.init()} out of an ini/Spring bootstrap. Nothing in + * goss-core, in Felix DS, or in shiro-core 2.0.0's own wiring calls it for a + * DS-managed realm: {@code RealmSecurityManager.setRealms()} reaches only + * {@code afterRealmsSet() -> applyCacheManagerToRealms()}. The realm therefore + * activated with a permanently empty map and the broker connect in + * GridOpticsServer.start() failed with UnknownAccountException for "system". + * {@code onInit()} is retained and delegates to the same idempotent loader, so + * the realm stays correct if any future host does drive Shiro's lifecycle. + * * @author Craig Allwardt * */ @@ -56,35 +73,152 @@ public class SystemBasedRealm extends AuthorizingRealm implements GossRealm { private static final Logger log = LoggerFactory.getLogger(SystemBasedRealm.class); + /** + * The permission string granted to the system principal. Consumed verbatim by + * the ActiveMQ Shiro authorization plugin through + * {@link pnnl.goss.core.security.impl.GossWildcardPermissionResolver}; changing + * it changes what the broker connection is allowed to do. + */ + private static final String SYSTEM_PERMISSIONS = "queue:*,topic:*,temp-queue:*,fusion:*:read,fusion:*:write"; + private final Map userMap = new ConcurrentHashMap<>(); private final Map> userPermissions = new ConcurrentHashMap<>(); @Reference GossPermissionResolver gossPermissionResolver; - @Reference private volatile SecurityConfig securityConfig; + /** + * The credential source, bound through methods rather than a field so this + * component is notified when it changes. + * + * The manager user and password do not come from this component's own PID + * (pnnl.goss.core.security.systemrealm carries only load=true); they come from + * the pnnl.goss.security PID by way of SecurityConfigImpl. A FileInstall edit + * to pnnl.goss.security.cfg calls SecurityConfigImpl's modified method, which + * leaves the same service instance registered with updated service properties. + * A field reference would give this component no callback for that, so the + * realm would keep serving a superseded credential. The updated= callback below + * is the DS 1.3+ hook for exactly that case, and the dynamic bind reloads on + * service replacement. + */ + @Reference(cardinality = ReferenceCardinality.MANDATORY, policy = ReferencePolicy.DYNAMIC, policyOption = ReferencePolicyOption.GREEDY, unbind = "unsetSecurityConfig", updated = "updatedSecurityConfig") + public void setSecurityConfig(SecurityConfig securityConfig) { + this.securityConfig = securityConfig; + loadSystemAccount(); + } + + /** + * Called when the bound SecurityConfig's service properties change, which DS + * does after SecurityConfigImpl's modified method runs for a + * pnnl.goss.security.cfg edit. Reloads so a rotated manager credential takes + * effect without a bundle restart. + */ + public void updatedSecurityConfig(SecurityConfig securityConfig) { + this.securityConfig = securityConfig; + loadSystemAccount(); + } + + /** + * Fails closed on unbind: the account map is emptied so this realm cannot keep + * authenticating against a credential source that is gone. The reference is + * mandatory, so DS deactivates this component (and transitively the + * SecurityManager and GridOpticsServer) when no replacement exists. The + * identity check keeps a greedy bind-then-unbind swap from wiping the account + * that the new service just loaded. + */ + public void unsetSecurityConfig(SecurityConfig securityConfig) { + if (this.securityConfig == securityConfig) { + this.securityConfig = null; + clearAccounts(); + log.warn("SecurityConfig unbound from SystemBasedRealm; system account cleared. " + + "This realm cannot authenticate the system principal until a SecurityConfig rebinds."); + } + } + + /** + * Rebuilds the system account from the bound {@link SecurityConfig}. + * + * The map is cleared before it is rebuilt, so a rename of the manager user + * cannot leave the previous principal behind and a failure cannot leave a + * half-built or stale account in place. + * + * @throws IllegalStateException + * when no SecurityConfig is bound or the configured manager + * credential is missing. Thrown rather than logged-and-swallowed so + * the failure is fail-closed: propagated out of activate() it + * aborts DS activation, this realm's service is never published, + * the Activator's mandatory realm.type=system reference stays + * unsatisfied, the SecurityManager service is never published, and + * GridOpticsServer never opens a broker connection against an empty + * realm. + */ + private synchronized void loadSystemAccount() { + clearAccounts(); + + SecurityConfig config = this.securityConfig; + if (config == null) { + throw new IllegalStateException("SystemBasedRealm cannot build the system account: no SecurityConfig " + + "service is bound. Check that pnnl.goss.security.cfg exists and that SecurityConfigImpl " + + "activated."); + } + + String managerUser = config.getManagerUser(); + if (managerUser == null || managerUser.trim().isEmpty()) { + throw new IllegalStateException("SystemBasedRealm cannot build the system account: property " + + "goss.system.manager is missing or blank in pnnl.goss.security.cfg."); + } + + String managerPassword = config.getManagerPassword(); + if (managerPassword == null || managerPassword.trim().isEmpty()) { + throw new IllegalStateException("SystemBasedRealm cannot build the system account for user '" + + managerUser + "': property goss.system.manager.password is missing or blank in " + + "pnnl.goss.security.cfg."); + } + + SimpleAccount account = new SimpleAccount(managerUser, managerPassword, getName()); + account.addStringPermission(SYSTEM_PERMISSIONS); + + Set perms = new HashSet<>(); + perms.add(SYSTEM_PERMISSIONS); + + userMap.put(managerUser, account); + userPermissions.put(managerUser, perms); + + log.info("SystemBasedRealm loaded the system account for manager user '{}'", managerUser); + } + + private void clearAccounts() { + userMap.clear(); + userPermissions.clear(); + } + + /** + * Shiro's Initializable hook. Nothing drives it in this OSGi deployment (see + * the class comment), but it is kept wired to the same idempotent loader so the + * realm is correct under either entry point rather than silently empty under + * one of them. + */ @Override protected void onInit() { super.onInit(); - Set perms = new HashSet<>(); - - SimpleAccount acnt = new SimpleAccount(securityConfig.getManagerUser(), securityConfig.getManagerPassword(), - getName()); - acnt.addStringPermission("queue:*,topic:*,temp-queue:*,fusion:*:read,fusion:*:write"); - perms.add("queue:*,topic:*,temp-queue:*,fusion:*:read,fusion:*:write"); - userMap.put(securityConfig.getManagerUser(), acnt); - userPermissions.put(securityConfig.getManagerUser(), perms); + loadSystemAccount(); } @Activate public void activate(Map properties) { log.info("Activating SystemBasedRealm"); + // The component properties (systemrealm.cfg carries only load=true) are not + // the credential source; the bound SecurityConfig is. Reload anyway so + // activation never depends on bind-callback ordering. + loadSystemAccount(); } @Modified public synchronized void updated(Map properties) { + log.info("Reloading SystemBasedRealm after a configuration update"); + loadSystemAccount(); } @Override diff --git a/pnnl.goss.core/test/pnnl/goss/core/security/impl/test/ActivatorRealmRemovalTest.java b/pnnl.goss.core/test/pnnl/goss/core/security/impl/test/ActivatorRealmRemovalTest.java new file mode 100644 index 00000000..411d67e0 --- /dev/null +++ b/pnnl.goss.core/test/pnnl/goss/core/security/impl/test/ActivatorRealmRemovalTest.java @@ -0,0 +1,126 @@ +package pnnl.goss.core.security.impl.test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.mockito.Mockito.mock; + +import java.util.HashSet; +import java.util.Set; + +import org.apache.shiro.authc.AuthenticationInfo; +import org.apache.shiro.authc.AuthenticationToken; +import org.apache.shiro.authc.SimpleAccount; +import org.apache.shiro.authc.UsernamePasswordToken; +import org.apache.shiro.authz.AuthorizationInfo; +import org.apache.shiro.mgt.RealmSecurityManager; +import org.apache.shiro.realm.AuthorizingRealm; +import org.apache.shiro.subject.PrincipalCollection; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; +import org.osgi.framework.ServiceReference; + +import pnnl.goss.core.security.GossRealm; +import pnnl.goss.core.security.impl.Activator; + +/** + * Covers the shutdown-path defect seen alongside GOSS-025: removing the last + * GossRealm made registerAllRealms() call Shiro's + * RealmSecurityManager.setRealms() with an empty collection, which throws + * IllegalArgumentException("Realms collection argument cannot be empty."). DS + * reported it as "The realmRemoved method has thrown an exception" on every + * teardown. + * + * Shiro offers no way to install an empty realm set, so the guard must decline + * the write and log it. The realm set the SecurityManager still holds is not a + * widened authorization surface: the Activator's realm reference is + * AT_LEAST_ONE, so DS deactivates this component when the last realm goes, and + * the SecurityManager service is unpublished with it. + */ +public class ActivatorRealmRemovalTest { + + private static final String SYSTEM_USER = "system"; + private static final String SYSTEM_PASSWORD = "manager"; + + private static final class FakeSystemRealm extends AuthorizingRealm implements GossRealm { + private final SimpleAccount account; + + FakeSystemRealm() { + this.account = new SimpleAccount(SYSTEM_USER, SYSTEM_PASSWORD, getName()); + this.account.addStringPermission("*"); + } + + @Override + protected AuthenticationInfo doGetAuthenticationInfo(AuthenticationToken token) { + UsernamePasswordToken upToken = (UsernamePasswordToken) token; + return SYSTEM_USER.equals(upToken.getUsername()) ? account : null; + } + + @Override + protected AuthorizationInfo doGetAuthorizationInfo(PrincipalCollection principals) { + String username = (String) getAvailablePrincipal(principals); + return SYSTEM_USER.equals(username) ? account : null; + } + + @Override + public Set getPermissions(String identifier) { + return SYSTEM_USER.equals(identifier) ? Set.of("*") : new HashSet<>(); + } + + @Override + public boolean hasIdentifier(String identifier) { + return SYSTEM_USER.equals(identifier); + } + } + + @SuppressWarnings("unchecked") + private static ServiceReference realmRef() { + return mock(ServiceReference.class); + } + + @Test + @DisplayName("Removing the last realm does not throw out of realmRemoved") + public void removingLastRealmDoesNotThrow() { + Activator securityManager = new Activator(); + ServiceReference ref = realmRef(); + securityManager.realmAdded(ref, new FakeSystemRealm()); + securityManager.activate(); + + assertThatCode(() -> securityManager.realmRemoved(ref)) + .as("teardown of the final realm must not surface IllegalArgumentException") + .doesNotThrowAnyException(); + } + + @Test + @DisplayName("Removing one of several realms still narrows the registered realm set") + public void removingOneOfSeveralRealmsStillRewritesTheSet() { + Activator securityManager = new Activator(); + ServiceReference first = realmRef(); + ServiceReference second = realmRef(); + FakeSystemRealm keep = new FakeSystemRealm(); + securityManager.realmAdded(first, new FakeSystemRealm()); + securityManager.realmAdded(second, keep); + securityManager.activate(); + + securityManager.realmRemoved(first); + + RealmSecurityManager rsm = securityManager; + assertThat(rsm.getRealms()) + .as("the removed realm must be gone and the surviving realm retained") + .containsExactly(keep); + } + + @Test + @DisplayName("Removing an unknown realm reference is a no-op on the registered set") + public void removingUnknownReferenceIsANoOp() { + Activator securityManager = new Activator(); + ServiceReference known = realmRef(); + FakeSystemRealm realm = new FakeSystemRealm(); + securityManager.realmAdded(known, realm); + securityManager.activate(); + + securityManager.realmRemoved(realmRef()); + + RealmSecurityManager rsm = securityManager; + assertThat(rsm.getRealms()).containsExactly(realm); + } +} diff --git a/pnnl.goss.core/test/pnnl/goss/core/security/impl/test/SystemRealmActivationOrderTest.java b/pnnl.goss.core/test/pnnl/goss/core/security/impl/test/SystemRealmActivationOrderTest.java index 0de2af90..cda86e9e 100644 --- a/pnnl.goss.core/test/pnnl/goss/core/security/impl/test/SystemRealmActivationOrderTest.java +++ b/pnnl.goss.core/test/pnnl/goss/core/security/impl/test/SystemRealmActivationOrderTest.java @@ -159,6 +159,55 @@ public void systemRealmPublishesDistinguishingProperty() throws Exception { .isTrue(); } + @Test + @DisplayName("GOSS-025: system realm hangs its account load off container-driven entry points, not Shiro init()") + public void systemRealmDeclaresContainerDrivenLifecycleEntryPoints() throws Exception { + Document doc = descriptor("pnnl.goss.core.security-system.jar", + "pnnl.goss.core.security.system.SystemBasedRealm"); + Element component = doc.getDocumentElement(); + + // The GOSS-025 defect was that the ONLY population path was Shiro's + // onInit(), which nothing in this deployment calls. These attributes are the + // paths Declarative Services is contractually guaranteed to invoke, so the + // descriptor is where that guarantee has to be asserted. + assertThat(component.getAttribute("activate")) + .as("DS must have an activate method to drive; without it the realm loads from nothing") + .isEqualTo("activate"); + assertThat(component.getAttribute("modified")) + .as("a configuration update must reach the realm rather than silently deactivating it") + .isEqualTo("updated"); + assertThat(component.getAttribute("configuration-policy")) + .as("GOSS-004 documents the required systemrealm.cfg dependency deliberately") + .isEqualTo("require"); + assertThat(component.getAttribute("configuration-pid")) + .isEqualTo("pnnl.goss.core.security.systemrealm"); + } + + @Test + @DisplayName("GOSS-025: the system realm's SecurityConfig reference is mandatory and notifies on change") + public void systemRealmSecurityConfigReferenceIsMandatoryAndNotifying() throws Exception { + Document doc = descriptor("pnnl.goss.core.security-system.jar", + "pnnl.goss.core.security.system.SystemBasedRealm"); + Element ref = referenceByInterface(doc, "pnnl.goss.core.security.SecurityConfig"); + assertNotNull(ref, "the system realm must declare a SecurityConfig reference; it is the credential source"); + + String cardinality = ref.getAttribute("cardinality"); + assertThat(cardinality.isEmpty() || "1..1".equals(cardinality)) + .as("the credential source must be mandatory so DS cannot activate an unauthenticatable realm;" + + " was '%s'", cardinality) + .isTrue(); + assertThat(ref.getAttribute("bind")) + .as("a bind callback is what lets the realm load on wiring rather than on an uncalled hook") + .isEqualTo("setSecurityConfig"); + assertThat(ref.getAttribute("unbind")) + .as("an unbind callback is what lets the realm fail closed when the credential source goes") + .isEqualTo("unsetSecurityConfig"); + assertThat(ref.getAttribute("updated")) + .as("pnnl.goss.security.cfg edits change the bound service's properties, not the instance;" + + " without this callback a rotated manager credential is never picked up") + .isEqualTo("updatedSecurityConfig"); + } + @Test @DisplayName("SecurityManager component gates activation on the system realm via a mandatory target filter") public void activatorReferencesSystemRealmWithTargetFilter() throws Exception { @@ -297,6 +346,20 @@ private JarEntry findByComponentName(JarFile jar, String componentName) throws I return null; } + private Element referenceByInterface(Document doc, String interfaceName) { + NodeList refs = doc.getElementsByTagName("reference"); + for (int i = 0; i < refs.getLength(); i++) { + Node node = refs.item(i); + if (node instanceof Element) { + Element ref = (Element) node; + if (interfaceName.equals(ref.getAttribute("interface"))) { + return ref; + } + } + } + return null; + } + private Element referenceByName(Document doc, String refName) { NodeList refs = doc.getElementsByTagName("reference"); for (int i = 0; i < refs.getLength(); i++) { diff --git a/pnnl.goss.core/test/pnnl/goss/core/security/system/test/SystemBasedRealmActivationTest.java b/pnnl.goss.core/test/pnnl/goss/core/security/system/test/SystemBasedRealmActivationTest.java new file mode 100644 index 00000000..700b142a --- /dev/null +++ b/pnnl.goss.core/test/pnnl/goss/core/security/system/test/SystemBasedRealmActivationTest.java @@ -0,0 +1,356 @@ +package pnnl.goss.core.security.system.test; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatCode; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.util.Collections; +import java.util.Map; +import java.util.Set; + +import org.apache.shiro.authc.AuthenticationInfo; +import org.apache.shiro.authc.IncorrectCredentialsException; +import org.apache.shiro.authc.SimpleAccount; +import org.apache.shiro.authc.UsernamePasswordToken; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import pnnl.goss.core.security.JWTAuthenticationToken; +import pnnl.goss.core.security.SecurityConfig; +import pnnl.goss.core.security.system.SystemBasedRealm; + +/** + * Covers the GOSS-025 bootstrap failure: SystemBasedRealm populated its account + * map only from Shiro's {@code onInit()} hook, and nothing in this OSGi + * deployment ever calls {@code Initializable.init()}. Declarative Services + * calls {@code @Activate}; Shiro's RealmSecurityManager.setRealms() reaches + * only afterRealmsSet() -> applyCacheManagerToRealms(), never init(). So the + * realm activated with a permanently empty userMap and every system/manager + * authentication against it threw UnknownAccountException, which broke the + * broker connect in GridOpticsServer.start(). + * + * The load-bearing assertion in this class is that the realm resolves the + * system account after DS activation WITHOUT any call to init(). Everything + * else here guards the surrounding lifecycle: config reload, reference rebind, + * and fail- closed behavior when the credential source is unusable. + */ +public class SystemBasedRealmActivationTest { + + private static final String SYSTEM_USER = "system"; + private static final String SYSTEM_PASSWORD = "manager"; + + /** + * The exact permission string the broker's authorization plugin consumes for + * the system principal. Deliberately duplicated here as a literal rather than + * referenced from production code: this test is the guard that the string is + * not silently altered. + */ + private static final String SYSTEM_PERMISSIONS = "queue:*,topic:*,temp-queue:*,fusion:*:read,fusion:*:write"; + + /** Mutable SecurityConfig stub standing in for SecurityConfigImpl. */ + private static final class FakeSecurityConfig implements SecurityConfig { + private String managerUser; + private String managerPassword; + + FakeSecurityConfig(String managerUser, String managerPassword) { + this.managerUser = managerUser; + this.managerPassword = managerPassword; + } + + @Override + public String getManagerUser() { + return managerUser; + } + + @Override + public String getManagerPassword() { + return managerPassword; + } + + @Override + public boolean getUseToken() { + return false; + } + + @Override + public boolean validateToken(String token) { + return false; + } + + @Override + public JWTAuthenticationToken parseToken(String token) { + return null; + } + + @Override + public String createToken(Object userId, Set roles) { + return null; + } + } + + /** + * Simulates the Declarative Services bind callback for the securityConfig + * reference. DS invokes bind methods before @Activate. + */ + private static void bindSecurityConfig(SystemBasedRealm realm, SecurityConfig config) { + realm.setSecurityConfig(config); + } + + /** The DS-supplied component properties: systemrealm.cfg carries load=true. */ + private static Map componentProperties() { + return Map.of("load", "true"); + } + + @Test + @DisplayName("Realm resolves the system account after DS activation, with no call to Shiro init()") + public void systemAccountResolvesAfterActivateWithoutShiroInit() throws Exception { + SystemBasedRealm realm = new SystemBasedRealm(); + bindSecurityConfig(realm, new FakeSecurityConfig(SYSTEM_USER, SYSTEM_PASSWORD)); + + // Exactly what the container does: @Activate, and nothing else. No init(). + realm.activate(componentProperties()); + + assertThat(realm.hasIdentifier(SYSTEM_USER)) + .as("the realm must know the system principal after DS activation alone") + .isTrue(); + + AuthenticationInfo info = realm.getAuthenticationInfo( + new UsernamePasswordToken(SYSTEM_USER, SYSTEM_PASSWORD)); + + assertThat(info).as("system/manager must authenticate against the activated realm").isNotNull(); + assertThat(info.getPrincipals().getPrimaryPrincipal()).isEqualTo(SYSTEM_USER); + assertThat(info.getCredentials()).isEqualTo(SYSTEM_PASSWORD); + + assertThat(info).isInstanceOf(SimpleAccount.class); + SimpleAccount account = (SimpleAccount) info; + assertThat(account.getStringPermissions()) + .as("the system account must carry exactly the broker permission string, unaltered") + .containsExactly(SYSTEM_PERMISSIONS); + + assertThat(realm.getPermissions(SYSTEM_USER)) + .as("PermissionAdapter view must report the same single permission string") + .containsExactly(SYSTEM_PERMISSIONS); + } + + @Test + @DisplayName("Unknown principals are still rejected after activation") + public void unknownPrincipalIsRejected() throws Exception { + SystemBasedRealm realm = new SystemBasedRealm(); + bindSecurityConfig(realm, new FakeSecurityConfig(SYSTEM_USER, SYSTEM_PASSWORD)); + realm.activate(componentProperties()); + + assertThat(realm.hasIdentifier("intruder")).isFalse(); + assertThat(realm.getPermissions("intruder")).isEmpty(); + assertThat(realm.getAuthenticationInfo(new UsernamePasswordToken("intruder", "manager"))) + .as("the realm must not vend an account for an unconfigured principal") + .isNull(); + } + + @Test + @DisplayName("A wrong password for the system principal is rejected") + public void wrongSystemPasswordIsRejected() throws Exception { + SystemBasedRealm realm = new SystemBasedRealm(); + bindSecurityConfig(realm, new FakeSecurityConfig(SYSTEM_USER, SYSTEM_PASSWORD)); + realm.activate(componentProperties()); + + assertThatThrownBy(() -> realm.getAuthenticationInfo( + new UsernamePasswordToken(SYSTEM_USER, "not-the-password"))) + .isInstanceOf(IncorrectCredentialsException.class); + } + + @Test + @DisplayName("Shiro init() remains safe and idempotent if anything ever calls it") + public void shiroInitIsIdempotentAfterActivation() throws Exception { + SystemBasedRealm realm = new SystemBasedRealm(); + bindSecurityConfig(realm, new FakeSecurityConfig(SYSTEM_USER, SYSTEM_PASSWORD)); + realm.activate(componentProperties()); + + realm.init(); + + assertThat(realm.hasIdentifier(SYSTEM_USER)).isTrue(); + assertThat(realm.getPermissions(SYSTEM_USER)).containsExactly(SYSTEM_PERMISSIONS); + AuthenticationInfo info = realm.getAuthenticationInfo( + new UsernamePasswordToken(SYSTEM_USER, SYSTEM_PASSWORD)); + assertThat(info.getPrincipals().getPrimaryPrincipal()).isEqualTo(SYSTEM_USER); + assertThat(info.getCredentials()).isEqualTo(SYSTEM_PASSWORD); + } + + @Test + @DisplayName("A configuration update replaces the stored credential rather than keeping the stale one") + public void modifiedReloadsChangedCredentials() throws Exception { + SystemBasedRealm realm = new SystemBasedRealm(); + FakeSecurityConfig config = new FakeSecurityConfig(SYSTEM_USER, SYSTEM_PASSWORD); + bindSecurityConfig(realm, config); + realm.activate(componentProperties()); + + config.managerPassword = "rotated"; + realm.updated(componentProperties()); + + assertThatThrownBy(() -> realm.getAuthenticationInfo( + new UsernamePasswordToken(SYSTEM_USER, SYSTEM_PASSWORD))) + .as("the superseded password must no longer authenticate") + .isInstanceOf(IncorrectCredentialsException.class); + + AuthenticationInfo info = realm.getAuthenticationInfo( + new UsernamePasswordToken(SYSTEM_USER, "rotated")); + assertThat(info).isNotNull(); + assertThat(info.getCredentials()).isEqualTo("rotated"); + } + + @Test + @DisplayName("A renamed manager user does not leave the previous account behind") + public void modifiedDoesNotLeaveStaleAccounts() throws Exception { + SystemBasedRealm realm = new SystemBasedRealm(); + FakeSecurityConfig config = new FakeSecurityConfig(SYSTEM_USER, SYSTEM_PASSWORD); + bindSecurityConfig(realm, config); + realm.activate(componentProperties()); + + config.managerUser = "operator"; + realm.updated(componentProperties()); + + assertThat(realm.hasIdentifier(SYSTEM_USER)) + .as("the previous manager user must be evicted, not accumulated") + .isFalse(); + assertThat(realm.getPermissions(SYSTEM_USER)).isEmpty(); + assertThat(realm.hasIdentifier("operator")).isTrue(); + assertThat(realm.getPermissions("operator")).containsExactly(SYSTEM_PERMISSIONS); + } + + @Test + @DisplayName("A rebound SecurityConfig replaces the account rather than adding to it") + public void rebindReplacesTheSystemAccount() { + SystemBasedRealm realm = new SystemBasedRealm(); + FakeSecurityConfig first = new FakeSecurityConfig(SYSTEM_USER, SYSTEM_PASSWORD); + bindSecurityConfig(realm, first); + realm.activate(componentProperties()); + + realm.setSecurityConfig(new FakeSecurityConfig("operator", "rotated")); + realm.unsetSecurityConfig(first); + + assertThat(realm.hasIdentifier(SYSTEM_USER)) + .as("greedy rebind must not leave the previous credential source's account behind") + .isFalse(); + assertThat(realm.hasIdentifier("operator")).isTrue(); + assertThat(realm.getPermissions("operator")).containsExactly(SYSTEM_PERMISSIONS); + + AuthenticationInfo info = realm.getAuthenticationInfo( + new UsernamePasswordToken("operator", "rotated")); + assertThat(info.getCredentials()).isEqualTo("rotated"); + } + + @Test + @DisplayName("An updated SecurityConfig service picks up a rotated credential without reactivation") + public void updatedReferenceCallbackReloadsCredentials() { + SystemBasedRealm realm = new SystemBasedRealm(); + FakeSecurityConfig config = new FakeSecurityConfig(SYSTEM_USER, SYSTEM_PASSWORD); + bindSecurityConfig(realm, config); + realm.activate(componentProperties()); + + // pnnl.goss.security.cfg edited: SecurityConfigImpl's modified method runs + // on the same instance, DS updates its service properties, and DS calls the + // updated= callback on this reference. + config.managerPassword = "rotated"; + realm.updatedSecurityConfig(config); + + assertThatThrownBy(() -> realm.getAuthenticationInfo( + new UsernamePasswordToken(SYSTEM_USER, SYSTEM_PASSWORD))) + .isInstanceOf(IncorrectCredentialsException.class); + assertThat(realm.getAuthenticationInfo(new UsernamePasswordToken(SYSTEM_USER, "rotated")) + .getCredentials()).isEqualTo("rotated"); + } + + @Test + @DisplayName("Unbinding the credential source empties the realm rather than leaving it authenticating") + public void unbindFailsClosed() { + SystemBasedRealm realm = new SystemBasedRealm(); + FakeSecurityConfig config = new FakeSecurityConfig(SYSTEM_USER, SYSTEM_PASSWORD); + bindSecurityConfig(realm, config); + realm.activate(componentProperties()); + + realm.unsetSecurityConfig(config); + + assertThat(realm.hasIdentifier(SYSTEM_USER)).isFalse(); + assertThat(realm.getPermissions(SYSTEM_USER)).isEmpty(); + assertThat(realm.getAuthenticationInfo(new UsernamePasswordToken(SYSTEM_USER, SYSTEM_PASSWORD))) + .isNull(); + } + + @Test + @DisplayName("Wiring fails closed with an empty realm when the manager user is absent") + public void wiringFailsClosedWithoutManagerUser() { + SystemBasedRealm realm = new SystemBasedRealm(); + + assertThatThrownBy(() -> realm.setSecurityConfig(new FakeSecurityConfig(null, SYSTEM_PASSWORD))) + .as("a missing manager user must abort DS wiring, not publish an empty realm") + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("goss.system.manager"); + + assertThat(realm.hasIdentifier(SYSTEM_USER)).isFalse(); + assertThat(realm.getAuthenticationInfo(new UsernamePasswordToken(SYSTEM_USER, SYSTEM_PASSWORD))) + .isNull(); + assertThatThrownBy(() -> realm.activate(componentProperties())) + .as("activation must not paper over the unusable credential source either") + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("goss.system.manager"); + } + + @Test + @DisplayName("Wiring fails closed with an empty realm when the manager password is blank") + public void wiringFailsClosedWithoutManagerPassword() { + SystemBasedRealm realm = new SystemBasedRealm(); + + assertThatThrownBy(() -> realm.setSecurityConfig(new FakeSecurityConfig(SYSTEM_USER, " "))) + .as("a blank manager password must abort DS wiring, not publish an empty realm") + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("goss.system.manager.password"); + + assertThat(realm.hasIdentifier(SYSTEM_USER)).isFalse(); + assertThat(realm.getAuthenticationInfo(new UsernamePasswordToken(SYSTEM_USER, " "))) + .isNull(); + } + + @Test + @DisplayName("Activation with no bound SecurityConfig fails closed instead of NPEing into an empty realm") + public void activationFailsClosedWithoutSecurityConfig() { + SystemBasedRealm realm = new SystemBasedRealm(); + + assertThatThrownBy(() -> realm.activate(componentProperties())) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("SecurityConfig"); + + assertThat(realm.hasIdentifier(SYSTEM_USER)).isFalse(); + } + + @Test + @DisplayName("A configuration update that breaks the credential source empties the realm") + public void modifiedFailsClosedOnBrokenConfig() { + SystemBasedRealm realm = new SystemBasedRealm(); + FakeSecurityConfig config = new FakeSecurityConfig(SYSTEM_USER, SYSTEM_PASSWORD); + bindSecurityConfig(realm, config); + realm.activate(componentProperties()); + + config.managerPassword = null; + assertThatThrownBy(() -> realm.updated(componentProperties())) + .isInstanceOf(IllegalStateException.class) + .hasMessageContaining("goss.system.manager.password"); + + assertThat(realm.hasIdentifier(SYSTEM_USER)) + .as("a failed reload must not leave the superseded account authenticating") + .isFalse(); + } + + @Test + @DisplayName("Component properties are never treated as the credential source") + public void componentPropertiesAreNotACredentialSource() throws Exception { + SystemBasedRealm realm = new SystemBasedRealm(); + bindSecurityConfig(realm, new FakeSecurityConfig(SYSTEM_USER, SYSTEM_PASSWORD)); + + // systemrealm.cfg carries only load=true; a null property map must not + // change which principal the realm serves. + assertThatCode(() -> realm.activate(Collections.emptyMap())).doesNotThrowAnyException(); + + assertThat(realm.hasIdentifier(SYSTEM_USER)).isTrue(); + assertThat(realm.hasIdentifier("load")).isFalse(); + assertThat(realm.getPermissions(SYSTEM_USER)).containsExactly(SYSTEM_PERMISSIONS); + } +}