Skip to content

Commit

Permalink
Grant read consumer groups by default in AKHQ v3 (#355)
Browse files Browse the repository at this point in the history
* Grant read to all in AKHQ v3

* Fix code smells
  • Loading branch information
loicgreffier authored Jan 23, 2024
1 parent aa36eb7 commit baeca40
Show file tree
Hide file tree
Showing 2 changed files with 93 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ public AkhqClaimResponseV2 generateClaimV2(@Valid @Body AkhqClaimRequest request
return AkhqClaimResponseV2.ofAdmin(config.getFormerAdminRoles());
}

List<AccessControlEntry> relatedAcl = getAllAclForGroups(groups);
List<AccessControlEntry> relatedAcl = getAclsByGroups(groups);

// Add all public ACLs.
relatedAcl.addAll(accessControlEntryService.findAllPublicGrantedTo());
Expand All @@ -133,25 +133,25 @@ public AkhqClaimResponseV2 generateClaimV2(@Valid @Body AkhqClaimRequest request
*/
@Post("/v3")
public AkhqClaimResponseV3 generateClaimV3(@Valid @Body AkhqClaimRequest request) {

final List<String> groups = Optional.ofNullable(request.getGroups()).orElse(new ArrayList<>());

if (groups.contains(config.getAdminGroup())) {
return AkhqClaimResponseV3.ofAdmin(config.getAdminRoles());
}

List<AccessControlEntry> relatedAcl = getAllAclForGroups(groups);
List<AccessControlEntry> 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<String, AkhqClaimResponseV3.Group> 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;

Expand Down Expand Up @@ -188,26 +188,34 @@ public AkhqClaimResponseV3 generateClaimV3(@Valid @Body AkhqClaimRequest request
List<AkhqClaimResponseV3.Group> 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<String> 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<String> 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))
Expand Down Expand Up @@ -239,7 +247,7 @@ private void optimizeAcl(List<AccessControlEntry> 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<AkhqClaimResponseV3.Group> optimizeV3Claim(Map<String, AkhqClaimResponseV3.Group> bindings) {
Expand Down Expand Up @@ -277,7 +285,7 @@ private List<AkhqClaimResponseV3.Group> optimizeV3Claim(Map<String, AkhqClaimRes
* @param groups the user LDAP groups
* @return the user's ACL
*/
private List<AccessControlEntry> getAllAclForGroups(List<String> groups) {
private List<AccessControlEntry> getAclsByGroups(List<String> groups) {
return namespaceService.listAll()
.stream()
.filter(namespace -> namespace.getMetadata().getLabels() != null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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<AkhqClaimProviderController.AkhqClaimResponseV3.Group> groups = actual.getGroups().get("group");
Assertions.assertEquals(2, groups.size());
Expand All @@ -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<AkhqClaimProviderController.AkhqClaimResponseV3.Group> 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()
Expand Down Expand Up @@ -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()
);
}
Expand Down

0 comments on commit baeca40

Please sign in to comment.