diff --git a/vault/identity_store_entities.go b/vault/identity_store_entities.go index efa1e51bda43c..ad6caa92c5a10 100644 --- a/vault/identity_store_entities.go +++ b/vault/identity_store_entities.go @@ -579,7 +579,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 { @@ -762,6 +762,7 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit } isPerfSecondaryOrStandby := i.core.ReplicationState().HasState(consts.ReplicationPerformanceSecondary) || i.core.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 @@ -809,6 +810,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 { @@ -830,6 +847,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 59d7cad554dd0..bd69ed1c8e75c 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" ) @@ -1072,6 +1073,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) @@ -1115,6 +1129,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}, @@ -1148,12 +1175,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 { @@ -1163,4 +1190,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) + } + } }