diff --git a/smack-core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java b/smack-core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java index d014fd53b..e9f087b4d 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/SASLAuthentication.java +++ b/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) { - 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; + mechanism.throwExceptionIfRequired(); - if (saslException instanceof SmackSaslException) { - 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; + return mechanism; } /** @@ -269,7 +238,9 @@ public final class SASLAuthentication { */ public void challengeReceived(String challenge, boolean finalChallenge) throws SmackSaslException, NotConnectedException, InterruptedException { try { - currentMechanism.challengeReceived(challenge, finalChallenge); + 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() { diff --git a/smack-core/src/main/java/org/jivesoftware/smack/sasl/SASLMechanism.java b/smack-core/src/main/java/org/jivesoftware/smack/sasl/SASLMechanism.java index a11df485b..236fce4d1 100644 --- a/smack-core/src/main/java/org/jivesoftware/smack/sasl/SASLMechanism.java +++ b/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 { 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 { */ 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 { 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) {