Feat: support for authentication#75
Conversation
5c83a2f to
ab8db41
Compare
| AuthnCredentialResult.success( | ||
| AuthnCredential.builder() | ||
| .setMechanism("ANONYMOUS") | ||
| .setData(new byte[0]) |
There was a problem hiding this comment.
We can assume that missing setData means empty byte array new byte[0], but in most cases we expect to have something in data, so this API simplification is not useful for the future use.
| SessionOptions.builder().setBrokerUri(URI.create(brokerUri)).build(), | ||
| SessionOptions.builder() | ||
| .setBrokerUri(URI.create(brokerUri)) | ||
| .setAuthnCredentialCb( |
There was a problem hiding this comment.
This callback is more convenient than C++ one.
In C++, the callback is:
typedef bsl::function<bsl::optional<AuthnCredential>(bsl::ostream& error)>
AuthnCredentialCb;
In Java, we don't want to provide a string builder arg when we can just return a "variant" type that is either a correct result or error message.
Maybe we can update the C++ implementation to match this.
a22a9e0 to
ebcff13
Compare
| /* s6 STOPPED*/ { | ||
| 1, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, 6, | ||
| /* s7 STOPPED */ { | ||
| 1, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, 7, |
There was a problem hiding this comment.
Changes here:
-
Added a new row
/* s2 AUTHENTICATING */. As a result, all values in this table that are> 2got incremented by 1. Value 2 was forNEGOTIATINGand now it is pointing toAUTHENTICATINGstate. -
In each existing row, 3 values added to the end. These values are for authentication events. We do not expect these events in any state except
AUTHENTICATINGso we do not change state if we observe them ins0-s1, s3-s7. -
We go from
AUTHENTICATINGtoNEGOTIATINGonly onAUTHENTICATION_RESPONSE
ebcff13 to
5663bc6
Compare
| */ | ||
| package com.bloomberg.bmq.impl.infr.msg; | ||
|
|
||
| public class AuthenticationResponse { |
There was a problem hiding this comment.
AuthenticationResponse is constructed by Gson during deserialization with Field.set, there is no need to provide any specific APIs to set fields such as setters or constructor with arguments
| * A message sent from a client to a broker during session initiation or reauthentication, | ||
| * indicating the client is attempting to authenticate with the supplied authentication material. | ||
| */ | ||
| public class AuthenticationRequest { |
There was a problem hiding this comment.
AuthenticationRequest is constructed in SDK.
We provide only a constructor with arguments and make it immutable after construction
5663bc6 to
a115359
Compare
| } catch (IOException | JsonSyntaxException ex) { | ||
| logger.error("Failed to decode Authentication event: ", ex); | ||
| } | ||
| authnChoice = decoded; |
There was a problem hiding this comment.
If decoding fails, we will log the error and still pass this event to FSM.
FSM handler TcpBrokerConnection::handleAuthenticationResponse will see the null authnChoice and will send AUTHENTICATION_FAILURE to FSM.
Signed-off-by: Evgeny Malygin <emalygin@bloomberg.net>
a115359 to
92ea514
Compare
| .setBrokerUri(uri) | ||
| .setAuthnCredentialCb( | ||
| () -> AuthnCredentialResult.success("ANONYMOUS", new byte[0])) | ||
| .build(); |
There was a problem hiding this comment.
This will make container ITs run with authn
Description
AuthenticationRequest/AuthenticationResponsemessages.SessionOptions.ANONYMOUSauthentication for docker bmqbrkr.tsk ITs and bmq-examples.AUTHENTICATINGstate, handle authentication events.Design decisions
Authentication auto enablement
I decided not to add
ANONYMOUSauthentication callback to the defaultSessionOptionsbecause it breaks a half of the existing tests. We have 2 types of ITs in SDK: the ones with real broker and the ones with mock broker simulator. The real broker works fine, the mocks require not-trivial updating. I will do it in the next PR.AuthnCredential callback
a. C++ mirror (Optional, StringBuilder):
Java already supports nullable references so there is no real need to use
Optional, we can just return null. Also it is not very convenient to provide a string builder to the call.b. Builder variant:
AuthnCredentialResultholds either a valid credential or an error message.c. Hidden builder variant - implemented:
The protocol is fixed, so we can rely on the fact that we will always have a mechanism and a data. It is unlikely that we will be able to extend the protocol easily. So we can hide the
AuthnCredentialbuilder and makeAuthnCredentialResultbuild easier.