Skip to content

Commit f4678c1

Browse files
EmyrkEmyrk
committed
work on unit test for role sync
1 parent 85ab7c2 commit f4678c1

File tree

3 files changed

+329
-44
lines changed

3 files changed

+329
-44
lines changed

‎coderd/idpsync/group_test.go

Lines changed: 118 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func TestGroupSyncTable(t *testing.T) {
8484
testCases := []orgSetupDefinition{
8585
{
8686
Name: "SwitchGroups",
87-
Settings: &idpsync.GroupSyncSettings{
87+
GroupSettings: &idpsync.GroupSyncSettings{
8888
Field: "groups",
8989
Mapping: map[string][]uuid.UUID{
9090
"foo": {ids.ID("sg-foo"), ids.ID("sg-foo-2")},
@@ -101,16 +101,18 @@ func TestGroupSyncTable(t *testing.T) {
101101
ids.ID("sg-bar"): false,
102102
ids.ID("sg-baz"): false,
103103
},
104-
ExpectedGroups: []uuid.UUID{
105-
ids.ID("sg-foo"),
106-
ids.ID("sg-foo-2"),
107-
ids.ID("sg-bar"),
108-
ids.ID("sg-baz"),
104+
assertGroups: &orgGroupAssert{
105+
ExpectedGroups: []uuid.UUID{
106+
ids.ID("sg-foo"),
107+
ids.ID("sg-foo-2"),
108+
ids.ID("sg-bar"),
109+
ids.ID("sg-baz"),
110+
},
109111
},
110112
},
111113
{
112114
Name: "StayInGroup",
113-
Settings: &idpsync.GroupSyncSettings{
115+
GroupSettings: &idpsync.GroupSyncSettings{
114116
Field: "groups",
115117
// Only match foo, so bar does not map
116118
RegexFilter: regexp.MustCompile("^foo$"),
@@ -124,13 +126,15 @@ func TestGroupSyncTable(t *testing.T) {
124126
ids.ID("gg-foo"): true,
125127
ids.ID("gg-bar"): false,
126128
},
127-
ExpectedGroups: []uuid.UUID{
128-
ids.ID("gg-foo"),
129+
assertGroups: &orgGroupAssert{
130+
ExpectedGroups: []uuid.UUID{
131+
ids.ID("gg-foo"),
132+
},
129133
},
130134
},
131135
{
132136
Name: "UserJoinsGroups",
133-
Settings: &idpsync.GroupSyncSettings{
137+
GroupSettings: &idpsync.GroupSyncSettings{
134138
Field: "groups",
135139
Mapping: map[string][]uuid.UUID{
136140
"foo": {ids.ID("ng-foo"), uuid.New()},
@@ -144,29 +148,33 @@ func TestGroupSyncTable(t *testing.T) {
144148
ids.ID("ng-bar-2"): false,
145149
ids.ID("ng-baz"): false,
146150
},
147-
ExpectedGroups: []uuid.UUID{
148-
ids.ID("ng-foo"),
149-
ids.ID("ng-bar"),
150-
ids.ID("ng-bar-2"),
151-
ids.ID("ng-baz"),
151+
assertGroups: &orgGroupAssert{
152+
ExpectedGroups: []uuid.UUID{
153+
ids.ID("ng-foo"),
154+
ids.ID("ng-bar"),
155+
ids.ID("ng-bar-2"),
156+
ids.ID("ng-baz"),
157+
},
152158
},
153159
},
154160
{
155161
Name: "CreateGroups",
156-
Settings: &idpsync.GroupSyncSettings{
162+
GroupSettings: &idpsync.GroupSyncSettings{
157163
Field: "groups",
158164
RegexFilter: regexp.MustCompile("^create"),
159165
AutoCreateMissing: true,
160166
},
161167
Groups: map[uuid.UUID]bool{},
162-
ExpectedGroupNames: []string{
163-
"create-bar",
164-
"create-baz",
168+
assertGroups: &orgGroupAssert{
169+
ExpectedGroupNames: []string{
170+
"create-bar",
171+
"create-baz",
172+
},
165173
},
166174
},
167175
{
168176
Name: "GroupNamesNoMapping",
169-
Settings: &idpsync.GroupSyncSettings{
177+
GroupSettings: &idpsync.GroupSyncSettings{
170178
Field: "groups",
171179
RegexFilter: regexp.MustCompile(".*"),
172180
AutoCreateMissing: false,
@@ -176,14 +184,16 @@ func TestGroupSyncTable(t *testing.T) {
176184
"bar": false,
177185
"goob": true,
178186
},
179-
ExpectedGroupNames: []string{
180-
"foo",
181-
"bar",
187+
assertGroups: &orgGroupAssert{
188+
ExpectedGroupNames: []string{
189+
"foo",
190+
"bar",
191+
},
182192
},
183193
},
184194
{
185195
Name: "NoUser",
186-
Settings: &idpsync.GroupSyncSettings{
196+
GroupSettings: &idpsync.GroupSyncSettings{
187197
Field: "groups",
188198
Mapping: map[string][]uuid.UUID{
189199
// Extra ID that does not map to a group
@@ -199,13 +209,13 @@ func TestGroupSyncTable(t *testing.T) {
199209
},
200210
},
201211
{
202-
Name: "NoSettingsNoUser",
203-
Settings: nil,
204-
Groups: map[uuid.UUID]bool{},
212+
Name: "NoSettingsNoUser",
213+
GroupSettings: nil,
214+
Groups: map[uuid.UUID]bool{},
205215
},
206216
{
207217
Name: "LegacyMapping",
208-
Settings: &idpsync.GroupSyncSettings{
218+
GroupSettings: &idpsync.GroupSyncSettings{
209219
Field: "groups",
210220
RegexFilter: regexp.MustCompile("^legacy"),
211221
LegacyNameMapping: map[string]string{
@@ -223,9 +233,11 @@ func TestGroupSyncTable(t *testing.T) {
223233
"extra": true,
224234
"legacy-bop": true,
225235
},
226-
ExpectedGroupNames: []string{
227-
"legacy-bar",
228-
"legacy-foo",
236+
assertGroups: &orgGroupAssert{
237+
ExpectedGroupNames: []string{
238+
"legacy-bar",
239+
"legacy-foo",
240+
},
229241
},
230242
},
231243
}
@@ -299,9 +311,10 @@ func TestGroupSyncTable(t *testing.T) {
299311
"random": true,
300312
},
301313
// No settings, because they come from the deployment values
302-
Settings: nil,
303-
ExpectedGroups: nil,
304-
ExpectedGroupNames: []string{"legacy-foo", "legacy-baz", "legacy-bar"},
314+
GroupSettings: nil,
315+
assertGroups: &orgGroupAssert{
316+
ExpectedGroupNames: []string{"legacy-foo", "legacy-baz", "legacy-bar"},
317+
},
305318
}
306319

307320
//nolint:gocritic // testing
@@ -373,16 +386,18 @@ func TestSyncDisabled(t *testing.T) {
373386
ids.ID("baz"): false,
374387
ids.ID("bop"): false,
375388
},
376-
Settings: &idpsync.GroupSyncSettings{
389+
GroupSettings: &idpsync.GroupSyncSettings{
377390
Field: "groups",
378391
Mapping: map[string][]uuid.UUID{
379392
"foo": {ids.ID("foo")},
380393
"baz": {ids.ID("baz")},
381394
},
382395
},
383-
ExpectedGroups: []uuid.UUID{
384-
ids.ID("foo"),
385-
ids.ID("bar"),
396+
assertGroups: &orgGroupAssert{
397+
ExpectedGroups: []uuid.UUID{
398+
ids.ID("foo"),
399+
ids.ID("bar"),
400+
},
386401
},
387402
}
388403

@@ -717,7 +732,10 @@ func SetupOrganization(t *testing.T, s *idpsync.AGPLIDPSync, db database.Store,
717732

718733
manager := runtimeconfig.NewManager()
719734
orgResolver := manager.OrganizationResolver(db, org.ID)
720-
err = s.Group.SetRuntimeValue(context.Background(), orgResolver, def.Settings)
735+
err = s.Group.SetRuntimeValue(context.Background(), orgResolver, def.GroupSettings)
736+
require.NoError(t, err)
737+
738+
err = s.Role.SetRuntimeValue(context.Background(), orgResolver, def.RoleSettings)
721739
require.NoError(t, err)
722740

723741
if !def.NotMember {
@@ -726,6 +744,14 @@ func SetupOrganization(t *testing.T, s *idpsync.AGPLIDPSync, db database.Store,
726744
OrganizationID: org.ID,
727745
})
728746
}
747+
if len(def.OrganizationRoles) > 0 {
748+
_, err := db.UpdateMemberRoles(context.Background(), database.UpdateMemberRolesParams{
749+
GrantedRoles: def.OrganizationRoles,
750+
UserID: user.ID,
751+
OrgID: org.ID,
752+
})
753+
require.NoError(t, err)
754+
}
729755
for groupID, in := range def.Groups {
730756
dbgen.Group(t, db, database.Group{
731757
ID: groupID,
@@ -757,9 +783,23 @@ type orgSetupDefinition struct {
757783
// True if the user is a member of the group
758784
Groups map[uuid.UUID]bool
759785
GroupNames map[string]bool
760-
NotMember bool
761786

762-
Settings *idpsync.GroupSyncSettings
787+
OrganizationRoles []string
788+
// NotMember if true will ensure the user is not a member of the organization.
789+
NotMember bool
790+
791+
GroupSettings *idpsync.GroupSyncSettings
792+
RoleSettings *idpsync.RoleSyncSettings
793+
794+
assertGroups *orgGroupAssert
795+
assertRoles *orgRoleAssert
796+
}
797+
798+
type orgRoleAssert struct {
799+
ExpectedOrgRoles []string
800+
}
801+
802+
type orgGroupAssert struct {
763803
ExpectedGroups []uuid.UUID
764804
ExpectedGroupNames []string
765805
}
@@ -780,6 +820,24 @@ func (o orgSetupDefinition) Assert(t *testing.T, orgID uuid.UUID, db database.St
780820
require.Len(t, members, 1, "should be a member")
781821
}
782822

823+
if o.assertGroups != nil {
824+
o.assertGroups.Assert(t, orgID, db, user)
825+
}
826+
if o.assertRoles != nil {
827+
o.assertRoles.Assert(t, orgID, db, o.NotMember, user)
828+
}
829+
830+
if o.assertGroups == nil && o.assertRoles == nil {
831+
t.Errorf("no group or role asserts present, must have at least one")
832+
t.FailNow()
833+
}
834+
}
835+
836+
func (o orgGroupAssert) Assert(t *testing.T, orgID uuid.UUID, db database.Store, user database.User) {
837+
t.Helper()
838+
839+
ctx := context.Background()
840+
783841
userGroups, err := db.GetGroups(ctx, database.GetGroupsParams{
784842
OrganizationID: orgID,
785843
HasMemberID: user.ID,
@@ -812,3 +870,22 @@ func (o orgSetupDefinition) Assert(t *testing.T, orgID uuid.UUID, db database.St
812870
require.Len(t, o.ExpectedGroupNames, 0, "ExpectedGroupNames should be empty")
813871
}
814872
}
873+
874+
func (o orgRoleAssert) Assert(t *testing.T, orgID uuid.UUID, db database.Store, notMember bool, user database.User) {
875+
t.Helper()
876+
877+
ctx := context.Background()
878+
879+
members, err := db.OrganizationMembers(ctx, database.OrganizationMembersParams{
880+
OrganizationID: orgID,
881+
UserID: user.ID,
882+
})
883+
if notMember {
884+
require.ErrorIs(t, err, sql.ErrNoRows)
885+
return
886+
}
887+
require.NoError(t, err)
888+
require.Len(t, members, 1)
889+
member := members[0]
890+
require.ElementsMatch(t, member.OrganizationMember.Roles, o.ExpectedOrgRoles)
891+
}

‎coderd/idpsync/role.go

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -141,11 +141,18 @@ func (s AGPLIDPSync) SyncRoles(ctx context.Context, db database.Store, user data
141141
validExpected = append(validExpected, role.Name)
142142
}
143143
}
144-
// Always add the member role to the user.
145-
validExpected = append(validExpected, rbac.RoleOrgMember())
144+
// Ignore the implied member role
145+
validExpected = slices.DeleteFunc(validExpected, func(s string) bool {
146+
return s == rbac.RoleOrgMember()
147+
})
148+
149+
existingFound := existingRoles[orgID]
150+
existingFound = slices.DeleteFunc(existingFound, func(s string) bool {
151+
return s == rbac.RoleOrgMember()
152+
})
146153

147154
// Is there a difference between the expected roles and the existing roles?
148-
if !slices.Equal(existingRoles[orgID], validExpected) {
155+
if !slices.Equal(existingFound, validExpected) {
149156
_, err = tx.UpdateMemberRoles(ctx, database.UpdateMemberRolesParams{
150157
GrantedRoles: validExpected,
151158
UserID: user.ID,

0 commit comments

Comments
 (0)