Addressed PR issues

doorman/src/main/kotlin/com/r3/corda/doorman/DoormanParameters.kt

@@ -15,7 +15,6 @@ class DoormanParameters(args: Array<String>) {
         accepts("basedir", "Overriding configuration filepath, default to current directory.").withRequiredArg().describedAs("filepath")
         accepts("keygen", "Generate CA keypair and certificate using provide Root CA key.").withOptionalArg()
         accepts("rootKeygen", "Generate Root CA keypair and certificate.").withOptionalArg()
-        accepts("approveAll", "Approve all certificate signing request.").withOptionalArg()
         accepts("keystorePath", "CA keystore filepath, default to [basedir]/certificates/caKeystore.jks.").withRequiredArg().describedAs("filepath")
         accepts("rootStorePath", "Root CA keystore filepath, default to [basedir]/certificates/rootCAKeystore.jks.").withRequiredArg().describedAs("filepath")
         accepts("keystorePassword", "CA keystore password.").withRequiredArg().describedAs("password")
@@ -33,7 +32,6 @@ class DoormanParameters(args: Array<String>) {
     val caPrivateKeyPassword: String? by config.getOrElse { null }
     val rootKeystorePassword: String? by config.getOrElse { null }
     val rootPrivateKeyPassword: String? by config.getOrElse { null }
-    val approveAll: Boolean by config.getOrElse { false }
     val host: String by config
     val port: Int by config
     val dataSourceProperties: Properties by config

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

@@ -164,12 +164,14 @@ private fun DoormanParameters.startDoorman() {
     // Create DB connection.
     val (datasource, database) = configureDatabase(dataSourceProperties)
 
-    val storage = if (approveAll) {
+    val storage = if (jiraConfig == null) {
         logger.warn("Doorman server is in 'Approve All' mode, this will approve all incoming certificate signing request.")
-        DBCertificateRequestStorage(database)
+        // Approve all pending request.
+        object : CertificationRequestStorage by DBCertificateRequestStorage(database) {
+            override fun getApprovedRequestIds() = getPendingRequestIds()
+        }
     } else {
-        // Require JIRA config to be non-null.
-        val jiraClient = AsynchronousJiraRestClientFactory().createWithBasicHttpAuthentication(URI(jiraConfig!!.address), jiraConfig.username, jiraConfig.password)
+        val jiraClient = AsynchronousJiraRestClientFactory().createWithBasicHttpAuthentication(URI(jiraConfig.address), jiraConfig.username, jiraConfig.password)
         JiraCertificateRequestStorage(DBCertificateRequestStorage(database), jiraClient, jiraConfig.projectCode, jiraConfig.doneTransitionCode)
     }
 
@@ -177,8 +179,8 @@ private fun DoormanParameters.startDoorman() {
     thread(name = "Request Approval Daemon") {
         while (true) {
             sleep(10.seconds.toMillis())
-            val approvedRequests = (storage as? JiraCertificateRequestStorage)?.getRequestByStatus(JiraCertificateRequestStorage.APPROVED) ?: storage.getPendingRequestIds()
-            for (id in approvedRequests) {
+            // TODO: Handle rejected request?
+            for (id in storage.getApprovedRequestIds()) {
                 storage.approveRequest(id) {
                     val request = JcaPKCS10CertificationRequest(request)
                     createServerCert(request.subject, request.publicKey, caCertAndKey,

doorman/src/main/kotlin/com/r3/corda/doorman/persistence/CertificationRequestStorage.kt

@@ -35,9 +35,13 @@ interface CertificationRequestStorage {
 
     /**
      * 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 getPendingRequestIds(): List<String>
+
+    /**
+     * Retrieve list of approved request IDs.
+     */
+    fun getApprovedRequestIds(): List<String>
 }
 
 data class CertificationRequestData(val hostName: String, val ipAddress: String, val request: PKCS10CertificationRequest)

doorman/src/main/kotlin/com/r3/corda/doorman/persistence/DBCertificateRequestStorage.kt

@@ -121,6 +121,8 @@ class DBCertificateRequestStorage(private val database: Database) : Certificatio
         }
     }
 
+    override fun getApprovedRequestIds(): List<String> = emptyList()
+
     private fun singleRequestWhere(where: SqlExpressionBuilder.() -> Op<Boolean>): CertificationRequestData? {
         return DataTable
                 .select(where)

doorman/src/main/kotlin/com/r3/corda/doorman/persistence/JiraCertificateRequestStorage.kt

@@ -1,64 +1,73 @@
 package com.r3.corda.doorman.persistence
 
 import com.atlassian.jira.rest.client.api.JiraRestClient
+import com.atlassian.jira.rest.client.api.domain.Field
+import com.atlassian.jira.rest.client.api.domain.IssueType
 import com.atlassian.jira.rest.client.api.domain.input.IssueInputBuilder
 import com.atlassian.jira.rest.client.api.domain.input.TransitionInput
 import net.corda.core.crypto.X509Utilities
 import net.corda.core.crypto.commonName
+import net.corda.core.utilities.loggerFor
 import org.bouncycastle.asn1.x500.style.BCStyle
 import org.bouncycastle.openssl.jcajce.JcaPEMWriter
 import org.bouncycastle.util.io.pem.PemObject
-import java.io.ByteArrayInputStream
 import java.io.StringWriter
 import java.security.cert.Certificate
 
-class JiraCertificateRequestStorage(val delegate: CertificationRequestStorage, val jiraClient: JiraRestClient, val projectCode: String, val doneTransitionCode: Int) : CertificationRequestStorage by delegate {
-    companion object {
-        val APPROVED = "Approved"
-        val REJECTED = "Rejected"
+class JiraCertificateRequestStorage(val delegate: CertificationRequestStorage,
+                                    val jiraClient: JiraRestClient,
+                                    val projectCode: String,
+                                    val doneTransitionCode: Int) : CertificationRequestStorage by delegate {
+    private enum class Status {
+        Approved, Rejected
     }
 
+    private val logger = loggerFor<JiraCertificateRequestStorage>()
+
     // The JIRA project must have a Request ID field and the Task issue type.
-    private val requestIdField = jiraClient.metadataClient.fields.claim().find { it.name == "Request ID" }!!
-    private val taskIssueType = jiraClient.metadataClient.issueTypes.claim().find { it.name == "Task" }!!
+    private val requestIdField: Field = jiraClient.metadataClient.fields.claim().find { it.name == "Request ID" }!!
+    private val taskIssueType: IssueType = jiraClient.metadataClient.issueTypes.claim().find { it.name == "Task" }!!
 
     override fun saveRequest(certificationData: CertificationRequestData): String {
         val requestId = delegate.saveRequest(certificationData)
         // Make sure request has been accepted.
         val response = getResponse(requestId)
         if (response !is CertificateResponse.Unauthorised) {
-            val request = StringWriter().use {
-                JcaPEMWriter(it).use {
-                    it.writeObject(PemObject("CERTIFICATE REQUEST", certificationData.request.encoded))
-                }
-                it.toString()
+            val request = StringWriter()
+            JcaPEMWriter(request).use {
+                it.writeObject(PemObject("CERTIFICATE REQUEST", certificationData.request.encoded))
             }
             val commonName = certificationData.request.subject.commonName
             val email = certificationData.request.subject.getRDNs(BCStyle.EmailAddress).firstOrNull()?.first?.value
             val nearestCity = certificationData.request.subject.getRDNs(BCStyle.L).firstOrNull()?.first?.value
 
             val issue = IssueInputBuilder().setIssueTypeId(taskIssueType.id)
-                    .setProjectKey("TD")
+                    .setProjectKey(projectCode)
                     .setDescription("Legal Name: $commonName\nNearest City: $nearestCity\nEmail: $email\n\n{code}$request{code}")
                     .setSummary(commonName)
                     .setFieldValue(requestIdField.id, requestId)
-
-            jiraClient.issueClient.createIssue(issue.build()).fail(Throwable::printStackTrace).claim()
+            // This will block until the jira is created.
+            jiraClient.issueClient.createIssue(issue.build()).fail { logger.error("Exception when creating JIRA issue.", it) }.claim()
         }
         return requestId
     }
 
     override fun approveRequest(requestId: String, generateCertificate: CertificationRequestData.() -> Certificate) {
         delegate.approveRequest(requestId, generateCertificate)
+        // Certificate should be created, retrieving it to attach to the jira task.
         val certificate = (getResponse(requestId) as? CertificateResponse.Ready)?.certificate
+        // Jira only support ~ (contains) search for custom textfield.
         val issue = jiraClient.searchClient.searchJql("'Request ID' ~ $requestId").claim().issues.firstOrNull()
-        issue?.let {
-            jiraClient.issueClient.transition(it, TransitionInput(doneTransitionCode)).fail(Throwable::printStackTrace)
-            jiraClient.issueClient.addAttachment(it.attachmentsUri, ByteArrayInputStream(certificate?.encoded), "${X509Utilities.CORDA_CLIENT_CA}.cer")
+        if (issue != null) {
+            jiraClient.issueClient.transition(issue, TransitionInput(doneTransitionCode)).fail { logger.error("Exception when transiting JIRA status.", it) }.claim()
+            jiraClient.issueClient.addAttachment(issue.attachmentsUri, certificate?.encoded?.inputStream(), "${X509Utilities.CORDA_CLIENT_CA}.cer")
+                    .fail { logger.error("Exception when uploading attachment to JIRA.", it) }.claim()
         }
     }
 
-    fun getRequestByStatus(status: String): List<String> {
+    override fun getApprovedRequestIds(): List<String> = getRequestByStatus(Status.Approved)
+
+    private fun getRequestByStatus(status: Status): List<String> {
         val issues = jiraClient.searchClient.searchJql("project = $projectCode AND status = $status").claim().issues
         return issues.map { it.getField(requestIdField.id)?.value?.toString() }.filterNotNull()
     }

doorman/src/main/resources/reference.conf

@@ -3,7 +3,6 @@ port = 0
 keystorePath = ${basedir}"/certificates/caKeystore.jks"
 keystorePassword = "password"
 caPrivateKeyPassword = "password"
-approveAll = true
 
 dataSourceProperties {
   dataSourceClassName = org.h2.jdbcx.JdbcDataSource

doorman/src/test/kotlin/com/r3/corda/doorman/DoormanParametersTest.kt

@@ -7,11 +7,10 @@ import kotlin.test.assertTrue
 class DoormanParametersTest {
     @Test
     fun `parse arg correctly`() {
-        val params = DoormanParameters(arrayOf("--keygen", "--keystorePath", "./testDummyPath.jks", "--approveAll"))
+        val params = DoormanParameters(arrayOf("--keygen", "--keystorePath", "./testDummyPath.jks"))
         assertEquals(DoormanParameters.Mode.CA_KEYGEN, params.mode)
         assertEquals("./testDummyPath.jks", params.keystorePath.toString())
         assertEquals(0, params.port)
-        assertTrue(params.approveAll)
 
         val params2 = DoormanParameters(arrayOf("--keystorePath", "./testDummyPath.jks", "--port", "1000"))
         assertEquals(DoormanParameters.Mode.DOORMAN, params2.mode)