Remove round parameter, introduce factories.

Author: Simon MacMullen

src/com/rabbitmq/client/AuthMechanism.java

@@ -5,21 +5,15 @@
 import java.io.IOException;
 
 /**
- * A pluggable authentication mechanism. Should be stateless.
+ * A pluggable authentication mechanism.
  */
 public interface AuthMechanism {
     /**
      * Handle one round of challenge-response
-     * @param round zero-based counter of the current round
-     * @param challenge the challenge this round, or null on round 0.
+     * @param challenge the challenge this round, or null on first round.
      * @param factory for reference to e.g. username and password.
      * @return response
      * @throws IOException
      */
-    LongString handleChallenge(int round, LongString challenge, ConnectionFactory factory);
-
-    /**
-     * The name of the authentication mechanism, as negotiated on the wire
-     */
-    String getName();
+    LongString handleChallenge(LongString challenge, ConnectionFactory factory);
 }

src/com/rabbitmq/client/AuthMechanismFactory.java

@@ -0,0 +1,16 @@
+package com.rabbitmq.client;
+
+/**
+ *
+ */
+public interface AuthMechanismFactory {
+    /**
+     * @return a new authentication mechanism implementation
+     */
+    public AuthMechanism getInstance();
+
+    /**
+     * The name of the authentication mechanism, as negotiated on the wire
+     */
+    public String getName();
+}

src/com/rabbitmq/client/ConnectionFactory.java

@@ -46,7 +46,7 @@
 
 import com.rabbitmq.client.impl.AMQConnection;
 import com.rabbitmq.client.impl.FrameHandler;
-import com.rabbitmq.client.impl.PlainMechanism;
+import com.rabbitmq.client.impl.PlainMechanismFactory;
 import com.rabbitmq.client.impl.SocketFrameHandler;
 
 /**
@@ -87,8 +87,8 @@ public class ConnectionFactory implements Cloneable {
     public static final int DEFAULT_AMQP_OVER_SSL_PORT = 5671;
 
     /** The default list of authentication mechanisms to use */
-    public static final AuthMechanism[] DEFAULT_AUTH_MECHANISMS =
-        new AuthMechanism[] { new PlainMechanism() };
+    public static final AuthMechanismFactory[] DEFAULT_AUTH_MECHANISMS =
+        new AuthMechanismFactory[] { new PlainMechanismFactory() };
 
     /**
      * The default SSL protocol (currently "SSLv3").
@@ -103,9 +103,9 @@ public class ConnectionFactory implements Cloneable {
     private int requestedChannelMax               = DEFAULT_CHANNEL_MAX;
     private int requestedFrameMax                 = DEFAULT_FRAME_MAX;
     private int requestedHeartbeat                = DEFAULT_HEARTBEAT;
-    private AuthMechanism[] authMechanisms        = DEFAULT_AUTH_MECHANISMS;
     private Map<String, Object> _clientProperties = AMQConnection.defaultClientProperties();
     private SocketFactory factory                 = SocketFactory.getDefault();
+    private AuthMechanismFactory[] authMechanismFactories = DEFAULT_AUTH_MECHANISMS;
 
     /**
      * Instantiate a ConnectionFactory with a default set of parameters.
@@ -273,11 +273,11 @@ public void setClientProperties(Map<String, Object> clientProperties) {
      * @param serverMechanisms
      * @return
      */
