Conversation
puellanivis
reviewed
Sep 8, 2025
Collaborator
puellanivis
left a comment
There was a problem hiding this comment.
Understanding this is a draft. I mean, it all looks pretty good so far. I think all I commented on was style things. 😂
gssapi_kerberos.go
Outdated
| } | ||
|
|
||
| // AuthorizeV2 performs the SASL v2 GSSAPI authentication with the Kafka broker. | ||
| func (krbAuth *GSSAPIKerberosAuth) AuthorizeV2(broker *Broker, authSendReceiver func(authBytes []byte) (*SaslAuthenticateResponse, error)) error { |
Collaborator
There was a problem hiding this comment.
😬 I’m not sure how I feel about passing in a fairly arbitrary function here.
Collaborator
Author
There was a problem hiding this comment.
Yeah as you've probably seen, this is mimicking what the other b.sendAndReceiveSASLFoo funcs currently accept and use and was just the simplest way to get a V2 flow working
Closed
For some reason we were still pinning the old v0 SASL when using GSSAPI and this doesn't work if an ApiVersionsRequest is sent before the auth flow. Add support for sending the krb5 bytes in the v1 SASL SaslAuthenticate protocol wrapping. Signed-off-by: Dominic Evans <[email protected]>
Signed-off-by: Dominic Evans <[email protected]>
Signed-off-by: Dominic Evans <[email protected]>
Use the simpler parameter name consistently. Also re-order params for initSecContext Signed-off-by: Dominic Evans <[email protected]>
Also use consistent func doc comment format Signed-off-by: Dominic Evans <[email protected]>
hindessm
approved these changes
Sep 17, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For some reason we were still pinning the old v0 SASL when using GSSAPI and this doesn't work if an ApiVersionsRequest is sent before the auth flow.
Add support for sending the krb5 bytes in the v1 SASL SaslAuthenticate protocol wrapping.
Note: this is just a draft for now as although it is tested and working against a GSSAPI enabled cluster, the underlying code is very much a copy-and-paste of the existing v0 auth flow, just wrappering the bytes in the SaslAuthenticate rather than sending them to the broker directly. This needs more work to tidyup the implementation