Skip to content

Commit b2cb9ba

Browse files
hc-github-team-secure-boundaryhc-github-team-secure-boundary
authored
Merge pull request #5248 from hashicorp/backport/jbrandhorst-more-limit-fixes/perfectly-gorgeous-pig
This pull request was automerged via backport-assistant
2 parents aca84e8 + d91d9cd commit b2cb9ba

File tree

4 files changed

+29
-15
lines changed

4 files changed

+29
-15
lines changed

‎internal/daemon/controller/handlers/accounts/account_service.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -606,7 +606,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (auth.Account, []st
606606
}
607607
return nil, nil, err
608608
}
609-
mgs, err := repo.ListManagedGroupMembershipsByMember(ctx, a.GetPublicId())
609+
mgs, err := repo.ListManagedGroupMembershipsByMember(ctx, a.GetPublicId(), oidc.WithLimit(-1))
610610
if err != nil {
611611
return nil, nil, err
612612
}
@@ -629,7 +629,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (auth.Account, []st
629629
}
630630
return nil, nil, err
631631
}
632-
mgs, err := repo.ListManagedGroupMembershipsByMember(ctx, a.GetPublicId())
632+
mgs, err := repo.ListManagedGroupMembershipsByMember(ctx, a.GetPublicId(), ldap.WithLimit(ctx, -1))
633633
if err != nil {
634634
return nil, nil, err
635635
}

‎internal/daemon/controller/handlers/accounts/account_service_test.go

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -134,13 +134,15 @@ func TestGet(t *testing.T) {
134134
return password.NewRepository(ctx, rw, rw, kmsCache)
135135
}
136136
oidcRepoFn := func() (*oidc.Repository, error) {
137-
return oidc.NewRepository(ctx, rw, rw, kmsCache)
137+
// Use a small limit to test that membership lookup is explicitly unlimited
138+
return oidc.NewRepository(ctx, rw, rw, kmsCache, oidc.WithLimit(1))
138139
}
139140
iamRepoFn := func() (*iam.Repository, error) {
140141
return iam.NewRepository(ctx, rw, rw, kmsCache)
141142
}
142143
ldapRepoFn := func() (*ldap.Repository, error) {
143-
return ldap.NewRepository(ctx, rw, rw, kmsCache)
144+
// Use a small limit to test that membership lookup is explicitly unlimited
145+
return ldap.NewRepository(ctx, rw, rw, kmsCache, ldap.WithLimit(ctx, 1))
144146
}
145147

146148
s, err := accounts.NewService(ctx, pwRepoFn, oidcRepoFn, ldapRepoFn, 1000)
@@ -175,9 +177,10 @@ func TestGet(t *testing.T) {
175177
oidc.WithApiUrl(oidc.TestConvertToUrls(t, "https://www.alice.com/callback")[0]),
176178
)
177179
oidcA := oidc.TestAccount(t, conn, oidcAm, "test-subject")
178-
// Create a managed group that will always match, so we can test that it is
180+
// Create some managed groups that will always match, so we can test that it is
179181
// returned in results
180182
mg := oidc.TestManagedGroup(t, conn, oidcAm, `"/token/sub" matches ".*"`)
183+
mg2 := oidc.TestManagedGroup(t, conn, oidcAm, `"/token/sub" matches ".*"`)
181184
oidcWireAccount := pb.Account{
182185
Id: oidcA.GetPublicId(),
183186
AuthMethodId: oidcA.GetAuthMethodId(),
@@ -193,7 +196,7 @@ func TestGet(t *testing.T) {
193196
},
194197
},
195198
AuthorizedActions: oidcAuthorizedActions,
196-
ManagedGroupIds: []string{mg.GetPublicId()},
199+
ManagedGroupIds: []string{mg.GetPublicId(), mg2.GetPublicId()},
197200
}
198201

199202
ldapAm := ldap.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, []string{"ldaps://ldap1"})
@@ -204,6 +207,7 @@ func TestGet(t *testing.T) {
204207
ldap.WithDn(ctx, "test-dn"),
205208
)
206209
ldapMg := ldap.TestManagedGroup(t, conn, ldapAm, []string{"admin"})
210+
ldapMg2 := ldap.TestManagedGroup(t, conn, ldapAm, []string{"admin"})
207211
ldapWireAccount := pb.Account{
208212
Id: ldapAcct.GetPublicId(),
209213
AuthMethodId: ldapAm.GetPublicId(),
@@ -222,7 +226,7 @@ func TestGet(t *testing.T) {
222226
},
223227
Type: ldap.Subtype.String(),
224228
AuthorizedActions: ldapAuthorizedActions,
225-
ManagedGroupIds: []string{ldapMg.GetPublicId()},
229+
ManagedGroupIds: []string{ldapMg.GetPublicId(), ldapMg2.GetPublicId()},
226230
}
227231

