Skip to content

Commit ce0225c

Browse files
yp05327lunny
andauthored
Forbid removing the last admin user (#28337)
Co-authored-by: Lunny Xiao <[email protected]>
1 parent b820019 commit ce0225c

File tree

8 files changed

+80
-7
lines changed

8 files changed

+80
-7
lines changed

models/error.go

+15
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,21 @@ func (err ErrUserOwnPackages) Error() string {
5757
return fmt.Sprintf("user still has ownership of packages [uid: %d]", err.UID)
5858
}
5959

60+
// ErrDeleteLastAdminUser represents a "DeleteLastAdminUser" kind of error.
61+
type ErrDeleteLastAdminUser struct {
62+
UID int64
63+
}
64+
65+
// IsErrDeleteLastAdminUser checks if an error is a ErrDeleteLastAdminUser.
66+
func IsErrDeleteLastAdminUser(err error) bool {
67+
_, ok := err.(ErrDeleteLastAdminUser)
68+
return ok
69+
}
70+
71+
func (err ErrDeleteLastAdminUser) Error() string {
72+
return fmt.Sprintf("can not delete the last admin user [uid: %d]", err.UID)
73+
}
74+
6075
// ErrNoPendingRepoTransfer is an error type for repositories without a pending
6176
// transfer request
6277
type ErrNoPendingRepoTransfer struct {

models/user/user.go

+25-4
Original file line numberDiff line numberDiff line change
@@ -730,9 +730,18 @@ func CreateUser(ctx context.Context, u *User, overwriteDefault ...*CreateUserOve
730730
return committer.Commit()
731731
}
732732

733+
// IsLastAdminUser check whether user is the last admin
734+
func IsLastAdminUser(ctx context.Context, user *User) bool {
735+
if user.IsAdmin && CountUsers(ctx, &CountUserFilter{IsAdmin: util.OptionalBoolTrue}) <= 1 {
736+
return true
737+
}
738+
return false
739+
}
740+
733741
// CountUserFilter represent optional filters for CountUsers
734742
type CountUserFilter struct {
735743
LastLoginSince *int64
744+
IsAdmin util.OptionalBool
736745
}
737746

738747
// CountUsers returns number of users.
@@ -741,13 +750,25 @@ func CountUsers(ctx context.Context, opts *CountUserFilter) int64 {
741750
}
742751

743752
func countUsers(ctx context.Context, opts *CountUserFilter) int64 {
744-
sess := db.GetEngine(ctx).Where(builder.Eq{"type": "0"})
753+
sess := db.GetEngine(ctx)
754+
cond := builder.NewCond()
755+
cond = cond.And(builder.Eq{"type": UserTypeIndividual})
745756

746-
if opts != nil && opts.LastLoginSince != nil {
747-
sess = sess.Where(builder.Gte{"last_login_unix": *opts.LastLoginSince})
757+
if opts != nil {
758+
if opts.LastLoginSince != nil {
759+
cond = cond.And(builder.Gte{"last_login_unix": *opts.LastLoginSince})
760+
}
761+
762+
if !opts.IsAdmin.IsNone() {
763+
cond = cond.And(builder.Eq{"is_admin": opts.IsAdmin.IsTrue()})
764+
}
765+
}
766+
767+
count, err := sess.Where(cond).Count(new(User))
768+
if err != nil {
769+
log.Error("user.countUsers: %v", err)
748770
}
749771

750-
count, _ := sess.Count(new(User))
751772
return count
752773
}
753774

options/locale/locale_en-US.ini

+3
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,7 @@ authorization_failed_desc = The authorization failed because we detected an inva
423423
sspi_auth_failed = SSPI authentication failed
424424
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.
425425
password_pwned_err = Could not complete request to HaveIBeenPwned
426+
last_admin = You cannot remove the last admin. There must be at least one admin.
426427
427428
[mail]
428429
view_it_on = View it on %s
@@ -588,6 +589,8 @@ org_still_own_packages = "This organization still owns one or more packages, del
588589
589590
target_branch_not_exist = Target branch does not exist.
590591
592+
admin_cannot_delete_self = You cannot delete yourself when you are an admin. Please remove your admin privileges first.
593+
591594
[user]
592595
change_avatar = Change your avatar…
593596
joined_on = Joined on %s

routers/api/v1/admin/user.go

+8-1
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,8 @@ func EditUser(ctx *context.APIContext) {
183183
// responses:
184184
// "200":
185185
// "$ref": "#/responses/User"
186+
// "400":
187+
// "$ref": "#/responses/error"
186188
// "403":
187189
// "$ref": "#/responses/forbidden"
188190
// "422":
@@ -264,6 +266,10 @@ func EditUser(ctx *context.APIContext) {
264266
ctx.ContextUser.Visibility = api.VisibilityModes[form.Visibility]
265267
}
266268
if form.Admin != nil {
269+
if !*form.Admin && user_model.IsLastAdminUser(ctx, ctx.ContextUser) {
270+
ctx.Error(http.StatusBadRequest, "LastAdmin", ctx.Tr("auth.last_admin"))
271+
return
272+
}
267273
ctx.ContextUser.IsAdmin = *form.Admin
268274
}
269275
if form.AllowGitHook != nil {
@@ -341,7 +347,8 @@ func DeleteUser(ctx *context.APIContext) {
341347
if err := user_service.DeleteUser(ctx, ctx.ContextUser, ctx.FormBool("purge")); err != nil {
342348
if models.IsErrUserOwnRepos(err) ||
343349
models.IsErrUserHasOrgs(err) ||
344-
models.IsErrUserOwnPackages(err) {
350+
models.IsErrUserOwnPackages(err) ||
351+
models.IsErrDeleteLastAdminUser(err) {
345352
ctx.Error(http.StatusUnprocessableEntity, "", err)
346353
} else {
347354
ctx.Error(http.StatusInternalServerError, "DeleteUser", err)

routers/web/admin/users.go

+10-1
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,12 @@ func EditUserPost(ctx *context.Context) {
436436

437437
}
438438

439+
// Check whether user is the last admin
440+
if !form.Admin && user_model.IsLastAdminUser(ctx, u) {
441+
ctx.RenderWithErr(ctx.Tr("auth.last_admin"), tplUserEdit, &form)
442+
return
443+
}
444+
439445
u.LoginName = form.LoginName
440446
u.FullName = form.FullName
441447
emailChanged := !strings.EqualFold(u.Email, form.Email)
@@ -503,7 +509,10 @@ func DeleteUser(ctx *context.Context) {
503509
ctx.Redirect(setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid")))
504510
case models.IsErrUserOwnPackages(err):
505511
ctx.Flash.Error(ctx.Tr("admin.users.still_own_packages"))
506-
ctx.Redirect(setting.AppSubURL + "/admin/users/" + ctx.Params(":userid"))
512+
ctx.Redirect(setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid")))
513+
case models.IsErrDeleteLastAdminUser(err):
514+
ctx.Flash.Error(ctx.Tr("auth.last_admin"))
515+
ctx.Redirect(setting.AppSubURL + "/admin/users/" + url.PathEscape(ctx.Params(":userid")))
507516
default:
508517
ctx.ServerError("DeleteUser", err)
509518
}

routers/web/user/setting/account.go

+10
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,13 @@ func DeleteAccount(ctx *context.Context) {
244244
return
245245
}
246246

247+
// admin should not delete themself
248+
if ctx.Doer.IsAdmin {
249+
ctx.Flash.Error(ctx.Tr("form.admin_cannot_delete_self"))
250+
ctx.Redirect(setting.AppSubURL + "/user/settings/account")
251+
return
252+
}
253+
247254
if err := user.DeleteUser(ctx, ctx.Doer, false); err != nil {
248255
switch {
249256
case models.IsErrUserOwnRepos(err):
@@ -255,6 +262,9 @@ func DeleteAccount(ctx *context.Context) {
255262
case models.IsErrUserOwnPackages(err):
256263
ctx.Flash.Error(ctx.Tr("form.still_own_packages"))
257264
ctx.Redirect(setting.AppSubURL + "/user/settings/account")
265+
case models.IsErrDeleteLastAdminUser(err):
266+
ctx.Flash.Error(ctx.Tr("auth.last_admin"))
267+
ctx.Redirect(setting.AppSubURL + "/user/settings/account")
258268
default:
259269
ctx.ServerError("DeleteUser", err)
260270
}

services/user/user.go

+6-1
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,10 @@ func DeleteUser(ctx context.Context, u *user_model.User, purge bool) error {
129129
return fmt.Errorf("%s is an organization not a user", u.Name)
130130
}
131131

132+
if user_model.IsLastAdminUser(ctx, u) {
133+
return models.ErrDeleteLastAdminUser{UID: u.ID}
134+
}
135+
132136
if purge {
133137
// Disable the user first
134138
// 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 {
295299
}
296300
if err := DeleteUser(ctx, u, false); err != nil {
297301
// Ignore users that were set inactive by admin.
298-
if models.IsErrUserOwnRepos(err) || models.IsErrUserHasOrgs(err) || models.IsErrUserOwnPackages(err) {
302+
if models.IsErrUserOwnRepos(err) || models.IsErrUserHasOrgs(err) ||
303+
models.IsErrUserOwnPackages(err) || models.IsErrDeleteLastAdminUser(err) {
299304
continue
300305
}
301306
return err

templates/swagger/v1_json.tmpl

+3
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)