Repository: hyperledger/firefly
Pull request #1074 (2022-09-22 23:21:32)

V1.1.x/mainline fix: Allow update of node using parent org identity
See #1073 


Patch:
diff --git a/internal/definitions/handler_identity_claim.go b/internal/definitions/handler_identity_claim.go
index 95ca37bb4..4155a9e11 100644
--- a/internal/definitions/handler_identity_claim.go
+++ b/internal/definitions/handler_identity_claim.go
@@ -57,20 +57,23 @@ func (dh *definitionHandler) handleIdentityClaimBroadcast(ctx context.Context, s
 	return dh.handleIdentityClaim(ctx, state, buildIdentityMsgInfo(msg, verifyMsgID), &claim)
 }
 
+func (dh *definitionHandler) getExpectedSigner(identity *core.Identity, parent *core.Identity) *core.Identity {
+	switch {
+	case identity.Type == core.IdentityTypeNode && parent != nil:
+		// In the special case of a node, the parent signs it directly
+		return parent
+	default:
+		return identity
+	}
+}
+
 func (dh *definitionHandler) verifyClaimSignature(ctx context.Context, msg *identityMsgInfo, identity *core.Identity, parent *core.Identity) error {
 	author := msg.Author
 	if author == "" {
 		return i18n.NewError(ctx, coremsgs.MsgDefRejectedAuthorBlank, "identity claim", msg.claimMsg.ID)
 	}
 
-	var expectedSigner *core.Identity
-	switch {
-	case identity.Type == core.IdentityTypeNode:
-		// In the special case of a node, the parent signs it directly
-		expectedSigner = parent
-	default:
-		expectedSigner = identity
-	}
+	expectedSigner := dh.getExpectedSigner(identity, parent)
 
 	valid := author == expectedSigner.DID ||
 		(expectedSigner.Type == core.IdentityTypeOrg && author == fmt.Sprintf("%s%s", core.FireFlyOrgDIDPrefix, expectedSigner.ID))
diff --git a/internal/definitions/handler_identity_update.go b/internal/definitions/handler_identity_update.go
index d6d9f32eb..511cae7ed 100644
--- a/internal/definitions/handler_identity_update.go
+++ b/internal/definitions/handler_identity_update.go
@@ -21,6 +21,7 @@ import (
 
 	"github.com/hyperledger/firefly-common/pkg/fftypes"
 	"github.com/hyperledger/firefly-common/pkg/i18n"
+	"github.com/hyperledger/firefly-common/pkg/log"
 	"github.com/hyperledger/firefly/internal/coremsgs"
 	"github.com/hyperledger/firefly/pkg/core"
 	"github.com/hyperledger/firefly/pkg/database"
@@ -56,9 +57,22 @@ func (dh *definitionHandler) handleIdentityUpdate(ctx context.Context, state *co
 		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedIdentityNotFound, "identity update", update.Identity.ID, update.Identity.ID)
 	}
 
-	// Check the author matches
-	if dh.multiparty && identity.DID != msg.Author {
-		return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedWrongAuthor, "identity update", update.Identity.ID, msg.Author)
+	if dh.multiparty {
+
+		parent, retryable, err := dh.identity.VerifyIdentityChain(ctx, identity)
+		if err != nil && retryable {
+			return HandlerResult{Action: ActionRetry}, err
+		} else if err != nil {
+			log.L(ctx).Infof("Unable to process identity update (parked) %s: %s", msg.ID, err)
+			return HandlerResult{Action: ActionWait}, nil
+		}
+
+		// Check the author matches
+		expectedSigner := dh.getExpectedSigner(identity, parent)
+		if expectedSigner.DID != msg.Author {
+			return HandlerResult{Action: ActionReject}, i18n.NewError(ctx, coremsgs.MsgDefRejectedWrongAuthor, "identity update", update.Identity.ID, msg.Author)
+		}
+
 	}
 
 	// Update the profile


Test patch:
diff --git a/internal/definitions/handler_identity_update_test.go b/internal/definitions/handler_identity_update_test.go
index ccabbe31f..3720d4b90 100644
--- a/internal/definitions/handler_identity_update_test.go
+++ b/internal/definitions/handler_identity_update_test.go
@@ -129,6 +129,7 @@ func TestHandleDefinitionIdentityInvalidIdentity(t *testing.T) {
 
 	mim := dh.identity.(*identitymanagermocks.Manager)
 	mim.On("CachedIdentityLookupByID", ctx, org1.ID).Return(org1, nil)
+	mim.On("VerifyIdentityChain", ctx, mock.Anything).Return(nil, false, nil)
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, &bs.BatchState, updateMsg, core.DataArray{updateData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
@@ -138,6 +139,46 @@ func TestHandleDefinitionIdentityInvalidIdentity(t *testing.T) {
 	bs.assertNoFinalizers()
 }
 
+func TestHandleDefinitionVerifyFail(t *testing.T) {
+	dh, bs := newTestDefinitionHandler(t)
+	ctx := context.Background()
+	dh.multiparty = true
+
+	org1, updateMsg, updateData, _ := testIdentityUpdate(t)
+	updateMsg.Header.Author = "wrong"
+
+	mim := dh.identity.(*identitymanagermocks.Manager)
+	mim.On("CachedIdentityLookupByID", ctx, org1.ID).Return(org1, nil)
+	mim.On("VerifyIdentityChain", ctx, mock.Anything).Return(nil, true, fmt.Errorf("pop"))
+
+	action, err := dh.HandleDefinitionBroadcast(ctx, &bs.BatchState, updateMsg, core.DataArray{updateData}, fftypes.NewUUID())
+	assert.Equal(t, HandlerResult{Action: ActionRetry}, action)
+	assert.Error(t, err)
+
+	mim.AssertExpectations(t)
+	bs.assertNoFinalizers()
+}
+
+func TestHandleDefinitionVerifyWait(t *testing.T) {
+	dh, bs := newTestDefinitionHandler(t)
+	ctx := context.Background()
+	dh.multiparty = true
+
+	org1, updateMsg, updateData, _ := testIdentityUpdate(t)
+	updateMsg.Header.Author = "wrong"
+
+	mim := dh.identity.(*identitymanagermocks.Manager)
+	mim.On("CachedIdentityLookupByID", ctx, org1.ID).Return(org1, nil)
+	mim.On("VerifyIdentityChain", ctx, mock.Anything).Return(nil, false, fmt.Errorf("pop"))
+
+	action, err := dh.HandleDefinitionBroadcast(ctx, &bs.BatchState, updateMsg, core.DataArray{updateData}, fftypes.NewUUID())
+	assert.Equal(t, HandlerResult{Action: ActionWait}, action)
+	assert.NoError(t, err)
+
+	mim.AssertExpectations(t)
+	bs.assertNoFinalizers()
+}
+
 func TestHandleDefinitionIdentityNotFound(t *testing.T) {
 	dh, bs := newTestDefinitionHandler(t)
 	ctx := context.Background()
@@ -149,7 +190,7 @@ func TestHandleDefinitionIdentityNotFound(t *testing.T) {
 
 	action, err := dh.HandleDefinitionBroadcast(ctx, &bs.BatchState, updateMsg, core.DataArray{updateData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
-	assert.Error(t, err)
+	assert.Regexp(t, "FF10408", err)
 
 	mim.AssertExpectations(t)
 	bs.assertNoFinalizers()