Fix last admin check when syncing users (#34649)

Fix #34358

routers/api/v1/admin/user.go

@@ -239,7 +239,7 @@ func EditUser(ctx *context.APIContext) {
 		Location:                optional.FromPtr(form.Location),
 		Description:             optional.FromPtr(form.Description),
 		IsActive:                optional.FromPtr(form.Active),
-		IsAdmin:                 optional.FromPtr(form.Admin),
+		IsAdmin:                 user_service.UpdateOptionFieldFromPtr(form.Admin),
 		Visibility:              optional.FromNonDefault(api.VisibilityModes[form.Visibility]),
 		AllowGitHook:            optional.FromPtr(form.AllowGitHook),
 		AllowImportLocal:        optional.FromPtr(form.AllowImportLocal),

routers/web/admin/users.go

@@ -432,7 +432,7 @@ func EditUserPost(ctx *context.Context) {
 		Website:                 optional.Some(form.Website),
 		Location:                optional.Some(form.Location),
 		IsActive:                optional.Some(form.Active),
-		IsAdmin:                 optional.Some(form.Admin),
+		IsAdmin:                 user_service.UpdateOptionFieldFromValue(form.Admin),
 		AllowGitHook:            optional.Some(form.AllowGitHook),
 		AllowImportLocal:        optional.Some(form.AllowImportLocal),
 		MaxRepoCreation:         optional.Some(form.MaxRepoCreation),

routers/web/auth/auth.go

@@ -613,7 +613,7 @@ func handleUserCreated(ctx *context.Context, u *user_model.User, gothUser *goth.
 	if user_model.CountUsers(ctx, nil) == 1 {
 		opts := &user_service.UpdateOptions{
 			IsActive:     optional.Some(true),
-			IsAdmin:      optional.Some(true),
+			IsAdmin:      user_service.UpdateOptionFieldFromValue(true),
 			SetLastLogin: true,
 		}
 		if err := user_service.UpdateUser(ctx, u, opts); err != nil {

routers/web/auth/oauth.go

@@ -193,8 +193,8 @@ func SignInOAuthCallback(ctx *context.Context) {
 			source := authSource.Cfg.(*oauth2.Source)
 
 			isAdmin, isRestricted := getUserAdminAndRestrictedFromGroupClaims(source, &gothUser)
-			u.IsAdmin = isAdmin.ValueOrDefault(false)
-			u.IsRestricted = isRestricted.ValueOrDefault(false)
+			u.IsAdmin = isAdmin.ValueOrDefault(user_service.UpdateOptionField[bool]{FieldValue: false}).FieldValue
+			u.IsRestricted = isRestricted.ValueOrDefault(setting.Service.DefaultUserIsRestricted)
 
 			if !createAndHandleCreatedUser(ctx, templates.TplName(""), nil, u, overwriteDefault, &gothUser, setting.OAuth2Client.AccountLinking != setting.OAuth2AccountLinkingDisabled) {
 				// error already handled
@@ -258,11 +258,11 @@ func getClaimedGroups(source *oauth2.Source, gothUser *goth.User) container.Set[
 	return claimValueToStringSet(groupClaims)
 }
 
-func getUserAdminAndRestrictedFromGroupClaims(source *oauth2.Source, gothUser *goth.User) (isAdmin, isRestricted optional.Option[bool]) {
+func getUserAdminAndRestrictedFromGroupClaims(source *oauth2.Source, gothUser *goth.User) (isAdmin optional.Option[user_service.UpdateOptionField[bool]], isRestricted optional.Option[bool]) {
 	groups := getClaimedGroups(source, gothUser)
 
 	if source.AdminGroup != "" {
-		isAdmin = optional.Some(groups.Contains(source.AdminGroup))
+		isAdmin = user_service.UpdateOptionFieldFromSync(groups.Contains(source.AdminGroup))
 	}
 	if source.RestrictedGroup != "" {
 		isRestricted = optional.Some(groups.Contains(source.RestrictedGroup))

services/auth/source/ldap/source_authenticate.go

@@ -58,7 +58,7 @@ func (source *Source) Authenticate(ctx context.Context, user *user_model.User, u
 			opts := &user_service.UpdateOptions{}
 			if source.AdminFilter != "" && user.IsAdmin != sr.IsAdmin {
 				// Change existing admin flag only if AdminFilter option is set
-				opts.IsAdmin = optional.Some(sr.IsAdmin)
+				opts.IsAdmin = user_service.UpdateOptionFieldFromSync(sr.IsAdmin)
 			}
 			if !sr.IsAdmin && source.RestrictedFilter != "" && user.IsRestricted != sr.IsRestricted {
 				// Change existing restricted flag only if RestrictedFilter option is set

services/auth/source/ldap/source_sync.go

@@ -162,7 +162,7 @@ func (source *Source) Sync(ctx context.Context, updateExisting bool) error {
 					IsActive: optional.Some(true),
 				}
 				if source.AdminFilter != "" {
-					opts.IsAdmin = optional.Some(su.IsAdmin)
+					opts.IsAdmin = user_service.UpdateOptionFieldFromSync(su.IsAdmin)
 				}
 				// Change existing restricted flag only if RestrictedFilter option is set
 				if !su.IsAdmin && source.RestrictedFilter != "" {

services/user/update.go

@@ -15,6 +15,26 @@ import (
 	"code.gitea.io/gitea/modules/structs"
 )
 
+type UpdateOptionField[T any] struct {
+	FieldValue T
+	FromSync   bool
+}
+
+func UpdateOptionFieldFromValue[T any](value T) optional.Option[UpdateOptionField[T]] {
+	return optional.Some(UpdateOptionField[T]{FieldValue: value})
+}
+
+func UpdateOptionFieldFromSync[T any](value T) optional.Option[UpdateOptionField[T]] {
+	return optional.Some(UpdateOptionField[T]{FieldValue: value, FromSync: true})
+}
+
+func UpdateOptionFieldFromPtr[T any](value *T) optional.Option[UpdateOptionField[T]] {
+	if value == nil {
+		return optional.None[UpdateOptionField[T]]()
+	}
+	return UpdateOptionFieldFromValue(*value)
+}
+
 type UpdateOptions struct {
 	KeepEmailPrivate             optional.Option[bool]
 	FullName                     optional.Option[string]
@@ -32,7 +52,7 @@ type UpdateOptions struct {
 	DiffViewStyle                optional.Option[string]
 	AllowCreateOrganization      optional.Option[bool]
 	IsActive                     optional.Option[bool]
-	IsAdmin                      optional.Option[bool]
+	IsAdmin                      optional.Option[UpdateOptionField[bool]]
 	EmailNotificationsPreference optional.Option[string]
 	SetLastLogin                 bool
 	RepoAdminChangeTeamAccess    optional.Option[bool]
@@ -111,13 +131,18 @@ func UpdateUser(ctx context.Context, u *user_model.User, opts *UpdateOptions) er
 		cols = append(cols, "is_restricted")
 	}
 	if opts.IsAdmin.Has() {
-		if !opts.IsAdmin.Value() && user_model.IsLastAdminUser(ctx, u) {
+		if opts.IsAdmin.Value().FieldValue /* true */ {
+			u.IsAdmin = opts.IsAdmin.Value().FieldValue // set IsAdmin=true
+			cols = append(cols, "is_admin")
+		} else if !user_model.IsLastAdminUser(ctx, u) /* not the last admin */ {
+			u.IsAdmin = opts.IsAdmin.Value().FieldValue // it's safe to change it from false to true (not the last admin)
+			cols = append(cols, "is_admin")
+		} else /* IsAdmin=false but this is the last admin user */ { //nolint
+			if !opts.IsAdmin.Value().FromSync {
 				return user_model.ErrDeleteLastAdminUser{UID: u.ID}
 			}
-
-		u.IsAdmin = opts.IsAdmin.Value()
-
-		cols = append(cols, "is_admin")
+			// else: syncing from external-source, this user is the last admin, so skip the "IsAdmin=false" change
+		}
 	}
 
 	if opts.Visibility.Has() {

services/user/update_test.go

@@ -22,7 +22,11 @@ func TestUpdateUser(t *testing.T) {
 	admin := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 1})
 
 	assert.Error(t, UpdateUser(db.DefaultContext, admin, &UpdateOptions{
-		IsAdmin: optional.Some(false),
+		IsAdmin: UpdateOptionFieldFromValue(false),
+	}))
+
+	assert.NoError(t, UpdateUser(db.DefaultContext, admin, &UpdateOptions{
+		IsAdmin: UpdateOptionFieldFromSync(false),
 	}))
 
 	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28})
@@ -38,7 +42,7 @@ func TestUpdateUser(t *testing.T) {
 		MaxRepoCreation:              optional.Some(10),
 		IsRestricted:                 optional.Some(true),
 		IsActive:                     optional.Some(false),
-		IsAdmin:                      optional.Some(true),
+		IsAdmin:                      UpdateOptionFieldFromValue(true),
 		Visibility:                   optional.Some(structs.VisibleTypePrivate),
 		KeepActivityPrivate:          optional.Some(true),
 		Language:                     optional.Some("lang"),
@@ -60,7 +64,7 @@ func TestUpdateUser(t *testing.T) {
 	assert.Equal(t, opts.MaxRepoCreation.Value(), user.MaxRepoCreation)
 	assert.Equal(t, opts.IsRestricted.Value(), user.IsRestricted)
 	assert.Equal(t, opts.IsActive.Value(), user.IsActive)
-	assert.Equal(t, opts.IsAdmin.Value(), user.IsAdmin)
+	assert.Equal(t, opts.IsAdmin.Value().FieldValue, user.IsAdmin)
 	assert.Equal(t, opts.Visibility.Value(), user.Visibility)
 	assert.Equal(t, opts.KeepActivityPrivate.Value(), user.KeepActivityPrivate)
 	assert.Equal(t, opts.Language.Value(), user.Language)
@@ -80,7 +84,7 @@ func TestUpdateUser(t *testing.T) {
 	assert.Equal(t, opts.MaxRepoCreation.Value(), user.MaxRepoCreation)
 	assert.Equal(t, opts.IsRestricted.Value(), user.IsRestricted)
 	assert.Equal(t, opts.IsActive.Value(), user.IsActive)
-	assert.Equal(t, opts.IsAdmin.Value(), user.IsAdmin)
+	assert.Equal(t, opts.IsAdmin.Value().FieldValue, user.IsAdmin)
 	assert.Equal(t, opts.Visibility.Value(), user.Visibility)
 	assert.Equal(t, opts.KeepActivityPrivate.Value(), user.KeepActivityPrivate)
 	assert.Equal(t, opts.Language.Value(), user.Language)