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

enterprise/coderd/coderd.go

@@ -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)
 
@@ -118,6 +111,11 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
 	}
 
 	options.Database = cryptDB
+
+	if options.IDPSync == nil {
+		options.IDPSync = enidpsync.NewSync(options.Logger, options.RuntimeConfig, options.Entitlements, idpsync.FromDeploymentValues(options.DeploymentValues))
+	}
+
 	api := &API{
 		ctx:     ctx,
 		cancel:  cancelFunc,
@@ -147,7 +145,6 @@ func New(ctx context.Context, options *Options) (_ *API, err error) {
 		}
 		return c.Subject, c.Trial, nil
 	}
-	api.AGPL.Options.SetUserGroups = api.setUserGroups
 	api.AGPL.Options.SetUserSiteRoles = api.setUserSiteRoles
 	api.AGPL.SiteHandler.RegionsFetcher = func(ctx context.Context) (any, error) {
 		// If the user can read the workspace proxy resource, return that.

enterprise/coderd/enidpsync/enidpsync.go

@@ -2,9 +2,9 @@ package enidpsync
 
 import (
 	"cdr.dev/slog"
-
 	"github.com/coder/coder/v2/coderd/entitlements"
 	"github.com/coder/coder/v2/coderd/idpsync"
+	"github.com/coder/coder/v2/coderd/runtimeconfig"
 )
 
 // EnterpriseIDPSync enabled syncing user information from an external IDP.
@@ -17,9 +17,9 @@ type EnterpriseIDPSync struct {
 	*idpsync.AGPLIDPSync
 }
 
-func NewSync(logger slog.Logger, set *entitlements.Set, settings idpsync.SyncSettings) *EnterpriseIDPSync {
+func NewSync(logger slog.Logger, manager *runtimeconfig.Manager, set *entitlements.Set, settings idpsync.DeploymentSyncSettings) *EnterpriseIDPSync {
 	return &EnterpriseIDPSync{
 		entitlements: set,
-		AGPLIDPSync:  idpsync.NewAGPLSync(logger.With(slog.F("enterprise_capable", "true")), settings),
+		AGPLIDPSync:  idpsync.NewAGPLSync(logger.With(slog.F("enterprise_capable", "true")), manager, settings),
 	}
 }

enterprise/coderd/enidpsync/groups.go

@@ -0,0 +1,70 @@
+package enidpsync
+
+import (
+	"context"
+	"net/http"
+
+	"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 parses the user claims and handles deployment wide group behavior.
+// Almost all behavior is deferred since each organization configures it's own
+// group sync settings.
+// GroupAllowList is implemented here to prevent login by unauthorized users.
+// TODO: GroupAllowList overlaps with the default organization group sync settings.
+func (e EnterpriseIDPSync) ParseGroupClaims(ctx context.Context, mergedClaims jwt.MapClaims) (idpsync.GroupParams, *idpsync.HTTPError) {
+	if !e.GroupSyncEnabled() {
+		return e.AGPLIDPSync.ParseGroupClaims(ctx, mergedClaims)
+	}
+
+	if e.GroupField != "" && len(e.GroupAllowList) > 0 {
+		groupsRaw, ok := mergedClaims[e.GroupField]
+		if !ok {
+			return idpsync.GroupParams{}, &idpsync.HTTPError{
+				Code:             http.StatusForbidden,
+				Msg:              "Not a member of an allowed group",
+				Detail:           "You have no groups in your claims!",
+				RenderStaticPage: true,
+			}
+		}
+		parsedGroups, err := idpsync.ParseStringSliceClaim(groupsRaw)
+		if err != nil {
+			return idpsync.GroupParams{}, &idpsync.HTTPError{
+				Code:             http.StatusBadRequest,
+				Msg:              "Failed read groups from claims for allow list check. Ask an administrator for help.",
+				Detail:           err.Error(),
+				RenderStaticPage: true,
+			}
+		}
+
+		inAllowList := false
+	AllowListCheckLoop:
+		for _, group := range parsedGroups {
+			if _, ok := e.GroupAllowList[group]; ok {
+				inAllowList = true
+				break AllowListCheckLoop
+			}
+		}
+
+		if !inAllowList {
+			return idpsync.GroupParams{}, &idpsync.HTTPError{
+				Code:             http.StatusForbidden,
+				Msg:              "Not a member of an allowed group",
+				Detail:           "Ask an administrator to add one of your groups to the allow list.",
+				RenderStaticPage: true,
+			}
+		}
+	}
+
+	return idpsync.GroupParams{
+		SyncEnabled:  true,
+		MergedClaims: mergedClaims,
+	}, nil
+}

enterprise/coderd/enidpsync/groups_test.go

@@ -0,0 +1,96 @@
+package enidpsync_test
+
+import (
+	"testing"
+
+	"github.com/golang-jwt/jwt/v4"
+	"github.com/stretchr/testify/require"
+
+	"cdr.dev/slog/sloggers/slogtest"
+	"github.com/coder/coder/v2/coderd/entitlements"
+	"github.com/coder/coder/v2/coderd/idpsync"
+	"github.com/coder/coder/v2/coderd/runtimeconfig"
+	"github.com/coder/coder/v2/codersdk"
+	"github.com/coder/coder/v2/enterprise/coderd/enidpsync"
+	"github.com/coder/coder/v2/testutil"
+)
+
+func TestEnterpriseParseGroupClaims(t *testing.T) {
+	t.Parallel()
+
+	entitled := entitlements.New()
+	entitled.Update(func(entitlements *codersdk.Entitlements) {
+		entitlements.Features[codersdk.FeatureTemplateRBAC] = codersdk.Feature{
+			Entitlement: codersdk.EntitlementEntitled,
+			Enabled:     true,
+		}
+	})
+
+	t.Run("NoEntitlements", func(t *testing.T) {
+		t.Parallel()
+
+		s := enidpsync.NewSync(slogtest.Make(t, &slogtest.Options{}),
+			runtimeconfig.NewManager(),
+			entitlements.New(),
+			idpsync.DeploymentSyncSettings{})
+
+		ctx := testutil.Context(t, testutil.WaitMedium)
+
+		params, err := s.ParseGroupClaims(ctx, jwt.MapClaims{})
+		require.Nil(t, err)
+
+		require.False(t, params.SyncEnabled)
+	})
+
+	t.Run("NotInAllowList", func(t *testing.T) {
+		t.Parallel()
+
+		s := enidpsync.NewSync(slogtest.Make(t, &slogtest.Options{}),
+			runtimeconfig.NewManager(),
+			entitled,
+			idpsync.DeploymentSyncSettings{
+				GroupField: "groups",
+				GroupAllowList: map[string]struct{}{
+					"foo": {},
+				},
+			})
+
+		ctx := testutil.Context(t, testutil.WaitMedium)
+
+		// Try with incorrect group
+		_, err := s.ParseGroupClaims(ctx, jwt.MapClaims{
+			"groups": []string{"bar"},
+		})
+		require.NotNil(t, err)
+		require.Equal(t, 403, err.Code)
+
+		// Try with no groups
+		_, err = s.ParseGroupClaims(ctx, jwt.MapClaims{})
+		require.NotNil(t, err)
+		require.Equal(t, 403, err.Code)
+	})
+
+	t.Run("InAllowList", func(t *testing.T) {
+		t.Parallel()
+
+		s := enidpsync.NewSync(slogtest.Make(t, &slogtest.Options{}),
+			runtimeconfig.NewManager(),
+			entitled,
+			idpsync.DeploymentSyncSettings{
+				GroupField: "groups",
+				GroupAllowList: map[string]struct{}{
+					"foo": {},
+				},
+			})
+
+		ctx := testutil.Context(t, testutil.WaitMedium)
+
+		claims := jwt.MapClaims{
+			"groups": []string{"foo", "bar"},
+		}
+		params, err := s.ParseGroupClaims(ctx, claims)
+		require.Nil(t, err)
+		require.True(t, params.SyncEnabled)
+		require.Equal(t, claims, params.MergedClaims)
+	})
+}

enterprise/coderd/enidpsync/organizations_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/coder/coder/v2/coderd/entitlements"
 	"github.com/coder/coder/v2/coderd/idpsync"
 	"github.com/coder/coder/v2/coderd/rbac"
+	"github.com/coder/coder/v2/coderd/runtimeconfig"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/enterprise/coderd/enidpsync"
 	"github.com/coder/coder/v2/testutil"
@@ -41,7 +42,7 @@ type Expectations struct {
 }
 
 type OrganizationSyncTestCase struct {
-	Settings     idpsync.SyncSettings
+	Settings     idpsync.DeploymentSyncSettings
 	Entitlements *entitlements.Set
 	Exps         []Expectations
 }
@@ -89,7 +90,7 @@ func TestOrganizationSync(t *testing.T) {
 				other := dbgen.Organization(t, db, database.Organization{})
 				return OrganizationSyncTestCase{
 					Entitlements: entitled,
-					Settings: idpsync.SyncSettings{
+					Settings: idpsync.DeploymentSyncSettings{
 						OrganizationField:         "",
 						OrganizationMapping:       nil,
 						OrganizationAssignDefault: true,
@@ -142,7 +143,7 @@ func TestOrganizationSync(t *testing.T) {
 				three := dbgen.Organization(t, db, database.Organization{})
 				return OrganizationSyncTestCase{
 					Entitlements: entitled,
-					Settings: idpsync.SyncSettings{
+					Settings: idpsync.DeploymentSyncSettings{
 						OrganizationField: "organizations",
 						OrganizationMapping: map[string][]uuid.UUID{
 							"first":  {one.ID},
@@ -236,7 +237,7 @@ func TestOrganizationSync(t *testing.T) {
 			}
 
 			// Create a new sync object
-			sync := enidpsync.NewSync(logger, caseData.Entitlements, caseData.Settings)
+			sync := enidpsync.NewSync(logger, runtimeconfig.NewManager(), caseData.Entitlements, caseData.Settings)
 			for _, exp := range caseData.Exps {
 				t.Run(exp.Name, func(t *testing.T) {
 					params, httpErr := sync.ParseOrganizationClaims(ctx, exp.Claims)

enterprise/coderd/userauth.go

@@ -8,75 +8,9 @@ import (
 
 	"cdr.dev/slog"
 	"github.com/coder/coder/v2/coderd/database"
-	"github.com/coder/coder/v2/coderd/database/dbauthz"
 	"github.com/coder/coder/v2/codersdk"
 )
 
-// nolint: revive
-func (api *API) setUserGroups(ctx context.Context, logger slog.Logger, db database.Store, userID uuid.UUID, orgGroupNames map[uuid.UUID][]string, createMissingGroups bool) error {
-	if !api.Entitlements.Enabled(codersdk.FeatureTemplateRBAC) {
-		return nil
-	}
-
-	return db.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.
-		orgs, err := tx.GetOrganizationsByUserID(ctx, userID)
-		if err != nil {
-			return xerrors.Errorf("get user orgs: %w", err)
-		}
-		if len(orgs) != 1 {
-			return xerrors.Errorf("expected 1 org, got %d", len(orgs))
-		}
-
-		// Delete all groups the user belongs to.
-		// nolint:gocritic // Requires system context to remove user from all groups.
-		err = tx.RemoveUserFromAllGroups(dbauthz.AsSystemRestricted(ctx), userID)
-		if err != nil {
-			return xerrors.Errorf("delete user groups: %w", err)
-		}
-
-		// TODO: This could likely be improved by making these single queries.
-		// 	Either by batching or some other means. This for loop could be really
-		//	inefficient if there are a lot of organizations. There was deployments
-		//	on v1 with >100 orgs.
-		for orgID, groupNames := range orgGroupNames {
-			// Create the missing groups for each organization.
-			if createMissingGroups {
-				// This is the system creating these additional groups, so we use the system restricted context.
-				// nolint:gocritic
-				created, err := tx.InsertMissingGroups(dbauthz.AsSystemRestricted(ctx), database.InsertMissingGroupsParams{
-					OrganizationID: orgID,
-					GroupNames:     groupNames,
-					Source:         database.GroupSourceOidc,
-				})
-				if err != nil {
-					return xerrors.Errorf("insert missing groups: %w", err)
-				}
-				if len(created) > 0 {
-					logger.Debug(ctx, "auto created missing groups",
-						slog.F("org_id", orgID.ID),
-						slog.F("created", created),
-						slog.F("num", len(created)),
-					)
-				}
-			}
-
-			// Re-add the user to all groups returned by the auth provider.
-			err = tx.InsertUserGroupsByName(ctx, database.InsertUserGroupsByNameParams{
-				UserID:         userID,
-				OrganizationID: orgID,
-				GroupNames:     groupNames,
-			})
-			if err != nil {
-				return xerrors.Errorf("insert user groups: %w", err)
-			}
-		}
-
-		return nil
-	}, nil)
-}
-
 func (api *API) setUserSiteRoles(ctx context.Context, logger slog.Logger, db database.Store, userID uuid.UUID, roles []string) error {
 	if !api.Entitlements.Enabled(codersdk.FeatureUserRoleManagement) {
 		logger.Warn(ctx, "attempted to assign OIDC user roles without enterprise entitlement, roles left unchanged",

enterprise/coderd/userauth_test.go

@@ -402,7 +402,9 @@ func TestUserOIDC(t *testing.T) {
 			runner := setupOIDCTest(t, oidcTestConfig{
 				Config: func(cfg *coderd.OIDCConfig) {
 					cfg.AllowSignups = true
-					cfg.GroupField = groupClaim
+				},
+				DeploymentValues: func(dv *codersdk.DeploymentValues) {
+					dv.OIDC.GroupField = groupClaim
 				},
 			})
 
@@ -433,8 +435,10 @@ func TestUserOIDC(t *testing.T) {
 			runner := setupOIDCTest(t, oidcTestConfig{
 				Config: func(cfg *coderd.OIDCConfig) {
 					cfg.AllowSignups = true
-					cfg.GroupField = groupClaim
-					cfg.GroupMapping = map[string]string{oidcGroupName: coderGroupName}
+				},
+				DeploymentValues: func(dv *codersdk.DeploymentValues) {
+					dv.OIDC.GroupField = groupClaim
+					dv.OIDC.GroupMapping = serpent.Struct[map[string]string]{Value: map[string]string{oidcGroupName: coderGroupName}}
 				},
 			})
 
@@ -468,7 +472,9 @@ func TestUserOIDC(t *testing.T) {
 			runner := setupOIDCTest(t, oidcTestConfig{
 				Config: func(cfg *coderd.OIDCConfig) {
 					cfg.AllowSignups = true
-					cfg.GroupField = groupClaim
+				},
+				DeploymentValues: func(dv *codersdk.DeploymentValues) {
+					dv.OIDC.GroupField = groupClaim
 				},
 			})
 
@@ -502,7 +508,9 @@ func TestUserOIDC(t *testing.T) {
 			runner := setupOIDCTest(t, oidcTestConfig{
 				Config: func(cfg *coderd.OIDCConfig) {
 					cfg.AllowSignups = true
-					cfg.GroupField = groupClaim
+				},
+				DeploymentValues: func(dv *codersdk.DeploymentValues) {
+					dv.OIDC.GroupField = groupClaim
 				},
 			})
 
@@ -537,7 +545,9 @@ func TestUserOIDC(t *testing.T) {
 			runner := setupOIDCTest(t, oidcTestConfig{
 				Config: func(cfg *coderd.OIDCConfig) {
 					cfg.AllowSignups = true
-					cfg.GroupField = groupClaim
+				},
+				DeploymentValues: func(dv *codersdk.DeploymentValues) {
+					dv.OIDC.GroupField = groupClaim
 				},
 			})
 
@@ -559,8 +569,10 @@ func TestUserOIDC(t *testing.T) {
 			runner := setupOIDCTest(t, oidcTestConfig{
 				Config: func(cfg *coderd.OIDCConfig) {
 					cfg.AllowSignups = true
-					cfg.GroupField = groupClaim
-					cfg.CreateMissingGroups = true
+				},
+				DeploymentValues: func(dv *codersdk.DeploymentValues) {
+					dv.OIDC.GroupField = groupClaim
+					dv.OIDC.GroupAutoCreate = true
 				},
 			})
 
@@ -582,8 +594,10 @@ func TestUserOIDC(t *testing.T) {
 			runner := setupOIDCTest(t, oidcTestConfig{
 				Config: func(cfg *coderd.OIDCConfig) {
 					cfg.AllowSignups = true
-					cfg.GroupField = groupClaim
-					cfg.CreateMissingGroups = true
+				},
+				DeploymentValues: func(dv *codersdk.DeploymentValues) {
+					dv.OIDC.GroupField = groupClaim
+					dv.OIDC.GroupAutoCreate = true
 				},
 			})
 
@@ -606,8 +620,10 @@ func TestUserOIDC(t *testing.T) {
 			runner := setupOIDCTest(t, oidcTestConfig{
 				Config: func(cfg *coderd.OIDCConfig) {
 					cfg.AllowSignups = true
-					cfg.GroupField = groupClaim
-					cfg.GroupAllowList = map[string]bool{allowedGroup: true}
+				},
+				DeploymentValues: func(dv *codersdk.DeploymentValues) {
+					dv.OIDC.GroupField = groupClaim
+					dv.OIDC.GroupAllowList = []string{allowedGroup}
 				},
 			})
 
@@ -697,6 +713,7 @@ func TestGroupSync(t *testing.T) {
 	testCases := []struct {
 		name   string
 		modCfg func(cfg *coderd.OIDCConfig)
+		modDV  func(dv *codersdk.DeploymentValues)
 		// initialOrgGroups is initial groups in the org
 		initialOrgGroups []string
 		// initialUserGroups is initial groups for the user
@@ -718,10 +735,10 @@ func TestGroupSync(t *testing.T) {
 		},
 		{
 			name: "GroupSyncDisabled",
-			modCfg: func(cfg *coderd.OIDCConfig) {
+			modDV: func(dv *codersdk.DeploymentValues) {
 				// Disable group sync
-				cfg.GroupField = ""
-				cfg.GroupFilter = regexp.MustCompile(".*")
+				dv.OIDC.GroupField = ""
+				dv.OIDC.GroupRegexFilter = serpent.Regexp(*regexp.MustCompile(".*"))
 			},
 			initialOrgGroups:   []string{"a", "b", "c", "d"},
 			initialUserGroups:  []string{"b", "c", "d"},
@@ -732,10 +749,8 @@ func TestGroupSync(t *testing.T) {
 		{
 			// From a,c,b -> b,c,d
 			name: "ChangeUserGroups",
-			modCfg: func(cfg *coderd.OIDCConfig) {
-				cfg.GroupMapping = map[string]string{
-					"D": "d",
-				}
+			modDV: func(dv *codersdk.DeploymentValues) {
+				dv.OIDC.GroupMapping = serpent.Struct[map[string]string]{Value: map[string]string{"D": "d"}}
 			},
 			initialOrgGroups:   []string{"a", "b", "c", "d"},
 			initialUserGroups:  []string{"a", "b", "c"},
@@ -749,8 +764,8 @@ func TestGroupSync(t *testing.T) {
 		{
 			// From a,c,b -> []
 			name: "RemoveAllGroups",
-			modCfg: func(cfg *coderd.OIDCConfig) {
-				cfg.GroupFilter = regexp.MustCompile(".*")
+			modDV: func(dv *codersdk.DeploymentValues) {
+				dv.OIDC.GroupRegexFilter = serpent.Regexp(*regexp.MustCompile(".*"))
 			},
 			initialOrgGroups:   []string{"a", "b", "c", "d"},
 			initialUserGroups:  []string{"a", "b", "c"},
@@ -763,8 +778,8 @@ func TestGroupSync(t *testing.T) {
 		{
 			// From a,c,b -> b,c,d,e,f
 			name: "CreateMissingGroups",
-			modCfg: func(cfg *coderd.OIDCConfig) {
-				cfg.CreateMissingGroups = true
+			modDV: func(dv *codersdk.DeploymentValues) {
+				dv.OIDC.GroupAutoCreate = true
 			},
 			initialOrgGroups:   []string{"a", "b", "c", "d"},
 			initialUserGroups:  []string{"a", "b", "c"},
@@ -777,14 +792,11 @@ func TestGroupSync(t *testing.T) {
 		{
 			// From a,c,b -> b,c,d,e,f
 			name: "CreateMissingGroupsFilter",
-			modCfg: func(cfg *coderd.OIDCConfig) {
-				cfg.CreateMissingGroups = true
+			modDV: func(dv *codersdk.DeploymentValues) {
+				dv.OIDC.GroupAutoCreate = true
 				// Only single letter groups
-				cfg.GroupFilter = regexp.MustCompile("^[a-z]$")
-				cfg.GroupMapping = map[string]string{
-					// Does not match the filter, but does after being mapped!
-					"zebra": "z",
-				}
+				dv.OIDC.GroupRegexFilter = serpent.Regexp(*regexp.MustCompile("^[a-z]$"))
+				dv.OIDC.GroupMapping = serpent.Struct[map[string]string]{Value: map[string]string{"zebra": "z"}}
 			},
 			initialOrgGroups:   []string{"a", "b", "c", "d"},
 			initialUserGroups:  []string{"a", "b", "c"},
@@ -806,8 +818,15 @@ func TestGroupSync(t *testing.T) {
 			t.Parallel()
 			runner := setupOIDCTest(t, oidcTestConfig{
 				Config: func(cfg *coderd.OIDCConfig) {
-					cfg.GroupField = "groups"
-					tc.modCfg(cfg)
+					if tc.modCfg != nil {
+						tc.modCfg(cfg)
+					}
+				},
+				DeploymentValues: func(dv *codersdk.DeploymentValues) {
+					dv.OIDC.GroupField = "groups"
+					if tc.modDV != nil {
+						tc.modDV(dv)
+					}
 				},
 			})