Merge pull request #3426 from soul2zimate/WFCORE-3995-master

[WFCORE-3995] forbid invalid Sensitivity Constraint attribute read-wr…

controller/src/main/java/org/jboss/as/controller/access/constraint/AbstractSensitivity.java

@@ -124,4 +124,42 @@ protected boolean isCompatibleWith(AbstractSensitivity other) {
                         && defaultRequiresReadPermission == other.defaultRequiresReadPermission
                         && defaultRequiresWritePermission == other.defaultRequiresWritePermission);
     }
+
+    public boolean isConfiguredRequiresAccessPermissionValid(Boolean requiresAccessPermission) {
+        boolean effectiveAccessPermission = requiresAccessPermission == null ? defaultRequiresAccessPermission : requiresAccessPermission;
+        boolean effectiveReadPermission = configuredRequiresReadPermission == null ? defaultRequiresReadPermission : configuredRequiresReadPermission;
+        boolean effectiveWritePermission = configuredRequiresWritePermission == null ? defaultRequiresWritePermission : configuredRequiresWritePermission;
+        if (effectiveAccessPermission == true && (effectiveReadPermission == false | effectiveWritePermission == false)) {
+            return false;
+        } else {
+            return true;
+        }
+    }
+
+    public boolean isConfiguredRequiresReadPermissionValid(Boolean requiresReadPermission) {
+        boolean effectiveReadPermission = requiresReadPermission == null ? defaultRequiresReadPermission : requiresReadPermission;
+        boolean effectiveAccessPermission = configuredRequiresAccessPermission == null ? defaultRequiresAccessPermission : configuredRequiresAccessPermission;
+        boolean effectiveWritePermission = configuredRequiresWritePermission == null ? defaultRequiresWritePermission : configuredRequiresWritePermission;
+        if (effectiveReadPermission == false && effectiveAccessPermission == true) {
+            // write false to configured-requires-read while configured-requires-access is true is invalid
+            return false;
+        } else if (effectiveReadPermission == true && effectiveWritePermission == false) {
+            // write true to configured-requires-read while configured-requires-write is false is invalid
+            return false;
+        } else {
+            return true;
+        }
+    }
+
+    public boolean isConfiguredRequiresWritePermissionValid(Boolean requiresWritePermission) {
+        boolean effectiveWritePermission = requiresWritePermission == null ? defaultRequiresWritePermission : requiresWritePermission;
+        boolean effectiveAccessPermission = configuredRequiresAccessPermission == null ? defaultRequiresAccessPermission : configuredRequiresAccessPermission;
+        boolean effectiveReadPermission = configuredRequiresReadPermission == null ? defaultRequiresReadPermission : configuredRequiresReadPermission;
+        if (effectiveWritePermission == false && (effectiveAccessPermission == true | effectiveReadPermission == true)) {
+            // write false to configured-requires-write while configured-requires-access or configured-requires-read is true is invalid
+            return false;
+        } else {
+            return true;
+        }
+    }
 }

core-model-test/tests/src/test/java/org/jboss/as/core/model/test/access/RuntimeSensitivityReconfigurationTestCase.java

