[
https://issues.apache.org/jira/browse/ARTEMIS-5316?focusedWorklogId=973896&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-973896
]
ASF GitHub Bot logged work on ARTEMIS-5316:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 08/Jul/25 22:54
Start Date: 08/Jul/25 22:54
Worklog Time Spent: 10m
Work Description: tabish121 commented on code in PR #5822:
URL: https://github.com/apache/activemq-artemis/pull/5822#discussion_r2193561268
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/AMQPBrokerConnection.java:
##########
@@ -1204,42 +1208,95 @@ public static boolean isApplicable(final
NettyConnection connection) {
}
}
+ private static class XOAuth2SASLMechanism implements ClientSASL {
+
+ private final String userName;
+ private final String token;
+
+ public XOAuth2SASLMechanism(String userName, String token) {
+ this.userName = userName;
+ this.token = token;
+ }
+
+ @Override
+ public String getName() {
+ return XOAUTH2;
+ }
+
+ @Override
+ public byte[] getInitialResponse() {
+ String response = String.format("user=%s\u0001auth=Bearer
%s\u0001\u0001", userName, token);
+ return response.getBytes(StandardCharsets.UTF_8);
+ }
+
+ @Override
+ public byte[] getResponse(byte[] challenge) {
+ return EMPTY;
+ }
+
+ public static boolean isApplicable(final String username, final String
password) {
+ return StringUtils.isNoneEmpty(username, password);
+ }
+ }
+
private static final class SaslFactory implements ClientSASLFactory {
private final NettyConnection connection;
private final AMQPBrokerConnectConfiguration brokerConnectConfiguration;
+ private final String[] availableSaslMechanisms;
- SaslFactory(NettyConnection connection, AMQPBrokerConnectConfiguration
brokerConnectConfiguration) {
+ SaslFactory(NettyConnection connection, AMQPBrokerConnectConfiguration
brokerConnectConfiguration, String[] availableSaslMechanisms) {
this.connection = connection;
this.brokerConnectConfiguration = brokerConnectConfiguration;
+ this.availableSaslMechanisms = availableSaslMechanisms;
}
@Override
- public ClientSASL chooseMechanism(String[] offeredMechanims) {
- List<String> availableMechanisms = offeredMechanims == null ?
Collections.emptyList() : Arrays.asList(offeredMechanims);
+ public ClientSASL chooseMechanism(String[] offeredMechanisms) {
+ Set<String> acceptedMechanisms =
getAcceptedMechanisms(offeredMechanisms, availableSaslMechanisms);
- if (availableMechanisms.contains(EXTERNAL) &&
ExternalSASLMechanism.isApplicable(connection)) {
+ 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 Set<String> getAcceptedMechanisms(String[]
offeredSaslMechanisms, String[] availableSaslMechanisms) {
+ Set<String> offeredSaslMechanismsSet = offeredSaslMechanisms == null
? Set.of() : new HashSet<>(Arrays.asList(offeredSaslMechanisms));
+ Set<String> availableSaslMechanismsSet = availableSaslMechanisms ==
null ? Set.of() : new HashSet<>(Arrays.asList(availableSaslMechanisms));
Review Comment:
This could be refactored to not allocate the enabled set of mechanisms
unless the array is non-null and not empty as it would be most of the time. It
also wouldn't be necessary to create yet another Set for the result if instead
of `Set.of()` for the offered set you used Collections.emptySet() which will
just no-op on retainAll as it is treated as empty.
##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/AMQPBrokerConnection.java:
##########
@@ -1204,42 +1208,95 @@ public static boolean isApplicable(final
NettyConnection connection) {
}
}
+ private static class XOAuth2SASLMechanism implements ClientSASL {
+
+ private final String userName;
+ private final String token;
+
+ public XOAuth2SASLMechanism(String userName, String token) {
+ this.userName = userName;
+ this.token = token;
+ }
+
+ @Override
+ public String getName() {
+ return XOAUTH2;
+ }
+
+ @Override
+ public byte[] getInitialResponse() {
+ String response = String.format("user=%s\u0001auth=Bearer
%s\u0001\u0001", userName, token);
+ return response.getBytes(StandardCharsets.UTF_8);
+ }
+
+ @Override
+ public byte[] getResponse(byte[] challenge) {
+ return EMPTY;
+ }
+
+ public static boolean isApplicable(final String username, final String
password) {
+ return StringUtils.isNoneEmpty(username, password);
+ }
+ }
+
private static final class SaslFactory implements ClientSASLFactory {
private final NettyConnection connection;
private final AMQPBrokerConnectConfiguration brokerConnectConfiguration;
+ private final String[] availableSaslMechanisms;
- SaslFactory(NettyConnection connection, AMQPBrokerConnectConfiguration
brokerConnectConfiguration) {
+ SaslFactory(NettyConnection connection, AMQPBrokerConnectConfiguration
brokerConnectConfiguration, String[] availableSaslMechanisms) {
this.connection = connection;
this.brokerConnectConfiguration = brokerConnectConfiguration;
+ this.availableSaslMechanisms = availableSaslMechanisms;
}
@Override
- public ClientSASL chooseMechanism(String[] offeredMechanims) {
- List<String> availableMechanisms = offeredMechanims == null ?
Collections.emptyList() : Arrays.asList(offeredMechanims);
+ public ClientSASL chooseMechanism(String[] offeredMechanisms) {
+ Set<String> acceptedMechanisms =
getAcceptedMechanisms(offeredMechanisms, availableSaslMechanisms);
- if (availableMechanisms.contains(EXTERNAL) &&
ExternalSASLMechanism.isApplicable(connection)) {
+ 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 Set<String> getAcceptedMechanisms(String[]
offeredSaslMechanisms, String[] availableSaslMechanisms) {
Review Comment:
As Dom pointed out above using enabledMechanisms probably makes it more
clear what's being filtered for in the offered set.
##########
tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/amqp/connect/AMQPConnectSaslTest.java:
##########
@@ -115,6 +117,81 @@ public void testConnectsWithPlain() throws Exception {
}
}
+ @Test
+ @Timeout(20)
+ public void testConnectsWithXOauth2() throws Exception {
+ try (ProtonTestServer peer = new ProtonTestServer()) {
+ peer.expectSaslXOauth2Connect(USER, PASSWD);
+ peer.expectOpen().respond();
+ peer.expectBegin().respond();
+ peer.start();
+
+ final URI remoteURI = peer.getServerURI();
+ logger.debug("Connect test started, peer listening on: {}",
remoteURI);
+
+ AMQPBrokerConnectConfiguration amqpConnection =
+ new AMQPBrokerConnectConfiguration(getTestName(),
"tcp://localhost:" + remoteURI.getPort() + "?saslMechanisms=" + XOAUTH2);
+ amqpConnection.setReconnectAttempts(0);// No reconnects
+ amqpConnection.setUser(USER);
+ amqpConnection.setPassword(PASSWD);
+
+ server.getConfiguration().addAMQPConnection(amqpConnection);
+ server.start();
+
+ peer.waitForScriptToComplete(5, TimeUnit.SECONDS);
+ }
+ }
+
+ @Test
+ @Timeout(20)
+ public void testConnectsWithXOauth2WithoutUserAndPassword() throws
Exception {
Review Comment:
suggest name change to
`testXOAuth2NotSelectedWhenUserAndPasswordNotProvided` to better describe the
test
Issue Time Tracking
-------------------
Worklog Id: (was: 973896)
Time Spent: 3h 20m (was: 3h 10m)
> 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
> Assignee: Timothy A. Bish
> Priority: Major
> Labels: pull-request-available
> Time Spent: 3h 20m
> 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
