From 0b3eea4441a85eb0f44aeffbd649c5d7442c91b3 Mon Sep 17 00:00:00 2001 From: Matt Greenfield Date: Tue, 12 Oct 2021 13:35:19 -0600 Subject: [PATCH] Dedup from_entity_ids when merging two entities (#10101) Fixes #10100 --- changelog/10101.txt | 3 + vault/identity_store_entities.go | 6 +- vault/identity_store_entities_test.go | 115 ++++++++++++++++++++++++++ 3 files changed, 122 insertions(+), 2 deletions(-) create mode 100644 changelog/10101.txt diff --git a/changelog/10101.txt b/changelog/10101.txt new file mode 100644 index 000000000..6f831c8a4 --- /dev/null +++ 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 index 4a0efc7a9..a8fa502b9 100644 --- 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 index 910b3ff1d..a5841ab40 100644 --- 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)) + } +}