Rebalance OIDC tests, security config validation

Signed-off-by: Michael Edgar <[email protected]>
streamshub · Nov 8, 2024 · c39cae9 · c39cae9
1 parent 9bef0e9
commit c39cae9
Show file tree

Hide file tree

Showing 22 changed files with 395 additions and 51 deletions.
diff --git a/api/pom.xml b/api/pom.xml
@@ -320,7 +320,7 @@
                             <excludes>
                                 <exclude>com/github/streamshub/console/api/support/TrustAllCertificateManager.class</exclude>
                             </excludes>
-                            <exclClassLoaders>*QuarkusClassLoader</exclClassLoaders>
+                            <!--<exclClassLoaders>*QuarkusClassLoader</exclClassLoaders>-->
                             <destFile>${project.build.directory}/jacoco-quarkus.exec</destFile>
                             <append>true</append>
                         </configuration>
@@ -334,7 +334,7 @@
                             <excludes>
                                 <exclude>com/github/streamshub/console/api/support/TrustAllCertificateManager.class</exclude>
                             </excludes>
-                            <exclClassLoaders>*QuarkusClassLoader</exclClassLoaders>
+                            <!--<exclClassLoaders>*QuarkusClassLoader</exclClassLoaders>-->
                             <destFile>${project.build.directory}/jacoco-quarkus.exec</destFile>
                             <append>true</append>
                         </configuration>

