Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forbid removing the last admin user #28337

Merged
merged 12 commits into from
Jan 15, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions models/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
29 changes: 25 additions & 4 deletions models/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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})
yp05327 marked this conversation as resolved.
Show resolved Hide resolved
}
}

count, err := sess.Where(cond).Count(new(User))
if err != nil {
log.Error("user.countUsers: %v", err)
}

count, _ := sess.Count(new(User))
return count
}

Expand Down
3 changes: 3 additions & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,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
Expand Down Expand Up @@ -588,6 +589,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
Expand Down
9 changes: 8 additions & 1 deletion routers/api/v1/admin/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ func EditUser(ctx *context.APIContext) {
// responses:
// "200":
// "$ref": "#/responses/User"
// "400":
// "$ref": "#/responses/error"
// "403":
// "$ref": "#/responses/forbidden"
// "422":
Expand Down Expand Up @@ -254,6 +256,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 {
Expand Down Expand Up @@ -331,7 +337,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)
Expand Down
11 changes: 10 additions & 1 deletion routers/web/admin/users.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,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)
Expand Down Expand Up @@ -503,7 +509,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)
}
Expand Down
10 changes: 10 additions & 0 deletions routers/web/user/setting/account.go
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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)
}
Expand Down
7 changes: 6 additions & 1 deletion services/user/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions templates/swagger/v1_json.tmpl

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.