From 5aebc4f000354d9fa448033445c6f313f4c6ab08 Mon Sep 17 00:00:00 2001 From: Jimmy Praet Date: Fri, 29 Apr 2022 21:38:11 +0200 Subject: [PATCH] Respect DefaultUserIsRestricted system default when creating new user (#19310) * Apply DefaultUserIsRestricted in CreateUser * Enforce system defaults in CreateUser Allow for overwrites with CreateUserOverwriteOptions * Fix compilation errors * Add "restricted" option to create user command * Add "restricted" option to create user admin api * Respect default setting.Service.RegisterEmailConfirm and setting.Service.RegisterManualConfirm where needed * Revert "Respect default setting.Service.RegisterEmailConfirm and setting.Service.RegisterManualConfirm where needed" This reverts commit ee95d3e8dc9e9fff4fa66a5111e4d3930280e033. --- cmd/admin.go | 20 ++++++++-- models/user/user.go | 37 ++++++++++++++++++- modules/structs/admin_user.go | 1 + routers/api/v1/admin/user.go | 16 +++++--- routers/install/install.go | 16 +++++--- routers/web/admin/users.go | 8 +++- routers/web/auth/auth.go | 18 ++++----- routers/web/auth/linkaccount.go | 3 +- routers/web/auth/oauth.go | 21 ++++++----- routers/web/auth/openid.go | 9 ++--- services/auth/reverseproxy.go | 13 +++++-- .../auth/source/ldap/source_authenticate.go | 25 +++++++------ services/auth/source/ldap/source_sync.go | 25 +++++++------ .../auth/source/pam/source_authenticate.go | 7 +++- .../auth/source/smtp/source_authenticate.go | 6 ++- services/auth/sspi_windows.go | 24 +++++++----- templates/swagger/v1_json.tmpl | 4 ++ 17 files changed, 169 insertions(+), 84 deletions(-) diff --git a/cmd/admin.go b/cmd/admin.go index e4a254c61..36cac50a4 100644 --- a/cmd/admin.go +++ b/cmd/admin.go @@ -25,6 +25,7 @@ import ( repo_module "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/storage" + "code.gitea.io/gitea/modules/util" auth_service "code.gitea.io/gitea/services/auth" "code.gitea.io/gitea/services/auth/source/oauth2" "code.gitea.io/gitea/services/auth/source/smtp" @@ -114,6 +115,10 @@ var ( Name: "access-token", Usage: "Generate access token for the user", }, + cli.BoolFlag{ + Name: "restricted", + Usage: "Make a restricted user account", + }, }, } @@ -559,17 +564,26 @@ func runCreateUser(c *cli.Context) error { changePassword = c.Bool("must-change-password") } + restricted := util.OptionalBoolNone + + if c.IsSet("restricted") { + restricted = util.OptionalBoolOf(c.Bool("restricted")) + } + u := &user_model.User{ Name: username, Email: c.String("email"), Passwd: password, - IsActive: true, IsAdmin: c.Bool("admin"), MustChangePassword: changePassword, - Theme: setting.UI.DefaultTheme, } - if err := user_model.CreateUser(u); err != nil { + overwriteDefault := &user_model.CreateUserOverwriteOptions{ + IsActive: util.OptionalBoolTrue, + IsRestricted: restricted, + } + + if err := user_model.CreateUser(u, overwriteDefault); err != nil { return fmt.Errorf("CreateUser: %v", err) } diff --git a/models/user/user.go b/models/user/user.go index c84889523..5b556c988 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -621,7 +621,14 @@ func IsUsableUsername(name string) error { // CreateUserOverwriteOptions are an optional options who overwrite system defaults on user creation type CreateUserOverwriteOptions struct { - Visibility structs.VisibleType + KeepEmailPrivate util.OptionalBool + Visibility *structs.VisibleType + AllowCreateOrganization util.OptionalBool + EmailNotificationsPreference *string + MaxRepoCreation *int + Theme *string + IsRestricted util.OptionalBool + IsActive util.OptionalBool } // CreateUser creates record of a new user. @@ -637,10 +644,36 @@ func CreateUser(u *User, overwriteDefault ...*CreateUserOverwriteOptions) (err e u.EmailNotificationsPreference = setting.Admin.DefaultEmailNotification u.MaxRepoCreation = -1 u.Theme = setting.UI.DefaultTheme + u.IsRestricted = setting.Service.DefaultUserIsRestricted + u.IsActive = !(setting.Service.RegisterEmailConfirm || setting.Service.RegisterManualConfirm) // overwrite defaults if set if len(overwriteDefault) != 0 && overwriteDefault[0] != nil { - u.Visibility = overwriteDefault[0].Visibility + overwrite := overwriteDefault[0] + if !overwrite.KeepEmailPrivate.IsNone() { + u.KeepEmailPrivate = overwrite.KeepEmailPrivate.IsTrue() + } + if overwrite.Visibility != nil { + u.Visibility = *overwrite.Visibility + } + if !overwrite.AllowCreateOrganization.IsNone() { + u.AllowCreateOrganization = overwrite.AllowCreateOrganization.IsTrue() + } + if overwrite.EmailNotificationsPreference != nil { + u.EmailNotificationsPreference = *overwrite.EmailNotificationsPreference + } + if overwrite.MaxRepoCreation != nil { + u.MaxRepoCreation = *overwrite.MaxRepoCreation + } + if overwrite.Theme != nil { + u.Theme = *overwrite.Theme + } + if !overwrite.IsRestricted.IsNone() { + u.IsRestricted = overwrite.IsRestricted.IsTrue() + } + if !overwrite.IsActive.IsNone() { + u.IsActive = overwrite.IsActive.IsTrue() + } } // validate data diff --git a/modules/structs/admin_user.go b/modules/structs/admin_user.go index facf16a39..eccbf29a4 100644 --- a/modules/structs/admin_user.go +++ b/modules/structs/admin_user.go @@ -19,6 +19,7 @@ type CreateUserOption struct { Password string `json:"password" binding:"Required;MaxSize(255)"` MustChangePassword *bool `json:"must_change_password"` SendNotify bool `json:"send_notify"` + Restricted *bool `json:"restricted"` Visibility string `json:"visibility" binding:"In(,public,limited,private)"` } diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index bf176f957..775802449 100644 --- a/routers/api/v1/admin/user.go +++ b/routers/api/v1/admin/user.go @@ -22,6 +22,7 @@ import ( "code.gitea.io/gitea/modules/password" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/routers/api/v1/user" "code.gitea.io/gitea/routers/api/v1/utils" @@ -82,7 +83,6 @@ func CreateUser(ctx *context.APIContext) { Email: form.Email, Passwd: form.Password, MustChangePassword: true, - IsActive: true, LoginType: auth.Plain, } if form.MustChangePassword != nil { @@ -108,11 +108,17 @@ func CreateUser(ctx *context.APIContext) { return } - var overwriteDefault *user_model.CreateUserOverwriteOptions + overwriteDefault := &user_model.CreateUserOverwriteOptions{ + IsActive: util.OptionalBoolTrue, + } + + if form.Restricted != nil { + overwriteDefault.IsRestricted = util.OptionalBoolOf(*form.Restricted) + } + if form.Visibility != "" { - overwriteDefault = &user_model.CreateUserOverwriteOptions{ - Visibility: api.VisibilityModes[form.Visibility], - } + visibility := api.VisibilityModes[form.Visibility] + overwriteDefault.Visibility = &visibility } if err := user_model.CreateUser(u, overwriteDefault); err != nil { diff --git a/routers/install/install.go b/routers/install/install.go index ec1719439..9d28bc157 100644 --- a/routers/install/install.go +++ b/routers/install/install.go @@ -499,13 +499,17 @@ func SubmitInstall(ctx *context.Context) { // Create admin account if len(form.AdminName) > 0 { u := &user_model.User{ - Name: form.AdminName, - Email: form.AdminEmail, - Passwd: form.AdminPasswd, - IsAdmin: true, - IsActive: true, + Name: form.AdminName, + Email: form.AdminEmail, + Passwd: form.AdminPasswd, + IsAdmin: true, } - if err = user_model.CreateUser(u); err != nil { + overwriteDefault := &user_model.CreateUserOverwriteOptions{ + IsRestricted: util.OptionalBoolFalse, + IsActive: util.OptionalBoolTrue, + } + + if err = user_model.CreateUser(u, overwriteDefault); err != nil { if !user_model.IsErrUserAlreadyExist(err) { setting.InstallLock = false ctx.Data["Err_AdminName"] = true diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index fcfea5380..57da319d7 100644 --- a/routers/web/admin/users.go +++ b/routers/web/admin/users.go @@ -125,10 +125,14 @@ func NewUserPost(ctx *context.Context) { Name: form.UserName, Email: form.Email, Passwd: form.Password, - IsActive: true, LoginType: auth.Plain, } + overwriteDefault := &user_model.CreateUserOverwriteOptions{ + IsActive: util.OptionalBoolTrue, + Visibility: &form.Visibility, + } + if len(form.LoginType) > 0 { fields := strings.Split(form.LoginType, "-") if len(fields) == 2 { @@ -163,7 +167,7 @@ func NewUserPost(ctx *context.Context) { u.MustChangePassword = form.MustChangePassword } - if err := user_model.CreateUser(u, &user_model.CreateUserOverwriteOptions{Visibility: form.Visibility}); err != nil { + if err := user_model.CreateUser(u, overwriteDefault); err != nil { switch { case user_model.IsErrUserAlreadyExist(err): ctx.Data["Err_UserName"] = true diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index c82fde49e..e8250616a 100644 --- a/routers/web/auth/auth.go +++ b/routers/web/auth/auth.go @@ -507,14 +507,12 @@ func SignUpPost(ctx *context.Context) { } u := &user_model.User{ - Name: form.UserName, - Email: form.Email, - Passwd: form.Password, - IsActive: !(setting.Service.RegisterEmailConfirm || setting.Service.RegisterManualConfirm), - IsRestricted: setting.Service.DefaultUserIsRestricted, + Name: form.UserName, + Email: form.Email, + Passwd: form.Password, } - if !createAndHandleCreatedUser(ctx, tplSignUp, form, u, nil, false) { + if !createAndHandleCreatedUser(ctx, tplSignUp, form, u, nil, nil, false) { // error already handled return } @@ -525,8 +523,8 @@ func SignUpPost(ctx *context.Context) { // createAndHandleCreatedUser calls createUserInContext and // then handleUserCreated. -func createAndHandleCreatedUser(ctx *context.Context, tpl base.TplName, form interface{}, u *user_model.User, gothUser *goth.User, allowLink bool) bool { - if !createUserInContext(ctx, tpl, form, u, gothUser, allowLink) { +func createAndHandleCreatedUser(ctx *context.Context, tpl base.TplName, form interface{}, u *user_model.User, overwrites *user_model.CreateUserOverwriteOptions, gothUser *goth.User, allowLink bool) bool { + if !createUserInContext(ctx, tpl, form, u, overwrites, gothUser, allowLink) { return false } return handleUserCreated(ctx, u, gothUser) @@ -534,8 +532,8 @@ func createAndHandleCreatedUser(ctx *context.Context, tpl base.TplName, form int // createUserInContext creates a user and handles errors within a given context. // Optionally a template can be specified. -func createUserInContext(ctx *context.Context, tpl base.TplName, form interface{}, u *user_model.User, gothUser *goth.User, allowLink bool) (ok bool) { - if err := user_model.CreateUser(u); err != nil { +func createUserInContext(ctx *context.Context, tpl base.TplName, form interface{}, u *user_model.User, overwrites *user_model.CreateUserOverwriteOptions, gothUser *goth.User, allowLink bool) (ok bool) { + if err := user_model.CreateUser(u, overwrites); err != nil { if allowLink && (user_model.IsErrUserAlreadyExist(err) || user_model.IsErrEmailAlreadyUsed(err)) { if setting.OAuth2Client.AccountLinking == setting.OAuth2AccountLinkingAuto { var user *user_model.User diff --git a/routers/web/auth/linkaccount.go b/routers/web/auth/linkaccount.go index bf5fb8326..c3e96f077 100644 --- a/routers/web/auth/linkaccount.go +++ b/routers/web/auth/linkaccount.go @@ -283,13 +283,12 @@ func LinkAccountPostRegister(ctx *context.Context) { Name: form.UserName, Email: form.Email, Passwd: form.Password, - IsActive: !(setting.Service.RegisterEmailConfirm || setting.Service.RegisterManualConfirm), LoginType: auth.OAuth2, LoginSource: authSource.ID, LoginName: gothUser.UserID, } - if !createAndHandleCreatedUser(ctx, tplLinkAccount, form, u, &gothUser, false) { + if !createAndHandleCreatedUser(ctx, tplLinkAccount, form, u, nil, &gothUser, false) { // error already handled return } diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 12de208ad..4c3e3c3ac 100644 --- a/routers/web/auth/oauth.go +++ b/routers/web/auth/oauth.go @@ -24,6 +24,7 @@ import ( "code.gitea.io/gitea/modules/session" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web/middleware" auth_service "code.gitea.io/gitea/services/auth" @@ -867,19 +868,21 @@ func SignInOAuthCallback(ctx *context.Context) { return } u = &user_model.User{ - Name: getUserName(&gothUser), - FullName: gothUser.Name, - Email: gothUser.Email, - IsActive: !setting.OAuth2Client.RegisterEmailConfirm, - LoginType: auth.OAuth2, - LoginSource: authSource.ID, - LoginName: gothUser.UserID, - IsRestricted: setting.Service.DefaultUserIsRestricted, + Name: getUserName(&gothUser), + FullName: gothUser.Name, + Email: gothUser.Email, + LoginType: auth.OAuth2, + LoginSource: authSource.ID, + LoginName: gothUser.UserID, + } + + overwriteDefault := &user_model.CreateUserOverwriteOptions{ + IsActive: util.OptionalBoolOf(!setting.OAuth2Client.RegisterEmailConfirm), } setUserGroupClaims(authSource, u, &gothUser) - if !createAndHandleCreatedUser(ctx, base.TplName(""), nil, u, &gothUser, setting.OAuth2Client.AccountLinking != setting.OAuth2AccountLinkingDisabled) { + if !createAndHandleCreatedUser(ctx, base.TplName(""), nil, u, overwriteDefault, &gothUser, setting.OAuth2Client.AccountLinking != setting.OAuth2AccountLinkingDisabled) { // error already handled return } diff --git a/routers/web/auth/openid.go b/routers/web/auth/openid.go index f3189887a..3012d8c5a 100644 --- a/routers/web/auth/openid.go +++ b/routers/web/auth/openid.go @@ -423,12 +423,11 @@ func RegisterOpenIDPost(ctx *context.Context) { } u := &user_model.User{ - Name: form.UserName, - Email: form.Email, - Passwd: password, - IsActive: !(setting.Service.RegisterEmailConfirm || setting.Service.RegisterManualConfirm), + Name: form.UserName, + Email: form.Email, + Passwd: password, } - if !createUserInContext(ctx, tplSignUpOID, form, u, nil, false) { + if !createUserInContext(ctx, tplSignUpOID, form, u, nil, nil, false) { // error already handled return } diff --git a/services/auth/reverseproxy.go b/services/auth/reverseproxy.go index 1b151f650..299d7abd3 100644 --- a/services/auth/reverseproxy.go +++ b/services/auth/reverseproxy.go @@ -12,6 +12,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/services/mailer" @@ -105,11 +106,15 @@ func (r *ReverseProxy) newUser(req *http.Request) *user_model.User { } user := &user_model.User{ - Name: username, - Email: email, - IsActive: true, + Name: username, + Email: email, } - if err := user_model.CreateUser(user); err != nil { + + overwriteDefault := user_model.CreateUserOverwriteOptions{ + IsActive: util.OptionalBoolTrue, + } + + if err := user_model.CreateUser(user, &overwriteDefault); err != nil { // FIXME: should I create a system notice? log.Error("CreateUser: %v", err) return nil diff --git a/services/auth/source/ldap/source_authenticate.go b/services/auth/source/ldap/source_authenticate.go index ddd70627e..d8d11f18e 100644 --- a/services/auth/source/ldap/source_authenticate.go +++ b/services/auth/source/ldap/source_authenticate.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/models/db" "code.gitea.io/gitea/models/organization" user_model "code.gitea.io/gitea/models/user" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/mailer" user_service "code.gitea.io/gitea/services/user" ) @@ -85,19 +86,21 @@ func (source *Source) Authenticate(user *user_model.User, userName, password str } user = &user_model.User{ - LowerName: strings.ToLower(sr.Username), - Name: sr.Username, - FullName: composeFullName(sr.Name, sr.Surname, sr.Username), - Email: sr.Mail, - LoginType: source.authSource.Type, - LoginSource: source.authSource.ID, - LoginName: userName, - IsActive: true, - IsAdmin: sr.IsAdmin, - IsRestricted: sr.IsRestricted, + LowerName: strings.ToLower(sr.Username), + Name: sr.Username, + FullName: composeFullName(sr.Name, sr.Surname, sr.Username), + Email: sr.Mail, + LoginType: source.authSource.Type, + LoginSource: source.authSource.ID, + LoginName: userName, + IsAdmin: sr.IsAdmin, + } + overwriteDefault := &user_model.CreateUserOverwriteOptions{ + IsRestricted: util.OptionalBoolOf(sr.IsRestricted), + IsActive: util.OptionalBoolTrue, } - err := user_model.CreateUser(user) + err := user_model.CreateUser(user, overwriteDefault) if err != nil { return user, err } diff --git a/services/auth/source/ldap/source_sync.go b/services/auth/source/ldap/source_sync.go index 65efed78c..a245f4c6f 100644 --- a/services/auth/source/ldap/source_sync.go +++ b/services/auth/source/ldap/source_sync.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/models/organization" user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" user_service "code.gitea.io/gitea/services/user" ) @@ -102,19 +103,21 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error { log.Trace("SyncExternalUsers[%s]: Creating user %s", source.authSource.Name, su.Username) usr = &user_model.User{ - LowerName: su.LowerName, - Name: su.Username, - FullName: fullName, - LoginType: source.authSource.Type, - LoginSource: source.authSource.ID, - LoginName: su.Username, - Email: su.Mail, - IsAdmin: su.IsAdmin, - IsRestricted: su.IsRestricted, - IsActive: true, + LowerName: su.LowerName, + Name: su.Username, + FullName: fullName, + LoginType: source.authSource.Type, + LoginSource: source.authSource.ID, + LoginName: su.Username, + Email: su.Mail, + IsAdmin: su.IsAdmin, + } + overwriteDefault := &user_model.CreateUserOverwriteOptions{ + IsRestricted: util.OptionalBoolOf(su.IsRestricted), + IsActive: util.OptionalBoolTrue, } - err = user_model.CreateUser(usr) + err = user_model.CreateUser(usr, overwriteDefault) if err != nil { log.Error("SyncExternalUsers[%s]: Error creating user %s: %v", source.authSource.Name, su.Username, err) diff --git a/services/auth/source/pam/source_authenticate.go b/services/auth/source/pam/source_authenticate.go index d5bd94099..16ddc0598 100644 --- a/services/auth/source/pam/source_authenticate.go +++ b/services/auth/source/pam/source_authenticate.go @@ -12,6 +12,7 @@ import ( user_model "code.gitea.io/gitea/models/user" "code.gitea.io/gitea/modules/auth/pam" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/services/mailer" "github.com/google/uuid" @@ -58,10 +59,12 @@ func (source *Source) Authenticate(user *user_model.User, userName, password str LoginType: auth.PAM, LoginSource: source.authSource.ID, LoginName: userName, // This is what the user typed in - IsActive: true, + } + overwriteDefault := &user_model.CreateUserOverwriteOptions{ + IsActive: util.OptionalBoolTrue, } - if err := user_model.CreateUser(user); err != nil { + if err := user_model.CreateUser(user, overwriteDefault); err != nil { return user, err } diff --git a/services/auth/source/smtp/source_authenticate.go b/services/auth/source/smtp/source_authenticate.go index 3be2f1128..dff24d494 100644 --- a/services/auth/source/smtp/source_authenticate.go +++ b/services/auth/source/smtp/source_authenticate.go @@ -74,10 +74,12 @@ func (source *Source) Authenticate(user *user_model.User, userName, password str LoginType: auth_model.SMTP, LoginSource: source.authSource.ID, LoginName: userName, - IsActive: true, + } + overwriteDefault := &user_model.CreateUserOverwriteOptions{ + IsActive: util.OptionalBoolTrue, } - if err := user_model.CreateUser(user); err != nil { + if err := user_model.CreateUser(user, overwriteDefault); err != nil { return user, err } diff --git a/services/auth/sspi_windows.go b/services/auth/sspi_windows.go index 63e70e61d..9bc4041a7 100644 --- a/services/auth/sspi_windows.go +++ b/services/auth/sspi_windows.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/templates" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/services/auth/source/sspi" "code.gitea.io/gitea/services/mailer" @@ -187,17 +188,20 @@ func (s *SSPI) shouldAuthenticate(req *http.Request) (shouldAuth bool) { func (s *SSPI) newUser(username string, cfg *sspi.Source) (*user_model.User, error) { email := gouuid.New().String() + "@localhost.localdomain" user := &user_model.User{ - Name: username, - Email: email, - KeepEmailPrivate: true, - Passwd: gouuid.New().String(), - IsActive: cfg.AutoActivateUsers, - Language: cfg.DefaultLanguage, - UseCustomAvatar: true, - Avatar: avatars.DefaultAvatarLink(), - EmailNotificationsPreference: user_model.EmailNotificationsDisabled, + Name: username, + Email: email, + Passwd: gouuid.New().String(), + Language: cfg.DefaultLanguage, + UseCustomAvatar: true, + Avatar: avatars.DefaultAvatarLink(), } - if err := user_model.CreateUser(user); err != nil { + emailNotificationPreference := user_model.EmailNotificationsDisabled + overwriteDefault := &user_model.CreateUserOverwriteOptions{ + IsActive: util.OptionalBoolOf(cfg.AutoActivateUsers), + KeepEmailPrivate: util.OptionalBoolTrue, + EmailNotificationsPreference: &emailNotificationPreference, + } + if err := user_model.CreateUser(user, overwriteDefault); err != nil { return nil, err } diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl index 3e4813f22..20e80967b 100644 --- a/templates/swagger/v1_json.tmpl +++ b/templates/swagger/v1_json.tmpl @@ -14471,6 +14471,10 @@ "type": "string", "x-go-name": "Password" }, + "restricted": { + "type": "boolean", + "x-go-name": "Restricted" + }, "send_notify": { "type": "boolean", "x-go-name": "SendNotify"