diff --git a/changelog/11506.txt b/changelog/11506.txt new file mode 100644 index 000000000000..15ab6f327705 --- /dev/null +++ b/changelog/11506.txt @@ -0,0 +1,3 @@ +```release-note:bug +identity: Use correct mount accessor when refreshing external group memberships. +``` diff --git a/vault/expiration.go b/vault/expiration.go index 0f8dcab5f8bf..360b8ca07f50 100644 --- a/vault/expiration.go +++ b/vault/expiration.go @@ -1301,7 +1301,11 @@ func (m *ExpirationManager) RenewToken(ctx context.Context, req *logical.Request // Refresh groups if resp.Auth.EntityID != "" && m.core.identityStore != nil { - validAliases, err := m.core.identityStore.refreshExternalGroupMembershipsByEntityID(ctx, resp.Auth.EntityID, resp.Auth.GroupAliases) + mountAccessor := "" + if resp.Auth.Alias != nil { + mountAccessor = resp.Auth.Alias.MountAccessor + } + validAliases, err := m.core.identityStore.refreshExternalGroupMembershipsByEntityID(ctx, resp.Auth.EntityID, resp.Auth.GroupAliases, mountAccessor) if err != nil { return nil, err } diff --git a/vault/external_tests/identity/identity_test.go b/vault/external_tests/identity/identity_test.go index 0ea5dd8f2326..553fe2edbc81 100644 --- a/vault/external_tests/identity/identity_test.go +++ b/vault/external_tests/identity/identity_test.go @@ -4,19 +4,141 @@ import ( "fmt" "testing" + "github.com/hashicorp/vault/api" + "github.com/hashicorp/vault/sdk/helper/ldaputil" + "github.com/hashicorp/vault/sdk/helper/strutil" + "github.com/hashicorp/vault/sdk/logical" + + "github.com/stretchr/testify/require" + + "github.com/hashicorp/vault/helper/testhelpers/teststorage" + "github.com/go-ldap/ldap/v3" log "github.com/hashicorp/go-hclog" - "github.com/hashicorp/vault/api" ldapcred "github.com/hashicorp/vault/builtin/credential/ldap" "github.com/hashicorp/vault/helper/namespace" ldaphelper "github.com/hashicorp/vault/helper/testhelpers/ldap" vaulthttp "github.com/hashicorp/vault/http" - "github.com/hashicorp/vault/sdk/helper/ldaputil" - "github.com/hashicorp/vault/sdk/helper/strutil" - "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/vault" ) +func TestIdentityStore_ExternalGroupMemberships_DifferentMounts(t *testing.T) { + coreConfig := &vault.CoreConfig{ + CredentialBackends: map[string]logical.Factory{ + "ldap": ldapcred.Factory, + }, + } + conf, opts := teststorage.ClusterSetup(coreConfig, nil, nil) + cluster := vault.NewTestCluster(t, conf, opts) + cluster.Start() + defer cluster.Cleanup() + + core := cluster.Cores[0].Core + client := cluster.Cores[0].Client + vault.TestWaitActive(t, core) + + // Create a entity + secret, err := client.Logical().Write("identity/entity", map[string]interface{}{ + "name": "testentityname", + }) + require.NoError(t, err) + entityID := secret.Data["id"].(string) + + cleanup, config1 := ldaphelper.PrepareTestContainer(t, "latest") + defer cleanup() + + cleanup2, config2 := ldaphelper.PrepareTestContainer(t, "latest") + defer cleanup2() + + setupFunc := func(path string, cfg *ldaputil.ConfigEntry) string { + // Create an external group + resp, err := client.Logical().Write("identity/group", map[string]interface{}{ + "type": "external", + "name": path + "ldap_admin_staff", + "policies": []string{"admin-policy"}, + }) + require.NoError(t, err) + require.NotNil(t, resp) + require.NotNil(t, resp.Data) + groupID := resp.Data["id"].(string) + + // Enable LDAP mount in Vault + err = client.Sys().EnableAuthWithOptions(path, &api.EnableAuthOptions{ + Type: "ldap", + }) + require.NoError(t, err) + + // Take out its accessor + auth, err := client.Sys().ListAuth() + require.NoError(t, err) + accessor := auth[path+"/"].Accessor + require.NotEmpty(t, accessor) + + // Create an external group alias + resp, err = client.Logical().Write("identity/group-alias", map[string]interface{}{ + "name": "admin_staff", + "canonical_id": groupID, + "mount_accessor": accessor, + }) + + // Create a user in Vault + _, err = client.Logical().Write("auth/"+path+"/users/hermes conrad", map[string]interface{}{ + "password": "hermes", + }) + require.NoError(t, err) + + // Create an entity alias + client.Logical().Write("identity/entity-alias", map[string]interface{}{ + "name": "hermes conrad", + "canonical_id": entityID, + "mount_accessor": accessor, + }) + + // Configure LDAP auth + secret, err = client.Logical().Write("auth/"+path+"/config", map[string]interface{}{ + "url": cfg.Url, + "userattr": cfg.UserAttr, + "userdn": cfg.UserDN, + "groupdn": cfg.GroupDN, + "groupattr": cfg.GroupAttr, + "binddn": cfg.BindDN, + "bindpass": cfg.BindPassword, + }) + require.NoError(t, err) + + secret, err = client.Logical().Write("auth/"+path+"/login/hermes conrad", map[string]interface{}{ + "password": "hermes", + }) + require.NoError(t, err) + + policies, err := secret.TokenPolicies() + require.NoError(t, err) + require.Contains(t, policies, "admin-policy") + + secret, err = client.Logical().Read("identity/group/id/" + groupID) + require.NoError(t, err) + require.Contains(t, secret.Data["member_entity_ids"], entityID) + + return groupID + } + groupID1 := setupFunc("ldap", config1) + groupID2 := setupFunc("ldap2", config2) + + // Remove hermes conrad from admin_staff group + removeLdapGroupMember(t, config1, "admin_staff", "hermes conrad") + secret, err = client.Logical().Write("auth/ldap/login/hermes conrad", map[string]interface{}{ + "password": "hermes", + }) + + secret, err = client.Logical().Read("identity/group/id/" + groupID1) + require.NoError(t, err) + require.NotContains(t, secret.Data["member_entity_ids"], entityID) + + secret, err = client.Logical().Read("identity/group/id/" + groupID2) + require.NoError(t, err) + require.Contains(t, secret.Data["member_entity_ids"], entityID) +} + func TestIdentityStore_Integ_GroupAliases(t *testing.T) { t.Parallel() diff --git a/vault/identity_store_util.go b/vault/identity_store_util.go index fdcfd9071975..46b90bd82c15 100644 --- a/vault/identity_store_util.go +++ b/vault/identity_store_util.go @@ -1886,7 +1886,7 @@ func (i *IdentityStore) MemDBGroupByAliasID(aliasID string, clone bool) (*identi return i.MemDBGroupByAliasIDInTxn(txn, aliasID, clone) } -func (i *IdentityStore) refreshExternalGroupMembershipsByEntityID(ctx context.Context, entityID string, groupAliases []*logical.Alias) ([]*logical.Alias, error) { +func (i *IdentityStore) refreshExternalGroupMembershipsByEntityID(ctx context.Context, entityID string, groupAliases []*logical.Alias, mountAccessor string) ([]*logical.Alias, error) { defer metrics.MeasureSince([]string{"identity", "refresh_external_groups"}, time.Now()) if entityID == "" { @@ -1908,11 +1908,6 @@ func (i *IdentityStore) refreshExternalGroupMembershipsByEntityID(ctx context.Co return false, nil, err } - mountAccessor := "" - if len(groupAliases) != 0 { - mountAccessor = groupAliases[0].MountAccessor - } - var newGroups []*identity.Group var validAliases []*logical.Alias for _, alias := range groupAliases { diff --git a/vault/request_handling.go b/vault/request_handling.go index 13c33ad7d72a..f29d53082dbc 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -1203,7 +1203,7 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re } auth.EntityID = entity.ID - validAliases, err := c.identityStore.refreshExternalGroupMembershipsByEntityID(ctx, auth.EntityID, auth.GroupAliases) + validAliases, err := c.identityStore.refreshExternalGroupMembershipsByEntityID(ctx, auth.EntityID, auth.GroupAliases, req.MountAccessor) if err != nil { return nil, nil, err }