Merge pull request #6330 from bstansberry/WFCORE-7153_27x

[WFCORE-7153] Add RBAC checks to ServerSuspendHandler and ServerResum…
wildfly · Jan 31, 2025 · c401bc5 · c401bc5
2 parents ac0acd8 + 73406b7
commit c401bc5
Show file tree

Hide file tree

Showing 3 changed files with 93 additions and 23 deletions.
diff --git a/server/src/main/java/org/jboss/as/server/operations/ServerResumeHandler.java b/server/src/main/java/org/jboss/as/server/operations/ServerResumeHandler.java
@@ -6,14 +6,20 @@
 package org.jboss.as.server.operations;
 
 
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.OP;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.RESUME;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.RUNNING_SERVER;
 
+import java.util.EnumSet;
+
 import org.jboss.as.controller.OperationContext;
 import org.jboss.as.controller.OperationFailedException;
 import org.jboss.as.controller.OperationStepHandler;
 import org.jboss.as.controller.SimpleOperationDefinition;
 import org.jboss.as.controller.SimpleOperationDefinitionBuilder;
+import org.jboss.as.controller.access.Action;
+import org.jboss.as.controller.access.AuthorizationResult;
+import org.jboss.as.controller.logging.ControllerLogger;
 import org.jboss.as.server.controller.descriptions.ServerDescriptions;
 import org.jboss.as.server.logging.ServerLogger;
 import org.jboss.as.server.suspend.ServerSuspendController;
