diff --git a/routers/api/v1/admin/user.go b/routers/api/v1/admin/user.go index 85dd547ac1..c29f4e4622 100644 --- a/routers/api/v1/admin/user.go +++ b/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), diff --git a/routers/web/admin/users.go b/routers/web/admin/users.go index 70fcc935b1..27577cd35b 100644 --- a/routers/web/admin/users.go +++ b/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), diff --git a/routers/web/auth/auth.go b/routers/web/auth/auth.go index 69b9d285b7..87edbc357b 100644 --- a/routers/web/auth/auth.go +++ b/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 { diff --git a/routers/web/auth/oauth.go b/routers/web/auth/oauth.go index 96c1dcf358..a13b987aab 100644 --- a/routers/web/auth/oauth.go +++ b/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)) diff --git a/services/auth/source/ldap/source_authenticate.go b/services/auth/source/ldap/source_authenticate.go index a2e8c2b86a..6005a4744a 100644 --- a/services/auth/source/ldap/source_authenticate.go +++ b/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 diff --git a/services/auth/source/ldap/source_sync.go b/services/auth/source/ldap/source_sync.go index 416b67dd46..7b401c5c96 100644 --- a/services/auth/source/ldap/source_sync.go +++ b/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 != "" { diff --git a/services/user/update.go b/services/user/update.go index 4a39f4f783..c4fd2d60fe 100644 --- a/services/user/update.go +++ b/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) { - return user_model.ErrDeleteLastAdminUser{UID: u.ID} + 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} + } + // else: syncing from external-source, this user is the last admin, so skip the "IsAdmin=false" change } - - u.IsAdmin = opts.IsAdmin.Value() - - cols = append(cols, "is_admin") } if opts.Visibility.Has() { diff --git a/services/user/update_test.go b/services/user/update_test.go index fc24a6c212..27513e8040 100644 --- a/services/user/update_test.go +++ b/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)