Proposal: Add kpro_connection:sasl_reauthenticate_after/2 #119
Conversation
`get_api_vsns` and `{ssl, Sock, Bin}` were missing from print_msg/3
cases and were reported as "unknown messages".
Allows authentication callback adapters to re-authenticate the connection before session lifetime expires (see the [KIP][kip]). Session lifetime is returned by the broker in [Version 1 SaslAuthenticate Response][sasl_authenticate_protocol]. Example usage pattern is given under function documentation. Re-authentication requires adding the connection config, containing SASL options, to the state record in `kpro_connection`. The `kpro_connection` process could read `session_lifetime_ms` out of the SASL authentication response and set a timer itself, but this would require quite a few changes to existing interfaces as the SASL authentication functions return just `ok`. The proposed function strikes a middle ground where adapters could use the function directly without polluting the top-level `kpro` module. [kip]: https://cwiki.apache.org/confluence/display/KAFKA/KIP-368%3A+Allow+SASL+Connections+to+Periodically+Re-Authenticate [sasl_authenticate_protocol]: https://kafka.apache.org/protocol#The_Messages_SaslAuthenticate
|
If you think it better aligns with the overall design, I could insert a case to https://github.com/kafka4beam/kafka_protocol/pull/119/files#diff-d0ce76e3ce9ae92f1e08fbcba270b20ae72e252c2c1fb716e0074e05812aebb7R293 where if the response if |
| do_sasl_reauthenticate(#state{client_id = ClientId, mod = Mod, sock = Sock, remote = {Host, _Port}, api_vsns = Versions, config = Config}) -> | ||
| %% Imitates logic in init -> connect, but using existing api_vsns and socket | ||
| Timeout = get_connect_timeout(Config), | ||
| Deadline = deadline(Timeout), | ||
| SaslOpts = get_sasl_opt(Config), | ||
| HandshakeVsn = case Versions of | ||
| #{sasl_handshake := {_, V}} -> V; | ||
| _ -> 0 | ||
| end, | ||
| ok = setopts(Sock, Mod, [{active, false}]), | ||
| ok = kpro_sasl:auth(Host, Sock, Mod, ClientId, | ||
| timeout(Deadline), SaslOpts, HandshakeVsn), | ||
| ok = setopts(Sock, Mod, [{active, once}]), | ||
| ok. |
There was a problem hiding this comment.
init_connection should call this function ?
There was a problem hiding this comment.
The calls can be unified, yes.
| #{sasl_handshake := {_, V}} -> V; | ||
| _ -> 0 | ||
| end, | ||
| ok = setopts(Sock, Mod, [{active, false}]), |
There was a problem hiding this comment.
before doing so. we should probably have to add a wait and drain all the inflight requests first ?
otherwise the kpro_sasl:auth may receive the response for previous requests and take it as an auth reply.
There was a problem hiding this comment.
That may indeed be the case. I will think about this a bit, and do some testing to verify that draining is necessary. I did not observe any draining in the reference Java client: https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/network/KafkaChannel.java#L602
There was a problem hiding this comment.
I did not study java code. but the problem for us is, the auth procedure enters a different request/response correlation (or rather the auth packets has no correlation at all).
in theory, since the auth backend is plugable, it might even enter a different framing mode
--- although currently it seems all auth mechanisms use packet-4 framing, which makes things easier.
|
Just FYI, it will take me a few days to get back to this PR. |
|
Implemented the suggestions in a new PR as the diff drifted substantially from the initial proposal. |
Allows authentication callback adapters to re-authenticate the
connection before session lifetime expires (see the KIP). Session
lifetime is returned by the broker in Version 1 SaslAuthenticate
Response.
Example usage pattern is given under function documentation.
Re-authentication requires adding the connection config, containing SASL
options, to the state record in
kpro_connection.The
kpro_connectionprocess could readsession_lifetime_msout ofthe SASL authentication response and set a timer itself, but this would
require quite a few changes to existing interfaces as the SASL
authentication functions return just
ok. The proposed function strikesa middle ground where adapters could use the function directly without
polluting the top-level
kpromodule.