Try to retry SASL if we get a RPL_SASLMECHS message

CHANGELOG

@@ -7,6 +7,7 @@ vNEXT (in development)
  * SASL improvements
    * The enabled mechanisms can now be configured (in the SASL DSL)
    * Added support for EXTERNAL mechanism, disabled by default
+   * Now attempts to renegotiate if the server doesn't recognise the SASL mechanism that was tried
  * (Internal) Minor version updates for Gradle, Kotlin and JUnit
 
 v0.6.0

src/main/kotlin/com/dmdirc/ktirc/events/CapabilitiesHandler.kt

@@ -20,6 +20,7 @@ internal class CapabilitiesHandler : EventHandler {
             is ServerCapabilitiesFinished -> handleCapabilitiesFinished(client)
             is ServerCapabilitiesAcknowledged -> handleCapabilitiesAcknowledged(client, event.capabilities)
             is AuthenticationMessage -> handleAuthenticationMessage(client, event.argument)
+            is SaslMechanismNotAvailableError -> handleSaslMechanismChange(client, event.mechanisms)
             is SaslFinished -> handleSaslFinished(client)
         }
         return emptyList()
@@ -32,7 +33,7 @@ internal class CapabilitiesHandler : EventHandler {
     private fun handleCapabilitiesFinished(client: IrcClient) {
         // TODO: We probably need to split the outgoing REQ lines if there are lots of caps
         // TODO: For caps with values we may need to decide which value to use/whether to enable them/etc
-        with (client.serverState.capabilities) {
+        with(client.serverState.capabilities) {
             if (advertisedCapabilities.keys.isEmpty()) {
                 negotiationState = CapabilitiesNegotiationState.FINISHED
                 client.sendCapabilityEnd()
@@ -48,17 +49,15 @@ internal class CapabilitiesHandler : EventHandler {
 
     private fun handleCapabilitiesAcknowledged(client: IrcClient, capabilities: Map<Capability, String>) {
         // TODO: Check if everything we wanted is enabled
-        with (client.serverState.capabilities) {
+        with(client.serverState.capabilities) {
             log.info { "Acknowledged capabilities: ${capabilities.keys.map { it.name }.toList()}" }
             enabledCapabilities.putAll(capabilities)
 
             if (client.serverState.sasl.mechanisms.isNotEmpty()) {
-                client.serverState.sasl.getPreferredSaslMechanism(enabledCapabilities[Capability.SaslAuthentication])?.let { mechanism ->
-                    log.info { "Attempting SASL authentication using ${mechanism.ircName}" }
-                    client.serverState.sasl.currentMechanism = mechanism
-                    negotiationState = CapabilitiesNegotiationState.AUTHENTICATING
-                    client.sendAuthenticationMessage(mechanism.ircName)
-                    return
+                enabledCapabilities[Capability.SaslAuthentication]?.let { serverCaps ->
+                    if (startSaslAuth(client, serverCaps.split(','))) {
+                        return
+                    }
                 }
                 log.warning { "SASL is enabled but we couldn't negotiate a SASL mechanism with the server" }
             }
@@ -67,6 +66,24 @@ internal class CapabilitiesHandler : EventHandler {
         }
     }
 
+    private fun handleSaslMechanismChange(client: IrcClient, mechanisms: Collection<String>) {
+        if (!startSaslAuth(client, mechanisms)) {
+            log.warning { "SASL is enabled but we couldn't negotiate a SASL mechanism with the server" }
+            client.endNegotiation()
+        }
+    }
+
+    private fun startSaslAuth(client: IrcClient, serverMechanisms: Collection<String>) =
+            with(client.serverState) {
+                sasl.getPreferredSaslMechanism(serverMechanisms)?.let { mechanism ->
+                    log.info { "Attempting SASL authentication using ${mechanism.ircName}" }
+                    sasl.currentMechanism = mechanism
+                    capabilities.negotiationState = CapabilitiesNegotiationState.AUTHENTICATING
+                    client.sendAuthenticationMessage(mechanism.ircName)
+                    true
+                } ?: false
+            }
+
     private fun handleAuthenticationMessage(client: IrcClient, argument: String?) {
         if (argument?.length == 400) {
             client.serverState.sasl.saslBuffer += argument
@@ -80,8 +97,8 @@ internal class CapabilitiesHandler : EventHandler {
         }
     }
 
-    private fun handleSaslFinished(client: IrcClient) = with (client) {
-        with (serverState.sasl) {
+    private fun handleSaslFinished(client: IrcClient) = with(client) {
+        with(serverState.sasl) {
             saslBuffer = ""
             mechanismState = null
             currentMechanism = null

src/main/kotlin/com/dmdirc/ktirc/events/Events.kt

@@ -104,4 +104,4 @@ class AuthenticationMessage(time: LocalDateTime, val argument: String?) : IrcEve
 class SaslFinished(time: LocalDateTime, var success: Boolean) : IrcEvent(time)
 
 /** Raised when the server says our SASL mechanism isn't available, but gives us a list of others. */
-class SaslMechanismNotAvailableError(time: LocalDateTime, var mechanisms: Array<String>) : IrcEvent(time)
+class SaslMechanismNotAvailableError(time: LocalDateTime, var mechanisms: Collection<String>) : IrcEvent(time)

src/main/kotlin/com/dmdirc/ktirc/messages/AuthenticationProcessor.kt

@@ -2,20 +2,25 @@ package com.dmdirc.ktirc.messages
 
 import com.dmdirc.ktirc.events.AuthenticationMessage
 import com.dmdirc.ktirc.events.SaslFinished
+import com.dmdirc.ktirc.events.SaslMechanismNotAvailableError
 import com.dmdirc.ktirc.model.IrcMessage
 
 internal class AuthenticationProcessor : MessageProcessor {
 
-    override val commands = arrayOf("AUTHENTICATE", RPL_SASLSUCCESS, ERR_SASLFAIL)
+    override val commands = arrayOf("AUTHENTICATE", RPL_SASLSUCCESS, ERR_SASLFAIL, RPL_SASLMECHS)
 
     override fun process(message: IrcMessage) = when(message.command) {
         "AUTHENTICATE" -> listOf(AuthenticationMessage(message.time, message.authenticateArgument))
         RPL_SASLSUCCESS -> listOf(SaslFinished(message.time, true))
         ERR_SASLFAIL -> listOf(SaslFinished(message.time, false))
+        RPL_SASLMECHS -> listOf(SaslMechanismNotAvailableError(message.time, message.mechanisms))
         else -> emptyList()
     }
 
     private val IrcMessage.authenticateArgument: String?
         get() = if (params.isEmpty() || params[0].size == 1 && String(params[0]) == "+") null else String(params[0])
 
+    private val IrcMessage.mechanisms: List<String>
+        get() = if (params.size < 2) emptyList() else String(params[1]).split(',')
+
 }

src/main/kotlin/com/dmdirc/ktirc/messages/NumericConstants.kt

@@ -14,3 +14,4 @@ internal const val ERR_NOMOTD = "422"
 
 internal const val RPL_SASLSUCCESS = "903"
 internal const val ERR_SASLFAIL = "904"
+internal const val RPL_SASLMECHS = "908"

src/main/kotlin/com/dmdirc/ktirc/model/SaslState.kt

@@ -18,11 +18,10 @@ internal class SaslState(config: SaslConfig?) {
 
     var mechanismState: Any? = null
 
-    fun getPreferredSaslMechanism(serverMechanisms: String?): SaslMechanism? {
-        val serverSupported = serverMechanisms?.split(',') ?: return null
+    fun getPreferredSaslMechanism(serverMechanisms: Collection<String>): SaslMechanism? {
         return mechanisms
                 .filter { it.priority < currentMechanism?.priority ?: Int.MAX_VALUE }
-                .filter { serverMechanisms.isEmpty() || it.ircName in serverSupported }
+                .filter { serverMechanisms.isEmpty() || it.ircName in serverMechanisms }
                 .maxBy { it.priority }
     }
 

src/test/kotlin/com/dmdirc/ktirc/events/CapabilitiesHandlerTest.kt

@@ -134,6 +134,18 @@ internal class CapabilitiesHandlerTest {
         assertSame(saslMech1, serverState.sasl.currentMechanism)
     }
 
+
+    @Test
+    fun `sends authenticate when capabilities acknowledged with shared mechanism`() {
+        serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
+        handler.processEvent(ircClient, ServerCapabilitiesAcknowledged(TestConstants.time, hashMapOf(
+                Capability.SaslAuthentication to "mech1,fake2",
+                Capability.HostsInNamesReply to "123"
+        )))
+
+        verify(ircClient).send("AUTHENTICATE mech1")
+    }
+
     @Test
     fun `updates negotiation state when capabilities acknowledged with shared mechanism`() {
         serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
@@ -265,4 +277,28 @@ internal class CapabilitiesHandlerTest {
         }
     }
 
+    @Test
+    fun `sends a new authenticate request when sasl mechanism rejected and new one is acceptable`() {
+        serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
+        handler.processEvent(ircClient, SaslMechanismNotAvailableError(TestConstants.time, listOf("mech1", "fake2")))
+
+        verify(ircClient).send("AUTHENTICATE mech1")
+    }
+
+    @Test
+    fun `sends cap end when sasl mechanism rejected and no new one is acceptable`() {
+        serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
+        handler.processEvent(ircClient, SaslMechanismNotAvailableError(TestConstants.time, listOf("fake1", "fake2")))
+
+        verify(ircClient).send("CAP END")
+    }
+
+    @Test
+    fun `sets negotiation state when sasl mechanism rejected and no new one is acceptable`() {
+        serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
+        handler.processEvent(ircClient, SaslMechanismNotAvailableError(TestConstants.time, listOf("fake1", "fake2")))
+
+        assertEquals(CapabilitiesNegotiationState.FINISHED, serverState.capabilities.negotiationState)
+    }
+
 }

src/test/kotlin/com/dmdirc/ktirc/messages/AuthenticationProcessorTest.kt

@@ -3,6 +3,7 @@ package com.dmdirc.ktirc.messages
 import com.dmdirc.ktirc.TestConstants
 import com.dmdirc.ktirc.events.AuthenticationMessage
 import com.dmdirc.ktirc.events.SaslFinished
+import com.dmdirc.ktirc.events.SaslMechanismNotAvailableError
 import com.dmdirc.ktirc.model.IrcMessage
 import com.dmdirc.ktirc.params
 import com.dmdirc.ktirc.util.currentTimeProvider
@@ -69,4 +70,24 @@ internal class AuthenticationProcessorTest {
         assertFalse(event.success)
     }
 
+    @Test
+    fun `raises not available error on mechs numeric`() {
+        val events = processor.process(IrcMessage(emptyMap(), ":the.gibson".toByteArray(), "908", params("*", "PLAIN,EXTERNAL,MAGIC", "are supported by this server")))
+
+        assertEquals(1, events.size)
+        val event = events[0] as SaslMechanismNotAvailableError
+        assertEquals(TestConstants.time, event.time)
+        assertEquals(listOf("PLAIN", "EXTERNAL", "MAGIC"), event.mechanisms)
+    }
+
+    @Test
+    fun `raises empty not available error on malformed mechs numeric`() {
+        val events = processor.process(IrcMessage(emptyMap(), ":the.gibson".toByteArray(), "908", params("*")))
+
+        assertEquals(1, events.size)
+        val event = events[0] as SaslMechanismNotAvailableError
+        assertEquals(TestConstants.time, event.time)
+        assertTrue(event.mechanisms.isEmpty())
+    }
+
 }

src/test/kotlin/com/dmdirc/ktirc/model/SaslStateTest.kt

@@ -30,7 +30,7 @@ internal class SaslStateTest {
         val state = SaslState(null)
         state.mechanisms.addAll(mechanisms)
 
-        assertEquals(mech3, state.getPreferredSaslMechanism(""))
+        assertEquals(mech3, state.getPreferredSaslMechanism(emptyList()))
     }
 
     @Test
@@ -39,7 +39,7 @@ internal class SaslStateTest {
         state.mechanisms.addAll(mechanisms)
         state.currentMechanism = mech3
 
-        assertEquals(mech2, state.getPreferredSaslMechanism(""))
+        assertEquals(mech2, state.getPreferredSaslMechanism(emptyList()))
     }
 
     @Test
@@ -48,7 +48,7 @@ internal class SaslStateTest {
         state.mechanisms.addAll(mechanisms)
         state.currentMechanism = mech1
 
-        assertNull(state.getPreferredSaslMechanism(""))
+        assertNull(state.getPreferredSaslMechanism(emptyList()))
     }
 
     @Test
@@ -56,7 +56,7 @@ internal class SaslStateTest {
         val state = SaslState(null)
         state.mechanisms.addAll(mechanisms)
 
-        assertEquals(mech3, state.getPreferredSaslMechanism("mech1,mech3,mech2"))
+        assertEquals(mech3, state.getPreferredSaslMechanism(listOf("mech1", "mech3", "mech2")))
     }
 
     @Test
@@ -64,7 +64,7 @@ internal class SaslStateTest {
         val state = SaslState(null)
         state.mechanisms.addAll(mechanisms)
 
-        assertEquals(mech2, state.getPreferredSaslMechanism("mech2,mech1,other"))
+        assertEquals(mech2, state.getPreferredSaslMechanism(listOf("mech2", "mech1", "other")))
     }
 
     @Test
@@ -72,7 +72,7 @@ internal class SaslStateTest {
         val state = SaslState(null)
         state.mechanisms.addAll(mechanisms)
 
-        assertNull(state.getPreferredSaslMechanism("foo,bar,baz"))
+        assertNull(state.getPreferredSaslMechanism(listOf("foo", "bar", "baz")))
     }
 
     @Test