Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit355ea31

Browse files
committed
fixup comments
1 parent7b585ec commit355ea31

File tree

2 files changed

+26
-8
lines changed

2 files changed

+26
-8
lines changed

‎coderd/idpsync/group.go

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -65,13 +65,17 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
6565
}
6666

6767
// Figure out which organizations the user is a member of.
68+
// The "Everyone" group is always included, so we can infer organization
69+
// membership via the groups the user is in.
6870
userOrgs:=make(map[uuid.UUID][]database.GetGroupsRow)
6971
for_,g:=rangeuserGroups {
7072
g:=g
7173
userOrgs[g.Group.OrganizationID]=append(userOrgs[g.Group.OrganizationID],g)
7274
}
7375

7476
// For each org, we need to fetch the sync settings
77+
// This loop also handles any legacy settings for the default
78+
// organization.
7579
orgSettings:=make(map[uuid.UUID]GroupSyncSettings)
7680
fororgID:=rangeuserOrgs {
7781
orgResolver:=s.Manager.OrganizationResolver(tx,orgID)
@@ -97,16 +101,23 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
97101
orgSettings[orgID]=*settings
98102
}
99103

100-
// collect all diffs to do 1 sql update for all orgs
104+
// groupIDsToAdd & groupIDsToRemove are the final group differences
105+
// needed to be applied to user. The loop below will iterate over all
106+
// organizations the user is in, and determine the diffs.
107+
// The diffs are applied as a batch sql query, rather than each
108+
// organization having to execute a query.
101109
groupIDsToAdd:=make([]uuid.UUID,0)
102110
groupIDsToRemove:=make([]uuid.UUID,0)
103111
// For each org, determine which groups the user should land in
104112
fororgID,settings:=rangeorgSettings {
105113
ifsettings.Field=="" {
106114
// No group sync enabled for this org, so do nothing.
115+
// The user can remain in their groups for this org.
107116
continue
108117
}
109118

119+
// expectedGroups is the set of groups the IDP expects the
120+
// user to be a member of.
110121
expectedGroups,err:=settings.ParseClaims(orgID,params.MergedClaims)
111122
iferr!=nil {
112123
s.Logger.Debug(ctx,"failed to parse claims for groups",
@@ -117,7 +128,7 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
117128
// Unsure where to raise this error on the UI or database.
118129
continue
119130
}
120-
// Everyone group is always implied.
131+
// Everyone group is always implied, so include it.
121132
expectedGroups=append(expectedGroups,ExpectedGroup{
122133
OrganizationID:orgID,
123134
GroupID:&orgID,
@@ -134,6 +145,7 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
134145
GroupName:&f.Group.Name,
135146
}
136147
})
148+
137149
add,remove:=slice.SymmetricDifferenceFunc(existingGroupsTyped,expectedGroups,func(a,bExpectedGroup)bool {
138150
// Must match
139151
ifa.OrganizationID!=b.OrganizationID {
@@ -150,10 +162,10 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
150162
})
151163

152164
for_,r:=rangeremove {
153-
// This should never happen. All group removals come from the
154-
// existing set, which come from the db. All groups from the
155-
// database have IDs. This code is purely defensive.
156165
ifr.GroupID==nil {
166+
// This should never happen. All group removals come from the
167+
// existing set, which come from the db. All groups from the
168+
// database have IDs. This code is purely defensive.
157169
detail:="user:"+user.Username
158170
ifr.GroupName!=nil {
159171
detail+=fmt.Sprintf(" from group %s",*r.GroupName)
@@ -166,6 +178,11 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
166178
// HandleMissingGroups will add the new groups to the org if
167179
// the settings specify. It will convert all group names into uuids
168180
// for easier assignment.
181+
// TODO: This code should be batched at the end of the for loop.
182+
// Optimizing this is being pushed because if AutoCreate is disabled,
183+
// this code will only add cost on the first login for each user.
184+
// AutoCreate is usually disabled for large deployments.
185+
// For small deployments, this is less of a problem.
169186
assignGroups,err:=settings.HandleMissingGroups(ctx,tx,orgID,add)
170187
iferr!=nil {
171188
returnxerrors.Errorf("handle missing groups: %w",err)
@@ -174,6 +191,8 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
174191
groupIDsToAdd=append(groupIDsToAdd,assignGroups...)
175192
}
176193

194+
// ApplyGroupDifference will take the total adds and removes, and apply
195+
// them.
177196
err=s.ApplyGroupDifference(ctx,tx,user,groupIDsToAdd,groupIDsToRemove)
178197
iferr!=nil {
179198
returnxerrors.Errorf("apply group difference: %w",err)
@@ -190,8 +209,6 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
190209

191210
// ApplyGroupDifference will add and remove the user from the specified groups.
192211
func (sAGPLIDPSync)ApplyGroupDifference(ctx context.Context,tx database.Store,user database.User,add []uuid.UUID,removeIDs []uuid.UUID)error {
193-
// Always do group removal before group add. This way if there is an error,
194-
// we error on the underprivileged side.
195212
iflen(removeIDs)>0 {
196213
removedGroupIDs,err:=tx.RemoveUserFromGroups(ctx, database.RemoveUserFromGroupsParams{
197214
UserID:user.ID,

‎enterprise/coderd/enidpsync/groups.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ func (e EnterpriseIDPSync) GroupSyncEnabled() bool {
1717
// ParseGroupClaims parses the user claims and handles deployment wide group behavior.
1818
// Almost all behavior is deferred since each organization configures it's own
1919
// group sync settings.
20-
// TODO: Implement group allow_list behavior here since that is deployment wide.
20+
// GroupAllowList is implemented here to prevent login by unauthorized users.
21+
// TODO: GroupAllowList overlaps with the default organization group sync settings.
2122
func (eEnterpriseIDPSync)ParseGroupClaims(ctx context.Context,mergedClaims jwt.MapClaims) (idpsync.GroupParams,*idpsync.HTTPError) {
2223
if!e.GroupSyncEnabled() {
2324
returne.AGPLIDPSync.ParseGroupClaims(ctx,mergedClaims)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp