diff --git a/changelog/12473.txt b/changelog/12473.txt new file mode 100644 index 000000000..3a0ecddb5 --- /dev/null +++ b/changelog/12473.txt @@ -0,0 +1,3 @@ +```release-note:bug +identity: Fail alias rename if the resulting (name,accessor) exists already +``` \ No newline at end of file diff --git a/vault/external_tests/identity/aliases_test.go b/vault/external_tests/identity/aliases_test.go index be00122b9..162672e92 100644 --- a/vault/external_tests/identity/aliases_test.go +++ b/vault/external_tests/identity/aliases_test.go @@ -4,12 +4,12 @@ import ( "testing" "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/builtin/credential/github" + credLdap "github.com/hashicorp/vault/builtin/credential/ldap" + "github.com/hashicorp/vault/builtin/credential/userpass" vaulthttp "github.com/hashicorp/vault/http" "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/vault" - - "github.com/hashicorp/vault/builtin/credential/github" - credLdap "github.com/hashicorp/vault/builtin/credential/ldap" ) func TestIdentityStore_EntityAliasLocalMount(t *testing.T) { @@ -219,3 +219,85 @@ func TestIdentityStore_ListAlias(t *testing.T) { } } } + +// TestIdentityStore_RenameAlias_CannotMergeEntity verifies that an error is +// returned on an attempt to rename an alias to match another alias with the +// same mount accessor. This used to result in a merge entity. +func TestIdentityStore_RenameAlias_CannotMergeEntity(t *testing.T) { + coreConfig := &vault.CoreConfig{ + CredentialBackends: map[string]logical.Factory{ + "userpass": userpass.Factory, + }, + } + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + + client := cluster.Cores[0].Client + + err := client.Sys().EnableAuthWithOptions("userpass", &api.EnableAuthOptions{ + Type: "userpass", + }) + if err != nil { + t.Fatal(err) + } + + _, err = client.Logical().Write("auth/userpass/users/bsmith", map[string]interface{}{ + "password": "training", + }) + if err != nil { + t.Fatal(err) + } + _, err = client.Logical().Write("auth/userpass/login/bsmith", map[string]interface{}{ + "password": "training", + }) + if err != nil { + t.Fatal(err) + } + + mounts, err := client.Sys().ListAuth() + if err != nil { + t.Fatal(err) + } + + var mountAccessor string + for k, v := range mounts { + if k == "userpass/" { + mountAccessor = v.Accessor + break + } + } + if mountAccessor == "" { + t.Fatal("did not find userpass accessor") + } + + // Now create a new unrelated entity and alias + entityResp, err := client.Logical().Write("identity/entity", map[string]interface{}{ + "name": "bob-smith", + }) + if err != nil { + t.Fatalf("err:%v resp:%#v", err, entityResp) + } + if entityResp == nil { + t.Fatalf("expected a non-nil response") + } + + aliasResp, err := client.Logical().Write("identity/entity-alias", map[string]interface{}{ + "name": "bob", + "mount_accessor": mountAccessor, + }) + if err != nil { + t.Fatalf("err:%v resp:%#v", err, aliasResp) + } + aliasID2 := aliasResp.Data["id"].(string) + + // Rename this new alias to have the same name as the one implicitly created by our login as bsmith + _, err = client.Logical().Write("identity/entity-alias/id/"+aliasID2, map[string]interface{}{ + "name": "bsmith", + }) + if err == nil { + t.Fatal("expected rename over existing entity to fail") + } +} diff --git a/vault/identity_store_aliases.go b/vault/identity_store_aliases.go index db827a33c..54e4ac121 100644 --- a/vault/identity_store_aliases.go +++ b/vault/identity_store_aliases.go @@ -334,7 +334,7 @@ func (i *IdentityStore) handleAliasUpdate(ctx context.Context, req *logical.Requ } // Bail unless it's just a case change - if existingAlias != nil && !strings.EqualFold(existingAlias.Name, name) { + if existingAlias != nil && existingAlias.ID != alias.ID { return logical.ErrorResponse("alias with combination of mount accessor and name already exists"), nil } @@ -369,7 +369,7 @@ func (i *IdentityStore) handleAliasUpdate(ctx context.Context, req *logical.Requ return logical.ErrorResponse("given 'canonical_id' associated with entity in a different namespace from the alias"), logical.ErrPermissionDenied } - // Update the canonical ID value and move it from the current enitity to the new one + // Update the canonical ID value and move it from the current entity to the new one alias.CanonicalID = newEntity.ID newEntity.Aliases = append(newEntity.Aliases, alias) for aliasIndex, item := range currentEntity.Aliases {