Skip to content

Commit 899a9ea

Browse files
committed
address feedback
Signed-off-by: Emelia Lei <[email protected]>
1 parent 524583f commit 899a9ea

File tree

6 files changed

+72
-77
lines changed

6 files changed

+72
-77
lines changed

src/groups/mqb/mqba/mqba_authenticator.cpp

Lines changed: 27 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,8 @@ int Authenticator::onAuthenticationRequest(
7979
const bmqp_ctrlmsg::AuthenticationMessage& authenticationMsg,
8080
const InitialConnectionContextSp& context)
8181
{
82+
// executed by one of the *IO* threads
83+
8284
// PRECONDITIONS
8385
BSLS_ASSERT_SAFE(authenticationMsg.isAuthenticateRequestValue());
8486
BSLS_ASSERT_SAFE(context->isIncoming());
@@ -95,14 +97,13 @@ int Authenticator::onAuthenticationRequest(
9597
authenticationMsg, // authenticationMessage
9698
context
9799
->authenticationEncodingType(), // authenticationEncodingType
98-
State::e_AUTHENTICATING, // state
99-
mqbnet::ConnectionType::e_UNKNOWN // connectionType
100+
State::e_AUTHENTICATING // state
100101
);
101102

102103
context->setAuthenticationContext(authenticationContext);
103104

104105
// Authenticate
105-
int rc = authenticateAsync(errorDescription, context, context->channel());
106+
int rc = authenticateAsync(errorDescription, context);
106107

107108
return rc;
108109
}
@@ -112,6 +113,8 @@ int Authenticator::onAuthenticationResponse(
112113
BSLA_UNUSED const bmqp_ctrlmsg::AuthenticationMessage& authenticationMsg,
113114
BSLA_UNUSED const InitialConnectionContextSp& context)
114115
{
116+
// executed by one of the *IO* threads
117+
115118
BALL_LOG_ERROR << "Not Implemented";
116119

117120
return -1;
@@ -160,22 +163,25 @@ int Authenticator::sendAuthenticationMessage(
160163
return rc_SUCCESS;
161164
}
162165

163-
int Authenticator::authenticateAsync(
164-
bsl::ostream& errorDescription,
165-
const InitialConnectionContextSp& context,
166-
const bsl::shared_ptr<bmqio::Channel>& channel)
166+
int Authenticator::authenticateAsync(bsl::ostream& errorDescription,
167+
const InitialConnectionContextSp& context)
167168
{
168-
int rc = d_threadPool.enqueueJob(
169-
bdlf::BindUtil::bindS(d_allocator_p,
170-
&Authenticator::authenticate,
171-
this,
172-
context,
173-
channel));
169+
// executed by one of the *IO* threads
170+
171+
int rc = d_threadPool.enqueueJob(bdlf::BindUtil::bindS(
172+
d_allocator_p,
173+
&Authenticator::authenticate,
174+
this,
175+
context,
176+
context->channel(),
177+
context->state() ==
178+
mqbnet::InitialConnectionState::e_DEFAULT_AUTHENTICATING));
174179

175180
if (rc != 0) {
176181
errorDescription
177182
<< "Failed to enqueue authentication job for '"
178-
<< channel->peerUri() << "' [rc: " << rc << ", message: "
183+
<< context->channel()->peerUri() << "' [rc: " << rc
184+
<< ", message: "
179185
<< context->authenticationContext()->authenticationMessage()
180186
<< "]";
181187
}
@@ -185,7 +191,8 @@ int Authenticator::authenticateAsync(
185191

186192
void Authenticator::authenticate(
187193
const InitialConnectionContextSp& context,
188-
const bsl::shared_ptr<bmqio::Channel>& channel)
194+
const bsl::shared_ptr<bmqio::Channel>& channel,
195+
bool isDefaultAuthn)
189196
{
190197
// executed by an *AUTHENTICATION* thread
191198

@@ -214,7 +221,8 @@ void Authenticator::authenticate(
214221

215222
int rc = rc_SUCCESS;
216223
bsl::string error;
217-
mqbnet::InitialConnectionEvent::Enum input;
224+
mqbnet::InitialConnectionEvent::Enum input =
225+
InitialConnectionEvent::e_ERROR;
218226

219227
bdlb::ScopeExitAny handleEventOnReturn(
220228
bdlf::BindUtil::bind(&mqbnet::InitialConnectionContext::handleEvent,
@@ -242,18 +250,15 @@ void Authenticator::authenticate(
242250
if (processRc != rc_SUCCESS) {
243251
rc = (processRc * 10) + rc_PROCESS_AUTHENTICATION_FAILED;
244252
error = processErrStream.str();
245-
input = InitialConnectionEvent::e_ERROR;
246253
return; // RETURN
247254
}
248255

249256
// In the case of a default authentication, we do not need to send
250257
// an AuthenticationResponse, we just need to continue the negotiation.
251-
if (context->state() ==
252-
mqbnet::InitialConnectionState::e_DEFAULT_AUTHENTICATING) {
258+
if (isDefaultAuthn) {
253259
if (status.category() != bmqp_ctrlmsg::StatusCategory::E_SUCCESS) {
254260
rc = (status.code() * 10) + rc_AUTHENTICATION_FAILED;
255261
error = status.message();
256-
input = InitialConnectionEvent::e_ERROR;
257262
}
258263
else {
259264
input = InitialConnectionEvent::e_AUTHN_SUCCESS;
@@ -272,14 +277,12 @@ void Authenticator::authenticate(
272277
if (status.category() != bmqp_ctrlmsg::StatusCategory::E_SUCCESS) {
273278
rc = (status.code() * 10) + rc_AUTHENTICATION_FAILED;
274279
error = status.message();
275-
input = InitialConnectionEvent::e_ERROR;
276280
return; // RETURN
277281
}
278282

279283
if (sendRc != rc_SUCCESS) {
280284
rc = (sendRc * 10) + rc_SEND_AUTHENTICATION_RESPONSE_FAILED;
281285
error = sendResponseErrStream.str();
282-
input = InitialConnectionEvent::e_ERROR;
283286
return; // RETURN
284287
}
285288

@@ -496,6 +499,8 @@ int Authenticator::handleAuthentication(
496499
const InitialConnectionContextSp& context,
497500
const bmqp_ctrlmsg::AuthenticationMessage& authenticationMsg)
498501
{
502+
// executed by one of the *IO* threads
503+
499504
enum RcEnum {
500505
// Value for the various RC error categories
501506
rc_SUCCESS = 0,

src/groups/mqb/mqba/mqba_authenticator.h

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ class Authenticator : public mqbnet::Authenticator {
9797
InitialConnectionContextSp;
9898

9999
typedef mqbnet::InitialConnectionEvent InitialConnectionEvent;
100+
typedef mqbnet::InitialConnectionState InitialConnectionState;
100101

101102
private:
102103
// DATA
@@ -165,20 +166,21 @@ class Authenticator : public mqbnet::Authenticator {
165166
bmqp::EncodingType::Enum authenticationEncodingType);
166167

167168
/// Schedule an authentication job in the thread pool using the
168-
/// specified `context` and `channel`. Return 0 on success, or a
169+
/// specified `context`. Return 0 on success, or a
169170
/// non-zero error code and populate the specified `errorDescription`
170171
/// with a description of the error otherwise.
171172
int authenticateAsync(bsl::ostream& errorDescription,
172-
const InitialConnectionContextSp& context,
173-
const bsl::shared_ptr<bmqio::Channel>& channel);
173+
const InitialConnectionContextSp& context);
174174

175175
/// Authenticate the connection using the `AuthenticationMessage` stored in
176176
/// `context`. If authentication fails, invoke
177-
/// `initialConnectionCompleteCb` to close the `channel`. Also, update the
178-
/// state of `context` as appropriate. Return 0 on success, or a
179-
/// non-zero error code otherwise.
177+
/// `initialConnectionCompleteCb` to close the `channel`. Also, update the
178+
/// state of `context` as appropriate. `isDefaultAuthn` indicates if this
179+
/// is default authentication. Return 0 on success, or a non-zero error
180+
/// code otherwise.
180181
void authenticate(const InitialConnectionContextSp& context,
181-
const bsl::shared_ptr<bmqio::Channel>& channel);
182+
const bsl::shared_ptr<bmqio::Channel>& channel,
183+
bool isDefaultAuthn);
182184

183185
/// Reauthenticate the connection using the `AuthenticationMessage`
184186
/// stored in `context`. If reauthentication fails, invoke

src/groups/mqb/mqbnet/mqbnet_authenticationcontext.cpp

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ AuthenticationContext::AuthenticationContext(
102102
const bmqp_ctrlmsg::AuthenticationMessage& authenticationMessage,
103103
bmqp::EncodingType::Enum authenticationEncodingType,
104104
State state,
105-
ConnectionType::Enum connectionType,
106105
bslma::Allocator* allocator)
107106
: d_self(this) // use default allocator
108107
, d_mutex()
@@ -112,7 +111,6 @@ AuthenticationContext::AuthenticationContext(
112111
, d_initialConnectionContext_p(initialConnectionContext)
113112
, d_authenticationMessage(authenticationMessage)
114113
, d_authenticationEncodingType(authenticationEncodingType)
115-
, d_connectionType(connectionType)
116114
, d_allocator_p(allocator)
117115
{
118116
// NOTHING
@@ -142,11 +140,6 @@ void AuthenticationContext::setAuthenticationEncodingType(
142140
d_authenticationEncodingType = value;
143141
}
144142

145-
void AuthenticationContext::setConnectionType(ConnectionType::Enum value)
146-
{
147-
d_connectionType = value;
148-
}
149-
150143
int AuthenticationContext::scheduleReauthn(
151144
bsl::ostream& errorDescription,
152145
bdlmt::EventScheduler* scheduler_p,
@@ -211,7 +204,7 @@ void AuthenticationContext::onReauthenticateErrorOrTimeout(
211204
<< channel->peerUri() << "' [error: " << errorName
212205
<< ", code: " << errorCode << "]";
213206

214-
bmqio::Status status(bmqio::StatusCategory::e_GENERIC_ERROR,
207+
bmqio::Status status(bmqio::StatusCategory::e_CANCELED,
215208
errorName,
216209
errorCode,
217210
d_allocator_p);
@@ -270,10 +263,5 @@ AuthenticationContext::authenticationEncodingType() const
270263
return d_authenticationEncodingType;
271264
}
272265

273-
ConnectionType::Enum AuthenticationContext::connectionType() const
274-
{
275-
return d_connectionType;
276-
}
277-
278266
} // namespace mqbnet
279267
} // namespace BloombergLP

src/groups/mqb/mqbnet/mqbnet_authenticationcontext.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -154,8 +154,6 @@ class AuthenticationContext {
154154
/// encoding type of the received message.
155155
bmqp::EncodingType::Enum d_authenticationEncodingType;
156156

157-
ConnectionType::Enum d_connectionType;
158-
159157
/// Allocator to use.
160158
bslma::Allocator* d_allocator_p;
161159

@@ -177,7 +175,6 @@ class AuthenticationContext {
177175
const bmqp_ctrlmsg::AuthenticationMessage& authenticationMessage,
178176
bmqp::EncodingType::Enum authenticationEncodingType,
179177
State state,
180-
ConnectionType::Enum connectionType,
181178
bslma::Allocator* allocator = 0);
182179

183180
// MANIPULATORS

0 commit comments

Comments
 (0)