Allow SASL methods to be configured, add EXTERNAL

CHANGELOG

@@ -4,6 +4,9 @@ vNEXT (in development)
  * IrcClients are now constructed using a DSL
    * Users of the library no longer need to care about the implementing class
    * Facilitates adding more options in the future without breaking existing implementations
+ * SASL improvements
+   * The enabled mechanisms can now be configured (in the SASL DSL)
+   * Added support for EXTERNAL mechanism, disabled by default
  * (Internal) Minor version updates for Gradle, Kotlin and JUnit
 
 v0.6.0

README.md

@@ -80,8 +80,8 @@ client.connect()
 ### `java.lang.IllegalStateException: Check failed` when connecting to some servers
 
 This happens when the IRC server requests an optional client certificate (for use
-in SASL auth, usually). At present there is no support for client certificates in
-the networking library used by KtIrc. This is fixed in the
+in SASL EXTERNAL auth, usually). At present there is no support for client
+certificates in the networking library used by KtIrc. This is fixed in the
 [upstream library](https://github.com/ktorio/ktor/issues/641) and will be included
 as soon as snapshot builds are available. There is no workaround other than using
 an insecure connection.

src/main/kotlin/com/dmdirc/ktirc/Dsl.kt

@@ -28,6 +28,9 @@ internal data class IrcClientConfig(val server: ServerConfig, val profile: Profi
  * }
  *
  * sasl {
+ *     mechanisms += "PLAIN" // or to set the list from scratch:
+ *     mechanisms("PLAIN")
+ *
  *     username = "botaccount"
  *     password = "s3cur3"
  * }
@@ -54,7 +57,7 @@ class IrcClientConfigBuilder {
     /**
      * Configures the profile of the IrcClient user.
      *
-     * At a minimum, [ProfileConfig.nickName] must be supplied.
+     * At a minimum, [ProfileConfig.nickname] must be supplied.
      */
     @IrcClientDsl
     fun profile(block: ProfileConfig.() -> Unit) {
@@ -109,11 +112,40 @@ class ProfileConfig {
 
 /**
  * Dsl for configuring SASL authentication.
+ *
+ * By default the `PLAIN` method will be enabled if SASL is configured.
+ *
+ * You can modify the mechanisms either by editing the [mechanisms] collection:
+ *
+ * ```
+ * mechanisms += "EXTERNAL"
+ * mechanisms.remove("PLAIN")
+ * ```
+ *
+ * or by calling the [mechanisms] function with all the mechanisms you wish
+ * to enable:
+ *
+ * ```
+ * mechanisms("PLAIN", "EXTERNAL")
+ * ```
+ *
+ * Priority of mechanisms is determined by KtIrc, regardless of the order
+ * they are specified in here.
  */
 @IrcClientDsl
 class SaslConfig {
+    /** The SASL mechanisms to enable. */
+    val mechanisms: MutableCollection<String> = mutableSetOf("PLAIN")
     /** The username to provide when authenticating using SASL. */
     var username: String = ""
     /** The username to provide when authenticating using SASL. */
     var password: String = ""
+
+    @IrcClientDsl
+    fun mechanisms(vararg methods: String) {
+        with (this.mechanisms) {
+            clear()
+            addAll(methods)
+        }
+    }
 }

src/main/kotlin/com/dmdirc/ktirc/IrcClient.kt

@@ -4,8 +4,6 @@ import com.dmdirc.ktirc.events.*
 import com.dmdirc.ktirc.io.*
 import com.dmdirc.ktirc.messages.*
 import com.dmdirc.ktirc.model.*
-import com.dmdirc.ktirc.sasl.PlainMechanism
-import com.dmdirc.ktirc.sasl.SaslMechanism
 import com.dmdirc.ktirc.util.currentTimeProvider
 import com.dmdirc.ktirc.util.logger
 import io.ktor.util.KtorExperimentalAPI
@@ -21,7 +19,6 @@ interface IrcClient {
     val serverState: ServerState
     val channelState: ChannelStateMap
     val userState: UserState
-    val hasSaslConfig: Boolean
 
     val caseMapping: CaseMapping
         get() = serverState.features[ServerFeature.ServerCaseMapping] ?: CaseMapping.Rfc
@@ -107,10 +104,9 @@ internal class IrcClientImpl(private val config: IrcClientConfig) : IrcClient, C
     @KtorExperimentalAPI
     internal var socketFactory: (CoroutineScope, String, Int, Boolean) -> LineBufferedSocket = ::KtorLineBufferedSocket
 
-    override val serverState = ServerState(config.profile.nickname, config.server.host, getSaslMechanisms())
+    override val serverState = ServerState(config.profile.nickname, config.server.host, config.sasl)
     override val channelState = ChannelStateMap { caseMapping }
     override val userState = UserState { caseMapping }
-    override val hasSaslConfig = config.sasl != null
 
     private val messageHandler = MessageHandler(messageProcessors.toList(), eventHandlers.toMutableList())
 
@@ -171,12 +167,4 @@ internal class IrcClientImpl(private val config: IrcClientConfig) : IrcClient, C
         connecting.set(false)
     }
 
-    private fun getSaslMechanisms(): Collection<SaslMechanism> {
-        // TODO: Move this somewhere else
-        // TODO: Allow mechanisms to be configured
-        config.sasl?.let {
-            return listOf(PlainMechanism(it))
-        } ?: return emptyList()
-    }
-
 }

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

@@ -52,7 +52,7 @@ internal class CapabilitiesHandler : EventHandler {
             log.info { "Acknowledged capabilities: ${capabilities.keys.map { it.name }.toList()}" }
             enabledCapabilities.putAll(capabilities)
 
-            if (client.hasSaslConfig) {
+            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
@@ -60,7 +60,7 @@ internal class CapabilitiesHandler : EventHandler {
                     client.sendAuthenticationMessage(mechanism.ircName)
                     return
                 }
-                log.warning { "User supplied credentials but we couldn't negotiate a SASL mechanism with the server" }
+                log.warning { "SASL is enabled but we couldn't negotiate a SASL mechanism with the server" }
             }
 
             client.endNegotiation()

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

@@ -1,10 +1,15 @@
 package com.dmdirc.ktirc.model
 
+import com.dmdirc.ktirc.SaslConfig
 import com.dmdirc.ktirc.sasl.SaslMechanism
+import com.dmdirc.ktirc.sasl.createSaslMechanism
 
-internal class SaslState(private val mechanisms: Collection<SaslMechanism>) {
+internal class SaslState(config: SaslConfig?) {
+
+    val mechanisms = (config?.createSaslMechanism() ?: emptyList()).toMutableList()
 
     var saslBuffer: String = ""
+
     var currentMechanism: SaslMechanism? = null
         set(value) {
             mechanismState = null

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

@@ -1,7 +1,7 @@
 package com.dmdirc.ktirc.model
 
+import com.dmdirc.ktirc.SaslConfig
 import com.dmdirc.ktirc.io.CaseMapping
-import com.dmdirc.ktirc.sasl.SaslMechanism
 import com.dmdirc.ktirc.util.logger
 import kotlin.reflect.KClass
 
@@ -11,7 +11,7 @@ import kotlin.reflect.KClass
 class ServerState internal constructor(
         private val initialNickname: String,
         private val initialServerName: String,
-        saslMechanisms: Collection<SaslMechanism>) {
+        saslConfig: SaslConfig? = null) {
 
     private val log by logger()
 
@@ -49,7 +49,7 @@ class ServerState internal constructor(
     val capabilities = CapabilitiesState()
 
     /** The current state of SASL authentication. */
-    internal val sasl = SaslState(saslMechanisms)
+    internal val sasl = SaslState(saslConfig)
 
     /**
      * Convenience accessor for the [ServerFeature.ModePrefixes] feature, which will always have a value.

src/main/kotlin/com/dmdirc/ktirc/sasl/ExternalMechanism.kt

@@ -0,0 +1,15 @@
+package com.dmdirc.ktirc.sasl
+
+import com.dmdirc.ktirc.IrcClient
+import com.dmdirc.ktirc.messages.sendAuthenticationMessage
+
+internal class ExternalMechanism : SaslMechanism {
+
+    override val ircName = "EXTERNAL"
+    override val priority = 100
+
+    override fun handleAuthenticationEvent(client: IrcClient, data: ByteArray?) {
+        client.sendAuthenticationMessage("+")
+    }
+
+}

src/main/kotlin/com/dmdirc/ktirc/sasl/SaslMechanism.kt

@@ -1,6 +1,7 @@
 package com.dmdirc.ktirc.sasl
 
 import com.dmdirc.ktirc.IrcClient
+import com.dmdirc.ktirc.SaslConfig
 
 internal interface SaslMechanism {
 
@@ -10,3 +11,11 @@ internal interface SaslMechanism {
     fun handleAuthenticationEvent(client: IrcClient, data: ByteArray?)
 
 }
+
+internal fun SaslConfig.createSaslMechanism(): List<SaslMechanism> = mechanisms.mapNotNull {
+    when (it.toUpperCase()) {
+        "EXTERNAL" -> ExternalMechanism()
+        "PLAIN" -> PlainMechanism(this)
+        else -> null
+    }
+}

src/test/kotlin/com/dmdirc/ktirc/DslTest.kt

@@ -123,3 +123,23 @@ internal class IrcClientConfigBuilderTest {
     }
 
 }
+
+internal class SaslConfigTest {
+
+    @Test
+    fun `mechanisms function clears all existing mechanisms`() {
+        val config = SaslConfig().apply {
+            mechanisms += "TEST"
+            mechanisms("FOO", "BAR")
+        }
+
+        assertEquals(setOf("FOO", "BAR"), config.mechanisms)
+    }
+
+    @Test
+    fun `defaults to plain mechanism`() {
+        val config = SaslConfig()
+        assertEquals(setOf("PLAIN"), config.mechanisms)
+    }
+
+}

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

@@ -30,7 +30,7 @@ internal class CapabilitiesHandlerTest {
     }
 
     private val handler = CapabilitiesHandler()
-    private val serverState = ServerState("", "", listOf(saslMech1, saslMech2, saslMech3))
+    private val serverState = ServerState("", "", null)
     private val ircClient = mock<IrcClient> {
         on { serverState } doReturn serverState
     }
@@ -92,7 +92,7 @@ internal class CapabilitiesHandlerTest {
 
     @Test
     fun `sends END when capabilities acknowledged and no sasl state`() {
-        whenever(ircClient.hasSaslConfig).thenReturn(true)
+        serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
         handler.processEvent(ircClient, ServerCapabilitiesAcknowledged(TestConstants.time, hashMapOf(
                 Capability.EchoMessages to "",
                 Capability.HostsInNamesReply to "123"
@@ -103,7 +103,7 @@ internal class CapabilitiesHandlerTest {
 
     @Test
     fun `sends END when capabilities acknowledged and no shared mechanism`() {
-        whenever(ircClient.hasSaslConfig).thenReturn(true)
+        serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
         handler.processEvent(ircClient, ServerCapabilitiesAcknowledged(TestConstants.time, hashMapOf(
                 Capability.SaslAuthentication to "fake1,fake2",
                 Capability.HostsInNamesReply to "123"
@@ -114,7 +114,7 @@ internal class CapabilitiesHandlerTest {
 
     @Test
     fun `sends AUTHENTICATE when capabilities acknowledged with shared mechanism`() {
-        whenever(ircClient.hasSaslConfig).thenReturn(true)
+        serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
         handler.processEvent(ircClient, ServerCapabilitiesAcknowledged(TestConstants.time, hashMapOf(
                 Capability.SaslAuthentication to "mech1,fake2",
                 Capability.HostsInNamesReply to "123"
@@ -125,7 +125,7 @@ internal class CapabilitiesHandlerTest {
 
     @Test
     fun `sets current SASL mechanism when capabilities acknowledged with shared mechanism`() {
-        whenever(ircClient.hasSaslConfig).thenReturn(true)
+        serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
         handler.processEvent(ircClient, ServerCapabilitiesAcknowledged(TestConstants.time, hashMapOf(
                 Capability.SaslAuthentication to "mech1,fake2",
                 Capability.HostsInNamesReply to "123"
@@ -136,7 +136,7 @@ internal class CapabilitiesHandlerTest {
 
     @Test
     fun `updates negotiation state when capabilities acknowledged with shared mechanism`() {
-        whenever(ircClient.hasSaslConfig).thenReturn(true)
+        serverState.sasl.mechanisms.addAll(listOf(saslMech1, saslMech2, saslMech3))
         handler.processEvent(ircClient, ServerCapabilitiesAcknowledged(TestConstants.time, hashMapOf(
                 Capability.SaslAuthentication to "mech1,fake2",
                 Capability.HostsInNamesReply to "123"

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

@@ -13,7 +13,7 @@ internal class ChannelStateHandlerTest {
 
     private val handler = ChannelStateHandler()
     private val channelStateMap = ChannelStateMap { CaseMapping.Rfc }
-    private val serverState = ServerState("", "", emptyList())
+    private val serverState = ServerState("", "")
     private val ircClient = mock<IrcClient> {
         on { serverState } doReturn serverState
         on { channelState } doReturn channelStateMap

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

@@ -16,7 +16,7 @@ import org.junit.jupiter.api.Test
 
 internal class EventUtilsTest {
 
-    private val serverState = ServerState("", "", emptyList())
+    private val serverState = ServerState("", "")
     private val ircClient = mock<IrcClient> {
         on { serverState } doReturn serverState
         on { caseMapping } doReturn CaseMapping.Ascii

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

@@ -11,7 +11,7 @@ import org.junit.jupiter.api.Test
 
 internal class ServerStateHandlerTest {
 
-    private val serverState = ServerState("", "", emptyList())
+    private val serverState = ServerState("", "")
     private val ircClient = mock<IrcClient> {
         on { serverState } doReturn serverState
     }

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

@@ -14,7 +14,7 @@ import org.junit.jupiter.api.Test
 
 internal class UserStateHandlerTest {
 
-    private val serverState = ServerState("", "", emptyList())
+    private val serverState = ServerState("", "")
     private val userState = UserState { CaseMapping.Rfc }
 
     private val ircClient = mock<IrcClient> {

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

@@ -27,14 +27,16 @@ internal class SaslStateTest {
 
     @Test
     fun `gets most preferred client SASL mechanism if none are specified by server`() {
-        val state = SaslState(mechanisms)
+        val state = SaslState(null)
+        state.mechanisms.addAll(mechanisms)
 
         assertEquals(mech3, state.getPreferredSaslMechanism(""))
     }
 
     @Test
     fun `gets next preferred client SASL mechanism if one was tried`() {
-        val state = SaslState(mechanisms)
+        val state = SaslState(null)
+        state.mechanisms.addAll(mechanisms)
         state.currentMechanism = mech3
 
         assertEquals(mech2, state.getPreferredSaslMechanism(""))
@@ -42,7 +44,8 @@ internal class SaslStateTest {
 
     @Test
     fun `gets no preferred client SASL mechanism if all were tried`() {
-        val state = SaslState(mechanisms)
+        val state = SaslState(null)
+        state.mechanisms.addAll(mechanisms)
         state.currentMechanism = mech1
 
         assertNull(state.getPreferredSaslMechanism(""))
@@ -50,35 +53,39 @@ internal class SaslStateTest {
 
     @Test
     fun `gets most preferred client SASL mechanism if the server supports all`() {
-        val state = SaslState(mechanisms)
+        val state = SaslState(null)
+        state.mechanisms.addAll(mechanisms)
 
         assertEquals(mech3, state.getPreferredSaslMechanism("mech1,mech3,mech2"))
     }
 
     @Test
     fun `gets most preferred client SASL mechanism if the server supports some`() {
-        val state = SaslState(mechanisms)
+        val state = SaslState(null)
+        state.mechanisms.addAll(mechanisms)
 
         assertEquals(mech2, state.getPreferredSaslMechanism("mech2,mech1,other"))
     }
 
     @Test
     fun `gets no preferred client SASL mechanism if the server supports none`() {
-        val state = SaslState(mechanisms)
+        val state = SaslState(null)
+        state.mechanisms.addAll(mechanisms)
 
         assertNull(state.getPreferredSaslMechanism("foo,bar,baz"))
     }
 
     @Test
     fun `setting the current mechanism clears the existing state`() {
-        val state = SaslState(mechanisms)
+        val state = SaslState(null)
+        state.mechanisms.addAll(mechanisms)
         state.mechanismState = "in progress"
         state.currentMechanism = mech2
         assertNull(state.mechanismState)
     }
 
     @Test
-    fun `reset clears all state`() = with(SaslState(mechanisms)) {
+    fun `reset clears all state`() = with(SaslState(null)) {
         currentMechanism = mech2
         mechanismState = "in progress"
         saslBuffer = "abcdef"

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

@@ -7,25 +7,25 @@ internal class ServerStateTest {
 
     @Test
     fun `ServerState should use the initial nickname as local nickname`() {
-        val serverState = ServerState("acidBurn", "", emptyList())
+        val serverState = ServerState("acidBurn", "")
         assertEquals("acidBurn", serverState.localNickname)
     }
 
     @Test
     fun `ServerState should use the initial name as server name`() {
-        val serverState = ServerState("", "the.gibson", emptyList())
+        val serverState = ServerState("", "the.gibson")
         assertEquals("the.gibson", serverState.serverName)
     }
 
     @Test
     fun `ServerState should default status to disconnected`() {
-        val serverState = ServerState("acidBurn", "", emptyList())
+        val serverState = ServerState("acidBurn", "")
         assertEquals(ServerStatus.Disconnected, serverState.status)
     }
 
     @Test
     fun `returns mode type for known channel mode`() {
-        val serverState = ServerState("acidBurn", "", emptyList())
+        val serverState = ServerState("acidBurn", "")
         serverState.features[ServerFeature.ChannelModes] = arrayOf("ab", "cd", "ef", "gh")
         assertEquals(ChannelModeType.List, serverState.channelModeType('a'))
         assertEquals(ChannelModeType.SetUnsetParameter, serverState.channelModeType('d'))
@@ -35,7 +35,7 @@ internal class ServerStateTest {
 
     @Test
     fun `returns whether a mode is a channel user mode or not`() {
-        val serverState = ServerState("acidBurn", "", emptyList())
+        val serverState = ServerState("acidBurn", "")
         serverState.features[ServerFeature.ModePrefixes] = ModePrefixMapping("oqv", "@~+")
         assertTrue(serverState.isChannelUserMode('o'))
         assertTrue(serverState.isChannelUserMode('q'))
@@ -47,19 +47,19 @@ internal class ServerStateTest {
 
     @Test
     fun `returns NoParameter for unknown channel mode`() {
-        val serverState = ServerState("acidBurn", "", emptyList())
+        val serverState = ServerState("acidBurn", "")
         serverState.features[ServerFeature.ChannelModes] = arrayOf("ab", "cd", "ef", "gh")
         assertEquals(ChannelModeType.NoParameter, serverState.channelModeType('z'))
     }
 
     @Test
     fun `returns NoParameter for channel modes if feature doesn't exist`() {
-        val serverState = ServerState("acidBurn", "", emptyList())
+        val serverState = ServerState("acidBurn", "")
         assertEquals(ChannelModeType.NoParameter, serverState.channelModeType('b'))
     }
 
     @Test
-    fun `reset clears all state`() = with(ServerState("acidBurn", "", emptyList())) {
+    fun `reset clears all state`() = with(ServerState("acidBurn", "")) {
         receivedWelcome = true
         status = ServerStatus.Connecting
         localNickname = "acidBurn3"

src/test/kotlin/com/dmdirc/ktirc/sasl/ExternalMechanismTest.kt

@@ -0,0 +1,20 @@
+package com.dmdirc.ktirc.sasl
+
+import com.dmdirc.ktirc.IrcClient
+import com.nhaarman.mockitokotlin2.mock
+import com.nhaarman.mockitokotlin2.verify
+import org.junit.jupiter.api.Test
+
+internal class ExternalMechanismTest {
+
+    @Test
+    fun `sends + when receiving server message`() {
+        val mechanism = ExternalMechanism()
+        val client = mock<IrcClient>()
+
+        mechanism.handleAuthenticationEvent(client, null)
+
+        verify(client).send("AUTHENTICATE +")
+    }
+
+}

src/test/kotlin/com/dmdirc/ktirc/sasl/PlainMechanismTest.kt

@@ -12,7 +12,7 @@ import org.junit.jupiter.api.Test
 
 internal class PlainMechanismTest {
 
-    private val serverState = ServerState("", "", emptyList())
+    private val serverState = ServerState("", "")
     private val ircClient = mock<IrcClient> {
         on { serverState } doReturn serverState
     }

src/test/kotlin/com/dmdirc/ktirc/sasl/SaslMechanismTest.kt

@@ -0,0 +1,25 @@
+package com.dmdirc.ktirc.sasl
+
+import com.dmdirc.ktirc.SaslConfig
+import org.junit.jupiter.api.Assertions.assertEquals
+import org.junit.jupiter.api.Assertions.assertTrue
+import org.junit.jupiter.api.Test
+
+internal class SaslMechanismTest {
+
+    @Test
+    fun `creates sasl mechanisms by name`() {
+        val mechanisms = SaslConfig().apply { mechanisms("PLAIN", "EXTERNAL") }.createSaslMechanism()
+        assertEquals(2, mechanisms.size)
+        assertTrue(mechanisms[0] is PlainMechanism)
+        assertTrue(mechanisms[1] is ExternalMechanism)
+    }
+
+    @Test
+    fun `ignores unknown sasl mechanisms`() {
+        val mechanisms = SaslConfig().apply { mechanisms("PLAIN", "SPICY") }.createSaslMechanism()
+        assertEquals(1, mechanisms.size)
+        assertTrue(mechanisms[0] is PlainMechanism)
+    }
+
+}