Merge pull request #1073 from kaleido-io/node-patch

V1.0.x fix: Allow update of node using parent org identity

Author: nguyer

internal/definitions/definition_handler_identity_claim.go

@@ -36,21 +36,24 @@ func (dh *definitionHandlers) handleIdentityClaimBroadcast(ctx context.Context,
 
 }
 
+func (dh *definitionHandlers) getExpectedSigner(identity *fftypes.Identity, parent *fftypes.Identity) *fftypes.Identity {
+	switch {
+	case identity.Type == fftypes.IdentityTypeNode && parent != nil:
+		// In the special case of a node, the parent signs it directly
+		return parent
+	default:
+		return identity
+	}
+}
+
 func (dh *definitionHandlers) verifyClaimSignature(ctx context.Context, msg *fftypes.Message, identity *fftypes.Identity, parent *fftypes.Identity) (valid bool) {
 
 	author := msg.Header.Author
 	if author == "" {
 		return false
 	}
 
-	var expectedSigner *fftypes.Identity
-	switch {
-	case identity.Type == fftypes.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 == fftypes.IdentityTypeOrg && author == fmt.Sprintf("%s%s", fftypes.FireFlyOrgDIDPrefix, expectedSigner.ID))

internal/definitions/definition_handler_identity_update.go

@@ -48,8 +48,17 @@ func (dh *definitionHandlers) handleIdentityUpdateBroadcast(ctx context.Context,
 		return HandlerResult{Action: ActionReject}, nil
 	}
 
+	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.Header.ID, err)
+		return HandlerResult{Action: ActionWait}, nil
+	}
+
 	// Check the author matches
-	if identity.DID != msg.Header.Author {
+	expectedSigner := dh.getExpectedSigner(identity, parent)
+	if expectedSigner.DID != msg.Header.Author {
 		log.L(ctx).Warnf("Invalid identity update message %s - wrong author: %s", msg.Header.ID, msg.Header.Author)
 		return HandlerResult{Action: ActionReject}, nil
 	}

internal/definitions/definition_handler_identity_update_test.go

@@ -74,6 +74,7 @@ func TestHandleDefinitionIdentityUpdateOk(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)
 
 	mdi := dh.database.(*databasemocks.Plugin)
 	mdi.On("UpsertIdentity", ctx, mock.MatchedBy(func(identity *fftypes.Identity) bool {
@@ -105,6 +106,7 @@ func TestHandleDefinitionIdentityUpdateUpsertFail(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)
 
 	mdi := dh.database.(*databasemocks.Plugin)
 	mdi.On("UpsertIdentity", ctx, mock.Anything, database.UpsertOptimizationExisting).Return(fmt.Errorf("pop"))
@@ -127,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, updateMsg, fftypes.DataArray{updateData}, fftypes.NewUUID())
 	assert.Equal(t, HandlerResult{Action: ActionReject}, action)
@@ -136,6 +139,44 @@ func TestHandleDefinitionIdentityInvalidIdentity(t *testing.T) {
 	bs.assertNoFinalizers()
 }
 
+func TestHandleDefinitionVerifyFail(t *testing.T) {
+	dh, bs := newTestDefinitionHandlers(t)
+	ctx := context.Background()
+
+	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, updateMsg, fftypes.DataArray{updateData}, fftypes.NewUUID())
+	assert.Equal(t, HandlerResult{Action: ActionRetry}, action)
+	assert.Regexp(t, "pop", err)
+
+	mim.AssertExpectations(t)
+	bs.assertNoFinalizers()
+}
+
+func TestHandleDefinitionVerifyWait(t *testing.T) {
+	dh, bs := newTestDefinitionHandlers(t)
+	ctx := context.Background()
+
+	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, updateMsg, fftypes.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 := newTestDefinitionHandlers(t)
 	ctx := context.Background()