Move SASL authentication state into SASLMechanism

smack-core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java

@@ -154,17 +154,6 @@ public final class SASLAuthentication {
     private final ConnectionConfiguration configuration;
     private SASLMechanism currentMechanism = null;
 
-    /**
-     * Boolean indicating if SASL negotiation has finished and was successful.
-     */
-    private boolean authenticationSuccessful;
-
-    /**
-     * Either of type {@link SmackSaslException},{@link SASLErrorException}, {@link NotConnectedException} or
-     * {@link InterruptedException}.
-     */
-    private Exception saslException;
-
     SASLAuthentication(AbstractXMPPConnection connection, ConnectionConfiguration configuration) {
         this.configuration = configuration;
         this.connection = connection;
@@ -194,22 +183,23 @@ public final class SASLAuthentication {
     public SASLMechanism authenticate(String username, String password, EntityBareJid authzid, SSLSession sslSession)
                     throws XMPPErrorException, SASLErrorException, IOException,
                     InterruptedException, SmackSaslException, NotConnectedException, NoResponseException {
-        currentMechanism = selectMechanism(authzid);
+        final SASLMechanism mechanism = selectMechanism(authzid);
         final CallbackHandler callbackHandler = configuration.getCallbackHandler();
         final String host = connection.getHost();
         final DomainBareJid xmppServiceDomain = connection.getXMPPServiceDomain();
 
-        authenticationSuccessful = false;
-
         synchronized (this) {
+            currentMechanism = mechanism;
+
             if (callbackHandler != null) {
                 currentMechanism.authenticate(host, xmppServiceDomain, callbackHandler, authzid, sslSession);
             }
             else {
                 currentMechanism.authenticate(username, host, xmppServiceDomain, password, authzid, sslSession);
             }
+
             final long deadline = System.currentTimeMillis() + connection.getReplyTimeout();
-            while (!authenticationSuccessful && saslException == null) {
+            while (!mechanism.isFinished()) {
                 final long now = System.currentTimeMillis();
                 if (now >= deadline) break;
                 // Wait until SASL negotiation finishes
@@ -217,30 +207,9 @@ public final class SASLAuthentication {
             }
         }
 
-        if (saslException != null) {
+        mechanism.throwExceptionIfRequired();
-            Exception saslException = this.saslException;
-            // Clear the saslException class field, so that this exception is not thrown after a new authenticate()
-            // invocation (with different credentials).
-            this.saslException = null;
 
-            if (saslException instanceof SmackSaslException) {
+        return mechanism;
-                throw (SmackSaslException) saslException;
-            } else if (saslException instanceof SASLErrorException) {
-                throw (SASLErrorException) saslException;
-            } else if (saslException instanceof NotConnectedException) {
-                throw (NotConnectedException) saslException;
-            } else if (saslException instanceof InterruptedException) {
-                throw (InterruptedException) saslException;
-            } else {
-                throw new IllegalStateException("Unexpected exception type" , saslException);
-            }
-        }
-
-        if (!authenticationSuccessful) {
-            throw NoResponseException.newWith(connection, "successful SASL authentication");
-        }
-
-        return currentMechanism;
     }
 
     /**
@@ -269,7 +238,9 @@ public final class SASLAuthentication {
      */
     public void challengeReceived(String challenge, boolean finalChallenge) throws SmackSaslException, NotConnectedException, InterruptedException {
         try {
+            synchronized (this) {
                 currentMechanism.challengeReceived(challenge, finalChallenge);
+            }
         } catch (InterruptedException | SmackSaslException | NotConnectedException e) {
             authenticationFailed(e);
             throw e;
@@ -292,10 +263,11 @@ public final class SASLAuthentication {
         if (success.getData() != null) {
             challengeReceived(success.getData(), true);
         }
-        currentMechanism.checkIfSuccessfulOrThrow();
+
-        authenticationSuccessful = true;
         // Wake up the thread that is waiting in the #authenticate method
         synchronized (this) {
+            currentMechanism.afterFinalSaslChallenge();
+
             notify();
         }
     }
@@ -312,15 +284,20 @@ public final class SASLAuthentication {
     }
 
     private void authenticationFailed(Exception exception) {
-        saslException = exception;
         // Wake up the thread that is waiting in the #authenticate method
         synchronized (this) {
+            currentMechanism.setException(exception);
             notify();
         }
     }
 
     public boolean authenticationSuccessful() {
-        return authenticationSuccessful;
+        synchronized (this) {
+            if (currentMechanism == null) {
+                return false;
+            }
+            return currentMechanism.isAuthenticationSuccessful();
+        }
     }
 
     String getNameOfLastUsedSaslMechansism() {

smack-core/src/main/java/org/jivesoftware/smack/sasl/SASLMechanism.java

@@ -23,6 +23,7 @@ import javax.net.ssl.SSLSession;
 import javax.security.auth.callback.CallbackHandler;
 
 import org.jivesoftware.smack.ConnectionConfiguration;
+import org.jivesoftware.smack.SmackException.NoResponseException;
 import org.jivesoftware.smack.SmackException.NotConnectedException;
 import org.jivesoftware.smack.SmackException.SmackSaslException;
 import org.jivesoftware.smack.XMPPConnection;
@@ -56,6 +57,17 @@ public abstract class SASLMechanism implements Comparable<SASLMechanism> {
     public static final String GSSAPI = "GSSAPI";
     public static final String PLAIN = "PLAIN";
 
+    /**
+     * Boolean indicating if SASL negotiation has finished and was successful.
+     */
+    private boolean authenticationSuccessful;
+
+    /**
+     * Either of type {@link SmackSaslException},{@link SASLErrorException}, {@link NotConnectedException} or
+     * {@link InterruptedException}.
+     */
+    private Exception exception;
+
     protected XMPPConnection connection;
 
     protected ConnectionConfiguration connectionConfiguration;
@@ -275,7 +287,18 @@ public abstract class SASLMechanism implements Comparable<SASLMechanism> {
      */
     public abstract int getPriority();
 
-    public abstract void checkIfSuccessfulOrThrow() throws SmackSaslException;
+    /**
+     * Check if the SASL mechanism was successful and if it was, then mark it so.
+     *
+     * @throws SmackSaslException in case of an SASL error.
+     */
+    public final void afterFinalSaslChallenge() throws SmackSaslException {
+        checkIfSuccessfulOrThrow();
+
+        authenticationSuccessful = true;
+    }
+
+    protected abstract void checkIfSuccessfulOrThrow() throws SmackSaslException;
 
     public SASLMechanism instanceForAuthentication(XMPPConnection connection, ConnectionConfiguration connectionConfiguration) {
         SASLMechanism saslMechansim = newInstance();
@@ -288,6 +311,39 @@ public abstract class SASLMechanism implements Comparable<SASLMechanism> {
         return false;
     }
 
+    public boolean isAuthenticationSuccessful() {
+        return authenticationSuccessful;
+    }
+
+    public boolean isFinished() {
+        return isAuthenticationSuccessful() || exception != null;
+    }
+
+    public void throwExceptionIfRequired() throws SmackSaslException, SASLErrorException, NotConnectedException,
+                    InterruptedException, NoResponseException {
+        if (exception != null) {
+            if (exception instanceof SmackSaslException) {
+                throw (SmackSaslException) exception;
+            } else if (exception instanceof SASLErrorException) {
+                throw (SASLErrorException) exception;
+            } else if (exception instanceof NotConnectedException) {
+                throw (NotConnectedException) exception;
+            } else if (exception instanceof InterruptedException) {
+                throw (InterruptedException) exception;
+            } else {
+                throw new IllegalStateException("Unexpected exception type", exception);
+            }
+        }
+
+        if (!authenticationSuccessful) {
+            throw NoResponseException.newWith(connection, "successful SASL authentication");
+        }
+    }
+
+    public void setException(Exception exception) {
+        this.exception = exception;
+    }
+
     protected abstract SASLMechanism newInstance();
 
     protected static byte[] toBytes(String string) {