Skip to content

Commit 6a846cd

Browse files
authored
chore: support multi-org group sync with runtime configuration (#14578)
- Implement multi-org group sync - Implement runtime configuration to change sync behavior - Legacy group sync migrated to new package
1 parent 7de576b commit 6a846cd

27 files changed

+1920
-341
lines changed

cli/server.go

-5
Original file line numberDiff line numberDiff line change
@@ -187,11 +187,6 @@ func createOIDCConfig(ctx context.Context, logger slog.Logger, vals *codersdk.De
187187
EmailField: vals.OIDC.EmailField.String(),
188188
AuthURLParams: vals.OIDC.AuthURLParams.Value,
189189
IgnoreUserInfo: vals.OIDC.IgnoreUserInfo.Value(),
190-
GroupField: vals.OIDC.GroupField.String(),
191-
GroupFilter: vals.OIDC.GroupRegexFilter.Value(),
192-
GroupAllowList: groupAllowList,
193-
CreateMissingGroups: vals.OIDC.GroupAutoCreate.Value(),
194-
GroupMapping: vals.OIDC.GroupMapping.Value,
195190
UserRoleField: vals.OIDC.UserRoleField.String(),
196191
UserRoleMapping: vals.OIDC.UserRoleMapping.Value,
197192
UserRolesDefault: vals.OIDC.UserRolesDefault.GetSlice(),

coderd/coderd.go

+4-18
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,6 @@ type Options struct {
181181
NetworkTelemetryBatchFrequency time.Duration
182182
NetworkTelemetryBatchMaxSize int
183183
SwaggerEndpoint bool
184-
SetUserGroups func(ctx context.Context, logger slog.Logger, tx database.Store, userID uuid.UUID, orgGroupNames map[uuid.UUID][]string, createMissingGroups bool) error
185184
SetUserSiteRoles func(ctx context.Context, logger slog.Logger, tx database.Store, userID uuid.UUID, roles []string) error
186185
TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
187186
UserQuietHoursScheduleStore *atomic.Pointer[schedule.UserQuietHoursScheduleStore]
@@ -276,13 +275,6 @@ func New(options *Options) *API {
276275
if options.Entitlements == nil {
277276
options.Entitlements = entitlements.New()
278277
}
279-
if options.IDPSync == nil {
280-
options.IDPSync = idpsync.NewAGPLSync(options.Logger, idpsync.SyncSettings{
281-
OrganizationField: options.DeploymentValues.OIDC.OrganizationField.Value(),
282-
OrganizationMapping: options.DeploymentValues.OIDC.OrganizationMapping.Value,
283-
OrganizationAssignDefault: options.DeploymentValues.OIDC.OrganizationAssignDefault.Value(),
284-
})
285-
}
286278
if options.NewTicker == nil {
287279
options.NewTicker = func(duration time.Duration) (tick <-chan time.Time, done func()) {
288280
ticker := time.NewTicker(duration)
@@ -318,6 +310,10 @@ func New(options *Options) *API {
318310
options.AccessControlStore,
319311
)
320312

313+
if options.IDPSync == nil {
314+
options.IDPSync = idpsync.NewAGPLSync(options.Logger, options.RuntimeConfig, idpsync.FromDeploymentValues(options.DeploymentValues))
315+
}
316+
321317
experiments := ReadExperiments(
322318
options.Logger, options.DeploymentValues.Experiments.Value(),
323319
)
@@ -377,16 +373,6 @@ func New(options *Options) *API {
377373
if options.TracerProvider == nil {
378374
options.TracerProvider = trace.NewNoopTracerProvider()
379375
}
380-
if options.SetUserGroups == nil {
381-
options.SetUserGroups = func(ctx context.Context, logger slog.Logger, _ database.Store, userID uuid.UUID, orgGroupNames map[uuid.UUID][]string, createMissingGroups bool) error {
382-
logger.Warn(ctx, "attempted to assign OIDC groups without enterprise license",
383-
slog.F("user_id", userID),
384-
slog.F("groups", orgGroupNames),
385-
slog.F("create_missing_groups", createMissingGroups),
386-
)
387-
return nil
388-
}
389-
}
390376
if options.SetUserSiteRoles == nil {
391377
options.SetUserSiteRoles = func(ctx context.Context, logger slog.Logger, _ database.Store, userID uuid.UUID, roles []string) error {
392378
logger.Warn(ctx, "attempted to assign OIDC user roles without enterprise license",

coderd/coderdtest/uuids.go

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package coderdtest
2+
3+
import "github.com/google/uuid"
4+
5+
// DeterministicUUIDGenerator allows "naming" uuids for unit tests.
6+
// An example of where this is useful, is when a tabled test references
7+
// a UUID that is not yet known. An alternative to this would be to
8+
// hard code some UUID strings, but these strings are not human friendly.
9+
type DeterministicUUIDGenerator struct {
10+
Named map[string]uuid.UUID
11+
}
12+
13+
func NewDeterministicUUIDGenerator() *DeterministicUUIDGenerator {
14+
return &DeterministicUUIDGenerator{
15+
Named: make(map[string]uuid.UUID),
16+
}
17+
}
18+
19+
func (d *DeterministicUUIDGenerator) ID(name string) uuid.UUID {
20+
if v, ok := d.Named[name]; ok {
21+
return v
22+
}
23+
d.Named[name] = uuid.New()
24+
return d.Named[name]
25+
}

coderd/coderdtest/uuids_test.go

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
package coderdtest_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/stretchr/testify/require"
7+
8+
"github.com/coder/coder/v2/coderd/coderdtest"
9+
)
10+
11+
func TestDeterministicUUIDGenerator(t *testing.T) {
12+
t.Parallel()
13+
14+
ids := coderdtest.NewDeterministicUUIDGenerator()
15+
require.Equal(t, ids.ID("g1"), ids.ID("g1"))
16+
require.NotEqual(t, ids.ID("g1"), ids.ID("g2"))
17+
}

coderd/database/dbauthz/dbauthz.go

+16
Original file line numberDiff line numberDiff line change
@@ -2892,6 +2892,14 @@ func (q *querier) InsertUser(ctx context.Context, arg database.InsertUserParams)
28922892
return insert(q.log, q.auth, obj, q.db.InsertUser)(ctx, arg)
28932893
}
28942894

2895+
func (q *querier) InsertUserGroupsByID(ctx context.Context, arg database.InsertUserGroupsByIDParams) ([]uuid.UUID, error) {
2896+
// This is used by OIDC sync. So only used by a system user.
2897+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
2898+
return nil, err
2899+
}
2900+
return q.db.InsertUserGroupsByID(ctx, arg)
2901+
}
2902+
28952903
func (q *querier) InsertUserGroupsByName(ctx context.Context, arg database.InsertUserGroupsByNameParams) error {
28962904
// This will add the user to all named groups. This counts as updating a group.
28972905
// NOTE: instead of checking if the user has permission to update each group, we instead
@@ -3100,6 +3108,14 @@ func (q *querier) RemoveUserFromAllGroups(ctx context.Context, userID uuid.UUID)
31003108
return q.db.RemoveUserFromAllGroups(ctx, userID)
31013109
}
31023110

3111+
func (q *querier) RemoveUserFromGroups(ctx context.Context, arg database.RemoveUserFromGroupsParams) ([]uuid.UUID, error) {
3112+
// This is a system function to clear user groups in group sync.
3113+
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
3114+
return nil, err
3115+
}
3116+
return q.db.RemoveUserFromGroups(ctx, arg)
3117+
}
3118+
31033119
func (q *querier) RevokeDBCryptKey(ctx context.Context, activeKeyDigest string) error {
31043120
if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil {
31053121
return err

coderd/database/dbauthz/dbauthz_test.go

+23
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,17 @@ func (s *MethodTestSuite) TestGroup() {
388388
GroupNames: slice.New(g1.Name, g2.Name),
389389
}).Asserts(rbac.ResourceGroup.InOrg(o.ID), policy.ActionUpdate).Returns()
390390
}))
391+
s.Run("InsertUserGroupsByID", s.Subtest(func(db database.Store, check *expects) {
392+
o := dbgen.Organization(s.T(), db, database.Organization{})
393+
u1 := dbgen.User(s.T(), db, database.User{})
394+
g1 := dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID})
395+
g2 := dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID})
396+
_ = dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g1.ID, UserID: u1.ID})
397+
check.Args(database.InsertUserGroupsByIDParams{
398+
UserID: u1.ID,
399+
GroupIds: slice.New(g1.ID, g2.ID),
400+
}).Asserts(rbac.ResourceSystem, policy.ActionUpdate).Returns(slice.New(g1.ID, g2.ID))
401+
}))
391402
s.Run("RemoveUserFromAllGroups", s.Subtest(func(db database.Store, check *expects) {
392403
o := dbgen.Organization(s.T(), db, database.Organization{})
393404
u1 := dbgen.User(s.T(), db, database.User{})
@@ -397,6 +408,18 @@ func (s *MethodTestSuite) TestGroup() {
397408
_ = dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g2.ID, UserID: u1.ID})
398409
check.Args(u1.ID).Asserts(rbac.ResourceSystem, policy.ActionUpdate).Returns()
399410
}))
411+
s.Run("RemoveUserFromGroups", s.Subtest(func(db database.Store, check *expects) {
412+
o := dbgen.Organization(s.T(), db, database.Organization{})
413+
u1 := dbgen.User(s.T(), db, database.User{})
414+
g1 := dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID})
415+
g2 := dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID})
416+
_ = dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g1.ID, UserID: u1.ID})
417+
_ = dbgen.GroupMember(s.T(), db, database.GroupMemberTable{GroupID: g2.ID, UserID: u1.ID})
418+
check.Args(database.RemoveUserFromGroupsParams{
419+
UserID: u1.ID,
420+
GroupIds: []uuid.UUID{g1.ID, g2.ID},
421+
}).Asserts(rbac.ResourceSystem, policy.ActionUpdate).Returns(slice.New(g1.ID, g2.ID))
422+
}))
400423
s.Run("UpdateGroupByID", s.Subtest(func(db database.Store, check *expects) {
401424
g := dbgen.Group(s.T(), db, database.Group{})
402425
check.Args(database.UpdateGroupByIDParams{

coderd/database/dbmem/dbmem.go

+66-4
Original file line numberDiff line numberDiff line change
@@ -2695,18 +2695,18 @@ func (q *FakeQuerier) GetGroups(_ context.Context, arg database.GetGroupsParams)
26952695
q.mutex.RLock()
26962696
defer q.mutex.RUnlock()
26972697

2698-
groupIDs := make(map[uuid.UUID]struct{})
2698+
userGroupIDs := make(map[uuid.UUID]struct{})
26992699
if arg.HasMemberID != uuid.Nil {
27002700
for _, member := range q.groupMembers {
27012701
if member.UserID == arg.HasMemberID {
2702-
groupIDs[member.GroupID] = struct{}{}
2702+
userGroupIDs[member.GroupID] = struct{}{}
27032703
}
27042704
}
27052705

27062706
// Handle the everyone group
27072707
for _, orgMember := range q.organizationMembers {
27082708
if orgMember.UserID == arg.HasMemberID {
2709-
groupIDs[orgMember.OrganizationID] = struct{}{}
2709+
userGroupIDs[orgMember.OrganizationID] = struct{}{}
27102710
}
27112711
}
27122712
}
@@ -2718,11 +2718,15 @@ func (q *FakeQuerier) GetGroups(_ context.Context, arg database.GetGroupsParams)
27182718
continue
27192719
}
27202720

2721-
_, ok := groupIDs[group.ID]
2721+
_, ok := userGroupIDs[group.ID]
27222722
if arg.HasMemberID != uuid.Nil && !ok {
27232723
continue
27242724
}
27252725

2726+
if len(arg.GroupNames) > 0 && !slices.Contains(arg.GroupNames, group.Name) {
2727+
continue
2728+
}
2729+
27262730
orgDetails, ok := orgDetailsCache[group.ID]
27272731
if !ok {
27282732
for _, org := range q.organizations {
@@ -7015,7 +7019,37 @@ func (q *FakeQuerier) InsertUser(_ context.Context, arg database.InsertUserParam
70157019
return user, nil
70167020
}
70177021

7022+
func (q *FakeQuerier) InsertUserGroupsByID(_ context.Context, arg database.InsertUserGroupsByIDParams) ([]uuid.UUID, error) {
7023+
err := validateDatabaseType(arg)
7024+
if err != nil {
7025+
return nil, err
7026+
}
7027+
7028+
q.mutex.Lock()
7029+
defer q.mutex.Unlock()
7030+
7031+
var groupIDs []uuid.UUID
7032+
for _, group := range q.groups {
7033+
for _, groupID := range arg.GroupIds {
7034+
if group.ID == groupID {
7035+
q.groupMembers = append(q.groupMembers, database.GroupMemberTable{
7036+
UserID: arg.UserID,
7037+
GroupID: groupID,
7038+
})
7039+
groupIDs = append(groupIDs, group.ID)
7040+
}
7041+
}
7042+
}
7043+
7044+
return groupIDs, nil
7045+
}
7046+
70187047
func (q *FakeQuerier) InsertUserGroupsByName(_ context.Context, arg database.InsertUserGroupsByNameParams) error {
7048+
err := validateDatabaseType(arg)
7049+
if err != nil {
7050+
return err
7051+
}
7052+
70197053
q.mutex.Lock()
70207054
defer q.mutex.Unlock()
70217055

@@ -7607,6 +7641,34 @@ func (q *FakeQuerier) RemoveUserFromAllGroups(_ context.Context, userID uuid.UUI
76077641
return nil
76087642
}
76097643

7644+
func (q *FakeQuerier) RemoveUserFromGroups(_ context.Context, arg database.RemoveUserFromGroupsParams) ([]uuid.UUID, error) {
7645+
err := validateDatabaseType(arg)
7646+
if err != nil {
7647+
return nil, err
7648+
}
7649+
7650+
q.mutex.Lock()
7651+
defer q.mutex.Unlock()
7652+
7653+
removed := make([]uuid.UUID, 0)
7654+
q.data.groupMembers = slices.DeleteFunc(q.data.groupMembers, func(groupMember database.GroupMemberTable) bool {
7655+
// Delete all group members that match the arguments.
7656+
if groupMember.UserID != arg.UserID {
7657+
// Not the right user, ignore.
7658+
return false
7659+
}
7660+
7661+
if !slices.Contains(arg.GroupIds, groupMember.GroupID) {
7662+
return false
7663+
}
7664+
7665+
removed = append(removed, groupMember.GroupID)
7666+
return true
7667+
})
7668+
7669+
return removed, nil
7670+
}
7671+
76107672
func (q *FakeQuerier) RevokeDBCryptKey(_ context.Context, activeKeyDigest string) error {
76117673
q.mutex.Lock()
76127674
defer q.mutex.Unlock()

coderd/database/dbmetrics/dbmetrics.go

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

coderd/database/dbmock/dbmock.go

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

coderd/database/querier.go

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

0 commit comments

Comments
 (0)