Forbid removing the last admin user (#28337) (#28793)

Backport #28337 by @yp05327 Co-authored-by: yp05327 <[email protected]> Co-authored-by: Lunny Xiao <[email protected]>
go-gitea · Jan 16, 2024 · 376fa0d · 376fa0d
1 parent be541d9
commit 376fa0d
Show file tree

Hide file tree

Showing 8 changed files with 80 additions and 7 deletions.
diff --git a/models/error.go b/models/error.go
@@ -57,6 +57,21 @@ func (err ErrUserOwnPackages) Error() string {
 	return fmt.Sprintf("user still has ownership of packages [uid: %d]", err.UID)
 }
 
+// ErrDeleteLastAdminUser represents a "DeleteLastAdminUser" kind of error.
+type ErrDeleteLastAdminUser struct {
+	UID int64
+}
+
+// IsErrDeleteLastAdminUser checks if an error is a ErrDeleteLastAdminUser.
+func IsErrDeleteLastAdminUser(err error) bool {
+	_, ok := err.(ErrDeleteLastAdminUser)
+	return ok
+}
+
+func (err ErrDeleteLastAdminUser) Error() string {
+	return fmt.Sprintf("can not delete the last admin user [uid: %d]", err.UID)
+}
+
 // ErrNoPendingRepoTransfer is an error type for repositories without a pending
 // transfer request
 type ErrNoPendingRepoTransfer struct {

diff --git a/models/user/user.go b/models/user/user.go
@@ -705,9 +705,18 @@ func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOve
 	return committer.Commit()
 }
 
+// IsLastAdminUser check whether user is the last admin
+func IsLastAdminUser(ctx context.Context, user *User) bool {
+	if user.IsAdmin && CountUsers(ctx, &CountUserFilter{IsAdmin: util.OptionalBoolTrue}) <= 1 {
+		return true
+	}
+	return false
+}
+
 // CountUserFilter represent optional filters for CountUsers
 type CountUserFilter struct {
 	LastLoginSince *int64
+	IsAdmin        util.OptionalBool
 }
 
 // CountUsers returns number of users.
@@ -716,13 +725,25 @@ func CountUsers(ctx context.Context, opts *CountUserFilter) int64 {
 }
 
 func countUsers(ctx context.Context, opts *CountUserFilter) int64 {
-	sess := db.GetEngine(ctx).Where(builder.Eq{"type": "0"})
+	sess := db.GetEngine(ctx)
+	cond := builder.NewCond()
+	cond = cond.And(builder.Eq{"type": UserTypeIndividual})
 
-	if opts != nil && opts.LastLoginSince != nil {
-		sess = sess.Where(builder.Gte{"last_login_unix": *opts.LastLoginSince})
+	if opts != nil {
+		if opts.LastLoginSince != nil {
+			cond = cond.And(builder.Gte{"last_login_unix": *opts.LastLoginSince})
+		}
+
+		if !opts.IsAdmin.IsNone() {
+			cond = cond.And(builder.Eq{"is_admin": opts.IsAdmin.IsTrue()})
+		}
+	}
+
+	count, err := sess.Where(cond).Count(new(User))
+	if err != nil {
+		log.Error("user.countUsers: %v", err)
 	}
 
-	count, _ := sess.Count(new(User))
 	return count
 }
 

diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini
@@ -422,6 +422,7 @@ authorization_failed_desc = The authorization failed because we detected an inva
 sspi_auth_failed = SSPI authentication failed
 password_pwned = The password you chose is on a <a target="_blank" rel="noopener noreferrer" href="https://haveibeenpwned.com/Passwords">list of stolen passwords</a> previously exposed in public data breaches. Please try again with a different password and consider changing this password elsewhere too.
 password_pwned_err = Could not complete request to HaveIBeenPwned
+last_admin = You cannot remove the last admin. There must be at least one admin.
 
 [mail]
 view_it_on = View it on %s
@@ -587,6 +588,8 @@ org_still_own_packages = "This organization still owns one or more packages, del
 
 target_branch_not_exist = Target branch does not exist.
 
+admin_cannot_delete_self = You cannot delete yourself when you are an admin. Please remove your admin privileges first.
+
 [user]
 change_avatar = Change your avatar…
 joined_on = Joined on %s

diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go
@@ -183,6 +183,8 @@ func EditUser(ctx *context.APIContext) {
 	// responses:
 	//   "200":
 	//     "$ref": "#/responses/User"
+	//   "400":
+	//     "$ref": "#/responses/error"
 	//   "403":
 	//     "$ref": "#/responses/forbidden"
 	//   "422":
@@ -264,6 +266,10 @@ func EditUser(ctx *context.APIContext) {
 		ctx.ContextUser.Visibility = api.VisibilityModes[form.Visibility]
 	}
 	if form.Admin != nil {
+		if !*form.Admin && user_model.IsLastAdminUser(ctx, ctx.ContextUser) {
+			ctx.Error(http.StatusBadRequest, "LastAdmin", ctx.Tr("auth.last_admin"))
+			return
+		}
 		ctx.ContextUser.IsAdmin = *form.Admin
 	}
 	if form.AllowGitHook != nil {
@@ -341,7 +347,8 @@ func DeleteUser(ctx *context.APIContext) {
 	if err := user_service.DeleteUser(ctx, ctx.ContextUser, ctx.FormBool("purge")); err != nil {
 		if models.IsErrUserOwnRepos(err) ||
 			models.IsErrUserHasOrgs(err) ||
-			models.IsErrUserOwnPackages(err) {
+			models.IsErrUserOwnPackages(err) ||
+			models.IsErrDeleteLastAdminUser(err) {
 			ctx.Error(http.StatusUnprocessableEntity, "", err)
 		} else {
 			ctx.Error(http.StatusInternalServerError, "DeleteUser", err)

diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go
@@ -429,6 +429,12 @@ func EditUserPost(ctx *context.Context) {
 
 	}
 
+	// Check whether user is the last admin
+	if !form.Admin && user_model.IsLastAdminUser(ctx, u) {
+		ctx.RenderWithErr(ctx.Tr("auth.last_admin"), tplUserEdit, &form)
+		return
+	}
+
 	u.LoginName = form.LoginName
 	u.FullName = form.FullName
 	emailChanged := !strings.EqualFold(u.Email, form.Email)
@@ -496,7 +502,10 @@ func DeleteUser(ctx *context.Context) {
 			ctx.Redirect(setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid")))
 		case models.IsErrUserOwnPackages(err):
 			ctx.Flash.Error(ctx.Tr("admin.users.still_own_packages"))
-			ctx.Redirect(setting.AppSubURL + "/admin/users/" + ctx.Params(":userid"))
+			ctx.Redirect(setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid")))
+		case models.IsErrDeleteLastAdminUser(err):
+			ctx.Flash.Error(ctx.Tr("auth.last_admin"))
+			ctx.Redirect(setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid")))
 		default:
 			ctx.ServerError("DeleteUser", err)
 		}

diff --git a/routers/web/user/setting/account.go b/routers/web/user/setting/account.go
@@ -246,6 +246,13 @@ func DeleteAccount(ctx *context.Context) {
 		return
 	}
 
+	// admin should not delete themself
+	if ctx.Doer.IsAdmin {
+		ctx.Flash.Error(ctx.Tr("form.admin_cannot_delete_self"))
+		ctx.Redirect(setting.AppSubURL + "/user/settings/account")
+		return
+	}
+
 	if err := user.DeleteUser(ctx, ctx.Doer, false); err != nil {
 		switch {
 		case models.IsErrUserOwnRepos(err):
@@ -257,6 +264,9 @@ func DeleteAccount(ctx *context.Context) {
 		case models.IsErrUserOwnPackages(err):
 			ctx.Flash.Error(ctx.Tr("form.still_own_packages"))
 			ctx.Redirect(setting.AppSubURL + "/user/settings/account")
+		case models.IsErrDeleteLastAdminUser(err):
+			ctx.Flash.Error(ctx.Tr("auth.last_admin"))
+			ctx.Redirect(setting.AppSubURL + "/user/settings/account")
 		default:
 			ctx.ServerError("DeleteUser", err)
 		}

diff --git a/services/user/user.go b/services/user/user.go
@@ -129,6 +129,10 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
 		return fmt.Errorf("%s is an organization not a user", u.Name)
 	}
 
+	if user_model.IsLastAdminUser(ctx, u) {
+		return models.ErrDeleteLastAdminUser{UID: u.ID}
+	}
+
 	if purge {
 		// Disable the user first
 		// NOTE: This is deliberately not within a transaction as it must disable the user immediately to prevent any further action by the user to be purged.
@@ -295,7 +299,8 @@ func DeleteInactiveUsers(ctx context.Context, olderThan time.Duration) error {
 		}
 		if err := DeleteUser(ctx, u, false); err != nil {
 			// Ignore users that were set inactive by admin.
-			if models.IsErrUserOwnRepos(err) || models.IsErrUserHasOrgs(err) || models.IsErrUserOwnPackages(err) {
+			if models.IsErrUserOwnRepos(err) || models.IsErrUserHasOrgs(err) ||
+				models.IsErrUserOwnPackages(err) || models.IsErrDeleteLastAdminUser(err) {
 				continue
 			}
 			return err

diff --git a/templates/swagger/v1_json.tmpl b/templates/swagger/v1_json.tmpl