Skip to content

Commit 3376494

Browse files
EmyrkEmyrk
committed
fixup tests, account for existing groups
1 parent ea6d61b commit 3376494

File tree

4 files changed

+106
-30
lines changed

4 files changed

+106
-30
lines changed

‎coderd/database/queries.sql.go

+6-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/groups.sql

+4
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,10 @@ WHERE
5252
)
5353
ELSE true
5454
END
55+
AND CASE WHEN array_length(@group_names :: text[], 1) > 0 THEN
56+
name = ANY(@group_names)
57+
ELSE true
58+
END
5559
;
5660

5761
-- name: InsertGroup :one

‎coderd/idpsync/group.go

+19-1
Original file line numberDiff line numberDiff line change
@@ -244,16 +244,34 @@ func (s GroupSyncSettings) ParseClaims(mergedClaims jwt.MapClaims) ([]ExpectedGr
244244

245245
func (s GroupSyncSettings) HandleMissingGroups(ctx context.Context, tx database.Store, orgID uuid.UUID, add []ExpectedGroup) ([]uuid.UUID, error) {
246246
if !s.AutoCreateMissingGroups {
247-
// Remove all groups that are missing, they will not be created
247+
// construct the list of groups to search by name to see if they exist.
248+
var lookups []string
248249
filter := make([]uuid.UUID, 0)
249250
for _, expected := range add {
250251
if expected.GroupID != nil {
251252
filter = append(filter, *expected.GroupID)
253+
} else if expected.GroupName != nil {
254+
lookups = append(lookups, *expected.GroupName)
255+
}
256+
}
257+
258+
if len(lookups) > 0 {
259+
newGroups, err := tx.GetGroups(ctx, database.GetGroupsParams{
260+
OrganizationID: uuid.UUID{},
261+
HasMemberID: uuid.UUID{},
262+
GroupNames: lookups,
263+
})
264+
if err != nil {
265+
return nil, xerrors.Errorf("get groups by names: %w", err)
266+
}
267+
for _, g := range newGroups {
268+
filter = append(filter, g.Group.ID)
252269
}
253270
}
254271

255272
return filter, nil
256273
}
274+
257275
// All expected that are missing IDs means the group does not exist
258276
// in the database. Either remove them, or create them if auto create is
259277
// turned on.

‎coderd/idpsync/group_test.go

+77-28
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"github.com/golang-jwt/jwt/v4"
99
"github.com/google/uuid"
1010
"github.com/stretchr/testify/require"
11+
"golang.org/x/exp/slices"
1112

1213
"cdr.dev/slog/sloggers/slogtest"
1314
"github.com/coder/coder/v2/coderd/coderdtest"
@@ -152,9 +153,26 @@ func TestGroupSyncTable(t *testing.T) {
152153
AutoCreateMissingGroups: true,
153154
},
154155
Groups: map[uuid.UUID]bool{},
155-
ExpectedGroups: []uuid.UUID{
156-
ids.ID("create-bar"),
157-
ids.ID("create-baz"),
156+
ExpectedGroupNames: []string{
157+
"create-bar",
158+
"create-baz",
159+
},
160+
},
161+
{
162+
Name: "GroupNamesNoMapping",
163+
Settings: &idpsync.GroupSyncSettings{
164+
GroupField: "groups",
165+
RegexFilter: regexp.MustCompile(".*"),
166+
AutoCreateMissingGroups: false,
167+
},
168+
GroupNames: map[string]bool{
169+
"foo": false,
170+
"bar": false,
171+
"goob": true,
172+
},
173+
ExpectedGroupNames: []string{
174+
"foo",
175+
"bar",
158176
},
159177
},
160178
{
@@ -219,10 +237,12 @@ func SetupOrganization(t *testing.T, s *idpsync.AGPLIDPSync, db database.Store,
219237
org := dbgen.Organization(t, db, database.Organization{
220238
ID: def.OrgID,
221239
})
240+
_, err := db.InsertAllUsersGroup(context.Background(), org.ID)
241+
require.NoError(t, err, "Everyone group for an org")
222242

223243
manager := runtimeconfig.NewStoreManager(db)
224244
orgResolver := manager.Scoped(org.ID.String())
225-
err := s.Group.SetRuntimeValue(context.Background(), orgResolver, def.Settings)
245+
err = s.Group.SetRuntimeValue(context.Background(), orgResolver, def.Settings)
226246
require.NoError(t, err)
227247

228248
if !def.NotMember {
@@ -243,47 +263,76 @@ func SetupOrganization(t *testing.T, s *idpsync.AGPLIDPSync, db database.Store,
243263
})
244264
}
245265
}
266+
for groupName, in := range def.GroupNames {
267+
group := dbgen.Group(t, db, database.Group{
268+
Name: groupName,
269+
OrganizationID: org.ID,
270+
})
271+
if in {
272+
dbgen.GroupMember(t, db, database.GroupMemberTable{
273+
UserID: user.ID,
274+
GroupID: group.ID,
275+
})
276+
}
277+
}
246278
}
247279

248280
type orgSetupDefinition struct {
249281
Name string
250282
OrgID uuid.UUID
251283
// True if the user is a member of the group
252-
Groups map[uuid.UUID]bool
253-
NotMember bool
284+
Groups map[uuid.UUID]bool
285+
GroupNames map[string]bool
286+
NotMember bool
254287

255-
Settings *idpsync.GroupSyncSettings
256-
ExpectedGroups []uuid.UUID
288+
Settings *idpsync.GroupSyncSettings
289+
ExpectedGroups []uuid.UUID
290+
ExpectedGroupNames []string
257291
}
258292

259293
func (o orgSetupDefinition) Assert(t *testing.T, orgID uuid.UUID, db database.Store, user database.User) {
260294
t.Helper()
261295

262-
t.Run(o.Name+"-Assert", func(t *testing.T) {
263-
ctx := context.Background()
296+
ctx := context.Background()
264297

265-
members, err := db.OrganizationMembers(ctx, database.OrganizationMembersParams{
266-
OrganizationID: orgID,
267-
UserID: user.ID,
268-
})
269-
require.NoError(t, err)
270-
if o.NotMember {
271-
require.Len(t, members, 0, "should not be a member")
272-
} else {
273-
require.Len(t, members, 1, "should be a member")
274-
}
298+
members, err := db.OrganizationMembers(ctx, database.OrganizationMembersParams{
299+
OrganizationID: orgID,
300+
UserID: user.ID,
301+
})
302+
require.NoError(t, err)
303+
if o.NotMember {
304+
require.Len(t, members, 0, "should not be a member")
305+
} else {
306+
require.Len(t, members, 1, "should be a member")
307+
}
308+
309+
userGroups, err := db.GetGroups(ctx, database.GetGroupsParams{
310+
OrganizationID: orgID,
311+
HasMemberID: user.ID,
312+
})
313+
require.NoError(t, err)
314+
if o.ExpectedGroups == nil {
315+
o.ExpectedGroups = make([]uuid.UUID, 0)
316+
}
317+
if len(o.ExpectedGroupNames) > 0 && len(o.ExpectedGroups) > 0 {
318+
t.Fatal("ExpectedGroups and ExpectedGroupNames are mutually exclusive")
319+
}
320+
321+
// Everyone groups mess up our asserts
322+
userGroups = slices.DeleteFunc(userGroups, func(row database.GetGroupsRow) bool {
323+
return row.Group.ID == row.Group.OrganizationID
324+
})
275325

276-
userGroups, err := db.GetGroups(ctx, database.GetGroupsParams{
277-
OrganizationID: orgID,
278-
HasMemberID: user.ID,
326+
if len(o.ExpectedGroupNames) > 0 {
327+
found := db2sdk.List(userGroups, func(g database.GetGroupsRow) string {
328+
return g.Group.Name
279329
})
280-
require.NoError(t, err)
281-
if o.ExpectedGroups == nil {
282-
o.ExpectedGroups = make([]uuid.UUID, 0)
283-
}
330+
require.ElementsMatch(t, o.ExpectedGroupNames, found, "user groups by name")
331+
} else {
332+
// Check by ID, recommended
284333
found := db2sdk.List(userGroups, func(g database.GetGroupsRow) uuid.UUID {
285334
return g.Group.ID
286335
})
287336
require.ElementsMatch(t, o.ExpectedGroups, found, "user groups")
288-
})
337+
}
289338
}

0 commit comments

Comments
 (0)