fix: 500 error when updating user visibility (#38480)

## How to reproduce this bug

1. Create a user with `visibility=public`
2. Update `ALLOWED_USER_VISIBILITY_MODES` setting to `limited, private`
3. Modify any field other than "User visibility" (e.g. "Full Name")
4. UI shows 500 error

## Fix

Only update the visibility field when it actually changes.
This commit is contained in:
Zettat123
2026-07-16 01:53:43 -06:00
committed by GitHub
parent 11363e2f0c
commit a77bf48b41
2 changed files with 34 additions and 1 deletions

View File

@@ -145,7 +145,8 @@ func UpdateUser(ctx context.Context, u *user_model.User, opts *UpdateOptions) er
}
}
if opts.Visibility.Has() {
// only validate and persist the visibility when it actually changes
if opts.Visibility.Has() && opts.Visibility.Value() != u.Visibility {
if !u.IsOrganization() && !setting.Service.AllowedUserVisibilityModesSlice.IsAllowedVisibility(opts.Visibility.Value()) {
return fmt.Errorf("visibility mode not allowed: %s", opts.Visibility.Value().String())
}

View File

@@ -10,7 +10,9 @@ import (
user_model "gitea.dev/models/user"
password_module "gitea.dev/modules/auth/password"
"gitea.dev/modules/optional"
"gitea.dev/modules/setting"
"gitea.dev/modules/structs"
"gitea.dev/modules/test"
"github.com/stretchr/testify/assert"
)
@@ -121,3 +123,33 @@ func TestUpdateAuth(t *testing.T) {
Password: optional.Some("aaaa"),
}), password_module.ErrMinLength)
}
func TestUpdateUserVisibility(t *testing.T) {
assert.NoError(t, unittest.PrepareTestDatabase())
// user28's current visibility is public, e.g. an account created before public was disallowed
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 28})
assert.Equal(t, structs.VisibleTypePublic, user.Visibility)
// public is no longer an allowed visibility mode, e.g. ALLOWED_USER_VISIBILITY_MODES = limited, private
defer test.MockVariableValue(&setting.Service.AllowedUserVisibilityModesSlice, setting.AllowedVisibility{false, true, true})()
// re-submitting the unchanged (now-disallowed) visibility must not fail the whole update
assert.NoError(t, UpdateUser(t.Context(), user, &UpdateOptions{
FullName: optional.Some("Changed Name"),
Visibility: optional.Some(structs.VisibleTypePublic),
}))
assert.Equal(t, "Changed Name", user.FullName)
assert.Equal(t, structs.VisibleTypePublic, user.Visibility)
// changing to an allowed visibility still works
assert.NoError(t, UpdateUser(t.Context(), user, &UpdateOptions{
Visibility: optional.Some(structs.VisibleTypePrivate),
}))
assert.Equal(t, structs.VisibleTypePrivate, user.Visibility)
// genuinely changing to a disallowed visibility is still rejected
assert.Error(t, UpdateUser(t.Context(), user, &UpdateOptions{
Visibility: optional.Some(structs.VisibleTypePublic),
}))
}