Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fail alias rename if the resulting (name,accessor) exists already #12473

Merged
merged 2 commits into from Sep 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions changelog/12473.txt
@@ -0,0 +1,3 @@
```release-note:bug
identity: Fail alias rename if the resulting (name,accessor) exists already
```
88 changes: 85 additions & 3 deletions vault/external_tests/identity/aliases_test.go
Expand Up @@ -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) {
Expand Down Expand Up @@ -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")
}
}
4 changes: 2 additions & 2 deletions vault/identity_store_aliases.go
Expand Up @@ -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
}

Expand Down Expand Up @@ -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 {
Expand Down