Entities may have duplicate policies (#12812)

* Entities may have duplicate plicies * Adding changelog * removing duplicates on reading entity policies * fix changelog
hashicorp · Oct 22, 2021 · 6c03006 · 6c03006
1 parent 375a7a6
commit 6c03006
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 5 deletions.
diff --git a/changelog/12812.txt b/changelog/12812.txt
@@ -0,0 +1,3 @@
+```release-note:bug
+identity: suppress duplicate policies on entities
+```
diff --git a/vault/identity_store_entities.go b/vault/identity_store_entities.go
@@ -238,7 +238,7 @@ func (i *IdentityStore) handleEntityUpdateCommon() framework.OperationFunc {
 		// Update the policies if supplied
 		entityPoliciesRaw, ok := d.GetOk("policies")
 		if ok {
-			entity.Policies = entityPoliciesRaw.([]string)
+			entity.Policies = strutil.RemoveDuplicates(entityPoliciesRaw.([]string), false)
 		}
 
 		if strutil.StrListContains(entity.Policies, "root") {
@@ -353,7 +353,7 @@ func (i *IdentityStore) handleEntityReadCommon(ctx context.Context, entity *iden
 	respData["name"] = entity.Name
 	respData["metadata"] = entity.Metadata
 	respData["merged_entity_ids"] = entity.MergedEntityIDs
-	respData["policies"] = entity.Policies
+	respData["policies"] = strutil.RemoveDuplicates(entity.Policies, false)
 	respData["disabled"] = entity.Disabled
 	respData["namespace_id"] = entity.NamespaceID
 
@@ -820,7 +820,7 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit
 
 		// If told to, merge policies
 		if mergePolicies {
-			toEntity.Policies = strutil.MergeSlices(toEntity.Policies, fromEntity.Policies)
+			toEntity.Policies = strutil.RemoveDuplicates(strutil.MergeSlices(toEntity.Policies, fromEntity.Policies), false)
 		}
 
 		// If the entity from which we are merging from was already a merged

diff --git a/vault/identity_store_entities_test.go b/vault/identity_store_entities_test.go
@@ -896,7 +896,7 @@ func TestIdentityStore_EntityCRUD(t *testing.T) {
 	registerData := map[string]interface{}{
 		"name":     "testentityname",
 		"metadata": []string{"someusefulkey=someusefulvalue"},
-		"policies": []string{"testpolicy1", "testpolicy2"},
+		"policies": []string{"testpolicy1", "testpolicy1", "testpolicy2", "testpolicy2"},
 	}
 
 	registerReq := &logical.Request{
@@ -932,7 +932,7 @@ func TestIdentityStore_EntityCRUD(t *testing.T) {
 
 	if resp.Data["id"] != id ||
 		resp.Data["name"] != registerData["name"] ||
-		!reflect.DeepEqual(resp.Data["policies"], registerData["policies"]) {
+		!reflect.DeepEqual(resp.Data["policies"], strutil.RemoveDuplicates(registerData["policies"].([]string), false)) {
 		t.Fatalf("bad: entity response")
 	}
 
@@ -1222,6 +1222,7 @@ func TestIdentityStore_MergeEntitiesByID_DuplicateFromEntityIDs(t *testing.T) {
 		Data: map[string]interface{}{
 			"name":     "testentityname2",
 			"metadata": []string{"someusefulkey=someusefulvalue"},
+			"policies": []string{"testPolicy1", "testPolicy1", "testPolicy2"},
 		},
 	}
 
@@ -1258,6 +1259,7 @@ func TestIdentityStore_MergeEntitiesByID_DuplicateFromEntityIDs(t *testing.T) {
 			"mount_accessor": githubAccessor,
 			"metadata":       []string{"organization=hashicorp", "team=vault"},
 			"entity_id":      entityID2,
+			"policies": []string{"testPolicy1", "testPolicy1", "testPolicy2"},
 		},
 	}
 
@@ -1321,4 +1323,8 @@ func TestIdentityStore_MergeEntitiesByID_DuplicateFromEntityIDs(t *testing.T) {
 	if len(entity1Lookup.Aliases) != 1 {
 		t.Fatalf("bad: number of aliases in entity; expected: 1, actual: %d", len(entity1Lookup.Aliases))
 	}
+
+	if len(entity1Lookup.Policies) != 2 {
+		t.Fatalf("invalid number of entity policies; expected: 2, actualL: %d", len(entity1Lookup.Policies))
+	}
 }