Skip to content

Commit

Permalink
Fix two problems with entity alias updating (#5733)
Browse files Browse the repository at this point in the history
Fix two problems with entity alias updating
  • Loading branch information
jefferai authored and briankassouf committed Nov 9, 2018
1 parent f69fe93 commit 9820ee6
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 4 deletions.
11 changes: 9 additions & 2 deletions vault/identity_store_aliases.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,13 @@ func (i *IdentityStore) handleAliasUpdateCommon() framework.OperationFunc {
if entity == nil {
return nil, fmt.Errorf("existing alias is not associated with an entity")
}
} else if alias.ID != "" {
// This is an update, not a create; if we have an associated entity
// already, load it
entity, err = i.MemDBEntityByAliasID(alias.ID, true)
if err != nil {
return nil, err
}
}

resp := &logical.Response{}
Expand Down Expand Up @@ -212,8 +219,8 @@ func (i *IdentityStore) handleAliasUpdateCommon() framework.OperationFunc {
entity = canonicalEntity
entity.Aliases = append(entity.Aliases, alias)
} else if entity.ID != canonicalEntity.ID {
// In this case we found an entity from alias factors but it's not
// the same, so it's a migration
// In this case we found an entity from alias factors or given
// alias ID but it's not the same, so it's a migration
previousEntity = entity
entity = canonicalEntity

Expand Down
30 changes: 28 additions & 2 deletions vault/identity_store_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -290,8 +290,34 @@ func (i *IdentityStore) upsertEntityInTxn(ctx context.Context, txn *memdb.Txn, e
return err
}

if aliasByFactors != nil && aliasByFactors.CanonicalID != entity.ID {
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)
switch {
case aliasByFactors == nil:
// Not found, no merging needed
case aliasByFactors.CanonicalID == entity.ID:
// Lookup found the same entity, so it's already attached to the
// right place
case previousEntity != nil && aliasByFactors.CanonicalID == previousEntity.ID:
// previousEntity isn't upserted yet so may still contain the old
// alias reference in memdb if it was just changed; validate
// whether or not it's _actually_ still tied to the entity
var found bool
for _, prevEntAlias := range previousEntity.Aliases {
if prevEntAlias.ID == alias.ID {
found = true
break
}
}
// If we didn't find the alias still tied to previousEntity, we
// shouldn't use the merging logic and should bail
if !found {
break
}

// Otherwise it's still tied to previousEntity and fall through
// into merging
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)
respErr, intErr := i.mergeEntity(ctx, txn, entity, []string{aliasByFactors.CanonicalID}, true, false, true)
switch {
case respErr != nil:
Expand Down

0 comments on commit 9820ee6

Please sign in to comment.