From c389747f1a0807469c87275cddbc001a7525d265 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 2 Aug 2023 10:54:48 -0500 Subject: [PATCH 01/18] add flag for auto create groups --- cli/server.go | 1 + coderd/userauth.go | 35 ++++++++++++++++++++--------------- codersdk/deployment.go | 11 +++++++++++ 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/cli/server.go b/cli/server.go index 170b7c5eb9f00..dc12637bc8682 100644 --- a/cli/server.go +++ b/cli/server.go @@ -596,6 +596,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. AuthURLParams: cfg.OIDC.AuthURLParams.Value, IgnoreUserInfo: cfg.OIDC.IgnoreUserInfo.Value(), GroupField: cfg.OIDC.GroupField.String(), + CreateMissingGroups: cfg.OIDC.GroupAutoCreate.Value(), GroupMapping: cfg.OIDC.GroupMapping.Value, UserRoleField: cfg.OIDC.UserRoleField.String(), UserRoleMapping: cfg.OIDC.UserRoleMapping.Value, diff --git a/coderd/userauth.go b/coderd/userauth.go index 9b6ba7992bad5..aac0c5f87c1ff 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -688,6 +688,9 @@ type OIDCConfig struct { // 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 // GroupMapping controls how groups returned by the OIDC provider get mapped // to groups within Coder. // map[oidcGroupName]coderGroupName @@ -1029,19 +1032,20 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { } params := (&oauthLoginParams{ - User: user, - Link: link, - State: state, - LinkedID: oidcLinkedID(idToken), - LoginType: database.LoginTypeOIDC, - AllowSignups: api.OIDCConfig.AllowSignups, - Email: email, - Username: username, - AvatarURL: picture, - UsingGroups: usingGroups, - UsingRoles: api.OIDCConfig.RoleSyncEnabled(), - Roles: roles, - Groups: groups, + User: user, + Link: link, + State: state, + LinkedID: oidcLinkedID(idToken), + LoginType: database.LoginTypeOIDC, + AllowSignups: api.OIDCConfig.AllowSignups, + Email: email, + Username: username, + AvatarURL: picture, + UsingGroups: usingGroups, + UsingRoles: api.OIDCConfig.RoleSyncEnabled(), + Roles: roles, + Groups: groups, + CreateMissingGroups: api.OIDCConfig.CreateMissingGroups, }).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) { return audit.InitRequest[database.User](rw, params) }) @@ -1125,8 +1129,9 @@ type oauthLoginParams struct { AvatarURL string // Is UsingGroups is true, then the user will be assigned // to the Groups provided. - UsingGroups bool - Groups []string + UsingGroups bool + CreateMissingGroups bool + Groups []string // Is UsingRoles is true, then the user will be assigned // the roles provided. UsingRoles bool diff --git a/codersdk/deployment.go b/codersdk/deployment.go index e27731b6aa6bf..af73392b6db83 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -271,6 +271,7 @@ type OIDCConfig struct { EmailField clibase.String `json:"email_field" typescript:",notnull"` AuthURLParams clibase.Struct[map[string]string] `json:"auth_url_params" typescript:",notnull"` IgnoreUserInfo clibase.Bool `json:"ignore_user_info" typescript:",notnull"` + GroupAutoCreate clibase.Bool `json:"group_auto_create" typescript:",notnull"` GroupField clibase.String `json:"groups_field" typescript:",notnull"` GroupMapping clibase.Struct[map[string]string] `json:"group_mapping" typescript:",notnull"` UserRoleField clibase.String `json:"user_role_field" typescript:",notnull"` @@ -1065,6 +1066,16 @@ when required by your organization's security policy.`, Group: &deploymentGroupOIDC, YAML: "groupMapping", }, + { + Name: "Enable OIDC Group Auto Create", + Description: "Automatically creates missing groups from a user's groups claim.", + Flag: "oidc-group-auto-create", + Env: "CODER_OIDC_GROUP_AUTO_CREATE", + Default: "false", + Value: &c.OIDC.GroupAutoCreate, + Group: &deploymentGroupOIDC, + YAML: "enableGroupAutoCreate", + }, { Name: "OIDC User Role Field", Description: "This field must be set if using the user roles sync feature. Set this to the name of the claim used to store the user's role. The roles should be sent as an array of strings.", From d72745145b0521d301209d1d4ff794ccb448b5bc Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 2 Aug 2023 10:56:39 -0500 Subject: [PATCH 02/18] fixup! add flag for auto create groups --- coderd/coderd.go | 6 +++--- enterprise/coderd/userauth.go | 4 +++- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index d7b80ff273097..aa9d63fcbdc6d 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -126,7 +126,7 @@ type Options struct { BaseDERPMap *tailcfg.DERPMap DERPMapUpdateFrequency time.Duration SwaggerEndpoint bool - SetUserGroups func(ctx context.Context, tx database.Store, userID uuid.UUID, groupNames []string) error + SetUserGroups func(ctx context.Context, tx database.Store, userID uuid.UUID, groupNames []string, createMissingGroups bool) error SetUserSiteRoles func(ctx context.Context, tx database.Store, userID uuid.UUID, roles []string) error TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore] UserQuietHoursScheduleStore *atomic.Pointer[schedule.UserQuietHoursScheduleStore] @@ -258,9 +258,9 @@ func New(options *Options) *API { options.TracerProvider = trace.NewNoopTracerProvider() } if options.SetUserGroups == nil { - options.SetUserGroups = func(ctx context.Context, _ database.Store, userID uuid.UUID, groups []string) error { + options.SetUserGroups = func(ctx context.Context, _ database.Store, userID uuid.UUID, groups []string, createMissingGroups bool) error { options.Logger.Warn(ctx, "attempted to assign OIDC groups without enterprise license", - slog.F("user_id", userID), slog.F("groups", groups), + slog.F("user_id", userID), slog.F("groups", groups), slog.F("create_missing_groups", createMissingGroups), ) return nil } diff --git a/enterprise/coderd/userauth.go b/enterprise/coderd/userauth.go index 86aa9f0ddf88b..e1c2dbd2fc0d1 100644 --- a/enterprise/coderd/userauth.go +++ b/enterprise/coderd/userauth.go @@ -12,7 +12,7 @@ import ( "github.com/coder/coder/codersdk" ) -func (api *API) setUserGroups(ctx context.Context, db database.Store, userID uuid.UUID, groupNames []string) error { +func (api *API) setUserGroups(ctx context.Context, db database.Store, userID uuid.UUID, groupNames []string, createMissingGroups bool) error { api.entitlementsMu.RLock() enabled := api.entitlements.Features[codersdk.FeatureTemplateRBAC].Enabled api.entitlementsMu.RUnlock() @@ -39,6 +39,8 @@ func (api *API) setUserGroups(ctx context.Context, db database.Store, userID uui return xerrors.Errorf("delete user groups: %w", err) } + // TODO: Create missing groups if createMissingGroups is true. + // Re-add the user to all groups returned by the auth provider. err = tx.InsertUserGroupsByName(ctx, database.InsertUserGroupsByNameParams{ UserID: userID, From f56113e7787fd1b32c8c4dfb6ae6c9d8d0b2d26f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 3 Aug 2023 11:08:18 -0500 Subject: [PATCH 03/18] sync missing groups Also added a regex filter to filter out groups that are not important --- cli/clibase/values.go | 31 ++++ cli/server.go | 1 + coderd/coderd.go | 12 +- coderd/database/dbauthz/dbauthz.go | 7 + coderd/database/dbfake/dbfake.go | 38 +++++ coderd/database/dbmetrics/dbmetrics.go | 7 + coderd/database/querier.go | 2 + coderd/database/queries.sql.go | 52 +++++++ coderd/database/queries/groups.sql | 17 +++ coderd/userauth.go | 21 ++- codersdk/deployment.go | 11 ++ enterprise/coderd/userauth.go | 25 +++- enterprise/coderd/userauth_test.go | 200 ++++++++++++++++++++++++- 13 files changed, 410 insertions(+), 14 deletions(-) diff --git a/cli/clibase/values.go b/cli/clibase/values.go index 288a7c372b152..9b215d613fa9d 100644 --- a/cli/clibase/values.go +++ b/cli/clibase/values.go @@ -7,6 +7,7 @@ import ( "net" "net/url" "reflect" + "regexp" "strconv" "strings" "time" @@ -461,6 +462,36 @@ func (e *Enum) String() string { return *e.Value } +type Regexp regexp.Regexp + +func RegexpOf(s *regexp.Regexp) *Regexp { + return (*Regexp)(s) +} + +func (s *Regexp) Set(v string) error { + exp, err := regexp.Compile(v) + if err != nil { + return xerrors.Errorf("invalid regexp %q: %w", v, err) + } + *s = Regexp(*exp) + return nil +} + +func (s Regexp) String() string { + return s.Value().String() +} + +func (s *Regexp) Value() *regexp.Regexp { + if s == nil { + return nil + } + return (*regexp.Regexp)(s) +} + +func (Regexp) Type() string { + return "regexp" +} + var _ pflag.Value = (*YAMLConfigPath)(nil) // YAMLConfigPath is a special value type that encodes a path to a YAML diff --git a/cli/server.go b/cli/server.go index dc12637bc8682..863a850792c49 100644 --- a/cli/server.go +++ b/cli/server.go @@ -596,6 +596,7 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. AuthURLParams: cfg.OIDC.AuthURLParams.Value, IgnoreUserInfo: cfg.OIDC.IgnoreUserInfo.Value(), GroupField: cfg.OIDC.GroupField.String(), + GroupFilter: cfg.OIDC.GroupRegexFilter.Value(), CreateMissingGroups: cfg.OIDC.GroupAutoCreate.Value(), GroupMapping: cfg.OIDC.GroupMapping.Value, UserRoleField: cfg.OIDC.UserRoleField.String(), diff --git a/coderd/coderd.go b/coderd/coderd.go index aa9d63fcbdc6d..f43480e9f67a4 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -126,8 +126,8 @@ type Options struct { BaseDERPMap *tailcfg.DERPMap DERPMapUpdateFrequency time.Duration SwaggerEndpoint bool - SetUserGroups func(ctx context.Context, tx database.Store, userID uuid.UUID, groupNames []string, createMissingGroups bool) error - SetUserSiteRoles func(ctx context.Context, tx database.Store, userID uuid.UUID, roles []string) error + SetUserGroups func(ctx context.Context, logger slog.Logger, tx database.Store, userID uuid.UUID, groupNames []string, createMissingGroups bool) error + SetUserSiteRoles func(ctx context.Context, logger slog.Logger, tx database.Store, userID uuid.UUID, roles []string) error TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore] UserQuietHoursScheduleStore *atomic.Pointer[schedule.UserQuietHoursScheduleStore] // AppSecurityKey is the crypto key used to sign and encrypt tokens related to @@ -258,16 +258,16 @@ func New(options *Options) *API { options.TracerProvider = trace.NewNoopTracerProvider() } if options.SetUserGroups == nil { - options.SetUserGroups = func(ctx context.Context, _ database.Store, userID uuid.UUID, groups []string, createMissingGroups bool) error { - options.Logger.Warn(ctx, "attempted to assign OIDC groups without enterprise license", + options.SetUserGroups = func(ctx context.Context, logger slog.Logger, _ database.Store, userID uuid.UUID, groups []string, createMissingGroups bool) error { + logger.Warn(ctx, "attempted to assign OIDC groups without enterprise license", slog.F("user_id", userID), slog.F("groups", groups), slog.F("create_missing_groups", createMissingGroups), ) return nil } } if options.SetUserSiteRoles == nil { - options.SetUserSiteRoles = func(ctx context.Context, _ database.Store, userID uuid.UUID, roles []string) error { - options.Logger.Warn(ctx, "attempted to assign OIDC user roles without enterprise license", + options.SetUserSiteRoles = func(ctx context.Context, logger slog.Logger, _ database.Store, userID uuid.UUID, roles []string) error { + logger.Warn(ctx, "attempted to assign OIDC user roles without enterprise license", slog.F("user_id", userID), slog.F("roles", roles), ) return nil diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 56e7c3d273655..37e4ae10c021f 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1834,6 +1834,13 @@ func (q *querier) InsertLicense(ctx context.Context, arg database.InsertLicenseP return q.db.InsertLicense(ctx, arg) } +func (q *querier) InsertMissingGroups(ctx context.Context, arg database.InsertMissingGroupsParams) ([]database.Group, error) { + if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceSystem); err != nil { + return nil, err + } + return q.db.InsertMissingGroups(ctx, arg) +} + func (q *querier) InsertOrganization(ctx context.Context, arg database.InsertOrganizationParams) (database.Organization, error) { return insert(q.log, q.auth, rbac.ResourceOrganization, q.db.InsertOrganization)(ctx, arg) } diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 41027f67acc78..a12c6f8df99d5 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -3578,6 +3578,44 @@ func (q *FakeQuerier) InsertLicense( return l, nil } +func (q *FakeQuerier) InsertMissingGroups(ctx context.Context, arg database.InsertMissingGroupsParams) ([]database.Group, error) { + err := validateDatabaseType(arg) + if err != nil { + return nil, err + } + + groupNameMap := make(map[string]struct{}) + for _, g := range arg.GroupNames { + groupNameMap[g] = struct{}{} + } + + q.mutex.Lock() + defer q.mutex.Unlock() + + for _, g := range q.groups { + if g.OrganizationID != arg.OrganizationID { + continue + } + delete(groupNameMap, g.Name) + } + + newGroups := make([]database.Group, 0, len(groupNameMap)) + for k := range groupNameMap { + g := database.Group{ + ID: uuid.New(), + Name: k, + OrganizationID: arg.OrganizationID, + AvatarURL: "", + QuotaAllowance: 0, + DisplayName: "", + } + q.groups = append(q.groups, g) + newGroups = append(newGroups, g) + } + + return newGroups, nil +} + func (q *FakeQuerier) InsertOrganization(_ context.Context, arg database.InsertOrganizationParams) (database.Organization, error) { if err := validateDatabaseType(arg); err != nil { return database.Organization{}, err diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index d9e8ef6d3c2ce..c721824b89395 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -1103,6 +1103,13 @@ func (m metricsStore) InsertLicense(ctx context.Context, arg database.InsertLice return license, err } +func (m metricsStore) InsertMissingGroups(ctx context.Context, arg database.InsertMissingGroupsParams) ([]database.Group, error) { + start := time.Now() + r0 := m.s.InsertMissingGroups(ctx, arg) + m.queryLatencies.WithLabelValues("InsertMissingGroups").Observe(time.Since(start).Seconds()) + return r0 +} + func (m metricsStore) InsertOrganization(ctx context.Context, arg database.InsertOrganizationParams) (database.Organization, error) { start := time.Now() organization, err := m.s.InsertOrganization(ctx, arg) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index c53c879dba5e7..bae7939b99e64 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -201,6 +201,8 @@ type sqlcQuerier interface { InsertGroup(ctx context.Context, arg InsertGroupParams) (Group, error) InsertGroupMember(ctx context.Context, arg InsertGroupMemberParams) error InsertLicense(ctx context.Context, arg InsertLicenseParams) (License, error) + // If the name conflicts, do nothing. + InsertMissingGroups(ctx context.Context, arg InsertMissingGroupsParams) ([]Group, error) InsertOrganization(ctx context.Context, arg InsertOrganizationParams) (Organization, error) InsertOrganizationMember(ctx context.Context, arg InsertOrganizationMemberParams) (OrganizationMember, error) InsertProvisionerDaemon(ctx context.Context, arg InsertProvisionerDaemonParams) (ProvisionerDaemon, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 53ddec2231321..2fa46e051fd84 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1346,6 +1346,58 @@ func (q *sqlQuerier) InsertGroup(ctx context.Context, arg InsertGroupParams) (Gr return i, err } +const insertMissingGroups = `-- name: InsertMissingGroups :many +INSERT INTO groups ( + id, + name, + organization_id +) +SELECT + gen_random_uuid(), + group_name, + $1 +FROM + UNNEST($2 :: text[]) AS group_name +ON CONFLICT DO NOTHING +RETURNING id, name, organization_id, avatar_url, quota_allowance, display_name +` + +type InsertMissingGroupsParams struct { + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + GroupNames []string `db:"group_names" json:"group_names"` +} + +// If the name conflicts, do nothing. +func (q *sqlQuerier) InsertMissingGroups(ctx context.Context, arg InsertMissingGroupsParams) ([]Group, error) { + rows, err := q.db.QueryContext(ctx, insertMissingGroups, arg.OrganizationID, pq.Array(arg.GroupNames)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []Group + for rows.Next() { + var i Group + if err := rows.Scan( + &i.ID, + &i.Name, + &i.OrganizationID, + &i.AvatarURL, + &i.QuotaAllowance, + &i.DisplayName, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const updateGroupByID = `-- name: UpdateGroupByID :one UPDATE groups diff --git a/coderd/database/queries/groups.sql b/coderd/database/queries/groups.sql index e1ee6635a5fe0..afa3786740977 100644 --- a/coderd/database/queries/groups.sql +++ b/coderd/database/queries/groups.sql @@ -42,6 +42,23 @@ INSERT INTO groups ( VALUES ($1, $2, $3, $4, $5, $6) RETURNING *; +-- name: InsertMissingGroups :many +INSERT INTO groups ( + id, + name, + organization_id +) +SELECT + gen_random_uuid(), + group_name, + @organization_id +FROM + UNNEST(@group_names :: text[]) AS group_name +-- If the name conflicts, do nothing. +ON CONFLICT DO NOTHING +RETURNING *; + + -- We use the organization_id as the id -- for simplicity since all users is -- every member of the org. diff --git a/coderd/userauth.go b/coderd/userauth.go index aac0c5f87c1ff..3a31fc60167a9 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -7,6 +7,7 @@ import ( "fmt" "net/http" "net/mail" + "regexp" "sort" "strconv" "strings" @@ -691,6 +692,10 @@ type OIDCConfig struct { // 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 // GroupMapping controls how groups returned by the OIDC provider get mapped // to groups within Coder. // map[oidcGroupName]coderGroupName @@ -1046,6 +1051,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { Roles: roles, Groups: groups, CreateMissingGroups: api.OIDCConfig.CreateMissingGroups, + GroupFilter: api.OIDCConfig.GroupFilter, }).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) { return audit.InitRequest[database.User](rw, params) }) @@ -1132,6 +1138,7 @@ type oauthLoginParams struct { UsingGroups bool CreateMissingGroups bool Groups []string + GroupFilter *regexp.Regexp // Is UsingRoles is true, then the user will be assigned // the roles provided. UsingRoles bool @@ -1347,8 +1354,18 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C // Ensure groups are correct. if params.UsingGroups { + filtered := params.Groups + if params.GroupFilter != nil { + filtered = make([]string, 0, len(params.Groups)) + for _, group := range params.Groups { + if params.GroupFilter.MatchString(group) { + filtered = append(filtered, group) + } + } + } + //nolint:gocritic - err := api.Options.SetUserGroups(dbauthz.AsSystemRestricted(ctx), tx, user.ID, params.Groups) + err := api.Options.SetUserGroups(dbauthz.AsSystemRestricted(ctx), logger, tx, user.ID, filtered, params.CreateMissingGroups) if err != nil { return xerrors.Errorf("set user groups: %w", err) } @@ -1367,7 +1384,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C } //nolint:gocritic - err := api.Options.SetUserSiteRoles(dbauthz.AsSystemRestricted(ctx), tx, user.ID, filtered) + err := api.Options.SetUserSiteRoles(dbauthz.AsSystemRestricted(ctx), logger, tx, user.ID, filtered) if err != nil { return httpError{ code: http.StatusBadRequest, diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 9403c37bb14a4..f3fa17d84e893 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -272,6 +272,7 @@ type OIDCConfig struct { AuthURLParams clibase.Struct[map[string]string] `json:"auth_url_params" typescript:",notnull"` IgnoreUserInfo clibase.Bool `json:"ignore_user_info" typescript:",notnull"` GroupAutoCreate clibase.Bool `json:"group_auto_create" typescript:",notnull"` + GroupRegexFilter clibase.Regexp `json:"group_regex_filter" typescript:",notnull"` GroupField clibase.String `json:"groups_field" typescript:",notnull"` GroupMapping clibase.Struct[map[string]string] `json:"group_mapping" typescript:",notnull"` UserRoleField clibase.String `json:"user_role_field" typescript:",notnull"` @@ -1076,6 +1077,16 @@ when required by your organization's security policy.`, Group: &deploymentGroupOIDC, YAML: "enableGroupAutoCreate", }, + { + Name: "OIDC Regex Group Filter", + Description: "If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed.", + Flag: "oidc-group-regex-filter", + Env: "CODER_OIDC_GROUP_REGEX_FILTER", + Default: "", + Value: &c.OIDC.GroupRegexFilter, + Group: &deploymentGroupOIDC, + YAML: "groupRegexFilter", + }, { Name: "OIDC User Role Field", Description: "This field must be set if using the user roles sync feature. Set this to the name of the claim used to store the user's role. The roles should be sent as an array of strings.", diff --git a/enterprise/coderd/userauth.go b/enterprise/coderd/userauth.go index e1c2dbd2fc0d1..a26c26554d5d1 100644 --- a/enterprise/coderd/userauth.go +++ b/enterprise/coderd/userauth.go @@ -9,10 +9,11 @@ import ( "cdr.dev/slog" "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/codersdk" ) -func (api *API) setUserGroups(ctx context.Context, db database.Store, userID uuid.UUID, groupNames []string, createMissingGroups bool) error { +func (api *API) setUserGroups(ctx context.Context, logger slog.Logger, db database.Store, userID uuid.UUID, groupNames []string, createMissingGroups bool) error { api.entitlementsMu.RLock() enabled := api.entitlements.Features[codersdk.FeatureTemplateRBAC].Enabled api.entitlementsMu.RUnlock() @@ -39,7 +40,23 @@ func (api *API) setUserGroups(ctx context.Context, db database.Store, userID uui return xerrors.Errorf("delete user groups: %w", err) } - // TODO: Create missing groups if createMissingGroups is true. + 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: orgs[0].ID, + GroupNames: groupNames, + }) + 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", orgs[0].ID), + slog.F("created", created), + ) + } + } // Re-add the user to all groups returned by the auth provider. err = tx.InsertUserGroupsByName(ctx, database.InsertUserGroupsByNameParams{ @@ -55,13 +72,13 @@ func (api *API) setUserGroups(ctx context.Context, db database.Store, userID uui }, nil) } -func (api *API) setUserSiteRoles(ctx context.Context, db database.Store, userID uuid.UUID, roles []string) error { +func (api *API) setUserSiteRoles(ctx context.Context, logger slog.Logger, db database.Store, userID uuid.UUID, roles []string) error { api.entitlementsMu.RLock() enabled := api.entitlements.Features[codersdk.FeatureUserRoleManagement].Enabled api.entitlementsMu.RUnlock() if !enabled { - api.Logger.Warn(ctx, "attempted to assign OIDC user roles without enterprise entitlement, roles left unchanged", + logger.Warn(ctx, "attempted to assign OIDC user roles without enterprise entitlement, roles left unchanged", slog.F("user_id", userID), slog.F("roles", roles), ) return nil diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 428cf91a6fef2..d7c27d5515774 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -5,10 +5,9 @@ import ( "fmt" "io" "net/http" + "regexp" "testing" - "github.com/coder/coder/enterprise/coderd/license" - "github.com/golang-jwt/jwt" "github.com/google/uuid" "github.com/stretchr/testify/assert" @@ -16,9 +15,13 @@ import ( "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/database" + "github.com/coder/coder/coderd/database/dbauthz" "github.com/coder/coder/coderd/rbac" + "github.com/coder/coder/coderd/util/slice" "github.com/coder/coder/codersdk" "github.com/coder/coder/enterprise/coderd/coderdenttest" + "github.com/coder/coder/enterprise/coderd/license" "github.com/coder/coder/testutil" ) @@ -352,6 +355,199 @@ func TestUserOIDC(t *testing.T) { }) } +func TestGroupSync(t *testing.T) { + t.Parallel() + + testCases := []struct { + name string + modCfg func(cfg *coderd.OIDCConfig) + // initialOrgGroups is initial groups in the org + initialOrgGroups []string + // initialUserGroups is initial groups for the user + initialUserGroups []string + // expectedUserGroups is expected groups for the user + expectedUserGroups []string + // expectedOrgGroups is expected all groups on the system + expectedOrgGroups []string + claims jwt.MapClaims + }{ + { + name: "NoGroups", + modCfg: func(cfg *coderd.OIDCConfig) { + + }, + initialOrgGroups: []string{}, + expectedUserGroups: []string{}, + expectedOrgGroups: []string{}, + claims: jwt.MapClaims{}, + }, + { + name: "GroupSyncDisabled", + modCfg: func(cfg *coderd.OIDCConfig) { + // Disable group sync + cfg.GroupField = "" + }, + initialOrgGroups: []string{"a", "b", "c", "d"}, + initialUserGroups: []string{"b", "c", "d"}, + expectedUserGroups: []string{"b", "c", "d"}, + expectedOrgGroups: []string{"a", "b", "c", "d"}, + claims: jwt.MapClaims{}, + }, + { + // From a,c,b -> b,c,d + name: "ChangeUserGroups", + modCfg: func(cfg *coderd.OIDCConfig) { + cfg.GroupMapping = map[string]string{ + "D": "d", + } + }, + initialOrgGroups: []string{"a", "b", "c", "d"}, + initialUserGroups: []string{"a", "b", "c"}, + expectedUserGroups: []string{"b", "c", "d"}, + expectedOrgGroups: []string{"a", "b", "c", "d"}, + claims: jwt.MapClaims{ + // D -> d mapped + "groups": []string{"b", "c", "D"}, + }, + }, + { + // From a,c,b -> [] + name: "RemoveAllGroups", + modCfg: func(cfg *coderd.OIDCConfig) { + }, + initialOrgGroups: []string{"a", "b", "c", "d"}, + initialUserGroups: []string{"a", "b", "c"}, + expectedUserGroups: []string{}, + expectedOrgGroups: []string{"a", "b", "c", "d"}, + claims: jwt.MapClaims{ + // No claim == no groups + }, + }, + { + // From a,c,b -> b,c,d,e,f + name: "CreateMissingGroups", + modCfg: func(cfg *coderd.OIDCConfig) { + cfg.CreateMissingGroups = true + }, + initialOrgGroups: []string{"a", "b", "c", "d"}, + initialUserGroups: []string{"a", "b", "c"}, + expectedUserGroups: []string{"b", "c", "d", "e", "f"}, + expectedOrgGroups: []string{"a", "b", "c", "d", "e", "f"}, + claims: jwt.MapClaims{ + "groups": []string{"b", "c", "d", "e", "f"}, + }, + }, + { + // From a,c,b -> b,c,d,e,f + name: "CreateMissingGroupsFilter", + modCfg: func(cfg *coderd.OIDCConfig) { + cfg.CreateMissingGroups = true + // Only single letter groups + cfg.GroupFilter = regexp.MustCompile("^[a-z]$") + }, + initialOrgGroups: []string{"a", "b", "c", "d"}, + initialUserGroups: []string{"a", "b", "c"}, + expectedUserGroups: []string{"b", "c", "d", "e", "f"}, + expectedOrgGroups: []string{"a", "b", "c", "d", "e", "f"}, + claims: jwt.MapClaims{ + "groups": []string{ + "b", "c", "d", "e", "f", + // These groups are ignored + "excess", "ignore", "dumb", "foobar", + }, + }, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + ctx := testutil.Context(t, testutil.WaitLong*40000) + conf := coderdtest.NewOIDCConfig(t, "") + + config := conf.OIDCConfig(t, jwt.MapClaims{}, tc.modCfg) + + client, _, api, _ := coderdenttest.NewWithAPI(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + OIDCConfig: config, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{codersdk.FeatureTemplateRBAC: 1}, + }, + }) + + admin, err := client.User(ctx, "me") + require.NoError(t, err) + require.Len(t, admin.OrganizationIDs, 1) + + // Setup + initialGroups := make(map[string]codersdk.Group) + for _, group := range tc.initialOrgGroups { + newGroup, err := client.CreateGroup(ctx, admin.OrganizationIDs[0], codersdk.CreateGroupRequest{ + Name: group, + }) + require.NoError(t, err) + require.Len(t, newGroup.Members, 0) + initialGroups[group] = newGroup + } + + // Create the user and add them to their initial groups + _, user := coderdtest.CreateAnotherUser(t, client, admin.OrganizationIDs[0]) + for _, group := range tc.initialUserGroups { + _, err := client.PatchGroup(ctx, initialGroups[group].ID, codersdk.PatchGroupRequest{ + AddUsers: []string{user.ID.String()}, + }) + require.NoError(t, err) + } + + // nolint:gocritic + _, err = api.Database.UpdateUserLoginType(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLoginTypeParams{ + NewLoginType: database.LoginTypeOIDC, + UserID: user.ID, + }) + require.NoError(t, err, "user must be oidc type") + + // Log in the new user + tc.claims["email"] = user.Email + resp := oidcCallback(t, client, conf.EncodeClaims(t, tc.claims)) + assert.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + + orgGroups, err := client.GroupsByOrganization(ctx, admin.OrganizationIDs[0]) + require.NoError(t, err) + + orgGroupsMap := make(map[string]struct{}) + for _, group := range orgGroups { + orgGroupsMap[group.Name] = struct{}{} + } + + for _, expected := range tc.expectedOrgGroups { + if _, ok := orgGroupsMap[expected]; !ok { + t.Errorf("expected group %s not found", expected) + } + delete(orgGroupsMap, expected) + } + require.Empty(t, orgGroupsMap, "unexpected groups found") + + expectedUserGroups := make(map[string]struct{}) + for _, group := range tc.expectedUserGroups { + expectedUserGroups[group] = struct{}{} + } + + for _, group := range orgGroups { + userInGroup := slice.ContainsCompare(group.Members, codersdk.User{Email: user.Email}, func(a, b codersdk.User) bool { + return a.Email == b.Email + }) + if _, ok := expectedUserGroups[group.Name]; ok { + require.Truef(t, userInGroup, "user should be in group %s", group.Name) + } else { + require.Falsef(t, userInGroup, "user should not be in group %s", group.Name) + } + } + }) + } +} + func oidcCallback(t *testing.T, client *codersdk.Client, code string) *http.Response { t.Helper() client.HTTPClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { From ec6ac573735a8814d511747b17b38e0b00033f75 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 3 Aug 2023 11:21:52 -0500 Subject: [PATCH 04/18] fixup! sync missing groups --- coderd/database/dbfake/dbfake.go | 2 + coderd/database/dump.sql | 10 +++- .../migrations/000148_group_source.down.sql | 8 +++ .../migrations/000148_group_source.up.sql | 15 +++++ coderd/database/models.go | 60 +++++++++++++++++++ coderd/database/queries.sql.go | 36 +++++++---- coderd/database/queries/groups.sql | 6 +- codersdk/deployment.go | 2 +- codersdk/groups.go | 22 ++++--- enterprise/audit/table.go | 1 + enterprise/coderd/groups.go | 1 + enterprise/coderd/userauth.go | 1 + enterprise/coderd/userauth_test.go | 18 +++++- 13 files changed, 155 insertions(+), 27 deletions(-) create mode 100644 coderd/database/migrations/000148_group_source.down.sql create mode 100644 coderd/database/migrations/000148_group_source.up.sql diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index a12c6f8df99d5..b37970fd7be89 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -3526,6 +3526,7 @@ func (q *FakeQuerier) InsertGroup(_ context.Context, arg database.InsertGroupPar OrganizationID: arg.OrganizationID, AvatarURL: arg.AvatarURL, QuotaAllowance: arg.QuotaAllowance, + Source: database.GroupSourceUser, } q.groups = append(q.groups, group) @@ -3608,6 +3609,7 @@ func (q *FakeQuerier) InsertMissingGroups(ctx context.Context, arg database.Inse AvatarURL: "", QuotaAllowance: 0, DisplayName: "", + Source: arg.Source, } q.groups = append(q.groups, g) newGroups = append(newGroups, g) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index f121fccf8cebb..59872f5ce8146 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -31,6 +31,11 @@ CREATE TYPE build_reason AS ENUM ( 'autodelete' ); +CREATE TYPE group_source AS ENUM ( + 'user', + 'oidc' +); + CREATE TYPE log_level AS ENUM ( 'trace', 'debug', @@ -299,11 +304,14 @@ CREATE TABLE groups ( organization_id uuid NOT NULL, avatar_url text DEFAULT ''::text NOT NULL, quota_allowance integer DEFAULT 0 NOT NULL, - display_name text DEFAULT ''::text NOT NULL + display_name text DEFAULT ''::text NOT NULL, + source group_source DEFAULT 'user'::group_source NOT NULL ); COMMENT ON COLUMN groups.display_name IS 'Display name is a custom, human-friendly group name that user can set. This is not required to be unique and can be the empty string.'; +COMMENT ON COLUMN groups.source IS 'Source indicates how the group was created. It can be created by a user manually, or through some system process like and OIDC group sync.'; + CREATE TABLE licenses ( id integer NOT NULL, uploaded_at timestamp with time zone NOT NULL, diff --git a/coderd/database/migrations/000148_group_source.down.sql b/coderd/database/migrations/000148_group_source.down.sql new file mode 100644 index 0000000000000..504c227d186bb --- /dev/null +++ b/coderd/database/migrations/000148_group_source.down.sql @@ -0,0 +1,8 @@ +BEGIN; + +ALTER TABLE groups + DROP COLUMN source; + +DROP TYPE group_source; + +COMMIT; diff --git a/coderd/database/migrations/000148_group_source.up.sql b/coderd/database/migrations/000148_group_source.up.sql new file mode 100644 index 0000000000000..ab4bc32a78f4a --- /dev/null +++ b/coderd/database/migrations/000148_group_source.up.sql @@ -0,0 +1,15 @@ +BEGIN; + +CREATE TYPE group_source AS ENUM ( + -- User created groups + 'user', + -- Groups created by the system through oidc sync + 'oidc' +); + +ALTER TABLE groups + ADD COLUMN source group_source NOT NULL DEFAULT 'user'; + +COMMENT ON COLUMN groups.source IS 'Source indicates how the group was created. It can be created by a user manually, or through some system process like and OIDC group sync.'; + +COMMIT; diff --git a/coderd/database/models.go b/coderd/database/models.go index 4e34989b09ae9..4eae4eb9549bb 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -281,6 +281,64 @@ func AllBuildReasonValues() []BuildReason { } } +type GroupSource string + +const ( + GroupSourceUser GroupSource = "user" + GroupSourceOidc GroupSource = "oidc" +) + +func (e *GroupSource) Scan(src interface{}) error { + switch s := src.(type) { + case []byte: + *e = GroupSource(s) + case string: + *e = GroupSource(s) + default: + return fmt.Errorf("unsupported scan type for GroupSource: %T", src) + } + return nil +} + +type NullGroupSource struct { + GroupSource GroupSource `json:"group_source"` + Valid bool `json:"valid"` // Valid is true if GroupSource is not NULL +} + +// Scan implements the Scanner interface. +func (ns *NullGroupSource) Scan(value interface{}) error { + if value == nil { + ns.GroupSource, ns.Valid = "", false + return nil + } + ns.Valid = true + return ns.GroupSource.Scan(value) +} + +// Value implements the driver Valuer interface. +func (ns NullGroupSource) Value() (driver.Value, error) { + if !ns.Valid { + return nil, nil + } + return string(ns.GroupSource), nil +} + +func (e GroupSource) Valid() bool { + switch e { + case GroupSourceUser, + GroupSourceOidc: + return true + } + return false +} + +func AllGroupSourceValues() []GroupSource { + return []GroupSource{ + GroupSourceUser, + GroupSourceOidc, + } +} + type LogLevel string const ( @@ -1498,6 +1556,8 @@ type Group struct { QuotaAllowance int32 `db:"quota_allowance" json:"quota_allowance"` // Display name is a custom, human-friendly group name that user can set. This is not required to be unique and can be the empty string. DisplayName string `db:"display_name" json:"display_name"` + // Source indicates how the group was created. It can be created by a user manually, or through some system process like and OIDC group sync. + Source GroupSource `db:"source" json:"source"` } type GroupMember struct { diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 2fa46e051fd84..19473e2268fc1 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1180,7 +1180,7 @@ func (q *sqlQuerier) DeleteGroupByID(ctx context.Context, id uuid.UUID) error { const getGroupByID = `-- name: GetGroupByID :one SELECT - id, name, organization_id, avatar_url, quota_allowance, display_name + id, name, organization_id, avatar_url, quota_allowance, display_name, source FROM groups WHERE @@ -1199,13 +1199,14 @@ func (q *sqlQuerier) GetGroupByID(ctx context.Context, id uuid.UUID) (Group, err &i.AvatarURL, &i.QuotaAllowance, &i.DisplayName, + &i.Source, ) return i, err } const getGroupByOrgAndName = `-- name: GetGroupByOrgAndName :one SELECT - id, name, organization_id, avatar_url, quota_allowance, display_name + id, name, organization_id, avatar_url, quota_allowance, display_name, source FROM groups WHERE @@ -1231,13 +1232,14 @@ func (q *sqlQuerier) GetGroupByOrgAndName(ctx context.Context, arg GetGroupByOrg &i.AvatarURL, &i.QuotaAllowance, &i.DisplayName, + &i.Source, ) return i, err } const getGroupsByOrganizationID = `-- name: GetGroupsByOrganizationID :many SELECT - id, name, organization_id, avatar_url, quota_allowance, display_name + id, name, organization_id, avatar_url, quota_allowance, display_name, source FROM groups WHERE @@ -1262,6 +1264,7 @@ func (q *sqlQuerier) GetGroupsByOrganizationID(ctx context.Context, organization &i.AvatarURL, &i.QuotaAllowance, &i.DisplayName, + &i.Source, ); err != nil { return nil, err } @@ -1283,7 +1286,7 @@ INSERT INTO groups ( organization_id ) VALUES - ($1, 'Everyone', $1) RETURNING id, name, organization_id, avatar_url, quota_allowance, display_name + ($1, 'Everyone', $1) RETURNING id, name, organization_id, avatar_url, quota_allowance, display_name, source ` // We use the organization_id as the id @@ -1299,6 +1302,7 @@ func (q *sqlQuerier) InsertAllUsersGroup(ctx context.Context, organizationID uui &i.AvatarURL, &i.QuotaAllowance, &i.DisplayName, + &i.Source, ) return i, err } @@ -1313,7 +1317,7 @@ INSERT INTO groups ( quota_allowance ) VALUES - ($1, $2, $3, $4, $5, $6) RETURNING id, name, organization_id, avatar_url, quota_allowance, display_name + ($1, $2, $3, $4, $5, $6) RETURNING id, name, organization_id, avatar_url, quota_allowance, display_name, source ` type InsertGroupParams struct { @@ -1342,6 +1346,7 @@ func (q *sqlQuerier) InsertGroup(ctx context.Context, arg InsertGroupParams) (Gr &i.AvatarURL, &i.QuotaAllowance, &i.DisplayName, + &i.Source, ) return i, err } @@ -1350,26 +1355,29 @@ const insertMissingGroups = `-- name: InsertMissingGroups :many INSERT INTO groups ( id, name, - organization_id + organization_id, + source ) SELECT gen_random_uuid(), group_name, - $1 + $1, + $2 FROM - UNNEST($2 :: text[]) AS group_name + UNNEST($3 :: text[]) AS group_name ON CONFLICT DO NOTHING -RETURNING id, name, organization_id, avatar_url, quota_allowance, display_name +RETURNING id, name, organization_id, avatar_url, quota_allowance, display_name, source ` type InsertMissingGroupsParams struct { - OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` - GroupNames []string `db:"group_names" json:"group_names"` + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + Source GroupSource `db:"source" json:"source"` + GroupNames []string `db:"group_names" json:"group_names"` } // If the name conflicts, do nothing. func (q *sqlQuerier) InsertMissingGroups(ctx context.Context, arg InsertMissingGroupsParams) ([]Group, error) { - rows, err := q.db.QueryContext(ctx, insertMissingGroups, arg.OrganizationID, pq.Array(arg.GroupNames)) + rows, err := q.db.QueryContext(ctx, insertMissingGroups, arg.OrganizationID, arg.Source, pq.Array(arg.GroupNames)) if err != nil { return nil, err } @@ -1384,6 +1392,7 @@ func (q *sqlQuerier) InsertMissingGroups(ctx context.Context, arg InsertMissingG &i.AvatarURL, &i.QuotaAllowance, &i.DisplayName, + &i.Source, ); err != nil { return nil, err } @@ -1408,7 +1417,7 @@ SET quota_allowance = $4 WHERE id = $5 -RETURNING id, name, organization_id, avatar_url, quota_allowance, display_name +RETURNING id, name, organization_id, avatar_url, quota_allowance, display_name, source ` type UpdateGroupByIDParams struct { @@ -1435,6 +1444,7 @@ func (q *sqlQuerier) UpdateGroupByID(ctx context.Context, arg UpdateGroupByIDPar &i.AvatarURL, &i.QuotaAllowance, &i.DisplayName, + &i.Source, ) return i, err } diff --git a/coderd/database/queries/groups.sql b/coderd/database/queries/groups.sql index afa3786740977..55a4f5999aee0 100644 --- a/coderd/database/queries/groups.sql +++ b/coderd/database/queries/groups.sql @@ -46,12 +46,14 @@ VALUES INSERT INTO groups ( id, name, - organization_id + organization_id, + source ) SELECT gen_random_uuid(), group_name, - @organization_id + @organization_id, + @source FROM UNNEST(@group_names :: text[]) AS group_name -- If the name conflicts, do nothing. diff --git a/codersdk/deployment.go b/codersdk/deployment.go index f3fa17d84e893..ea962de830458 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1079,7 +1079,7 @@ when required by your organization's security policy.`, }, { Name: "OIDC Regex Group Filter", - Description: "If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed.", + Description: "If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed. This filter is applied after the group mapping.", Flag: "oidc-group-regex-filter", Env: "CODER_OIDC_GROUP_REGEX_FILTER", Default: "", diff --git a/codersdk/groups.go b/codersdk/groups.go index c04267e4e0eb2..b50455d693469 100644 --- a/codersdk/groups.go +++ b/codersdk/groups.go @@ -10,6 +10,13 @@ import ( "golang.org/x/xerrors" ) +type GroupSource string + +const ( + GroupSourceUser GroupSource = "user" + GroupSourceOIDC GroupSource = "oidc" +) + type CreateGroupRequest struct { Name string `json:"name"` DisplayName string `json:"display_name"` @@ -18,13 +25,14 @@ type CreateGroupRequest struct { } type Group struct { - ID uuid.UUID `json:"id" format:"uuid"` - Name string `json:"name"` - DisplayName string `json:"display_name"` - OrganizationID uuid.UUID `json:"organization_id" format:"uuid"` - Members []User `json:"members"` - AvatarURL string `json:"avatar_url"` - QuotaAllowance int `json:"quota_allowance"` + ID uuid.UUID `json:"id" format:"uuid"` + Name string `json:"name"` + DisplayName string `json:"display_name"` + OrganizationID uuid.UUID `json:"organization_id" format:"uuid"` + Members []User `json:"members"` + AvatarURL string `json:"avatar_url"` + QuotaAllowance int `json:"quota_allowance"` + Source GroupSource `json:"source"` } func (c *Client) CreateGroup(ctx context.Context, orgID uuid.UUID, req CreateGroupRequest) (Group, error) { diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index b5ab50457c963..17c91ff8adfb6 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -156,6 +156,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "avatar_url": ActionTrack, "quota_allowance": ActionTrack, "members": ActionTrack, + "source": ActionIgnore, }, &database.APIKey{}: { "id": ActionIgnore, diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index b6f126e1f62e0..5119f3f91a5d8 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -409,6 +409,7 @@ func convertGroup(g database.Group, users []database.User) codersdk.Group { AvatarURL: g.AvatarURL, QuotaAllowance: int(g.QuotaAllowance), Members: convertUsers(users, orgs), + Source: codersdk.GroupSource(g.Source), } } diff --git a/enterprise/coderd/userauth.go b/enterprise/coderd/userauth.go index a26c26554d5d1..270b0acb1fd40 100644 --- a/enterprise/coderd/userauth.go +++ b/enterprise/coderd/userauth.go @@ -46,6 +46,7 @@ func (api *API) setUserGroups(ctx context.Context, logger slog.Logger, db databa created, err := tx.InsertMissingGroups(dbauthz.AsSystemRestricted(ctx), database.InsertMissingGroupsParams{ OrganizationID: orgs[0].ID, GroupNames: groupNames, + Source: database.GroupSourceOidc, }) if err != nil { return xerrors.Errorf("insert missing groups: %w", err) diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index d7c27d5515774..eb08357ea87af 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -444,16 +444,20 @@ func TestGroupSync(t *testing.T) { cfg.CreateMissingGroups = 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", + } }, initialOrgGroups: []string{"a", "b", "c", "d"}, initialUserGroups: []string{"a", "b", "c"}, - expectedUserGroups: []string{"b", "c", "d", "e", "f"}, - expectedOrgGroups: []string{"a", "b", "c", "d", "e", "f"}, + expectedUserGroups: []string{"b", "c", "d", "e", "f", "z"}, + expectedOrgGroups: []string{"a", "b", "c", "d", "e", "f", "z"}, claims: jwt.MapClaims{ "groups": []string{ "b", "c", "d", "e", "f", // These groups are ignored - "excess", "ignore", "dumb", "foobar", + "excess", "ignore", "dumb", "foobar", "zebra", }, }, }, @@ -516,6 +520,14 @@ func TestGroupSync(t *testing.T) { orgGroups, err := client.GroupsByOrganization(ctx, admin.OrganizationIDs[0]) require.NoError(t, err) + for _, group := range orgGroups { + if slice.Contains(tc.initialOrgGroups, group.Name) { + require.Equal(t, group.Source, codersdk.GroupSourceUser) + } else { + require.Equal(t, group.Source, codersdk.GroupSourceOIDC) + } + } + orgGroupsMap := make(map[string]struct{}) for _, group := range orgGroups { orgGroupsMap[group.Name] = struct{}{} From f8ea15db394316b7d64d74bb232ce0af4cc1b0c4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 3 Aug 2023 11:29:50 -0500 Subject: [PATCH 05/18] Add test for regexp --- cli/clibase/option_test.go | 34 ++++++++++++++++++++++++++++++++++ cli/clibase/values.go | 2 +- codersdk/deployment.go | 2 +- 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/cli/clibase/option_test.go b/cli/clibase/option_test.go index cacd8d3a10793..9affda90668e4 100644 --- a/cli/clibase/option_test.go +++ b/cli/clibase/option_test.go @@ -72,6 +72,40 @@ func TestOptionSet_ParseFlags(t *testing.T) { err := os.FlagSet().Parse([]string{"--some-unknown", "foo"}) require.Error(t, err) }) + + t.Run("RegexValid", func(t *testing.T) { + t.Parallel() + + var regexpString clibase.Regexp + + os := clibase.OptionSet{ + clibase.Option{ + Name: "RegexpString", + Value: ®expString, + Flag: "regexp-string", + }, + } + + err := os.FlagSet().Parse([]string{"--regexp-string", "$test^"}) + require.NoError(t, err) + }) + + t.Run("RegexInvalid", func(t *testing.T) { + t.Parallel() + + var regexpString clibase.Regexp + + os := clibase.OptionSet{ + clibase.Option{ + Name: "RegexpString", + Value: ®expString, + Flag: "regexp-string", + }, + } + + err := os.FlagSet().Parse([]string{"--regexp-string", "(("}) + require.Error(t, err) + }) } func TestOptionSet_ParseEnv(t *testing.T) { diff --git a/cli/clibase/values.go b/cli/clibase/values.go index 9b215d613fa9d..50581c6f40c94 100644 --- a/cli/clibase/values.go +++ b/cli/clibase/values.go @@ -471,7 +471,7 @@ func RegexpOf(s *regexp.Regexp) *Regexp { func (s *Regexp) Set(v string) error { exp, err := regexp.Compile(v) if err != nil { - return xerrors.Errorf("invalid regexp %q: %w", v, err) + return xerrors.Errorf("invalid regex expression: %w", v, err) } *s = Regexp(*exp) return nil diff --git a/codersdk/deployment.go b/codersdk/deployment.go index ea962de830458..692d09a741517 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1082,7 +1082,7 @@ when required by your organization's security policy.`, Description: "If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed. This filter is applied after the group mapping.", Flag: "oidc-group-regex-filter", Env: "CODER_OIDC_GROUP_REGEX_FILTER", - Default: "", + Default: "*", Value: &c.OIDC.GroupRegexFilter, Group: &deploymentGroupOIDC, YAML: "groupRegexFilter", From cd5dd2f5499093d1a063ff0bfb815b14b817f1aa Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 3 Aug 2023 11:33:33 -0500 Subject: [PATCH 06/18] Fix regepx default --- codersdk/deployment.go | 2 +- enterprise/coderd/userauth_test.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 692d09a741517..93f1ce58e112c 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1082,7 +1082,7 @@ when required by your organization's security policy.`, Description: "If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed. This filter is applied after the group mapping.", Flag: "oidc-group-regex-filter", Env: "CODER_OIDC_GROUP_REGEX_FILTER", - Default: "*", + Default: ".*", Value: &c.OIDC.GroupRegexFilter, Group: &deploymentGroupOIDC, YAML: "groupRegexFilter", diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index eb08357ea87af..e0ad8db60c9cf 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -386,6 +386,7 @@ func TestGroupSync(t *testing.T) { modCfg: func(cfg *coderd.OIDCConfig) { // Disable group sync cfg.GroupField = "" + cfg.GroupFilter = regexp.MustCompile(".*") }, initialOrgGroups: []string{"a", "b", "c", "d"}, initialUserGroups: []string{"b", "c", "d"}, @@ -414,6 +415,7 @@ func TestGroupSync(t *testing.T) { // From a,c,b -> [] name: "RemoveAllGroups", modCfg: func(cfg *coderd.OIDCConfig) { + cfg.GroupFilter = regexp.MustCompile(".*") }, initialOrgGroups: []string{"a", "b", "c", "d"}, initialUserGroups: []string{"a", "b", "c"}, From 86390d22244936121cbc7edb1b4f5efce7507d67 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 3 Aug 2023 14:31:08 -0500 Subject: [PATCH 07/18] make gen --- coderd/apidoc/docs.go | 23 +++++ coderd/apidoc/swagger.json | 17 ++++ coderd/database/dbmetrics/dbmetrics.go | 4 +- coderd/database/dbmock/dbmock.go | 15 ++++ docs/admin/audit-logs.md | 2 +- docs/api/enterprise.md | 119 ++++++++++++++----------- docs/api/general.md | 2 + docs/api/schemas.md | 98 +++++++++++++------- docs/cli/server.md | 22 +++++ site/src/api/typesGenerated.ts | 9 ++ 10 files changed, 224 insertions(+), 87 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 6ce7eb21c0b94..4cddc4ebd658b 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -6624,6 +6624,9 @@ const docTemplate = `{ } } }, + "clibase.Regexp": { + "type": "object" + }, "clibase.Struct-array_codersdk_GitAuthConfig": { "type": "object", "properties": { @@ -8274,9 +8277,23 @@ const docTemplate = `{ }, "quota_allowance": { "type": "integer" + }, + "source": { + "$ref": "#/definitions/codersdk.GroupSource" } } }, + "codersdk.GroupSource": { + "type": "string", + "enum": [ + "user", + "oidc" + ], + "x-enum-varnames": [ + "GroupSourceUser", + "GroupSourceOIDC" + ] + }, "codersdk.Healthcheck": { "type": "object", "properties": { @@ -8583,9 +8600,15 @@ const docTemplate = `{ "email_field": { "type": "string" }, + "group_auto_create": { + "type": "boolean" + }, "group_mapping": { "type": "object" }, + "group_regex_filter": { + "$ref": "#/definitions/clibase.Regexp" + }, "groups_field": { "type": "string" }, diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 500307a23fa62..5c5e05c14050b 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -5874,6 +5874,9 @@ } } }, + "clibase.Regexp": { + "type": "object" + }, "clibase.Struct-array_codersdk_GitAuthConfig": { "type": "object", "properties": { @@ -7430,9 +7433,17 @@ }, "quota_allowance": { "type": "integer" + }, + "source": { + "$ref": "#/definitions/codersdk.GroupSource" } } }, + "codersdk.GroupSource": { + "type": "string", + "enum": ["user", "oidc"], + "x-enum-varnames": ["GroupSourceUser", "GroupSourceOIDC"] + }, "codersdk.Healthcheck": { "type": "object", "properties": { @@ -7703,9 +7714,15 @@ "email_field": { "type": "string" }, + "group_auto_create": { + "type": "boolean" + }, "group_mapping": { "type": "object" }, + "group_regex_filter": { + "$ref": "#/definitions/clibase.Regexp" + }, "groups_field": { "type": "string" }, diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index c721824b89395..a581a7ed43166 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -1105,9 +1105,9 @@ func (m metricsStore) InsertLicense(ctx context.Context, arg database.InsertLice func (m metricsStore) InsertMissingGroups(ctx context.Context, arg database.InsertMissingGroupsParams) ([]database.Group, error) { start := time.Now() - r0 := m.s.InsertMissingGroups(ctx, arg) + r0, r1 := m.s.InsertMissingGroups(ctx, arg) m.queryLatencies.WithLabelValues("InsertMissingGroups").Observe(time.Since(start).Seconds()) - return r0 + return r0, r1 } func (m metricsStore) InsertOrganization(ctx context.Context, arg database.InsertOrganizationParams) (database.Organization, error) { diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index bf1732568fd42..4faafe719f0a9 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2317,6 +2317,21 @@ func (mr *MockStoreMockRecorder) InsertLicense(arg0, arg1 interface{}) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertLicense", reflect.TypeOf((*MockStore)(nil).InsertLicense), arg0, arg1) } +// InsertMissingGroups mocks base method. +func (m *MockStore) InsertMissingGroups(arg0 context.Context, arg1 database.InsertMissingGroupsParams) ([]database.Group, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InsertMissingGroups", arg0, arg1) + ret0, _ := ret[0].([]database.Group) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// InsertMissingGroups indicates an expected call of InsertMissingGroups. +func (mr *MockStoreMockRecorder) InsertMissingGroups(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertMissingGroups", reflect.TypeOf((*MockStore)(nil).InsertMissingGroups), arg0, arg1) +} + // InsertOrganization mocks base method. func (m *MockStore) InsertOrganization(arg0 context.Context, arg1 database.InsertOrganizationParams) (database.Organization, error) { m.ctrl.T.Helper() diff --git a/docs/admin/audit-logs.md b/docs/admin/audit-logs.md index 882a7274c737f..230e99be53f7e 100644 --- a/docs/admin/audit-logs.md +++ b/docs/admin/audit-logs.md @@ -13,7 +13,7 @@ We track the following resources: | -------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | APIKey
login, logout, register, create, delete |
FieldTracked
created_attrue
expires_attrue
hashed_secretfalse
idfalse
ip_addressfalse
last_usedtrue
lifetime_secondsfalse
login_typefalse
scopefalse
token_namefalse
updated_atfalse
user_idtrue
| | AuditOAuthConvertState
|
FieldTracked
created_attrue
expires_attrue
from_login_typetrue
to_login_typetrue
user_idtrue
| -| Group
create, write, delete |
FieldTracked
avatar_urltrue
display_nametrue
idtrue
memberstrue
nametrue
organization_idfalse
quota_allowancetrue
| +| Group
create, write, delete |
FieldTracked
avatar_urltrue
display_nametrue
idtrue
memberstrue
nametrue
organization_idfalse
quota_allowancetrue
sourcefalse
| | GitSSHKey
create |
FieldTracked
created_atfalse
private_keytrue
public_keytrue
updated_atfalse
user_idtrue
| | License
create, delete |
FieldTracked
exptrue
idfalse
jwtfalse
uploaded_attrue
uuidtrue
| | Template
write, delete |
FieldTracked
active_version_idtrue
allow_user_autostarttrue
allow_user_autostoptrue
allow_user_cancel_workspace_jobstrue
created_atfalse
created_bytrue
created_by_avatar_urlfalse
created_by_usernamefalse
default_ttltrue
deletedfalse
descriptiontrue
display_nametrue
failure_ttltrue
group_acltrue
icontrue
idtrue
inactivity_ttltrue
locked_ttltrue
max_ttltrue
nametrue
organization_idfalse
provisionertrue
restart_requirement_days_of_weektrue
restart_requirement_weekstrue
updated_atfalse
user_acltrue
| diff --git a/docs/api/enterprise.md b/docs/api/enterprise.md index d859ac59ad09b..fc887cd12b6e3 100644 --- a/docs/api/enterprise.md +++ b/docs/api/enterprise.md @@ -197,7 +197,8 @@ curl -X GET http://coder-server:8080/api/v2/groups/{group} \ ], "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", - "quota_allowance": 0 + "quota_allowance": 0, + "source": "user" } ``` @@ -258,7 +259,8 @@ curl -X DELETE http://coder-server:8080/api/v2/groups/{group} \ ], "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", - "quota_allowance": 0 + "quota_allowance": 0, + "source": "user" } ``` @@ -319,7 +321,8 @@ curl -X PATCH http://coder-server:8080/api/v2/groups/{group} \ ], "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", - "quota_allowance": 0 + "quota_allowance": 0, + "source": "user" } ``` @@ -455,7 +458,8 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/groups ], "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", - "quota_allowance": 0 + "quota_allowance": 0, + "source": "user" } ] ``` @@ -470,28 +474,29 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/groups Status Code **200** -| Name | Type | Required | Restrictions | Description | -| --------------------- | ---------------------------------------------------- | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» avatar_url` | string | false | | | -| `» display_name` | string | false | | | -| `» id` | string(uuid) | false | | | -| `» members` | array | false | | | -| `»» avatar_url` | string(uri) | false | | | -| `»» created_at` | string(date-time) | true | | | -| `»» email` | string(email) | true | | | -| `»» id` | string(uuid) | true | | | -| `»» last_seen_at` | string(date-time) | false | | | -| `»» login_type` | [codersdk.LoginType](schemas.md#codersdklogintype) | false | | | -| `»» organization_ids` | array | false | | | -| `»» roles` | array | false | | | -| `»»» display_name` | string | false | | | -| `»»» name` | string | false | | | -| `»» status` | [codersdk.UserStatus](schemas.md#codersdkuserstatus) | false | | | -| `»» username` | string | true | | | -| `» name` | string | false | | | -| `» organization_id` | string(uuid) | false | | | -| `» quota_allowance` | integer | false | | | +| Name | Type | Required | Restrictions | Description | +| --------------------- | ------------------------------------------------------ | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» avatar_url` | string | false | | | +| `» display_name` | string | false | | | +| `» id` | string(uuid) | false | | | +| `» members` | array | false | | | +| `»» avatar_url` | string(uri) | false | | | +| `»» created_at` | string(date-time) | true | | | +| `»» email` | string(email) | true | | | +| `»» id` | string(uuid) | true | | | +| `»» last_seen_at` | string(date-time) | false | | | +| `»» login_type` | [codersdk.LoginType](schemas.md#codersdklogintype) | false | | | +| `»» organization_ids` | array | false | | | +| `»» roles` | array | false | | | +| `»»» display_name` | string | false | | | +| `»»» name` | string | false | | | +| `»» status` | [codersdk.UserStatus](schemas.md#codersdkuserstatus) | false | | | +| `»» username` | string | true | | | +| `» name` | string | false | | | +| `» organization_id` | string(uuid) | false | | | +| `» quota_allowance` | integer | false | | | +| `» source` | [codersdk.GroupSource](schemas.md#codersdkgroupsource) | false | | | #### Enumerated Values @@ -504,6 +509,8 @@ Status Code **200** | `login_type` | `none` | | `status` | `active` | | `status` | `suspended` | +| `source` | `user` | +| `source` | `oidc` | To perform this operation, you must be authenticated. [Learn more](authentication.md). @@ -569,7 +576,8 @@ curl -X POST http://coder-server:8080/api/v2/organizations/{organization}/groups ], "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", - "quota_allowance": 0 + "quota_allowance": 0, + "source": "user" } ``` @@ -631,7 +639,8 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/groups/ ], "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", - "quota_allowance": 0 + "quota_allowance": 0, + "source": "user" } ``` @@ -1202,7 +1211,8 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/acl/available \ ], "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", - "quota_allowance": 0 + "quota_allowance": 0, + "source": "user" } ], "users": [ @@ -1238,30 +1248,31 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/acl/available \ Status Code **200** -| Name | Type | Required | Restrictions | Description | -| ---------------------- | ---------------------------------------------------- | -------- | ------------ | ----------- | -| `[array item]` | array | false | | | -| `» groups` | array | false | | | -| `»» avatar_url` | string | false | | | -| `»» display_name` | string | false | | | -| `»» id` | string(uuid) | false | | | -| `»» members` | array | false | | | -| `»»» avatar_url` | string(uri) | false | | | -| `»»» created_at` | string(date-time) | true | | | -| `»»» email` | string(email) | true | | | -| `»»» id` | string(uuid) | true | | | -| `»»» last_seen_at` | string(date-time) | false | | | -| `»»» login_type` | [codersdk.LoginType](schemas.md#codersdklogintype) | false | | | -| `»»» organization_ids` | array | false | | | -| `»»» roles` | array | false | | | -| `»»»» display_name` | string | false | | | -| `»»»» name` | string | false | | | -| `»»» status` | [codersdk.UserStatus](schemas.md#codersdkuserstatus) | false | | | -| `»»» username` | string | true | | | -| `»» name` | string | false | | | -| `»» organization_id` | string(uuid) | false | | | -| `»» quota_allowance` | integer | false | | | -| `» users` | array | false | | | +| Name | Type | Required | Restrictions | Description | +| ---------------------- | ------------------------------------------------------ | -------- | ------------ | ----------- | +| `[array item]` | array | false | | | +| `» groups` | array | false | | | +| `»» avatar_url` | string | false | | | +| `»» display_name` | string | false | | | +| `»» id` | string(uuid) | false | | | +| `»» members` | array | false | | | +| `»»» avatar_url` | string(uri) | false | | | +| `»»» created_at` | string(date-time) | true | | | +| `»»» email` | string(email) | true | | | +| `»»» id` | string(uuid) | true | | | +| `»»» last_seen_at` | string(date-time) | false | | | +| `»»» login_type` | [codersdk.LoginType](schemas.md#codersdklogintype) | false | | | +| `»»» organization_ids` | array | false | | | +| `»»» roles` | array | false | | | +| `»»»» display_name` | string | false | | | +| `»»»» name` | string | false | | | +| `»»» status` | [codersdk.UserStatus](schemas.md#codersdkuserstatus) | false | | | +| `»»» username` | string | true | | | +| `»» name` | string | false | | | +| `»» organization_id` | string(uuid) | false | | | +| `»» quota_allowance` | integer | false | | | +| `»» source` | [codersdk.GroupSource](schemas.md#codersdkgroupsource) | false | | | +| `» users` | array | false | | | #### Enumerated Values @@ -1274,6 +1285,8 @@ Status Code **200** | `login_type` | `none` | | `status` | `active` | | `status` | `suspended` | +| `source` | `user` | +| `source` | `oidc` | To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/api/general.md b/docs/api/general.md index bb64823bd86c9..8bc248a6e332b 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -260,7 +260,9 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "client_secret": "string", "email_domain": ["string"], "email_field": "string", + "group_auto_create": true, "group_mapping": {}, + "group_regex_filter": {}, "groups_field": "string", "icon_url": { "forceQuery": true, diff --git a/docs/api/schemas.md b/docs/api/schemas.md index db7ec2873226a..f1ae5ef44eb1e 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -595,6 +595,16 @@ | `value_source` | [clibase.ValueSource](#clibasevaluesource) | false | | | | `yaml` | string | false | | Yaml is the YAML key used to configure this option. If unset, YAML configuring is disabled. | +## clibase.Regexp + +```json +{} +``` + +### Properties + +_None_ + ## clibase.Struct-array_codersdk_GitAuthConfig ```json @@ -788,7 +798,8 @@ ], "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", - "quota_allowance": 0 + "quota_allowance": 0, + "source": "user" } ], "users": [ @@ -2054,7 +2065,9 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "client_secret": "string", "email_domain": ["string"], "email_field": "string", + "group_auto_create": true, "group_mapping": {}, + "group_regex_filter": {}, "groups_field": "string", "icon_url": { "forceQuery": true, @@ -2411,7 +2424,9 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "client_secret": "string", "email_domain": ["string"], "email_field": "string", + "group_auto_create": true, "group_mapping": {}, + "group_regex_filter": {}, "groups_field": "string", "icon_url": { "forceQuery": true, @@ -2957,21 +2972,38 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ], "name": "string", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", - "quota_allowance": 0 + "quota_allowance": 0, + "source": "user" } ``` ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------------- | --------------------------------------- | -------- | ------------ | ----------- | -| `avatar_url` | string | false | | | -| `display_name` | string | false | | | -| `id` | string | false | | | -| `members` | array of [codersdk.User](#codersdkuser) | false | | | -| `name` | string | false | | | -| `organization_id` | string | false | | | -| `quota_allowance` | integer | false | | | +| Name | Type | Required | Restrictions | Description | +| ----------------- | -------------------------------------------- | -------- | ------------ | ----------- | +| `avatar_url` | string | false | | | +| `display_name` | string | false | | | +| `id` | string | false | | | +| `members` | array of [codersdk.User](#codersdkuser) | false | | | +| `name` | string | false | | | +| `organization_id` | string | false | | | +| `quota_allowance` | integer | false | | | +| `source` | [codersdk.GroupSource](#codersdkgroupsource) | false | | | + +## codersdk.GroupSource + +```json +"user" +``` + +### Properties + +#### Enumerated Values + +| Value | +| ------ | +| `user` | +| `oidc` | ## codersdk.Healthcheck @@ -3303,7 +3335,9 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "client_secret": "string", "email_domain": ["string"], "email_field": "string", + "group_auto_create": true, "group_mapping": {}, + "group_regex_filter": {}, "groups_field": "string", "icon_url": { "forceQuery": true, @@ -3332,26 +3366,28 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------------------- | -------------------------- | -------- | ------------ | ----------- | -| `allow_signups` | boolean | false | | | -| `auth_url_params` | object | false | | | -| `client_id` | string | false | | | -| `client_secret` | string | false | | | -| `email_domain` | array of string | false | | | -| `email_field` | string | false | | | -| `group_mapping` | object | false | | | -| `groups_field` | string | false | | | -| `icon_url` | [clibase.URL](#clibaseurl) | false | | | -| `ignore_email_verified` | boolean | false | | | -| `ignore_user_info` | boolean | false | | | -| `issuer_url` | string | false | | | -| `scopes` | array of string | false | | | -| `sign_in_text` | string | false | | | -| `user_role_field` | string | false | | | -| `user_role_mapping` | object | false | | | -| `user_roles_default` | array of string | false | | | -| `username_field` | string | false | | | +| Name | Type | Required | Restrictions | Description | +| ----------------------- | -------------------------------- | -------- | ------------ | ----------- | +| `allow_signups` | boolean | false | | | +| `auth_url_params` | object | false | | | +| `client_id` | string | false | | | +| `client_secret` | string | false | | | +| `email_domain` | array of string | false | | | +| `email_field` | string | false | | | +| `group_auto_create` | boolean | false | | | +| `group_mapping` | object | false | | | +| `group_regex_filter` | [clibase.Regexp](#clibaseregexp) | false | | | +| `groups_field` | string | false | | | +| `icon_url` | [clibase.URL](#clibaseurl) | false | | | +| `ignore_email_verified` | boolean | false | | | +| `ignore_user_info` | boolean | false | | | +| `issuer_url` | string | false | | | +| `scopes` | array of string | false | | | +| `sign_in_text` | string | false | | | +| `user_role_field` | string | false | | | +| `user_role_mapping` | object | false | | | +| `user_roles_default` | array of string | false | | | +| `username_field` | string | false | | | ## codersdk.Organization diff --git a/docs/cli/server.md b/docs/cli/server.md index 90c60d7392f00..f3b8b151b00a9 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -243,6 +243,17 @@ Disable automatic session expiry bumping due to activity. This forces all sessio Specifies the custom docs URL. +### --oidc-group-auto-create + +| | | +| ----------- | ------------------------------------------ | +| Type | bool | +| Environment | $CODER_OIDC_GROUP_AUTO_CREATE | +| YAML | oidc.enableGroupAutoCreate | +| Default | false | + +Automatically creates missing groups from a user's groups claim. + ### --enable-terraform-debug-mode | | | @@ -521,6 +532,17 @@ Ignore the userinfo endpoint and only use the ID token for user information. Issuer URL to use for Login with OIDC. +### --oidc-group-regex-filter + +| | | +| ----------- | ------------------------------------------- | +| Type | regexp | +| Environment | $CODER_OIDC_GROUP_REGEX_FILTER | +| YAML | oidc.groupRegexFilter | +| Default | .\* | + +If provided any group name not matching the regex is ignored. This allows for filtering out groups that are not needed. This filter is applied after the group mapping. + ### --oidc-scopes | | | diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index d483fa3ccc860..e63072a86bfff 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -512,6 +512,7 @@ export interface Group { readonly members: User[] readonly avatar_url: string readonly quota_allowance: number + readonly source: GroupSource } // From codersdk/workspaceapps.go @@ -625,6 +626,10 @@ export interface OIDCConfig { // eslint-disable-next-line @typescript-eslint/no-explicit-any -- External type readonly auth_url_params: any readonly ignore_user_info: boolean + readonly group_auto_create: boolean + // Named type "github.com/coder/coder/cli/clibase.Regexp" unknown, using "any" + // eslint-disable-next-line @typescript-eslint/no-explicit-any -- External type + readonly group_regex_filter: any readonly groups_field: string // Named type "github.com/coder/coder/cli/clibase.Struct[map[string]string]" unknown, using "any" // eslint-disable-next-line @typescript-eslint/no-explicit-any -- External type @@ -1620,6 +1625,10 @@ export const GitProviders: GitProvider[] = [ "gitlab", ] +// From codersdk/groups.go +export type GroupSource = "oidc" | "user" +export const GroupSources: GroupSource[] = ["oidc", "user"] + // From codersdk/insights.go export type InsightsReportInterval = "day" export const InsightsReportIntervals: InsightsReportInterval[] = ["day"] From 6816602658e235efdb5b0bfb14d9bc0228a731ac Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 3 Aug 2023 20:30:48 +0000 Subject: [PATCH 08/18] make gen --- cli/testdata/coder_server_--help.golden | 8 ++++++++ cli/testdata/server-config.yaml.golden | 8 ++++++++ coderd/database/models.go | 2 +- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 2 +- enterprise/cli/testdata/coder_server_--help.golden | 8 ++++++++ 6 files changed, 27 insertions(+), 3 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index cb7ca61b4913a..5955ad5c2e624 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -298,6 +298,9 @@ can safely ignore these settings. GitHub. OIDC Options + --oidc-group-auto-create bool, $CODER_OIDC_GROUP_AUTO_CREATE (default: false) + Automatically creates missing groups from a user's groups claim. + --oidc-allow-signups bool, $CODER_OIDC_ALLOW_SIGNUPS (default: true) Whether new users can sign up with OIDC. @@ -334,6 +337,11 @@ can safely ignore these settings. --oidc-issuer-url string, $CODER_OIDC_ISSUER_URL Issuer URL to use for Login with OIDC. + --oidc-group-regex-filter regexp, $CODER_OIDC_GROUP_REGEX_FILTER (default: .*) + If provided any group name not matching the regex is ignored. This + allows for filtering out groups that are not needed. This filter is + applied after the group mapping. + --oidc-scopes string-array, $CODER_OIDC_SCOPES (default: openid,profile,email) Scopes to grant when authenticating with OIDC. diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index c7a8df03414e6..6d971ff665353 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -271,6 +271,14 @@ oidc: # for when OIDC providers only return group IDs. # (default: {}, type: struct[map[string]string]) groupMapping: {} + # Automatically creates missing groups from a user's groups claim. + # (default: false, type: bool) + enableGroupAutoCreate: false + # If provided any group name not matching the regex is ignored. This allows for + # filtering out groups that are not needed. This filter is applied after the group + # mapping. + # (default: .*, type: regexp) + groupRegexFilter: {} # This field must be set if using the user roles sync feature. Set this to the # name of the claim used to store the user's role. The roles should be sent as an # array of strings. diff --git a/coderd/database/models.go b/coderd/database/models.go index 4eae4eb9549bb..e2a4ea6d84fbc 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.19.1 +// sqlc v1.20.0 package database diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 762d09a56ebc5..a9cd5889e915e 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.19.1 +// sqlc v1.20.0 package database diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 339137d1fae4a..f8adbf8743076 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.19.1 +// sqlc v1.20.0 package database diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index cb7ca61b4913a..5955ad5c2e624 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -298,6 +298,9 @@ can safely ignore these settings. GitHub. OIDC Options + --oidc-group-auto-create bool, $CODER_OIDC_GROUP_AUTO_CREATE (default: false) + Automatically creates missing groups from a user's groups claim. + --oidc-allow-signups bool, $CODER_OIDC_ALLOW_SIGNUPS (default: true) Whether new users can sign up with OIDC. @@ -334,6 +337,11 @@ can safely ignore these settings. --oidc-issuer-url string, $CODER_OIDC_ISSUER_URL Issuer URL to use for Login with OIDC. + --oidc-group-regex-filter regexp, $CODER_OIDC_GROUP_REGEX_FILTER (default: .*) + If provided any group name not matching the regex is ignored. This + allows for filtering out groups that are not needed. This filter is + applied after the group mapping. + --oidc-scopes string-array, $CODER_OIDC_SCOPES (default: openid,profile,email) Scopes to grant when authenticating with OIDC. From 11d0f15b04691a0967ff4bb3fb2cc2210b7974cd Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 3 Aug 2023 15:31:51 -0500 Subject: [PATCH 09/18] Sqlc --- coderd/database/models.go | 2 +- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/coderd/database/models.go b/coderd/database/models.go index e2a4ea6d84fbc..4eae4eb9549bb 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.20.0 +// sqlc v1.19.1 package database diff --git a/coderd/database/querier.go b/coderd/database/querier.go index a9cd5889e915e..762d09a56ebc5 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.20.0 +// sqlc v1.19.1 package database diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index f8adbf8743076..339137d1fae4a 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1,6 +1,6 @@ // Code generated by sqlc. DO NOT EDIT. // versions: -// sqlc v1.20.0 +// sqlc v1.19.1 package database From 863cb3809182f1ff17ca96c0f95786eb10626593 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 4 Aug 2023 09:03:26 -0500 Subject: [PATCH 10/18] formatting --- cli/clibase/values.go | 2 +- coderd/database/dbfake/dbfake.go | 2 +- enterprise/coderd/userauth_test.go | 1 - 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cli/clibase/values.go b/cli/clibase/values.go index 50581c6f40c94..d292faaec73ae 100644 --- a/cli/clibase/values.go +++ b/cli/clibase/values.go @@ -471,7 +471,7 @@ func RegexpOf(s *regexp.Regexp) *Regexp { func (s *Regexp) Set(v string) error { exp, err := regexp.Compile(v) if err != nil { - return xerrors.Errorf("invalid regex expression: %w", v, err) + return xerrors.Errorf("invalid regex expression: %w", err) } *s = Regexp(*exp) return nil diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 09a300e54dfe0..c6c9d89dbccf8 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -3688,7 +3688,7 @@ func (q *FakeQuerier) InsertLicense( return l, nil } -func (q *FakeQuerier) InsertMissingGroups(ctx context.Context, arg database.InsertMissingGroupsParams) ([]database.Group, error) { +func (q *FakeQuerier) InsertMissingGroups(_ context.Context, arg database.InsertMissingGroupsParams) ([]database.Group, error) { err := validateDatabaseType(arg) if err != nil { return nil, err diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index e0ad8db60c9cf..f82c01d57cbc1 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -374,7 +374,6 @@ func TestGroupSync(t *testing.T) { { name: "NoGroups", modCfg: func(cfg *coderd.OIDCConfig) { - }, initialOrgGroups: []string{}, expectedUserGroups: []string{}, From 946179e4a91df7daa58e340bd8b5585361ba4b04 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 4 Aug 2023 09:10:29 -0500 Subject: [PATCH 11/18] nolint --- enterprise/coderd/userauth.go | 1 + 1 file changed, 1 insertion(+) diff --git a/enterprise/coderd/userauth.go b/enterprise/coderd/userauth.go index 270b0acb1fd40..98833263355e3 100644 --- a/enterprise/coderd/userauth.go +++ b/enterprise/coderd/userauth.go @@ -13,6 +13,7 @@ import ( "github.com/coder/coder/codersdk" ) +// nolint: revive func (api *API) setUserGroups(ctx context.Context, logger slog.Logger, db database.Store, userID uuid.UUID, groupNames []string, createMissingGroups bool) error { api.entitlementsMu.RLock() enabled := api.entitlements.Features[codersdk.FeatureTemplateRBAC].Enabled From abc5efb232bf6f4533d4424122890306e13d3955 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 4 Aug 2023 09:11:41 -0500 Subject: [PATCH 12/18] Close resp body --- enterprise/coderd/userauth_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index f82c01d57cbc1..62d7eadff46d1 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -517,6 +517,7 @@ func TestGroupSync(t *testing.T) { tc.claims["email"] = user.Email resp := oidcCallback(t, client, conf.EncodeClaims(t, tc.claims)) assert.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + _ = resp.Body.Close() orgGroups, err := client.GroupsByOrganization(ctx, admin.OrganizationIDs[0]) require.NoError(t, err) From 819115e99c1bbb17a01b15c07203987a970205c4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 4 Aug 2023 09:45:20 -0500 Subject: [PATCH 13/18] Add yaml marshal fields --- cli/clibase/values.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/cli/clibase/values.go b/cli/clibase/values.go index d292faaec73ae..bee7f8e31f9c2 100644 --- a/cli/clibase/values.go +++ b/cli/clibase/values.go @@ -464,8 +464,15 @@ func (e *Enum) String() string { type Regexp regexp.Regexp -func RegexpOf(s *regexp.Regexp) *Regexp { - return (*Regexp)(s) +func (d *Regexp) MarshalYAML() (interface{}, error) { + return yaml.Node{ + Kind: yaml.ScalarNode, + Value: d.String(), + }, nil +} + +func (d *Regexp) UnmarshalYAML(n *yaml.Node) error { + return d.Set(n.Value) } func (s *Regexp) Set(v string) error { From 87ee32a668c0d2f0199f8290c14d621e12a0f5a0 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 4 Aug 2023 09:58:59 -0500 Subject: [PATCH 14/18] Update golden files --- cli/testdata/server-config.yaml.golden | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 6d971ff665353..77ca4bb882d94 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -278,7 +278,7 @@ oidc: # filtering out groups that are not needed. This filter is applied after the group # mapping. # (default: .*, type: regexp) - groupRegexFilter: {} + groupRegexFilter: .* # This field must be set if using the user roles sync feature. Set this to the # name of the claim used to store the user's role. The roles should be sent as an # array of strings. From 1360f6592fbcdc927fc47124190bee038bf89a35 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 4 Aug 2023 10:10:32 -0500 Subject: [PATCH 15/18] FE mocks --- site/src/testHelpers/entities.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 8b208548018ed..c19c317cbf562 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -1670,6 +1670,7 @@ export const MockGroup: TypesGen.Group = { organization_id: MockOrganization.id, members: [MockUser, MockUser2], quota_allowance: 5, + source: "user", } export const MockTemplateACL: TypesGen.TemplateACL = { From 5ecc1b3134d55dd233806fa18be87422e233e63e Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 4 Aug 2023 10:11:09 -0500 Subject: [PATCH 16/18] Linting --- cli/clibase/values.go | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/cli/clibase/values.go b/cli/clibase/values.go index bee7f8e31f9c2..6ec67d2d1bc09 100644 --- a/cli/clibase/values.go +++ b/cli/clibase/values.go @@ -464,35 +464,35 @@ func (e *Enum) String() string { type Regexp regexp.Regexp -func (d *Regexp) MarshalYAML() (interface{}, error) { +func (r *Regexp) MarshalYAML() (interface{}, error) { return yaml.Node{ Kind: yaml.ScalarNode, - Value: d.String(), + Value: r.String(), }, nil } -func (d *Regexp) UnmarshalYAML(n *yaml.Node) error { - return d.Set(n.Value) +func (r *Regexp) UnmarshalYAML(n *yaml.Node) error { + return r.Set(n.Value) } -func (s *Regexp) Set(v string) error { +func (r *Regexp) Set(v string) error { exp, err := regexp.Compile(v) if err != nil { return xerrors.Errorf("invalid regex expression: %w", err) } - *s = Regexp(*exp) + *r = Regexp(*exp) return nil } -func (s Regexp) String() string { - return s.Value().String() +func (r Regexp) String() string { + return r.Value().String() } -func (s *Regexp) Value() *regexp.Regexp { - if s == nil { +func (r *Regexp) Value() *regexp.Regexp { + if r == nil { return nil } - return (*regexp.Regexp)(s) + return (*regexp.Regexp)(r) } func (Regexp) Type() string { From fd800910aa999d1c50a832de5c8ab144cb72c930 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 4 Aug 2023 10:29:05 -0500 Subject: [PATCH 17/18] Fix fe test --- coderd/database/querier.go | 3 +++ coderd/database/queries.sql.go | 3 +++ coderd/database/queries/groups.sql | 3 +++ site/src/utils/groups.ts | 1 + 4 files changed, 10 insertions(+) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 762d09a56ebc5..75b17c9159957 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -206,6 +206,9 @@ type sqlcQuerier interface { InsertGroup(ctx context.Context, arg InsertGroupParams) (Group, error) InsertGroupMember(ctx context.Context, arg InsertGroupMemberParams) error InsertLicense(ctx context.Context, arg InsertLicenseParams) (License, error) + // Inserts any group by name that does not exist. All new groups are given + // a random uuid, are inserted into the same organization. They have the default + // values for avatar, display name, and quota allowance (all zero values). // If the name conflicts, do nothing. InsertMissingGroups(ctx context.Context, arg InsertMissingGroupsParams) ([]Group, error) InsertOrganization(ctx context.Context, arg InsertOrganizationParams) (Organization, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 339137d1fae4a..bd451ca0ea5d5 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -1375,6 +1375,9 @@ type InsertMissingGroupsParams struct { GroupNames []string `db:"group_names" json:"group_names"` } +// Inserts any group by name that does not exist. All new groups are given +// a random uuid, are inserted into the same organization. They have the default +// values for avatar, display name, and quota allowance (all zero values). // If the name conflicts, do nothing. func (q *sqlQuerier) InsertMissingGroups(ctx context.Context, arg InsertMissingGroupsParams) ([]Group, error) { rows, err := q.db.QueryContext(ctx, insertMissingGroups, arg.OrganizationID, arg.Source, pq.Array(arg.GroupNames)) diff --git a/coderd/database/queries/groups.sql b/coderd/database/queries/groups.sql index 55a4f5999aee0..da47116983c87 100644 --- a/coderd/database/queries/groups.sql +++ b/coderd/database/queries/groups.sql @@ -43,6 +43,9 @@ VALUES ($1, $2, $3, $4, $5, $6) RETURNING *; -- name: InsertMissingGroups :many +-- Inserts any group by name that does not exist. All new groups are given +-- a random uuid, are inserted into the same organization. They have the default +-- values for avatar, display name, and quota allowance (all zero values). INSERT INTO groups ( id, name, diff --git a/site/src/utils/groups.ts b/site/src/utils/groups.ts index 9afc048ce7cef..88dad942beddb 100644 --- a/site/src/utils/groups.ts +++ b/site/src/utils/groups.ts @@ -8,6 +8,7 @@ export const everyOneGroup = (organizationId: string): Group => ({ members: [], avatar_url: "", quota_allowance: 0, + source: "user", }) export const getGroupSubtitle = (group: Group): string => { From feddfc8e6f64e053b0e8081292f2158cb34706f4 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Mon, 7 Aug 2023 15:01:43 -0500 Subject: [PATCH 18/18] Make gen --- coderd/database/dump.sql | 2 +- coderd/database/migrations/000148_group_source.up.sql | 2 +- coderd/database/models.go | 2 +- enterprise/coderd/userauth_test.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 59872f5ce8146..9dfb9ded10e64 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -310,7 +310,7 @@ CREATE TABLE groups ( COMMENT ON COLUMN groups.display_name IS 'Display name is a custom, human-friendly group name that user can set. This is not required to be unique and can be the empty string.'; -COMMENT ON COLUMN groups.source IS 'Source indicates how the group was created. It can be created by a user manually, or through some system process like and OIDC group sync.'; +COMMENT ON COLUMN groups.source IS 'Source indicates how the group was created. It can be created by a user manually, or through some system process like OIDC group sync.'; CREATE TABLE licenses ( id integer NOT NULL, diff --git a/coderd/database/migrations/000148_group_source.up.sql b/coderd/database/migrations/000148_group_source.up.sql index ab4bc32a78f4a..d06e89ca2b1d6 100644 --- a/coderd/database/migrations/000148_group_source.up.sql +++ b/coderd/database/migrations/000148_group_source.up.sql @@ -10,6 +10,6 @@ CREATE TYPE group_source AS ENUM ( ALTER TABLE groups ADD COLUMN source group_source NOT NULL DEFAULT 'user'; -COMMENT ON COLUMN groups.source IS 'Source indicates how the group was created. It can be created by a user manually, or through some system process like and OIDC group sync.'; +COMMENT ON COLUMN groups.source IS 'Source indicates how the group was created. It can be created by a user manually, or through some system process like OIDC group sync.'; COMMIT; diff --git a/coderd/database/models.go b/coderd/database/models.go index 4eae4eb9549bb..d3b7700b56bfa 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1556,7 +1556,7 @@ type Group struct { QuotaAllowance int32 `db:"quota_allowance" json:"quota_allowance"` // Display name is a custom, human-friendly group name that user can set. This is not required to be unique and can be the empty string. DisplayName string `db:"display_name" json:"display_name"` - // Source indicates how the group was created. It can be created by a user manually, or through some system process like and OIDC group sync. + // Source indicates how the group was created. It can be created by a user manually, or through some system process like OIDC group sync. Source GroupSource `db:"source" json:"source"` } diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 62d7eadff46d1..bafb2e1bfce13 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -468,7 +468,7 @@ func TestGroupSync(t *testing.T) { tc := tc t.Run(tc.name, func(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitLong*40000) + ctx := testutil.Context(t, testutil.WaitLong) conf := coderdtest.NewOIDCConfig(t, "") config := conf.OIDCConfig(t, jwt.MapClaims{}, tc.modCfg)