Skip to content

Commit

Permalink
Improve error messages when ACL creation fails due to bad principal
Browse files Browse the repository at this point in the history
Why:
User ACL management has caused some confusion and pain when users with
ACL management rights cannot create ACL permissions with themselves as
principal. The error message was eventually found but was not descriptive
enough.

MGDSTRM-9800
  • Loading branch information
robobario committed Sep 28, 2022
1 parent 1cb32ce commit 28b7ff9
Show file tree
Hide file tree
Showing 2 changed files with 97 additions and 140 deletions.
28 changes: 15 additions & 13 deletions src/main/java/io/bf2/kafka/authorizer/CustomAclAuthorizer.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@
import java.util.stream.Collectors;
import java.util.stream.IntStream;

import static java.lang.String.format;

/**
* A authorizer for Kafka that defines custom ACLs. The configuration is provided as
* a string a semicolon-delimited key/value pairs to specify
Expand Down Expand Up @@ -101,7 +103,8 @@ public class CustomAclAuthorizer implements Authorizer {
private static final int CREATE_PARTITIONS_APIKEY = 37;
private static final Logger log = LoggerFactory.getLogger(CustomAclAuthorizer.class);

static final String CREATE_ACL_INVALID_PRINCIPAL = "Invalid ACL principal name";
static final String INVALID_ACL_PRINCIPAL_RESTRICTED_TEMPLATE = "ACL rules including principal '%s' are prohibited - this principal is restricted";
static final String INVALID_ACL_PRINCIPAL_NON_USER_PREFIXED_TEMPLATE = "ACL rules including principal '%s' are prohibited - principal is not type User";
static final String CREATE_ACL_INVALID_BINDING = "Invalid ACL resource or operation";

static final String CONFIG_PREFIX = Config.PREFIX + "authorizer.";
Expand Down Expand Up @@ -421,24 +424,23 @@ public List<CompletionStage<AclCreateResult>> createAcls(AuthorizableRequestCont
.map(binding -> {
final CompletionStage<AclCreateResult> result;

if (!binding.entry().principal().startsWith(CustomAclBinding.USER_TYPE_PREFIX)) {
/* Reject ACL operations as invalid where the principal named in the ACL binding is the principal performing the operation */
log.info("Rejected attempt by user {} to create ACL binding with invalid principal name: {}",
String bindingPrincipal = binding.entry().principal();
if (!bindingPrincipal.startsWith(CustomAclBinding.USER_TYPE_PREFIX)) {
log.info("Rejected attempt by user {} to create ACL binding with incorrect prefixing: {}",
requestContext.principal().getName(),
binding.entry().principal());
result = errorResult(AclCreateResult::new, CREATE_ACL_INVALID_PRINCIPAL);
}
else if (hasPrincipalBindings(binding.entry().principal())) {
/* Reject ACL operations as invalid where the principal named in the ACL binding is a principal with configured custom ACLs */
log.info("Rejected attempt by user {} to create ACL binding for principal {} with existing custom ACL configuration",
bindingPrincipal);
result = errorResult(AclCreateResult::new, format(INVALID_ACL_PRINCIPAL_NON_USER_PREFIXED_TEMPLATE, bindingPrincipal));
} else if (hasPrincipalBindings(bindingPrincipal)) {
/* Reject ACL operations as invalid where the principal named in the ACL binding is a restricted principal with configured custom ACLs */
log.info("Rejected attempt by user {} to create ACL binding for restricted principal {}",
requestContext.principal().getName(),
binding.entry().principal());
result = errorResult(AclCreateResult::new, CREATE_ACL_INVALID_PRINCIPAL);
bindingPrincipal);
result = errorResult(AclCreateResult::new, format(INVALID_ACL_PRINCIPAL_RESTRICTED_TEMPLATE, bindingPrincipal));
} else if (!isAclBindingAllowed(binding)) {
/* Request to create an ACL that is not explicitly allowed */
log.info("Rejected attempt by user {} to create ACL binding for principal {} with existing custom ACL configuration",
requestContext.principal().getName(),
binding.entry().principal());
bindingPrincipal);
result = errorResult(AclCreateResult::new, CREATE_ACL_INVALID_BINDING);
} else {
log.debug("Delegating createAcls to parent");
Expand Down
Loading

0 comments on commit 28b7ff9

Please sign in to comment.