From 7ecbf052a98e2f5cc9df151ad0dd4aacc3de3b39 Mon Sep 17 00:00:00 2001
From: Nick Cabatoff <ncabatoff@hashicorp.com>
Date: Tue, 21 Sep 2021 08:19:44 -0400
Subject: [PATCH] Fail alias rename if the resulting (name,accessor) exists
 already (#12473)

---
 changelog/12473.txt                           |  3 +
 vault/external_tests/identity/aliases_test.go | 88 ++++++++++++++++++-
 vault/identity_store_aliases.go               |  4 +-
 3 files changed, 90 insertions(+), 5 deletions(-)
 create mode 100644 changelog/12473.txt

diff --git a/changelog/12473.txt b/changelog/12473.txt
new file mode 100644
index 0000000000000..3a0ecddb5e710
--- /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 be00122b90eb6..162672e92229b 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 f51f2d2e67f9a..64804d0c1269e 100644
--- a/vault/identity_store_aliases.go
+++ b/vault/identity_store_aliases.go
@@ -297,7 +297,7 @@ func (i *IdentityStore) handleAliasUpdate(ctx context.Context, req *logical.Requ
 			return nil, err
 		}
 		// 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
 		}
 
@@ -332,7 +332,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 {