Refactoring modifiedBy and status fields for the certificate signing … (#475)

* Refactoring modifiedBy and status fields for the certificate signing request entity

* Fixing migration
This commit is contained in:
Michal Kit 2018-03-02 08:50:38 +00:00 committed by GitHub
parent 26a11bccc9
commit 2d16647498
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 104 additions and 44 deletions

View File

@ -78,7 +78,7 @@ class SigningServiceIntegrationTest : HsmBaseTest() {
approvedRequest.certPath = X509Utilities.buildCertPath(nodeCa.certificate, intermediateCa.certificate, rootCaCert)
}
}
storage.store(approvedRequests, listOf("TEST"))
storage.store(approvedRequests, "TEST")
}
}
}

View File

@ -12,7 +12,7 @@ data class CertificateSigningRequest(val requestId: String,
val status: RequestStatus,
val request: PKCS10CertificationRequest,
val remark: String?,
val modifiedBy: List<String>,
val modifiedBy: String,
val certData: CertificateData?)
/**
@ -64,12 +64,12 @@ 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.DONE].
* @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.
*/
fun putCertificatePath(requestId: String, certificates: CertPath, signedBy: List<String>)
fun putCertificatePath(requestId: String, certificates: CertPath, signedBy: String)
}
sealed class CertificateResponse {
@ -100,9 +100,9 @@ enum class RequestStatus {
REJECTED,
/**
* The request has been signed, this is a terminal state, once a request gets in this state it won't change anymore.
* The request has been successfully processed, this is a terminal state, once a request gets in this state it won't change anymore.
*/
SIGNED
DONE
}
enum class CertificateStatus {

View File

@ -25,7 +25,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
private val allowedCertRoles = setOf(CertRole.NODE_CA, CertRole.SERVICE_IDENTITY)
}
override fun putCertificatePath(requestId: String, certificates: CertPath, signedBy: List<String>) {
override fun putCertificatePath(requestId: String, certificates: CertPath, signedBy: String) {
return database.transaction(TransactionIsolationLevel.SERIALIZABLE) {
val request = singleRequestWhere(CertificateSigningRequestEntity::class.java) { builder, path ->
val requestIdEq = builder.equal(path.get<String>(CertificateSigningRequestEntity::requestId.name), requestId)
@ -36,7 +36,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
val certificateSigningRequest = request.copy(
modifiedBy = signedBy,
modifiedAt = Instant.now(),
status = RequestStatus.SIGNED)
status = RequestStatus.DONE)
session.merge(certificateSigningRequest)
val certificateDataEntity = CertificateDataEntity(
certificateStatus = CertificateStatus.VALID,
@ -56,7 +56,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
legalName = legalName,
publicKeyHash = toSupportedPublicKey(request.subjectPublicKeyInfo).hashString(),
requestBytes = request.encoded,
modifiedBy = emptyList(),
modifiedBy = CertificationRequestStorage.DOORMAN_SIGNATURE,
status = RequestStatus.NEW
)
} catch (e: RequestValidationException) {
@ -66,7 +66,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
publicKeyHash = toSupportedPublicKey(request.subjectPublicKeyInfo).hashString(),
requestBytes = request.encoded,
remark = e.rejectMessage,
modifiedBy = emptyList(),
modifiedBy = CertificationRequestStorage.DOORMAN_SIGNATURE,
status = RequestStatus.REJECTED
)
}
@ -103,7 +103,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
return database.transaction(TransactionIsolationLevel.SERIALIZABLE) {
findRequest(requestId, RequestStatus.TICKET_CREATED)?.let {
val update = it.copy(
modifiedBy = listOf(approvedBy),
modifiedBy = approvedBy,
modifiedAt = Instant.now(),
status = RequestStatus.APPROVED)
session.merge(update)
@ -116,7 +116,7 @@ class PersistentCertificateRequestStorage(private val database: CordaPersistence
val request = findRequest(requestId)
request ?: throw IllegalArgumentException("Error when rejecting request with id: $requestId. Request does not exist.")
val update = request.copy(
modifiedBy = listOf(rejectedBy),
modifiedBy = rejectedBy,
modifiedAt = Instant.now(),
status = RequestStatus.REJECTED,
remark = rejectReason

View File

@ -66,7 +66,7 @@ class PersistentNodeInfoStorage(private val database: CordaPersistence) : NodeIn
private fun getSignedRequestByPublicHash(publicKeyHash: SecureHash, transaction: DatabaseTransaction): CertificateSigningRequestEntity? {
return transaction.singleRequestWhere(CertificateSigningRequestEntity::class.java) { builder, path ->
val publicKeyEq = builder.equal(path.get<String>(CertificateSigningRequestEntity::publicKeyHash.name), publicKeyHash.toString())
val statusEq = builder.equal(path.get<RequestStatus>(CertificateSigningRequestEntity::status.name), RequestStatus.SIGNED)
val statusEq = builder.equal(path.get<RequestStatus>(CertificateSigningRequestEntity::status.name), RequestStatus.DONE)
builder.and(publicKeyEq, statusEq)
}
}

View File

@ -33,8 +33,7 @@ class CertificateSigningRequestEntity(
@Audited
@Column(name = "modified_by", length = 512)
@ElementCollection(targetClass = String::class, fetch = FetchType.EAGER)
val modifiedBy: List<String> = emptyList(),
val modifiedBy: String,
@Audited
@Column(name = "modified_at", nullable = false)
@ -66,7 +65,7 @@ class CertificateSigningRequestEntity(
legalName: String = this.legalName,
publicKeyHash: String = this.publicKeyHash,
status: RequestStatus = this.status,
modifiedBy: List<String> = this.modifiedBy,
modifiedBy: String = this.modifiedBy,
modifiedAt: Instant = this.modifiedAt,
remark: String? = this.remark,
certificateData: CertificateDataEntity? = this.certificateData,

View File

@ -34,7 +34,7 @@ class DefaultCsrHandler(private val storage: CertificationRequestStorage,
val nodeCertPath = createSignedNodeCertificate(it.request, csrCertPathAndKey)
// Since Doorman is deployed in the auto-signing mode (i.e. signer != null),
// we use DOORMAN_SIGNATURE as the signer.
storage.putCertificatePath(it.requestId, nodeCertPath, listOf(DOORMAN_SIGNATURE))
storage.putCertificatePath(it.requestId, nodeCertPath, DOORMAN_SIGNATURE)
}
}
@ -45,7 +45,7 @@ class DefaultCsrHandler(private val storage: CertificationRequestStorage,
return when (response?.status) {
RequestStatus.NEW, RequestStatus.APPROVED, RequestStatus.TICKET_CREATED, 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.DONE -> CertificateResponse.Ready(response.certData?.certPath ?: throw IllegalArgumentException("Certificate should not be null."))
}
}

View File

@ -8,7 +8,6 @@ import com.r3.corda.networkmanage.doorman.ApprovedRequest
import com.r3.corda.networkmanage.doorman.JiraClient
import com.r3.corda.networkmanage.doorman.RejectedRequest
import net.corda.core.utilities.contextLogger
import net.corda.core.utilities.loggerFor
import org.bouncycastle.pkcs.PKCS10CertificationRequest
class JiraCsrHandler(private val jiraClient: JiraClient, private val storage: CertificationRequestStorage, private val delegate: CsrHandler) : CsrHandler by delegate {
@ -50,7 +49,7 @@ class JiraCsrHandler(private val jiraClient: JiraClient, private val storage: Ce
private fun updateJiraTickets(approvedRequest: List<ApprovedRequest>, rejectedRequest: List<RejectedRequest>) {
// Reconfirm request status and update jira status
val signedRequests = approvedRequest.mapNotNull { storage.getRequest(it.requestId) }
.filter { it.status == RequestStatus.SIGNED && it.certData != null }
.filter { it.status == RequestStatus.DONE && it.certData != null }
.associateBy { it.requestId }
.mapValues { it.value.certData!!.certPath }
jiraClient.updateSignedRequests(signedRequests)

View File

@ -13,9 +13,9 @@ class DBSignedCertificateRequestStorage(database: CordaPersistence) : SignedCert
private val storage = PersistentCertificateRequestStorage(database)
override fun store(requests: List<ApprovedCertificateRequestData>, signers: List<String>) {
override fun store(requests: List<ApprovedCertificateRequestData>, signer: String) {
for ((requestId, _, certPath) in requests) {
storage.putCertificatePath(requestId, certPath!!, signers)
storage.putCertificatePath(requestId, certPath!!, signer)
}
}

View File

@ -16,5 +16,5 @@ interface SignedCertificateRequestStorage {
* @param requests Signed requests that are to be stored.
* @param signers List of user names that signed those requests. To be specific, each request has been signed by all of those users.
*/
fun store(requests: List<ApprovedCertificateRequestData>, signers: List<String>)
fun store(requests: List<ApprovedCertificateRequestData>, signer: String)
}

View File

@ -64,7 +64,7 @@ class HsmCsrSigner(private val storage: SignedCertificateRequestStorage,
it.certPath = buildCertPath(nodeCaCert, doormanCertAndKey.certificate, rootCert)
}
logger.debug("Storing signed CSRs...")
storage.store(toSign, signers)
storage.store(toSign, signers.toString())
printStream.println("The following certificates have been signed by $signers:")
logger.debug("The following certificates have been signed by $signers:")
toSign.forEachIndexed { index, data ->

View File

@ -4,5 +4,6 @@
<include file="migration/network-manager.changelog-init.xml"/>
<include file="migration/network-manager.changelog-signing-network-params.xml"/>
<include file="migration/network-manager.changelog-pub-key-move.xml"/>
<include file="migration/network-manager.changelog-modified-by-refactor.xml"/>
</databaseChangeLog>

View File

@ -0,0 +1,61 @@
<?xml version="1.1" encoding="UTF-8" standalone="no"?>
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog" xmlns:ext="http://www.liquibase.org/xml/ns/dbchangelog-ext" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog-ext http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-ext.xsd http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-3.5.xsd">
<changeSet author="R3.Corda" id="refactor_request_status">
<update tableName="certificate_signing_request">
<column name="status" value="DONE"/>
<where>status = 'SIGNED'</where>
</update>
<update tableName="certificate_signing_request_AUD">
<column name="status" value="DONE"/>
<where>status = 'SIGNED'</where>
</update>
</changeSet>
<changeSet author="R3.Corda" id="refactor_modified_by">
<addColumn tableName="certificate_signing_request">
<column name="modified_by" type="NVARCHAR(512)"/>
</addColumn>
<addColumn tableName="certificate_signing_request_AUD">
<column name="modified_by" type="NVARCHAR(512)"/>
</addColumn>
<update tableName="certificate_signing_request">
<column name="modified_by" valueComputed="(select b.modified_by from CertificateSigningRequestEntity_modifiedBy b where b.CertificateSigningRequestEntity_request_id=request_id)"/>
</update>
</changeSet>
<changeSet author="R3.Corda" id="refactor_modified_by_sql_azure" dbms="azure">
<sql>
UPDATE
csr_a
SET
csr_a.modified_by = csr_mb_a.modified_by
FROM
certificate_signing_request_AUD AS csr_a
INNER JOIN CertificateSigningRequestEntity_modifiedBy_AUD AS csr_mb_a
ON csr_a.rev = csr_mb_a.rev
WHERE
csr_mb_a.revtype = 0
</sql>
</changeSet>
<changeSet author="R3.Corda" id="refactor_modified_by_sql_h2" dbms="h2">
<sql>
UPDATE
certificate_signing_request_AUD as csr_a
SET
(csr_a.modified_by) = (SELECT csr_mb_a.modified_by
FROM
certificate_signing_request_AUD AS csr_a
INNER JOIN CertificateSigningRequestEntity_modifiedBy_AUD AS csr_mb_a
ON csr_a.rev = csr_mb_a.rev
WHERE
csr_mb_a.revtype = 0)
</sql>
</changeSet>
<changeSet author="R3.Corda" id="refactor_modified_by_drops">
<dropForeignKeyConstraint baseTableName="CertificateSigningRequestEntity_modifiedBy" constraintName="FKLFW2KLKDPLYDROVIBVEOMF9PU"/>
<dropIndex indexName="FKLFW2KLKDPLYDROVIBVEOMF9PU_INDEX_C"
tableName="CertificateSigningRequestEntity_modifiedBy"/>
<dropTable cascadeConstraints="true"
tableName="CertificateSigningRequestEntity_modifiedBy"/>
<dropTable cascadeConstraints="true"
tableName="CertificateSigningRequestEntity_modifiedBy_AUD"/>
</changeSet>
</databaseChangeLog>

View File

@ -24,7 +24,7 @@ abstract class TestBase {
remark: String = "Test remark",
request: PKCS10CertificationRequest = mock(),
certData: CertificateData = mock(),
modifiedBy: List<String> = emptyList()
modifiedBy: String = "Test"
): CertificateSigningRequest {
return CertificateSigningRequest(
requestId = requestId,

View File

@ -108,7 +108,7 @@ class PersistentCertificateRequestStorageTest : TestBase() {
storage.putCertificatePath(
requestId,
generateSignedCertPath(csr, nodeKeyPair),
listOf(DOORMAN_SIGNATURE)
DOORMAN_SIGNATURE
)
// Check request is ready
assertNotNull(storage.getRequest(requestId)!!.certData)
@ -126,14 +126,14 @@ class PersistentCertificateRequestStorageTest : TestBase() {
storage.putCertificatePath(
requestId,
generateSignedCertPath(csr, nodeKeyPair),
listOf(DOORMAN_SIGNATURE)
DOORMAN_SIGNATURE
)
// When subsequent signature requested
assertFailsWith(IllegalArgumentException::class) {
storage.putCertificatePath(
requestId,
generateSignedCertPath(csr, nodeKeyPair),
listOf(DOORMAN_SIGNATURE))
DOORMAN_SIGNATURE)
}
}
@ -149,7 +149,7 @@ class PersistentCertificateRequestStorageTest : TestBase() {
storage.putCertificatePath(
requestId,
generateSignedCertPath(csr, nodeKeyPair),
listOf(DOORMAN_SIGNATURE)
DOORMAN_SIGNATURE
)
// Sign certificate
// When request with the same public key is requested
@ -202,7 +202,7 @@ class PersistentCertificateRequestStorageTest : TestBase() {
storage.putCertificatePath(
requestId,
generateSignedCertPath(csr, nodeKeyPair),
listOf(DOORMAN_SIGNATURE)
DOORMAN_SIGNATURE
)
val rejectedRequestId = storage.saveRequest(createRequest("BankA", certRole = CertRole.NODE_CA).first)
assertThat(storage.getRequest(rejectedRequestId)!!.remark).containsIgnoringCase("duplicate")
@ -234,15 +234,15 @@ class PersistentCertificateRequestStorageTest : TestBase() {
val auditReader = AuditReaderFactory.get(persistence.entityManagerFactory.createEntityManager())
val newRevision = auditReader.find(CertificateSigningRequestEntity::class.java, requestId, 1)
assertEquals(RequestStatus.NEW, newRevision.status)
assertTrue(newRevision.modifiedBy.isEmpty())
assertEquals(DOORMAN_SIGNATURE, newRevision.modifiedBy)
val ticketCreatedRevision = auditReader.find(CertificateSigningRequestEntity::class.java, requestId, 2)
assertEquals(RequestStatus.TICKET_CREATED, ticketCreatedRevision.status)
assertTrue(ticketCreatedRevision.modifiedBy.isEmpty())
assertEquals(DOORMAN_SIGNATURE, ticketCreatedRevision.modifiedBy)
val approvedRevision = auditReader.find(CertificateSigningRequestEntity::class.java, requestId, 3)
assertEquals(RequestStatus.APPROVED, approvedRevision.status)
assertEquals(approver, approvedRevision.modifiedBy.first())
assertEquals(approver, approvedRevision.modifiedBy)
}
}

View File

@ -72,7 +72,7 @@ class PersistentNodeInfoStorageTest : TestBase() {
requestStorage.putCertificatePath(
requestId,
X509Utilities.buildCertPath(nodeCaCert, intermediateCa.certificate, rootCaCert),
listOf(CertificationRequestStorage.DOORMAN_SIGNATURE))
CertificationRequestStorage.DOORMAN_SIGNATURE)
val storedCertPath = nodeInfoStorage.getCertificatePath(SecureHash.parse(keyPair.public.hashString()))
assertNotNull(storedCertPath)
@ -139,7 +139,7 @@ internal fun createValidSignedNodeInfo(organisation: String,
storage.approveRequest(requestId, "TestUser")
val nodeInfoBuilder = TestNodeInfoBuilder()
val (identity, key) = nodeInfoBuilder.addIdentity(CordaX500Name.build(X500Principal(csr.subject.encoded)), nodeKeyPair)
storage.putCertificatePath(requestId, identity.certPath, listOf("Test"))
storage.putCertificatePath(requestId, identity.certPath, "Test")
val (_, signedNodeInfo) = nodeInfoBuilder.buildWithSigned(1)
return Pair(NodeInfoWithSigned(signedNodeInfo), key)
}

View File

@ -29,7 +29,7 @@ class DefaultCsrHandlerTest : TestBase() {
val requestStorage: CertificationRequestStorage = mock {
on { getRequest("New") }.thenReturn(certificateSigningRequest())
on { getRequest("Signed") }.thenReturn(certificateSigningRequest(
status = RequestStatus.SIGNED,
status = RequestStatus.DONE,
certData = certificateData(CertificateStatus.VALID, X509Utilities.buildCertPath(cert))
))
on { getRequest("Rejected") }.thenReturn(certificateSigningRequest(status = RequestStatus.REJECTED, remark = "Random reason"))
@ -71,8 +71,8 @@ class DefaultCsrHandlerTest : TestBase() {
// Verify only the approved requests are taken
verify(requestStorage, times(1)).getRequests(RequestStatus.APPROVED)
verify(requestStorage, times(1)).putCertificatePath(eq("1"), certPathCapture.capture(), eq(listOf(DOORMAN_SIGNATURE)))
verify(requestStorage, times(1)).putCertificatePath(eq("2"), certPathCapture.capture(), eq(listOf(DOORMAN_SIGNATURE)))
verify(requestStorage, times(1)).putCertificatePath(eq("1"), certPathCapture.capture(), eq(DOORMAN_SIGNATURE))
verify(requestStorage, times(1)).putCertificatePath(eq("2"), certPathCapture.capture(), eq(DOORMAN_SIGNATURE))
// Then make sure the generated node cert paths are correct
certPathCapture.allValues.forEachIndexed { index, certPath ->
@ -113,7 +113,7 @@ class DefaultCsrHandlerTest : TestBase() {
// Verify only the approved requests are taken
verify(requestStorage, times(1)).getRequests(RequestStatus.APPROVED)
verify(requestStorage, times(1)).putCertificatePath(eq("1"), certPathCapture.capture(), eq(listOf(DOORMAN_SIGNATURE)))
verify(requestStorage, times(1)).putCertificatePath(eq("1"), certPathCapture.capture(), eq(DOORMAN_SIGNATURE))
// Then make sure the generated node cert paths are correct
certPathCapture.allValues.forEachIndexed { index, certPath ->

View File

@ -95,8 +95,8 @@ class JiraCsrHandlerTest : TestBase() {
fun `sync tickets status`() {
val id1 = SecureHash.randomSHA256().toString()
val id2 = SecureHash.randomSHA256().toString()
val csr1 = CertificateSigningRequest(id1, "name1", SecureHash.randomSHA256(), RequestStatus.NEW, pkcS10CertificationRequest, null, emptyList(), null)
val csr2 = CertificateSigningRequest(id2, "name2", SecureHash.randomSHA256(), RequestStatus.NEW, pkcS10CertificationRequest, null, emptyList(), null)
val csr1 = CertificateSigningRequest(id1, "name1", SecureHash.randomSHA256(), RequestStatus.NEW, pkcS10CertificationRequest, null, "Test", null)
val csr2 = CertificateSigningRequest(id2, "name2", SecureHash.randomSHA256(), RequestStatus.NEW, pkcS10CertificationRequest, null, "Test", null)
val requests = mutableMapOf(id1 to csr1, id2 to csr2)
@ -106,13 +106,13 @@ class JiraCsrHandlerTest : TestBase() {
whenever(certificationRequestStorage.approveRequest(any(), any())).then {
val id = it.getArgument<String>(0)
if (requests[id]?.status == RequestStatus.NEW) {
requests[id] = requests[id]!!.copy(status = RequestStatus.APPROVED, modifiedBy = listOf(it.getArgument(1)))
requests[id] = requests[id]!!.copy(status = RequestStatus.APPROVED, modifiedBy = it.getArgument(1))
}
null
}
whenever(certificationRequestStorage.rejectRequest(any(), any(), any())).then {
val id = it.getArgument<String>(0)
requests[id] = requests[id]!!.copy(status = RequestStatus.REJECTED, modifiedBy = listOf(it.getArgument(1)), remark = it.getArgument(2))
requests[id] = requests[id]!!.copy(status = RequestStatus.REJECTED, modifiedBy = it.getArgument(1), remark = it.getArgument(2))
null
}
@ -140,7 +140,7 @@ class JiraCsrHandlerTest : TestBase() {
// Sign request 1
val certPath = mock<CertPath>()
val certData = CertificateData(CertificateStatus.VALID, certPath)
requests[id1] = requests[id1]!!.copy(status = RequestStatus.SIGNED, certData = certData)
requests[id1] = requests[id1]!!.copy(status = RequestStatus.DONE, certData = certData)
// Process request again.
jiraCsrHandler.processRequests()