switch oidc test config to deployment values

coder · Emyrk · Sep 11, 2024 · Sep 3, 2024 · Sep 3, 2024 · Sep 3, 2024
commit 986498d5fb0ad4fe8a79b481617a71a233b99db7
diff --git a/cli/server.go b/cli/server.go
@@ -187,11 +187,6 @@ func createOIDCConfig(ctx context.Context, logger slog.Logger, vals *codersdk.De
 		EmailField:          vals.OIDC.EmailField.String(),
 		AuthURLParams:       vals.OIDC.AuthURLParams.Value,
 		IgnoreUserInfo:      vals.OIDC.IgnoreUserInfo.Value(),
-		GroupField:          vals.OIDC.GroupField.String(),
-		GroupFilter:         vals.OIDC.GroupRegexFilter.Value(),
-		GroupAllowList:      groupAllowList,
-		CreateMissingGroups: vals.OIDC.GroupAutoCreate.Value(),
-		GroupMapping:        vals.OIDC.GroupMapping.Value,
 		UserRoleField:       vals.OIDC.UserRoleField.String(),
 		UserRoleMapping:     vals.OIDC.UserRoleMapping.Value,
 		UserRolesDefault:    vals.OIDC.UserRolesDefault.GetSlice(),

diff --git a/coderd/idpsync/group.go b/coderd/idpsync/group.go
@@ -41,6 +41,9 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
 		return nil
 	}
 
+	// nolint:gocritic // all syncing is done as a system user
+	ctx = dbauthz.AsSystemRestricted(ctx)
+
 	// Only care about the default org for deployment settings if the
 	// legacy deployment settings exist.
 	defaultOrgID := uuid.Nil
@@ -53,9 +56,6 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
 		defaultOrgID = defaultOrganization.ID
 	}
 