@@ -209,9 +209,11 @@ private void reconfigureSensitivity(String sensitivity, Boolean requiresAccess,
 
         ModelNode operation = Util.createOperation(WRITE_ATTRIBUTE_OPERATION, address);
 
-        if (requiresAccess != null) {
-            operation.get(NAME).set(CONFIGURED_REQUIRES_ADDRESSABLE);
-            operation.get(VALUE).set(requiresAccess);
+        // WFCORE-3995 in separate operation execution, we can not set access to true first without setting read and write.
+        // Update to following order write - read - access for validation
+        if (requiresWrite != null) {
+            operation.get(NAME).set(CONFIGURED_REQUIRES_WRITE);
+            operation.get(VALUE).set(requiresWrite);
             executeWithRoles(operation, StandardRole.SUPERUSER);
         }
 
@@ -221,9 +223,9 @@ private void reconfigureSensitivity(String sensitivity, Boolean requiresAccess,
             executeWithRoles(operation, StandardRole.SUPERUSER);
         }
 
-        if (requiresWrite != null) {
-            operation.get(NAME).set(CONFIGURED_REQUIRES_WRITE);
-            operation.get(VALUE).set(requiresWrite);
+        if (requiresAccess != null) {
+            operation.get(NAME).set(CONFIGURED_REQUIRES_ADDRESSABLE);
+            operation.get(VALUE).set(requiresAccess);
             executeWithRoles(operation, StandardRole.SUPERUSER);
         }
     }

core-model-test/tests/src/test/java/org/jboss/as/core/model/test/access/StandaloneAccessControlTestCase.java

@@ -65,6 +65,7 @@
     public void testConfiguration() throws Exception {
         //Initialize some additional constraints
         new SensitiveTargetAccessConstraintDefinition(new SensitivityClassification("play", "security-realm", true, true, true));
+        new SensitiveTargetAccessConstraintDefinition(new SensitivityClassification("system-property", "system-property", true, true, true));
         new ApplicationTypeAccessConstraintDefinition(new ApplicationTypeConfig("play", "deployment", false));
 
 
@@ -80,6 +81,12 @@ public void testConfiguration() throws Exception {
         //////////////////////////////////////////////////////////////////////////////////
         //Check that both set and undefined configured constraint settings get returned
 
+        /*
+         * <sensitive-classification type="play" name="security-realm" requires-addressable="false" requires-read="false" requires-write="false" />
+         * <sensitive-classification type="system-property" name="system-property" requires-addressable="true" requires-read="true" requires-write="true" />
+         * system-property sensitive classification default values are false, false, true
+         */
+
         System.out.println(kernelServices.readWholeModel());
         //Sensitivity classification
         //This one is undefined
@@ -103,6 +110,86 @@ public void testConfiguration() throws Exception {
                         pathElement(CLASSIFICATION, SECURITY_REALM)), SensitivityResourceDefinition.CONFIGURED_REQUIRES_ADDRESSABLE.getName())));
         checkResultExists(result, new ModelNode(false));
 
+        // WFCORE-3995 Test write operations on configured-requires-addressable
+        // This should fail as sensitivity constraint attribute configured-requires-read and configured-requires-write must not be false before writing configured-requires-addressable to true
+        result = ModelTestUtils.checkFailed(
+              kernelServices.executeOperation(
+                  Util.getWriteAttributeOperation(PathAddress.pathAddress(
+                      pathElement(CORE_SERVICE, MANAGEMENT),
+                      pathElement(ACCESS, AUTHORIZATION),
+                      pathElement(CONSTRAINT, SENSITIVITY_CLASSIFICATION),
+                      pathElement(TYPE, "play"),
+                      pathElement(CLASSIFICATION, SECURITY_REALM)), SensitivityResourceDefinition.CONFIGURED_REQUIRES_ADDRESSABLE.getName(), true)));
+        checkResultNotExists(result);
+
+        // This should fail as sensitivity constraint attribute configured-requires-read and configured-requires-write must not be false before undefine configured-requires-addressable to its default value true
+        result = ModelTestUtils.checkFailed(
+              kernelServices.executeOperation(
+                  Util.getUndefineAttributeOperation(PathAddress.pathAddress(
+                      pathElement(CORE_SERVICE, MANAGEMENT),
+                      pathElement(ACCESS, AUTHORIZATION),
+                      pathElement(CONSTRAINT, SENSITIVITY_CLASSIFICATION),
+                      pathElement(TYPE, "play"),
+                      pathElement(CLASSIFICATION, SECURITY_REALM)), SensitivityResourceDefinition.CONFIGURED_REQUIRES_ADDRESSABLE.getName())));
+        checkResultNotExists(result);
+
+        // WFCORE-3995 Test write operations on configured-requires-read
+        // This should fail as sensitivity constraint attribute configured-requires-addressable must not be true before writing configured-requires-read to false
+        result = ModelTestUtils.checkFailed(
+                kernelServices.executeOperation(
+                    Util.getWriteAttributeOperation(PathAddress.pathAddress(
+                        pathElement(CORE_SERVICE, MANAGEMENT),
+                        pathElement(ACCESS, AUTHORIZATION),
+                        pathElement(CONSTRAINT, SENSITIVITY_CLASSIFICATION),
+                        pathElement(TYPE, "system-propery"),
+                        pathElement(CLASSIFICATION, SECURITY_REALM)), SensitivityResourceDefinition.CONFIGURED_REQUIRES_READ.getName(), false)));
+        checkResultNotExists(result);
+
+        // This should fail as sensitivity constraint attribute configured-requires-addressable must not be true before undefine configured-requires-read its default value false
+        result = ModelTestUtils.checkFailed(
+                kernelServices.executeOperation(
+                    Util.getUndefineAttributeOperation(PathAddress.pathAddress(
+                        pathElement(CORE_SERVICE, MANAGEMENT),
+                        pathElement(ACCESS, AUTHORIZATION),
+                        pathElement(CONSTRAINT, SENSITIVITY_CLASSIFICATION),
+                        pathElement(TYPE, "system-propery"),
+                        pathElement(CLASSIFICATION, SECURITY_REALM)), SensitivityResourceDefinition.CONFIGURED_REQUIRES_READ.getName())));
+        checkResultNotExists(result);
+
+        // This should fail as sensitivity constraint attribute configured-requires-write must not be false before writing configured-requires-read to true
+        result = ModelTestUtils.checkFailed(
+                kernelServices.executeOperation(
+                    Util.getWriteAttributeOperation(PathAddress.pathAddress(
+                        pathElement(CORE_SERVICE, MANAGEMENT),
+                        pathElement(ACCESS, AUTHORIZATION),
+                        pathElement(CONSTRAINT, SENSITIVITY_CLASSIFICATION),
+                        pathElement(TYPE, "play"),
+                        pathElement(CLASSIFICATION, SECURITY_REALM)), SensitivityResourceDefinition.CONFIGURED_REQUIRES_READ.getName(), true)));
+        checkResultNotExists(result);
+
+        // This should fail as sensitivity constraint attribute configured-requires-addressable must not be false before undefine configured-requires-read to its default value true
+        result = ModelTestUtils.checkFailed(
+                kernelServices.executeOperation(
+                    Util.getUndefineAttributeOperation(PathAddress.pathAddress(
+                        pathElement(CORE_SERVICE, MANAGEMENT),
+                        pathElement(ACCESS, AUTHORIZATION),
+                        pathElement(CONSTRAINT, SENSITIVITY_CLASSIFICATION),
+                        pathElement(TYPE, "play"),
+                        pathElement(CLASSIFICATION, SECURITY_REALM)), SensitivityResourceDefinition.CONFIGURED_REQUIRES_READ.getName())));
+        checkResultNotExists(result);
+
+        // WFCORE-3995 Test write operations on configured-requires-write
+        // This should fail as sensitivity constraint attribute configured-requires-addressable and configured-requires-read must not be true before writing configured-requires-read to false
+        result = ModelTestUtils.checkFailed(
+                kernelServices.executeOperation(
+                    Util.getWriteAttributeOperation(PathAddress.pathAddress(
+                        pathElement(CORE_SERVICE, MANAGEMENT),
+                        pathElement(ACCESS, AUTHORIZATION),
+                        pathElement(CONSTRAINT, SENSITIVITY_CLASSIFICATION),
+                        pathElement(TYPE, "system-propery"),
+                        pathElement(CLASSIFICATION, SECURITY_REALM)), SensitivityResourceDefinition.CONFIGURED_REQUIRES_READ.getName(), false)));
+        checkResultNotExists(result);
+
         //VaultExpression
         //It is defined
         PathAddress vaultAddress = PathAddress.pathAddress(
@@ -113,7 +200,10 @@ public void testConfiguration() throws Exception {
                 kernelServices.executeOperation(
                         Util.getReadAttributeOperation(vaultAddress, SensitivityResourceDefinition.CONFIGURED_REQUIRES_READ.getName())));
         checkResultExists(result, new ModelNode(false));
-        //Now undefine it and check again
+        //Now undefine it and check again (need to undefine configured-requires-write first)
+        ModelTestUtils.checkOutcome(
+                kernelServices.executeOperation(
+                        Util.getUndefineAttributeOperation(vaultAddress, SensitivityResourceDefinition.CONFIGURED_REQUIRES_WRITE.getName())));
         ModelTestUtils.checkOutcome(
                 kernelServices.executeOperation(
                         Util.getUndefineAttributeOperation(vaultAddress, SensitivityResourceDefinition.CONFIGURED_REQUIRES_READ.getName())));
@@ -150,4 +240,8 @@ private void checkResultExists(ModelNode result, ModelNode expected) {
         Assert.assertTrue(result.has(RESULT));
         Assert.assertEquals(expected, result.get(RESULT));
     }
+
+    private void checkResultNotExists(ModelNode result) {
+        Assert.assertFalse(result.has(RESULT));
+    }
 }

core-model-test/tests/src/test/resources/org/jboss/as/core/model/test/access/standalone.xml

@@ -42,6 +42,7 @@
                <sensitive-classification type="core" name="access-control" requires-addressable="false" requires-read="false" requires-write="false" />
                <sensitive-classification type="core" name="security-realm" requires-addressable="false" requires-read="false" requires-write="false" />
                <sensitive-classification type="play" name="security-realm" requires-addressable="false" requires-read="false" requires-write="false" />
+               <sensitive-classification type="system-property" name="system-property" requires-addressable="true" requires-read="true" requires-write="true" />
             </sensitive-classifications>
             <application-classifications>
                <application-classification type="core" name="deployment" application="false"/>

domain-management/src/main/java/org/jboss/as/domain/management/access/ApplicationClassificationConfigResourceDefinition.java

@@ -98,14 +98,11 @@ public void execute(OperationContext context, ModelNode operation) throws Operat
             final String attribute = operation.require(NAME).asString();
             final ApplicationTypeConfigResource resource = (ApplicationTypeConfigResource)context.readResource(PathAddress.EMPTY_ADDRESS);
             final ApplicationTypeConfig applicationType = resource.applicationType;
-            Boolean result;
+            Boolean result = null;
             if (attribute.equals(DEFAULT_APPLICATION.getName())) {
                 result = applicationType.isDefaultApplication();
             } else if (attribute.equals(CONFIGURED_APPLICATION.getName())) {
                 result = applicationType.getConfiguredApplication();
-            } else {
-                //TODO i18n
-                throw new IllegalStateException();
             }
 
             context.getResult();
@@ -121,17 +118,28 @@ public void execute(OperationContext context, ModelNode operation) throws Operat
 
         @Override
         public void execute(OperationContext context, ModelNode operation) throws OperationFailedException {
-            final String attribute = operation.require(NAME).asString();
             final ModelNode value = operation.require(VALUE);
-            final ApplicationTypeConfigResource resource = (ApplicationTypeConfigResource)context.readResourceForUpdate(PathAddress.EMPTY_ADDRESS);
+            final ApplicationTypeConfigResource resource = (ApplicationTypeConfigResource) context.readResourceForUpdate(PathAddress.EMPTY_ADDRESS);
+            final ModelNode modelNode = resource.getModel();
+            // record model values for rollback handler
+            final ModelNode configuredApplication = modelNode.get(ModelDescriptionConstants.CONFIGURED_APPLICATION);
+
             final ApplicationTypeConfig classification = resource.applicationType;
-            if (attribute.equals(CONFIGURED_APPLICATION.getName())) {
-                Boolean confValue = readValue(context, value, CONFIGURED_APPLICATION);
-                classification.setConfiguredApplication(confValue);
-            } else {
-                //TODO i18n
-                throw new IllegalStateException();
-            }
+            classification.setConfiguredApplication(readValue(context, value, CONFIGURED_APPLICATION));
+
+            context.completeStep(new OperationContext.RollbackHandler() {
+
+                @Override
+                public void handleRollback(OperationContext context, ModelNode operation) {
+                    try {
+                        classification.setConfiguredApplication(readValue(context, configuredApplication, CONFIGURED_APPLICATION));
+                    } catch (OperationFailedException e) {
+                        // Should not happen since configured value is retrieved from resource.
+                        throw new RuntimeException(e);
+                    }
+
+                }
+            });
         }
 
         private Boolean readValue(OperationContext context, ModelNode value, AttributeDefinition definition) throws OperationFailedException {