strimzi / strimzi-kafka-oauth

OAuth2 support for Apache Kafka® to work with many OAuth2 authorization servers
Apache License 2.0
140 stars 89 forks source link

KeycloakAuthorizer: differentiate between authorization and ACL read/write operations #219

Open MikeEdgar opened 7 months ago

MikeEdgar commented 7 months ago

Currently, the KeycloakAuthorizer allows method invocations to be deleted to either a StandardAuthorizer or an AclAuthorizer, depending on whether a Kafka cluster is running in KRaft mode. There is no way access the ACLs when delegation is disabled. That is, any ACLs that may be present cannot be described or deleted, and new ones cannot be created. This is reasonable in most cases, but there may be instances where managing/viewing the ACLs is needed, even though they should not be used for authorization.

  1. Pre-migration from Keycloak to ACLs to allow configuration before changing authorizer
  2. Post-migration from ACLs to Keycloak, as a reference
  3. Mirrored from another cluster (?)

It would be useful to allow the ACLs to be accessed and mutated, but not used for authorization. A possible option is to add a config strimzi.authorization.delegate.kafka.acl.management (or similar) that only allows delegation to the StandardAuthorizer or AclAuthorizer for ACL read/write operations.

scholzj commented 7 months ago

I'm not sure I follow this. I would create a very weird UX if the API calls delegate to the Kafka ACLs but the authorization does not.

mstruk commented 7 months ago
  1. Pre-migration from Keycloak to ACLs to allow configuration before changing authorizer
  2. Post-migration from ACLs to Keycloak, as a reference
  3. Mirrored from another cluster (?)

You think it would not be good enough to simply enable delegateToKafkaAcls in order to cover all the listed cases (like @scholzj suggests)? When delegation is enabled the Keycloak Authorization grants are consulted first. Only if access is not allowed based on those grants, are the ACLs consulted.

It would be useful to allow the ACLs to be accessed and mutated, but not used for authorization. A possible option is to add a config strimzi.authorization.delegate.kafka.acl.management (or similar) that only allows delegation to the StandardAuthorizer or AclAuthorizer for ACL read/write operations.

Standard Kafka authorizer itself doesn't have 'ACL management only' mode. When you add an ACL it is immediately enforced. Similar for other ACL CRUD operations. Why is it a problem if the Keycloak Authorizer with delegateToKafkaAcls enabled behaves the same?

MikeEdgar commented 7 months ago

You think it would not be good enough to simply enable delegateToKafkaAcls in order to cover all the listed cases (like @scholzj suggests)?

I do think it's sufficient, yes.

Standard Kafka authorizer itself doesn't have 'ACL management only' mode. When you add an ACL it is immediately enforced. Similar for other ACL CRUD operations. Why is it a problem if the Keycloak Authorizer with delegateToKafkaAcls enabled behaves the same?

The difference I see is that with the Standard authorizer, all information (the rules) is accessible in addition to being used for enforcement. With the Keycloak Authorizer, the information stored in Kafka metadata about ACLs is effectively hidden and we can't know anything about it without changing authorization enforcement.

very weird UX if the API calls delegate to the Kafka ACLs but the authorization does not.

I think you're right, but with an additional piece of information where someone querying the ACLs knows they are not enforced, the weirdness could be addressed/mitigated.

The root of my concern is that access to information (the ACLs) is bound together with use of the information for enforcement. I think there's a benefit to allowing access to as much information as possible, provided it's made available in a way that is clear.

scholzj commented 7 months ago

I think you're right, but with an additional piece of information where someone querying the ACLs knows they are not enforced, the weirdness could be addressed/mitigated.

Yeah, but these are Kafka APIs. So we cannot just add a warning to the Kafka CLI clients and Admin API.