luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Update group memberships when entity is deleted (#5786) 

* Use common abstraction for entity deletion

* Update group memberships before deleting entity

* Added test

* Fix return statements

* Update comment

* Cleanup member entity IDs while loading groups

* Added test to ensure that upgrade happens properly

* Ensure that the group gets persisted if upgrade code modifies it
This commit is contained in:
Vishal Nayak 2018-11-15 20:07:45 -05:00 committed by GitHub
parent 227a664b06
commit 43e3ff808a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 193 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

88
vault/identity_store_entities.go
View File

@ -412,39 +412,15 @@ func (i *IdentityStore) pathEntityIDDelete() framework.OperationFunc {
if err != nil {
return nil, err
}
// If there is no entity for the ID, do nothing
if entity == nil {
return nil, nil
}
ns, err := namespace.FromContext(ctx)
if err != nil {
return nil, err
}
if entity.NamespaceID != ns.ID {
return nil, nil
}
// Delete all the aliases in the entity. This function will also remove
// the corresponding alias indexes too.
err = i.deleteAliasesInEntityInTxn(txn, entity, entity.Aliases)
err = i.handleEntityDeleteCommon(ctx, txn, entity)
if err != nil {
return nil, err
}
// Delete the entity using the same transaction
err = i.MemDBDeleteEntityByIDInTxn(txn, entity.ID)
if err != nil {
return nil, err
}
// Delete the entity from storage
err = i.entityPacker.DeleteItem(entity.ID)
if err != nil {
return nil, err
}
// Committing the transaction *after* successfully deleting entity
txn.Commit()
return nil, nil
@ -484,32 +460,62 @@ func (i *IdentityStore) pathEntityNameDelete() framework.OperationFunc {
return nil, nil
}
// Delete all the aliases in the entity. This function will also remove
// the corresponding alias indexes too.
err = i.deleteAliasesInEntityInTxn(txn, entity, entity.Aliases)
err = i.handleEntityDeleteCommon(ctx, txn, entity)
if err != nil {
return nil, err
}
// Delete the entity using the same transaction
err = i.MemDBDeleteEntityByIDInTxn(txn, entity.ID)
if err != nil {
return nil, err
}
// Delete the entity from storage
err = i.entityPacker.DeleteItem(entity.ID)
if err != nil {
return nil, err
}
// Committing the transaction *after* successfully deleting entity
txn.Commit()
return nil, nil
}
}
func (i *IdentityStore) handleEntityDeleteCommon(ctx context.Context, txn *memdb.Txn, entity *identity.Entity) error {
ns, err := namespace.FromContext(ctx)
if err != nil {
return err
}
if entity.NamespaceID != ns.ID {
return nil
}
// Remove entity ID as a member from all the groups it belongs, both
// internal and external
groups, err := i.MemDBGroupsByMemberEntityIDInTxn(txn, entity.ID, true, false)
if err != nil {
return nil
}
for _, group := range groups {
group.MemberEntityIDs = strutil.StrListDelete(group.MemberEntityIDs, entity.ID)
err = i.UpsertGroupInTxn(txn, group, true)
if err != nil {
return err
}
}
// Delete all the aliases in the entity and the respective indexes
err = i.deleteAliasesInEntityInTxn(txn, entity, entity.Aliases)
if err != nil {
return err
}
// Delete the entity using the same transaction
err = i.MemDBDeleteEntityByIDInTxn(txn, entity.ID)
if err != nil {
return err
}
// Delete the entity from storage
err = i.entityPacker.DeleteItem(entity.ID)
if err != nil {
return err
}
return nil
}
func (i *IdentityStore) pathEntityIDList() framework.OperationFunc {
return func(ctx context.Context, req *logical.Request, d *framework.FieldData) (*logical.Response, error) {
return i.handlePathEntityListCommon(ctx, req, d, true)

67
vault/identity_store_entities_test.go
View File

@ -15,6 +15,73 @@ import (
"github.com/hashicorp/vault/logical"
)
func TestIdentityStore_EntityDeleteGroupMembershipUpdate(t *testing.T) {
i, _, _ := testIdentityStoreWithGithubAuth(namespace.RootContext(nil), t)
// Create an entity
resp, err := i.HandleRequest(namespace.RootContext(nil), &logical.Request{
Path: "entity",
Operation: logical.UpdateOperation,
Data: map[string]interface{}{
"name": "testentity",
},
})
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: err:%v\nresp: %#v", err, resp)
}
entityID := resp.Data["id"].(string)
// Create a group
resp, err = i.HandleRequest(namespace.RootContext(nil), &logical.Request{
Path: "group",
Operation: logical.UpdateOperation,
Data: map[string]interface{}{
"name": "testgroup",
"member_entity_ids": []string{entityID},
},
})
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: err:%v\nresp: %#v", err, resp)
}
// Ensure that the group has entity ID as its member
resp, err = i.HandleRequest(namespace.RootContext(nil), &logical.Request{
Path: "group/name/testgroup",
Operation: logical.ReadOperation,
})
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: err:%v\nresp: %#v", err, resp)
}
expected := []string{entityID}
actual := resp.Data["member_entity_ids"].([]string)
if !reflect.DeepEqual(expected, actual) {
t.Fatalf("bad: member entity ids; expected: %#v\nactual: %#v", expected, actual)
}
// Delete the entity
resp, err = i.HandleRequest(namespace.RootContext(nil), &logical.Request{
Path: "entity/name/testentity",
Operation: logical.DeleteOperation,
})
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: err:%v\nresp: %#v", err, resp)
}
// Ensure that the group does not have entity ID as it's member anymore
resp, err = i.HandleRequest(namespace.RootContext(nil), &logical.Request{
Path: "group/name/testgroup",
Operation: logical.ReadOperation,
})
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: err:%v\nresp: %#v", err, resp)
}
expected = []string{}
actual = resp.Data["member_entity_ids"].([]string)
if !reflect.DeepEqual(expected, actual) {
t.Fatalf("bad: member entity ids; expected: %#v\nactual: %#v", expected, actual)
}
}
func TestIdentityStore_CaseInsensitiveEntityName(t *testing.T) {
ctx := namespace.RootContext(nil)
i, _, _ := testIdentityStoreWithGithubAuth(ctx, t)

62
vault/identity_store_groups_test.go
View File

@ -12,6 +12,68 @@ import (
"github.com/hashicorp/vault/logical"
)
func TestIdentityStore_GroupEntityMembershipUpgrade(t *testing.T) {
c, keys, rootToken := TestCoreUnsealed(t)
// Create a group
resp, err := c.identityStore.HandleRequest(namespace.RootContext(nil), &logical.Request{
Path: "group",
Operation: logical.UpdateOperation,
Data: map[string]interface{}{
"name": "testgroup",
},
})
if err != nil || (resp != nil && resp.IsError()) {
t.Fatalf("bad: err:%v\nresp: %#v", err, resp)
}
// Create a memdb transaction
txn := c.identityStore.db.Txn(true)
defer txn.Abort()
// Fetch the above created group
group, err := c.identityStore.MemDBGroupByNameInTxn(namespace.RootContext(nil), txn, "testgroup", true)
if err != nil {
t.Fatal(err)
}
// Manually add an invalid entity as the group's member
group.MemberEntityIDs = []string{"invalidentityid"}
// Persist the group
err = c.identityStore.UpsertGroupInTxn(txn, group, true)
if err != nil {
t.Fatal(err)
}
txn.Commit()
// Perform seal and unseal forcing an upgrade
err = c.Seal(rootToken)
if err != nil {
t.Fatal(err)
}
for i, key := range keys {
unseal, err := TestCoreUnseal(c, key)
if err != nil {
t.Fatal(err)
}
if i+1 == len(keys) && !unseal {
t.Fatalf("failed to unseal")
}
}
// Read the group and ensure that invalid entity id is cleaned up
group, err = c.identityStore.MemDBGroupByName(namespace.RootContext(nil), "testgroup", false)
if err != nil {
t.Fatal(err)
}
if len(group.MemberEntityIDs) != 0 {
t.Fatalf("bad: member entity IDs; expected: none, actual: %#v", group.MemberEntityIDs)
}
}
func TestIdentityStore_MemberGroupIDDelete(t *testing.T) {
ctx := namespace.RootContext(nil)
i, _, _ := testIdentityStoreWithGithubAuth(ctx, t)

18
vault/identity_store_util.go
View File

@ -118,7 +118,23 @@ func (i *IdentityStore) loadGroups(ctx context.Context) error {
txn := i.db.Txn(true)
err = i.UpsertGroupInTxn(txn, group, false)
// Before pull#5786, entity memberships in groups were not getting
// updated when respective entities were deleted. This is here to
// check that the entity IDs in the group are indeed valid, and if
// not remove them.
persist := false
for _, memberEntityID := range group.MemberEntityIDs {
entity, err := i.MemDBEntityByID(memberEntityID, false)
if err != nil {
return err
}
if entity == nil {
persist = true
group.MemberEntityIDs = strutil.StrListDelete(group.MemberEntityIDs, memberEntityID)
}
}
err = i.UpsertGroupInTxn(txn, group, persist)
if err != nil {
txn.Abort()
return errwrap.Wrapf("failed to update group in memdb: {{err}}", err)