CORE-11214 - Prevent null or blank notary protocol (#4727)

Reject a registration as INVALID, when the notary protocol is either missing or is a blank string.
corda · Sep 29, 2023 · 9e924e4 · 9e924e4
1 parent eacc9ce
commit 9e924e4
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 22 deletions.
diff --git a/...ip/registration-impl/src/main/kotlin/net/corda/membership/impl/registration/MemberRole.kt b/...ip/registration-impl/src/main/kotlin/net/corda/membership/impl/registration/MemberRole.kt
@@ -48,10 +48,17 @@ internal sealed class MemberRole {
             }
         }
 
+        @Suppress("ThrowsCount")
         private fun readNotary(context: Map<String, String>): Notary {
             val serviceName = context[NOTARY_SERVICE_NAME]
             if(serviceName.isNullOrEmpty()) throw IllegalArgumentException("Notary must have a non-empty service name.")
             val protocol = context[NOTARY_SERVICE_PROTOCOL]
+            if (protocol == null) {
+                throw IllegalArgumentException("No value provided for $NOTARY_SERVICE_PROTOCOL, which is required for a notary.")
+            }
+            if (protocol.isBlank()) {
+                throw IllegalArgumentException("Value provided for $NOTARY_SERVICE_PROTOCOL was a blank string." )
+            }
             val protocolVersions = NOTARY_SERVICE_PROTOCOL_VERSIONS.format("([0-9]+)").toRegex().let { regex ->
                 context.filter { it.key.matches(regex) }.mapTo(mutableSetOf()) { it.value.toInt() }
             }
@@ -70,7 +77,7 @@ internal sealed class MemberRole {
 
     data class Notary(
         val serviceName: MemberX500Name,
-        val protocol: String?,
+        val protocol: String,
         val protocolVersions: Collection<Int>,
     ) : MemberRole() {
         override fun toMemberInfo(
@@ -92,13 +99,8 @@ internal sealed class MemberRole {
             return keys + versions + listOf(
                 "$ROLES_PREFIX.$index" to NOTARY_ROLE,
                 NOTARY_SERVICE_NAME to serviceName.toString(),
-            ) + if (protocol == null) {
-                emptyList()
-            } else {
-                listOf(
-                    NOTARY_SERVICE_PROTOCOL to protocol,
-                )
-            }
+                NOTARY_SERVICE_PROTOCOL to protocol
+            )
         }
     }
 }
diff --git a/...egistration-impl/src/test/kotlin/net/corda/membership/impl/registration/MemberRoleTest.kt b/...egistration-impl/src/test/kotlin/net/corda/membership/impl/registration/MemberRoleTest.kt
@@ -67,22 +67,28 @@ class MemberRoleTest {
     }
 
     @Test
-    fun `accept context when notary protocol is missing`() {
-        val roles = extractRolesFromContext(
-            mapOf(
-                "${ROLES_PREFIX}.0" to "notary",
-                NOTARY_SERVICE_NAME to "O=MyNotaryService, L=London, C=GB",
+    fun `throws exception if notary protocol is missing`() {
+        assertThrows<IllegalArgumentException> {
+            extractRolesFromContext(
+                mapOf(
+                    "${ROLES_PREFIX}.0" to "notary",
+                    NOTARY_SERVICE_NAME to "O=MyNotaryService, L=London, C=GB",
+                )
             )
-        )
+        }
+    }
 
-        assertThat(roles.toList())
-            .hasSize(1)
-            .allSatisfy {
-                it is MemberRole.Notary
-            }
-            .allSatisfy {
-                assertThat((it as? MemberRole.Notary)?.protocol).isNull()
-            }
+    @Test
+    fun `throws exception if notary protocol is blank string`() {
+        assertThrows<IllegalArgumentException> {
+            extractRolesFromContext(
+                mapOf(
+                    "${ROLES_PREFIX}.0" to "notary",
+                    NOTARY_SERVICE_NAME to "O=MyNotaryService, L=London, C=GB",
+                    NOTARY_SERVICE_PROTOCOL to "  ",
+                )
+            )
+        }
     }
 
     @Test
@@ -91,6 +97,7 @@ class MemberRoleTest {
             mapOf(
                 "${ROLES_PREFIX}.0" to "notary",
                 NOTARY_SERVICE_NAME to "O=MyNotaryService, L=London, C=GB",
+                NOTARY_SERVICE_PROTOCOL to "net.corda.notary.MyNotaryService"
             )
         )
 

diff --git a/...corda/membership/impl/registration/dynamic/member/DynamicMemberRegistrationServiceTest.kt b/...corda/membership/impl/registration/dynamic/member/DynamicMemberRegistrationServiceTest.kt
@@ -60,6 +60,7 @@ import net.corda.membership.lib.MemberInfoExtension.Companion.NOTARY_KEY_HASH
 import net.corda.membership.lib.MemberInfoExtension.Companion.NOTARY_KEY_PEM
 import net.corda.membership.lib.MemberInfoExtension.Companion.NOTARY_KEY_SPEC
 import net.corda.membership.lib.MemberInfoExtension.Companion.NOTARY_SERVICE_NAME
+import net.corda.membership.lib.MemberInfoExtension.Companion.NOTARY_SERVICE_PROTOCOL
 import net.corda.membership.lib.MemberInfoExtension.Companion.NOTARY_SERVICE_PROTOCOL_VERSIONS
 import net.corda.membership.lib.MemberInfoExtension.Companion.PARTY_NAME
 import net.corda.membership.lib.MemberInfoExtension.Companion.PARTY_SESSION_KEYS_ID
@@ -1178,6 +1179,7 @@ class DynamicMemberRegistrationServiceTest {
             val newContext = mock<MemberContext> {
                 on { entries } doReturn context.entries + mapOf(
                     String.format(ROLES_PREFIX, 0) to "notary",
+                    NOTARY_SERVICE_PROTOCOL to "net.corda.notary.MyNotaryService",
                     NOTARY_SERVICE_NAME to "O=ChangedNotaryService, L=London, C=GB",
                     NOTARY_KEY_ID_KEY to NOTARY_KEY_ID,
                 ).entries
@@ -1289,6 +1291,7 @@ class DynamicMemberRegistrationServiceTest {
             }
             val newContextEntries = context.toMutableMap().apply {
                 put(String.format(ROLES_PREFIX, 0), "notary")
+                put(NOTARY_SERVICE_PROTOCOL, "net.corda.notary.MyNotaryService")
                 put(NOTARY_SERVICE_NAME, "O=MyNotaryService, L=London, C=GB")
                 put(NOTARY_KEY_ID_KEY, NOTARY_KEY_ID)
             }.entries
@@ -1449,6 +1452,7 @@ class DynamicMemberRegistrationServiceTest {
             val testProperties =
                 context + mapOf(
                     String.format(ROLES_PREFIX, 0) to "notary",
+                    NOTARY_SERVICE_PROTOCOL to "net.corda.notary.MyNotaryService",
                     NOTARY_SERVICE_NAME to "O=MyNotaryService, L=London, C=GB",
                     NOTARY_KEY_ID_KEY to NOTARY_KEY_ID,
                 )
@@ -1480,6 +1484,7 @@ class DynamicMemberRegistrationServiceTest {
             val testProperties =
                 context + mapOf(
                     String.format(ROLES_PREFIX, 0) to "notary",
+                    NOTARY_SERVICE_PROTOCOL to "net.corda.notary.MyNotaryService",
                     NOTARY_SERVICE_NAME to "O=MyNotaryService, L=London, C=GB",
                     NOTARY_KEY_ID_KEY to NOTARY_KEY_ID,
                 )
@@ -1538,6 +1543,7 @@ class DynamicMemberRegistrationServiceTest {
             val testProperties =
                 context + mapOf(
                     String.format(ROLES_PREFIX, 0) to "notary",
+                    NOTARY_SERVICE_PROTOCOL to "net.corda.notary.MyNotaryService",
                     NOTARY_SERVICE_NAME to "O=MyNotaryService, L=London, C=GB",
                     NOTARY_KEY_ID_KEY to NOTARY_KEY_ID,
                 )
@@ -1583,6 +1589,7 @@ class DynamicMemberRegistrationServiceTest {
             val testProperties =
                 context.filterNot { it.key.startsWith("corda.ledger") } + mapOf(
                     String.format(ROLES_PREFIX, 0) to "notary",
+                    NOTARY_SERVICE_PROTOCOL to "net.corda.notary.MyNotaryService",
                     NOTARY_SERVICE_NAME to "O=MyNotaryService, L=London, C=GB",
                     NOTARY_KEY_ID_KEY to NOTARY_KEY_ID,
                 )