feat: security network component logic pt 2

* If there are no decision providers, always allow
* If at least one decision provider denies, fail
* If at least one decision provider allows, accept
* If all decision providers pass, use fallback policy
* If no fallback policy is present, fail
* If at least one fallback policy denies, fail
* If at least one fallback policy allows, accept
* If all fallback policies pass, fail

This is a 2-pass check to ensure regular policies get prioritized over default policies.

CHANGELOG.md

@@ -19,7 +19,8 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 -   The Security Card can be bound to other (currently online) players via its GUI.
 -   The binding of a Security Card can now be cleared.
 -   The Security Card tooltip and GUI now show whether the permission has been touched/changed in any way.
--   A global (fallback) permission set for a network can be defined using the Fallback Security Card (instead of using an "unbound" Security Card).
+-   As soon as a Security Manager is placed, the storage network will be locked down by default. Start adding Security Cards to allow or deny specific access to players.
+-   To not lock the entire network by default for players who do not have a matching Security Card, a Fallback Security Card can be used to configure this behavior.
 -   Smooth scrolling, screen size and max row stretch are no longer Grid-specific settings, but are now global settings.
 
 ### Fixed

refinedstorage2-network-api/src/main/java/com/refinedmods/refinedstorage2/api/network/security/SecurityDecisionProvider.java

@@ -5,14 +5,9 @@
 @API(status = API.Status.STABLE, since = "2.0.0-milestone.3.5")
 @FunctionalInterface
 public interface SecurityDecisionProvider {
-    /**
-     * Returns the {@link SecurityDecision} for the given {@link Permission} and {@link SecurityActor}.
-     * If there is no {@link SecurityPolicy} for the given {@link SecurityActor}, the decision should
-     * be {@link SecurityDecision#PASS}.
-     *
-     * @param permission the permission
-     * @param actor      the actor
-     * @return the security decision
-     */
     SecurityDecision isAllowed(Permission permission, SecurityActor actor);
+
+    default SecurityDecision isAllowed(Permission permission) {
+        return SecurityDecision.PASS;
+    }
 }

refinedstorage2-network-api/src/main/java/com/refinedmods/refinedstorage2/api/network/security/SecurityNetworkComponent.java

@@ -7,10 +7,5 @@
 @API(status = API.Status.STABLE, since = "2.0.0-milestone.3.5")
 @FunctionalInterface
 public interface SecurityNetworkComponent extends NetworkComponent {
-    /**
-     * @param permission the permission
-     * @param actor      the actor
-     * @return true if the actor is allowed to perform the action, false otherwise
-     */
     boolean isAllowed(Permission permission, SecurityActor actor);
 }

refinedstorage2-network/src/main/java/com/refinedmods/refinedstorage2/api/network/impl/node/security/SecurityDecisionProviderProxyNetworkNode.java

@@ -6,22 +6,48 @@
 import com.refinedmods.refinedstorage2.api.network.security.SecurityDecision;
 import com.refinedmods.refinedstorage2.api.network.security.SecurityDecisionProvider;
 
