Respect DefaultUserIsRestricted system default when creating new user (go-gitea#19310)

jpraet · Gusted · commit 0f9f73d516bb · 2022-05-01T00:33:29.000+02:00
* 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 ee95d3e.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,17 +523,17 @@ 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)
 }
 
 // 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/services/auth/source/pam/source_authenticate.go b/services/auth/source/pam/source_authenticate.go
diff --git a/services/auth/source/smtp/source_authenticate.go b/services/auth/source/smtp/source_authenticate.go
diff --git a/services/auth/sspi_windows.go b/services/auth/sspi_windows.go
diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl

Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,7 @@ type CreateUserOption struct {`
`19`	`19`	Password string `json:"password" binding:"Required;MaxSize(255)"`
`20`	`20`	MustChangePassword *bool `json:"must_change_password"`
`21`	`21`	SendNotify bool `json:"send_notify"`
	`22`	+ Restricted *bool `json:"restricted"`
`22`	`23`	Visibility string `json:"visibility" binding:"In(,public,limited,private)"`
`23`	`24`	`}`
`24`	`25`
Original file line number	Diff line number	Diff line change
`@@ -283,13 +283,12 @@ func LinkAccountPostRegister(ctx *context.Context) {`
`283`	`283`	`Name: form.UserName,`
`284`	`284`	`Email: form.Email,`
`285`	`285`	`Passwd: form.Password,`
`286`		`- IsActive: !(setting.Service.RegisterEmailConfirm \|\| setting.Service.RegisterManualConfirm),`
`287`	`286`	`LoginType: auth.OAuth2,`
`288`	`287`	`LoginSource: authSource.ID,`
`289`	`288`	`LoginName: gothUser.UserID,`
`290`	`289`	`}`
`291`	`290`
`292`		`- if !createAndHandleCreatedUser(ctx, tplLinkAccount, form, u, &gothUser, false) {`
	`291`	`+ if !createAndHandleCreatedUser(ctx, tplLinkAccount, form, u, nil, &gothUser, false) {`
`293`	`292`	`// error already handled`
`294`	`293`	`return`
`295`	`294`	`}`
Original file line number	Diff line number	Diff line change
`@@ -423,12 +423,11 @@ func RegisterOpenIDPost(ctx *context.Context) {`
`423`	`423`	`}`
`424`	`424`
`425`	`425`	`u := &user_model.User{`
`426`		`- Name: form.UserName,`
`427`		`- Email: form.Email,`
`428`		`- Passwd: password,`
`429`		`- IsActive: !(setting.Service.RegisterEmailConfirm \|\| setting.Service.RegisterManualConfirm),`
	`426`	`+ Name: form.UserName,`
	`427`	`+ Email: form.Email,`
	`428`	`+ Passwd: password,`
`430`	`429`	`}`
`431`		`- if !createUserInContext(ctx, tplSignUpOID, form, u, nil, false) {`
	`430`	`+ if !createUserInContext(ctx, tplSignUpOID, form, u, nil, nil, false) {`
`432`	`431`	`// error already handled`
`433`	`432`	`return`
`434`	`433`	`}`