From 84dac8486a559297fcafd2b56e36fd9336bd6ca2 Mon Sep 17 00:00:00 2001 From: Mike Palmiotto Date: Mon, 16 Dec 2024 10:46:36 -0500 Subject: [PATCH] identity: Introduce ConflictResolver interface (#29187) This PR introduces a new interface for conflict resolution of duplicate Identity artifacts. The initial implementation just reorganizes the code to use the interface with no behavior change. The interface is intended to provide a minimal touchpoint for implementing new conflict resolution behavior. Since those changes will also introduce significant testcases, the aim here is to merge the new interface and ensure the current code works as intended (according to existing tests). --- vault/identity_store_conflicts.go | 85 +++++++++++++++++++++++++++++++ vault/identity_store_structs.go | 2 + vault/identity_store_util.go | 40 ++++++++------- 3 files changed, 110 insertions(+), 17 deletions(-) create mode 100644 vault/identity_store_conflicts.go diff --git a/vault/identity_store_conflicts.go b/vault/identity_store_conflicts.go new file mode 100644 index 0000000000..8495af1a25 --- /dev/null +++ b/vault/identity_store_conflicts.go @@ -0,0 +1,85 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package vault + +import ( + "context" + "errors" + + log "github.com/hashicorp/go-hclog" + "github.com/hashicorp/vault/helper/identity" +) + +var errDuplicateIdentityName = errors.New("duplicate identity name") + +// ConflictResolver defines the interface for resolving conflicts between +// entities, groups, and aliases. All methods should implement a check for +// existing=nil. This is an intentional design choice to allow the caller to +// search for extra information if necessary. +type ConflictResolver interface { + ResolveEntities(ctx context.Context, existing, duplicate *identity.Entity) error + ResolveGroups(ctx context.Context, existing, duplicate *identity.Group) error + ResolveAliases(ctx context.Context, parent *identity.Entity, existing, duplicate *identity.Alias) error +} + +// The 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 +} + +// ResolveEntities logs a warning message when a pre-existing Entity is found +// and returns a duplicate name error, which should be handled by the caller by +// putting the system in case-sensitive mode. +func (r *errorResolver) ResolveEntities(ctx context.Context, existing, duplicate *identity.Entity) error { + if existing == nil { + return nil + } + + r.logger.Warn(errDuplicateIdentityName.Error(), + "entity_name", duplicate.Name, + "duplicate_of_name", existing.Name, + "duplicate_of_id", existing.ID, + "action", "merge the duplicate entities into one") + + return errDuplicateIdentityName +} + +// ResolveGroups logs a warning message when a pre-existing Group is found and +// returns a duplicate name error, which should be handled by the caller by +// putting the system in case-sensitive mode. +func (r *errorResolver) ResolveGroups(ctx context.Context, existing, duplicate *identity.Group) error { + if existing == nil { + return nil + } + + r.logger.Warn(errDuplicateIdentityName.Error(), + "group_name", duplicate.Name, + "duplicate_of_name", existing.Name, + "duplicate_of_id", existing.ID, + "action", "merge the contents of duplicated groups into one and delete the other") + + return errDuplicateIdentityName +} + +// ResolveAliases logs a warning message when a pre-existing Alias is found and +// returns a duplicate name error, which should be handled by the caller by +// putting the system in case-sensitive mode. +func (r *errorResolver) ResolveAliases(ctx context.Context, parent *identity.Entity, existing, duplicate *identity.Alias) error { + if existing == nil { + return nil + } + + r.logger.Warn(errDuplicateIdentityName.Error(), + "alias_name", duplicate.Name, + "mount_accessor", duplicate.MountAccessor, + "local", duplicate.Local, + "entity_name", parent.Name, + "alias_canonical_id", duplicate.CanonicalID, + "duplicate_of_name", existing.Name, + "duplicate_of_canonical_id", existing.CanonicalID, + "action", "merge the canonical entity IDs into one") + + return errDuplicateIdentityName +} diff --git a/vault/identity_store_structs.go b/vault/identity_store_structs.go index 46a8b1d9a9..8ff260876f 100644 --- a/vault/identity_store_structs.go +++ b/vault/identity_store_structs.go @@ -110,6 +110,8 @@ type IdentityStore struct { // aliasLocks is used to protect modifications to alias entries based on the uniqueness factor // which is name + accessor aliasLocks []*locksutil.LockEntry + + conflictResolver ConflictResolver } type groupDiff struct { diff --git a/vault/identity_store_util.go b/vault/identity_store_util.go index 67d27aa829..845b0672d1 100644 --- a/vault/identity_store_util.go +++ b/vault/identity_store_util.go @@ -28,9 +28,8 @@ import ( ) var ( - errDuplicateIdentityName = errors.New("duplicate identity name") - errCycleDetectedPrefix = "cyclic relationship detected for member group ID" - tmpSuffix = ".tmp" + errCycleDetectedPrefix = "cyclic relationship detected for member group ID" + tmpSuffix = ".tmp" ) func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error { @@ -39,6 +38,12 @@ func (c *Core) loadIdentityStoreArtifacts(ctx context.Context) error { return nil } + // Resolve all conflicts by logging a warning and returning + // errDuplicateIdentityName. The error will flip the identityStore into + // case-sensitive mode by switching the underlying schema to one with a + // relaxed lowerCase constraint and reload all artifacts into MemDB. + c.identityStore.conflictResolver = &errorResolver{c.identityStore.logger} + loadFunc := func(context.Context) error { if err := c.identityStore.loadEntities(ctx); err != nil { return err @@ -141,11 +146,8 @@ func (i *IdentityStore) loadGroups(ctx context.Context) error { if err != nil { return err } - if groupByName != nil { - i.logger.Warn(errDuplicateIdentityName.Error(), "group_name", group.Name, "conflicting_group_name", groupByName.Name, "action", "merge the contents of duplicated groups into one and delete the other") - if !i.disableLowerCasedNames { - return errDuplicateIdentityName - } + if err := i.conflictResolver.ResolveGroups(ctx, groupByName, group); err != nil && !i.disableLowerCasedNames { + return err } if i.logger.IsDebug() { @@ -468,11 +470,8 @@ LOOP: if err != nil { return nil } - if entityByName != nil { - i.logger.Warn(errDuplicateIdentityName.Error(), "entity_name", entity.Name, "conflicting_entity_name", entityByName.Name, "action", "merge the duplicate entities into one") - if !i.disableLowerCasedNames { - return errDuplicateIdentityName - } + if err := i.conflictResolver.ResolveEntities(ctx, entityByName, entity); err != nil && !i.disableLowerCasedNames { + return err } mountAccessors := getAccessorsOnDuplicateAliases(entity.Aliases) @@ -622,7 +621,15 @@ func (i *IdentityStore) upsertEntityInTxn(ctx context.Context, txn *memdb.Txn, e fallthrough default: - i.logger.Warn("alias is already tied to a different entity; these entities are being merged", "alias_id", alias.ID, "other_entity_id", aliasByFactors.CanonicalID, "entity_aliases", entity.Aliases, "alias_by_factors", aliasByFactors) + // Though this is technically a conflict that should be resolved by the + // ConflictResolver implementation, the behavior here is a bit nuanced. + // Rather than introduce a behavior change, handle this case directly as + // before by merging. + i.logger.Warn("alias is already tied to a different entity; these entities are being merged", + "alias_id", alias.ID, + "other_entity_id", aliasByFactors.CanonicalID, + "entity_aliases", entity.Aliases, + "alias_by_factors", aliasByFactors) respErr, intErr := i.mergeEntityAsPartOfUpsert(ctx, txn, entity, aliasByFactors.CanonicalID, persist) switch { @@ -638,9 +645,8 @@ func (i *IdentityStore) upsertEntityInTxn(ctx context.Context, txn *memdb.Txn, e } if strutil.StrListContains(aliasFactors, i.sanitizeName(alias.Name)+alias.MountAccessor) { - i.logger.Warn(errDuplicateIdentityName.Error(), "alias_name", alias.Name, "mount_accessor", alias.MountAccessor, "local", alias.Local, "entity_name", entity.Name, "action", "delete one of the duplicate aliases") - if !i.disableLowerCasedNames { - return errDuplicateIdentityName + if err := i.conflictResolver.ResolveAliases(ctx, entity, aliasByFactors, alias); err != nil && !i.disableLowerCasedNames { + return err } }