Skip to content

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 38 commits into from
Sep 11, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
99c97c2
wip
Emyrk Sep 3, 2024
bfddeb6
begin group sync main work
Emyrk Sep 3, 2024
f2857c6
initial implementation of group sync
Emyrk Sep 3, 2024
791a059
work on moving to the manager
Emyrk Sep 4, 2024
4326e9d
fixup compile issues
Emyrk Sep 4, 2024
6d3ed2e
fixup some tests
Emyrk Sep 4, 2024
0803619
handle allow list
Emyrk Sep 4, 2024
596e7b4
WIP unit test for group sync
Emyrk Sep 4, 2024
b9476ac
fixup tests, account for existing groups
Emyrk Sep 4, 2024
ee8e4e4
fix compile issues
Emyrk Sep 5, 2024
d5ff0f7
add comment for test helper
Emyrk Sep 5, 2024
86c0f6f
handle legacy params
Emyrk Sep 5, 2024
2f03e18
make gen
Emyrk Sep 5, 2024
ec8092d
cleanup
Emyrk Sep 5, 2024
d63727d
add unit test for legacy behavior
Emyrk Sep 5, 2024
2a1769c
work on batching removal by name or id
Emyrk Sep 5, 2024
640e86e
group sync adjustments
Emyrk Sep 5, 2024
c544a29
test legacy params
Emyrk Sep 5, 2024
476be45
add unit test for ApplyGroupDifference
Emyrk Sep 5, 2024
164aeac
chore: remove old group sync code
Emyrk Sep 5, 2024
986498d
switch oidc test config to deployment values
Emyrk Sep 5, 2024
290cfa5
fix err name
Emyrk Sep 6, 2024
c563b10
some linting cleanup
Emyrk Sep 6, 2024
d2c247f
dbauthz test for new query
Emyrk Sep 6, 2024
12685bd
fixup comments
Emyrk Sep 6, 2024
bf0d4ed
fixup compile issues from rebase
Emyrk Sep 6, 2024
f95128e
add test for disabled sync
Emyrk Sep 6, 2024
88b0ad9
linting
Emyrk Sep 6, 2024
6491f6a
chore: handle db conflicts gracefully
Emyrk Sep 6, 2024
bd23288
test expected group equality
Emyrk Sep 6, 2024
a390ec4
cleanup comments
Emyrk Sep 6, 2024
a0a1c53
spelling mistake
Emyrk Sep 6, 2024
a86ba83
linting:
Emyrk Sep 6, 2024
0df7f28
add interface method to allow api crud
Emyrk Sep 9, 2024
7a802a9
Remove testable example
Emyrk Sep 11, 2024
611f1e3
fix formatting of sql, add a comment
Emyrk Sep 11, 2024
7f28a53
remove function only used in 1 place
Emyrk Sep 11, 2024
41994d2
make fmt
Emyrk Sep 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
wip
  • Loading branch information