228232
cases := []struct {
@@ -289,12 +293,14 @@ func TestGet(t *testing.T) {
289293

290294
if globals.ResourceInfoFromPrefix(tc.req.Id).Subtype == oidc.Subtype {
291295
// Set up managed groups before getting. First get the current
292-
// managed group to make sure we have the right version.
296+
// managed groups to make sure we have the right version.
293297
oidcRepo, err := oidcRepoFn()
294298
require.NoError(err)
295299
currMg, err := oidcRepo.LookupManagedGroup(ctx, mg.GetPublicId())
296300
require.NoError(err)
297-
_, _, err = oidcRepo.SetManagedGroupMemberships(ctx, oidcAm, oidcA, []*oidc.ManagedGroup{currMg})
301+
currMg2, err := oidcRepo.LookupManagedGroup(ctx, mg2.GetPublicId())
302+
require.NoError(err)
303+
_, _, err = oidcRepo.SetManagedGroupMemberships(ctx, oidcAm, oidcA, []*oidc.ManagedGroup{currMg, currMg2})
298304
require.NoError(err)
299305
}
300306

‎internal/daemon/controller/handlers/managed_groups/managed_group_service.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (auth.ManagedGroup,
446446
}
447447
return nil, nil, err
448448
}
449-
ids, err := repo.ListManagedGroupMembershipsByGroup(ctx, mg.GetPublicId())
449+
ids, err := repo.ListManagedGroupMembershipsByGroup(ctx, mg.GetPublicId(), oidc.WithLimit(-1))
450450
if err != nil {
451451
return nil, nil, err
452452
}
@@ -469,7 +469,7 @@ func (s Service) getFromRepo(ctx context.Context, id string) (auth.ManagedGroup,
469469
}
470470
return nil, nil, err
471471
}
472-
ids, err := repo.ListManagedGroupMembershipsByGroup(ctx, mg.GetPublicId())
472+
ids, err := repo.ListManagedGroupMembershipsByGroup(ctx, mg.GetPublicId(), ldap.WithLimit(ctx, -1))
473473
if err != nil {
474474
return nil, nil, err
475475
}

‎internal/daemon/controller/handlers/managed_groups/managed_group_service_test.go

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,15 @@ func TestGet(t *testing.T) {
118118
wrap := db.TestWrapper(t)
119119
kmsCache := kms.TestKms(t, conn, wrap)
120120
oidcRepoFn := func() (*oidc.Repository, error) {
121-
return oidc.NewRepository(ctx, rw, rw, kmsCache)
121+
// Use a small limit to test that membership lookup is explicitly unlimited
122+
return oidc.NewRepository(ctx, rw, rw, kmsCache, oidc.WithLimit(1))
122123
}
123124
iamRepoFn := func() (*iam.Repository, error) {
124125
return iam.NewRepository(ctx, rw, rw, kmsCache)
125126
}
126127
ldapRepoFn := func() (*ldap.Repository, error) {
127-
return ldap.NewRepository(ctx, rw, rw, kmsCache)
128+
// Use a small limit to test that membership lookup is explicitly unlimited
129+
return ldap.NewRepository(ctx, rw, rw, kmsCache, ldap.WithLimit(ctx, 1))
128130
}
129131

130132
s, err := managed_groups.NewService(ctx, oidcRepoFn, ldapRepoFn, 1000)
@@ -142,6 +144,7 @@ func TestGet(t *testing.T) {
142144
oidc.WithApiUrl(oidc.TestConvertToUrls(t, "https://www.alice.com/callback")[0]),
143145
)
144146
oidcA := oidc.TestAccount(t, conn, oidcAm, "test-subject")
147+
oidcB := oidc.TestAccount(t, conn, oidcAm, "test-subject-2")
145148
omg := oidc.TestManagedGroup(t, conn, oidcAm, oidc.TestFakeManagedGroupFilter)
146149

147150
// Set up managed group before getting. First get the current
@@ -153,6 +156,10 @@ func TestGet(t *testing.T) {
153156
require.NoError(t, err)
154157
_, _, err = oidcRepo.SetManagedGroupMemberships(ctx, oidcAm, oidcA, []*oidc.ManagedGroup{currMg})
155158
require.NoError(t, err)
159+
currMg, err = oidcRepo.LookupManagedGroup(ctx, omg.GetPublicId())
160+
require.NoError(t, err)
161+
_, _, err = oidcRepo.SetManagedGroupMemberships(ctx, oidcAm, oidcB, []*oidc.ManagedGroup{currMg})
162+
require.NoError(t, err)
156163
// Fetch the group once more to get the updated time
157164
currMg, err = oidcRepo.LookupManagedGroup(ctx, omg.GetPublicId())
158165
require.NoError(t, err)
@@ -171,11 +178,12 @@ func TestGet(t *testing.T) {
171178
},
172179
},
173180
AuthorizedActions: oidcAuthorizedActions,
174-
MemberIds: []string{oidcA.GetPublicId()},
181+
MemberIds: []string{oidcA.GetPublicId(), oidcB.GetPublicId()},
175182
}
176183

177184
ldapAm := ldap.TestAuthMethod(t, conn, databaseWrapper, org.PublicId, []string{"ldaps://ldap1"})
178185
ldapAcct := ldap.TestAccount(t, conn, ldapAm, "test-login-name", ldap.WithMemberOfGroups(ctx, "admin"))
186+
ldapAcct2 := ldap.TestAccount(t, conn, ldapAm, "test-login-name-2", ldap.WithMemberOfGroups(ctx, "admin"))
179187
ldapMg := ldap.TestManagedGroup(t, conn, ldapAm, []string{"admin"})
180188
ldapWireManagedGroup := pb.ManagedGroup{
181189
Id: ldapMg.GetPublicId(),
@@ -191,7 +199,7 @@ func TestGet(t *testing.T) {
191199
},
192200
},
193201
AuthorizedActions: ldapAuthorizedActions,
194-
MemberIds: []string{ldapAcct.GetPublicId()},
202+
MemberIds: []string{ldapAcct.GetPublicId(), ldapAcct2.GetPublicId()},
195203
}
196204

197205
cases := []struct {

0 commit comments

Comments
 (0)