[
https://issues.apache.org/jira/browse/ARTEMIS-5316?focusedWorklogId=971160&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-971160
]
ASF GitHub Bot logged work on ARTEMIS-5316:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 29/May/25 16:46
Start Date: 29/May/25 16:46
Worklog Time Spent: 10m
Work Description: tabish121 commented on code in PR #5721:
URL: https://github.com/apache/activemq-artemis/pull/5721#discussion_r2114350476
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/AMQPBrokerConnection.java:
##########
@@ -1218,28 +1251,53 @@ 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) {
+ TransportConfiguration connectorConfig =
connection.getConnectorConfig();
+ if(connectorConfig == null){
Review Comment:
I believe the formatting in this method breaks checkstyle rules which needs
to be fixed beyond fixing the bit about not putting the sasl bits into the
netty transport options
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/AMQPConnectSaslTest.java:
##########
@@ -115,6 +115,31 @@ public void testConnectsWithPlain() throws Exception {
}
}
+ @Test
Review Comment:
Testing is insufficient as there are routes through the is applicable code
that involved checking if user and passwords are set or not which should be
tested.
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/remoting/impl/netty/TransportConstants.java:
##########
@@ -536,6 +538,7 @@ private static int parseDefaultVariable(String
variableName, int defaultValue) {
allowableConnectorKeys.add(TransportConstants.TRUST_MANAGER_FACTORY_PLUGIN_PROP_NAME);
allowableConnectorKeys.add(TransportConstants.HANDSHAKE_TIMEOUT);
allowableConnectorKeys.add(TransportConstants.CRL_PATH_PROP_NAME);
+ allowableConnectorKeys.add(TransportConstants.SASL_MECHANISMS);
Review Comment:
Same issue as before, we should not be adding SASL mechanisms to netty
options as AMQP is the protocol that's implementing the SASL bits, others do
not so the options should be handled as other AMQP specific options are.
Issue Time Tracking
-------------------
Worklog Id: (was: 971160)
Time Spent: 1h 10m (was: 1h)
> Support for SASL XOAUTH2 Mechanism in Broker Connection
> -------------------------------------------------------
>
> Key: ARTEMIS-5316
> URL: https://issues.apache.org/jira/browse/ARTEMIS-5316
> Project: ActiveMQ Artemis
> Issue Type: New Feature
> Reporter: Tomasz Łukasiewicz
> Priority: Major
> Labels: pull-request-available
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> There is a need to support XOAUTH2 authentication between two AMQP brokers,
> as the existing mechanisms are not sufficiently secure for certain use cases.
> Currently, Artemis does not support this authentication method on the client
> side, and the SaslFactory implementation is both private and final, making it
> impossible to extend.
> To address this, an XOAuth2SASLMechanism should be implemented within the
> AMQPBrokerConnection class and integrated into the SaslFactory. The new SASL
> mechanism should return its name as "XOAUTH2" and include the appropriate
> authentication headers.
> A working example of this approach has been successfully tested with the
> Solace broker:
> {code:java}
> @Override
> public byte[] getInitialResponse() {
> String response = String.format("user=%s\u0001auth=Bearer %s\u0001\u0001",
> userName, token);
> return response.getBytes(StandardCharsets.UTF_8);
> }
> {code}
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
