[CORDA-2376]: Unable to make contract attachment trusted if it was received via network (fixed). (#4474)

* [CORDA-2376]: Unable to make contract attachment trusted if it was received via network (fixed).

* [CORDA-2376]: Wrote some tests.

* [CORDA-2376]: Wrote some tests.

* [CORDA-2376]: Code review changes.

* [CORDA-2376]: Code review changes.

node/src/main/kotlin/net/corda/node/services/persistence/NodeAttachmentService.kt

@@ -342,16 +342,20 @@ class NodeAttachmentService(
                 if (isUploaderTrusted(uploader)) {
                     val session = currentDBSession()
                     val attachment = session.get(NodeAttachmentService.DBAttachment::class.java, id.toString())
-                    // update the `upLoader` field (as the existing attachment may have been resolved from a peer)
+                    // update the `uploader` field (as the existing attachment may have been resolved from a peer)
                     if (attachment.uploader != uploader) {
                         attachment.uploader = uploader
                         session.saveOrUpdate(attachment)
                         log.info("Updated attachment $id with uploader $uploader")
                         contractClassNames.forEach { contractsCache.invalidate(it) }
+                        loadAttachmentContent(id)?.let { attachmentAndContent ->
                             // TODO: this is racey. ENT-2870
-                        attachmentCache.invalidate(id)
+                            attachmentContentCache.put(id, Optional.of(attachmentAndContent))
-                        attachmentContentCache.invalidate(id)
+                            attachmentCache.put(id, Optional.of(attachmentAndContent.first))
                         }
+                        return@withContractsInJar id
+                    }
+                    // If the uploader is the same, throw the exception because the attachment cannot be overridden by the same uploader.
                 }
                 throw DuplicateAttachmentException(id.toString())
             }

node/src/test/kotlin/net/corda/node/CordaRPCOpsImplTest.kt

@@ -280,7 +280,7 @@ class CordaRPCOpsImplTest {
     }
 
     @Test
-    fun `can't upload the same attachment`() {
+    fun `cannot upload the same attachment`() {
         withPermissions(invokeRpc(CordaRPCOps::uploadAttachment), invokeRpc(CordaRPCOps::attachmentExists)) {
             val inputJar1 = Thread.currentThread().contextClassLoader.getResourceAsStream(testJar)
             val inputJar2 = Thread.currentThread().contextClassLoader.getResourceAsStream(testJar)

node/src/test/kotlin/net/corda/node/services/persistence/NodeAttachmentServiceTest.kt

@@ -17,6 +17,7 @@ import net.corda.core.node.services.vault.Builder
 import net.corda.core.node.services.vault.Sort
 import net.corda.core.utilities.getOrThrow
 import net.corda.node.services.transactions.PersistentUniquenessProvider
+import net.corda.nodeapi.exceptions.DuplicateAttachmentException
 import net.corda.nodeapi.internal.persistence.CordaPersistence
 import net.corda.nodeapi.internal.persistence.DatabaseConfig
 import net.corda.testing.core.internal.ContractJarTestUtils.makeTestContractJar
@@ -30,7 +31,7 @@ import net.corda.testing.internal.rigorousMock
 import net.corda.testing.node.MockServices.Companion.makeTestDataSourceProperties
 import net.corda.testing.node.internal.InternalMockNetwork
 import net.corda.testing.node.internal.startFlow
-import org.assertj.core.api.Assertions.assertThatIllegalArgumentException
+import org.assertj.core.api.Assertions.*
 import org.junit.After
 import org.junit.Before
 import org.junit.Ignore
@@ -121,6 +122,43 @@ class NodeAttachmentServiceTest {
         }
     }
 
+    @Test
+    fun `attachment can be overridden by trusted uploader`() {
+        SelfCleaningDir().use { file ->
+            val contractJarName = makeTestContractJar(file.path, "com.example.MyContract")
+            val attachment = file.path.resolve(contractJarName)
+            val expectedAttachmentId = attachment.readAll().sha256()
+
+            val initialUploader = "test"
+            val attachmentId = attachment.read { storage.privilegedImportAttachment(it, initialUploader, null) }
+            assertThat(attachmentId).isEqualTo(expectedAttachmentId)
+            assertThat((storage.openAttachment(expectedAttachmentId) as ContractAttachment).uploader).isEqualTo(initialUploader)
+
+            val trustedUploader = TRUSTED_UPLOADERS.randomOrNull()!!
+
+            val overriddenAttachmentId = attachment.read { storage.privilegedImportAttachment(it, trustedUploader, null) }
+            assertThat(overriddenAttachmentId).isEqualTo(expectedAttachmentId)
+            assertThat((storage.openAttachment(expectedAttachmentId) as ContractAttachment).uploader).isEqualTo(trustedUploader)
+        }
+    }
+
+    @Test
+    fun `attachment cannot be overridden by untrusted uploader`() {
+        SelfCleaningDir().use { file ->
+            val contractJarName = makeTestContractJar(file.path, "com.example.MyContract")
+            val attachment = file.path.resolve(contractJarName)
+            val expectedAttachmentId = attachment.readAll().sha256()
+
+            val trustedUploader = TRUSTED_UPLOADERS.randomOrNull()!!
+            val attachmentId = attachment.read { storage.privilegedImportAttachment(it, trustedUploader, null) }
+            assertThat(attachmentId).isEqualTo(expectedAttachmentId)
+            assertThat((storage.openAttachment(expectedAttachmentId) as ContractAttachment).uploader).isEqualTo(trustedUploader)
+
+            val untrustedUploader = "test"
+            assertThatThrownBy { attachment.read { storage.privilegedImportAttachment(it, untrustedUploader, null) } }.isInstanceOf(DuplicateAttachmentException::class.java)
+        }
+    }
+
     @Test
     fun `insert contract attachment as an untrusted uploader and then as trusted CorDapp uploader`() {
         SelfCleaningDir().use { file ->