Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dedup from_entity_ids when merging two entities #10101

Merged
merged 1 commit into from Oct 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/10101.txt
@@ -0,0 +1,3 @@
```release-note:bug
identity: dedup from_entity_ids when merging two entities
```
6 changes: 4 additions & 2 deletions vault/identity_store_entities.go
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
115 changes: 115 additions & 0 deletions vault/identity_store_entities_test.go
Expand Up @@ -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))
}
}