Skip to content

Commit

Permalink
CORE-12411 - Add declination reason (#4696)
Browse files Browse the repository at this point in the history
This change adjusts the MGM code, so that it communicates the reason for declined requests back to the user and the appropriate handler, so that this information is stored in the DB and surfaced to the user. In some cases, we don't want to reveal potentially sensitive information to a member (especially in cases where the request hasn't gone fully through authentication yet). In these cases, a general message is communicated instructing the user to communicate with the network operator for more details.
  • Loading branch information
dimosr authored Sep 27, 2023
1 parent f41bab1 commit bacee09
Show file tree
Hide file tree
Showing 23 changed files with 172 additions and 103 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import net.corda.membership.lib.MemberInfoExtension.Companion.isMgm
import net.corda.membership.lib.MemberInfoFactory
import net.corda.membership.lib.approval.ApprovalRuleParams
import net.corda.membership.lib.deserializeContext
import net.corda.membership.lib.registration.DECLINED_REASON_FOR_USER_GENERAL_MANUAL_DECLINED
import net.corda.membership.lib.toPersistentGroupParameters
import net.corda.membership.persistence.client.MembershipPersistenceClient
import net.corda.membership.persistence.client.MembershipQueryClient
Expand Down Expand Up @@ -597,7 +598,8 @@ class MGMResourceClientImpl @Activate constructor(
if (approve) {
publishRegistrationCommand(ApproveRegistration(), memberName, mgm.groupId)
} else {
publishRegistrationCommand(DeclineRegistration(reason ?: ""), memberName, mgm.groupId)
publishRegistrationCommand(DeclineRegistration(reason ?: "",
DECLINED_REASON_FOR_USER_GENERAL_MANUAL_DECLINED), memberName, mgm.groupId)
}
}

Expand All @@ -615,7 +617,7 @@ class MGMResourceClientImpl @Activate constructor(
"declined. Refer to the docs on Member Suspension to suspend approved members." }