-    public AuthMechanism getAuthMechanism(List<String> serverMechanisms) {
+    public AuthMechanismFactory getAuthMechanismFactory(List<String> serverMechanisms) {
         // Our list is in order of preference, the server one is not.
-        for (AuthMechanism mechanism : authMechanisms) {
-            if (serverMechanisms.contains(mechanism.getName())) {
-                return mechanism;
+        for (AuthMechanismFactory f : authMechanismFactories) {
+            if (serverMechanisms.contains(f.getName())) {
+                return f;
             }
         }
 
@@ -286,11 +286,11 @@ public AuthMechanism getAuthMechanism(List<String> serverMechanisms) {
 
     /**
      * Set authentication mechanisms to use (in descending preference order)
-     * @param mechanisms
+     * @param factories
      * @see #DEFAULT_AUTH_MECHANISMS
      */
-    public void setAuthMechanisms(AuthMechanism[] mechanisms) {
-        this.authMechanisms = mechanisms;
+    public void setAuthMechanismFactories(AuthMechanismFactory[] factories) {
+        this.authMechanismFactories = factories;
     }
 
     /**

src/com/rabbitmq/client/impl/AMQConnection.java

@@ -39,14 +39,11 @@
 import java.util.Map;
 import java.util.HashMap;
 import java.util.concurrent.TimeoutException;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.TimeUnit;
 
 import com.rabbitmq.client.AMQP;
-import com.rabbitmq.client.Address;
 import com.rabbitmq.client.AlreadyClosedException;
 import com.rabbitmq.client.AuthMechanism;
+import com.rabbitmq.client.AuthMechanismFactory;
 import com.rabbitmq.client.Channel;
 import com.rabbitmq.client.Command;
 import com.rabbitmq.client.Connection;
@@ -278,21 +275,21 @@ public void start()
 
         List<String> mechanisms = Arrays.asList(
                     connStart.getMechanisms().toString().split(" "));
-        AuthMechanism mechanism = _factory.getAuthMechanism(mechanisms);
-        if (mechanism == null) {
+        AuthMechanismFactory mechanismFactory = _factory.getAuthMechanismFactory(mechanisms);
+        if (mechanismFactory == null) {
             throw new IOException("No compatible authentication mechanism found - " +
                     "server offered [" + connStart.getMechanisms() + "]");
         }
+        AuthMechanism mechanism = mechanismFactory.getInstance();
 
-        int round = 0;
         LongString challenge = null;
         AMQP.Connection.Tune connTune = null;
         do {
-            LongString response = mechanism.handleChallenge(round, challenge, _factory);
-            Method method = (round == 0)
+            LongString response = mechanism.handleChallenge(challenge, _factory);
+            Method method = (challenge == null)
                 ? new AMQImpl.Connection.StartOk(_clientProperties,
-                                                 mechanism.getName(), response,
-                                                 "en_US")
+                                                 mechanismFactory.getName(),
+                                                 response, "en_US")
                 : new AMQImpl.Connection.SecureOk(response);
 
             try {
@@ -301,7 +298,6 @@ public void start()
                     connTune = (AMQP.Connection.Tune) serverResponse;
                 } else {
                     challenge = ((AMQP.Connection.Secure) serverResponse).getChallenge();
-                    round++;
                 }
             } catch (ShutdownSignalException e) {
                 throw AMQChannel.wrap(e, "Possibly caused by authentication failure");

src/com/rabbitmq/client/impl/PlainMechanism.java

@@ -7,13 +7,9 @@
  * The PLAIN auth mechanism
  */
 public class PlainMechanism implements AuthMechanism {
-    public LongString handleChallenge(int round, LongString challenge,
+    public LongString handleChallenge(LongString challenge,
                                       ConnectionFactory factory) {
         return LongStringHelper.asLongString("\0" + factory.getUsername() +
                                              "\0" + factory.getPassword());
     }
-
-    public String getName() {
-        return "PLAIN";
-    }
 }

src/com/rabbitmq/client/impl/PlainMechanismFactory.java

@@ -0,0 +1,17 @@
+package com.rabbitmq.client.impl;
+
+import com.rabbitmq.client.AuthMechanism;
+import com.rabbitmq.client.AuthMechanismFactory;
+
+/**
+ *
+ */
+public class PlainMechanismFactory implements AuthMechanismFactory {
+    public AuthMechanism getInstance() {
+        return new PlainMechanism();
+    }
+
+    public String getName() {
+        return "PLAIN";
+    }
+}

src/com/rabbitmq/client/impl/ScramMD5Mechanism.java

@@ -23,9 +23,9 @@ This is only somewhat improved security over PLAIN (if you can
 */
 
 public class ScramMD5Mechanism implements AuthMechanism {
-    public LongString handleChallenge(int round, LongString challengeStr,
+    public LongString handleChallenge(LongString challengeStr,
                                       ConnectionFactory factory) {
-        if (round == 0) {
+        if (challengeStr == null) {
             return LongStringHelper.asLongString(factory.getUsername());
         } else {
             try {
@@ -43,10 +43,6 @@ public LongString handleChallenge(int round, LongString challengeStr,
         }
     }
 
-    public String getName() {
-        return "RABBIT-SCRAM-MD5";
-    }
-
     private static byte[] digest(byte[] arr1, byte[] arr2) {
         try {
             MessageDigest digest = MessageDigest.getInstance("MD5");

src/com/rabbitmq/client/impl/ScramMD5MechanismFactory.java

@@ -0,0 +1,17 @@
+package com.rabbitmq.client.impl;
+
+import com.rabbitmq.client.AuthMechanism;
+import com.rabbitmq.client.AuthMechanismFactory;
+
+/**
+ *
+ */
+public class ScramMD5MechanismFactory implements AuthMechanismFactory {
+    public AuthMechanism getInstance() {
+        return new ScramMD5Mechanism();
+    }
+
+    public String getName() {
+        return "RABBIT-SCRAM-MD5";
+    }
+}