ARTEMIS-4451: fix non-SASL AMQP connections when resource audit loggi…

…ng enabled
apache · Oct 4, 2023 · ef5fb0b · ef5fb0b
1 parent 260f774
commit ef5fb0b
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 6 deletions.
diff --git a/...rc/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPConnectionCallback.java b/...rc/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPConnectionCallback.java
@@ -130,7 +130,7 @@ private boolean isPermittedMechanism(String mechanism) {
    public boolean isSupportsAnonymous() {
       boolean supportsAnonymous = false;
       try {
-         server.getSecurityStore().authenticate(null, null, null);
+         server.getSecurityStore().authenticate(null, null, protonConnectionDelegate);
          supportsAnonymous = true;
       } catch (Exception e) {
          // authentication failed so no anonymous support

diff --git a/...est/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPConnectionCallbackTest.java b/...est/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPConnectionCallbackTest.java
@@ -16,16 +16,25 @@
  */
 package org.apache.activemq.artemis.protocol.amqp.broker;
 
+import org.apache.activemq.artemis.api.core.ActiveMQSecurityException;
 import org.apache.activemq.artemis.core.remoting.impl.invm.InVMConnection;
+import org.apache.activemq.artemis.core.remoting.impl.netty.NettyConnection;
+import org.apache.activemq.artemis.core.security.SecurityStore;
+import org.apache.activemq.artemis.core.server.ActiveMQServer;
 import org.apache.activemq.artemis.core.server.impl.ActiveMQServerImpl;
 import org.apache.activemq.artemis.protocol.amqp.sasl.AnonymousServerSASL;
 import org.apache.activemq.artemis.protocol.amqp.sasl.GSSAPIServerSASL;
 import org.apache.activemq.artemis.protocol.amqp.sasl.PlainSASL;
+import org.apache.activemq.artemis.utils.ExecutorFactory;
+import org.apache.activemq.artemis.utils.actors.ArtemisExecutor;
 import org.junit.Test;
+import org.mockito.Mockito;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 public class AMQPConnectionCallbackTest {
 
@@ -48,4 +57,40 @@ public void getServerSASLAnonDefault() throws Exception {
       AMQPConnectionCallback connectionCallback = new AMQPConnectionCallback(protonProtocolManager, null, null, new ActiveMQServerImpl());
       assertNotNull("can get anon with empty list", connectionCallback.getServerSASL(AnonymousServerSASL.NAME));
    }
+
+   @Test
+   public void testAnonymousSupportCheck() throws Exception {
+      ArtemisExecutor executor = Mockito.mock(ArtemisExecutor.class);
+      ExecutorFactory executorFactory = Mockito.mock(ExecutorFactory.class);
+      Mockito.when(executorFactory.getExecutor()).thenReturn(executor);
+
+      SecurityStore securityStore = Mockito.mock(SecurityStore.class);
+
+      ActiveMQServer server = Mockito.mock(ActiveMQServer.class);
+      Mockito.when(server.getExecutorFactory()).thenReturn(executorFactory);
+      Mockito.when(server.getSecurityStore()).thenReturn(securityStore);
+
+      NettyConnection transportConnection = Mockito.mock(NettyConnection.class);
+      ProtonProtocolManager protocolManager = Mockito.mock(ProtonProtocolManager.class);
+      Mockito.when(protocolManager.getServer()).thenReturn(server);
+
+      AMQPConnectionCallback callback = new AMQPConnectionCallback(protocolManager, transportConnection, executor, server);
+      ActiveMQProtonRemotingConnection connectionDelegate = Mockito.mock(ActiveMQProtonRemotingConnection.class);
+      callback.setProtonConnectionDelegate(connectionDelegate);
+
+      // Make it succeed
+      Mockito.when(securityStore.authenticate(Mockito.any(), Mockito.any(), Mockito.any())).thenReturn("validatedAnonUser");
+
+      // Verify result and expected args are passed
+      assertTrue(callback.isSupportsAnonymous());
+      Mockito.verify(securityStore).authenticate(Mockito.any(), Mockito.any(), Mockito.same(connectionDelegate));
+
+      // Make it fail
+      Mockito.reset(securityStore);
+      Mockito.when(securityStore.authenticate(Mockito.any(), Mockito.any(), Mockito.any())).thenThrow(new ActiveMQSecurityException("auth-failed"));
+
+      // Verify result and expected args are passed
+      assertFalse(callback.isSupportsAnonymous());
+      Mockito.verify(securityStore).authenticate(Mockito.any(), Mockito.any(), Mockito.same(connectionDelegate));
+   }
 }
diff --git a/...rc/test/java/org/apache/activemq/artemis/tests/smoke/logging/AuditLoggerResourceTest.java b/...rc/test/java/org/apache/activemq/artemis/tests/smoke/logging/AuditLoggerResourceTest.java
@@ -98,21 +98,26 @@ public void testAuditResourceLog() throws Exception {
 
    @Test
    public void testCoreConnectionAuditLog() throws Exception {
-      testConnectionAuditLog("CORE");
+      testConnectionAuditLog("CORE", "tcp://localhost:61616");
    }
 
    @Test
    public void testAMQPConnectionAuditLog() throws Exception {
-      testConnectionAuditLog("AMQP");
+      testConnectionAuditLog("AMQP", "amqp://localhost:61616");
+   }
+
+   @Test
+   public void testAMQPNoSaslConnectionAuditLog() throws Exception {
+      testConnectionAuditLog("AMQP", "amqp://localhost:61616?amqp.saslLayer=false");
    }
 
    @Test
    public void testOpenWireConnectionAuditLog() throws Exception {
-      testConnectionAuditLog("OPENWIRE");
+      testConnectionAuditLog("OPENWIRE", "tcp://localhost:61616");
    }
 
-   private void testConnectionAuditLog(String protocol) throws Exception {
-      ConnectionFactory factory = CFUtil.createConnectionFactory(protocol, "tcp://localhost:61616");
+   private void testConnectionAuditLog(String protocol, String url) throws Exception {
+      ConnectionFactory factory = CFUtil.createConnectionFactory(protocol, url);
       Connection connection = factory.createConnection();
       Session s = connection.createSession(false, Session.AUTO_ACKNOWLEDGE);
       checkAuditLogRecord(true, "AMQ601767: " + protocol + " connection");