hanzoai/team
2
0
Fork 0
mirror of https://github.com/hanzoai/team.git synced 2026-09-30 01:57:35 +00:00
Code Issues Releases Activity

Revert "Strip remote_id field from user patch API requests (#35910)" (#35996)

Browse Source 
This reverts commit d5fbd8bfae which introduced a CI regression.
This commit is contained in:
Pavel Zeman
2026-04-09 05:06:42 -04:00
committed by GitHub
parent e1b4546867
commit c66f4ec153
12 changed files with 33 additions and 143 deletions
Download Patch File Download Diff File Expand all files Collapse all files
server/channels/api4/apitestlib.go
-10
View File
@@ -1409,16 +1409,6 @@ func (th *TestHelper) SetupScheme(tb testing.TB, scope string) *model.Scheme {
return scheme
}
func (th *TestHelper) SetUserRemoteID(tb testing.TB, userID, remoteID string) *model.User {
tb.Helper()
err := testlib.SetUserRemoteID(mainHelper.GetSQLStore(), userID, remoteID)
require.NoError(tb, err)
th.App.InvalidateCacheForUser(userID)
user, appErr := th.App.GetUser(userID)
require.Nil(tb, appErr)
return user
}
func (th *TestHelper) Parallel(t *testing.T) {
mainHelper.Parallel(t)
}
server/channels/api4/shared_channel_test.go
+14 -4
View File
@@ -221,7 +221,10 @@ func TestCreateDirectChannelWithRemoteUser(t *testing.T) {
}()
localUser := th.BasicUser
remoteUser := th.SetUserRemoteID(t, th.CreateUser(t).Id, model.NewId())
remoteUser := th.CreateUser(t)
remoteUser.RemoteId = model.NewPointer(model.NewId())
remoteUser, appErr := th.App.UpdateUser(th.Context, remoteUser, false)
require.Nil(t, appErr)
dm, _, err := client.CreateDirectChannel(context.Background(), localUser.Id, remoteUser.Id)
require.Error(t, err)
@@ -240,7 +243,10 @@ func TestCreateDirectChannelWithRemoteUser(t *testing.T) {
}()
localUser := th.BasicUser
remoteUser := th.SetUserRemoteID(t, th.CreateUser(t).Id, model.NewId())
remoteUser := th.CreateUser(t)
remoteUser.RemoteId = model.NewPointer(model.NewId())
remoteUser, appErr := th.App.UpdateUser(th.Context, remoteUser, false)
require.Nil(t, appErr)
dm, _, err := client.CreateDirectChannel(context.Background(), localUser.Id, remoteUser.Id)
require.NoError(t, err)
@@ -272,7 +278,9 @@ func TestCreateDirectChannelWithRemoteUser(t *testing.T) {
rc, appErr := th.App.AddRemoteCluster(rc)
require.Nil(t, appErr)
th.SetUserRemoteID(t, remoteUser.Id, rc.RemoteId)
remoteUser.RemoteId = model.NewPointer(rc.RemoteId)
remoteUser, appErr = th.App.UpdateUser(th.Context, remoteUser, false)
require.Nil(t, appErr)
dm, _, err := client.CreateDirectChannel(context.Background(), localUser.Id, remoteUser.Id)
require.NoError(t, err)
@@ -304,7 +312,9 @@ func TestCreateDirectChannelWithRemoteUser(t *testing.T) {
rc, appErr := th.App.AddRemoteCluster(rc)
require.Nil(t, appErr)
th.SetUserRemoteID(t, remoteUser.Id, rc.RemoteId)
remoteUser.RemoteId = model.NewPointer(rc.RemoteId)
remoteUser, appErr = th.App.UpdateUser(th.Context, remoteUser, false)
require.Nil(t, appErr)
dm, _, err := client.CreateDirectChannel(context.Background(), remoteUser.Id, localUser.Id)
require.NoError(t, err)
server/channels/api4/user.go
-2
View File
@@ -1450,8 +1450,6 @@ func patchUser(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
patch.RemoteId = nil
auditRec := c.MakeAuditRecord(model.AuditEventPatchUser, model.AuditStatusFail)
model.AddEventParameterAuditableToAuditRec(auditRec, "user_patch", &patch)
defer c.LogAuditRec(auditRec)
server/channels/api4/user_test.go
-98
View File
@@ -2316,50 +2316,6 @@ func TestUpdateUser(t *testing.T) {
})
}
func TestUpdateUserRemoteIdIgnored(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t)
t.Run("remote_id in update body is ignored for regular user", func(t *testing.T) {
user := th.CreateUser(t)
_, _, err := th.Client.Login(context.Background(), user.Email, user.Password)
require.NoError(t, err)
user.RemoteId = model.NewPointer("attacker-remote-id")
user.Nickname = "updated-nickname"
ruser, _, err := th.Client.UpdateUser(context.Background(), user)
require.NoError(t, err)
require.Equal(t, "updated-nickname", ruser.Nickname)
require.Empty(t, model.SafeDereference(ruser.RemoteId), "remote_id should remain empty")
dbUser, appErr := th.App.GetUser(user.Id)
require.Nil(t, appErr)
require.Empty(t, model.SafeDereference(dbUser.RemoteId), "remote_id should not be persisted")
})
t.Run("remote_id in update body is ignored for system admin", func(t *testing.T) {
user := th.CreateUser(t)
user.RemoteId = model.NewPointer("admin-remote-id")
user.Nickname = "admin-updated"
ruser, _, err := th.SystemAdminClient.UpdateUser(context.Background(), user)
require.NoError(t, err)
require.Equal(t, "admin-updated", ruser.Nickname)
require.Empty(t, model.SafeDereference(ruser.RemoteId), "remote_id should remain empty even for admin")
})
t.Run("existing remote_id is preserved when updating other fields", func(t *testing.T) {
remoteId := model.NewId()
user := th.SetUserRemoteID(t, th.CreateUser(t).Id, remoteId)
user.Nickname = "updated-nickname"
ruser, _, err := th.SystemAdminClient.UpdateUser(context.Background(), user)
require.NoError(t, err)
require.Equal(t, "updated-nickname", ruser.Nickname)
require.Equal(t, remoteId, model.SafeDereference(ruser.RemoteId), "existing remote_id should be preserved")
})
}
func TestUpdateAdminUser(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic(t)
@@ -2518,60 +2474,6 @@ func TestPatchUser(t *testing.T) {
require.NoError(t, err)
}
func TestPatchUserRemoteIdIgnored(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic(t)
t.Run("remote_id in patch is ignored for regular user", func(t *testing.T) {
user := th.CreateUser(t)
_, _, err := th.Client.Login(context.Background(), user.Email, user.Password)
require.NoError(t, err)
patch := &model.UserPatch{
RemoteId: model.NewPointer("attacker-remote-id"),
Nickname: model.NewPointer("new-nickname"),
}
ruser, _, err := th.Client.PatchUser(context.Background(), user.Id, patch)
require.NoError(t, err)
require.Equal(t, "new-nickname", ruser.Nickname)
require.Empty(t, model.SafeDereference(ruser.RemoteId), "remote_id should remain empty")
dbUser, appErr := th.App.GetUser(user.Id)
require.Nil(t, appErr)
require.Empty(t, model.SafeDereference(dbUser.RemoteId), "remote_id should not be persisted")
})
t.Run("remote_id in patch is ignored for system admin", func(t *testing.T) {
user := th.CreateUser(t)
patch := &model.UserPatch{
RemoteId: model.NewPointer("admin-remote-id"),
Nickname: model.NewPointer("admin-patched"),
}
ruser, _, err := th.SystemAdminClient.PatchUser(context.Background(), user.Id, patch)
require.NoError(t, err)
require.Equal(t, "admin-patched", ruser.Nickname)
require.Empty(t, model.SafeDereference(ruser.RemoteId), "remote_id should remain empty even for admin")
dbUser, appErr := th.App.GetUser(user.Id)
require.Nil(t, appErr)
require.Empty(t, model.SafeDereference(dbUser.RemoteId), "remote_id should not be persisted even for admin")
})
t.Run("existing remote_id is preserved when patching other fields", func(t *testing.T) {
remoteId := model.NewId()
user := th.SetUserRemoteID(t, th.CreateUser(t).Id, remoteId)
patch := &model.UserPatch{
Nickname: model.NewPointer("updated-nickname"),
}
ruser, _, err := th.SystemAdminClient.PatchUser(context.Background(), user.Id, patch)
require.NoError(t, err)
require.Equal(t, "updated-nickname", ruser.Nickname)
require.Equal(t, remoteId, model.SafeDereference(ruser.RemoteId), "existing remote_id should be preserved")
})
}
func TestPatchBotUser(t *testing.T) {
mainHelper.Parallel(t)
th := Setup(t).InitBasic(t)
server/channels/app/channel_test.go
+6 -2
View File
@@ -599,7 +599,9 @@ func TestGetOrCreateDirectChannel(t *testing.T) {
})
t.Run("Cannot create with a remote user", func(t *testing.T) {
th.SetUserRemoteID(t, user2.Id, model.NewId())
user2.RemoteId = model.NewPointer(model.NewId())
_, appErr := th.App.UpdateUser(th.Context, user2, false)
require.Nil(t, appErr)
dm, appErr := th.App.GetOrCreateDirectChannel(th.Context, user1.Id, user2.Id)
require.Nil(t, dm)
@@ -620,7 +622,9 @@ func TestCreateGroupChannel(t *testing.T) {
groupUserIds = append(groupUserIds, th.BasicUser.Id)
t.Run("Should not allow to create a group with a remote user", func(t *testing.T) {
th.SetUserRemoteID(t, user2.Id, model.NewId())
user2.RemoteId = model.NewPointer(model.NewId())
_, appErr := th.App.UpdateUser(th.Context, user2, false)
require.Nil(t, appErr)
dm, appErr := th.App.CreateGroupChannel(th.Context, groupUserIds, th.BasicUser.Id)
require.NotNil(t, appErr)
server/channels/app/helper_test.go
-10
View File
@@ -814,16 +814,6 @@ func decodeJSON[T any](tb testing.TB, o any, result *T) *T {
return result
}
func (th *TestHelper) SetUserRemoteID(tb testing.TB, userID, remoteID string) *model.User {
tb.Helper()
err := testlib.SetUserRemoteID(th.GetSqlStore(), userID, remoteID)
require.NoError(tb, err)
th.App.InvalidateCacheForUser(userID)
user, appErr := th.App.GetUser(userID)
require.Nil(tb, appErr)
return user
}
func (th *TestHelper) Parallel(t *testing.T) {
mainHelper.Parallel(t)
}
server/channels/app/shared_channel_global_user_sync_self_referential_test.go
+1 -1
View File
@@ -227,7 +227,7 @@ func TestSharedChannelGlobalUserSyncSelfReferential(t *testing.T) {
// Create remote user (should NOT be synced)
remoteUser := th.CreateUser(t)
remoteUser = th.SetUserRemoteID(t, remoteUser.Id, selfCluster.RemoteId)
remoteUser.RemoteId = &selfCluster.RemoteId
remoteUser.UpdateAt = baseTime + 600
_, err = ss.User().Update(th.Context, remoteUser, true)
require.NoError(t, err)
server/channels/app/shared_channel_membership_sync_self_referential_test.go
+3 -1
View File
@@ -984,7 +984,9 @@ func TestSharedChannelMembershipSyncSelfReferential(t *testing.T) {
// Create a remote user belonging to cluster-2
userFromCluster2 := th.CreateUser(t)
userFromCluster2 = th.SetUserRemoteID(t, userFromCluster2.Id, clusters[1].RemoteId)
userFromCluster2.RemoteId = &clusters[1].RemoteId
userFromCluster2, appErr = th.App.UpdateUser(th.Context, userFromCluster2, false)
require.Nil(t, appErr)
_, _, appErr = th.App.AddUserToTeam(th.Context, team.Id, userFromCluster2.Id, th.BasicUser.Id)
require.Nil(t, appErr)
server/channels/app/shared_channel_test.go
+5 -6
View File
@@ -598,13 +598,12 @@ func TestTransformMentionsOnReceive(t *testing.T) {
// Helper to create test users
createUser := func(username string, remoteId *string) *model.User {
user := th.CreateUser(t)
if remoteId != nil {
user = th.SetUserRemoteID(t, user.Id, *remoteId)
}
user.Username = username
var appErr *model.AppError
user, appErr = th.App.UpdateUser(th.Context, user, false)
require.Nil(t, appErr)
if remoteId != nil {
user.RemoteId = remoteId
}
user, updateErr := th.App.UpdateUser(th.Context, user, false)
require.Nil(t, updateErr)
th.LinkUserToTeam(t, user, th.BasicTeam)
th.AddUserToChannel(t, user, sharedChannel)
return user
server/channels/app/user_test.go
+4 -1
View File
@@ -2602,7 +2602,10 @@ func createTestRemoteCluster(t *testing.T, th *TestHelper, ss store.Store, name,
func createRemoteUser(t *testing.T, th *TestHelper, remoteCluster *model.RemoteCluster) *model.User {
user := th.CreateUser(t)
return th.SetUserRemoteID(t, user.Id, remoteCluster.RemoteId)
user.RemoteId = &remoteCluster.RemoteId
updatedUser, appErr := th.App.UpdateUser(th.Context, user, false)
require.Nil(t, appErr)
return updatedUser
}
func ensureRemoteClusterConnected(t *testing.T, ss store.Store, cluster *model.RemoteCluster, connected bool) {
server/channels/store/sqlstore/user_store.go
-1
View File
@@ -287,7 +287,6 @@ func (us SqlUserStore) Update(rctx request.CTX, user *model.User, trustedUpdateD
user.MfaActive = oldUser.MfaActive
user.MfaUsedTimestamps = oldUser.MfaUsedTimestamps
user.LastLogin = oldUser.LastLogin
user.RemoteId = oldUser.RemoteId
if !trustedUpdateData {
user.Roles = oldUser.Roles
server/channels/testlib/helper.go
-7
View File
@@ -342,13 +342,6 @@ func (h *MainHelper) GetSQLStore() *sqlstore.SqlStore {
return h.SQLStore
}
// SetUserRemoteID sets the RemoteId on a user via direct SQL, bypassing the
// store's Update method which preserves RemoteId from the existing row.
func SetUserRemoteID(sqlStore *sqlstore.SqlStore, userID, remoteID string) error {
_, err := sqlStore.GetMaster().Exec("UPDATE Users SET RemoteId = ? WHERE Id = ?", remoteID, userID)
return err
}
func (h *MainHelper) GetClusterInterface() *FakeClusterInterface {
if h.ClusterInterface == nil {
panic("MainHelper not initialized with cluster interface.")