tabish121 commented on code in PR #5822:
URL: https://github.com/apache/activemq-artemis/pull/5822#discussion_r2190855735
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/AMQPBrokerConnection.java:
##########
@@ -1218,28 +1251,48 @@ private static final class SaslFactory implements
ClientSASLFactory {
public ClientSASL chooseMechanism(String[] offeredMechanims) {
List<String> availableMechanisms = offeredMechanims == null ?
Collections.emptyList() : Arrays.asList(offeredMechanims);
- if (availableMechanisms.contains(EXTERNAL) &&
ExternalSASLMechanism.isApplicable(connection)) {
+ List<String> acceptedMechanisms =
getAcceptedMechanisms(availableMechanisms);
+
+ if (acceptedMechanisms.contains(EXTERNAL) &&
ExternalSASLMechanism.isApplicable(connection)) {
return new ExternalSASLMechanism();
}
if (SCRAMClientSASL.isApplicable(brokerConnectConfiguration.getUser(),
brokerConnectConfiguration.getPassword())) {
for (SCRAM scram : SCRAM.values()) {
- if (availableMechanisms.contains(scram.getName())) {
+ if (acceptedMechanisms.contains(scram.getName())) {
return new SCRAMClientSASL(scram,
brokerConnectConfiguration.getUser(),
brokerConnectConfiguration.getPassword());
}
}
}
- if (availableMechanisms.contains(PLAIN) &&
PlainSASLMechanism.isApplicable(brokerConnectConfiguration.getUser(),
brokerConnectConfiguration.getPassword())) {
+
+ if (acceptedMechanisms.contains(PLAIN) &&
PlainSASLMechanism.isApplicable(brokerConnectConfiguration.getUser(),
brokerConnectConfiguration.getPassword())) {
return new
PlainSASLMechanism(brokerConnectConfiguration.getUser(),
brokerConnectConfiguration.getPassword());
}
- if (availableMechanisms.contains(ANONYMOUS)) {
+ if (acceptedMechanisms.contains(XOAUTH2) &&
XOAuth2SASLMechanism.isApplicable(brokerConnectConfiguration.getUser(),
brokerConnectConfiguration.getPassword())) {
+ return new
XOAuth2SASLMechanism(brokerConnectConfiguration.getUser(),
brokerConnectConfiguration.getPassword());
+ }
+
+ if (acceptedMechanisms.contains(ANONYMOUS)) {
return new AnonymousSASLMechanism();
}
return null;
}
+
+ private List<String> getAcceptedMechanisms(List<String>
availableMechanisms) {
+ List<String> configuredSaslMechanisms =
brokerConnectConfiguration.getSaslMechanisms();
Review Comment:
It doesn't seem necessary to add additional configuration to the broker
connection configuration as the URI handling already parses out the URI option
'saslMechanisms' when creating the ProtonProtocolManager. By default is
'saslMechanisms' isn't set a limited default set of mechanisms is used so you
would need to check the TransportConfiguration extra params to see if
'saslMechanisms' is in there but that would allow you to just use the
mechanisms set in the protocol manager or 'null' if non was set in the extra
params which avoids the need for new configuration additions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