+import javax.annotation.Nullable;
+
 public class SecurityDecisionProviderProxyNetworkNode extends AbstractNetworkNode implements SecurityDecisionProvider {
-    private final long energyUsage;
-    private final SecurityDecisionProvider delegate;
+    private long energyUsage;
+    @Nullable
+    private SecurityDecisionProvider delegate;
 
-    public SecurityDecisionProviderProxyNetworkNode(final long energyUsage, final SecurityDecisionProvider delegate) {
+    public SecurityDecisionProviderProxyNetworkNode(final long energyUsage) {
         this.energyUsage = energyUsage;
+    }
+
+    public SecurityDecisionProviderProxyNetworkNode(final long energyUsage, final SecurityDecisionProvider delegate) {
+        this(energyUsage);
+        this.delegate = delegate;
+    }
+
+    public void setDelegate(@Nullable final SecurityDecisionProvider delegate) {
         this.delegate = delegate;
     }
 
+    public void setEnergyUsage(final long energyUsage) {
+        this.energyUsage = energyUsage;
+    }
+
     @Override
     public long getEnergyUsage() {
         return energyUsage;
     }
 
     @Override
     public SecurityDecision isAllowed(final Permission permission, final SecurityActor actor) {
+        if (delegate == null) {
+            return SecurityDecision.PASS;
+        }
         return delegate.isAllowed(permission, actor);
     }
+
+    @Override
+    public SecurityDecision isAllowed(final Permission permission) {
+        if (delegate == null) {
+            return SecurityDecision.PASS;
+        }
+        return delegate.isAllowed(permission);
+    }
 }

refinedstorage2-network/src/main/java/com/refinedmods/refinedstorage2/api/network/impl/security/SecurityDecisionProviderImpl.java

@@ -8,21 +8,45 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import javax.annotation.Nullable;
 
 public class SecurityDecisionProviderImpl implements SecurityDecisionProvider {
     private final Map<SecurityActor, SecurityPolicy> policyByActor = new HashMap<>();
+    @Nullable
+    private SecurityPolicy defaultPolicy;
 
     public SecurityDecisionProviderImpl setPolicy(final SecurityActor actor, final SecurityPolicy policy) {
         policyByActor.put(actor, policy);
         return this;
     }
 
+    public SecurityDecisionProviderImpl setDefaultPolicy(@Nullable final SecurityPolicy policy) {
+        this.defaultPolicy = policy;
+        return this;
+    }
+
+    public void clearPolicies() {
+        policyByActor.clear();
+    }
+
     @Override
     public SecurityDecision isAllowed(final Permission permission, final SecurityActor actor) {
         final SecurityPolicy policy = policyByActor.get(actor);
         if (policy == null) {
             return SecurityDecision.PASS;
         }
-        return policy.isAllowed(permission) ? SecurityDecision.ALLOW : SecurityDecision.DENY;
+        return allowOrDeny(policy.isAllowed(permission));
+    }
+
+    @Override
+    public SecurityDecision isAllowed(final Permission permission) {
+        if (defaultPolicy == null) {
+            return SecurityDecision.PASS;
+        }
+        return allowOrDeny(defaultPolicy.isAllowed(permission));
+    }
+
+    private static SecurityDecision allowOrDeny(final boolean allowed) {
+        return allowed ? SecurityDecision.ALLOW : SecurityDecision.DENY;
     }
 }

refinedstorage2-network/src/main/java/com/refinedmods/refinedstorage2/api/network/impl/security/SecurityNetworkComponentImpl.java

@@ -11,6 +11,7 @@
 
 import java.util.LinkedHashSet;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 public class SecurityNetworkComponentImpl implements SecurityNetworkComponent {
     private final Set<SecurityDecisionProvider> providers = new LinkedHashSet<>();
@@ -36,17 +37,33 @@ public void onContainerRemoved(final NetworkNodeContainer container) {
 
     @Override
     public boolean isAllowed(final Permission permission, final SecurityActor actor) {
-        for (final SecurityDecisionProvider provider : providers) {
-            final SecurityDecision decision = CoreValidations.validateNotNull(
-                provider.isAllowed(permission, actor),
-                "Security decision provider must not return null"
-            );
-            if (decision == SecurityDecision.DENY) {
-                return false;
-            } else if (decision == SecurityDecision.ALLOW) {
-                return true;
-            }
+        if (providers.isEmpty()) {
+            return defaultPolicy.isAllowed(permission);
         }
-        return defaultPolicy.isAllowed(permission);
+        final Set<SecurityDecision> decisions = providers.stream().map(provider -> CoreValidations.validateNotNull(
+            provider.isAllowed(permission, actor),
+            "Decision cannot be null"
+        )).collect(Collectors.toSet());
+        final boolean anyDenied = decisions.stream().anyMatch(decision -> decision == SecurityDecision.DENY);
+        if (anyDenied) {
+            return false;
+        }
+        final boolean anyAllowed = decisions.stream().anyMatch(decision -> decision == SecurityDecision.ALLOW);
+        if (anyAllowed) {
+            return true;
+        }
+        return tryFallback(permission);
+    }
+
+    private boolean tryFallback(final Permission permission) {
+        final Set<SecurityDecision> decisions = providers.stream().map(provider -> CoreValidations.validateNotNull(
+            provider.isAllowed(permission),
+            "Decision cannot be null"
+        )).collect(Collectors.toSet());
+        final boolean anyDenied = decisions.stream().anyMatch(decision -> decision == SecurityDecision.DENY);
+        if (anyDenied) {
+            return false;
+        }
+        return decisions.stream().anyMatch(decision -> decision == SecurityDecision.ALLOW);
     }
 }

refinedstorage2-network/src/test/java/com/refinedmods/refinedstorage2/api/network/impl/security/SecurityNetworkComponentImplTest.java

@@ -1,7 +1,6 @@
 package com.refinedmods.refinedstorage2.api.network.impl.security;
 
 import com.refinedmods.refinedstorage2.api.network.impl.node.security.SecurityDecisionProviderProxyNetworkNode;
-import com.refinedmods.refinedstorage2.api.network.node.NetworkNode;
 import com.refinedmods.refinedstorage2.api.network.security.Permission;
 import com.refinedmods.refinedstorage2.api.network.security.SecurityActor;
 import com.refinedmods.refinedstorage2.api.network.security.SecurityNetworkComponent;
@@ -29,84 +28,140 @@ void shouldUseDefaultPolicyIfNoSecurityDecisionProvidersArePresent() {
         // Act & assert
         assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.A)).isTrue();
         assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.A)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.A)).isFalse();
