From b2d88d836d82023af2c18d2b6e89226b87599bf3 Mon Sep 17 00:00:00 2001 From: Matt Greenfield Date: Fri, 2 Oct 2020 15:20:53 -0600 Subject: [PATCH] Fix entity group associations - When two entities are merged, remove the from entity ID in any associated groups. - When two entities are merged, also merge their associated group memberships. Fixes #10084 --- changelog/10085.txt | 3 + vault/identity_store_entities.go | 27 ++++++++- vault/identity_store_entities_test.go | 55 +++++++++++++++++-- .../api-docs/secret/identity/entity.mdx | 2 +- 4 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 changelog/10085.txt diff --git a/changelog/10085.txt b/changelog/10085.txt new file mode 100644 index 0000000000000..6688b0fd345f7 --- /dev/null +++ b/changelog/10085.txt @@ -0,0 +1,3 @@ +```release-note:bug +identity: merge associated entity groups when merging entities +``` diff --git a/vault/identity_store_entities.go b/vault/identity_store_entities.go index 08f9f2bdfef8d..4a0efc7a97d4b 100644 --- a/vault/identity_store_entities.go +++ b/vault/identity_store_entities.go @@ -583,7 +583,7 @@ func (i *IdentityStore) handleEntityDeleteCommon(ctx context.Context, txn *memdb // internal and external groups, err := i.MemDBGroupsByMemberEntityIDInTxn(txn, entity.ID, true, false) if err != nil { - return nil + return err } for _, group := range groups { @@ -767,6 +767,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 { if fromEntityID == toEntity.ID { return errors.New("to_entity_id should not be present in from_entity_ids"), nil @@ -814,6 +815,22 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit // the entity we are merging into is composed of. toEntity.MergedEntityIDs = append(toEntity.MergedEntityIDs, fromEntity.ID) + // Remove entity ID as a member from all the groups it belongs, both + // internal and external + groups, err := i.MemDBGroupsByMemberEntityIDInTxn(txn, fromEntity.ID, true, false) + if err != nil { + return nil, err + } + for _, group := range groups { + group.MemberEntityIDs = strutil.StrListDelete(group.MemberEntityIDs, fromEntity.ID) + err = i.UpsertGroupInTxn(ctx, txn, group, persist && !isPerfSecondaryOrStandby) + if err != nil { + return nil, err + } + + fromEntityGroups = append(fromEntityGroups, group) + } + // Delete the entity which we are merging from in MemDB using the same transaction err = i.MemDBDeleteEntityByIDInTxn(txn, fromEntity.ID) if err != nil { @@ -835,6 +852,14 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit return nil, err } + for _, group := range fromEntityGroups { + group.MemberEntityIDs = append(group.MemberEntityIDs, toEntity.ID) + err = i.UpsertGroupInTxn(ctx, txn, group, persist && !isPerfSecondaryOrStandby) + if err != nil { + return nil, err + } + } + if persist && !isPerfSecondaryOrStandby { // Persist the entity which we are merging to toEntityAsAny, err := ptypes.MarshalAny(toEntity) diff --git a/vault/identity_store_entities_test.go b/vault/identity_store_entities_test.go index ec9dc74e14c81..910b3ff1dd1af 100644 --- a/vault/identity_store_entities_test.go +++ b/vault/identity_store_entities_test.go @@ -12,6 +12,7 @@ import ( credGithub "github.com/hashicorp/vault/builtin/credential/github" "github.com/hashicorp/vault/helper/identity" "github.com/hashicorp/vault/helper/namespace" + "github.com/hashicorp/vault/sdk/helper/strutil" "github.com/hashicorp/vault/sdk/logical" ) @@ -1071,6 +1072,19 @@ func TestIdentityStore_MergeEntitiesByID(t *testing.T) { t.Fatalf("bad: number of aliases in entity; expected: 2, actual: %d", len(entity1.Aliases)) } + entity1GroupReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "group", + Data: map[string]interface{}{ + "member_entity_ids": entityID1, + }, + } + resp, err = is.HandleRequest(ctx, entity1GroupReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + entity1GroupID := resp.Data["id"].(string) + registerReq.Data = registerData2 // Register another entity resp, err = is.HandleRequest(ctx, registerReq) @@ -1114,6 +1128,19 @@ func TestIdentityStore_MergeEntitiesByID(t *testing.T) { t.Fatalf("bad: number of aliases in entity; expected: 2, actual: %d", len(entity2.Aliases)) } + entity2GroupReq := &logical.Request{ + Operation: logical.UpdateOperation, + Path: "group", + Data: map[string]interface{}{ + "member_entity_ids": entityID2, + }, + } + resp, err = is.HandleRequest(ctx, entity2GroupReq) + if err != nil || (resp != nil && resp.IsError()) { + t.Fatalf("err:%v resp:%#v", err, resp) + } + entity2GroupID := resp.Data["id"].(string) + mergeData := map[string]interface{}{ "to_entity_id": entityID1, "from_entity_ids": []string{entityID2}, @@ -1147,12 +1174,12 @@ func TestIdentityStore_MergeEntitiesByID(t *testing.T) { t.Fatalf("err:%v resp:%#v", err, resp) } - entity2Aliases := resp.Data["aliases"].([]interface{}) - if len(entity2Aliases) != 4 { - t.Fatalf("bad: number of aliases in entity; expected: 4, actual: %d", len(entity2Aliases)) + entity1Aliases := resp.Data["aliases"].([]interface{}) + if len(entity1Aliases) != 4 { + t.Fatalf("bad: number of aliases in entity; expected: 4, actual: %d", len(entity1Aliases)) } - for _, aliasRaw := range entity2Aliases { + for _, aliasRaw := range entity1Aliases { alias := aliasRaw.(map[string]interface{}) aliasLookedUp, err := is.MemDBAliasByID(alias["id"].(string), false, false) if err != nil { @@ -1162,4 +1189,24 @@ func TestIdentityStore_MergeEntitiesByID(t *testing.T) { t.Fatalf("index for alias id %q is not updated", alias["id"].(string)) } } + + entity1Groups := resp.Data["direct_group_ids"].([]string) + if len(entity1Groups) != 2 { + t.Fatalf("bad: number of groups in entity; expected: 2, actual: %d", len(entity1Groups)) + } + + for _, group := range []string{entity1GroupID, entity2GroupID} { + if !strutil.StrListContains(entity1Groups, group) { + t.Fatalf("group id %q not found in merged entity direct groups %q", group, entity1Groups) + } + + groupLookedUp, err := is.MemDBGroupByID(group, false) + if err != nil { + t.Fatal(err) + } + expectedEntityIDs := []string{entity1.ID} + if !strutil.EquivalentSlices(groupLookedUp.MemberEntityIDs, expectedEntityIDs) { + t.Fatalf("group id %q should contain %q but contains %q", group, expectedEntityIDs, groupLookedUp.MemberEntityIDs) + } + } } diff --git a/website/content/api-docs/secret/identity/entity.mdx b/website/content/api-docs/secret/identity/entity.mdx index e7bd8ea9d0a1a..89d9553a7c76e 100644 --- a/website/content/api-docs/secret/identity/entity.mdx +++ b/website/content/api-docs/secret/identity/entity.mdx @@ -395,7 +395,7 @@ $ curl \ ## Merge Entities -This endpoint merges many entities into one entity. +This endpoint merges many entities into one entity. Additionally, all groups associated with `from_entity_ids` are merged with those of `to_entity_id`. | Method | Path | | :----- | :----------------------- |