Skip to content

Commit 331ab79

Browse files
EmyrkEmyrk
committed
feat: groupsync set groups defaults to default org
1 parent ddb828c commit 331ab79

File tree

11 files changed

+112
-124
lines changed

11 files changed

+112
-124
lines changed

‎coderd/coderd.go

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ type Options struct {
134134
BaseDERPMap *tailcfg.DERPMap
135135
DERPMapUpdateFrequency time.Duration
136136
SwaggerEndpoint bool
137-
SetUserGroups func(ctx context.Context, logger slog.Logger, tx database.Store, userID uuid.UUID, groupNames []string, createMissingGroups bool) error
137+
SetUserGroups func(ctx context.Context, logger slog.Logger, tx database.Store, userID uuid.UUID, orgGroupNames map[uuid.UUID][]string, createMissingGroups bool) error
138138
SetUserSiteRoles func(ctx context.Context, logger slog.Logger, tx database.Store, userID uuid.UUID, roles []string) error
139139
TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore]
140140
UserQuietHoursScheduleStore *atomic.Pointer[schedule.UserQuietHoursScheduleStore]
@@ -301,9 +301,11 @@ func New(options *Options) *API {
301301
options.TracerProvider = trace.NewNoopTracerProvider()
302302
}
303303
if options.SetUserGroups == nil {
304-
options.SetUserGroups = func(ctx context.Context, logger slog.Logger, _ database.Store, userID uuid.UUID, groups []string, createMissingGroups bool) error {
304+
options.SetUserGroups = func(ctx context.Context, logger slog.Logger, _ database.Store, userID uuid.UUID, orgGroupNames map[uuid.UUID][]string, createMissingGroups bool) error {
305305
logger.Warn(ctx, "attempted to assign OIDC groups without enterprise license",
306-
slog.F("user_id", userID), slog.F("groups", groups), slog.F("create_missing_groups", createMissingGroups),
306+
slog.F("user_id", userID),
307+
slog.F("groups", orgGroupNames),
308+
slog.F("create_missing_groups", createMissingGroups),
307309
)
308310
return nil
309311
}

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -656,6 +656,14 @@ func authorizedTemplateVersionFromJob(ctx context.Context, q *querier, job datab
656656
}
657657
}
658658

659+
func (q *querier) RemoveUserFromAllGroups(ctx context.Context, userID uuid.UUID) error {
660+
// This is a system function to clear user groups in group sync.
661+
if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil {
662+
return err
663+
}
664+
return q.db.RemoveUserFromAllGroups(ctx, userID)
665+
}
666+
659667
func (q *querier) AcquireLock(ctx context.Context, id int64) error {
660668
return q.db.AcquireLock(ctx, id)
661669
}
@@ -793,16 +801,6 @@ func (q *querier) DeleteGroupMemberFromGroup(ctx context.Context, arg database.D
793801
return update(q.log, q.auth, fetch, q.db.DeleteGroupMemberFromGroup)(ctx, arg)
794802
}
795803

796-
func (q *querier) DeleteGroupMembersByOrgAndUser(ctx context.Context, arg database.DeleteGroupMembersByOrgAndUserParams) error {
797-
// This will remove the user from all groups in the org. This counts as updating a group.
798-
// NOTE: instead of fetching all groups in the org with arg.UserID as a member, we instead
799-
// check if the caller has permission to update any group in the org.
800-
fetch := func(ctx context.Context, arg database.DeleteGroupMembersByOrgAndUserParams) (rbac.Objecter, error) {
801-
return rbac.ResourceGroup.InOrg(arg.OrganizationID), nil
802-
}
803-
return update(q.log, q.auth, fetch, q.db.DeleteGroupMembersByOrgAndUser)(ctx, arg)
804-
}
805-
806804
func (q *querier) DeleteLicense(ctx context.Context, id int32) (int32, error) {
807805
err := deleteQ(q.log, q.auth, q.db.GetLicenseByID, func(ctx context.Context, id int32) error {
808806
_, err := q.db.DeleteLicense(ctx, id)

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -344,17 +344,14 @@ func (s *MethodTestSuite) TestGroup() {
344344
GroupNames: slice.New(g1.Name, g2.Name),
345345
}).Asserts(rbac.ResourceGroup.InOrg(o.ID), rbac.ActionUpdate).Returns()
346346
}))
347-
s.Run("DeleteGroupMembersByOrgAndUser", s.Subtest(func(db database.Store, check *expects) {
347+
s.Run("RemoveUserFromAllGroups", s.Subtest(func(db database.Store, check *expects) {
348348
o := dbgen.Organization(s.T(), db, database.Organization{})
349349
u1 := dbgen.User(s.T(), db, database.User{})
350350
g1 := dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID})
351351
g2 := dbgen.Group(s.T(), db, database.Group{OrganizationID: o.ID})
352352
_ = dbgen.GroupMember(s.T(), db, database.GroupMember{GroupID: g1.ID, UserID: u1.ID})
353353
_ = dbgen.GroupMember(s.T(), db, database.GroupMember{GroupID: g2.ID, UserID: u1.ID})
354-
check.Args(database.DeleteGroupMembersByOrgAndUserParams{
355-
OrganizationID: o.ID,
356-
UserID: u1.ID,
357-
}).Asserts(rbac.ResourceGroup.InOrg(o.ID), rbac.ActionUpdate).Returns()
354+
check.Args(u1.ID).Asserts(rbac.ResourceSystem, rbac.ActionUpdate).Returns()
358355
}))
359356
s.Run("UpdateGroupByID", s.Subtest(func(db database.Store, check *expects) {
360357
g := dbgen.Group(s.T(), db, database.Group{})

‎coderd/database/dbmem/dbmem.go

Lines changed: 16 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -733,6 +733,22 @@ func isNotNull(v interface{}) bool {
733733
return reflect.ValueOf(v).FieldByName("Valid").Bool()
734734
}
735735

736+
func (q *FakeQuerier) RemoveUserFromAllGroups(_ context.Context, userID uuid.UUID) error {
737+
q.mutex.Lock()
738+
defer q.mutex.Unlock()
739+
740+
newMembers := q.groupMembers[:0]
741+
for _, member := range q.groupMembers {
742+
if member.UserID == userID {
743+
continue
744+
}
745+
newMembers = append(newMembers, member)
746+
}
747+
q.groupMembers = newMembers
748+
749+
return nil
750+
}
751+
736752
func (*FakeQuerier) AcquireLock(_ context.Context, _ int64) error {
737753
return xerrors.New("AcquireLock must only be called within a transaction")
738754
}
@@ -1135,36 +1151,6 @@ func (q *FakeQuerier) DeleteGroupMemberFromGroup(_ context.Context, arg database
11351151
return nil
11361152
}
11371153

1138-
func (q *FakeQuerier) DeleteGroupMembersByOrgAndUser(_ context.Context, arg database.DeleteGroupMembersByOrgAndUserParams) error {
1139-
q.mutex.Lock()
1140-
defer q.mutex.Unlock()
1141-
1142-
newMembers := q.groupMembers[:0]
1143-
for _, member := range q.groupMembers {
1144-
if member.UserID != arg.UserID {
1145-
// Do not delete the other members
1146-
newMembers = append(newMembers, member)
1147-
} else if member.UserID == arg.UserID {
1148-
// We only want to delete from groups in the organization in the args.
1149-
for _, group := range q.groups {
1150-
// Find the group that the member is apartof.
1151-
if group.ID == member.GroupID {
1152-
// Only add back the member if the organization ID does not match
1153-
// the arg organization ID. Since the arg is saying which
1154-
// org to delete.
1155-
if group.OrganizationID != arg.OrganizationID {
1156-
newMembers = append(newMembers, member)
1157-
}
1158-
break
1159-
}
1160-
}
1161-
}
1162-
}
1163-
q.groupMembers = newMembers
1164-
1165-
return nil
1166-
}
1167-
11681154
func (q *FakeQuerier) DeleteLicense(_ context.Context, id int32) (int32, error) {
11691155
q.mutex.Lock()
11701156
defer q.mutex.Unlock()

‎coderd/database/dbmetrics/dbmetrics.go

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 14 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 12 additions & 18 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/groupmembers.sql

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,11 @@ SELECT
4242
FROM
4343
groups;
4444

45-
-- name: DeleteGroupMembersByOrgAndUser :exec
45+
-- name: RemoveUserFromAllGroups :exec
4646
DELETE FROM
4747
group_members
4848
WHERE
49-
group_members.user_id = @user_id
50-
AND group_id = ANY(SELECT id FROM groups WHERE organization_id = @organization_id);
49+
user_id = @user_id;
5150

5251
-- name: InsertGroupMember :exec
5352
INSERT INTO

‎coderd/userauth.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,7 +1217,7 @@ type oauthLoginParams struct {
12171217
// to the Groups provided.
12181218
UsingGroups bool
12191219
CreateMissingGroups bool
1220-
Groups []string
1220+
Groups map[uuid.UUID][]string
12211221
GroupFilter *regexp.Regexp
12221222
// Is UsingRoles is true, then the user will be assigned
12231223
// the roles provided.
@@ -1460,11 +1460,15 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C
14601460
if params.UsingGroups {
14611461
filtered := params.Groups
14621462
if params.GroupFilter != nil {
1463-
filtered = make([]string, 0, len(params.Groups))
1464-
for _, group := range params.Groups {
1465-
if params.GroupFilter.MatchString(group) {
1466-
filtered = append(filtered, group)
1463+
// For each org, filter the groups.
1464+
for orgID, groups := range filtered {
1465+
filteredList := make([]string, 0, len(groups))
1466+
for _, group := range groups {
1467+
if params.GroupFilter.MatchString(group) {
1468+
filteredList = append(filteredList, group)
1469+
}
14671470
}
1471+
filtered[orgID] = filteredList
14681472
}
14691473
}
14701474

0 commit comments

Comments
 (0)