Dedup from_entity_ids when merging two entities (#10101)

Fixes #10100
hashicorp · Oct 12, 2021 · c0d8df9 · c0d8df9
1 parent e558e58
commit c0d8df9
Show file tree

Hide file tree

Showing 3 changed files with 122 additions and 2 deletions.
diff --git a/changelog/10101.txt b/changelog/10101.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+identity: dedup from_entity_ids when merging two entities
+```
diff --git a/vault/identity_store_entities.go b/vault/identity_store_entities.go
@@ -733,8 +733,10 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit
 		return errors.New("entity id to merge into does not belong to the request's namespace"), nil
 	}
 
+	sanitizedFromEntityIDs := strutil.RemoveDuplicates(fromEntityIDs, false)
+
 	// Merge the MFA secrets
-	for _, fromEntityID := range fromEntityIDs {
+	for _, fromEntityID := range sanitizedFromEntityIDs {
 		if fromEntityID == toEntity.ID {
 			return errors.New("to_entity_id should not be present in from_entity_ids"), nil
 		}
@@ -768,7 +770,7 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit
 	isPerfSecondaryOrStandby := i.localNode.ReplicationState().HasState(consts.ReplicationPerformanceSecondary) ||
 		i.localNode.HAState() == consts.PerfStandby
 	var fromEntityGroups []*identity.Group
-	for _, fromEntityID := range fromEntityIDs {
+	for _, fromEntityID := range sanitizedFromEntityIDs {
 		if fromEntityID == toEntity.ID {
 			return errors.New("to_entity_id should not be present in from_entity_ids"), nil
 		}

diff --git a/vault/identity_store_entities_test.go b/vault/identity_store_entities_test.go
@@ -1210,3 +1210,118 @@ func TestIdentityStore_MergeEntitiesByID(t *testing.T) {
 		}
 	}
 }
+
+func TestIdentityStore_MergeEntitiesByID_DuplicateFromEntityIDs(t *testing.T) {
+	var err error
+	var resp *logical.Response
+
+	ctx := namespace.RootContext(nil)
+	is, githubAccessor, _ := testIdentityStoreWithGithubAuth(ctx, t)
+
+	// Register the entity
+	registerReq := &logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      "entity",
+		Data: map[string]interface{}{
+			"name":     "testentityname2",
+			"metadata": []string{"someusefulkey=someusefulvalue"},
+		},
+	}
+
+	resp, err = is.HandleRequest(ctx, registerReq)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+	entityID1 := resp.Data["id"].(string)
+	entity1, err := is.MemDBEntityByID(entityID1, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if entity1 == nil {
+		t.Fatalf("failed to create entity: %v", err)
+	}
+
+	// Register another entity
+	registerReq.Data = map[string]interface{}{
+		"name":     "testentityname",
+		"metadata": []string{"someusefulkey=someusefulvalue"},
+	}
+
+	resp, err = is.HandleRequest(ctx, registerReq)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+	entityID2 := resp.Data["id"].(string)
+
+	aliasReq := &logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      "alias",
+		Data: map[string]interface{}{
+			"name":           "testaliasname1",
+			"mount_accessor": githubAccessor,
+			"metadata":       []string{"organization=hashicorp", "team=vault"},
+			"entity_id":      entityID2,
+		},
+	}
+
+	// Register the alias
+	resp, err = is.HandleRequest(ctx, aliasReq)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+
+	entity2, err := is.MemDBEntityByID(entityID2, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if entity2 == nil {
+		t.Fatalf("failed to create entity: %v", err)
+	}
+	if len(entity2.Aliases) != 1 {
+		t.Fatalf("bad: number of aliases in entity; expected: 1, actual: %d", len(entity2.Aliases))
+	}
+
+	mergeReq := &logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      "entity/merge",
+		Data: map[string]interface{}{
+			"to_entity_id":    entityID1,
+			"from_entity_ids": []string{entityID2, entityID2},
+		},
+	}
+
+	resp, err = is.HandleRequest(ctx, mergeReq)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+
+	entityReq := &logical.Request{
+		Operation: logical.ReadOperation,
+		Path:      "entity/id/" + entityID2,
+	}
+	resp, err = is.HandleRequest(ctx, entityReq)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+	if resp != nil {
+		t.Fatalf("entity should have been deleted")
+	}
+
+	entityReq.Path = "entity/id/" + entityID1
+	resp, err = is.HandleRequest(ctx, entityReq)
+	if err != nil || (resp != nil && resp.IsError()) {
+		t.Fatalf("err:%v resp:%#v", err, resp)
+	}
+
+	entity1Lookup, err := is.MemDBEntityByID(entityID1, false)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if entity1Lookup == nil {
+		t.Fatalf("failed to create entity: %v", err)
+	}
+
+	if len(entity1Lookup.Aliases) != 1 {
+		t.Fatalf("bad: number of aliases in entity; expected: 1, actual: %d", len(entity1Lookup.Aliases))
+	}
+}