From ea6490a5eb764f5c032c2cdc59a3a754a94481f6 Mon Sep 17 00:00:00 2001 From: Scott Bishel Date: Tue, 16 Jul 2024 11:39:47 -0600 Subject: [PATCH] MM-58847 Sanitize User (#27471) * add more fields to sanitizeInput on User * add test for user sanoitizeInput * add more fields * remove line, lint fix * additional fields and sanitize update * Update user_test.go * remove fields that are unnecessary to check * add check to test --------- Co-authored-by: Mattermost Build --- api/v4/source/users.yaml | 6 ++++ server/channels/api4/user_test.go | 31 ++++++++++++++++----- server/public/model/user.go | 3 ++ server/public/model/user_test.go | 46 +++++++++++++++++++++++++++++++ 4 files changed, 79 insertions(+), 7 deletions(-) diff --git a/api/v4/source/users.yaml b/api/v4/source/users.yaml index 0e1e4f2485d..18b215067be 100644 --- a/api/v4/source/users.yaml +++ b/api/v4/source/users.yaml @@ -142,6 +142,10 @@ type: string nickname: type: string + position: + type: string + timezone: + $ref: "#/components/schemas/Timezone" auth_data: description: Service-specific authentication data, such as email address. type: string @@ -911,6 +915,8 @@ type: string position: type: string + timezone: + $ref: "#/components/schemas/Timezone" props: type: object notify_props: diff --git a/server/channels/api4/user_test.go b/server/channels/api4/user_test.go index 07faec9935d..08e58e1f1ef 100644 --- a/server/channels/api4/user_test.go +++ b/server/channels/api4/user_test.go @@ -36,13 +36,27 @@ func TestCreateUser(t *testing.T) { defer th.TearDown() user := model.User{ - Email: th.GenerateTestEmail(), - Nickname: "Corey Hulen", - Password: "hello1", - Username: GenerateTestUsername(), - Roles: model.SystemAdminRoleId + " " + model.SystemUserRoleId, - EmailVerified: true, - DeleteAt: 1, + Id: model.NewId(), + Email: th.GenerateTestEmail(), + Nickname: "Corey Hulen", + Password: "hello1", + Username: GenerateTestUsername(), + } + _, resp, err := th.Client.CreateUser(context.Background(), &user) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + + user = model.User{ + Email: th.GenerateTestEmail(), + Nickname: "Corey Hulen", + Password: "hello1", + Username: GenerateTestUsername(), + Roles: model.SystemAdminRoleId + " " + model.SystemUserRoleId, + EmailVerified: true, + DeleteAt: 1, + CreateAt: 1, + UpdateAt: 1, + LastActivityAt: 1, } ruser, resp, err := th.Client.CreateUser(context.Background(), &user) @@ -56,6 +70,9 @@ func TestCreateUser(t *testing.T) { require.Equal(t, user.Nickname, ruser.Nickname, "nickname didn't match") require.Equal(t, model.SystemUserRoleId, ruser.Roles, "did not clear roles") require.Equal(t, int64(0), ruser.DeleteAt, "did not reset deleteAt") + require.NotEqual(t, user.UpdateAt, ruser.UpdateAt, "did not reset updateAt") + require.NotEqual(t, user.CreateAt, ruser.CreateAt, "did not reset createAt") + require.NotEqual(t, user.LastActivityAt, ruser.LastActivityAt, "did not reset LastActivityAt") CheckUserSanitization(t, ruser) diff --git a/server/public/model/user.go b/server/public/model/user.go index b1882e2bd56..7cb19d7ba5b 100644 --- a/server/public/model/user.go +++ b/server/public/model/user.go @@ -686,6 +686,8 @@ func (u *User) SanitizeInput(isAdmin bool) { u.EmailVerified = false } u.RemoteId = NewString("") + u.CreateAt = 0 + u.UpdateAt = 0 u.DeleteAt = 0 u.LastPasswordUpdate = 0 u.LastPictureUpdate = 0 @@ -693,6 +695,7 @@ func (u *User) SanitizeInput(isAdmin bool) { u.MfaActive = false u.MfaSecret = "" u.Email = strings.TrimSpace(u.Email) + u.LastActivityAt = 0 } func (u *User) ClearNonProfileFields(asAdmin bool) { diff --git a/server/public/model/user_test.go b/server/public/model/user_test.go index 0a8ad7faaa9..6cd0f11542d 100644 --- a/server/public/model/user_test.go +++ b/server/public/model/user_test.go @@ -311,6 +311,52 @@ func TestUserIsValid(t *testing.T) { require.True(t, HasExpectedUserIsValidError(appErr, "roles_limit", user.Id, user.Roles), "expected user is valid error: %s", appErr.Error()) } +func TestUserSanitizeInput(t *testing.T) { + user := User{} + user.CreateAt = GetMillis() + user.UpdateAt = GetMillis() + user.DeleteAt = GetMillis() + user.LastPasswordUpdate = GetMillis() + user.LastPictureUpdate = GetMillis() + + user.Username = "username" + user.Email = " user@example.com " + user.Nickname = "nickname" + user.FirstName = "firstname" + user.LastName = "lastname" + user.RemoteId = NewString(NewId()) + user.Position = "position" + user.Roles = "system_admin" + user.AuthData = NewString("authdata") + user.AuthService = "saml" + user.EmailVerified = true + user.FailedAttempts = 10 + user.LastActivityAt = GetMillis() + + user.SanitizeInput(false) + + // these fields should be reset + require.Equal(t, NewString(""), user.AuthData) + require.Equal(t, "", user.AuthService) + require.False(t, user.EmailVerified) + require.Equal(t, NewString(""), user.RemoteId) + require.Equal(t, int64(0), user.CreateAt) + require.Equal(t, int64(0), user.UpdateAt) + require.Equal(t, int64(0), user.DeleteAt) + require.Equal(t, int64(0), user.LastPasswordUpdate) + require.Equal(t, int64(0), user.LastPictureUpdate) + require.Equal(t, int64(0), user.LastActivityAt) + require.Equal(t, 0, user.FailedAttempts) + + // these fields should remain intact + require.Equal(t, "user@example.com", user.Email) + require.Equal(t, "username", user.Username) + require.Equal(t, "nickname", user.Nickname) + require.Equal(t, "firstname", user.FirstName) + require.Equal(t, "lastname", user.LastName) + require.Equal(t, "position", user.Position) +} + func HasExpectedUserIsValidError(err *AppError, fieldName, userId string, fieldValue any) bool { if err == nil { return false