Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OAuth with KafkaUser and simple authorization #135

Open
corlettb opened this issue Jan 19, 2022 · 12 comments
Open

OAuth with KafkaUser and simple authorization #135

corlettb opened this issue Jan 19, 2022 · 12 comments

Comments

@corlettb
Copy link

I'm trying to set up oauth for authentication (Azure AD) but use simple authorization.

I have the authenitcation/authorisation working. My issues are around linking the simple authorization through KafkaUser crds.

I realised you could set the KafkaUser without an authentication section and it would just create the permissions.

Sadly my issue comes down to the lack of a spec.kafkaUser setting like the way the KafkaTopic has. I understand there are reasons not to have this to avoid conflicts. However without this functionaltion it is causing me more issues.

  1. I need to run multiple clusters in the same namespace (sadly not up to me). This will create conflicts if I try to add the same user to multiple clusters.

  2. Oauth user unique_name in Azure AD come out as email addresses. So [email protected]. When trying to apply a KafkaUser with that name you get the classic:

failed to create resource: KafkaUser.kafka.strimzi.io "[email protected]" is invalid: metadata.name: Invalid value: "[email protected]": a lowercase RFC 1123 subdomain must consist of lower case alphanumeric characters, '-' or '.', and must start and end with an alphanumeric character (e.g. 'example.com', regex used for validation is 'a-z0-9?(.a-z0-9?)*')

I tried experimenting with the following settings:
userNameClaim: unique_name
fallbackUserNameClaim: sub
fallbackUserNamePrefix: clustername-

This would mean jwk auth requests without a unique_name would avoid point 1. (e.g. username comes out as clustername-11111111-1111-1111-1111-111111111111), however not so much on point 2.

I tried forking with the following hack:

--- a/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/PrincipalExtractor.java
+++ b/oauth-common/src/main/java/io/strimzi/kafka/oauth/common/PrincipalExtractor.java
@@ -28,7 +28,8 @@ public class PrincipalExtractor {
         if (usernameClaim != null) {
             result = getClaimFromJWT(json, usernameClaim);
             if (result != null) {
-                return result;
+                result = result.replaceAll("@.+$", "");
+                return fallbackUsernamePrefix + result;
             }

             if (fallbackUsernameClaim != null) {

Which made the usernames come out like clustername-username. Which I can make KafkaUsers from.

Although I think having a spec.KafkaUser would be a better solution do you think we could add a "substitution regex" option in the code and crds?

This would allow us to map our usernames to ones that we can create with KafkaUsers.

@mstruk
Copy link
Contributor

mstruk commented Jan 19, 2022

Thank you for a thorough description of your problem.

Actually, could you open another issue in Strimzi Operator since it's an issue that also concerns KafkaUser CR, and a possibly more elegant solution could be achieved through changes there?

Looks like the substitution regex config option would make perfect sense.

@scholzj
Copy link
Member

scholzj commented Jan 19, 2022

Although I think having a spec.KafkaUser would be a better solution do you think we could add a "substitution regex" option in the code and crds?

We do not plan to support any custom usernames in the KafkaUser resource. It adds a lot of complexity and causes problems because it means there is no unique identification for the user. Please keep in mind that you do not need to use the User Operator. You can just disable it (by removing the userOperator section in the Kafka CR) and managing the ACLs and Quotas using the Kafka APIs and use whatever username you prefer.

@mstruk
Copy link
Contributor

mstruk commented Jan 19, 2022

We do not plan to support any custom usernames in the KafkaUser resource.

Ok, we can still add it in strimzi-kafka-oauth and expose extra config in Kafka CR oauth authentication section.

@scholzj
Copy link
Member

scholzj commented Jan 19, 2022

Ok, we can still add it in strimzi-kafka-oauth and expose extra config in Kafka CR oauth authentication section.

It is road to hell as proved by the Topic Operator. As I said, the User Operator is optional. It might not be able to cover all usecases, but users don't have to use it.

@mstruk
Copy link
Contributor

mstruk commented Jan 19, 2022

As I said, the User Operator is optional. It might not be able to cover all usecases, but users don't have to use it.

So in that case the username containing the '@' would not be a problem and the need for the substitution goes away.

@corlettb
Copy link
Author

Thank you for your fast responses.

I would like to use strimzi for managing ACL's through kubernetes if I can rather than going directly to the kafka api.

Maybe another idea for a better fix. A KafkaAcl crd which is similar to the KafkaUser crd however it has a spec.KafkaUser option and no authentication option.

KafkaAcl will be applied as a union. So you could have one KafkaUser and 10 KafkaAcls for example. Same sort of pattern as terraform and aws_security_group and aws_security_group_rule.

Think of a scenario like you have a connect cluster but seperate deploys for your connectors. Each of the connectors need additional access to topics so would deploy their own KafkaAcl.

This would also mean for OAuth users they could just use KafkaAcls.

Granted you could get yourself into trouble if two KafkaAcls gives the same permission. You could dectect this in the code and throw out an error and refuse to do anything with the user until the conflict is resolved.

Failing that would you be okay with an oauth regex option?

Thoughts?

@scholzj
Copy link
Member

scholzj commented Jan 19, 2022

But that causes the same problems in whatever operator processes the KafkaAcl resource. The key if you want to use the KafkaUser for it is to make sure your usernames are valid Kubernetes names. That is the right way to use it.

@corlettb
Copy link
Author

Obviously oauth users are outside of kubernetes. Not something I can change. Would you be okay with adding an optional regex to the oauth options in the kafka crd then?

The regex can be used to transform the oauth user to a kubernetes friendly name.

@scholzj
Copy link
Member

scholzj commented Jan 19, 2022

I do not have problem with that => but for the Oauth library, @mstruk is the one to consider it.

@corlettb
Copy link
Author

There would need to be some changes in the strimzi operator. Add the option to the Kafka CRD and write it into the kafka server.properties file if the option is set. @mstruk is this solution something you would accept?

@mstruk
Copy link
Contributor

mstruk commented Jan 19, 2022

That's the approach I had in mind when I said:

Ok, we can still add it in strimzi-kafka-oauth and expose extra config in Kafka CR oauth authentication section.

It is really just another principal name mapping facility in addition to existing fallbackXXX mechanisms. It didn't seem like something anyone would need or want at the time, but now there is a real use-case for it.

@scholzj
Copy link
Member

scholzj commented Jan 19, 2022

That's the approach I had in mind when I said: ...

Ahh, sorry. I misunderstood that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants