From baeca40a62b2c7e97ee36ccde3638b5afaa1d497 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20GREFFIER?= Date: Tue, 23 Jan 2024 22:53:28 +0100 Subject: [PATCH] Grant read consumer groups by default in AKHQ v3 (#355) * Grant read to all in AKHQ v3 * Fix code smells --- .../AkhqClaimProviderController.java | 66 +++++++++++-------- .../AkhqClaimProviderControllerV3Test.java | 60 +++++++++++++++-- 2 files changed, 93 insertions(+), 33 deletions(-) diff --git a/src/main/java/com/michelin/ns4kafka/controllers/AkhqClaimProviderController.java b/src/main/java/com/michelin/ns4kafka/controllers/AkhqClaimProviderController.java index c738ea02..44cce681 100644 --- a/src/main/java/com/michelin/ns4kafka/controllers/AkhqClaimProviderController.java +++ b/src/main/java/com/michelin/ns4kafka/controllers/AkhqClaimProviderController.java @@ -110,7 +110,7 @@ public AkhqClaimResponseV2 generateClaimV2(@Valid @Body AkhqClaimRequest request return AkhqClaimResponseV2.ofAdmin(config.getFormerAdminRoles()); } - List relatedAcl = getAllAclForGroups(groups); + List relatedAcl = getAclsByGroups(groups); // Add all public ACLs. relatedAcl.addAll(accessControlEntryService.findAllPublicGrantedTo()); @@ -133,25 +133,25 @@ public AkhqClaimResponseV2 generateClaimV2(@Valid @Body AkhqClaimRequest request */ @Post("/v3") public AkhqClaimResponseV3 generateClaimV3(@Valid @Body AkhqClaimRequest request) { - final List groups = Optional.ofNullable(request.getGroups()).orElse(new ArrayList<>()); if (groups.contains(config.getAdminGroup())) { return AkhqClaimResponseV3.ofAdmin(config.getAdminRoles()); } - List relatedAcl = getAllAclForGroups(groups); + List acls = getAclsByGroups(groups); // Add all public ACLs - relatedAcl.addAll(accessControlEntryService.findAllPublicGrantedTo()); + acls.addAll(accessControlEntryService.findAllPublicGrantedTo()); - // Remove unnecessary ACLs (project.topic1 when project.* is granted on the same resource type and cluster) - optimizeAcl(relatedAcl); + // Remove unnecessary ACLs + // E.g., project.topic1 when project.* is granted on the same resource type and cluster + optimizeAcl(acls); Map bindings = new LinkedHashMap<>(); // Start by creating a map that store permissions by role/cluster - relatedAcl.forEach(acl -> { + acls.forEach(acl -> { String escapedString = Pattern.quote(acl.getSpec().getResource()); String patternRegex; @@ -188,26 +188,34 @@ public AkhqClaimResponseV3 generateClaimV3(@Valid @Body AkhqClaimRequest request List result = optimizeV3Claim(bindings); // Add the same pattern and cluster filtering for SCHEMA as the TOPIC ones - result.addAll(result.stream() - .filter(g -> g.role.equals(config.getRoles().get(AccessControlEntry.ResourceType.TOPIC))) - .map(g -> { - // Takes all the PREFIXED patterns as-is - List patterns = new ArrayList<>( - g.getPatterns().stream().filter(p -> p.endsWith("\\E.*$")).toList()); - - // Add -key or -value prefix to the schema pattern for LITERAL patterns - patterns.addAll(g.getPatterns().stream() - .filter(p -> p.endsWith("\\E$")) - .map(p -> p.replace("\\E$", "-\\E(key|value)$")) - .toList()); - - return AkhqClaimResponseV3.Group.builder() - .role(config.getRoles().get(AccessControlEntry.ResourceType.SCHEMA)) - .patterns(patterns) - .clusters(g.getClusters()) - .build(); - } - ).toList()); + result.addAll(result + .stream() + .filter(g -> g.role.equals(config.getRoles().get(AccessControlEntry.ResourceType.TOPIC))) + .map(g -> { + // Takes all the PREFIXED patterns as-is + List patterns = new ArrayList<>( + g.getPatterns().stream().filter(p -> p.endsWith("\\E.*$")).toList()); + + // Add -key or -value prefix to the schema pattern for LITERAL patterns + patterns.addAll(g.getPatterns().stream() + .filter(p -> p.endsWith("\\E$")) + .map(p -> p.replace("\\E$", "-\\E(key|value)$")) + .toList()); + + return AkhqClaimResponseV3.Group.builder() + .role(config.getRoles().get(AccessControlEntry.ResourceType.SCHEMA)) + .patterns(patterns) + .clusters(g.getClusters()) + .build(); + }) + .toList()); + + // If "GROUP" claim is present, remove the patterns to allow all the groups + result + .stream() + .filter(group -> group.getRole().equals(config.getRoles().get(AccessControlEntry.ResourceType.GROUP))) + .findFirst() + .ifPresent(group -> group.setPatterns(null)); return AkhqClaimResponseV3.builder() .groups(result.isEmpty() ? null : Map.of("group", result)) @@ -239,7 +247,7 @@ private void optimizeAcl(List acl) { /** * Optimize the claim by merging bindings patterns with the same role and clusters, etc. * - * @param bindings - the raw claim + * @param bindings the raw claim * @return an optimized claim */ private List optimizeV3Claim(Map bindings) { @@ -277,7 +285,7 @@ private List optimizeV3Claim(Map getAllAclForGroups(List groups) { + private List getAclsByGroups(List groups) { return namespaceService.listAll() .stream() .filter(namespace -> namespace.getMetadata().getLabels() != null diff --git a/src/test/java/com/michelin/ns4kafka/controllers/AkhqClaimProviderControllerV3Test.java b/src/test/java/com/michelin/ns4kafka/controllers/AkhqClaimProviderControllerV3Test.java index 43db538a..be238e07 100644 --- a/src/test/java/com/michelin/ns4kafka/controllers/AkhqClaimProviderControllerV3Test.java +++ b/src/test/java/com/michelin/ns4kafka/controllers/AkhqClaimProviderControllerV3Test.java @@ -39,10 +39,12 @@ private AkhqProperties getAkhqClaimProviderControllerConfig() { config.setAdminGroup("GP-ADMIN"); config.setRoles(Map.of(AccessControlEntry.ResourceType.TOPIC, "topic-read", AccessControlEntry.ResourceType.CONNECT, "connect-rw", - AccessControlEntry.ResourceType.SCHEMA, "registry-read")); + AccessControlEntry.ResourceType.SCHEMA, "registry-read", + AccessControlEntry.ResourceType.GROUP, "group-read")); config.setAdminRoles(Map.of(AccessControlEntry.ResourceType.TOPIC, "topic-admin", AccessControlEntry.ResourceType.CONNECT, "connect-admin", - AccessControlEntry.ResourceType.SCHEMA, "registry-admin")); + AccessControlEntry.ResourceType.SCHEMA, "registry-admin", + AccessControlEntry.ResourceType.GROUP, "group-read")); return config; } @@ -76,7 +78,7 @@ void generateClaimHappyPath() { AkhqClaimProviderController.AkhqClaimResponseV3 actual = akhqClaimProviderController.generateClaimV3(request); - Assertions.assertEquals(actual.getGroups().size(), 1); + Assertions.assertEquals(1, actual.getGroups().size()); List groups = actual.getGroups().get("group"); Assertions.assertEquals(2, groups.size()); @@ -86,6 +88,56 @@ void generateClaimHappyPath() { Assertions.assertEquals("registry-read", groups.get(1).getRole()); } + @Test + void shouldGrantAllAccessToGroup() { + Namespace ns1Cluster1 = Namespace.builder() + .metadata(ObjectMeta.builder().name("ns1").cluster("cluster1") + .labels(Map.of("support-group", "GP-PROJECT1-SUPPORT")) + .build()) + .build(); + + AccessControlEntry ace1Ns1Cluster1 = AccessControlEntry.builder() + .metadata(ObjectMeta.builder().cluster("cluster1").build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.TOPIC) + .resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED) + .resource("project1_t.") + .build()) + .build(); + + AccessControlEntry ace2Ns1Cluster1 = AccessControlEntry.builder() + .metadata(ObjectMeta.builder().cluster("cluster1").build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.GROUP) + .resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED) + .resource("project1_t.") + .build()) + .build(); + + akhqClaimProviderController.managedClusters = + List.of(new ManagedClusterProperties("cluster1"), new ManagedClusterProperties("cluster2")); + when(namespaceService.listAll()) + .thenReturn(List.of(ns1Cluster1)); + when(accessControlEntryService.findAllGrantedToNamespace(ns1Cluster1)) + .thenReturn(List.of(ace1Ns1Cluster1, ace2Ns1Cluster1)); + + AkhqClaimProviderController.AkhqClaimRequest request = AkhqClaimProviderController.AkhqClaimRequest.builder() + .groups(List.of("GP-PROJECT1-SUPPORT")) + .build(); + + AkhqClaimProviderController.AkhqClaimResponseV3 actual = akhqClaimProviderController.generateClaimV3(request); + + Assertions.assertEquals(actual.getGroups().size(), 1); + + List groups = actual.getGroups().get("group"); + Assertions.assertEquals(3, groups.size()); + Assertions.assertEquals("topic-read", groups.get(0).getRole()); + Assertions.assertEquals(List.of("^\\Qproject1_t.\\E.*$"), groups.get(0).getPatterns()); + Assertions.assertEquals(List.of("^cluster1$"), groups.get(0).getClusters()); + Assertions.assertEquals("group-read", groups.get(1).getRole()); + Assertions.assertEquals("registry-read", groups.get(2).getRole()); + } + @Test void generateClaimMultipleSupportGroups() { Namespace ns1Cluster1 = Namespace.builder() @@ -494,7 +546,7 @@ void generateClaimAndOptimizePatterns() { Assertions.assertEquals("registry-read", groups.get(2).getRole()); Assertions.assertEquals( List.of("^\\Qproject1.\\E.*$", "^\\Qproject3.\\E.*$", "^\\Qproject2.topic2-\\E(key|value)$", - "^\\Qproject2.topic2a-\\E(key|value)$", "^\\Qproject2.topic3-\\E(key|value)$"), + "^\\Qproject2.topic2a-\\E(key|value)$", "^\\Qproject2.topic3-\\E(key|value)$"), groups.get(2).getPatterns() ); }