+
+        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.B)).isTrue();
+        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.B)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.B)).isFalse();
     }
 
     @Test
-    void shouldUseDefaultPolicyIfANotConfiguredSecurityDecisionProviderIsPresent() {
+    void shouldDenyAllIfAtLeastOneSecurityDecisionProviderIsPresent() {
         // Arrange
         sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, securityDecisionProvider));
 
         // Act & assert
-        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.A)).isTrue();
+        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.A)).isFalse();
         assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.A)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.A)).isFalse();
+
+        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.B)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.B)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.B)).isFalse();
     }
 
     @Test
-    void shouldUseDefaultPolicyIfAConfiguredSecurityDecisionProviderIsPresentForAnotherActor() {
+    void shouldAllowOrDeny() {
         // Arrange
+        securityDecisionProvider.setPolicy(TestActors.A, policy(TestPermissions.OTHER));
         sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, securityDecisionProvider));
-        securityDecisionProvider.setPolicy(TestActors.B, policy(TestPermissions.OTHER));
 
         // Act & assert
-        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.A)).isTrue();
-        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.A)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.A)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.A)).isTrue();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.A)).isFalse();
+
+        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.B)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.B)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.B)).isFalse();
     }
 
     @Test
-    void shouldDenyPermissionIfAConfiguredSecurityDecisionProviderIsPresentForTheActor() {
+    void shouldOnlyAllowIfAllSecurityDecisionProvidersAllow() {
         // Arrange
-        sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, securityDecisionProvider));
-        securityDecisionProvider.setPolicy(TestActors.B, policy(TestPermissions.OTHER));
+        sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, new SecurityDecisionProviderImpl()
+            .setPolicy(TestActors.A, policy(TestPermissions.OTHER))
+        ));
+
+        sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, new SecurityDecisionProviderImpl()
+            .setPolicy(TestActors.A, policy(TestPermissions.OTHER2))
+        ));
+
+        sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, new SecurityDecisionProviderImpl()
+            .setPolicy(TestActors.B, policy(TestPermissions.OTHER))
+        ));
 
         // Act & assert
-        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.B)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.A)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.A)).isFalse();
+
         assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.B)).isTrue();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.B)).isFalse();
+
+        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.C)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.C)).isFalse();
     }
 
     @Test