@@ -45,7 +51,12 @@ public void execute(OperationContext context, ModelNode operation) throws Operat
         context.acquireControllerLock();
         context.addStep(new OperationStepHandler() {
             @Override
-            public void execute(OperationContext context, ModelNode operation) throws OperationFailedException {
+            public void execute(OperationContext context, ModelNode operation) {
+                AuthorizationResult authorizationResult = context.authorize(operation, EnumSet.of(Action.ActionEffect.WRITE_RUNTIME));
+                if (authorizationResult.getDecision() == AuthorizationResult.Decision.DENY) {
+                    throw ControllerLogger.ACCESS_LOGGER.unauthorized(operation.get(OP).asString(),
+                            context.getCurrentAddress(), authorizationResult.getExplanation());
+                }
                 context.completeStep(new OperationContext.ResultHandler() {
                     @Override
                     public void handleResult(OperationContext.ResultAction resultAction, OperationContext context, ModelNode operation) {

diff --git a/server/src/main/java/org/jboss/as/server/operations/ServerSuspendHandler.java b/server/src/main/java/org/jboss/as/server/operations/ServerSuspendHandler.java
@@ -6,11 +6,13 @@
 package org.jboss.as.server.operations;
 
 
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.OP;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.RUNNING_SERVER;
 import static org.jboss.as.server.controller.resources.ServerRootResourceDefinition.SUSPEND_TIMEOUT;
 import static org.jboss.as.server.controller.resources.ServerRootResourceDefinition.TIMEOUT;
 import static org.jboss.as.server.controller.resources.ServerRootResourceDefinition.renameTimeoutToSuspendTimeout;
 
+import java.util.EnumSet;
 import java.util.concurrent.CancellationException;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.TimeUnit;
@@ -20,7 +22,10 @@
 import org.jboss.as.controller.OperationStepHandler;
 import org.jboss.as.controller.SimpleOperationDefinition;
 import org.jboss.as.controller.SimpleOperationDefinitionBuilder;
+import org.jboss.as.controller.access.Action;
+import org.jboss.as.controller.access.AuthorizationResult;
 import org.jboss.as.controller.descriptions.ModelDescriptionConstants;
+import org.jboss.as.controller.logging.ControllerLogger;
 import org.jboss.as.server.controller.descriptions.ServerDescriptions;
 import org.jboss.as.server.logging.ServerLogger;
 import org.jboss.as.server.suspend.ServerSuspendController;
@@ -57,7 +62,13 @@ public void execute(final OperationContext context, ModelNode operation) throws
 
         context.addStep(new OperationStepHandler() {
             @Override
-            public void execute(final OperationContext context, ModelNode operation) throws OperationFailedException {
+            public void execute(final OperationContext context, ModelNode operation) {
+                AuthorizationResult authorizationResult = context.authorize(operation, EnumSet.of(Action.ActionEffect.WRITE_RUNTIME));
+                if (authorizationResult.getDecision() == AuthorizationResult.Decision.DENY) {
+                    throw ControllerLogger.ACCESS_LOGGER.unauthorized(operation.get(OP).asString(),
+                            context.getCurrentAddress(), authorizationResult.getExplanation());
+                }
+
                 final ServerSuspendController suspendController = ServerSuspendHandler.this.suspendController;
                 ServerLogger.ROOT_LOGGER.suspendingServer(seconds, TimeUnit.SECONDS);
                 CompletableFuture<Void> suspend = suspendController.suspend(ServerSuspendController.Context.RUNNING).toCompletableFuture();

diff --git a/...c/src/test/java/org/jboss/as/test/integration/mgmt/access/StandardRolesBasicTestCase.java b/...c/src/test/java/org/jboss/as/test/integration/mgmt/access/StandardRolesBasicTestCase.java
@@ -19,14 +19,17 @@
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.READ_CONFIG_AS_XML_FILE_OPERATION;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.READ_CONFIG_AS_XML_OPERATION;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.READ_RESOURCE_OPERATION;
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.RELOAD;
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.RELOAD_ENHANCED;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.REMOVE;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.RESULT;
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.RESUME;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.SHUTDOWN;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.SUBSYSTEM;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.SUCCESS;
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.SUSPEND;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.TYPE;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.UNDEFINE_ATTRIBUTE_OPERATION;
-import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.UPLOAD_DEPLOYMENT_BYTES;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.USER;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.USERNAME;
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.VALUE;
@@ -45,6 +48,7 @@
 import java.nio.charset.Charset;
 
 import org.jboss.as.controller.PathAddress;
+import org.jboss.as.controller.descriptions.ModelDescriptionConstants;
 import org.jboss.as.controller.operations.common.Util;
 import org.jboss.as.test.integration.management.interfaces.ManagementInterface;
 import org.jboss.as.test.integration.management.rbac.Outcome;
@@ -73,12 +77,12 @@ public abstract class StandardRolesBasicTestCase extends AbstractManagementInter
     protected static final String EXAMPLE_DS = "subsystem=rbac/rbac-constrained=default";
     private static final String TEST_PATH = "path=rbac.test";
 
-    private static final ModelNode WFLY_1916_OP;
+    private static final ModelNode UPLOAD_DEPLOYMENT_BYTES;
 
     static {
-        WFLY_1916_OP = Util.createEmptyOperation(UPLOAD_DEPLOYMENT_BYTES, PathAddress.EMPTY_ADDRESS);
-        WFLY_1916_OP.get(BYTES).set(new byte[64]);
-        WFLY_1916_OP.protect();
+        UPLOAD_DEPLOYMENT_BYTES = Util.createEmptyOperation(ModelDescriptionConstants.UPLOAD_DEPLOYMENT_BYTES, PathAddress.EMPTY_ADDRESS);
+        UPLOAD_DEPLOYMENT_BYTES.get(BYTES).set(new byte[64]);
+        UPLOAD_DEPLOYMENT_BYTES.protect();
     }
 
     protected static void deployDeployment1(ManagementClient client) throws IOException {
@@ -141,10 +145,15 @@ public void testMonitor() throws Exception {
         addDeployment2(client, Outcome.UNAUTHORIZED);
         addPath(client, Outcome.UNAUTHORIZED);
 
-        testWFLY1916(client, Outcome.UNAUTHORIZED);
+        uploadDeploymentBytes(client, Outcome.UNAUTHORIZED);
 
-        // Monitor can't shutdown
-        testWCORE1067(client);
+        suspend(client, Outcome.UNAUTHORIZED);
+        resume(client, Outcome.UNAUTHORIZED);
+
+        // Monitor can't shut down or reload
+        shutdownUnauthorized(client);
+        reloadUnauthorized(client);
+        reloadEnhancedUnauthorized(client);
     }
 
     @Test
@@ -164,7 +173,10 @@ public void testOperator() throws Exception {
         addDeployment2(client, Outcome.UNAUTHORIZED);
         addPath(client, Outcome.UNAUTHORIZED);
 
-        testWFLY1916(client, Outcome.SUCCESS);
+        uploadDeploymentBytes(client, Outcome.SUCCESS);
+
+        suspend(client, Outcome.SUCCESS);
+        resume(client, Outcome.SUCCESS);
     }
 
     @Test
@@ -184,7 +196,10 @@ public void testMaintainer() throws Exception {
         addDeployment2(client, Outcome.SUCCESS);
         addPath(client, Outcome.SUCCESS);
 
-        testWFLY1916(client, Outcome.SUCCESS);
+        uploadDeploymentBytes(client, Outcome.SUCCESS);
+
+        suspend(client, Outcome.SUCCESS);
+        resume(client, Outcome.SUCCESS);
     }
 
     @Test
@@ -204,10 +219,15 @@ public void testDeployer() throws Exception {
         addDeployment2(client, Outcome.SUCCESS);
         addPath(client, Outcome.UNAUTHORIZED);
 
-        testWFLY1916(client, Outcome.SUCCESS);
+        uploadDeploymentBytes(client, Outcome.SUCCESS);
 
-        // Deployer can't shutdown
-        testWCORE1067(client);
+        suspend(client, Outcome.UNAUTHORIZED);
+        resume(client, Outcome.UNAUTHORIZED);
+
+        // Deployer can't shut down or reload
+        shutdownUnauthorized(client);
+        reloadUnauthorized(client);
+        reloadEnhancedUnauthorized(client);
     }
 
     @Test
@@ -229,7 +249,10 @@ public void testAdministrator() throws Exception {
         addDeployment2(client, Outcome.SUCCESS);
         addPath(client, Outcome.SUCCESS);
 
-        testWFLY1916(client, Outcome.SUCCESS);
+        uploadDeploymentBytes(client, Outcome.SUCCESS);
+
+        suspend(client, Outcome.SUCCESS);
+        resume(client, Outcome.SUCCESS);
     }
 
     @Test
@@ -250,10 +273,15 @@ public void testAuditor() throws Exception {
         addDeployment2(client, Outcome.UNAUTHORIZED);
         addPath(client, Outcome.UNAUTHORIZED);
 
-        testWFLY1916(client, Outcome.UNAUTHORIZED);
+        uploadDeploymentBytes(client, Outcome.UNAUTHORIZED);
 
-        // Auditor can't shutdown
-        testWCORE1067(client);
+        suspend(client, Outcome.UNAUTHORIZED);
+        resume(client, Outcome.UNAUTHORIZED);
+
+        // Auditor can't shut down or reload
+        shutdownUnauthorized(client);
+        reloadUnauthorized(client);
+        reloadEnhancedUnauthorized(client);
     }
 
     @Test
@@ -274,7 +302,7 @@ public void testSuperUser() throws Exception {
         addDeployment2(client, Outcome.SUCCESS);
         addPath(client, Outcome.SUCCESS);
 
-        testWFLY1916(client, Outcome.SUCCESS);
+        uploadDeploymentBytes(client, Outcome.SUCCESS);
     }
 
     private static void whoami(ManagementInterface client, String expectedUsername) throws IOException {
@@ -438,14 +466,34 @@ private static void addRemoveIncldueForRole(final ManagementInterface client, fi
         }
     }
 
-    private void testWFLY1916(ManagementInterface client, Outcome expected) throws IOException {
-        ModelNode op = WFLY_1916_OP.clone();
+    private static void uploadDeploymentBytes(ManagementInterface client, Outcome expected) throws IOException {
+        ModelNode op = UPLOAD_DEPLOYMENT_BYTES.clone();
+        RbacUtil.executeOperation(client, op, expected);
+    }
+
+    private static void suspend(ManagementInterface client, Outcome expected) throws IOException {
+        ModelNode op = Util.createEmptyOperation(SUSPEND, PathAddress.EMPTY_ADDRESS);
         RbacUtil.executeOperation(client, op, expected);
     }
 
-    private void testWCORE1067(ManagementInterface client) throws IOException {
+    private static void resume(ManagementInterface client, Outcome expected) throws IOException {
+        ModelNode op = Util.createEmptyOperation(RESUME, PathAddress.EMPTY_ADDRESS);
+        RbacUtil.executeOperation(client, op, expected);
+    }
+
+    private static void shutdownUnauthorized(ManagementInterface client) throws IOException {
         ModelNode op = Util.createEmptyOperation(SHUTDOWN, PathAddress.EMPTY_ADDRESS);
         RbacUtil.executeOperation(client, op, Outcome.UNAUTHORIZED);
     }
 
+    private static void reloadUnauthorized(ManagementInterface client) throws IOException {
+        ModelNode op = Util.createEmptyOperation(RELOAD, PathAddress.EMPTY_ADDRESS);
+        RbacUtil.executeOperation(client, op, Outcome.UNAUTHORIZED);
+    }
+
+    private static void reloadEnhancedUnauthorized(ManagementInterface client) throws IOException {
+        ModelNode op = Util.createEmptyOperation(RELOAD_ENHANCED, PathAddress.EMPTY_ADDRESS);
+        RbacUtil.executeOperation(client, op, Outcome.UNAUTHORIZED);
+    }
+
 }