From d830bdd41695272612d745305f7935017ee50e1c Mon Sep 17 00:00:00 2001 From: Johan Brandhorst-Satzkorn Date: Thu, 14 Nov 2024 15:47:08 -0800 Subject: [PATCH] internal/auth/oidc: ensure all managed groups are tested on auth (#5242) Previously, we would list the managed groups with an implied filter of db.DefaultLimit (10,000), which would incorrectly remove a user from managed group memberships if there were more than 10,000 managed groups in an auth method. Explicitly set the list limit to unlimited to ensure all managed groups are updated appropriately. --- internal/auth/oidc/repository_managed_group_members.go | 4 ++-- internal/auth/oidc/service_callback.go | 2 +- internal/auth/oidc/service_callback_test.go | 7 ++++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/internal/auth/oidc/repository_managed_group_members.go b/internal/auth/oidc/repository_managed_group_members.go index 96e04b60b..6c6541319 100644 --- a/internal/auth/oidc/repository_managed_group_members.go +++ b/internal/auth/oidc/repository_managed_group_members.go @@ -111,7 +111,7 @@ func (r *Repository) SetManagedGroupMemberships(ctx context.Context, am *AuthMet msgs = append(msgs, &mgOplogMsg) } - currentMemberships, err = r.ListManagedGroupMembershipsByMember(ctx, acct.PublicId, WithReader(reader)) + currentMemberships, err = r.ListManagedGroupMembershipsByMember(ctx, acct.PublicId, WithReader(reader), WithLimit(-1)) if err != nil { return errors.Wrap(ctx, err, op, errors.WithMsg("unable to retrieve current managed group memberships before deletion")) } @@ -181,7 +181,7 @@ func (r *Repository) SetManagedGroupMemberships(ctx context.Context, am *AuthMet } } - currentMemberships, err = r.ListManagedGroupMembershipsByMember(ctx, acct.PublicId, WithReader(reader)) + currentMemberships, err = r.ListManagedGroupMembershipsByMember(ctx, acct.PublicId, WithReader(reader), WithLimit(-1)) if err != nil { return errors.Wrap(ctx, err, op, errors.WithMsg("unable to retrieve current managed group memberships after set")) } diff --git a/internal/auth/oidc/service_callback.go b/internal/auth/oidc/service_callback.go index 505c252ae..4bd62b07d 100644 --- a/internal/auth/oidc/service_callback.go +++ b/internal/auth/oidc/service_callback.go @@ -193,7 +193,7 @@ func Callback( } // Get the set of all managed groups so we can filter - mgs, _, err := r.ListManagedGroups(ctx, am.GetPublicId()) + mgs, _, err := r.ListManagedGroups(ctx, am.GetPublicId(), WithLimit(-1)) if err != nil { return "", errors.Wrap(ctx, err, op) } diff --git a/internal/auth/oidc/service_callback_test.go b/internal/auth/oidc/service_callback_test.go index e5b088aea..f9a321995 100644 --- a/internal/auth/oidc/service_callback_test.go +++ b/internal/auth/oidc/service_callback_test.go @@ -675,7 +675,8 @@ func Test_ManagedGroupFiltering(t *testing.T) { return iam.NewRepository(ctx, rw, rw, kmsCache) } repoFn := func() (*Repository, error) { - return NewRepository(ctx, rw, rw, kmsCache) + // Set a low limit to test that the managed group listing overrides the limit + return NewRepository(ctx, rw, rw, kmsCache, WithLimit(1)) } atRepoFn := func() (*authtoken.Repository, error) { return authtoken.NewRepository(ctx, rw, rw, kmsCache) @@ -819,7 +820,7 @@ func Test_ManagedGroupFiltering(t *testing.T) { tp.SetExpectedState(state) // Set the filters on the MGs for this test. First we need to get the current versions. - currMgs, ttime, err := repo.ListManagedGroups(ctx, testAuthMethod.PublicId) + currMgs, ttime, err := repo.ListManagedGroups(ctx, testAuthMethod.PublicId, WithLimit(-1)) require.NoError(err) // Transaction timestamp should be within ~10 seconds of now assert.True(time.Now().Before(ttime.Add(10 * time.Second))) @@ -860,7 +861,7 @@ func Test_ManagedGroupFiltering(t *testing.T) { assert.Contains(key.(map[string]any)["payload"], "auth_token_end") } // Ensure that we get the expected groups - memberships, err := repo.ListManagedGroupMembershipsByMember(ctx, account.PublicId) + memberships, err := repo.ListManagedGroupMembershipsByMember(ctx, account.PublicId, WithLimit(-1)) require.NoError(err) assert.Equal(len(tt.matchingMgs), len(memberships)) var matchingIds []string