Small clean-up (#98)

* Small clean-up

* Unifying enum casing

* Adding TODOs

network-management/build.gradle

@@ -124,6 +124,7 @@ dependencies {
     testCompile "com.nhaarman:mockito-kotlin:0.6.1"
     testRuntime "net.corda:corda-rpc:$corda_dependency_version"
     testCompile "com.spotify:docker-client:8.9.1"
+    integrationTestCompile "net.corda:corda-test-utils:$corda_dependency_version"
     integrationTestRuntime "net.corda:corda-rpc:$corda_dependency_version"
 
     compile('com.atlassian.jira:jira-rest-java-client-core:4.0.0') {

network-management/src/integration-test/kotlin/com/r3/corda/networkmanage/hsm/DemoMain.kt

@@ -23,12 +23,12 @@ import java.util.*
  */
 fun main(args: Array<String>) {
     run(Parameters(
-            dataSourceProperties = makeTestDataSourceProperties("localhost"),
+            dataSourceProperties = makeTestDataSourceProperties(),
             databaseProperties = makeNotInitialisingTestDatabaseProperties()
     ))
 }
 
-private fun makeTestDataSourceProperties(nodeName: String): Properties {
+private fun makeTestDataSourceProperties(): Properties {
     val props = Properties()
     props.setProperty("dataSourceClassName", "org.h2.jdbcx.JdbcDataSource")
     props.setProperty("dataSource.url", "jdbc:h2:tcp://$HOST:$H2_TCP_PORT/mem:$DB_NAME;DB_CLOSE_DELAY=-1")

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/CertificationRequestStorage.kt

@@ -57,7 +57,7 @@ interface CertificationRequestStorage {
     fun rejectRequest(requestId: String, rejectedBy: String, rejectReason: String)
 
     /**
-     * Store certificate path with [requestId], this will store the encoded [CertPath] and transit request status to [RequestStatus.Signed].
+     * Store certificate path with [requestId], this will store the encoded [CertPath] and transit request status to [RequestStatus.SIGNED].
      * @param requestId id of the certificate signing request
      * @param signedBy authority (its identifier) signing this request.
      * @throws IllegalArgumentException if request is not found or not in Approved state.
@@ -71,16 +71,10 @@ sealed class CertificateResponse {
     data class Unauthorised(val message: String) : CertificateResponse()
 }
 
-/**
- * Describes certificate status
- */
-enum class CertificateStatus {
-    VALID, SUSPENDED, REVOKED
+enum class RequestStatus {
+    NEW, APPROVED, REJECTED, SIGNED
 }
 
-/**
- * Describes entire certificate signing request status
- */
-enum class RequestStatus {
-    New, Approved, Rejected, Signed
+enum class CertificateStatus {
+    VALID, SUSPENDED, REVOKED
 }

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentCertificateRequestStorage.kt

@@ -22,7 +22,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
         return database.transaction(Connection.TRANSACTION_SERIALIZABLE) {
             val request = singleRequestWhere(CertificateSigningRequestEntity::class.java) { builder, path ->
                 val requestIdEq = builder.equal(path.get<String>(CertificateSigningRequestEntity::requestId.name), requestId)
-                val statusEq = builder.equal(path.get<String>(CertificateSigningRequestEntity::status.name), RequestStatus.Approved)
+                val statusEq = builder.equal(path.get<String>(CertificateSigningRequestEntity::status.name), RequestStatus.APPROVED)
                 builder.and(requestIdEq, statusEq)
             }
             request ?: throw IllegalArgumentException("Cannot retrieve 'APPROVED' certificate signing request for request id: $requestId")
@@ -30,7 +30,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
             val certificateSigningRequest = request.copy(
                     modifiedBy = signedBy,
                     modifiedAt = Instant.now(),
-                    status = RequestStatus.Signed)
+                    status = RequestStatus.SIGNED)
             session.merge(certificateSigningRequest)
             val certificateDataEntity = CertificateDataEntity(
                     publicKeyHash = publicKeyHash,
@@ -51,7 +51,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
                     requestBytes = request.encoded,
                     remark = rejectReason,
                     modifiedBy = emptyList(),
-                    status = if (rejectReason == null) RequestStatus.New else RequestStatus.Rejected
+                    status = if (rejectReason == null) RequestStatus.NEW else RequestStatus.REJECTED
             ))
         }
         return requestId
@@ -61,13 +61,13 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
         return database.transaction(Connection.TRANSACTION_SERIALIZABLE) {
             val request = singleRequestWhere(CertificateSigningRequestEntity::class.java) { builder, path ->
                 builder.and(builder.equal(path.get<String>(CertificateSigningRequestEntity::requestId.name), requestId),
-                        builder.equal(path.get<String>(CertificateSigningRequestEntity::status.name), RequestStatus.New))
+                        builder.equal(path.get<String>(CertificateSigningRequestEntity::status.name), RequestStatus.NEW))
             }
             request ?: throw IllegalArgumentException("Error when approving request with id: $requestId. Request does not exist or its status is not NEW.")
             val update = request.copy(
                     modifiedBy = listOf(approvedBy),
                     modifiedAt = Instant.now(),
-                    status = RequestStatus.Approved)
+                    status = RequestStatus.APPROVED)
             session.merge(update)
         }
     }
@@ -81,7 +81,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
             val update = request.copy(
                     modifiedBy = listOf(rejectedBy),
                     modifiedAt = Instant.now(),
-                    status = RequestStatus.Rejected,
+                    status = RequestStatus.REJECTED,
                     remark = rejectReason
             )
             session.merge(update)
@@ -121,7 +121,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
             }
         }
         val duplicates = session.createQuery(query).resultList.filter {
-            it.status == RequestStatus.New || it.status == RequestStatus.Approved || it.certificateData?.certificateStatus == CertificateStatus.VALID
+            it.status == RequestStatus.NEW || it.status == RequestStatus.APPROVED || it.certificateData?.certificateStatus == CertificateStatus.VALID
         }
         return if (duplicates.isEmpty()) {
             Pair(legalName.x500Name, null)

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/PersistentNetworkMapStorage.kt

@@ -68,7 +68,7 @@ class PersistentNetworkMapStorage(private val database: CordaPersistence) : Netw
         val networkMapEntity = getCurrentNetworkMapEntity(getNetworkMapWithNodeInfoAndCsrHint(session))
         if (networkMapEntity != null) {
             networkMapEntity.nodeInfoList.filter({
-                certificateStatuses == null || certificateStatuses.isEmpty() || certificateStatuses.contains(it.certificateSigningRequest?.certificateData?.certificateStatus)
+                certificateStatuses.isEmpty() || certificateStatuses.contains(it.certificateSigningRequest?.certificateData?.certificateStatus)
             }).map { SecureHash.parse(it.nodeInfoHash) }
         } else {
             emptyList()

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/persistence/entity/CertificateSigningRequestEntity.kt

@@ -25,7 +25,7 @@ class CertificateSigningRequestEntity(
         @Audited
         @Column(name = "status", nullable = false)
         @Enumerated(EnumType.STRING)
-        val status: RequestStatus = RequestStatus.New,
+        val status: RequestStatus = RequestStatus.NEW,
 
         @Audited
         @Column(name = "modified_by", length = 512)

network-management/src/main/kotlin/com/r3/corda/networkmanage/common/signer/NetworkMapSigner.kt

@@ -35,7 +35,7 @@ class NetworkMapSigner(private val networkMapStorage: NetworkMapStorage,
         val nodeInfoHashes = currentNetworkMapValidNodeInfo + detachedValidNodeInfo
         val networkParameters = networkMapStorage.getLatestNetworkParameters()
         val networkMap = NetworkMap(nodeInfoHashes.map { it.toString() }, networkParameters.serialize().hash.toString())
-        if (currentSignedNetworkMap == null || networkMap != currentSignedNetworkMap.networkMap) {
+        if (networkMap != currentSignedNetworkMap.networkMap) {
             val digitalSignature = signer.sign(networkMap.serialize().bytes)
             require(digitalSignature != null) { "Error while signing network map." }
             val signedHashedNetworkMap = SignedNetworkMap(networkMap, digitalSignature!!)

network-management/src/main/kotlin/com/r3/corda/networkmanage/doorman/signer/CsrHandler.kt

@@ -15,7 +15,7 @@ interface CsrHandler {
 
 class DefaultCsrHandler(private val storage: CertificationRequestStorage, private val signer: LocalSigner?) : CsrHandler {
     override fun processApprovedRequests() {
-        storage.getRequests(RequestStatus.Approved)
+        storage.getRequests(RequestStatus.APPROVED)
                 .forEach { processRequest(it.requestId, it.request) }
     }
 
@@ -35,9 +35,9 @@ class DefaultCsrHandler(private val storage: CertificationRequestStorage, privat
     override fun getResponse(requestId: String): CertificateResponse {
         val response = storage.getRequest(requestId)
         return when (response?.status) {
-            RequestStatus.New, RequestStatus.Approved, null -> CertificateResponse.NotReady
-            RequestStatus.Rejected -> CertificateResponse.Unauthorised(response.remark ?: "Unknown reason")
-            RequestStatus.Signed -> CertificateResponse.Ready(response.certData?.certPath?: throw IllegalArgumentException("Certificate should not be null."))
+            RequestStatus.NEW, RequestStatus.APPROVED, null -> CertificateResponse.NotReady
+            RequestStatus.REJECTED -> CertificateResponse.Unauthorised(response.remark ?: "Unknown reason")
+            RequestStatus.SIGNED -> CertificateResponse.Ready(response.certData?.certPath ?: throw IllegalArgumentException("Certificate should not be null."))
         }
     }
 }
@@ -55,7 +55,7 @@ class JiraCsrHandler(private val jiraClient: JiraClient, private val storage: Ce
     override fun processApprovedRequests() {
         jiraClient.getApprovedRequests().forEach { (id, approvedBy) -> storage.approveRequest(id, approvedBy) }
         delegate.processApprovedRequests()
-        val signedRequests = storage.getRequests(RequestStatus.Signed).mapNotNull {
+        val signedRequests = storage.getRequests(RequestStatus.SIGNED).mapNotNull {
             it.certData?.certPath.let { certs -> it.requestId to certs!! }
         }.toMap()
         jiraClient.updateSignedRequests(signedRequests)

network-management/src/main/kotlin/com/r3/corda/networkmanage/hsm/authentication/AuthMode.kt

@@ -1,6 +1,6 @@
 package com.r3.corda.networkmanage.hsm.authentication
 
-/*
+/**
  * Supported authentication modes
  */
 enum class AuthMode {

network-management/src/main/kotlin/com/r3/corda/networkmanage/hsm/authentication/Authenticator.kt

@@ -8,6 +8,9 @@ import java.io.Console
 import java.nio.file.Path
 import kotlin.reflect.full.memberProperties
 
+/**
+ * Performs user authentication against the HSM
+ */
 class Authenticator(private val provider: CryptoServerProvider,
                     private val mode: AuthMode = AuthMode.PASSWORD,
                     private val autoUsername: String? = null,

network-management/src/main/kotlin/com/r3/corda/networkmanage/hsm/menu/Menu.kt

@@ -1,5 +1,9 @@
 package com.r3.corda.networkmanage.hsm.menu
 
+/**
+ * Single menu item described by the selection [key], label associated with this item, action to be executed on response
+ * to the item selection and termination flag used to decide whether to loop after action completion.
+ */
 data class MenuItem(val key: String, val label: String, val action: () -> Unit, val isTerminating: Boolean = false)
 
 /**

network-management/src/main/kotlin/com/r3/corda/networkmanage/hsm/persistence/DBSignedCertificateRequestStorage.kt

@@ -20,7 +20,7 @@ class DBSignedCertificateRequestStorage(database: CordaPersistence) : SignedCert
     }
 
     override fun getApprovedRequests(): List<ApprovedCertificateRequestData> {
-        return storage.getRequests(RequestStatus.Approved).map { it.toRequestData() }
+        return storage.getRequests(RequestStatus.APPROVED).map { it.toRequestData() }
     }
 
     private fun CertificateSigningRequest.toRequestData() = ApprovedCertificateRequestData(requestId, request)

network-management/src/main/kotlin/com/r3/corda/networkmanage/hsm/utils/HsmErrors.kt

@@ -7,6 +7,7 @@ import java.util.*
  * CryptoServer error translator object.
  * It holds mapping between CryptoServer error code to its human readable description.
  */
+// TODO this code (incl. the hsm_errors file) is duplicated with the SGX module
 object HsmErrors {
     val errors: Map<Int, String> by lazy(HsmErrors::load)
 

network-management/src/test/kotlin/com/r3/corda/networkmanage/TestBase.kt

@@ -25,7 +25,7 @@ abstract class TestBase {
 
     protected fun certificateSigningRequestEntity(
             requestId: String = SecureHash.randomSHA256().toString(),
-            status: RequestStatus = RequestStatus.New,
+            status: RequestStatus = RequestStatus.NEW,
             legalName: String = "TestLegalName",
             modifiedBy: List<String> = emptyList(),
             modifiedAt: Instant = Instant.now(),
@@ -47,7 +47,7 @@ abstract class TestBase {
 
     protected fun certificateSigningRequest(
             requestId: String = SecureHash.randomSHA256().toString(),
-            status: RequestStatus = RequestStatus.New,
+            status: RequestStatus = RequestStatus.NEW,
             legalName: String = "TestLegalName",
             remark: String = "Test remark",
             request: PKCS10CertificationRequest = mock(),

network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/DBCertificateRequestStorageTest.kt

@@ -46,7 +46,7 @@ class DBCertificateRequestStorageTest : TestBase() {
         assertNotNull(storage.getRequest(requestId)).apply {
             assertEquals(request, this.request)
         }
-        assertThat(storage.getRequests(RequestStatus.New).map { it.requestId }).containsOnly(requestId)
+        assertThat(storage.getRequests(RequestStatus.NEW).map { it.requestId }).containsOnly(requestId)
     }
 
     @Test
@@ -55,7 +55,7 @@ class DBCertificateRequestStorageTest : TestBase() {
         // Add request to DB.
         val requestId = storage.saveRequest(request)
         // Pending request should equals to 1.
-        assertEquals(1, storage.getRequests(RequestStatus.New).size)
+        assertEquals(1, storage.getRequests(RequestStatus.NEW).size)
         // Certificate should be empty.
         assertNull(storage.getRequest(requestId)!!.certData)
         // Store certificate to DB.
@@ -63,7 +63,7 @@ class DBCertificateRequestStorageTest : TestBase() {
         // Check request is not ready yet.
         // assertTrue(storage.getResponse(requestId) is CertificateResponse.NotReady)
         // New request should be empty.
-        assertTrue(storage.getRequests(RequestStatus.New).isEmpty())
+        assertTrue(storage.getRequests(RequestStatus.NEW).isEmpty())
     }
 
     @Test
@@ -91,15 +91,15 @@ class DBCertificateRequestStorageTest : TestBase() {
         // Add request to DB.
         val requestId = storage.saveRequest(csr)
         // New request should equals to 1.
-        assertEquals(1, storage.getRequests(RequestStatus.New).size)
+        assertEquals(1, storage.getRequests(RequestStatus.NEW).size)
         // Certificate should be empty.
         assertNull(storage.getRequest(requestId)!!.certData)
         // Store certificate to DB.
         storage.approveRequest(requestId, DOORMAN_SIGNATURE)
         // Check request is not ready yet.
-        assertEquals(RequestStatus.Approved, storage.getRequest(requestId)!!.status)
+        assertEquals(RequestStatus.APPROVED, storage.getRequest(requestId)!!.status)
         // New request should be empty.
-        assertTrue(storage.getRequests(RequestStatus.New).isEmpty())
+        assertTrue(storage.getRequests(RequestStatus.NEW).isEmpty())
         // Sign certificate
         storage.putCertificatePath(requestId, JcaPKCS10CertificationRequest(csr).run {
             val rootCAKey = Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME)
@@ -146,21 +146,21 @@ class DBCertificateRequestStorageTest : TestBase() {
     fun `reject request`() {
         val requestId = storage.saveRequest(createRequest("BankA").first)
         storage.rejectRequest(requestId, DOORMAN_SIGNATURE, "Because I said so!")
-        assertThat(storage.getRequests(RequestStatus.New)).isEmpty()
+        assertThat(storage.getRequests(RequestStatus.NEW)).isEmpty()
         assertThat(storage.getRequest(requestId)!!.remark).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.getRequests(RequestStatus.New).map { it.requestId }).containsOnly(requestId1)
+        assertThat(storage.getRequests(RequestStatus.NEW).map { it.requestId }).containsOnly(requestId1)
         val requestId2 = storage.saveRequest(createRequest("BankA").first)
-        assertThat(storage.getRequests(RequestStatus.New).map { it.requestId }).containsOnly(requestId1)
-        assertEquals(RequestStatus.Rejected, storage.getRequest(requestId2)!!.status)
+        assertThat(storage.getRequests(RequestStatus.NEW).map { it.requestId }).containsOnly(requestId1)
+        assertEquals(RequestStatus.REJECTED, storage.getRequest(requestId2)!!.status)
         assertThat(storage.getRequest(requestId2)!!.remark).containsIgnoringCase("duplicate")
         // Make sure the first request is processed properly
         storage.approveRequest(requestId1, DOORMAN_SIGNATURE)
-        assertThat(storage.getRequest(requestId1)!!.status).isEqualTo(RequestStatus.Approved)
+        assertThat(storage.getRequest(requestId1)!!.status).isEqualTo(RequestStatus.APPROVED)
     }
 
     @Test
@@ -176,9 +176,9 @@ class DBCertificateRequestStorageTest : TestBase() {
         val requestId1 = storage.saveRequest(createRequest("BankA").first)
         storage.rejectRequest(requestId1, DOORMAN_SIGNATURE, "Because I said so!")
         val requestId2 = storage.saveRequest(createRequest("BankA").first)
-        assertThat(storage.getRequests(RequestStatus.New).map { it.requestId }).containsOnly(requestId2)
+        assertThat(storage.getRequests(RequestStatus.NEW).map { it.requestId }).containsOnly(requestId2)
         storage.approveRequest(requestId2, DOORMAN_SIGNATURE)
-        assertThat(storage.getRequest(requestId2)!!.status).isEqualTo(RequestStatus.Approved)
+        assertThat(storage.getRequest(requestId2)!!.status).isEqualTo(RequestStatus.APPROVED)
     }
 
     @Test
@@ -194,10 +194,10 @@ class DBCertificateRequestStorageTest : TestBase() {
         persistence.transaction {
             val auditReader = AuditReaderFactory.get(persistence.entityManagerFactory.createEntityManager())
             val newRevision = auditReader.find(CertificateSigningRequestEntity::class.java, requestId, 1)
-            assertEquals(RequestStatus.New, newRevision.status)
+            assertEquals(RequestStatus.NEW, newRevision.status)
             assertTrue(newRevision.modifiedBy.isEmpty())
             val approvedRevision = auditReader.find(CertificateSigningRequestEntity::class.java, requestId, 2)
-            assertEquals(RequestStatus.Approved, approvedRevision.status)
+            assertEquals(RequestStatus.APPROVED, approvedRevision.status)
             assertEquals(approver, approvedRevision.modifiedBy.first())
         }
     }

network-management/src/test/kotlin/com/r3/corda/networkmanage/common/persistence/DBNetworkMapStorageTest.kt

@@ -68,7 +68,7 @@ class DBNetworkMapStorageTest : TestBase() {
         nodeInfoStorage.signNodeInfo(nodeInfoHash, signature)
 
         // Create network parameters
-        val networkParametersHash = networkMapStorage.putNetworkParameters(testNetworkParameters(emptyList<NotaryInfo>()))
+        val networkParametersHash = networkMapStorage.putNetworkParameters(testNetworkParameters(emptyList()))
 
         val signatureData = SignatureAndCertPath(signature, certPath)
         val signedNetworkMap = SignedNetworkMap(NetworkMap(listOf(nodeInfoHash.toString()), networkParametersHash.toString()), signatureData)

network-management/src/test/kotlin/com/r3/corda/networkmanage/common/signer/NetworkMapSignerTest.kt

@@ -29,6 +29,8 @@ class NetworkMapSignerTest : TestBase() {
         val signedNodeInfoHashes = listOf(SecureHash.randomSHA256(), SecureHash.randomSHA256())
         val detachedNodeInfoHashes = listOf(SecureHash.randomSHA256())
         val networkMapParameters = createNetworkParameters()
+        whenever(networkMapStorage.getCurrentNetworkMap())
+                .thenReturn(SignedNetworkMap(NetworkMap(signedNodeInfoHashes.map { it.toString() }, "Dummy"), mock()))
         whenever(networkMapStorage.getCurrentNetworkMapNodeInfoHashes(any())).thenReturn(signedNodeInfoHashes)
         whenever(networkMapStorage.getDetachedSignedAndValidNodeInfoHashes()).thenReturn(detachedNodeInfoHashes)
         whenever(networkMapStorage.getLatestNetworkParameters()).thenReturn(networkMapParameters)

network-management/src/test/kotlin/com/r3/corda/networkmanage/doorman/DefaultRequestProcessorTest.kt

@@ -27,8 +27,8 @@ class DefaultRequestProcessorTest : TestBase() {
 
         val requestStorage: CertificationRequestStorage = mock {
             on { getRequest("New") }.thenReturn(certificateSigningRequest())
-            on { getRequest("Signed") }.thenReturn(certificateSigningRequest(status = RequestStatus.Signed, certData = certificateData("", CertificateStatus.VALID, buildCertPath(cert.toX509Certificate()))))
-            on { getRequest("Rejected") }.thenReturn(certificateSigningRequest(status = RequestStatus.Rejected, remark = "Random reason"))
+            on { getRequest("Signed") }.thenReturn(certificateSigningRequest(status = RequestStatus.SIGNED, certData = certificateData("", CertificateStatus.VALID, buildCertPath(cert.toX509Certificate()))))
+            on { getRequest("Rejected") }.thenReturn(certificateSigningRequest(status = RequestStatus.REJECTED, remark = "Random reason"))
         }
         val signer: LocalSigner = mock()
         val requestProcessor = DefaultCsrHandler(requestStorage, signer)
@@ -46,10 +46,10 @@ class DefaultRequestProcessorTest : TestBase() {
         val request3 = X509Utilities.createCertificateSigningRequest(CordaX500Name(locality = "London", organisation = "Test3", country = "GB"), "my@email.com", Crypto.generateKeyPair(X509Utilities.DEFAULT_TLS_SIGNATURE_SCHEME))
 
         val requestStorage: CertificationRequestStorage = mock {
-            on { getRequests(RequestStatus.Approved) }.thenReturn(listOf(
-                    certificateSigningRequest(requestId = "1", request = request1, status = RequestStatus.Approved),
-                    certificateSigningRequest(requestId = "2", request = request2, status = RequestStatus.Approved),
-                    certificateSigningRequest(requestId = "3", request = request3, status = RequestStatus.Approved)
+            on { getRequests(RequestStatus.APPROVED) }.thenReturn(listOf(
+                    certificateSigningRequest(requestId = "1", request = request1, status = RequestStatus.APPROVED),
+                    certificateSigningRequest(requestId = "2", request = request2, status = RequestStatus.APPROVED),
+                    certificateSigningRequest(requestId = "3", request = request3, status = RequestStatus.APPROVED)
             ))
         }
         val signer: LocalSigner = mock()

sgx-jvm/hsm-tool/src/main/kotlin/com/r3cev/sgx/utils/HsmErrors.kt

@@ -2,7 +2,7 @@ package com.r3cev.sgx.utils
 
 import java.util.*
 
-
+// TODO this code (incl. the hsm_errors file) is duplicated with the Network-Management module
 object HsmErrors {
     val errors: Map<Int, String> by lazy(HsmErrors::load)