Moved the RPC user config out of the properties file and into the main config file

node/src/main/kotlin/net/corda/node/driver/Driver.kt

@@ -8,10 +8,10 @@ import com.typesafe.config.ConfigRenderOptions
 import net.corda.core.ThreadBox
 import net.corda.core.crypto.Party
 import net.corda.core.div
+import net.corda.core.future
 import net.corda.core.node.NodeInfo
 import net.corda.core.node.services.ServiceInfo
 import net.corda.core.utilities.loggerFor
-import net.corda.core.write
 import net.corda.node.services.User
 import net.corda.node.services.config.ConfigHelper
 import net.corda.node.services.config.FullNodeConfiguration
@@ -28,7 +28,9 @@ import java.time.Instant
 import java.time.ZoneOffset.UTC
 import java.time.format.DateTimeFormatter
 import java.util.*
-import java.util.concurrent.*
+import java.util.concurrent.Future
+import java.util.concurrent.TimeUnit.SECONDS
+import java.util.concurrent.TimeoutException
 
 /**
  * This file defines a small "Driver" DSL for starting up nodes that is only intended for development, demos and tests.
@@ -246,11 +248,11 @@ open class DriverDSL(
             registeredProcesses.forEach(Process::destroy)
         }
         /** Wait 5 seconds, then [Process.destroyForcibly] */
