identity: Resolve conflicts with rename

changelog/29356.txt

@@ -0,0 +1,5 @@
+```release-note:feature
+**Identity De-duplication**: Vault can now automatically resolve duplicate
+Entities and Groups by renaming them. This feature is disabled by default and
+can be enabled through the `force_identity_deduplication` activation flag.
+```

helper/activationflags/activation_flags.go

@@ -14,6 +14,7 @@ import (
 
 const (
 	storagePathActivationFlags = "activation-flags"
+	IdentityDeduplication      = "force-identity-deduplication"
 )
 
 type FeatureActivationFlags struct {

helper/identity/identity.go

@@ -58,6 +58,22 @@ func (e *Entity) UpsertAlias(alias *Alias) {
 	e.Aliases = append(e.Aliases, alias)
 }
 
+func (e *Entity) DeleteAliasByID(aliasID string) {
+	idx := -1
+	for i, item := range e.Aliases {
+		if item.ID == aliasID {
+			idx = i
+			break
+		}
+	}
+
+	if idx < 0 {
+		return
+	}
+
+	e.Aliases = append(e.Aliases[:idx], e.Aliases[idx+1:]...)
+}
+
 func (p *Alias) Clone() (*Alias, error) {
 	if p == nil {
 		return nil, fmt.Errorf("nil alias")

vault/identity_store.go

@@ -45,7 +45,7 @@ func (c *Core) IdentityStore() *IdentityStore {
 	return c.identityStore
 }
 
-func (i *IdentityStore) resetDB(ctx context.Context) error {
+func (i *IdentityStore) resetDB() error {
 	var err error
 
 	i.db, err = memdb.NewMemDB(identityStoreSchema(!i.disableLowerCasedNames))
@@ -76,7 +76,7 @@ func NewIdentityStore(ctx context.Context, core *Core, config *logical.BackendCo
 
 	// Create a memdb instance, which by default, operates on lower cased
 	// identity names
-	err := iStore.resetDB(ctx)
+	err := iStore.resetDB()
 	if err != nil {
 		return nil, err
 	}

vault/identity_store_conflicts.go

@@ -27,7 +27,7 @@ type ConflictResolver interface {
 	ResolveAliases(ctx context.Context, parent *identity.Entity, existing, duplicate *identity.Alias) error
 }
 
-// The errorResolver is a ConflictResolver that logs a warning message when a
+// errorResolver is a ConflictResolver that logs a warning message when a
 // pre-existing Identity artifact is found with the same factors as a new one.
 type errorResolver struct {
 	logger log.Logger
@@ -207,7 +207,7 @@ func (r *duplicateReportingErrorResolver) Report() identityDuplicateReport {
 				index:        idx,
 				numOthers:    len(entities) - 1,
 			}
-			if idx < len(entities)-1 {
+			if idx > 0 {
 				r.resolutionHint = fmt.Sprintf("would rename to %s-%s", entity.Name, entity.ID)
 			} else {
 				r.resolutionHint = "would not rename"
@@ -234,7 +234,7 @@ func (r *duplicateReportingErrorResolver) Report() identityDuplicateReport {
 				index:        idx,
 				numOthers:    len(groups) - 1,
 			}
-			if idx < len(groups)-1 {
+			if idx > 0 {
 				r.resolutionHint = fmt.Sprintf("would rename to %s-%s", group.Name, group.ID)
 			} else {
 				r.resolutionHint = "would not rename"
@@ -278,8 +278,8 @@ func reportAliases(report *identityDuplicateReport, seen map[string][]*identity.
 				index:        idx,
 				numOthers:    len(aliases) - 1,
 			}
-			if idx < len(aliases)-1 {
-				r.resolutionHint = fmt.Sprintf("would merge into entity %s", aliases[len(aliases)-1].CanonicalID)
+			if idx > 0 {
+				r.resolutionHint = fmt.Sprintf("would merge into entity %s", aliases[0].CanonicalID)
 			} else {
 				r.resolutionHint = "would merge others into this entity"
 			}
@@ -361,3 +361,68 @@ func (r *duplicateReportingErrorResolver) LogReport(log Warner) {
 	log.Warn("end of identity duplicate report, refer to " +
 		identityDuplicateReportUrl + " for resolution.")
 }
+
+// renameResolver is a ConflictResolver that appends the artifact's UUID to its
+// name to resolve potential conflicts. The renamed resource is associated with
+// the duplicated artifact by adding a `duplicate_of_canonical_id` metadata
+// field.
+type renameResolver struct {
+	logger log.Logger
+}
+
+// ResolveEntities renames an entity duplicate in a deterministic way so that
+// all entities end up addressable by a unique name still. We rename the
+// pre-existing entity such that only the last occurrence retains its unmodified
+// name. Note that this is potentially destructive but is the best option
+// available to resolve duplicates in storage caused by bugs in our validation.
+func (r *renameResolver) ResolveEntities(ctx context.Context, existing, duplicate *identity.Entity) error {
+	if existing == nil {
+		return nil
+	}
+
+	duplicate.Name = duplicate.Name + "-" + duplicate.ID
+	if duplicate.Metadata == nil {
+		duplicate.Metadata = make(map[string]string)
+	}
+	duplicate.Metadata["duplicate_of_canonical_id"] = existing.ID
+
+	r.logger.Warn("renaming entity with duplicate name",
+		"namespace_id", duplicate.NamespaceID,
+		"entity_id", duplicate.ID,
+		"duplicate_of_canonical_id", existing.ID,
+		"renamed_from", duplicate.Name,
+		"renamed_to", duplicate.Name,
+	)
+
+	return nil
+}
+
+// ResolveGroups deals with group name duplicates by renaming those that
+// were "hidden" in memDB so they are queryable. It's important this is
+// deterministic so we don't end up with different group names on different
+// nodes. We use the ID to ensure the new name is unique bit also
+// deterministic. For now, don't persist this. The user can choose to
+// resolve it permanently by renaming or deleting explicitly.
+func (r *renameResolver) ResolveGroups(ctx context.Context, existing, duplicate *identity.Group) error {
+	if existing == nil {
+		return nil
+	}
+
+	duplicate.Name = duplicate.Name + "-" + duplicate.ID
+	if duplicate.Metadata == nil {
+		duplicate.Metadata = make(map[string]string)
+	}
+	duplicate.Metadata["duplicate_of_canonical_id"] = existing.ID
+	r.logger.Warn("renaming group with duplicate name",
+		"namespace_id", duplicate.NamespaceID,
+		"group_id", duplicate.ID,
+		"duplicate_of_canonical_id", existing.ID,
+		"new_name", duplicate.Name,
+	)
+	return nil
+}
+
+// ResolveAliases is a no-op for the renameResolver implementation.
+func (r *renameResolver) ResolveAliases(ctx context.Context, parent *identity.Entity, existing, duplicate *identity.Alias) error {
+	return nil
+}

vault/identity_store_conflicts_test.go

@@ -67,37 +67,37 @@ func TestDuplicateReportingErrorResolver(t *testing.T) {
 	expectReport := `
 DUPLICATES DETECTED, see following logs for details and refer to https://developer.hashicorp.com/vault/docs/upgrading/identity-deduplication for resolution.:
 1 different-case local entity alias duplicates found (potential security risk):
-local entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "local-mount" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" canonical_id="11111111-0000-0000-0000-000000000009" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000010"
-local entity-alias "different-CASE-ALIAS-1" with mount accessor "local-mount" duplicates 1 others: id="00000000-0000-0000-0000-000000000010" canonical_id="11111111-0000-0000-0000-000000000010" force_deduplication="would merge others into this entity"
+local entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "local-mount" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" canonical_id="11111111-0000-0000-0000-000000000009" force_deduplication="would merge others into this entity"
+local entity-alias "different-CASE-ALIAS-1" with mount accessor "local-mount" duplicates 1 others: id="00000000-0000-0000-0000-000000000010" canonical_id="11111111-0000-0000-0000-000000000010" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000009"
 end of different-case local entity-alias duplicates:
 2 different-case entity alias duplicates found (potential security risk):
-entity-alias "different-case-alias-1" with mount accessor "mount1" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" canonical_id="11111111-0000-0000-0000-000000000004" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000005"
-entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "mount1" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" canonical_id="11111111-0000-0000-0000-000000000005" force_deduplication="would merge others into this entity"
-entity-alias "different-case-alias-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000006" canonical_id="11111111-0000-0000-0000-000000000006" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000008"
-entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000007" canonical_id="11111111-0000-0000-0000-000000000007" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000008"
-entity-alias "different-CASE-ALIAS-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000008" canonical_id="11111111-0000-0000-0000-000000000008" force_deduplication="would merge others into this entity"
+entity-alias "different-case-alias-1" with mount accessor "mount1" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" canonical_id="11111111-0000-0000-0000-000000000004" force_deduplication="would merge others into this entity"
+entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "mount1" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" canonical_id="11111111-0000-0000-0000-000000000005" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000004"
+entity-alias "different-case-alias-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000006" canonical_id="11111111-0000-0000-0000-000000000006" force_deduplication="would merge others into this entity"
+entity-alias "DIFFERENT-CASE-ALIAS-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000007" canonical_id="11111111-0000-0000-0000-000000000007" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000006"
+entity-alias "different-CASE-ALIAS-1" with mount accessor "mount2" duplicates 2 others: id="00000000-0000-0000-0000-000000000008" canonical_id="11111111-0000-0000-0000-000000000008" force_deduplication="would merge into entity 11111111-0000-0000-0000-000000000006"
 end of different-case entity-alias duplicates:
 4 entity duplicates found:
-entity "different-case-dupe-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000010" force_deduplication="would rename to different-case-dupe-1-00000000-0000-0000-0000-000000000010"
+entity "different-case-dupe-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000010" force_deduplication="would not rename"
 entity "DIFFERENT-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000011" force_deduplication="would rename to DIFFERENT-case-DUPE-1-00000000-0000-0000-0000-000000000011"
-entity "different-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000012" force_deduplication="would not rename"
-entity "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000006" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000006"
-entity "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000007" force_deduplication="would not rename"
-entity "different-case-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000008" force_deduplication="would rename to different-case-dupe-1-00000000-0000-0000-0000-000000000008"
-entity "DIFFERENT-CASE-DUPE-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" force_deduplication="would not rename"
-entity "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000004"
-entity "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" force_deduplication="would not rename"
+entity "different-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000012" force_deduplication="would rename to different-case-DUPE-1-00000000-0000-0000-0000-000000000012"
+entity "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000006" force_deduplication="would not rename"
+entity "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000007" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000007"
+entity "different-case-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000008" force_deduplication="would not rename"
+entity "DIFFERENT-CASE-DUPE-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" force_deduplication="would rename to DIFFERENT-CASE-DUPE-1-00000000-0000-0000-0000-000000000009"
+entity "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" force_deduplication="would not rename"
+entity "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000005"
 end of entity duplicates:
 4 group duplicates found:
-group "different-case-dupe-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000010" force_deduplication="would rename to different-case-dupe-1-00000000-0000-0000-0000-000000000010"
+group "different-case-dupe-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000010" force_deduplication="would not rename"
 group "DIFFERENT-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000011" force_deduplication="would rename to DIFFERENT-case-DUPE-1-00000000-0000-0000-0000-000000000011"
-group "different-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000012" force_deduplication="would not rename"
-group "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000006" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000006"
-group "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000007" force_deduplication="would not rename"
-group "different-case-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000008" force_deduplication="would rename to different-case-dupe-1-00000000-0000-0000-0000-000000000008"
-group "DIFFERENT-CASE-DUPE-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" force_deduplication="would not rename"
-group "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000004"
-group "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" force_deduplication="would not rename"
+group "different-case-DUPE-1" with namespace ID "admin" duplicates 2 others: id="00000000-0000-0000-0000-000000000012" force_deduplication="would rename to different-case-DUPE-1-00000000-0000-0000-0000-000000000012"
+group "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000006" force_deduplication="would not rename"
+group "exact-dupe-1" with namespace ID "admin" duplicates 1 others: id="00000000-0000-0000-0000-000000000007" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000007"
+group "different-case-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000008" force_deduplication="would not rename"
+group "DIFFERENT-CASE-DUPE-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000009" force_deduplication="would rename to DIFFERENT-CASE-DUPE-1-00000000-0000-0000-0000-000000000009"
+group "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000004" force_deduplication="would not rename"
+group "exact-dupe-1" with namespace ID "root" duplicates 1 others: id="00000000-0000-0000-0000-000000000005" force_deduplication="would rename to exact-dupe-1-00000000-0000-0000-0000-000000000005"
 end of group duplicates:
 end of identity duplicate report, refer to https://developer.hashicorp.com/vault/docs/upgrading/identity-deduplication for resolution.:
 `
@@ -178,3 +178,78 @@ func (l *identityTestWarnLogger) Warn(msg string, args ...interface{}) {
 	}
 	l.buf.WriteString("\n")
 }
+
+// TestDuplicateRenameResolver tests that the rename resolver
+// correctly renames pre-existing entities and groups.
+func TestDuplicateRenameResolver(t *testing.T) {
+	t.Parallel()
+
+	entities := map[string][]string{
+		// 2 non-duplicates and 2 duplicates
+		"root": {
+			"foo",
+			"bar",
+			"exact-dupe-1", "exact-dupe-1",
+			"different-case-dupe-1", "DIFFERENT-CASE-DUPE-1",
+		},
+		// 1 non-duplicate and 3 duplicates
+		"admin": {
+			"foo",
+			"exact-dupe-1", "exact-dupe-1",
+			"different-case-dupe-1", "DIFFERENT-case-DUPE-1", "different-case-DUPE-1",
+		},
+		"developers": {"BAR"},
+	}
+
+	// Create a new errorResolver
+	r := &renameResolver{log.NewNullLogger()}
+
+	seenEntities := make(map[string]*identity.Entity)
+	seenGroups := make(map[string]*identity.Group)
+
+	for ns, entityList := range entities {
+		for i, name := range entityList {
+
+			id := fmt.Sprintf("00000000-0000-0000-0000-%012d", i)
+			// Create a new entity with the name/ns pair
+			entity := &identity.Entity{
+				ID:          id,
+				Name:        name,
+				NamespaceID: ns,
+			}
+
+			// Simulate a MemDB lookup
+			existingEntity := seenEntities[name]
+			err := r.ResolveEntities(context.Background(), existingEntity, entity)
+			require.NoError(t, err)
+
+			if existingEntity != nil {
+				require.Equal(t, name+"-"+id, entity.Name)
+				require.Equal(t, existingEntity.ID, entity.Metadata["duplicate_of_canonical_id"])
+			} else {
+				seenEntities[name] = entity
+			}
+
+			// Also, since the data model is the same, pretend these are groups too
+			group := &identity.Group{
+				ID:          id,
+				Name:        name,
+				NamespaceID: ns,
+			}
+
+			// More MemDB mocking
+			existingGroup := seenGroups[name]
+			err = r.ResolveGroups(context.Background(), existingGroup, group)
+			require.NoError(t, err)
+
+			if existingGroup != nil {
+				require.Equal(t, name+"-"+id, group.Name)
+				require.Equal(t, existingGroup.ID, group.Metadata["duplicate_of_canonical_id"])
+			} else {
+				seenGroups[name] = group
+			}
+		}
+	}
+
+	// No need to test entity alias merges here, since that's handled separately.
+}

vault/identity_store_entities.go

@@ -1038,12 +1038,16 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit
 						if err != nil {
 							return nil, fmt.Errorf("aborting entity merge - failed to delete orphaned alias %q during merge into entity %q: %w", toAliasId, toEntity.ID, err), nil
 						}
+						// Remove the alias from the entity's list in memory too!
+						toEntity.DeleteAliasByID(toAliasId)
 					} else if strutil.StrListContains(conflictingAliasIDsToKeep, toAliasId) {
 						i.logger.Info("Deleting from_entity alias during entity merge", "from_entity", fromEntityID, "deleted_alias", fromAlias.ID)
 						err := i.MemDBDeleteAliasByIDInTxn(txn, fromAlias.ID, false)
 						if err != nil {
 							return nil, fmt.Errorf("aborting entity merge - failed to delete orphaned alias %q during merge into entity %q: %w", fromAlias.ID, toEntity.ID, err), nil
 						}
+						// Remove the alias from the entity's list in memory too!
+						toEntity.DeleteAliasByID(toAliasId)
 
 						// Continue to next alias, as there's no alias to merge left in the from_entity
 						continue
@@ -1053,6 +1057,8 @@ func (i *IdentityStore) mergeEntity(ctx context.Context, txn *memdb.Txn, toEntit
 						if err != nil {
 							return nil, fmt.Errorf("aborting entity merge - failed to delete orphaned alias %q during merge into entity %q: %w", toAliasId, toEntity.ID, err), nil
 						}
+						// Remove the alias from the entity's list in memory too!
+						toEntity.DeleteAliasByID(toAliasId)
 					} else {
 						return fmt.Errorf("conflicting mount accessors in following alias IDs and neither were present in conflicting_alias_ids_to_keep: %s, %s", fromAlias.ID, toAliasId), nil, nil
 					}