Doorman rejects requests for duplicate legal names and legal names which contain , or = (X.500)

netpermission/src/main/kotlin/com/r3/corda/netpermission/Main.kt

@@ -107,11 +107,11 @@ fun main(args: Array<String>) {
             thread {
                 while (!stopSigner) {
                     Thread.sleep(1000)
-                    for (id in storage.pendingRequestIds()) {
+                    for (id in storage.getPendingRequestIds()) {
-                        storage.saveCertificate(id, {
+                        storage.approveRequest(id, {
                             JcaPKCS10CertificationRequest(it.request).run {
                                 X509Utilities.createServerCert(subject, publicKey, intermediateCACertAndKey,
-                                        if (it.ipAddr == it.hostName) listOf() else listOf(it.hostName), listOf(it.ipAddr))
+                                        if (it.ipAddress == it.hostName) listOf() else listOf(it.hostName), listOf(it.ipAddress))
                             }
                         })
                         log.debug { "Approved $id" }

netpermission/src/main/kotlin/com/r3/corda/netpermission/internal/CertificateSigningService.kt

@@ -1,5 +1,6 @@
 package com.r3.corda.netpermission.internal
 
+import com.r3.corda.netpermission.internal.persistence.CertificateResponse
 import com.r3.corda.netpermission.internal.persistence.CertificationData
 import com.r3.corda.netpermission.internal.persistence.CertificationRequestStorage
 import net.corda.core.crypto.X509Utilities.CACertAndKey
@@ -17,8 +18,8 @@ import javax.ws.rs.*
 import javax.ws.rs.core.Context
 import javax.ws.rs.core.MediaType
 import javax.ws.rs.core.Response
-import javax.ws.rs.core.Response.noContent
+import javax.ws.rs.core.Response.*
-import javax.ws.rs.core.Response.ok
+import javax.ws.rs.core.Response.Status.UNAUTHORIZED
 
 /**
  * Provides functionality for asynchronous submission of certificate signing requests and retrieval of the results.
@@ -53,28 +54,31 @@ class CertificateSigningService(val intermediateCACertAndKey: CACertAndKey, val
     @Path("certificate/{var}")
     @Produces(MediaType.APPLICATION_OCTET_STREAM)
     fun retrieveCert(@PathParam("var") requestId: String): Response {
-        val clientCert = storage.getCertificate(requestId)
+        val response = storage.getResponse(requestId)
-        return if (clientCert != null) {
+        return when (response) {
+            is CertificateResponse.Ready -> {
                 // Write certificate chain to a zip stream and extract the bit array output.
-            ByteArrayOutputStream().use {
+                val baos = ByteArrayOutputStream()
-                ZipOutputStream(it).use {
+                ZipOutputStream(baos).use { zip ->
-                    zipStream ->
                     // Client certificate must come first and root certificate should come last.
-                    mapOf(CORDA_CLIENT_CA to clientCert,
+                    val entries = listOf(
+                            CORDA_CLIENT_CA to response.certificate,
                             CORDA_INTERMEDIATE_CA to intermediateCACertAndKey.certificate,
-                            CORDA_ROOT_CA to rootCert).forEach {
+                            CORDA_ROOT_CA to rootCert
-                        zipStream.putNextEntry(ZipEntry("${it.key}.cer"))
+                    )
-                        zipStream.write(it.value.encoded)
+                    entries.forEach {
-                        zipStream.setComment(it.key)
+                        zip.putNextEntry(ZipEntry("${it.first}.cer"))
-                        zipStream.closeEntry()
+                        zip.write(it.second.encoded)
+                        zip.setComment(it.first)
+                        zip.closeEntry()
                     }
                 }
-                ok(it.toByteArray())
+                ok(baos.toByteArray())
                         .type("application/zip")
                         .header("Content-Disposition", "attachment; filename=\"certificates.zip\"")
             }
-        } else {
+            is CertificateResponse.NotReady -> noContent()
-            noContent()
+            is CertificateResponse.Unauthorised -> status(UNAUTHORIZED).entity(response.message)
         }.build()
     }
 }

netpermission/src/main/kotlin/com/r3/corda/netpermission/internal/persistence/CertificationRequestStorage.kt

@@ -8,7 +8,8 @@ import java.security.cert.Certificate
  */
 interface CertificationRequestStorage {
     /**
-     * Persist [certificationData] in storage for further approval, returns randomly generated request ID.
+     * Persist [certificationData] in storage for further approval if it's a valid request. If not then it will be automically
+     * rejected and not subject to any approval process. In both cases a randomly generated request ID is returned.
      */
     fun saveRequest(certificationData: CertificationData): String
 
@@ -18,20 +19,31 @@ interface CertificationRequestStorage {
     fun getRequest(requestId: String): CertificationData?
 
     /**
-     * Retrieve client certificate with provided [requestId].
+     * Return the response for a previously saved request with ID [requestId].
      */
-    fun getCertificate(requestId: String): Certificate?
+    fun getResponse(requestId: String): CertificateResponse
 
     /**
-     * Generate new certificate and store in storage using provided [certificateGenerator].
+     * Approve the given request by generating and storing a new certificate using the provided generator.
      */
-    fun saveCertificate(requestId: String, certificateGenerator: (CertificationData) -> Certificate)
+    fun approveRequest(requestId: String, certificateGenerator: (CertificationData) -> Certificate)
+
+    /**
+     * Reject the given request using the given reason.
+     */
+    fun rejectRequest(requestId: String, rejectReason: String)
 
     /**
      * Retrieve list of request IDs waiting for approval.
      * TODO : This is used for the background thread to approve request automatically without KYC checks, should be removed after testnet.
      */
-    fun pendingRequestIds(): List<String>
+    fun getPendingRequestIds(): List<String>
 }
 
-data class CertificationData(val hostName: String, val ipAddr: String, val request: PKCS10CertificationRequest)
+data class CertificationData(val hostName: String, val ipAddress: String, val request: PKCS10CertificationRequest)
+
+sealed class CertificateResponse {
+    object NotReady : CertificateResponse()
+    class Ready(val certificate: Certificate) : CertificateResponse()
+    class Unauthorised(val message: String) : CertificateResponse()
+}

netpermission/src/main/kotlin/com/r3/corda/netpermission/internal/persistence/DBCertificateRequestStorage.kt

@@ -1,21 +1,25 @@
 package com.r3.corda.netpermission.internal.persistence
 
 import net.corda.core.crypto.SecureHash
+import net.corda.core.crypto.commonName
 import net.corda.node.utilities.*
 import org.jetbrains.exposed.sql.*
 import java.security.cert.Certificate
-import java.time.LocalDateTime
+import java.time.Instant
 
+// TODO Relax the uniqueness requirement to be on the entire X.500 subject rather than just the legal name
 class DBCertificateRequestStorage(private val database: Database) : CertificationRequestStorage {
     private object DataTable : Table("certificate_signing_request") {
         val requestId = varchar("request_id", 64).index().primaryKey()
         val hostName = varchar("hostName", 100)
         val ipAddress = varchar("ip_address", 15)
+        val legalName = varchar("legal_name", 256)
         // TODO : Do we need to store this in column? or is it ok with blob.
         val request = blob("request")
-        val requestTimestamp = localDateTime("request_timestamp")
+        val requestTimestamp = instant("request_timestamp")
-        val approvedTimestamp = localDateTime("approved_timestamp").nullable()
+        val processTimestamp = instant("process_timestamp").nullable()
         val certificate = blob("certificate").nullable()
+        val rejectReason = varchar("reject_reason", 256).nullable()
     }
 
     init {
@@ -25,45 +29,102 @@ class DBCertificateRequestStorage(private val database: Database) : Certificatio
         }
     }
 
-    override fun getCertificate(requestId: String): Certificate? {
+    override fun saveRequest(certificationData: CertificationData): String {
-        return databaseTransaction(database) { DataTable.select { DataTable.requestId.eq(requestId) }.map { it[DataTable.certificate] }.filterNotNull().map { deserializeFromBlob<Certificate>(it) }.firstOrNull() }
+        val legalName = certificationData.request.subject.commonName
+        val requestId = SecureHash.randomSHA256().toString()
+        databaseTransaction(database) {
+            val duplicate = DataTable.select {
+                // A duplicate legal name is one where a previously approved, or currently pending, request has the same legal name.
+                // A rejected request with the same legal name doesn't count as a duplicate
+                DataTable.legalName eq legalName and (DataTable.certificate.isNotNull() or DataTable.processTimestamp.isNull())
+            }.any()
+            val rejectReason = if (duplicate) {
+                "Duplicate legal name"
+            } else if ("[=,]".toRegex() in legalName) {
+                "Legal name cannot contain '=' or ','"
+            } else {
+                null
+            }
+            val now = Instant.now()
+            withFinalizables { finalizables ->
+                DataTable.insert {
+                    it[this.requestId] = requestId
+                    it[hostName] = certificationData.hostName
+                    it[ipAddress] = certificationData.ipAddress
+                    it[this.legalName] = legalName
+                    it[request] = serializeToBlob(certificationData.request, finalizables)
+                    it[requestTimestamp] = now
+                    if (rejectReason != null) {
+                        it[this.rejectReason] = rejectReason
+                        it[processTimestamp] = now
+                    }
+                }
+            }
+        }
+        return requestId
     }
 
-    override fun saveCertificate(requestId: String, certificateGenerator: (CertificationData) -> Certificate) {
+    override fun getResponse(requestId: String): CertificateResponse {
-        databaseTransaction(database) {
+        return databaseTransaction(database) {
-            withFinalizables { finalizables ->
+            val response = DataTable
-                getRequest(requestId)?.let {
+                    .select { DataTable.requestId eq requestId and DataTable.processTimestamp.isNotNull() }
-                    val clientCert = certificateGenerator(it)
+                    .map { Pair(it[DataTable.certificate], it[DataTable.rejectReason]) }
-                    DataTable.update({ DataTable.requestId eq requestId }) {
+                    .singleOrNull()
-                        it[approvedTimestamp] = LocalDateTime.now()
+            if (response == null) {
-                        it[certificate] = serializeToBlob(clientCert, finalizables)
+                CertificateResponse.NotReady
+            } else {
+                val (certificate, rejectReason) = response
+                if (certificate != null) {
+                    CertificateResponse.Ready(deserializeFromBlob<Certificate>(certificate))
+                } else {
+                    CertificateResponse.Unauthorised(rejectReason!!)
                 }
             }
         }
     }
+
+    override fun approveRequest(requestId: String, certificateGenerator: (CertificationData) -> Certificate) {
+        databaseTransaction(database) {
+            val request = singleRequestWhere { DataTable.requestId eq requestId and DataTable.processTimestamp.isNull() }
+            if (request != null) {
+                withFinalizables { finalizables ->
+                    DataTable.update({ DataTable.requestId eq requestId }) {
+                        it[certificate] = serializeToBlob(certificateGenerator(request), finalizables)
+                        it[processTimestamp] = Instant.now()
+                    }
+                }
+            }
+        }
+    }
+
+    override fun rejectRequest(requestId: String, rejectReason: String) {
+        databaseTransaction(database) {
+            val request = singleRequestWhere { DataTable.requestId eq requestId and DataTable.processTimestamp.isNull() }
+            if (request != null) {
+                DataTable.update({ DataTable.requestId eq requestId }) {
+                    it[this.rejectReason] = rejectReason
+                    it[processTimestamp] = Instant.now()
+                }
+            }
+        }
     }
 
     override fun getRequest(requestId: String): CertificationData? {
-        return databaseTransaction(database) { DataTable.select { DataTable.requestId eq requestId }.map { CertificationData(it[DataTable.hostName], it[DataTable.ipAddress], deserializeFromBlob(it[DataTable.request])) }.firstOrNull() }
-    }
-
-    override fun saveRequest(certificationData: CertificationData): String {
         return databaseTransaction(database) {
-            withFinalizables { finalizables ->
+            singleRequestWhere { DataTable.requestId eq requestId }
-                val requestId = SecureHash.Companion.randomSHA256().toString()
-                DataTable.insert {
-                    it[DataTable.requestId] = requestId
-                    it[hostName] = certificationData.hostName
-                    it[ipAddress] = certificationData.ipAddr
-                    it[request] = serializeToBlob(certificationData.request, finalizables)
-                    it[requestTimestamp] = LocalDateTime.now()
-                }
-                requestId
-            }
         }
     }
 
-    override fun pendingRequestIds(): List<String> {
+    override fun getPendingRequestIds(): List<String> {
-        return databaseTransaction(database) { DataTable.select { DataTable.approvedTimestamp.isNull() }.map { it[DataTable.requestId] } }
+        return databaseTransaction(database) {
+            DataTable.select { DataTable.processTimestamp.isNull() }.map { it[DataTable.requestId] }
+        }
+    }
+
+    private fun singleRequestWhere(where: SqlExpressionBuilder.() -> Op<Boolean>): CertificationData? {
+        return DataTable
+                .select(where)
+                .map { CertificationData(it[DataTable.hostName], it[DataTable.ipAddress], deserializeFromBlob(it[DataTable.request])) }
+                .singleOrNull()
     }
 }

netpermission/src/main/kotlin/com/r3/corda/netpermission/internal/persistence/InMemoryCertificationRequestStorage.kt

@@ -1,34 +0,0 @@
-package com.r3.corda.netpermission.internal.persistence
-
-import net.corda.core.crypto.SecureHash
-import java.security.cert.Certificate
-import java.util.*
-
-class InMemoryCertificationRequestStorage : CertificationRequestStorage {
-    private val requestStore = HashMap<String, CertificationData>()
-    private val certificateStore = HashMap<String, Certificate>()
-
-    override fun pendingRequestIds(): List<String> {
-        return requestStore.keys.filter { !certificateStore.keys.contains(it) }
-    }
-
-    override fun getCertificate(requestId: String): Certificate? {
-        return certificateStore[requestId]
-    }
-
-    override fun saveCertificate(requestId: String, certificateGenerator: (CertificationData) -> Certificate) {
-        requestStore[requestId]?.let {
-            certificateStore.putIfAbsent(requestId, certificateGenerator(it))
-        }
-    }
-
-    override fun getRequest(requestId: String): CertificationData? {
-        return requestStore[requestId]
-    }
-
-    override fun saveRequest(certificationData: CertificationData): String {
-        val requestId = SecureHash.randomSHA256().toString()
-        requestStore.put(requestId, certificationData)
-        return requestId
-    }
-}

netpermission/src/test/kotlin/com/r3/corda/netpermission/CertificateSigningServiceTest.kt

@@ -4,124 +4,156 @@ import com.google.common.net.HostAndPort
 import com.nhaarman.mockito_kotlin.*
 import com.r3.corda.netpermission.CertificateSigningServer.Companion.hostAndPort
 import com.r3.corda.netpermission.internal.CertificateSigningService
+import com.r3.corda.netpermission.internal.persistence.CertificateResponse
 import com.r3.corda.netpermission.internal.persistence.CertificationData
 import com.r3.corda.netpermission.internal.persistence.CertificationRequestStorage
+import net.corda.core.crypto.CertificateStream
 import net.corda.core.crypto.SecureHash
 import net.corda.core.crypto.X509Utilities
+import org.apache.commons.io.IOUtils
+import org.assertj.core.api.Assertions.assertThat
+import org.bouncycastle.pkcs.PKCS10CertificationRequest
 import org.bouncycastle.pkcs.jcajce.JcaPKCS10CertificationRequest
+import org.junit.After
 import org.junit.Test
-import sun.security.x509.X500Name
 import java.io.IOException
 import java.net.HttpURLConnection
+import java.net.HttpURLConnection.*
 import java.net.URL
 import java.security.cert.Certificate
-import java.security.cert.CertificateFactory
 import java.security.cert.X509Certificate
 import java.util.*
 import java.util.zip.ZipInputStream
+import javax.ws.rs.core.MediaType
 import kotlin.test.assertEquals
-import kotlin.test.assertNotNull
-import kotlin.test.assertNull
 
 class CertificateSigningServiceTest {
-    val rootCA = X509Utilities.createSelfSignedCACert("Corda Node Root CA")
+    private val rootCA = X509Utilities.createSelfSignedCACert("Corda Node Root CA")
-    val intermediateCA = X509Utilities.createSelfSignedCACert("Corda Node Intermediate CA")
+    private val intermediateCA = X509Utilities.createSelfSignedCACert("Corda Node Intermediate CA")
+    private lateinit var signingServer: CertificateSigningServer
 
-    private fun getSigningServer(storage: CertificationRequestStorage): CertificateSigningServer {
+    private fun startSigningServer(storage: CertificationRequestStorage) {
-        return CertificateSigningServer(HostAndPort.fromParts("localhost", 0), CertificateSigningService(intermediateCA, rootCA.certificate, storage))
+        signingServer = CertificateSigningServer(HostAndPort.fromParts("localhost", 0), CertificateSigningService(intermediateCA, rootCA.certificate, storage))
+    }
+
+    @After
+    fun close() {
+        signingServer.close()
     }
 
     @Test
-    fun `test submit request`() {
+    fun `submit request`() {
         val id = SecureHash.randomSHA256().toString()
 
-        val storage: CertificationRequestStorage = mock {
+        val storage = mock<CertificationRequestStorage> {
             on { saveRequest(any()) }.then { id }
         }
 
-        getSigningServer(storage).use {
+        startSigningServer(storage)
-            val keyPair = X509Utilities.generateECDSAKeyPairForSSL()
-            val request = X509Utilities.createCertificateSigningRequest("Test", "London", "admin@test.com", keyPair)
-            // Post request to signing server via http.
-            val submitRequest = {
-                val conn = URL("http://${it.server.hostAndPort()}/api/certificate").openConnection() as HttpURLConnection
-                conn.doOutput = true
-                conn.requestMethod = "POST"
-                conn.setRequestProperty("Content-Type", "application/octet-stream")
-                conn.outputStream.write(request.encoded)
-                conn.inputStream.bufferedReader().readLine()
-            }
 
-            assertEquals(id, submitRequest())
+        val keyPair = X509Utilities.generateECDSAKeyPairForSSL()
+        val request = X509Utilities.createCertificateSigningRequest("LegalName", "London", "admin@test.com", keyPair)
+        // Post request to signing server via http.
+
+        assertEquals(id, submitRequest(request))
         verify(storage, times(1)).saveRequest(any())
-            submitRequest()
+        submitRequest(request)
         verify(storage, times(2)).saveRequest(any())
     }
-    }
 
     @Test
-    fun `test retrieve certificate`() {
+    fun `retrieve certificate`() {
         val keyPair = X509Utilities.generateECDSAKeyPairForSSL()
         val id = SecureHash.randomSHA256().toString()
 
         // Mock Storage behaviour.
         val certificateStore = mutableMapOf<String, Certificate>()
-        val storage: CertificationRequestStorage = mock {
+        val storage = mock<CertificationRequestStorage> {
-            on { getCertificate(eq(id)) }.then { certificateStore[id] }
+            on { getResponse(eq(id)) }.then {
-            on { saveCertificate(eq(id), any()) }.then {
+                certificateStore[id]?.let { CertificateResponse.Ready(it) } ?: CertificateResponse.NotReady
-                val certGen = it.arguments[1] as (CertificationData) -> Certificate
+            }
+            on { approveRequest(eq(id), any()) }.then {
+                @Suppress("UNCHECKED_CAST")
+                val certGen = it.arguments[1] as ((CertificationData) -> Certificate)
                 val request = CertificationData("", "", X509Utilities.createCertificateSigningRequest("LegalName", "London", "admin@test.com", keyPair))
                 certificateStore[id] = certGen(request)
                 Unit
             }
-            on { pendingRequestIds() }.then { listOf(id) }
+            on { getPendingRequestIds() }.then { listOf(id) }
         }
 
-        getSigningServer(storage).use {
+        startSigningServer(storage)
-            val poll = {
+
-                val url = URL("http://${it.server.hostAndPort()}/api/certificate/$id")
+        assertThat(pollForResponse(id)).isEqualTo(PollResponse.NotReady)
+
+        storage.approveRequest(id) {
+            JcaPKCS10CertificationRequest(it.request).run {
+                X509Utilities.createServerCert(subject, publicKey, intermediateCA,
+                        if (it.ipAddress == it.hostName) listOf() else listOf(it.hostName), listOf(it.ipAddress))
+            }
+        }
+
+        val certificates = (pollForResponse(id) as PollResponse.Ready).certChain
+        verify(storage, times(2)).getResponse(any())
+        assertEquals(3, certificates.size)
+
+        certificates.first().run {
+            assertThat(subjectDN.name).contains("CN=LegalName")
+            assertThat(subjectDN.name).contains("L=London")
+        }
+
+        certificates.last().run {
+            assertThat(subjectDN.name).contains("CN=Corda Node Root CA")
+            assertThat(subjectDN.name).contains("L=London")
+        }
+    }
+
+    @Test
+    fun `request not authorised`() {
+        val id = SecureHash.randomSHA256().toString()
+
+        val storage = mock<CertificationRequestStorage> {
+            on { getResponse(eq(id)) }.then { CertificateResponse.Unauthorised("Not Allowed") }
+            on { getPendingRequestIds() }.then { listOf(id) }
+        }
+
+        startSigningServer(storage)
+
+        assertThat(pollForResponse(id)).isEqualTo(PollResponse.Unauthorised("Not Allowed"))
+    }
+
+    private fun submitRequest(request: PKCS10CertificationRequest): String {
+        val conn = URL("http://${signingServer.server.hostAndPort()}/api/certificate").openConnection() as HttpURLConnection
+        conn.doOutput = true
+        conn.requestMethod = "POST"
+        conn.setRequestProperty("Content-Type", MediaType.APPLICATION_OCTET_STREAM)
+        conn.outputStream.write(request.encoded)
+        return conn.inputStream.bufferedReader().use { it.readLine() }
+    }
+
+    private fun pollForResponse(id: String): PollResponse {
+        val url = URL("http://${signingServer.server.hostAndPort()}/api/certificate/$id")
         val conn = url.openConnection() as HttpURLConnection
         conn.requestMethod = "GET"
 
-                when (conn.responseCode) {
+        return when (conn.responseCode) {
-                    HttpURLConnection.HTTP_OK -> conn.inputStream.use {
+            HTTP_OK -> ZipInputStream(conn.inputStream).use {
-                        ZipInputStream(it).use {
+                val stream = CertificateStream(it)
-                            val certificates = ArrayList<Certificate>()
+                val certificates = ArrayList<X509Certificate>()
                 while (it.nextEntry != null) {
-                                certificates.add(CertificateFactory.getInstance("X.509").generateCertificate(it))
+                    certificates.add(stream.nextCertificate())
                 }
-                            certificates
+                PollResponse.Ready(certificates)
             }
-                    }
+            HTTP_NO_CONTENT -> PollResponse.NotReady
-                    HttpURLConnection.HTTP_NO_CONTENT -> null
+            HTTP_UNAUTHORIZED -> PollResponse.Unauthorised(IOUtils.toString(conn.errorStream))
-                    else ->
+            else -> throw IOException("Cannot connect to Certificate Signing Server, HTTP response code : ${conn.responseCode}")
-                        throw IOException("Cannot connect to Certificate Signing Server, HTTP response code : ${conn.responseCode}")
         }
     }
 
-            assertNull(poll())
+    private interface PollResponse {
-            assertNull(poll())
+        object NotReady : PollResponse
-
+        data class Ready(val certChain: List<X509Certificate>) : PollResponse
-            storage.saveCertificate(id, {
+        data class Unauthorised(val message: String) : PollResponse
-                JcaPKCS10CertificationRequest(it.request).run {
-                    X509Utilities.createServerCert(subject, publicKey, intermediateCA,
-                            if (it.ipAddr == it.hostName) listOf() else listOf(it.hostName), listOf(it.ipAddr))
-                }
-            })
-
-            val certificates = assertNotNull(poll())
-            verify(storage, times(3)).getCertificate(any())
-            assertEquals(3, certificates.size)
-
-            (certificates.first() as X509Certificate).run {
-                assertEquals("LegalName", (subjectDN as X500Name).commonName)
-                assertEquals("London", (subjectDN as X500Name).locality)
-            }
-
-            (certificates.last() as X509Certificate).run {
-                assertEquals("Corda Node Root CA", (subjectDN as X500Name).commonName)
-                assertEquals("London", (subjectDN as X500Name).locality)
-            }
-        }
     }
 }

netpermission/src/test/kotlin/com/r3/corda/netpermission/internal/persistence/DBCertificateRequestStorageTest.kt

@@ -3,78 +3,141 @@ package com.r3.corda.netpermission.internal.persistence
 import net.corda.core.crypto.X509Utilities
 import net.corda.node.utilities.configureDatabase
 import net.corda.testing.node.makeTestDataSourceProperties
+import org.assertj.core.api.Assertions.assertThat
 import org.bouncycastle.pkcs.jcajce.JcaPKCS10CertificationRequest
+import org.junit.After
+import org.junit.Before
 import org.junit.Test
+import java.io.Closeable
+import java.security.KeyPair
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
-import kotlin.test.assertNull
 import kotlin.test.assertTrue
 
 class DBCertificateRequestStorageTest {
-    val intermediateCA = X509Utilities.createSelfSignedCACert("Corda Node Intermediate CA")
+    private val intermediateCA = X509Utilities.createSelfSignedCACert("Corda Node Intermediate CA")
+    private var closeDb: Closeable? = null
+    private lateinit var storage: DBCertificateRequestStorage
+
+    @Before
+    fun startDb() {
+        configureDatabase(makeTestDataSourceProperties()).apply {
+            closeDb = first
+            storage = DBCertificateRequestStorage(second)
+        }
+    }
+
+    @After
+    fun closeDb() {
+        closeDb?.close()
+    }
 
     @Test
-    fun `test save request`() {
+    fun `valid request`() {
-        val keyPair = X509Utilities.generateECDSAKeyPairForSSL()
+        val request = createRequest("LegalName").first
-        val request = CertificationData("", "", X509Utilities.createCertificateSigningRequest("LegalName", "London", "admin@test.com", keyPair))
-
-        val (connection, db) = configureDatabase(makeTestDataSourceProperties())
-        connection.use {
-            val storage = DBCertificateRequestStorage(db)
         val requestId = storage.saveRequest(request)
-
         assertNotNull(storage.getRequest(requestId)).apply {
             assertEquals(request.hostName, hostName)
-                assertEquals(request.ipAddr, ipAddr)
+            assertEquals(request.ipAddress, ipAddress)
             assertEquals(request.request, this.request)
         }
-        }
+        assertThat(storage.getPendingRequestIds()).containsOnly(requestId)
     }
 
     @Test
-    fun `test pending request`() {
+    fun `approve request`() {
-        val keyPair = X509Utilities.generateECDSAKeyPairForSSL()
+        val (request, keyPair) = createRequest("LegalName")
-        val request = CertificationData("", "", X509Utilities.createCertificateSigningRequest("LegalName", "London", "admin@test.com", keyPair))
-
-        val (connection, db) = configureDatabase(makeTestDataSourceProperties())
-        connection.use {
-            val storage = DBCertificateRequestStorage(db)
-            val requestId = storage.saveRequest(request)
-            storage.pendingRequestIds().apply {
-                assertTrue(isNotEmpty())
-                assertEquals(1, size)
-                assertEquals(requestId, first())
-            }
-        }
-    }
-
-    @Test
-    fun `test save certificate`() {
-        val keyPair = X509Utilities.generateECDSAKeyPairForSSL()
-        val request = CertificationData("", "", X509Utilities.createCertificateSigningRequest("LegalName", "London", "admin@test.com", keyPair))
-
-        val (connection, db) = configureDatabase(makeTestDataSourceProperties())
-        connection.use {
-            val storage = DBCertificateRequestStorage(db)
         // Add request to DB.
         val requestId = storage.saveRequest(request)
         // Pending request should equals to 1.
-            assertEquals(1, storage.pendingRequestIds().size)
+        assertEquals(1, storage.getPendingRequestIds().size)
         // Certificate should be empty.
-            assertNull(storage.getCertificate(requestId))
+        assertEquals(CertificateResponse.NotReady, storage.getResponse(requestId))
         // Store certificate to DB.
-            storage.saveCertificate(requestId, {
+        approveRequest(requestId)
-                JcaPKCS10CertificationRequest(it.request).run {
-                    X509Utilities.createServerCert(subject, publicKey, intermediateCA,
-                            if (it.ipAddr == it.hostName) listOf() else listOf(it.hostName), listOf(it.ipAddr))
-                }
-            })
         // Check certificate is stored in DB correctly.
-            assertNotNull(storage.getCertificate(requestId)).apply {
+        val response = storage.getResponse(requestId) as CertificateResponse.Ready
-                assertEquals(keyPair.public, this.publicKey)
+        assertThat(response.certificate.publicKey).isEqualTo(keyPair.public)
-            }
         // Pending request should be empty.
-            assertTrue(storage.pendingRequestIds().isEmpty())
+        assertTrue(storage.getPendingRequestIds().isEmpty())
+    }
+
+    @Test
+    fun `reject request`() {
+        val requestId = storage.saveRequest(createRequest("BankA").first)
+        storage.rejectRequest(requestId, "Because I said so!")
+        assertThat(storage.getPendingRequestIds()).isEmpty()
+        val response = storage.getResponse(requestId) as CertificateResponse.Unauthorised
+        assertThat(response.message).isEqualTo("Because I said so!")
+    }
+
+    @Test
+    fun `request with the same legal name as a pending request`() {
+        val requestId1 = storage.saveRequest(createRequest("BankA").first)
+        assertThat(storage.getPendingRequestIds()).containsOnly(requestId1)
+        val requestId2 = storage.saveRequest(createRequest("BankA").first)
+        assertThat(storage.getPendingRequestIds()).containsOnly(requestId1)
+        val response2 = storage.getResponse(requestId2) as CertificateResponse.Unauthorised
+        assertThat(response2.message).containsIgnoringCase("duplicate")
+        // Make sure the first request is processed properly
+        approveRequest(requestId1)
+        assertThat(storage.getResponse(requestId1)).isInstanceOf(CertificateResponse.Ready::class.java)
+    }
+
+    @Test
+    fun `request with the same legal name as a previously approved request`() {
+        val requestId1 = storage.saveRequest(createRequest("BankA").first)
+        approveRequest(requestId1)
+        val requestId2 = storage.saveRequest(createRequest("BankA").first)
+        val response2 = storage.getResponse(requestId2) as CertificateResponse.Unauthorised
+        assertThat(response2.message).containsIgnoringCase("duplicate")
+    }
+
+    @Test
+    fun `request with the same legal name as a previously rejected request`() {
+        val requestId1 = storage.saveRequest(createRequest("BankA").first)
+        storage.rejectRequest(requestId1, "Because I said so!")
+        val requestId2 = storage.saveRequest(createRequest("BankA").first)
+        assertThat(storage.getPendingRequestIds()).containsOnly(requestId2)
+        approveRequest(requestId2)
+        assertThat(storage.getResponse(requestId2)).isInstanceOf(CertificateResponse.Ready::class.java)
+    }
+
+    @Test
+    fun `request with equals symbol in legal name`() {
+        val requestId = storage.saveRequest(createRequest("Bank=A").first)
+        assertThat(storage.getPendingRequestIds()).isEmpty()
+        val response = storage.getResponse(requestId) as CertificateResponse.Unauthorised
+        assertThat(response.message).contains("=")
+    }
+
+    @Test
+    fun `request with comma in legal name`() {
+        val requestId = storage.saveRequest(createRequest("Bank,A").first)
+        assertThat(storage.getPendingRequestIds()).isEmpty()
+        val response = storage.getResponse(requestId) as CertificateResponse.Unauthorised
+        assertThat(response.message).contains(",")
+    }
+
+    private fun createRequest(legalName: String): Pair<CertificationData, KeyPair> {
+        val keyPair = X509Utilities.generateECDSAKeyPairForSSL()
+        val request = CertificationData(
+                "hostname",
+                "0.0.0.0",
+                X509Utilities.createCertificateSigningRequest(legalName, "London", "admin@test.com", keyPair))
+        return Pair(request, keyPair)
+    }
+
+    private fun approveRequest(requestId: String) {
+        storage.approveRequest(requestId) {
+            JcaPKCS10CertificationRequest(it.request).run {
+                X509Utilities.createServerCert(
+                        subject,
+                        publicKey,
+                        intermediateCA,
+                        if (it.ipAddress == it.hostName) listOf() else listOf(it.hostName),
+                        listOf(it.ipAddress))
+            }
         }
     }
 }

node/src/main/kotlin/net/corda/node/utilities/DatabaseSupport.kt

@@ -39,7 +39,7 @@ fun <T> databaseTransaction(db: Database, statement: Transaction.() -> T): T {
 /**
  *  Helper method wrapping code in try finally block. A mutable list is used to keep track of functions that need to be executed in finally block.
  */
-fun <T> withFinalizables(statement: (MutableList<() -> Unit>) -> T): T {
+inline fun <T> withFinalizables(statement: (MutableList<() -> Unit>) -> T): T {
     val finalizables = mutableListOf<() -> Unit>()
     return try {
         statement(finalizables)

node/src/main/kotlin/net/corda/node/utilities/certsigning/HTTPCertificateSigningService.kt

@@ -10,6 +10,7 @@ import java.net.URL
 import java.security.cert.Certificate
 import java.util.*
 import java.util.zip.ZipInputStream
+import javax.ws.rs.core.MediaType
 
 class HTTPCertificateSigningService(val server: URL) : CertificateSigningService {
     companion object {
@@ -20,7 +21,6 @@ class HTTPCertificateSigningService(val server: URL) : CertificateSigningService
     override fun retrieveCertificates(requestId: String): Array<Certificate>? {
         // Poll server to download the signed certificate once request has been approved.
         val url = URL("$server/api/certificate/$requestId")
-
         val conn = url.openConnection() as HttpURLConnection
         conn.requestMethod = "GET"
 
@@ -44,7 +44,7 @@ class HTTPCertificateSigningService(val server: URL) : CertificateSigningService
         val conn = URL("$server/api/certificate").openConnection() as HttpURLConnection
         conn.doOutput = true
         conn.requestMethod = "POST"
-        conn.setRequestProperty("Content-Type", "application/octet-stream")
+        conn.setRequestProperty("Content-Type", MediaType.APPLICATION_OCTET_STREAM)
         conn.setRequestProperty("Client-Version", clientVersion)
         conn.outputStream.write(request.encoded)