-
Notifications
You must be signed in to change notification settings - Fork 894
chore: support multi-org group sync with runtime configuration #14578
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit
Hold shift + click to select a range
99c97c2
wip
Emyrk bfddeb6
begin group sync main work
Emyrk f2857c6
initial implementation of group sync
Emyrk 791a059
work on moving to the manager
Emyrk 4326e9d
fixup compile issues
Emyrk 6d3ed2e
fixup some tests
Emyrk 0803619
handle allow list
Emyrk 596e7b4
WIP unit test for group sync
Emyrk b9476ac
fixup tests, account for existing groups
Emyrk ee8e4e4
fix compile issues
Emyrk d5ff0f7
add comment for test helper
Emyrk 86c0f6f
handle legacy params
Emyrk 2f03e18
make gen
Emyrk ec8092d
cleanup
Emyrk d63727d
add unit test for legacy behavior
Emyrk 2a1769c
work on batching removal by name or id
Emyrk 640e86e
group sync adjustments
Emyrk c544a29
test legacy params
Emyrk 476be45
add unit test for ApplyGroupDifference
Emyrk 164aeac
chore: remove old group sync code
Emyrk 986498d
switch oidc test config to deployment values
Emyrk 290cfa5
fix err name
Emyrk c563b10
some linting cleanup
Emyrk d2c247f
dbauthz test for new query
Emyrk 12685bd
fixup comments
Emyrk bf0d4ed
fixup compile issues from rebase
Emyrk f95128e
add test for disabled sync
Emyrk 88b0ad9
linting
Emyrk 6491f6a
chore: handle db conflicts gracefully
Emyrk bd23288
test expected group equality
Emyrk a390ec4
cleanup comments
Emyrk a0a1c53
spelling mistake
Emyrk a86ba83
linting:
Emyrk 0df7f28
add interface method to allow api crud
Emyrk 7a802a9
Remove testable example
Emyrk 611f1e3
fix formatting of sql, add a comment
Emyrk 7f28a53
remove function only used in 1 place
Emyrk 41994d2
make fmt
Emyrk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
package idpsync | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/golang-jwt/jwt/v4" | ||
"github.com/google/uuid" | ||
"golang.org/x/xerrors" | ||
|
||
"github.com/coder/coder/v2/coderd/database" | ||
"github.com/coder/coder/v2/coderd/database/dbauthz" | ||
) | ||
|
||
type GroupParams struct { | ||
// SyncEnabled if false will skip syncing the user's groups | ||
SyncEnabled bool | ||
MergedClaims jwt.MapClaims | ||
} | ||
|
||
func (AGPLIDPSync) GroupSyncEnabled() bool { | ||
// AGPL does not support syncing groups. | ||
return false | ||
} | ||
|
||
func (s AGPLIDPSync) ParseGroupClaims(_ context.Context, _ jwt.MapClaims) (GroupParams, *HTTPError) { | ||
return GroupParams{ | ||
SyncEnabled: s.GroupSyncEnabled(), | ||
}, nil | ||
} | ||
|
||
// TODO: Group allowlist behavior should probably happen at this step. | ||
func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user database.User, params GroupParams) error { | ||
// Nothing happens if sync is not enabled | ||
if !params.SyncEnabled { | ||
return nil | ||
} | ||
|
||
// nolint:gocritic // all syncing is done as a system user | ||
ctx = dbauthz.AsSystemRestricted(ctx) | ||
|
||
db.InTx(func(tx database.Store) error { | ||
userGroups, err := db.GetGroups(ctx, database.GetGroupsParams{ | ||
HasMemberID: user.ID, | ||
}) | ||
if err != nil { | ||
return xerrors.Errorf("get user groups: %w", err) | ||
} | ||
|
||
// Figure out which organizations the user is a member of. | ||
userOrgs := make(map[uuid.UUID][]database.GetGroupsRow) | ||
for _, g := range userGroups { | ||
g := g | ||
userOrgs[g.Group.OrganizationID] = append(userOrgs[g.Group.OrganizationID], g) | ||
} | ||
|
||
// Force each organization, we sync the groups. | ||
db.RemoveUserFromAllGroups(ctx, user.ID) | ||
|
||
return nil | ||
}, nil) | ||
|
||
// | ||
//tx.InTx(func(tx database.Store) error { | ||
// // When setting the user's groups, it's easier to just clear their groups and re-add them. | ||
// // This ensures that the user's groups are always in sync with the auth provider. | ||
// err := tx.RemoveUserFromAllGroups(ctx, user.ID) | ||
// if err != nil { | ||
// return err | ||
// } | ||
// | ||
// for _, org := range userOrgs { | ||
// | ||
// } | ||
// | ||
// return nil | ||
//}, nil) | ||
|
||
return nil | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package enidpsync | ||
|
||
import ( | ||
"context" | ||
|
||
"github.com/golang-jwt/jwt/v4" | ||
|
||
"github.com/coder/coder/v2/coderd/idpsync" | ||
"github.com/coder/coder/v2/codersdk" | ||
) | ||
|
||
func (e EnterpriseIDPSync) GroupSyncEnabled() bool { | ||
return e.entitlements.Enabled(codersdk.FeatureTemplateRBAC) | ||
|
||
} | ||
|
||
// ParseGroupClaims returns the groups from the external IDP. | ||
// TODO: Implement group allow_list behavior here since that is deployment wide. | ||
func (e EnterpriseIDPSync) ParseGroupClaims(ctx context.Context, mergedClaims jwt.MapClaims) (idpsync.GroupParams, *idpsync.HTTPError) { | ||
if !e.GroupSyncEnabled() { | ||
return e.AGPLIDPSync.ParseGroupClaims(ctx, mergedClaims) | ||
} | ||
|
||
return idpsync.GroupParams{ | ||
SyncEnabled: e.OrganizationSyncEnabled(), | ||
MergedClaims: mergedClaims, | ||
}, nil | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enterprise license code just handles the
ParseGroupClaims
. So AGPL will always be disabled.I put the actual
SyncGroups
code in AGPL so that I can unit test and maintain the sync code all in 1 package. It is a convenience, while still making sure AGPL code cannot leverage this feature.