From 81feea5ff337f36978d6de5e6d9f5c220eb6cf1b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 19 Jul 2023 10:52:54 -0400 Subject: [PATCH 01/21] feat: oidc user role sync User roles come from oidc claims. Prevent manual user role changes if set. --- cli/server.go | 3 ++ cli/server_test.go | 2 + coderd/coderd.go | 9 ++++ coderd/userauth.go | 79 +++++++++++++++++++++++++++++++++++ coderd/users.go | 14 +++++-- codersdk/deployment.go | 35 ++++++++++++++++ enterprise/coderd/coderd.go | 1 + enterprise/coderd/userauth.go | 15 +++++++ 8 files changed, 155 insertions(+), 3 deletions(-) diff --git a/cli/server.go b/cli/server.go index ec9c049ecefaa..c43a750c8e0f6 100644 --- a/cli/server.go +++ b/cli/server.go @@ -595,6 +595,9 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. IgnoreUserInfo: cfg.OIDC.IgnoreUserInfo.Value(), GroupField: cfg.OIDC.GroupField.String(), GroupMapping: cfg.OIDC.GroupMapping.Value, + UserRoleField: cfg.OIDC.UserRoleField.String(), + UserRoleMapping: cfg.OIDC.UserRoleMapping.Value, + UserRolesDefault: cfg.OIDC.UserRolesDefault.GetSlice(), SignInText: cfg.OIDC.SignInText.String(), IconURL: cfg.OIDC.IconURL.String(), IgnoreEmailVerified: cfg.OIDC.IgnoreEmailVerified.Value(), diff --git a/cli/server_test.go b/cli/server_test.go index 6437fc68c28e1..a6f33be695672 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -1095,6 +1095,8 @@ func TestServer(t *testing.T) { require.False(t, deploymentConfig.Values.OIDC.IgnoreUserInfo.Value()) require.Empty(t, deploymentConfig.Values.OIDC.GroupField.Value()) require.Empty(t, deploymentConfig.Values.OIDC.GroupMapping.Value) + require.Empty(t, deploymentConfig.Values.OIDC.UserRoleField.Value()) + require.Empty(t, deploymentConfig.Values.OIDC.UserRoleMapping.Value) require.Equal(t, "OpenID Connect", deploymentConfig.Values.OIDC.SignInText.Value()) require.Empty(t, deploymentConfig.Values.OIDC.IconURL.Value()) }) diff --git a/coderd/coderd.go b/coderd/coderd.go index 3d41d62fded00..9fcb58c6f9253 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -124,6 +124,7 @@ type Options struct { DERPMap *tailcfg.DERPMap SwaggerEndpoint bool SetUserGroups func(ctx context.Context, tx database.Store, userID uuid.UUID, groupNames []string) error + SetUserSiteRoles func(ctx context.Context, tx database.Store, userID uuid.UUID, roles []string) error TemplateScheduleStore *atomic.Pointer[schedule.TemplateScheduleStore] // AppSecurityKey is the crypto key used to sign and encrypt tokens related to // workspace applications. It consists of both a signing and encryption key. @@ -257,6 +258,14 @@ func New(options *Options) *API { 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", + slog.F("user_id", userID), slog.F("roles", roles), + ) + return nil + } + } if options.TemplateScheduleStore == nil { options.TemplateScheduleStore = &atomic.Pointer[schedule.TemplateScheduleStore]{} } diff --git a/coderd/userauth.go b/coderd/userauth.go index 6a1aead1ef8cb..065c828ab4011 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -684,12 +684,27 @@ type OIDCConfig struct { // to groups within Coder. // map[oidcGroupName]coderGroupName GroupMapping map[string]string + // UserRoleField selects the claim field to be used as the created user's + // roles. If the field is the empty string, then no role updates + // will ever come from the OIDC provider. + UserRoleField string + // UserRoleMapping controls how groups returned by the OIDC provider get mapped + // to roles within Coder. + // map[oidcRoleName]coderRoleName + UserRoleMapping map[string]string + // UserRolesDefault is the default set of roles to assign to a user if role sync + // is enabled. + UserRolesDefault []string // SignInText is the text to display on the OIDC login button SignInText string // IconURL points to the URL of an icon to display on the OIDC login button IconURL string } +func (cfg OIDCConfig) RoleSyncEnabled() bool { + return cfg.UserRoleField != "" +} + // @Summary OpenID Connect Callback // @ID openid-connect-callback // @Security CoderSessionToken @@ -890,6 +905,55 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { } } + roles := api.OIDCConfig.UserRolesDefault + if api.OIDCConfig.RoleSyncEnabled() { + rolesRow, ok := claims[api.OIDCConfig.UserRoleField] + if !ok { + logger.Error(ctx, "oidc user roles are missing from claim") + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Login disabled until OIDC config is fixed, contact your administrator. Missing OIDC user roles in claim.", + Detail: "If role sync is enabled, then the OIDC user roles must be present in the claim. Disabling role sync will allow login to proceed.", + }) + return + } + + // Convert the []interface{} we get to a []string. + rolesInterface, ok := rolesRow.([]interface{}) + if !ok { + api.Logger.Error(ctx, "oidc claim user roles field was an unknown type", + slog.F("type", fmt.Sprintf("%T", rolesRow)), + ) + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Login disabled until OIDC config is fixed, contact your administrator. Missing OIDC user roles in claim.", + Detail: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow), + }) + return + } + + api.Logger.Debug(ctx, "roles returned in oidc claims", + slog.F("len", len(rolesInterface)), + slog.F("roles", rolesInterface), + ) + for _, roleInterface := range rolesInterface { + role, ok := roleInterface.(string) + if !ok { + api.Logger.Error(ctx, "invalid oidc user role type", + slog.F("type", fmt.Sprintf("%T", rolesRow)), + ) + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Invalid user role type. Expected string, got: %T", roleInterface), + }) + return + } + + if mappedRole, ok := api.OIDCConfig.UserRoleMapping[role]; ok { + role = mappedRole + } + + roles = append(roles, role) + } + } + // This conditional is purely to warn the user they might have misconfigured their OIDC // configuration. if _, groupClaimExists := claims["groups"]; !usingGroups && groupClaimExists { @@ -959,6 +1023,8 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { Username: username, AvatarURL: picture, UsingGroups: usingGroups, + UsingRoles: api.OIDCConfig.RoleSyncEnabled(), + Roles: roles, Groups: groups, }).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) { return audit.InitRequest[database.User](rw, params) @@ -1045,6 +1111,10 @@ type oauthLoginParams struct { // to the Groups provided. UsingGroups bool Groups []string + // Is UsingRoles is true, then the user will be assigned + // the roles provided. + UsingRoles bool + Roles []string commitLock sync.Mutex initAuditRequest func(params *audit.RequestParams) *audit.Request[database.User] @@ -1248,6 +1318,15 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C } } + // Ensure roles are correct. + if params.UsingRoles { + //nolint:gocritic + err := api.Options.SetUserSiteRoles(dbauthz.AsSystemRestricted(ctx), tx, user.ID, params.Roles) + if err != nil { + return xerrors.Errorf("set user groups: %w", err) + } + } + needsUpdate := false if user.AvatarURL.String != params.AvatarURL { user.AvatarURL = sql.NullString{ diff --git a/coderd/users.go b/coderd/users.go index 7f9f7a7f23a7d..7fced3ff31ccc 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -889,6 +889,14 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) { defer commitAudit() aReq.Old = user + if user.LoginType == database.LoginTypeOIDC && api.OIDCConfig.RoleSyncEnabled() { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Cannot modify roles for OIDC users when role sync is enabled.", + Detail: "'User Role Field' is set in the OIDC configuration. All role changes must come from the oidc identity provider.", + }) + return + } + if apiKey.UserID == user.ID { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "You cannot change your own roles.", @@ -901,7 +909,7 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) { return } - updatedUser, err := api.updateSiteUserRoles(ctx, database.UpdateUserRolesParams{ + updatedUser, err := api.UpdateSiteUserRoles(ctx, database.UpdateUserRolesParams{ GrantedRoles: params.Roles, ID: user.ID, }) @@ -929,9 +937,9 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, db2sdk.User(updatedUser, organizationIDs)) } -// updateSiteUserRoles will ensure only site wide roles are passed in as arguments. +// UpdateSiteUserRoles will ensure only site wide roles are passed in as arguments. // If an organization role is included, an error is returned. -func (api *API) updateSiteUserRoles(ctx context.Context, args database.UpdateUserRolesParams) (database.User, error) { +func (api *API) UpdateSiteUserRoles(ctx context.Context, args database.UpdateUserRolesParams) (database.User, error) { // Enforce only site wide roles. for _, r := range args.GrantedRoles { if _, ok := rbac.IsOrgRole(r); ok { diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 73e3c2dd5d450..2ced958622007 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -269,6 +269,9 @@ type OIDCConfig struct { IgnoreUserInfo clibase.Bool `json:"ignore_user_info" 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"` + UserRoleMapping clibase.Struct[map[string]string] `json:"user_role_mapping" typescript:",notnull"` + UserRolesDefault clibase.StringArray `json:"user_roles_default" typescript:",notnull"` SignInText clibase.String `json:"sign_in_text" typescript:",notnull"` IconURL clibase.URL `json:"icon_url" typescript:",notnull"` } @@ -1029,6 +1032,38 @@ when required by your organization's security policy.`, Group: &deploymentGroupOIDC, YAML: "groupMapping", }, + { + 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.", + Flag: "oidc-user-role-field", + Env: "CODER_OIDC_USER_ROLE_FIELD", + // This value is intentionally blank. If this is empty, then OIDC user role + // sync behavior is disabled. + Default: "", + Value: &c.OIDC.UserRoleField, + Group: &deploymentGroupOIDC, + YAML: "userRoleField", + }, + { + Name: "OIDC User Role Mapping", + Description: "A map of the OIDC passed in user roles and the groups in Coder it should map to. This is useful if the group names do not match.", + Flag: "oidc-user-role-mapping", + Env: "CODER_OIDC_USER_ROLE_MAPPING", + Default: "{}", + Value: &c.OIDC.UserRoleMapping, + Group: &deploymentGroupOIDC, + YAML: "userRoleMapping", + }, + { + Name: "OIDC User Role Default", + Description: "If user role sync is enabled, these roles are always included for all authenticated users. The 'member' role is always assigned.", + Flag: "oidc-user-role-default", + Env: "CODER_OIDC_USER_ROLE_DEFAULT", + Default: strings.Join([]string{}, ","), + Value: &c.OIDC.UserRolesDefault, + Group: &deploymentGroupOIDC, + YAML: "userRoleDefault", + }, { Name: "OpenID Connect sign in text", Description: "The text to show on the OpenID Connect sign in button.", diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 701eb405b9242..5d484d8aabf06 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -67,6 +67,7 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { }() api.AGPL.Options.SetUserGroups = api.setUserGroups + api.AGPL.Options.SetUserSiteRoles = api.setUserSiteRoles api.AGPL.SiteHandler.AppearanceFetcher = api.fetchAppearanceConfig api.AGPL.SiteHandler.RegionsFetcher = func(ctx context.Context) (any, error) { // If the user can read the workspace proxy resource, return that. diff --git a/enterprise/coderd/userauth.go b/enterprise/coderd/userauth.go index 46d486a3cebe1..0f02ecad1dbf6 100644 --- a/enterprise/coderd/userauth.go +++ b/enterprise/coderd/userauth.go @@ -50,3 +50,18 @@ func (api *API) setUserGroups(ctx context.Context, db database.Store, userID uui return nil }, nil) } + +func (api *API) setUserSiteRoles(ctx context.Context, db database.Store, userID uuid.UUID, roles []string) error { + // Should this be feature protected? + return db.InTx(func(tx database.Store) error { + _, err := api.AGPL.UpdateSiteUserRoles(ctx, database.UpdateUserRolesParams{ + GrantedRoles: roles, + ID: userID, + }) + if err != nil { + return xerrors.Errorf("set user roles: %w", err) + } + + return nil + }, nil) +} From 1e6bd370b2fd57c7de4509943224cbe89ed96cb7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 19 Jul 2023 14:11:39 -0400 Subject: [PATCH 02/21] allow mapping 1:many --- coderd/database/dbauthz/dbauthz.go | 2 +- coderd/rbac/roles.go | 11 ++- coderd/userauth.go | 135 ++++++++++++++++++----------- coderd/users.go | 6 +- codersdk/deployment.go | 38 ++++---- enterprise/coderd/userauth.go | 6 +- enterprise/coderd/userauth_test.go | 56 ++++++++++++ site/src/api/typesGenerated.ts | 6 ++ 8 files changed, 179 insertions(+), 81 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index ef2b7b9d18b91..e12eac547128d 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -207,7 +207,7 @@ var ( rbac.ResourceWildcard.Type: {rbac.ActionRead}, rbac.ResourceAPIKey.Type: {rbac.ActionCreate, rbac.ActionUpdate, rbac.ActionDelete}, rbac.ResourceGroup.Type: {rbac.ActionCreate, rbac.ActionUpdate}, - rbac.ResourceRoleAssignment.Type: {rbac.ActionCreate}, + rbac.ResourceRoleAssignment.Type: {rbac.ActionCreate, rbac.ActionDelete}, rbac.ResourceSystem.Type: {rbac.WildcardSymbol}, rbac.ResourceOrganization.Type: {rbac.ActionCreate}, rbac.ResourceOrganizationMember.Type: {rbac.ActionCreate}, diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index ee3805b716402..6286f8d9c8796 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -283,10 +283,13 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // map[actor_role][assign_role] var assignRoles = map[string]map[string]bool{ "system": { - owner: true, - member: true, - orgAdmin: true, - orgMember: true, + owner: true, + auditor: true, + member: true, + orgAdmin: true, + orgMember: true, + templateAdmin: true, + userAdmin: true, }, owner: { owner: true, diff --git a/coderd/userauth.go b/coderd/userauth.go index 065c828ab4011..ddb4d85c44501 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -691,7 +691,7 @@ type OIDCConfig struct { // UserRoleMapping controls how groups returned by the OIDC provider get mapped // to roles within Coder. // map[oidcRoleName]coderRoleName - UserRoleMapping map[string]string + UserRoleMapping map[string][]string // UserRolesDefault is the default set of roles to assign to a user if role sync // is enabled. UserRolesDefault []string @@ -905,55 +905,6 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { } } - roles := api.OIDCConfig.UserRolesDefault - if api.OIDCConfig.RoleSyncEnabled() { - rolesRow, ok := claims[api.OIDCConfig.UserRoleField] - if !ok { - logger.Error(ctx, "oidc user roles are missing from claim") - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Login disabled until OIDC config is fixed, contact your administrator. Missing OIDC user roles in claim.", - Detail: "If role sync is enabled, then the OIDC user roles must be present in the claim. Disabling role sync will allow login to proceed.", - }) - return - } - - // Convert the []interface{} we get to a []string. - rolesInterface, ok := rolesRow.([]interface{}) - if !ok { - api.Logger.Error(ctx, "oidc claim user roles field was an unknown type", - slog.F("type", fmt.Sprintf("%T", rolesRow)), - ) - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Login disabled until OIDC config is fixed, contact your administrator. Missing OIDC user roles in claim.", - Detail: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow), - }) - return - } - - api.Logger.Debug(ctx, "roles returned in oidc claims", - slog.F("len", len(rolesInterface)), - slog.F("roles", rolesInterface), - ) - for _, roleInterface := range rolesInterface { - role, ok := roleInterface.(string) - if !ok { - api.Logger.Error(ctx, "invalid oidc user role type", - slog.F("type", fmt.Sprintf("%T", rolesRow)), - ) - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: fmt.Sprintf("Invalid user role type. Expected string, got: %T", roleInterface), - }) - return - } - - if mappedRole, ok := api.OIDCConfig.UserRoleMapping[role]; ok { - role = mappedRole - } - - roles = append(roles, role) - } - } - // This conditional is purely to warn the user they might have misconfigured their OIDC // configuration. if _, groupClaimExists := claims["groups"]; !usingGroups && groupClaimExists { @@ -1006,6 +957,63 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { return } + roles := api.OIDCConfig.UserRolesDefault + if api.OIDCConfig.RoleSyncEnabled() { + rolesRow, ok := claims[api.OIDCConfig.UserRoleField] + if !ok { + // If no claim is provided than we can assume the user is just + // a member. This is because there is no way to tell the difference + // between []string{} and nil for OIDC claims. IDPs omit claims + // if they are empty ([]string{}). + rolesRow = []string{} + } + + // Convert the []interface{} we get to a []string. + rolesInterface, ok := rolesRow.([]interface{}) + if !ok { + api.Logger.Error(ctx, "oidc claim user roles field was an unknown type", + slog.F("type", fmt.Sprintf("%T", rolesRow)), + ) + site.RenderStaticErrorPage(rw, r, site.ErrorPageData{ + Status: http.StatusInternalServerError, + HideStatus: true, + Title: "Login disabled until OIDC config is fixed", + Description: fmt.Sprintf("Roles claim must be an array of strings, type found: %T. Disabling role sync will allow login to proceed.", rolesRow), + RetryEnabled: false, + DashboardURL: "/login", + }) + return + } + + api.Logger.Debug(ctx, "roles returned in oidc claims", + slog.F("len", len(rolesInterface)), + slog.F("roles", rolesInterface), + ) + for _, roleInterface := range rolesInterface { + role, ok := roleInterface.(string) + if !ok { + api.Logger.Error(ctx, "invalid oidc user role type", + slog.F("type", fmt.Sprintf("%T", rolesRow)), + ) + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Invalid user role type. Expected string, got: %T", roleInterface), + }) + return + } + + if mappedRoles, ok := api.OIDCConfig.UserRoleMapping[role]; ok { + if len(mappedRoles) == 0 { + continue + } + // Mapped roles are added to the list of roles + roles = append(roles, mappedRoles...) + continue + } + + roles = append(roles, role) + } + } + // If a new user is authenticating for the first time // the audit action is 'register', not 'login' if user.ID == uuid.Nil { @@ -1178,6 +1186,7 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C ctx = r.Context() user database.User cookies []*http.Cookie + logger = api.Logger.Named(userAuthLoggerName) ) var isConvertLoginType bool @@ -1320,10 +1329,32 @@ func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.C // Ensure roles are correct. if params.UsingRoles { + ignored := make([]string, 0) + filtered := make([]string, 0, len(params.Roles)) + for _, role := range params.Roles { + if _, err := rbac.RoleByName(role); err == nil { + filtered = append(filtered, role) + } else { + ignored = append(ignored, role) + } + } + //nolint:gocritic - err := api.Options.SetUserSiteRoles(dbauthz.AsSystemRestricted(ctx), tx, user.ID, params.Roles) + err := api.Options.SetUserSiteRoles(dbauthz.AsSystemRestricted(ctx), tx, user.ID, filtered) if err != nil { - return xerrors.Errorf("set user groups: %w", err) + return httpError{ + code: http.StatusBadRequest, + msg: "Invalid roles through OIDC claim", + detail: fmt.Sprintf("Error from role assignment attempt: %s", err.Error()), + renderStaticPage: true, + } + } + if len(ignored) > 0 { + logger.Debug(ctx, "OIDC roles ignored in assignment", + slog.F("ignored", ignored), + slog.F("assigned", filtered), + slog.F("user_id", user.ID), + ) } } diff --git a/coderd/users.go b/coderd/users.go index 7fced3ff31ccc..cbd2880e52e88 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -909,7 +909,7 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) { return } - updatedUser, err := api.UpdateSiteUserRoles(ctx, database.UpdateUserRolesParams{ + updatedUser, err := UpdateSiteUserRoles(ctx, api.Database, database.UpdateUserRolesParams{ GrantedRoles: params.Roles, ID: user.ID, }) @@ -939,7 +939,7 @@ func (api *API) putUserRoles(rw http.ResponseWriter, r *http.Request) { // UpdateSiteUserRoles will ensure only site wide roles are passed in as arguments. // If an organization role is included, an error is returned. -func (api *API) UpdateSiteUserRoles(ctx context.Context, args database.UpdateUserRolesParams) (database.User, error) { +func UpdateSiteUserRoles(ctx context.Context, db database.Store, args database.UpdateUserRolesParams) (database.User, error) { // Enforce only site wide roles. for _, r := range args.GrantedRoles { if _, ok := rbac.IsOrgRole(r); ok { @@ -951,7 +951,7 @@ func (api *API) UpdateSiteUserRoles(ctx context.Context, args database.UpdateUse } } - updatedUser, err := api.Database.UpdateUserRoles(ctx, args) + updatedUser, err := db.UpdateUserRoles(ctx, args) if err != nil { return database.User{}, xerrors.Errorf("update site roles: %w", err) } diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 2ced958622007..57e25a5c3d45d 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -256,24 +256,24 @@ type OAuth2GithubConfig struct { } type OIDCConfig struct { - AllowSignups clibase.Bool `json:"allow_signups" typescript:",notnull"` - ClientID clibase.String `json:"client_id" typescript:",notnull"` - ClientSecret clibase.String `json:"client_secret" typescript:",notnull"` - EmailDomain clibase.StringArray `json:"email_domain" typescript:",notnull"` - IssuerURL clibase.String `json:"issuer_url" typescript:",notnull"` - Scopes clibase.StringArray `json:"scopes" typescript:",notnull"` - IgnoreEmailVerified clibase.Bool `json:"ignore_email_verified" typescript:",notnull"` - UsernameField clibase.String `json:"username_field" typescript:",notnull"` - 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"` - 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"` - UserRoleMapping clibase.Struct[map[string]string] `json:"user_role_mapping" typescript:",notnull"` - UserRolesDefault clibase.StringArray `json:"user_roles_default" typescript:",notnull"` - SignInText clibase.String `json:"sign_in_text" typescript:",notnull"` - IconURL clibase.URL `json:"icon_url" typescript:",notnull"` + AllowSignups clibase.Bool `json:"allow_signups" typescript:",notnull"` + ClientID clibase.String `json:"client_id" typescript:",notnull"` + ClientSecret clibase.String `json:"client_secret" typescript:",notnull"` + EmailDomain clibase.StringArray `json:"email_domain" typescript:",notnull"` + IssuerURL clibase.String `json:"issuer_url" typescript:",notnull"` + Scopes clibase.StringArray `json:"scopes" typescript:",notnull"` + IgnoreEmailVerified clibase.Bool `json:"ignore_email_verified" typescript:",notnull"` + UsernameField clibase.String `json:"username_field" typescript:",notnull"` + 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"` + 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"` + UserRoleMapping clibase.Struct[map[string][]string] `json:"user_role_mapping" typescript:",notnull"` + UserRolesDefault clibase.StringArray `json:"user_roles_default" typescript:",notnull"` + SignInText clibase.String `json:"sign_in_text" typescript:",notnull"` + IconURL clibase.URL `json:"icon_url" typescript:",notnull"` } type TelemetryConfig struct { @@ -1046,7 +1046,7 @@ when required by your organization's security policy.`, }, { Name: "OIDC User Role Mapping", - Description: "A map of the OIDC passed in user roles and the groups in Coder it should map to. This is useful if the group names do not match.", + Description: "A map of the OIDC passed in user roles and the groups in Coder it should map to. This is useful if the group names do not match. If mapped to the empty string, the role will ignored.", Flag: "oidc-user-role-mapping", Env: "CODER_OIDC_USER_ROLE_MAPPING", Default: "{}", diff --git a/enterprise/coderd/userauth.go b/enterprise/coderd/userauth.go index 0f02ecad1dbf6..beed8bbae91f8 100644 --- a/enterprise/coderd/userauth.go +++ b/enterprise/coderd/userauth.go @@ -3,6 +3,8 @@ package coderd import ( "context" + "github.com/coder/coder/coderd" + "github.com/google/uuid" "golang.org/x/xerrors" @@ -54,12 +56,12 @@ func (api *API) setUserGroups(ctx context.Context, db database.Store, userID uui func (api *API) setUserSiteRoles(ctx context.Context, db database.Store, userID uuid.UUID, roles []string) error { // Should this be feature protected? return db.InTx(func(tx database.Store) error { - _, err := api.AGPL.UpdateSiteUserRoles(ctx, database.UpdateUserRolesParams{ + _, err := coderd.UpdateSiteUserRoles(ctx, db, database.UpdateUserRolesParams{ GrantedRoles: roles, ID: userID, }) if err != nil { - return xerrors.Errorf("set user roles: %w", err) + return xerrors.Errorf("set user roles(%s): %w", userID.String(), err) } return nil diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index ca787ba8f74e6..d8719f57ceda0 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/coderdtest" + "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/codersdk" "github.com/coder/coder/enterprise/coderd/coderdenttest" "github.com/coder/coder/testutil" @@ -24,6 +25,61 @@ import ( // nolint:bodyclose func TestUserOIDC(t *testing.T) { t.Parallel() + t.Run("Roles", func(t *testing.T) { + t.Parallel() + + t.Run("NewUser", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + conf := coderdtest.NewOIDCConfig(t, "") + + oidcRoleName := "TemplateAuthor" + + config := conf.OIDCConfig(t, jwt.MapClaims{}, func(cfg *coderd.OIDCConfig) { + cfg.UserRoleMapping = map[string][]string{oidcRoleName: {rbac.RoleTemplateAdmin(), rbac.RoleUserAdmin()}} + }) + config.AllowSignups = true + config.UserRoleField = "roles" + + client, _ := coderdenttest.New(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) + + resp := oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{ + "email": "alice@coder.com", + "roles": []string{"random", oidcRoleName, rbac.RoleOwner()}, + })) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + user, err := client.User(ctx, "alice") + require.NoError(t, err) + + require.Len(t, user.Roles, 3) + roleNames := []string{user.Roles[0].Name, user.Roles[1].Name, user.Roles[2].Name} + require.ElementsMatch(t, roleNames, []string{rbac.RoleTemplateAdmin(), rbac.RoleUserAdmin(), rbac.RoleOwner()}) + + // Now remove the roles with a new oidc login + resp = oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{ + "email": "alice@coder.com", + "roles": []string{"random"}, + })) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + user, err = client.User(ctx, "alice") + require.NoError(t, err) + + require.Len(t, user.Roles, 0) + }) + }) + t.Run("Groups", func(t *testing.T) { t.Parallel() t.Run("Assigns", func(t *testing.T) { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 26ca40b60aad4..841a702654e69 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -605,6 +605,12 @@ export interface OIDCConfig { // 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 readonly group_mapping: any + readonly user_role_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 + readonly user_role_mapping: any + // This is likely an enum in an external package ("github.com/coder/coder/cli/clibase.StringArray") + readonly user_roles_default: string[] readonly sign_in_text: string readonly icon_url: string } From aad57442d18bb090fd06493f064bf90479b5d8ba Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 19 Jul 2023 14:31:38 -0400 Subject: [PATCH 03/21] Expose user login type via api --- codersdk/users.go | 1 + enterprise/coderd/groups.go | 1 + enterprise/coderd/userauth_test.go | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/codersdk/users.go b/codersdk/users.go index 316d1579c621a..d421fae1312a1 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -45,6 +45,7 @@ type User struct { OrganizationIDs []uuid.UUID `json:"organization_ids" format:"uuid"` Roles []Role `json:"roles"` AvatarURL string `json:"avatar_url" format:"uri"` + LoginType LoginType `json:"login_type"` } type GetUsersResponse struct { diff --git a/enterprise/coderd/groups.go b/enterprise/coderd/groups.go index 2ce1685d97ca8..79fba137f81e8 100644 --- a/enterprise/coderd/groups.go +++ b/enterprise/coderd/groups.go @@ -416,6 +416,7 @@ func convertUser(user database.User, organizationIDs []uuid.UUID) codersdk.User OrganizationIDs: organizationIDs, Roles: make([]codersdk.Role, 0, len(user.RBACRoles)), AvatarURL: user.AvatarURL.String, + LoginType: codersdk.LoginType(user.LoginType), } for _, roleName := range user.RBACRoles { diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index d8719f57ceda0..a0f63240b0f1c 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -28,7 +28,7 @@ func TestUserOIDC(t *testing.T) { t.Run("Roles", func(t *testing.T) { t.Parallel() - t.Run("NewUser", func(t *testing.T) { + t.Run("NewUserAndRemoveRoles", func(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitMedium) From f9c7cc5f6eefcea68a90de7810fa760551ec90a7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 19 Jul 2023 14:32:46 -0400 Subject: [PATCH 04/21] make gen --- coderd/apidoc/docs.go | 18 ++++++++++++++ coderd/apidoc/swagger.json | 18 ++++++++++++++ docs/api/audit.md | 1 + docs/api/enterprise.md | 40 ++++++++++++++++++++++++-------- docs/api/general.md | 3 +++ docs/api/schemas.md | 21 +++++++++++++++++ docs/api/templates.md | 19 +++++++++++++++ docs/api/users.md | 9 +++++++ docs/cli/server.md | 31 +++++++++++++++++++++++++ site/src/api/typesGenerated.ts | 1 + site/src/testHelpers/entities.ts | 4 ++++ 11 files changed, 155 insertions(+), 10 deletions(-) diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index dd6c958448391..5417bd9974bb9 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -8164,6 +8164,18 @@ const docTemplate = `{ "sign_in_text": { "type": "string" }, + "user_role_field": { + "type": "string" + }, + "user_role_mapping": { + "type": "object" + }, + "user_roles_default": { + "type": "array", + "items": { + "type": "string" + } + }, "username_field": { "type": "string" } @@ -9036,6 +9048,9 @@ const docTemplate = `{ "type": "string", "format": "date-time" }, + "login_type": { + "$ref": "#/definitions/codersdk.LoginType" + }, "organization_ids": { "type": "array", "items": { @@ -9477,6 +9492,9 @@ const docTemplate = `{ "type": "string", "format": "date-time" }, + "login_type": { + "$ref": "#/definitions/codersdk.LoginType" + }, "organization_ids": { "type": "array", "items": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index aa2fedba869a6..ebff543aeaf4b 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -7331,6 +7331,18 @@ "sign_in_text": { "type": "string" }, + "user_role_field": { + "type": "string" + }, + "user_role_mapping": { + "type": "object" + }, + "user_roles_default": { + "type": "array", + "items": { + "type": "string" + } + }, "username_field": { "type": "string" } @@ -8158,6 +8170,9 @@ "type": "string", "format": "date-time" }, + "login_type": { + "$ref": "#/definitions/codersdk.LoginType" + }, "organization_ids": { "type": "array", "items": { @@ -8564,6 +8579,9 @@ "type": "string", "format": "date-time" }, + "login_type": { + "$ref": "#/definitions/codersdk.LoginType" + }, "organization_ids": { "type": "array", "items": { diff --git a/docs/api/audit.md b/docs/api/audit.md index 25d47af0bd36a..d5aeb78665d31 100644 --- a/docs/api/audit.md +++ b/docs/api/audit.md @@ -63,6 +63,7 @@ curl -X GET http://coder-server:8080/api/v2/audit?q=string \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { diff --git a/docs/api/enterprise.md b/docs/api/enterprise.md index e459d0ecdf2fa..5d317cb89ddc2 100644 --- a/docs/api/enterprise.md +++ b/docs/api/enterprise.md @@ -182,6 +182,7 @@ curl -X GET http://coder-server:8080/api/v2/groups/{group} \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -241,6 +242,7 @@ curl -X DELETE http://coder-server:8080/api/v2/groups/{group} \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -300,6 +302,7 @@ curl -X PATCH http://coder-server:8080/api/v2/groups/{group} \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -434,6 +437,7 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/groups "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -473,6 +477,7 @@ Status Code **200** | `»» 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 | | | @@ -485,10 +490,15 @@ Status Code **200** #### Enumerated Values -| Property | Value | -| -------- | ----------- | -| `status` | `active` | -| `status` | `suspended` | +| Property | Value | +| ------------ | ----------- | +| `login_type` | `password` | +| `login_type` | `github` | +| `login_type` | `oidc` | +| `login_type` | `token` | +| `login_type` | `none` | +| `status` | `active` | +| `status` | `suspended` | To perform this operation, you must be authenticated. [Learn more](authentication.md). @@ -538,6 +548,7 @@ curl -X POST http://coder-server:8080/api/v2/organizations/{organization}/groups "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -598,6 +609,7 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/groups/ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -959,6 +971,7 @@ curl -X PATCH http://coder-server:8080/api/v2/scim/v2/Users/{id} \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -1010,6 +1023,7 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/acl \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "role": "admin", "roles": [ @@ -1042,6 +1056,7 @@ Status Code **200** | `» 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 | | | | `» role` | [codersdk.TemplateRole](schemas.md#codersdktemplaterole) | false | | | | `» roles` | array | false | | | @@ -1052,12 +1067,17 @@ Status Code **200** #### Enumerated Values -| Property | Value | -| -------- | ----------- | -| `role` | `admin` | -| `role` | `use` | -| `status` | `active` | -| `status` | `suspended` | +| Property | Value | +| ------------ | ----------- | +| `login_type` | `password` | +| `login_type` | `github` | +| `login_type` | `oidc` | +| `login_type` | `token` | +| `login_type` | `none` | +| `role` | `admin` | +| `role` | `use` | +| `status` | `active` | +| `status` | `suspended` | 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 4b19ecd4f87e2..8d49b5a34bb75 100644 --- a/docs/api/general.md +++ b/docs/api/general.md @@ -279,6 +279,9 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \ "issuer_url": "string", "scopes": ["string"], "sign_in_text": "string", + "user_role_field": "string", + "user_role_mapping": {}, + "user_roles_default": ["string"], "username_field": "string" }, "pg_connection_url": "string", diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 51fe80fdab213..40d70e3d07739 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -1000,6 +1000,7 @@ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -1076,6 +1077,7 @@ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -1985,6 +1987,9 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "issuer_url": "string", "scopes": ["string"], "sign_in_text": "string", + "user_role_field": "string", + "user_role_mapping": {}, + "user_roles_default": ["string"], "username_field": "string" }, "pg_connection_url": "string", @@ -2335,6 +2340,9 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "issuer_url": "string", "scopes": ["string"], "sign_in_text": "string", + "user_role_field": "string", + "user_role_mapping": {}, + "user_roles_default": ["string"], "username_field": "string" }, "pg_connection_url": "string", @@ -2621,6 +2629,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -2837,6 +2846,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -3181,6 +3191,9 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "issuer_url": "string", "scopes": ["string"], "sign_in_text": "string", + "user_role_field": "string", + "user_role_mapping": {}, + "user_roles_default": ["string"], "username_field": "string" } ``` @@ -3203,6 +3216,9 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `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 @@ -4110,6 +4126,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "role": "admin", "roles": [ @@ -4132,6 +4149,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `email` | string | true | | | | `id` | string | true | | | | `last_seen_at` | string | false | | | +| `login_type` | [codersdk.LoginType](#codersdklogintype) | false | | | | `organization_ids` | array of string | false | | | | `role` | [codersdk.TemplateRole](#codersdktemplaterole) | false | | | | `roles` | array of [codersdk.Role](#codersdkrole) | false | | | @@ -4158,6 +4176,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -4610,6 +4629,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -4631,6 +4651,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `email` | string | true | | | | `id` | string | true | | | | `last_seen_at` | string | false | | | +| `login_type` | [codersdk.LoginType](#codersdklogintype) | false | | | | `organization_ids` | array of string | false | | | | `roles` | array of [codersdk.Role](#codersdkrole) | false | | | | `status` | [codersdk.UserStatus](#codersdkuserstatus) | false | | | diff --git a/docs/api/templates.md b/docs/api/templates.md index 032c620900834..2437b4656b867 100644 --- a/docs/api/templates.md +++ b/docs/api/templates.md @@ -360,6 +360,7 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/templat "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -441,6 +442,7 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/templat "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -546,6 +548,7 @@ curl -X POST http://coder-server:8080/api/v2/organizations/{organization}/templa "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -850,6 +853,7 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/versions \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -910,6 +914,7 @@ Status Code **200** | `»» 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 | | | @@ -944,6 +949,11 @@ Status Code **200** | Property | Value | | ------------ | ----------------------------- | +| `login_type` | `password` | +| `login_type` | `github` | +| `login_type` | `oidc` | +| `login_type` | `token` | +| `login_type` | `none` | | `status` | `active` | | `status` | `suspended` | | `error_code` | `MISSING_TEMPLATE_PARAMETER` | @@ -1045,6 +1055,7 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template}/versions/{templ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -1105,6 +1116,7 @@ Status Code **200** | `»» 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 | | | @@ -1139,6 +1151,11 @@ Status Code **200** | Property | Value | | ------------ | ----------------------------- | +| `login_type` | `password` | +| `login_type` | `github` | +| `login_type` | `oidc` | +| `login_type` | `token` | +| `login_type` | `none` | | `status` | `active` | | `status` | `suspended` | | `error_code` | `MISSING_TEMPLATE_PARAMETER` | @@ -1184,6 +1201,7 @@ curl -X GET http://coder-server:8080/api/v2/templateversions/{templateversion} \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -1274,6 +1292,7 @@ curl -X PATCH http://coder-server:8080/api/v2/templateversions/{templateversion} "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { diff --git a/docs/api/users.md b/docs/api/users.md index 1206d42c2e260..f22f97adb4ab8 100644 --- a/docs/api/users.md +++ b/docs/api/users.md @@ -36,6 +36,7 @@ curl -X GET http://coder-server:8080/api/v2/users \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -101,6 +102,7 @@ curl -X POST http://coder-server:8080/api/v2/users \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -359,6 +361,7 @@ curl -X GET http://coder-server:8080/api/v2/users/{user} \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -409,6 +412,7 @@ curl -X DELETE http://coder-server:8080/api/v2/users/{user} \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -1002,6 +1006,7 @@ curl -X PUT http://coder-server:8080/api/v2/users/{user}/profile \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -1052,6 +1057,7 @@ curl -X GET http://coder-server:8080/api/v2/users/{user}/roles \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -1112,6 +1118,7 @@ curl -X PUT http://coder-server:8080/api/v2/users/{user}/roles \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -1162,6 +1169,7 @@ curl -X PUT http://coder-server:8080/api/v2/users/{user}/status/activate \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { @@ -1212,6 +1220,7 @@ curl -X PUT http://coder-server:8080/api/v2/users/{user}/status/suspend \ "email": "user@example.com", "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "password", "organization_ids": ["497f6eca-6276-4993-bfeb-53cbbbba6f08"], "roles": [ { diff --git a/docs/cli/server.md b/docs/cli/server.md index 10ce138ea4d4c..258d44a77e401 100644 --- a/docs/cli/server.md +++ b/docs/cli/server.md @@ -512,6 +512,37 @@ Issuer URL to use for Login with OIDC. Scopes to grant when authenticating with OIDC. +### --oidc-user-role-default + +| | | +| ----------- | ------------------------------------------ | +| Type | string-array | +| Environment | $CODER_OIDC_USER_ROLE_DEFAULT | +| YAML | oidc.userRoleDefault | + +If user role sync is enabled, these roles are always included for all authenticated users. The 'member' role is always assigned. + +### --oidc-user-role-field + +| | | +| ----------- | ---------------------------------------- | +| Type | string | +| Environment | $CODER_OIDC_USER_ROLE_FIELD | +| YAML | oidc.userRoleField | + +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. + +### --oidc-user-role-mapping + +| | | +| ----------- | ------------------------------------------ | +| Type | struct[map[string][]string] | +| Environment | $CODER_OIDC_USER_ROLE_MAPPING | +| YAML | oidc.userRoleMapping | +| Default | {} | + +A map of the OIDC passed in user roles and the groups in Coder it should map to. This is useful if the group names do not match. If mapped to the empty string, the role will ignored. + ### --oidc-username-field | | | diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 841a702654e69..81890eccb0444 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1105,6 +1105,7 @@ export interface User { readonly organization_ids: string[] readonly roles: Role[] readonly avatar_url: string + readonly login_type: LoginType } // From codersdk/users.go diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 87c30ee22495e..8163d20c5bc1e 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -268,6 +268,7 @@ export const MockUser: TypesGen.User = { roles: [MockOwnerRole], avatar_url: "https://avatars.githubusercontent.com/u/95932066?s=200&v=4", last_seen_at: "", + login_type: "password", } export const MockUserAdmin: TypesGen.User = { @@ -280,6 +281,7 @@ export const MockUserAdmin: TypesGen.User = { roles: [MockUserAdminRole], avatar_url: "", last_seen_at: "", + login_type: "password", } export const MockUser2: TypesGen.User = { @@ -292,6 +294,7 @@ export const MockUser2: TypesGen.User = { roles: [], avatar_url: "", last_seen_at: "2022-09-14T19:12:21Z", + login_type: "oidc", } export const SuspendedMockUser: TypesGen.User = { @@ -304,6 +307,7 @@ export const SuspendedMockUser: TypesGen.User = { roles: [], avatar_url: "", last_seen_at: "", + login_type: "password", } export const MockProvisioner: TypesGen.ProvisionerDaemon = { From a7614e34ee2388d6f7282e910327b487a773797b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 19 Jul 2023 16:09:41 -0400 Subject: [PATCH 05/21] Adjust users page to not allow editing roles when they cannot --- coderd/database/db2sdk/db2sdk.go | 1 + .../EditRolesButton.stories.tsx | 2 + .../EditRolesButton/EditRolesButton.tsx | 41 +++++++++++++++---- .../components/UsersTable/UsersTableBody.tsx | 15 +++++++ .../AccountPage/AccountPage.test.tsx | 1 + 5 files changed, 51 insertions(+), 9 deletions(-) diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index be472131881d5..263611d5b168b 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -115,6 +115,7 @@ func User(user database.User, organizationIDs []uuid.UUID) codersdk.User { OrganizationIDs: organizationIDs, Roles: make([]codersdk.Role, 0, len(user.RBACRoles)), AvatarURL: user.AvatarURL.String, + LoginType: codersdk.LoginType(user.LoginType), } for _, roleName := range user.RBACRoles { diff --git a/site/src/components/EditRolesButton/EditRolesButton.stories.tsx b/site/src/components/EditRolesButton/EditRolesButton.stories.tsx index 0323d3dcc60be..66b58f0741300 100644 --- a/site/src/components/EditRolesButton/EditRolesButton.stories.tsx +++ b/site/src/components/EditRolesButton/EditRolesButton.stories.tsx @@ -34,6 +34,8 @@ Loading.args = { isLoading: true, roles: MockSiteRoles, selectedRoles: [MockUserAdminRole, MockOwnerRole], + userLoginType: "password", + oidcRoleSync: false, } Loading.parameters = { chromatic: { delay: 300 }, diff --git a/site/src/components/EditRolesButton/EditRolesButton.tsx b/site/src/components/EditRolesButton/EditRolesButton.tsx index 39c1b40bdb875..e49902936bee3 100644 --- a/site/src/components/EditRolesButton/EditRolesButton.tsx +++ b/site/src/components/EditRolesButton/EditRolesButton.tsx @@ -8,6 +8,12 @@ import { Stack } from "components/Stack/Stack" import Checkbox from "@mui/material/Checkbox" import UserIcon from "@mui/icons-material/PersonOutline" import { Role } from "api/typesGenerated" +import { + HelpTooltip, + HelpTooltipText, + HelpTooltipTitle, +} from "components/Tooltips/HelpTooltip" +import { Maybe } from "components/Conditionals/Maybe" const Option: React.FC<{ value: string @@ -46,6 +52,8 @@ export interface EditRolesButtonProps { selectedRoles: Role[] onChange: (roles: Role["name"][]) => void defaultIsOpen?: boolean + oidcRoleSync: boolean + userLoginType: string } export const EditRolesButton: FC = ({ @@ -54,6 +62,8 @@ export const EditRolesButton: FC = ({ onChange, isLoading, defaultIsOpen = false, + userLoginType, + oidcRoleSync, }) => { const styles = useStyles() const { t } = useTranslation("usersPage") @@ -71,17 +81,30 @@ export const EditRolesButton: FC = ({ onChange([...selectedRoleNames, roleName]) } + const canSetRoles = + userLoginType !== "oidc" || (userLoginType === "oidc" && !oidcRoleSync) + return ( <> - setIsOpen(true)} - > - - + + setIsOpen(true)} + > + + + + + + Externally controlled + + Roles for this user are controlled by the OIDC identity provider. + + + { return role.name === "owner" @@ -72,6 +75,16 @@ export const UsersTableBody: FC< const styles = useStyles() const { t } = useTranslation("usersPage") + const permissions = usePermissions() + const canViewDeployment = Boolean(permissions.viewDeploymentValues) + const [state] = useMachine(deploymentConfigMachine) + const { deploymentValues } = state.context + + // Indicates if oidc roles are synced from the oidc idp. + // Assign 'false' if unknown. + const oidcRoleSync = + canViewDeployment && deploymentValues?.config.oidc?.user_role_field !== "" + return ( @@ -127,6 +140,8 @@ export const UsersTableBody: FC< roles={roles ? sortRoles(roles) : []} selectedRoles={userRoles} isLoading={Boolean(isUpdatingUserRoles)} + userLoginType={user.login_type} + oidcRoleSync={oidcRoleSync} onChange={(roles) => { // Remove the fallback role because it is only for the UI const rolesWithoutFallback = roles.filter( diff --git a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx index b7576f58803c8..468d986fada23 100644 --- a/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx +++ b/site/src/pages/UserSettingsPage/AccountPage/AccountPage.test.tsx @@ -38,6 +38,7 @@ describe("AccountPage", () => { roles: [], avatar_url: "", last_seen_at: new Date().toString(), + login_type: "password", ...data, }), ) From 3fcb55d0ed8ff888553c68a6106d673d741326f7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 19 Jul 2023 16:13:39 -0400 Subject: [PATCH 06/21] Add TOD --- site/src/components/UsersTable/UsersTableBody.tsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/site/src/components/UsersTable/UsersTableBody.tsx b/site/src/components/UsersTable/UsersTableBody.tsx index 8a18a712662db..8995b7d277dac 100644 --- a/site/src/components/UsersTable/UsersTableBody.tsx +++ b/site/src/components/UsersTable/UsersTableBody.tsx @@ -77,6 +77,8 @@ export const UsersTableBody: FC< const permissions = usePermissions() const canViewDeployment = Boolean(permissions.viewDeploymentValues) + // Ideally this only runs if 'canViewDeployment' is true. + // TODO: Prevent api call if the user does not have the perms. const [state] = useMachine(deploymentConfigMachine) const { deploymentValues } = state.context From 89b1b69e192033f0fb3093ffef7ee3a442556f3c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 19 Jul 2023 16:18:19 -0400 Subject: [PATCH 07/21] Add unit test to test manual role assign --- enterprise/coderd/userauth_test.go | 40 +++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index a0f63240b0f1c..456554e7fa4a4 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -25,7 +25,7 @@ import ( // nolint:bodyclose func TestUserOIDC(t *testing.T) { t.Parallel() - t.Run("Roles", func(t *testing.T) { + t.Run("RoleSync", func(t *testing.T) { t.Parallel() t.Run("NewUserAndRemoveRoles", func(t *testing.T) { @@ -78,6 +78,44 @@ func TestUserOIDC(t *testing.T) { require.Len(t, user.Roles, 0) }) + t.Run("BlockAssignRoles", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + conf := coderdtest.NewOIDCConfig(t, "") + + config := conf.OIDCConfig(t, jwt.MapClaims{}) + config.AllowSignups = true + config.UserRoleField = "roles" + + client, _ := coderdenttest.New(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) + + resp := oidcCallback(t, client, conf.EncodeClaims(t, jwt.MapClaims{ + "email": "alice@coder.com", + "roles": []string{}, + })) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + // Try to manually update user roles, even though controlled by oidc + // role sync. + _, err = client.UpdateUserRoles(ctx, "alice", codersdk.UpdateRoles{ + Roles: []string{ + rbac.RoleTemplateAdmin(), + }, + }) + require.Error(t, err) + require.ErrorContains(t, err, "Cannot modify roles for OIDC users when role sync is enabled.") + }) }) t.Run("Groups", func(t *testing.T) { From b64e372d0058de83703451a6e0cbb6c59fb42fe6 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 19 Jul 2023 16:21:02 -0400 Subject: [PATCH 08/21] Add swagger docs as generated --- .gitattributes | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitattributes b/.gitattributes index 6a13a6f03307b..1e9db06a78dfc 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,5 +1,6 @@ # Generated files coderd/apidoc/docs.go linguist-generated=true +docs/api/*.md linguist-generated=true coderd/apidoc/swagger.json linguist-generated=true coderd/database/dump.sql linguist-generated=true peerbroker/proto/*.go linguist-generated=true @@ -9,3 +10,4 @@ provisionersdk/proto/*.go linguist-generated=true *.tfstate.json linguist-generated=true *.tfstate.dot linguist-generated=true *.tfplan.dot linguist-generated=true + From 9fddd5e59be7c27b14f9d11f5eb83f4f6bd184dc Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 19 Jul 2023 16:21:51 -0400 Subject: [PATCH 09/21] add cli generated files as generated --- .gitattributes | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitattributes b/.gitattributes index 1e9db06a78dfc..35f59c7ce002d 100644 --- a/.gitattributes +++ b/.gitattributes @@ -1,6 +1,7 @@ # Generated files coderd/apidoc/docs.go linguist-generated=true docs/api/*.md linguist-generated=true +docs/cli/*.md linguist-generated=true coderd/apidoc/swagger.json linguist-generated=true coderd/database/dump.sql linguist-generated=true peerbroker/proto/*.go linguist-generated=true From 882f93c2cc3f8fba829f4c47423c065fb6bce41f Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 19 Jul 2023 16:24:05 -0400 Subject: [PATCH 10/21] Fix import --- enterprise/coderd/userauth.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/enterprise/coderd/userauth.go b/enterprise/coderd/userauth.go index beed8bbae91f8..ee162319fa80f 100644 --- a/enterprise/coderd/userauth.go +++ b/enterprise/coderd/userauth.go @@ -3,11 +3,10 @@ package coderd import ( "context" - "github.com/coder/coder/coderd" - "github.com/google/uuid" "golang.org/x/xerrors" + "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" ) From f092c508cc12292abb5bbb6c1b383f9200c7f22d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 19 Jul 2023 16:25:59 -0400 Subject: [PATCH 11/21] Remove comment --- coderd/userauth.go | 1 - 1 file changed, 1 deletion(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index ddb4d85c44501..3285ccc0a27bb 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -968,7 +968,6 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { rolesRow = []string{} } - // Convert the []interface{} we get to a []string. rolesInterface, ok := rolesRow.([]interface{}) if !ok { api.Logger.Error(ctx, "oidc claim user roles field was an unknown type", From 5e3b6f88cf71f882a4ec68cf792b49a6d2675dc7 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 20 Jul 2023 08:41:34 -0400 Subject: [PATCH 12/21] Add new entitlement to control this feature --- codersdk/deployment.go | 2 ++ enterprise/coderd/userauth.go | 12 ++++++++++++ 2 files changed, 14 insertions(+) diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 57e25a5c3d45d..eed4b6b7c62ac 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -40,6 +40,7 @@ const ( FeatureBrowserOnly FeatureName = "browser_only" FeatureSCIM FeatureName = "scim" FeatureTemplateRBAC FeatureName = "template_rbac" + FeatureUserRoleManagement FeatureName = "user_role_management" FeatureHighAvailability FeatureName = "high_availability" FeatureMultipleGitAuth FeatureName = "multiple_git_auth" FeatureExternalProvisionerDaemons FeatureName = "external_provisioner_daemons" @@ -61,6 +62,7 @@ var FeatureNames = []FeatureName{ FeatureAppearance, FeatureAdvancedTemplateScheduling, FeatureWorkspaceProxy, + FeatureUserRoleManagement, } // Humanize returns the feature name in a human-readable format. diff --git a/enterprise/coderd/userauth.go b/enterprise/coderd/userauth.go index ee162319fa80f..8497d03851f75 100644 --- a/enterprise/coderd/userauth.go +++ b/enterprise/coderd/userauth.go @@ -6,6 +6,7 @@ import ( "github.com/google/uuid" "golang.org/x/xerrors" + "cdr.dev/slog" "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/database" "github.com/coder/coder/codersdk" @@ -53,6 +54,17 @@ func (api *API) setUserGroups(ctx context.Context, db database.Store, userID uui } func (api *API) setUserSiteRoles(ctx context.Context, 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.", + slog.F("user_id", userID), slog.F("roles", roles), + ) + return nil + } + // Should this be feature protected? return db.InTx(func(tx database.Store) error { _, err := coderd.UpdateSiteUserRoles(ctx, db, database.UpdateUserRolesParams{ From 4bf31c0391221a08661eeacd1255fc2f6a7d4f68 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 20 Jul 2023 09:10:48 -0400 Subject: [PATCH 13/21] Linting --- enterprise/coderd/userauth.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/enterprise/coderd/userauth.go b/enterprise/coderd/userauth.go index 8497d03851f75..86aa9f0ddf88b 100644 --- a/enterprise/coderd/userauth.go +++ b/enterprise/coderd/userauth.go @@ -59,7 +59,7 @@ func (api *API) setUserSiteRoles(ctx context.Context, db database.Store, userID api.entitlementsMu.RUnlock() if !enabled { - api.Logger.Warn(ctx, "attempted to assign OIDC user roles without enterprise entitlement, roles left unchanged.", + api.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 From 3ad152fdb2544c2dbad0be6623c49e351d8cee63 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 20 Jul 2023 09:17:39 -0400 Subject: [PATCH 14/21] not sure what feature enablements are, but required for tests --- enterprise/coderd/coderd.go | 1 + enterprise/coderd/userauth_test.go | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 5d484d8aabf06..b0ff1a9fee5f8 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -378,6 +378,7 @@ func (api *API) updateEntitlements(ctx context.Context) error { codersdk.FeatureExternalProvisionerDaemons: true, codersdk.FeatureAdvancedTemplateScheduling: true, codersdk.FeatureWorkspaceProxy: true, + codersdk.FeatureUserRoleManagement: true, }) if err != nil { return err diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 456554e7fa4a4..d6b5903c16766 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -47,7 +47,7 @@ func TestUserOIDC(t *testing.T) { OIDCConfig: config, }, LicenseOptions: &coderdenttest.LicenseOptions{ - Features: license.Features{codersdk.FeatureTemplateRBAC: 1}, + Features: license.Features{codersdk.FeatureUserRoleManagement: 1}, }, }) @@ -93,7 +93,7 @@ func TestUserOIDC(t *testing.T) { OIDCConfig: config, }, LicenseOptions: &coderdenttest.LicenseOptions{ - Features: license.Features{codersdk.FeatureTemplateRBAC: 1}, + Features: license.Features{codersdk.FeatureUserRoleManagement: 1}, }, }) From b0ae7952c49b1effc8c50068a80255e34ef7149c Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 20 Jul 2023 10:31:41 -0400 Subject: [PATCH 15/21] Fix merge error --- coderd/coderd.go | 4 ++-- site/src/api/typesGenerated.ts | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/coderd/coderd.go b/coderd/coderd.go index a71bf98a8bc27..28890429b183d 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -124,7 +124,7 @@ type Options struct { DERPMap *tailcfg.DERPMap SwaggerEndpoint bool SetUserGroups func(ctx context.Context, tx database.Store, userID uuid.UUID, groupNames []string) error - SetUserSiteRoles func(ctx context.Context, tx database.Store, userID uuid.UUID, roles []string) 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] // AppSecurityKey is the crypto key used to sign and encrypt tokens related to @@ -1106,7 +1106,7 @@ func (api *API) CreateInMemoryProvisionerDaemon(ctx context.Context, debounce ti } // nolint:revive -func initExperiments(log slog.Logger, raw []string) codersdk.Experiments { +func ReadExperiments(log slog.Logger, raw []string) codersdk.Experiments { exps := make([]codersdk.Experiment, 0, len(raw)) for _, v := range raw { switch v { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index a2ec5a91e54ac..8b3b9b839cce2 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1497,6 +1497,7 @@ export type FeatureName = | "template_rbac" | "template_restart_requirement" | "user_limit" + | "user_role_management" | "workspace_proxy" export const FeatureNames: FeatureName[] = [ "advanced_template_scheduling", @@ -1510,6 +1511,7 @@ export const FeatureNames: FeatureName[] = [ "template_rbac", "template_restart_requirement", "user_limit", + "user_role_management", "workspace_proxy", ] From 231e47b98397af58bcbc6bd0299daf27a545e6b5 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 20 Jul 2023 10:40:15 -0400 Subject: [PATCH 16/21] Fix unit test --- enterprise/coderd/coderd_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/enterprise/coderd/coderd_test.go b/enterprise/coderd/coderd_test.go index 71f95c97a69e2..2271c79064b4f 100644 --- a/enterprise/coderd/coderd_test.go +++ b/enterprise/coderd/coderd_test.go @@ -56,6 +56,7 @@ func TestEntitlements(t *testing.T) { codersdk.FeatureExternalProvisionerDaemons: 1, codersdk.FeatureAdvancedTemplateScheduling: 1, codersdk.FeatureWorkspaceProxy: 1, + codersdk.FeatureUserRoleManagement: 1, }, GraceAt: time.Now().Add(59 * 24 * time.Hour), }) From 40e362c59de1f8076c292c57c573f1930c61c549 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 20 Jul 2023 10:41:33 -0400 Subject: [PATCH 17/21] Update golden files --- cli/testdata/coder_server_--help.golden | 14 ++++++++++++++ cli/testdata/coder_users_list_--output_json.golden | 6 ++++-- cli/testdata/server-config.yaml.golden | 14 ++++++++++++++ enterprise/cli/testdata/coder_server_--help.golden | 14 ++++++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index c779bd399945a..35d68b84a0772 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -337,6 +337,20 @@ can safely ignore these settings. --oidc-scopes string-array, $CODER_OIDC_SCOPES (default: openid,profile,email) Scopes to grant when authenticating with OIDC. + --oidc-user-role-default string-array, $CODER_OIDC_USER_ROLE_DEFAULT + If user role sync is enabled, these roles are always included for all + authenticated users. The 'member' role is always assigned. + + --oidc-user-role-field string, $CODER_OIDC_USER_ROLE_FIELD + 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. + + --oidc-user-role-mapping struct[map[string][]string], $CODER_OIDC_USER_ROLE_MAPPING (default: {}) + A map of the OIDC passed in user roles and the groups in Coder it + should map to. This is useful if the group names do not match. If + mapped to the empty string, the role will ignored. + --oidc-username-field string, $CODER_OIDC_USERNAME_FIELD (default: preferred_username) OIDC claim field to use as the username. diff --git a/cli/testdata/coder_users_list_--output_json.golden b/cli/testdata/coder_users_list_--output_json.golden index b75519f4d2aa0..c3d8fe6695cd5 100644 --- a/cli/testdata/coder_users_list_--output_json.golden +++ b/cli/testdata/coder_users_list_--output_json.golden @@ -15,7 +15,8 @@ "display_name": "Owner" } ], - "avatar_url": "" + "avatar_url": "", + "login_type": "password" }, { "id": "[second user ID]", @@ -28,6 +29,7 @@ "[first org ID]" ], "roles": [], - "avatar_url": "" + "avatar_url": "", + "login_type": "password" } ] diff --git a/cli/testdata/server-config.yaml.golden b/cli/testdata/server-config.yaml.golden index 4ecfe0f071b73..5d4171a8888df 100644 --- a/cli/testdata/server-config.yaml.golden +++ b/cli/testdata/server-config.yaml.golden @@ -268,6 +268,20 @@ oidc: # for when OIDC providers only return group IDs. # (default: {}, type: struct[map[string]string]) groupMapping: {} + # 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. + # (default: , type: string) + userRoleField: "" + # A map of the OIDC passed in user roles and the groups in Coder it should map to. + # This is useful if the group names do not match. If mapped to the empty string, + # the role will ignored. + # (default: {}, type: struct[map[string][]string]) + userRoleMapping: {} + # If user role sync is enabled, these roles are always included for all + # authenticated users. The 'member' role is always assigned. + # (default: , type: string-array) + userRoleDefault: [] # The text to show on the OpenID Connect sign in button. # (default: OpenID Connect, type: string) signInText: OpenID Connect diff --git a/enterprise/cli/testdata/coder_server_--help.golden b/enterprise/cli/testdata/coder_server_--help.golden index c779bd399945a..35d68b84a0772 100644 --- a/enterprise/cli/testdata/coder_server_--help.golden +++ b/enterprise/cli/testdata/coder_server_--help.golden @@ -337,6 +337,20 @@ can safely ignore these settings. --oidc-scopes string-array, $CODER_OIDC_SCOPES (default: openid,profile,email) Scopes to grant when authenticating with OIDC. + --oidc-user-role-default string-array, $CODER_OIDC_USER_ROLE_DEFAULT + If user role sync is enabled, these roles are always included for all + authenticated users. The 'member' role is always assigned. + + --oidc-user-role-field string, $CODER_OIDC_USER_ROLE_FIELD + 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. + + --oidc-user-role-mapping struct[map[string][]string], $CODER_OIDC_USER_ROLE_MAPPING (default: {}) + A map of the OIDC passed in user roles and the groups in Coder it + should map to. This is useful if the group names do not match. If + mapped to the empty string, the role will ignored. + --oidc-username-field string, $CODER_OIDC_USERNAME_FIELD (default: preferred_username) OIDC claim field to use as the username. From bac7743acb2842da73035f8b88f82a3b279d5e5d Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 20 Jul 2023 12:06:36 -0400 Subject: [PATCH 18/21] raise a FE prop field up the stack --- site/src/components/UsersTable/UsersTable.tsx | 3 +++ .../components/UsersTable/UsersTableBody.tsx | 22 +++++-------------- site/src/pages/UsersPage/UsersPage.tsx | 14 +++++++++++- site/src/pages/UsersPage/UsersPageView.tsx | 3 +++ 4 files changed, 25 insertions(+), 17 deletions(-) diff --git a/site/src/components/UsersTable/UsersTable.tsx b/site/src/components/UsersTable/UsersTable.tsx index 456f9199e2373..6c991ee2a1dd0 100644 --- a/site/src/components/UsersTable/UsersTable.tsx +++ b/site/src/components/UsersTable/UsersTable.tsx @@ -36,6 +36,7 @@ export interface UsersTableProps { ) => void isNonInitialPage: boolean actorID: string + oidcRoleSyncEnabled: boolean } export const UsersTable: FC> = ({ @@ -54,6 +55,7 @@ export const UsersTable: FC> = ({ isLoading, isNonInitialPage, actorID, + oidcRoleSyncEnabled, }) => { return ( @@ -91,6 +93,7 @@ export const UsersTable: FC> = ({ onUpdateUserRoles={onUpdateUserRoles} isNonInitialPage={isNonInitialPage} actorID={actorID} + oidcRoleSyncEnabled={oidcRoleSyncEnabled} /> diff --git a/site/src/components/UsersTable/UsersTableBody.tsx b/site/src/components/UsersTable/UsersTableBody.tsx index 8995b7d277dac..24b958acfd07a 100644 --- a/site/src/components/UsersTable/UsersTableBody.tsx +++ b/site/src/components/UsersTable/UsersTableBody.tsx @@ -16,9 +16,6 @@ import { TableRowMenu } from "../TableRowMenu/TableRowMenu" import { EditRolesButton } from "components/EditRolesButton/EditRolesButton" import { Stack } from "components/Stack/Stack" import { EnterpriseBadge } from "components/DeploySettingsLayout/Badges" -import { usePermissions } from "hooks" -import { deploymentConfigMachine } from "xServices/deploymentConfig/deploymentConfigMachine" -import { useMachine } from "@xstate/react" const isOwnerRole = (role: TypesGen.Role): boolean => { return role.name === "owner" @@ -51,6 +48,10 @@ interface UsersTableBodyProps { ) => void isNonInitialPage: boolean actorID: string + // oidcRoleSyncEnabled should be set to false if unknown. + // This is used to determine if the oidc roles are synced from the oidc idp and + // editing via the UI should be disabled. + oidcRoleSyncEnabled: boolean } export const UsersTableBody: FC< @@ -71,22 +72,11 @@ export const UsersTableBody: FC< isLoading, isNonInitialPage, actorID, + oidcRoleSyncEnabled, }) => { const styles = useStyles() const { t } = useTranslation("usersPage") - const permissions = usePermissions() - const canViewDeployment = Boolean(permissions.viewDeploymentValues) - // Ideally this only runs if 'canViewDeployment' is true. - // TODO: Prevent api call if the user does not have the perms. - const [state] = useMachine(deploymentConfigMachine) - const { deploymentValues } = state.context - - // Indicates if oidc roles are synced from the oidc idp. - // Assign 'false' if unknown. - const oidcRoleSync = - canViewDeployment && deploymentValues?.config.oidc?.user_role_field !== "" - return ( @@ -143,7 +133,7 @@ export const UsersTableBody: FC< selectedRoles={userRoles} isLoading={Boolean(isUpdatingUserRoles)} userLoginType={user.login_type} - oidcRoleSync={oidcRoleSync} + oidcRoleSync={oidcRoleSyncEnabled} onChange={(roles) => { // Remove the fallback role because it is only for the UI const rolesWithoutFallback = roles.filter( diff --git a/site/src/pages/UsersPage/UsersPage.tsx b/site/src/pages/UsersPage/UsersPage.tsx index 95c0f5670be7d..67bdc9d4ae20c 100644 --- a/site/src/pages/UsersPage/UsersPage.tsx +++ b/site/src/pages/UsersPage/UsersPage.tsx @@ -20,6 +20,7 @@ import { UsersPageView } from "./UsersPageView" import { useStatusFilterMenu } from "./UsersFilter" import { useFilter } from "components/Filter/filter" import { useDashboard } from "components/Dashboard/DashboardProvider" +import { deploymentConfigMachine } from "xServices/deploymentConfig/deploymentConfigMachine" export const Language = { suspendDialogTitle: "Suspend user", @@ -61,7 +62,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { count, } = usersState.context - const { updateUsers: canEditUsers } = usePermissions() + const { updateUsers: canEditUsers, viewDeploymentValues } = usePermissions() const [rolesState] = useMachine(siteRolesMachine, { context: { hasPermission: canEditUsers, @@ -69,6 +70,16 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { }) const { roles } = rolesState.context + // Ideally this only runs if 'canViewDeployment' is true. + // TODO: Prevent api call if the user does not have the perms. + const [state] = useMachine(deploymentConfigMachine) + const { deploymentValues } = state.context + // Indicates if oidc roles are synced from the oidc idp. + // Assign 'false' if unknown. + const oidcRoleSyncEnabled = + viewDeploymentValues && + deploymentValues?.config.oidc?.user_role_field !== "" + // Is loading if // - users are loading or // - the user can edit the users but the roles are loading @@ -102,6 +113,7 @@ export const UsersPage: FC<{ children?: ReactNode }> = () => { {pageTitle("Users")} void @@ -47,6 +48,7 @@ export const UsersPageView: FC> = ({ onUpdateUserRoles, isUpdatingUserRoles, canEditUsers, + oidcRoleSyncEnabled, canViewActivity, isLoading, filterProps, @@ -77,6 +79,7 @@ export const UsersPageView: FC> = ({ onUpdateUserRoles={onUpdateUserRoles} isUpdatingUserRoles={isUpdatingUserRoles} canEditUsers={canEditUsers} + oidcRoleSyncEnabled={oidcRoleSyncEnabled} canViewActivity={canViewActivity} isLoading={isLoading} isNonInitialPage={isNonInitialPage} From 6cf15e7b391775e13f9af972c83be7b7c34df5fd Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 20 Jul 2023 12:14:53 -0400 Subject: [PATCH 19/21] Fix ts type --- site/src/pages/UsersPage/UsersPageView.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/pages/UsersPage/UsersPageView.tsx b/site/src/pages/UsersPage/UsersPageView.tsx index 4a3bb62e9838a..ba3577337d8c0 100644 --- a/site/src/pages/UsersPage/UsersPageView.tsx +++ b/site/src/pages/UsersPage/UsersPageView.tsx @@ -15,7 +15,7 @@ export interface UsersPageViewProps { count?: number roles?: TypesGen.AssignableRoles[] isUpdatingUserRoles?: boolean - canEditUsers?: boolean + canEditUsers: boolean oidcRoleSyncEnabled?: boolean canViewActivity?: boolean isLoading?: boolean From a2208bb521f98c94ea668abd313dbf255a57b555 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Thu, 20 Jul 2023 13:18:11 -0400 Subject: [PATCH 20/21] fixup! Fix ts type --- site/src/pages/UsersPage/UsersPageView.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/site/src/pages/UsersPage/UsersPageView.tsx b/site/src/pages/UsersPage/UsersPageView.tsx index ba3577337d8c0..a25d79391cfc5 100644 --- a/site/src/pages/UsersPage/UsersPageView.tsx +++ b/site/src/pages/UsersPage/UsersPageView.tsx @@ -15,8 +15,8 @@ export interface UsersPageViewProps { count?: number roles?: TypesGen.AssignableRoles[] isUpdatingUserRoles?: boolean - canEditUsers: boolean - oidcRoleSyncEnabled?: boolean + canEditUsers?: boolean + oidcRoleSyncEnabled: boolean canViewActivity?: boolean isLoading?: boolean onSuspendUser: (user: TypesGen.User) => void From a5227e8c221b9ed50f4353bd5cec5010de2c68db Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 21 Jul 2023 15:13:33 -0400 Subject: [PATCH 21/21] Fix comment --- coderd/userauth.go | 2 +- codersdk/deployment.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/userauth.go b/coderd/userauth.go index 3285ccc0a27bb..5b13a3d8d23ba 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -690,7 +690,7 @@ type OIDCConfig struct { UserRoleField string // UserRoleMapping controls how groups returned by the OIDC provider get mapped // to roles within Coder. - // map[oidcRoleName]coderRoleName + // map[oidcRoleName][]coderRoleName UserRoleMapping map[string][]string // UserRolesDefault is the default set of roles to assign to a user if role sync // is enabled. diff --git a/codersdk/deployment.go b/codersdk/deployment.go index 2107d22dd2059..a01e925abe1c8 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1075,7 +1075,7 @@ when required by your organization's security policy.`, Description: "If user role sync is enabled, these roles are always included for all authenticated users. The 'member' role is always assigned.", Flag: "oidc-user-role-default", Env: "CODER_OIDC_USER_ROLE_DEFAULT", - Default: strings.Join([]string{}, ","), + Default: "", Value: &c.OIDC.UserRolesDefault, Group: &deploymentGroupOIDC, YAML: "userRoleDefault",