-    void shouldUseFirstSecurityDecisionProviderThatIsConfiguredForActor() {
+    void shouldUseDefaultPolicyOfSecurityDecisionProviderIfAllProvidersPassDecision() {
         // Arrange
         sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, new SecurityDecisionProviderImpl()
-            .setPolicy(TestActors.A, policy(TestPermissions.OTHER2))));
+            .setPolicy(TestActors.A, policy(TestPermissions.OTHER))
+            .setDefaultPolicy(policy(TestPermissions.ALLOW_BY_DEFAULT))
+        ));
 
-        sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, securityDecisionProvider));
-        securityDecisionProvider.setPolicy(TestActors.A, policy(TestPermissions.ALLOW_BY_DEFAULT)); // will be ignored
-        securityDecisionProvider.setPolicy(TestActors.B, policy(TestPermissions.OTHER));
+        sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, new SecurityDecisionProviderImpl()
+            .setPolicy(TestActors.A, policy(TestPermissions.OTHER))
+            .setDefaultPolicy(policy(TestPermissions.ALLOW_BY_DEFAULT, TestPermissions.OTHER2))
+        ));
+
+        sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, new SecurityDecisionProviderImpl()
+            .setPolicy(TestActors.C, policy(TestPermissions.OTHER))
+        ));
 
         // Act & assert
         assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.A)).isFalse();
-        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.A)).isFalse();
-        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.A)).isTrue();
+        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.A)).isTrue();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.A)).isFalse();
 
-        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.B)).isFalse();
-        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.B)).isTrue();
+        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.B)).isTrue();
+        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.B)).isFalse();
         assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.B)).isFalse();
+
+        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.C)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.C)).isTrue();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.C)).isFalse();
     }
 
     @Test
-    void shouldRemoveSecurityDecisionProvider() {
+    void shouldRemoveContainer() {
         // Arrange
-        final NetworkNode node = new SecurityDecisionProviderProxyNetworkNode(0, securityDecisionProvider);
-        sut.onContainerAdded(() -> node);
-        securityDecisionProvider.setPolicy(TestActors.B, policy(TestPermissions.OTHER));
+        sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, new SecurityDecisionProviderImpl()
+            .setDefaultPolicy(policy(TestPermissions.ALLOW_BY_DEFAULT))
+        ));
+
+        final var removedNode = new SecurityDecisionProviderProxyNetworkNode(0, new SecurityDecisionProviderImpl()
+            .setDefaultPolicy(policy(TestPermissions.OTHER)));
+        sut.onContainerAdded(() -> removedNode);
 
         // Act
-        sut.onContainerRemoved(() -> node);
-        sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, new SecurityDecisionProviderImpl()
-            .setPolicy(TestActors.B, policy(TestPermissions.OTHER2))));
+        sut.onContainerRemoved(() -> removedNode);
 
         // Assert
-        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.B)).isFalse();
-        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.B)).isFalse();
-        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.B)).isTrue();
+        assertThat(sut.isAllowed(TestPermissions.ALLOW_BY_DEFAULT, TestActors.A)).isTrue();
+    }
+
+    @Test
+    void shouldClearPolicies() {
+        // Arrange
+        sut.onContainerAdded(() -> new SecurityDecisionProviderProxyNetworkNode(0, securityDecisionProvider));
+        securityDecisionProvider.setPolicy(TestActors.A, policy(TestPermissions.OTHER));
+        securityDecisionProvider.setDefaultPolicy(policy(TestPermissions.OTHER2));
+
+        // Act
+        securityDecisionProvider.clearPolicies();
+
+        // Assert
+        assertThat(sut.isAllowed(TestPermissions.OTHER, TestActors.A)).isFalse();
+        assertThat(sut.isAllowed(TestPermissions.OTHER2, TestActors.A)).isTrue();
     }
 
     enum TestPermissions implements Permission {
         ALLOW_BY_DEFAULT, OTHER, OTHER2
     }
 
     enum TestActors implements SecurityActor {
-        A, B
+        A, B, C
     }
 
     private SecurityPolicy policy(final Permission... permissions) {