-	// nolint:gocritic // all syncing is done as a system user
-	ctx = dbauthz.AsSystemRestricted(ctx)
-
 	err := db.InTx(func(tx database.Store) error {
 		userGroups, err := tx.GetGroups(ctx, database.GetGroupsParams{
 			HasMemberID: user.ID,
@@ -86,12 +86,12 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
 			}
 
 			// Legacy deployment settings will override empty settings.
-			if orgID == defaultOrgID && settings.GroupField == "" {
+			if orgID == defaultOrgID && settings.Field == "" {
 				settings = &GroupSyncSettings{
-					GroupField:              s.Legacy.GroupField,
-					LegacyGroupNameMapping:  s.Legacy.GroupMapping,
-					RegexFilter:             s.Legacy.GroupFilter,
-					AutoCreateMissingGroups: s.Legacy.CreateMissingGroups,
+					Field:             s.Legacy.GroupField,
+					LegacyNameMapping: s.Legacy.GroupMapping,
+					RegexFilter:       s.Legacy.GroupFilter,
+					AutoCreateMissing: s.Legacy.CreateMissingGroups,
 				}
 			}
 			orgSettings[orgID] = *settings
@@ -102,7 +102,7 @@ func (s AGPLIDPSync) SyncGroups(ctx context.Context, db database.Store, user dat
 		groupIDsToRemove := make([]uuid.UUID, 0)
 		// For each org, determine which groups the user should land in
 		for orgID, settings := range orgSettings {
-			if settings.GroupField == "" {
+			if settings.Field == "" {
 				// No group sync enabled for this org, so do nothing.
 				continue
 			}
@@ -231,17 +231,25 @@ func (s AGPLIDPSync) ApplyGroupDifference(ctx context.Context, tx database.Store
 }
 
 type GroupSyncSettings struct {
-	GroupField string `json:"field"`
-	// GroupMapping maps from an OIDC group --> Coder group ID
-	GroupMapping            map[string][]uuid.UUID `json:"mapping"`
-	RegexFilter             *regexp.Regexp         `json:"regex_filter"`
-	AutoCreateMissingGroups bool                   `json:"auto_create_missing_groups"`
-	// LegacyGroupNameMapping is deprecated. It remaps an IDP group name to
+	// Field selects the claim field to be used as the created user's
+	// groups. If the group field is the empty string, then no group updates
+	// will ever come from the OIDC provider.
+	Field string `json:"field"`
+	// Mapping maps from an OIDC group --> Coder group ID
+	Mapping map[string][]uuid.UUID `json:"mapping"`
+	// RegexFilter is a regular expression that filters the groups returned by
+	// the OIDC provider. Any group not matched by this regex will be ignored.
+	// If the group filter is nil, then no group filtering will occur.
+	RegexFilter *regexp.Regexp `json:"regex_filter"`
+	// AutoCreateMissing controls whether groups returned by the OIDC provider
+	// are automatically created in Coder if they are missing.
+	AutoCreateMissing bool `json:"auto_create_missing_groups"`
+	// LegacyNameMapping is deprecated. It remaps an IDP group name to
 	// a Coder group name. Since configuration is now done at runtime,
 	// group IDs are used to account for group renames.
 	// For legacy configurations, this config option has to remain.
-	// Deprecated: Use GroupMapping instead.
-	LegacyGroupNameMapping map[string]string `json:"legacy_group_name_mapping,omitempty"`
+	// Deprecated: Use Mapping instead.
+	LegacyNameMapping map[string]string `json:"legacy_group_name_mapping,omitempty"`
 }
 
 func (s *GroupSyncSettings) Set(v string) error {
@@ -275,7 +283,7 @@ type ExpectedGroup struct {
 // We have to keep names because group sync supports syncing groups by name if
 // the external IDP group name matches the Coder one.
 func (s GroupSyncSettings) ParseClaims(orgID uuid.UUID, mergedClaims jwt.MapClaims) ([]ExpectedGroup, error) {
-	groupsRaw, ok := mergedClaims[s.GroupField]
+	groupsRaw, ok := mergedClaims[s.Field]
 	if !ok {
 		return []ExpectedGroup{}, nil
 	}
@@ -290,7 +298,7 @@ func (s GroupSyncSettings) ParseClaims(orgID uuid.UUID, mergedClaims jwt.MapClai
 		group := group
 
 		// Legacy group mappings happen before the regex filter.
-		mappedGroupName, ok := s.LegacyGroupNameMapping[group]
+		mappedGroupName, ok := s.LegacyNameMapping[group]
 		if ok {
 			group = mappedGroupName
 		}
@@ -302,7 +310,7 @@ func (s GroupSyncSettings) ParseClaims(orgID uuid.UUID, mergedClaims jwt.MapClai
 			}
 		}
 
-		mappedGroupIDs, ok := s.GroupMapping[group]
+		mappedGroupIDs, ok := s.Mapping[group]
 		if ok {
 			for _, gid := range mappedGroupIDs {
 				gid := gid
@@ -338,7 +346,7 @@ func (s GroupSyncSettings) HandleMissingGroups(ctx context.Context, tx database.
 		}
 	}
 
-	if s.AutoCreateMissingGroups && len(missingGroups) > 0 {
+	if s.AutoCreateMissing && len(missingGroups) > 0 {
 		// Insert any missing groups. If the groups already exist, this is a noop.
 		_, err := tx.InsertMissingGroups(ctx, database.InsertMissingGroupsParams{
 			OrganizationID: orgID,

diff --git a/coderd/idpsync/group_test.go b/coderd/idpsync/group_test.go
@@ -81,8 +81,8 @@ func TestGroupSyncTable(t *testing.T) {
 		{
 			Name: "SwitchGroups",
 			Settings: &idpsync.GroupSyncSettings{
-				GroupField: "groups",
-				GroupMapping: map[string][]uuid.UUID{
+				Field: "groups",
+				Mapping: map[string][]uuid.UUID{
 					"foo": {ids.ID("sg-foo"), ids.ID("sg-foo-2")},
 					"bar": {ids.ID("sg-bar")},
 					"baz": {ids.ID("sg-baz")},
@@ -107,10 +107,10 @@ func TestGroupSyncTable(t *testing.T) {
 		{
 			Name: "StayInGroup",
 			Settings: &idpsync.GroupSyncSettings{
-				GroupField: "groups",
+				Field: "groups",
 				// Only match foo, so bar does not map
 				RegexFilter: regexp.MustCompile("^foo$"),
-				GroupMapping: map[string][]uuid.UUID{
+				Mapping: map[string][]uuid.UUID{
 					"foo": {ids.ID("gg-foo"), uuid.New()},
 					"bar": {ids.ID("gg-bar")},
 					"baz": {ids.ID("gg-baz")},
@@ -127,8 +127,8 @@ func TestGroupSyncTable(t *testing.T) {
 		{
 			Name: "UserJoinsGroups",
 			Settings: &idpsync.GroupSyncSettings{
-				GroupField: "groups",
-				GroupMapping: map[string][]uuid.UUID{
+				Field: "groups",
+				Mapping: map[string][]uuid.UUID{
 					"foo": {ids.ID("ng-foo"), uuid.New()},
 					"bar": {ids.ID("ng-bar"), ids.ID("ng-bar-2")},
 					"baz": {ids.ID("ng-baz")},
@@ -150,9 +150,9 @@ func TestGroupSyncTable(t *testing.T) {
 		{
 			Name: "CreateGroups",
 			Settings: &idpsync.GroupSyncSettings{
-				GroupField:              "groups",
-				RegexFilter:             regexp.MustCompile("^create"),
-				AutoCreateMissingGroups: true,
+				Field:             "groups",
+				RegexFilter:       regexp.MustCompile("^create"),
+				AutoCreateMissing: true,
 			},
 			Groups: map[uuid.UUID]bool{},
 			ExpectedGroupNames: []string{
@@ -163,9 +163,9 @@ func TestGroupSyncTable(t *testing.T) {
 		{
 			Name: "GroupNamesNoMapping",
 			Settings: &idpsync.GroupSyncSettings{
-				GroupField:              "groups",
-				RegexFilter:             regexp.MustCompile(".*"),
-				AutoCreateMissingGroups: false,
+				Field:             "groups",
+				RegexFilter:       regexp.MustCompile(".*"),
+				AutoCreateMissing: false,
 			},
 			GroupNames: map[string]bool{
 				"foo":  false,
@@ -180,13 +180,13 @@ func TestGroupSyncTable(t *testing.T) {
 		{
 			Name: "NoUser",
 			Settings: &idpsync.GroupSyncSettings{
-				GroupField: "groups",
-				GroupMapping: map[string][]uuid.UUID{
+				Field: "groups",
+				Mapping: map[string][]uuid.UUID{
 					// Extra ID that does not map to a group
 					"foo": {ids.ID("ow-foo"), uuid.New()},
 				},
-				RegexFilter:             nil,
-				AutoCreateMissingGroups: false,
+				RegexFilter:       nil,
+				AutoCreateMissing: false,
 			},
 			NotMember: true,
 			Groups: map[uuid.UUID]bool{
@@ -202,14 +202,14 @@ func TestGroupSyncTable(t *testing.T) {
 		{
 			Name: "LegacyMapping",
 			Settings: &idpsync.GroupSyncSettings{
-				GroupField:  "groups",
+				Field:       "groups",
 				RegexFilter: regexp.MustCompile("^legacy"),
-				LegacyGroupNameMapping: map[string]string{
+				LegacyNameMapping: map[string]string{
 					"create-bar": "legacy-bar",
 					"foo":        "legacy-foo",
 					"bop":        "legacy-bop",
 				},
-				AutoCreateMissingGroups: true,
+				AutoCreateMissing: true,
 			},
 			Groups: map[uuid.UUID]bool{
 				ids.ID("lg-foo"): true,

diff --git a/coderd/userauth.go b/coderd/userauth.go
@@ -8,7 +8,6 @@ import (
 	"fmt"
 	"net/http"
 	"net/mail"
-	"regexp"
 	"sort"
 	"strconv"
 	"strings"
@@ -659,7 +658,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
 		Name:         normName,
 		DebugContext: OauthDebugContext{},
 		GroupSync: idpsync.GroupParams{
-			SyncEnabled:    false,
+			SyncEnabled: false,
 		},
 		OrganizationSync: idpsync.OrganizationParams{
 			SyncEnabled:    false,
@@ -743,27 +742,6 @@ type OIDCConfig struct {
 	// support the userinfo endpoint, or if the userinfo endpoint causes
 	// undesirable behavior.
 	IgnoreUserInfo bool
-
-	// TODO: Move all idp fields into the IDPSync struct
-	// GroupField selects the claim field to be used as the created user's
-	// groups. If the group field is the empty string, then no group updates
-	// will ever come from the OIDC provider.
-	GroupField string
-	// CreateMissingGroups controls whether groups returned by the OIDC provider
-	// are automatically created in Coder if they are missing.
-	CreateMissingGroups bool
-	// GroupFilter is a regular expression that filters the groups returned by
-	// the OIDC provider. Any group not matched by this regex will be ignored.
-	// If the group filter is nil, then no group filtering will occur.
-	GroupFilter *regexp.Regexp
-	// GroupAllowList is a list of groups that are allowed to log in.
-	// If the list length is 0, then the allow list will not be applied and
-	// this feature is disabled.
-	GroupAllowList map[string]bool
-	// GroupMapping controls how groups returned by the OIDC provider get mapped
-	// to groups within Coder.
-	// map[oidcGroupName]coderGroupName
-	GroupMapping map[string]string
 	// UserRoleField selects the claim field to be used as the created user's
 	// roles. If the field is the empty string, then no role updates
 	// will ever come from the OIDC provider.