diff --git a/.../main/java/com/github/streamshub/console/api/security/ConsoleAuthenticationMechanism.java b/.../main/java/com/github/streamshub/console/api/security/ConsoleAuthenticationMechanism.java
@@ -340,7 +340,7 @@ private void addRoleChecker(KafkaContext ctx, QuarkusSecurityIdentity.Builder bu
 
         Stream<Permission> globalPermissions = getPermissions(globalSecurity, roleNames, "");
         Stream<Permission> clusterPermissions = clusterSecurity
-                .map(cs -> getPermissions(cs, roleNames, "kafkas/" + ctx.clusterId() + '/'))
+                .map(cs -> getPermissions(cs, roleNames, "kafkas/" + ctx.clusterConfig().getName() + '/'))
                 .orElseGet(Stream::empty);
 
         List<Permission> possessedPermissions = Stream.concat(globalPermissions, clusterPermissions).toList();

diff --git a/api/src/main/java/com/github/streamshub/console/api/security/ConsolePermission.java b/api/src/main/java/com/github/streamshub/console/api/security/ConsolePermission.java
@@ -36,19 +36,19 @@ public ConsolePermission(String resource, Collection<String> resourceNames, Priv
     }
 
     private static Set<Privilege> checkActions(Privilege[] actions) {
-        Set<Privilege> validActions = new HashSet<>(actions.length, 1);
-        for (Privilege action : actions) {
-            validActions.add(validateAndTrim(action, "Action"));
+        Objects.requireNonNull(actions);
+
+        if (actions.length == 0) {
+            throw new IllegalArgumentException("actions must not be zero length");
         }
-        return Collections.unmodifiableSet(validActions);
-    }
 
-    private static Privilege validateAndTrim(Privilege action, String paramName) {
-        if (action == null) {
-            throw new IllegalArgumentException(String.format("%s must not be null", paramName));
+        Set<Privilege> validActions = new HashSet<>(actions.length, 1);
+
+        for (Privilege action : actions) {
+            validActions.add(Objects.requireNonNull(action));
         }
 
-        return action;
+        return Collections.unmodifiableSet(validActions);
     }
 
     public ConsolePermission resourceName(String resourceName) {
@@ -74,18 +74,6 @@ boolean implies(ConsolePermission requiredPermission) {
             return false;
         }
 
-        // actions are optional, however if at least one action was specified,
-        // an intersection of compared sets must not be empty
-        if (requiredPermission.actions.isEmpty()) {
-            // no required actions
-            return true;
-        }
-
-        if (actions.isEmpty()) {
-            // no possessed actions
-            return false;
-        }
-
         if (actions.contains(Privilege.ALL)) {
             // all actions possessed
             return true;

diff --git a/api/src/main/java/com/github/streamshub/console/api/service/TopicDescribeService.java b/api/src/main/java/com/github/streamshub/console/api/service/TopicDescribeService.java
@@ -284,7 +284,7 @@ private CompletableFuture<Void> maybeFetchConsumerGroups(Map<Uuid, Topic> topics
 
         return consumerGroupService
                 .listConsumerGroupMembership(searchTopics)
-                .thenAccept(consumerGroups -> {
+                .thenAccept(consumerGroups ->
                     topics.forEach((topicId, topic) -> {
                         String idString = topicId.toString();
 
@@ -294,8 +294,8 @@ private CompletableFuture<Void> maybeFetchConsumerGroups(Map<Uuid, Topic> topics
                             topic.consumerGroups().data().addAll(identifiers);
                             topic.consumerGroups().addMeta("count", identifiers.size());
                         }
-                    });
-                }).toCompletableFuture();
+                    }))
+                .toCompletableFuture();
     }
 
     /* package */ CompletionStage<Map<Uuid, Either<Topic, Throwable>>> describeTopics(

diff --git a/api/src/test/java/com/github/streamshub/console/api/BrokersResourceIT.java b/api/src/test/java/com/github/streamshub/console/api/BrokersResourceIT.java
@@ -63,6 +63,8 @@ void setup() {
         utils = new TestHelper(bootstrapServers, config, null);
 
         client.resources(Kafka.class).inAnyNamespace().delete();
+        consoleConfig.clearSecurity();
+
         utils.apply(client, new KafkaBuilder()
                 .withNewMetadata()
                     .withName("test-kafka1")

diff --git a/api/src/test/java/com/github/streamshub/console/api/ConsumerGroupsResourceIT.java b/api/src/test/java/com/github/streamshub/console/api/ConsumerGroupsResourceIT.java
@@ -125,6 +125,8 @@ void setup() throws IOException {
         utils = new TestHelper(bootstrapServers, config, null);
 
         client.resources(Kafka.class).inAnyNamespace().delete();
+        consoleConfig.clearSecurity();
+
         utils.apply(client, utils.buildKafkaResource("test-kafka1", utils.getClusterId(), bootstrapServers));
 
         // Wait for the informer cache to be populated with all Kafka CRs

diff --git a/api/src/test/java/com/github/streamshub/console/api/KafkaClustersResourceIT.java b/api/src/test/java/com/github/streamshub/console/api/KafkaClustersResourceIT.java
@@ -44,7 +44,6 @@
 import com.github.streamshub.console.api.support.KafkaContext;
 import com.github.streamshub.console.config.ConsoleConfig;
 import com.github.streamshub.console.config.KafkaClusterConfig;
-import com.github.streamshub.console.config.security.GlobalSecurityConfig;
 import com.github.streamshub.console.config.security.GlobalSecurityConfigBuilder;
 import com.github.streamshub.console.config.security.Privilege;
 import com.github.streamshub.console.kafka.systemtest.TestPlainProfile;
@@ -136,10 +135,10 @@ void setup() throws IOException {
                 .map(k -> k.getProperties().get("bootstrap.servers"))
                 .orElseThrow());
 
-        consoleConfig.setSecurity(new GlobalSecurityConfig());
         utils = new TestHelper(bootstrapServers, config, null);
 
         client.resources(Kafka.class).inAnyNamespace().delete();
+        consoleConfig.clearSecurity();
 
         utils.apply(client, new KafkaBuilder(utils.buildKafkaResource("test-kafka1", utils.getClusterId(), bootstrapServers,
                         new KafkaListenerAuthenticationCustomBuilder()

diff --git a/api/src/test/java/com/github/streamshub/console/api/KafkaClustersResourceNoK8sIT.java b/api/src/test/java/com/github/streamshub/console/api/KafkaClustersResourceNoK8sIT.java
@@ -66,6 +66,7 @@ void setup() throws IOException {
                 .map(k -> k.getProperties().get("bootstrap.servers"))
                 .orElseThrow());
 
+        consoleConfig.clearSecurity();
         utils = new TestHelper(bootstrapServers, config, null);
 
         clusterId1 = consoleConfig.getKafka().getCluster("test-kafka1").get().getId();

diff --git a/api/src/test/java/com/github/streamshub/console/api/KafkaClustersResourceOidcIT.java b/api/src/test/java/com/github/streamshub/console/api/KafkaClustersResourceOidcIT.java
@@ -15,7 +15,6 @@
 import org.apache.kafka.clients.CommonClientConfigs;
 import org.apache.kafka.common.security.auth.SecurityProtocol;
 import org.eclipse.microprofile.config.Config;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -25,7 +24,6 @@
 import com.github.streamshub.console.api.support.KafkaContext;
 import com.github.streamshub.console.config.ConsoleConfig;
 import com.github.streamshub.console.config.KafkaClusterConfig;
-import com.github.streamshub.console.config.security.GlobalSecurityConfig;
 import com.github.streamshub.console.config.security.GlobalSecurityConfigBuilder;
 import com.github.streamshub.console.config.security.Privilege;
 import com.github.streamshub.console.kafka.systemtest.TestPlainProfile;
@@ -86,6 +84,7 @@ void setup() throws IOException {
         tokens = new TokenUtils(config);
 
         client.resources(Kafka.class).inAnyNamespace().delete();
+        consoleConfig.clearSecurity();
 
         Kafka kafka1 = new KafkaBuilder(utils.buildKafkaResource("test-kafka1", utils.getClusterId(), bootstrapServers))
             .editOrNewStatus()
@@ -127,12 +126,6 @@ void setup() throws IOException {
         clusterId1 = consoleConfig.getKafka().getCluster("default/test-kafka1").get().getId();
     }
 
-    @AfterEach
-    void teardown() throws IOException {
-        client.resources(Kafka.class).inAnyNamespace().delete();
-        consoleConfig.setSecurity(new GlobalSecurityConfig());
-    }
-
     @Test
     void testListClustersWithNoRolesDefined() {
         consoleConfig.setSecurity(oidcSecurity().build());

diff --git a/api/src/test/java/com/github/streamshub/console/api/KafkaRebalancesResourceIT.java b/api/src/test/java/com/github/streamshub/console/api/KafkaRebalancesResourceIT.java
@@ -14,7 +14,6 @@
 import jakarta.ws.rs.core.Response.Status;
 
 import org.eclipse.microprofile.config.Config;
-import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -121,6 +120,7 @@ void setup() {
 
         client.resources(Kafka.class).inAnyNamespace().delete();
         client.resources(KafkaRebalance.class).inAnyNamespace().delete();
+        consoleConfig.clearSecurity();
 
         utils.apply(client, new KafkaBuilder(utils.buildKafkaResource("test-kafka1", utils.getClusterId(), bootstrapServers))
                 .editSpec()
@@ -160,12 +160,6 @@ void setup() {
         clusterId2 = consoleConfig.getKafka().getCluster("default/test-kafka2").get().getId();
     }
 
-    @AfterEach
-    void tearDown() {
-        client.resources(Kafka.class).inAnyNamespace().delete();
-        client.resources(KafkaRebalance.class).inAnyNamespace().delete();
-    }
-
     @Test
     void testListRebalancesIncludesAllowedActions() {
         whenRequesting(req -> req.get("", clusterId1))