diff --git a/src/main/java/com/michelin/ns4kafka/controller/acl/AclController.java b/src/main/java/com/michelin/ns4kafka/controller/acl/AclController.java index 6b6bb820..f0b24ec9 100644 --- a/src/main/java/com/michelin/ns4kafka/controller/acl/AclController.java +++ b/src/main/java/com/michelin/ns4kafka/controller/acl/AclController.java @@ -3,6 +3,7 @@ import static com.michelin.ns4kafka.util.FormatErrorUtils.invalidAclDeleteOnlyAdmin; import static com.michelin.ns4kafka.util.FormatErrorUtils.invalidImmutableField; import static com.michelin.ns4kafka.util.FormatErrorUtils.invalidNotFound; +import static com.michelin.ns4kafka.util.FormatErrorUtils.invalidSelfAssignedAclDelete; import static com.michelin.ns4kafka.util.enumation.Kind.ACCESS_CONTROL_ENTRY; import static io.micronaut.core.util.StringUtils.EMPTY_STRING; @@ -58,7 +59,7 @@ public List list(String namespace, .stream() .sorted(Comparator.comparing((AccessControlEntry acl) -> acl.getMetadata().getNamespace())) .toList(); - case GRANTOR -> aclService.findAllGrantedByNamespaceByWildcardName(ns, name) + case GRANTOR -> aclService.findAllGrantedByNamespaceToOthersByWildcardName(ns, name) .stream() .sorted(Comparator.comparing(acl -> acl.getSpec().getGrantedTo())) .toList(); @@ -77,7 +78,7 @@ public List list(String namespace, * @param namespace The name * @param acl The ACL name * @return The ACL - * @deprecated use list(String, Optional ALL, String name) instead. + * @deprecated use {@link #list(String, Optional, String)} instead. */ @Get("/{acl}") @Deprecated(since = "1.12.0") @@ -94,7 +95,7 @@ public Optional get(String namespace, String acl) { * @param authentication The authentication entity * @param namespace The namespace * @param accessControlEntry The ACL - * @param dryrun Is dry run mode or not ? + * @param dryrun Is dry run mode or not? * @return An HTTP response */ @Post("{?dryrun}") @@ -152,17 +153,74 @@ public HttpResponse apply(Authentication authentication, Str return formatHttpResponse(aclService.create(accessControlEntry), status); } + /** + * Delete an ACL. + * + * @param authentication The authentication entity + * @param namespace The namespace + * @param name The ACL name parameter + * @param dryrun Is dry run mode or not? + * @return An HTTP response + */ + @Delete + @Status(HttpStatus.NO_CONTENT) + public HttpResponse bulkDelete(Authentication authentication, String namespace, + @QueryValue(defaultValue = "*") String name, + @QueryValue(defaultValue = "false") boolean dryrun) { + + Namespace ns = getNamespace(namespace); + List acls = aclService.findAllGrantedByNamespaceByWildcardName(ns, name); + List selfAssignedAcls = acls + .stream() + .filter(acl -> namespace.equals(acl.getSpec().getGrantedTo())) + .toList(); + boolean isAdmin = authentication.getRoles().contains(ResourceBasedSecurityRule.IS_ADMIN); + + if (acls.isEmpty()) { + return HttpResponse.notFound(); + } + + // If non-admin tries to delete at least one self-assigned ACL, throw validation error + if (!isAdmin && !selfAssignedAcls.isEmpty()) { + List selfAssignedAclsNames = selfAssignedAcls + .stream() + .map(acl -> acl.getMetadata().getName()) + .toList(); + throw new ResourceValidationException(ACCESS_CONTROL_ENTRY, name, + invalidSelfAssignedAclDelete(name, String.join(", ", selfAssignedAclsNames)) + ); + } + + if (dryrun) { + return HttpResponse.noContent(); + } + + acls.forEach(acl -> { + sendEventLog( + acl, + ApplyStatus.deleted, + acl.getSpec(), + null, + EMPTY_STRING); + aclService.delete(acl); + }); + + return HttpResponse.noContent(); + } + /** * Delete an ACL. * * @param authentication The authentication entity * @param namespace The namespace * @param name The ACL name - * @param dryrun Is dry run mode or not ? + * @param dryrun Is dry run mode or not? * @return An HTTP response + * @deprecated use {@link #bulkDelete(Authentication, String, String, boolean)} instead. */ @Delete("/{name}{?dryrun}") @Status(HttpStatus.NO_CONTENT) + @Deprecated(since = "1.13.0") public HttpResponse delete(Authentication authentication, String namespace, String name, @QueryValue(defaultValue = "false") boolean dryrun) { AccessControlEntry accessControlEntry = aclService diff --git a/src/main/java/com/michelin/ns4kafka/service/AclService.java b/src/main/java/com/michelin/ns4kafka/service/AclService.java index 2d3cfe0b..d476f6f9 100644 --- a/src/main/java/com/michelin/ns4kafka/service/AclService.java +++ b/src/main/java/com/michelin/ns4kafka/service/AclService.java @@ -263,12 +263,25 @@ public List findAllGrantedToNamespace(Namespace namespace) { } /** - * Find all ACLs that a given namespace granted to other namespaces. + * Find all ACLs granted by a given namespace. * * @param namespace The namespace * @return A list of ACLs */ public List findAllGrantedByNamespace(Namespace namespace) { + return accessControlEntryRepository.findAll() + .stream() + .filter(acl -> acl.getMetadata().getNamespace().equals(namespace.getMetadata().getName())) + .toList(); + } + + /** + * Find all ACLs that a given namespace granted to other namespaces. + * + * @param namespace The namespace + * @return A list of ACLs + */ + public List findAllGrantedByNamespaceToOthers(Namespace namespace) { return accessControlEntryRepository.findAll() .stream() .filter(acl -> acl.getMetadata().getNamespace().equals(namespace.getMetadata().getName())) @@ -308,7 +321,7 @@ public List findAllGrantedToNamespaceByWildcardName(Namespac } /** - * Find all ACLs that a given namespace granted to other namespaces, filtered by name parameter. + * Find all ACLs granted by a given namespace, filtered by name parameter. * * @param namespace The namespace * @param name The name parameter @@ -322,6 +335,21 @@ public List findAllGrantedByNamespaceByWildcardName(Namespac .toList(); } + /** + * Find all ACLs that a given namespace granted to other namespaces, filtered by name parameter. + * + * @param namespace The namespace + * @param name The name parameter + * @return A list of ACLs + */ + public List findAllGrantedByNamespaceToOthersByWildcardName(Namespace namespace, String name) { + List nameFilterPatterns = RegexUtils.convertWildcardStringsToRegex(List.of(name)); + return findAllGrantedByNamespaceToOthers(namespace) + .stream() + .filter(acl -> RegexUtils.isResourceCoveredByRegex(acl.getMetadata().getName(), nameFilterPatterns)) + .toList(); + } + /** * Find all ACLs that a given namespace granted to other namespaces, filtered by name parameter. * diff --git a/src/main/java/com/michelin/ns4kafka/util/FormatErrorUtils.java b/src/main/java/com/michelin/ns4kafka/util/FormatErrorUtils.java index 7b118bcd..9d22c087 100644 --- a/src/main/java/com/michelin/ns4kafka/util/FormatErrorUtils.java +++ b/src/main/java/com/michelin/ns4kafka/util/FormatErrorUtils.java @@ -50,6 +50,17 @@ public static String invalidAclDeleteOnlyAdmin(String invalidAclName) { return String.format(INVALID_FIELD, invalidAclName, FIELD_NAME, "only administrators can delete this ACL"); } + /** + * Invalid self-assigned ACL delete, authorized only by admin. + * + * @param invalidAclName the invalid ACL name + * @return the error message + */ + public static String invalidSelfAssignedAclDelete(String invalidAclName, String acls) { + return String.format(INVALID_FIELD, invalidAclName, FIELD_NAME, + "only administrators can delete the following self-assigned ACLs: " + acls); + } + /** * Invalid ACL field. * diff --git a/src/test/java/com/michelin/ns4kafka/controller/AclControllerTest.java b/src/test/java/com/michelin/ns4kafka/controller/AclControllerTest.java index 05787420..c68bb3b4 100644 --- a/src/test/java/com/michelin/ns4kafka/controller/AclControllerTest.java +++ b/src/test/java/com/michelin/ns4kafka/controller/AclControllerTest.java @@ -135,7 +135,7 @@ void shouldListAclsWithoutNameParameter() { when(aclService.findAllGrantedToNamespaceByWildcardName(namespace, "*")).thenReturn( List.of(aceTopicPrefixedOwnerAdminToTest, aceConnectPrefixedOwnerAdminToTest, aceTopicPrefixedReadNamespaceOtherToTest, aceTopicPrefixedReadAdminToAll)); - when(aclService.findAllGrantedByNamespaceByWildcardName(namespace, "*")) + when(aclService.findAllGrantedByNamespaceToOthersByWildcardName(namespace, "*")) .thenReturn(List.of(aceTopicPrefixedReadTestToNamespaceOther)); when(aclService.findAllRelatedToNamespaceByWildcardName(namespace, "*")).thenReturn( List.of(aceTopicPrefixedReadTestToNamespaceOther, aceTopicPrefixedOwnerAdminToTest, @@ -223,7 +223,7 @@ void shouldListAclsWithNameParameter() { .thenReturn(List.of(aclGrantedToNamespace)); when(aclService.findAllGrantedToNamespaceByWildcardName(namespace, "ownerAcl")) .thenReturn(List.of()); - when(aclService.findAllGrantedByNamespaceByWildcardName(namespace, "aclGrantedByNamespace")) + when(aclService.findAllGrantedByNamespaceToOthersByWildcardName(namespace, "aclGrantedByNamespace")) .thenReturn(List.of(aclGrantedByNamespace)); when(aclService.findAllGrantedToNamespaceByWildcardName(namespace, "ownerAcl")) .thenReturn(List.of()); @@ -706,7 +706,8 @@ void shouldApplyAclInDryRunMode() { } @Test - void shouldDeleteAclFailWhenNotFound() { + @SuppressWarnings("deprecation") + void shouldNotDeleteAclWhenNotFound() { Authentication authentication = Authentication.build("user", Map.of("roles", List.of())); when(aclService.findByName("test", "ace1")).thenReturn(Optional.empty()); @@ -719,7 +720,8 @@ void shouldDeleteAclFailWhenNotFound() { } @Test - void shouldDeleteSelfAssignedAclFailWhenNotAdmin() { + @SuppressWarnings("deprecation") + void shouldNotDeleteSelfAssignedAclWhenNotAdmin() { AccessControlEntry accessControlEntry = AccessControlEntry.builder() .metadata(Metadata.builder() .name("ace1") @@ -746,7 +748,8 @@ void shouldDeleteSelfAssignedAclFailWhenNotAdmin() { } @Test - void shouldDeleteSelfAssignedAclWithSuccessAsAdmin() { + @SuppressWarnings("deprecation") + void shouldDeleteSelfAssignedAclAsAdmin() { AccessControlEntry accessControlEntry = AccessControlEntry.builder() .metadata(Metadata.builder() .name("ace1") @@ -772,7 +775,8 @@ void shouldDeleteSelfAssignedAclWithSuccessAsAdmin() { } @Test - void shouldDeleteAclWithSuccess() { + @SuppressWarnings("deprecation") + void shouldDeleteAcl() { AccessControlEntry accessControlEntry = AccessControlEntry.builder() .metadata(Metadata.builder() .name("ace1") @@ -801,7 +805,8 @@ void shouldDeleteAclWithSuccess() { } @Test - void shouldDeleteInDryRunMode() { + @SuppressWarnings("deprecation") + void shouldNotDeleteInDryRunMode() { AccessControlEntry accessControlEntry = AccessControlEntry.builder() .metadata(Metadata.builder() .name("ace1") @@ -824,4 +829,188 @@ void shouldDeleteInDryRunMode() { verify(aclService, never()).delete(any()); assertEquals(HttpStatus.NO_CONTENT, actual.status()); } + + @Test + void shouldNotBulkDeleteAclWhenNotFound() { + Authentication authentication = Authentication.build("user", Map.of("roles", List.of())); + Namespace namespace = Namespace.builder() + .metadata(Metadata.builder() + .name("test") + .cluster("local") + .build()) + .build(); + + when(namespaceService.findByName("test")).thenReturn(Optional.of(namespace)); + when(aclService.findAllGrantedByNamespaceByWildcardName(namespace, "ace1")).thenReturn(List.of()); + + HttpResponse actual = accessControlListController.bulkDelete(authentication, "test", "ace1", false); + assertEquals(HttpStatus.NOT_FOUND, actual.status()); + } + + @Test + void shouldNotBulkDeleteSelfAssignedAclWhenNotAdmin() { + Namespace namespace = Namespace.builder() + .metadata(Metadata.builder() + .name("test") + .cluster("local") + .build()) + .build(); + AccessControlEntry acl1 = AccessControlEntry.builder() + .metadata(Metadata.builder() + .name("ace1") + .namespace("test") + .cluster("local") + .build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.TOPIC) + .resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED) + .permission(AccessControlEntry.Permission.OWNER) + .resource("prefix") + .grantedTo("test").build()) + .build(); + + AccessControlEntry acl2 = AccessControlEntry.builder() + .metadata(Metadata.builder() + .name("ace2") + .namespace("test") + .cluster("local") + .build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.TOPIC) + .resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED) + .permission(AccessControlEntry.Permission.READ) + .resource("prefix") + .grantedTo("other").build()) + .build(); + + Authentication authentication = Authentication.build("user", Map.of("roles", List.of())); + + when(namespaceService.findByName("test")).thenReturn(Optional.of(namespace)); + when(aclService.findAllGrantedByNamespaceByWildcardName(namespace, "ace*")).thenReturn(List.of(acl1, acl2)); + + ResourceValidationException actual = assertThrows(ResourceValidationException.class, + () -> accessControlListController.bulkDelete(authentication, "test", "ace*", false)); + + assertEquals("Invalid value \"ace*\" for field \"name\":" + + " only administrators can delete the following self-assigned ACLs: ace1.", + actual.getValidationErrors().getFirst()); + } + + @Test + void shouldBulkDeleteSelfAssignedAclAsAdmin() { + Namespace namespace = Namespace.builder() + .metadata(Metadata.builder() + .name("test") + .cluster("local") + .build()) + .build(); + AccessControlEntry accessControlEntry = AccessControlEntry.builder() + .metadata(Metadata.builder() + .name("ace1") + .namespace("test") + .cluster("local") + .build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.TOPIC) + .resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED) + .permission(AccessControlEntry.Permission.OWNER) + .resource("prefix") + .grantedTo("test").build()) + .build(); + + Authentication authentication = Authentication.build("user", List.of("isAdmin()"), + Map.of("roles", List.of("isAdmin()"))); + + when(namespaceService.findByName("test")).thenReturn(Optional.of(namespace)); + when(aclService.findAllGrantedByNamespaceByWildcardName(namespace, "ace1")) + .thenReturn(List.of(accessControlEntry)); + + HttpResponse actual = accessControlListController.bulkDelete(authentication, "test", "ace1", false); + + assertEquals(HttpStatus.NO_CONTENT, actual.status()); + } + + @Test + void shouldBulkDeleteAcl() { + Namespace namespace = Namespace.builder() + .metadata(Metadata.builder() + .name("test") + .cluster("local") + .build()) + .build(); + AccessControlEntry acl1 = AccessControlEntry.builder() + .metadata(Metadata.builder() + .name("ace1") + .namespace("test") + .cluster("local") + .build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.TOPIC) + .resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED) + .permission(AccessControlEntry.Permission.READ) + .resource("prefix") + .grantedTo("namespace-other") + .build()) + .build(); + + AccessControlEntry acl2 = AccessControlEntry.builder() + .metadata(Metadata.builder() + .name("ace2") + .namespace("test") + .cluster("local") + .build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.TOPIC) + .resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED) + .permission(AccessControlEntry.Permission.WRITE) + .resource("prefix") + .grantedTo("namespace-other2") + .build()) + .build(); + + Authentication authentication = Authentication.build("user", Map.of("roles", List.of())); + + when(namespaceService.findByName("test")).thenReturn(Optional.of(namespace)); + when(aclService.findAllGrantedByNamespaceByWildcardName(namespace, "ace*")).thenReturn(List.of(acl1, acl2)); + when(securityService.username()).thenReturn(Optional.of("test-user")); + when(securityService.hasRole(ResourceBasedSecurityRule.IS_ADMIN)).thenReturn(false); + doNothing().when(applicationEventPublisher).publishEvent(any()); + + HttpResponse actual = accessControlListController.bulkDelete(authentication, "test", "ace*", false); + + assertEquals(HttpStatus.NO_CONTENT, actual.status()); + } + + @Test + void shouldNotBulkDeleteInDryRunMode() { + Namespace namespace = Namespace.builder() + .metadata(Metadata.builder() + .name("test") + .cluster("local") + .build()) + .build(); + AccessControlEntry accessControlEntry = AccessControlEntry.builder() + .metadata(Metadata.builder() + .name("ace1") + .namespace("test") + .cluster("local") + .build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.TOPIC) + .resourcePatternType(AccessControlEntry.ResourcePatternType.PREFIXED) + .permission(AccessControlEntry.Permission.READ) + .resource("prefix") + .grantedTo("namespace-other") + .build()).build(); + + Authentication authentication = Authentication.build("user", Map.of("roles", List.of())); + + when(namespaceService.findByName("test")).thenReturn(Optional.of(namespace)); + when(aclService.findAllGrantedByNamespaceByWildcardName(namespace, "ace1")) + .thenReturn(List.of(accessControlEntry)); + HttpResponse actual = accessControlListController.bulkDelete(authentication, "test", "ace1", true); + + verify(aclService, never()).delete(any()); + assertEquals(HttpStatus.NO_CONTENT, actual.status()); + } } diff --git a/src/test/java/com/michelin/ns4kafka/service/AclServiceTest.java b/src/test/java/com/michelin/ns4kafka/service/AclServiceTest.java index fdc41c3c..54045e9a 100644 --- a/src/test/java/com/michelin/ns4kafka/service/AclServiceTest.java +++ b/src/test/java/com/michelin/ns4kafka/service/AclServiceTest.java @@ -1022,7 +1022,7 @@ void shouldFindAclGrantedByNamespaceByWildcardName() { AccessControlEntry acl2 = AccessControlEntry.builder() .metadata(Metadata.builder() - .name("acl-ns1-read-to-ns2") + .name("ns1-read-ns2") .namespace("namespace1") .build()) .spec(AccessControlEntry.AccessControlEntrySpec.builder() @@ -1066,12 +1066,83 @@ void shouldFindAclGrantedByNamespaceByWildcardName() { when(accessControlEntryRepository.findAll()).thenReturn(List.of(acl1, acl2, acl3, acl4, acl5)); - assertEquals(List.of(acl2, acl3), aclService.findAllGrantedByNamespaceByWildcardName(ns, "*")); - assertEquals(List.of(acl2), aclService.findAllGrantedByNamespaceByWildcardName(ns, "acl-ns1-read-to-ns2")); - assertEquals(List.of(acl2, acl3), aclService.findAllGrantedByNamespaceByWildcardName(ns, "*-to-ns2")); + assertEquals(List.of(acl1, acl2, acl3), aclService.findAllGrantedByNamespaceByWildcardName(ns, "*")); + assertEquals(List.of(acl2), aclService.findAllGrantedByNamespaceByWildcardName(ns, "ns1-read-ns2")); + assertEquals(List.of(acl3), aclService.findAllGrantedByNamespaceByWildcardName(ns, "*-to-ns2")); assertTrue(aclService.findAllGrantedByNamespaceByWildcardName(ns, "not-found").isEmpty()); } + @Test + void shouldFindAclGrantedByNamespaceToOthersByWildcardName() { + Namespace ns = Namespace.builder() + .metadata(Metadata.builder() + .name("namespace1") + .build()) + .build(); + + AccessControlEntry acl1 = AccessControlEntry.builder() + .metadata(Metadata.builder() + .name("ns1-acl-topic") + .namespace("namespace1") + .build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.TOPIC) + .permission(AccessControlEntry.Permission.OWNER) + .grantedTo("namespace1").build()) + .build(); + + AccessControlEntry acl2 = AccessControlEntry.builder() + .metadata(Metadata.builder() + .name("ns1-read-ns2") + .namespace("namespace1") + .build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.TOPIC) + .permission(AccessControlEntry.Permission.READ) + .grantedTo("namespace2").build()) + .build(); + + AccessControlEntry acl3 = AccessControlEntry.builder() + .metadata(Metadata.builder() + .name("ns1-connect-write-to-ns2") + .namespace("namespace1") + .build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.CONNECT) + .permission(AccessControlEntry.Permission.WRITE) + .grantedTo("namespace2").build()) + .build(); + + AccessControlEntry acl4 = AccessControlEntry.builder() + .metadata(Metadata.builder() + .name("ns2-acl-topic") + .namespace("namespace2") + .build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.TOPIC) + .permission(AccessControlEntry.Permission.OWNER) + .grantedTo("namespace2").build()) + .build(); + + AccessControlEntry acl5 = AccessControlEntry.builder() + .metadata(Metadata.builder() + .name("ns3-read-topic-all") + .namespace("namespace3") + .build()) + .spec(AccessControlEntry.AccessControlEntrySpec.builder() + .resourceType(AccessControlEntry.ResourceType.TOPIC) + .permission(AccessControlEntry.Permission.READ) + .grantedTo("*").build()) + .build(); + + when(accessControlEntryRepository.findAll()).thenReturn(List.of(acl1, acl2, acl3, acl4, acl5)); + + assertEquals(List.of(acl2, acl3), aclService.findAllGrantedByNamespaceToOthersByWildcardName(ns, "*")); + assertEquals(List.of(acl2), aclService.findAllGrantedByNamespaceToOthersByWildcardName(ns, "ns1-read-ns2")); + assertEquals(List.of(acl3), aclService.findAllGrantedByNamespaceToOthersByWildcardName(ns, "*-to-ns2")); + assertTrue(aclService.findAllGrantedByNamespaceToOthersByWildcardName(ns, "not-found").isEmpty()); + } + @Test void shouldFindAclRelatedToNamespaceByWildcardName() { AccessControlEntry acl1 = AccessControlEntry.builder()