publishRegistrationCommand(
DeclineRegistration(FORCE_DECLINE_MESSAGE),
DeclineRegistration(FORCE_DECLINE_MESSAGE, DECLINED_REASON_FOR_USER_GENERAL_MANUAL_DECLINED),
findMemberName(requestStatus.memberProvidedContext),
mgm.groupId
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import net.corda.membership.lib.approval.ApprovalRuleParams
import net.corda.membership.lib.impl.MemberInfoFactoryImpl
import net.corda.membership.lib.impl.converter.EndpointInfoConverter
import net.corda.membership.lib.impl.converter.MemberNotaryDetailsConverter
import net.corda.membership.lib.registration.DECLINED_REASON_FOR_USER_GENERAL_MANUAL_DECLINED
import net.corda.membership.persistence.client.MembershipPersistenceClient
import net.corda.membership.persistence.client.MembershipPersistenceOperation
import net.corda.membership.persistence.client.MembershipPersistenceResult
Expand Down Expand Up @@ -912,7 +913,7 @@ class MGMResourceClientTest {
Record(
Schemas.Membership.REGISTRATION_COMMAND_TOPIC,
"$memberName-$DEFAULT_MEMBER_GROUP_ID",
RegistrationCommand(DeclineRegistration(reason))
RegistrationCommand(DeclineRegistration(reason, DECLINED_REASON_FOR_USER_GENERAL_MANUAL_DECLINED))
)
)
)
Expand Down Expand Up @@ -1086,7 +1087,7 @@ class MGMResourceClientTest {
Record(
Schemas.Membership.REGISTRATION_COMMAND_TOPIC,
"$memberName-$DEFAULT_MEMBER_GROUP_ID",
RegistrationCommand(DeclineRegistration("Force declined by MGM"))
RegistrationCommand(DeclineRegistration("Force declined by MGM", DECLINED_REASON_FOR_USER_GENERAL_MANUAL_DECLINED))
)
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package net.corda.membership.impl.p2p
import net.corda.data.membership.command.registration.RegistrationCommand
import net.corda.data.membership.command.registration.mgm.DeclineRegistration
import net.corda.data.p2p.markers.AppMessageMarker
import net.corda.membership.lib.registration.DECLINED_REASON_COMMS_ISSUE
import net.corda.membership.p2p.helpers.TtlIdsFactory
import net.corda.messaging.api.processor.DurableProcessor
import net.corda.messaging.api.records.Record
Expand All @@ -26,7 +27,7 @@ internal class MembershipP2PMarkersProcessor(
Schemas.Membership.REGISTRATION_COMMAND_TOPIC,
key,
RegistrationCommand(
DeclineRegistration("Could not send message to member.")
DeclineRegistration(DECLINED_REASON_COMMS_ISSUE, DECLINED_REASON_COMMS_ISSUE)
)
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,6 @@ internal class SetOwnRegistrationStatusHandler(
RegistrationStatus.FAILED -> RegistrationStatusV2.FAILED
else -> throw IllegalArgumentException("Unknown status '${newStatus.name}' received.")
}
return SetOwnRegistrationStatusV2(registrationId, status)
return SetOwnRegistrationStatusV2(registrationId, status, null)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ class SetOwnRegistrationStatusHandlerTest {
RegistrationStatus.DECLINED
)
private val payloadV2 = ByteBuffer.wrap(byteArrayOf(4, 5, 6))
private val reason = "some reason"
private val statusV2 = SetOwnRegistrationStatusV2(
"id",
RegistrationStatusV2.DECLINED
RegistrationStatusV2.DECLINED,
reason
)
private val avroSchemaRegistry: AvroSchemaRegistry = mock {
on { getClassType(payloadV1) } doReturn SetOwnRegistrationStatus::class.java
Expand All @@ -44,6 +46,11 @@ class SetOwnRegistrationStatusHandlerTest {
@Test
fun `invokeAuthenticatedMessage returns PersistMemberRegistrationState command - V1 version converted to V2 successfully`() {
val record = handler.invokeAuthenticatedMessage(header, payloadV1)
val statusV2WithoutReason = SetOwnRegistrationStatusV2(
"id",
RegistrationStatusV2.DECLINED,
null
)

assertSoftly { softly ->
softly.assertThat(record.topic).isEqualTo(REGISTRATION_COMMAND_TOPIC)
Expand All @@ -52,7 +59,7 @@ class SetOwnRegistrationStatusHandlerTest {
RegistrationCommand(
PersistMemberRegistrationState(
identity,
statusV2
statusV2WithoutReason
)
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ internal class PersistMemberRegistrationStateHandler(
member,
command.setStatusRequest.registrationId,
command.setStatusRequest.newStatus,
command.setStatusRequest.reason
).createAsyncCommands()
return RegistrationHandlerResult(
null,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import net.corda.membership.lib.MemberInfoExtension.Companion.notaryDetails
import net.corda.membership.lib.MemberInfoFactory
import net.corda.membership.lib.exceptions.MembershipPersistenceException
import net.corda.membership.lib.VersionedMessageBuilder.retrieveRegistrationStatusMessage
import net.corda.membership.lib.registration.DECLINED_REASON_FOR_USER_INTERNAL_ERROR
import net.corda.membership.p2p.helpers.P2pRecordsFactory
import net.corda.membership.persistence.client.MembershipPersistenceClient
import net.corda.membership.persistence.client.MembershipPersistenceResult
Expand Down Expand Up @@ -130,7 +131,8 @@ internal class ApproveRegistrationHandler(
val statusUpdateMessage = retrieveRegistrationStatusMessage(
memberInfo.platformVersion,
registrationId,
RegistrationStatus.APPROVED.name
RegistrationStatus.APPROVED.name,
null
)
val persistApproveMessage = if (statusUpdateMessage != null) {
p2pRecordsFactory.createAuthenticatedMessageRecord(
Expand All @@ -152,7 +154,9 @@ internal class ApproveRegistrationHandler(
logger.warn("Could not approve registration request: '$registrationId'", e)
return RegistrationHandlerResult(
state,
listOf(Record(REGISTRATION_COMMAND_TOPIC, key, RegistrationCommand(DeclineRegistration(e.message))))
listOf(Record(REGISTRATION_COMMAND_TOPIC, key,
RegistrationCommand(DeclineRegistration(e.message, DECLINED_REASON_FOR_USER_INTERNAL_ERROR)))
)
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ internal class DeclineRegistrationHandler(
pendingMemberInfo.platformVersion,
registrationId,
RegistrationStatus.DECLINED.name,
command.reasonForUser
)
if (statusUpdateMessage != null) {
p2pRecordsFactory.createAuthenticatedMessageRecord(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import net.corda.membership.lib.approval.RegistrationRulesEngine
import net.corda.membership.lib.registration.PRE_AUTH_TOKEN
import net.corda.membership.lib.VersionedMessageBuilder.retrieveRegistrationStatusMessage
import net.corda.membership.lib.deserializeContext
import net.corda.membership.lib.registration.DECLINED_REASON_FOR_USER_INTERNAL_ERROR
import net.corda.membership.lib.toMap
import net.corda.membership.p2p.helpers.P2pRecordsFactory
import net.corda.membership.p2p.helpers.P2pRecordsFactory.Companion.getTtlMinutes
Expand Down Expand Up @@ -112,7 +113,7 @@ internal class ProcessMemberVerificationResponseHandler(
status
).createAsyncCommands()
val statusUpdateMessage = retrieveRegistrationStatusMessage(
pendingInfo.platformVersion, registrationId, status.name
pendingInfo.platformVersion, registrationId, status.name, null
)
val persistStatusMessage = if (statusUpdateMessage != null) {
p2pRecordsFactory.createAuthenticatedMessageRecord(
Expand Down Expand Up @@ -142,7 +143,7 @@ internal class ProcessMemberVerificationResponseHandler(
REGISTRATION_COMMAND_TOPIC,
key,
RegistrationCommand(
DeclineRegistration(e.message)
DeclineRegistration(e.message, DECLINED_REASON_FOR_USER_INTERNAL_ERROR)
)
),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ internal class QueueRegistrationHandler(
platformVersion,
command.memberRegistrationRequest.registrationId,
RegistrationStatus.RECEIVED_BY_MGM.name,
null
)
// if we are unable to create the status message, then we won't send anything
val statusUpdateRecord = statusUpdateMessage?.let {
Expand Down
Loading

0 comments on commit bacee09

Please sign in to comment.