Emyrk committed Sep 9, 2024
commit 99c97c215e634a6ef118f5366dd9499ec68f8d64
15 changes: 8 additions & 7 deletions coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,13 +276,6 @@ func New(options *Options) *API {
if options.Entitlements == nil {
options.Entitlements = entitlements.New()
}
if options.IDPSync == nil {
options.IDPSync = idpsync.NewAGPLSync(options.Logger, idpsync.SyncSettings{
OrganizationField: options.DeploymentValues.OIDC.OrganizationField.Value(),
OrganizationMapping: options.DeploymentValues.OIDC.OrganizationMapping.Value,
OrganizationAssignDefault: options.DeploymentValues.OIDC.OrganizationAssignDefault.Value(),
})
}
if options.NewTicker == nil {
options.NewTicker = func(duration time.Duration) (tick <-chan time.Time, done func()) {
ticker := time.NewTicker(duration)
Expand Down Expand Up @@ -318,6 +311,14 @@ func New(options *Options) *API {
options.AccessControlStore,
)

if options.IDPSync == nil {
options.IDPSync = idpsync.NewAGPLSync(options.Logger, idpsync.SyncSettings{
OrganizationField: options.DeploymentValues.OIDC.OrganizationField.Value(),
OrganizationMapping: options.DeploymentValues.OIDC.OrganizationMapping.Value,
OrganizationAssignDefault: options.DeploymentValues.OIDC.OrganizationAssignDefault.Value(),
})
}

experiments := ReadExperiments(
options.Logger, options.DeploymentValues.Experiments.Value(),
)
Expand Down
79 changes: 79 additions & 0 deletions coderd/idpsync/group.go
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 {
Copy link
Member Author

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.

// 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
}
22 changes: 12 additions & 10 deletions coderd/idpsync/idpsync.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package idpsync
import (
"context"
"net/http"
"regexp"
"strings"

"github.com/golang-jwt/jwt/v4"
Expand All @@ -29,6 +30,11 @@ type IDPSync interface {
// SyncOrganizations assigns and removed users from organizations based on the
// provided params.
SyncOrganizations(ctx context.Context, tx database.Store, user database.User, params OrganizationParams) error

GroupSyncEnabled() bool
// ParseGroupClaims takes claims from an OIDC provider, and returns the
// group sync params for assigning users into groups.
ParseGroupClaims(ctx context.Context, _ jwt.MapClaims) (GroupParams, *HTTPError)
}

// AGPLIDPSync is the configuration for syncing user information from an external
Expand All @@ -50,17 +56,13 @@ type SyncSettings struct {
// placed into the default organization. This is mostly a hack to support
// legacy deployments.
OrganizationAssignDefault bool
}

type OrganizationParams struct {
// SyncEnabled if false will skip syncing the user's organizations.
SyncEnabled bool
// IncludeDefault is primarily for single org deployments. It will ensure
// a user is always inserted into the default org.
IncludeDefault bool
// Organizations is the list of organizations the user should be a member of
// assuming syncing is turned on.
Organizations []uuid.UUID
// Group options here are set by the deployment config and only apply to
// the default organization.
GroupField string
CreateMissingGroups bool
GroupMapping map[string]string
GroupFilter *regexp.Regexp
}

func NewAGPLSync(logger slog.Logger, settings SyncSettings) *AGPLIDPSync {
Expand Down
11 changes: 11 additions & 0 deletions coderd/idpsync/organization.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@ import (
"github.com/coder/coder/v2/coderd/util/slice"
)

type OrganizationParams struct {
// SyncEnabled if false will skip syncing the user's organizations.
SyncEnabled bool
// IncludeDefault is primarily for single org deployments. It will ensure
// a user is always inserted into the default org.
IncludeDefault bool
// Organizations is the list of organizations the user should be a member of
// assuming syncing is turned on.
Organizations []uuid.UUID
}

func (AGPLIDPSync) OrganizationSyncEnabled() bool {
// AGPL does not support syncing organizations.
return false
Expand Down
16 changes: 9 additions & 7 deletions enterprise/coderd/coderd.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,13 +80,6 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
if options.Entitlements == nil {
options.Entitlements = entitlements.New()
}
if options.IDPSync == nil {
options.IDPSync = enidpsync.NewSync(options.Logger, options.Entitlements, idpsync.SyncSettings{
OrganizationField: options.DeploymentValues.OIDC.OrganizationField.Value(),
OrganizationMapping: options.DeploymentValues.OIDC.OrganizationMapping.Value,
OrganizationAssignDefault: options.DeploymentValues.OIDC.OrganizationAssignDefault.Value(),
})
}

ctx, cancelFunc := context.WithCancel(ctx)

Expand Down Expand Up @@ -118,6 +111,15 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
}

options.Database = cryptDB

if options.IDPSync == nil {
options.IDPSync = enidpsync.NewSync(options.Logger, options.Entitlements, idpsync.SyncSettings{
OrganizationField: options.DeploymentValues.OIDC.OrganizationField.Value(),
OrganizationMapping: options.DeploymentValues.OIDC.OrganizationMapping.Value,
OrganizationAssignDefault: options.DeploymentValues.OIDC.OrganizationAssignDefault.Value(),
})
}

api := &API{
ctx: ctx,
cancel: cancelFunc,
Expand Down
1 change: 0 additions & 1 deletion enterprise/coderd/enidpsync/enidpsync.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package enidpsync

import (
"cdr.dev/slog"

"github.com/coder/coder/v2/coderd/entitlements"
"github.com/coder/coder/v2/coderd/idpsync"
)
Expand Down
28 changes: 28 additions & 0 deletions enterprise/coderd/enidpsync/groups.go
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
}