-        val finishedFuture = Executors.newSingleThreadExecutor().submit {
+        val finishedFuture = future {
             waitForAllNodesToFinish()
         }
         try {
-            finishedFuture.get(5, TimeUnit.SECONDS)
+            finishedFuture.get(5, SECONDS)
         } catch (exception: TimeoutException) {
             finishedFuture.cancel(true)
             state.locked {
@@ -306,22 +308,19 @@ open class DriverDSL(
                         "webAddress" to apiAddress.toString(),
                         "extraAdvertisedServiceIds" to advertisedServices.joinToString(","),
                         "networkMapAddress" to networkMapAddress.toString(),
-                        "useTestClock" to useTestClock
+                        "useTestClock" to useTestClock,
+                        "rpcUsers" to rpcUsers.map { mapOf(
+                                "user" to it.username,
+                                "password" to it.password,
+                                "permissions" to it.permissions)
+                        }
                 )
         )
 
-        val nodeConfig = FullNodeConfiguration(config)
-
-        nodeConfig.rpcUsersFile.write(createDirs = true) {
-            rpcUsers.map { it.username to "${it.password},${it.permissions.joinToString(",")}" }
-                    .toMap(Properties())
-                    .store(it, null)
-        }
-
-        return Executors.newSingleThreadExecutor().submit(Callable<NodeInfoAndConfig> {
-            registerProcess(DriverDSL.startNode(nodeConfig, quasarJarPath, debugPort))
+        return future {
+            registerProcess(DriverDSL.startNode(FullNodeConfiguration(config), quasarJarPath, debugPort))
             NodeInfoAndConfig(queryNodeInfo(apiAddress)!!, config)
-        })
+        }
     }
 
     override fun start() {

node/src/main/kotlin/net/corda/node/internal/Node.kt

@@ -8,8 +8,8 @@ import net.corda.core.node.services.ServiceInfo
 import net.corda.core.then
 import net.corda.core.utilities.loggerFor
 import net.corda.node.serialization.NodeClock
-import net.corda.node.services.PropertiesFileRPCUserService
 import net.corda.node.services.RPCUserService
+import net.corda.node.services.RPCUserServiceImpl
 import net.corda.node.services.api.MessagingServiceInternal
 import net.corda.node.services.config.FullNodeConfiguration
 import net.corda.node.services.messaging.ArtemisMessagingServer
@@ -118,7 +118,7 @@ class Node(override val configuration: FullNodeConfiguration, networkMapAddress:
     private lateinit var userService: RPCUserService
 
     override fun makeMessagingService(): MessagingServiceInternal {
-        userService = PropertiesFileRPCUserService(configuration.rpcUsersFile)
+        userService = RPCUserServiceImpl(configuration.config)
         val serverAddr = with(configuration) {
             messagingServerAddress ?: {
                 messageBroker = ArtemisMessagingServer(this, artemisAddress, services.networkMapCache, userService)

node/src/main/kotlin/net/corda/node/services/RPCUserService.kt

@@ -1,9 +1,7 @@
 package net.corda.node.services
 
-import net.corda.core.exists
-import net.corda.core.read
-import java.nio.file.Path
-import java.util.*
+import com.typesafe.config.Config
+import net.corda.node.services.config.getListOrElse
 
 /**
  * Service for retrieving [User] objects representing RPC users who are authorised to use the RPC system. A [User]
@@ -15,30 +13,22 @@ interface RPCUserService {
     val users: List<User>
 }
 
-// TODO If this sticks around then change it to use HOCON ...
-// TODO  ... and also store passwords as salted hashes.
-// TODO Otherwise consider something like Apache Shiro
-class PropertiesFileRPCUserService(file: Path) : RPCUserService {
+// TODO Store passwords as salted hashes
+// TODO Or ditch this and consider something like Apache Shiro
+class RPCUserServiceImpl(config: Config) : RPCUserService {
 
     private val _users: Map<String, User>
 
     init {
-        _users = if (file.exists()) {
-            val properties = Properties()
-            file.read {
-                properties.load(it)
-            }
-            properties.map {
-                val parts = it.value.toString().split(delimiters = ",")
-                val username = it.key.toString()
-                require(!username.contains("""\.|\*|#""".toRegex())) { """Usernames cannot have the following characters: * . # """ }
-                val password = parts[0]
-                val permissions = parts.drop(1).map(String::toUpperCase).toSet()
-                User(username, password, permissions)
-            }.associateBy(User::username)
-        } else {
-            emptyMap()
-        }
+        _users = config.getListOrElse<Config>("rpcUsers") { emptyList() }
+                .map {
+                    val username = it.getString("user")
+                    require(username.matches("\\w+".toRegex())) { "Username $username contains invalid characters" }
+                    val password = it.getString("password")
+                    val permissions = it.getListOrElse<String>("permissions") { emptyList() }.map(String::toUpperCase).toSet()
+                    User(username, password, permissions)
+                }
+                .associateBy(User::username)
     }
 
     override fun getUser(usename: String): User? = _users[usename]

node/src/main/kotlin/net/corda/node/services/config/ConfigUtilities.kt

@@ -87,6 +87,15 @@ fun Config.getProperties(path: String): Properties {
     return props
 }
 
+@Suppress("UNCHECKED_CAST")
+inline fun <reified T : Any> Config.getListOrElse(path: String, default: Config.() -> List<T>): List<T> {
+    return if (hasPath(path)) {
+        (if (T::class == String::class) getStringList(path) else getConfigList(path)) as List<T>
+    } else {
+        this.default()
+    }
+}
+
 /**
  * Strictly for dev only automatically construct a server certificate/private key signed from
  * the CA certs in Node resources. Then provision KeyStores into certificates folder under node path.

node/src/main/kotlin/net/corda/node/services/config/NodeConfiguration.kt

@@ -24,7 +24,6 @@ interface NodeSSLConfiguration {
 interface NodeConfiguration : NodeSSLConfiguration {
     val basedir: Path
     override val certificatesPath: Path get() = basedir / "certificates"
-    val rpcUsersFile: Path get() = basedir / "rpc-users.properties"
     val myLegalName: String
     val nearestCity: String
     val emailAddress: String

node/src/test/kotlin/com/r3corda/node/services/RPCUserServiceImplTest.kt

@@ -0,0 +1,75 @@
+package com.r3corda.node.services
+
+import com.typesafe.config.ConfigFactory
+import net.corda.node.services.RPCUserServiceImpl
+import net.corda.node.services.User
+import org.assertj.core.api.Assertions.assertThat
+import org.assertj.core.api.Assertions.assertThatThrownBy
+import org.junit.Test
+
+class RPCUserServiceImplTest {
+
+    @Test
+    fun `missing config`() {
+        val service = loadWithContents("{}")
+        assertThat(service.getUser("user")).isNull()
+        assertThat(service.users).isEmpty()
+    }
+
+    @Test
+    fun `no users`() {
+        val service = loadWithContents("rpcUsers : []")
+        assertThat(service.getUser("user")).isNull()
+        assertThat(service.users).isEmpty()
+    }
+
+    @Test
+    fun `no permissions`() {
+        val service = loadWithContents("rpcUsers : [{ user=user1, password=letmein }]")
+        val expectedUser = User("user1", "letmein", permissions = emptySet())
+        assertThat(service.getUser("user1")).isEqualTo(expectedUser)
+        assertThat(service.users).containsOnly(expectedUser)
+    }
+
+    @Test
+    fun `single permission, which is in lower case`() {
+        val service = loadWithContents("rpcUsers : [{ user=user1, password=letmein, permissions=[cash] }]")
+        assertThat(service.getUser("user1")?.permissions).containsOnly("CASH")
+    }
+
+    @Test
+    fun `two permissions, which are upper case`() {
+        val service = loadWithContents("rpcUsers : [{ user=user1, password=letmein, permissions=[CASH, ADMIN] }]")
+        assertThat(service.getUser("user1")?.permissions).containsOnly("CASH", "ADMIN")
+    }
+
+    @Test
+    fun `two users`() {
+        val service = loadWithContents("""rpcUsers : [
+        { user=user, password=password, permissions=[ADMIN] }
+        { user=user2, password=password2, permissions=[] }
+        ]""")
+        val user1 = User("user", "password", permissions = setOf("ADMIN"))
+        val user2 = User("user2", "password2", permissions = emptySet())
+        assertThat(service.getUser("user")).isEqualTo(user1)
+        assertThat(service.getUser("user2")).isEqualTo(user2)
+        assertThat(service.users).containsOnly(user1, user2)
+    }
+
+    @Test
+    fun `unknown user`() {
+        val service = loadWithContents("rpcUsers : [{ user=user1, password=letmein }]")
+        assertThat(service.getUser("test")).isNull()
+    }
+
+    @Test
+    fun `Artemis special characters not permitted in usernames`() {
+        assertThatThrownBy { loadWithContents("rpcUsers : [{ user=user.1, password=letmein }]") }.hasMessageContaining(".")
+        assertThatThrownBy { loadWithContents("rpcUsers : [{ user=user*1, password=letmein }]") }.hasMessageContaining("*")
+        assertThatThrownBy { loadWithContents("""rpcUsers : [{ user="user#1", password=letmein }]""") }.hasMessageContaining("#")
+    }
+
+    private fun loadWithContents(configString: String): RPCUserServiceImpl {
+        return RPCUserServiceImpl(ConfigFactory.parseString(configString))
+    }
+}

node/src/test/kotlin/net/corda/node/services/ArtemisMessagingTests.kt

@@ -18,6 +18,7 @@ import net.corda.node.utilities.configureDatabase
 import net.corda.node.utilities.databaseTransaction
 import net.corda.testing.freeLocalHostAndPort
 import net.corda.testing.node.makeTestDataSourceProperties
+import com.typesafe.config.ConfigFactory
 import org.assertj.core.api.Assertions.assertThatThrownBy
 import org.jetbrains.exposed.sql.Database
 import org.junit.After
@@ -59,7 +60,7 @@ class ArtemisMessagingTests {
 
     @Before
     fun setUp() {
-        userService = PropertiesFileRPCUserService(temporaryFolder.newFile().toPath())
+        userService = RPCUserServiceImpl(ConfigFactory.empty())
         // TODO: create a base class that provides a default implementation
         config = object : NodeConfiguration {
             override val basedir: Path = temporaryFolder.newFolder().toPath()

node/src/test/kotlin/net/corda/node/services/PropertiesFileRPCUserServiceTest.kt

@@ -1,81 +0,0 @@
-package net.corda.node.services
-
-import com.google.common.jimfs.Configuration.unix
-import com.google.common.jimfs.Jimfs
-import net.corda.core.writeLines
-import org.assertj.core.api.Assertions.assertThat
-import org.assertj.core.api.Assertions.assertThatThrownBy
-import org.junit.After
-import org.junit.Test
-
-class PropertiesFileRPCUserServiceTest {
-
-    private val fileSystem = Jimfs.newFileSystem(unix())
-    private val file = fileSystem.getPath("users.properties")
-
-    @After
-    fun cleanUp() {
-        fileSystem.close()
-    }
-
-    @Test
-    fun `file doesn't exist`() {
-        val service = PropertiesFileRPCUserService(file)
-        assertThat(service.getUser("user")).isNull()
-        assertThat(service.users).isEmpty()
-    }
-
-    @Test
-    fun `empty file`() {
-        val service = loadWithContents()
-        assertThat(service.getUser("user")).isNull()
-        assertThat(service.users).isEmpty()
-    }
-
-    @Test
-    fun `no permissions`() {
-        val service = loadWithContents("user=password")
-        assertThat(service.getUser("user")).isEqualTo(User("user", "password", permissions = emptySet()))
-        assertThat(service.users).containsOnly(User("user", "password", permissions = emptySet()))
-    }
-
-    @Test
-    fun `single permission, which is in lower case`() {
-        val service = loadWithContents("user=password,cash")
-        assertThat(service.getUser("user")?.permissions).containsOnly("CASH")
-    }
-
-    @Test
-    fun `two permissions, which are upper case`() {
-        val service = loadWithContents("user=password,CASH,ADMIN")
-        assertThat(service.getUser("user")?.permissions).containsOnly("CASH", "ADMIN")
-    }
-
-    @Test
-    fun `two users`() {
-        val service = loadWithContents("user=password,ADMIN", "user2=password2")
-        assertThat(service.getUser("user")).isNotNull()
-        assertThat(service.getUser("user2")).isNotNull()
-        assertThat(service.users).containsOnly(
-                User("user", "password", permissions = setOf("ADMIN")),
-                User("user2", "password2", permissions = emptySet()))
-    }
-
-    @Test
-    fun `unknown user`() {
-        val service = loadWithContents("user=password")
-        assertThat(service.getUser("test")).isNull()
-    }
-
-    @Test
-    fun `Artemis special characters not permitted in usernames`() {
-        assertThatThrownBy { loadWithContents("user.name=password") }
-        assertThatThrownBy { loadWithContents("user*name=password") }
-        assertThatThrownBy { loadWithContents("user#name=password") }
-    }
-
-    private fun loadWithContents(vararg lines: String): PropertiesFileRPCUserService {
-        file.writeLines(lines.asList())
-        return PropertiesFileRPCUserService(file)
-    }
-}