-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Disable identity for local mounts #4407
Changes from 3 commits
149d0da
16f5c91
9e56ee3
4ec5bfe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
package vault_test | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/hashicorp/vault/api" | ||
vaulthttp "github.com/hashicorp/vault/http" | ||
"github.com/hashicorp/vault/logical" | ||
"github.com/hashicorp/vault/vault" | ||
|
||
credLdap "github.com/hashicorp/vault/builtin/credential/ldap" | ||
) | ||
|
||
func TestIdentityStore_EntityAliasLocalMount(t *testing.T) { | ||
coreConfig := &vault.CoreConfig{ | ||
CredentialBackends: map[string]logical.Factory{ | ||
"ldap": credLdap.Factory, | ||
}, | ||
} | ||
cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ | ||
HandlerFunc: vaulthttp.Handler, | ||
}) | ||
cluster.Start() | ||
defer cluster.Cleanup() | ||
|
||
core := cluster.Cores[0].Core | ||
vault.TestWaitActive(t, core) | ||
client := cluster.Cores[0].Client | ||
|
||
// Create a local auth mount | ||
err := client.Sys().EnableAuthWithOptions("ldap", &api.EnableAuthOptions{ | ||
Type: "ldap", | ||
Local: true, | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
// Extract out the mount accessor for LDAP auth | ||
auths, err := client.Sys().ListAuth() | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
ldapMountAccessor := auths["ldap/"].Accessor | ||
|
||
// Create an entity | ||
secret, err := client.Logical().Write("identity/entity", nil) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
entityID := secret.Data["id"].(string) | ||
|
||
// Attempt to create an entity alias against a local mount should fail | ||
secret, err = client.Logical().Write("identity/entity-alias", map[string]interface{}{ | ||
"name": "testuser", | ||
"mount_accessor": ldapMountAccessor, | ||
"canonical_id": entityID, | ||
}) | ||
if err == nil { | ||
t.Fatalf("expected error since mount is local") | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
package vault_test | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/hashicorp/vault/api" | ||
vaulthttp "github.com/hashicorp/vault/http" | ||
"github.com/hashicorp/vault/logical" | ||
"github.com/hashicorp/vault/vault" | ||
|
||
credLdap "github.com/hashicorp/vault/builtin/credential/ldap" | ||
) | ||
|
||
func TestIdentityStore_GroupAliasLocalMount(t *testing.T) { | ||
coreConfig := &vault.CoreConfig{ | ||
CredentialBackends: map[string]logical.Factory{ | ||
"ldap": credLdap.Factory, | ||
}, | ||
} | ||
cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ | ||
HandlerFunc: vaulthttp.Handler, | ||
}) | ||
cluster.Start() | ||
defer cluster.Cleanup() | ||
|
||
core := cluster.Cores[0].Core | ||
vault.TestWaitActive(t, core) | ||
client := cluster.Cores[0].Client | ||
|
||
// Create a local auth mount | ||
err := client.Sys().EnableAuthWithOptions("ldap", &api.EnableAuthOptions{ | ||
Type: "ldap", | ||
Local: true, | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
|
||
// Extract out the mount accessor for LDAP auth | ||
auths, err := client.Sys().ListAuth() | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
ldapMountAccessor := auths["ldap/"].Accessor | ||
|
||
// Create an external group | ||
secret, err := client.Logical().Write("identity/group", map[string]interface{}{ | ||
"type": "external", | ||
}) | ||
if err != nil { | ||
t.Fatal(err) | ||
} | ||
groupID := secret.Data["id"].(string) | ||
|
||
// Attempt to create a group alias against a local mount should fail | ||
secret, err = client.Logical().Write("identity/group-alias", map[string]interface{}{ | ||
"name": "testuser", | ||
"mount_accessor": ldapMountAccessor, | ||
"canonical_id": groupID, | ||
}) | ||
if err == nil { | ||
t.Fatalf("expected error since mount is local") | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -496,7 +496,9 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re | |
var entity *identity.Entity | ||
auth = resp.Auth | ||
|
||
if auth.Alias != nil { | ||
mEntry := c.router.MatchingMountEntry(req.Path) | ||
|
||
if auth.Alias != nil && mEntry != nil && !mEntry.Local { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're missing the auth/token/renew path earlier on, which can also end up updating memberships. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I left the renewal flow as it is to allow it to work if the tokens were created before this fix. The thought I had was to only disallow entity attachment for the newly created tokens. |
||
// Overwrite the mount type and mount path in the alias | ||
// information | ||
auth.Alias.MountType = req.MountType | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this happen later so that existing values are returned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It wouldn't matter. This function will not be called for the new tokens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I just figured from the name (Create or Fetch) that it could end up creating new entities...which it can... :-)