MM-40690: Populate the error message with email address (#21491)

When logging via openID, if the user email is invalid,
we don't log the invalid email address. This makes
debugging difficult.

https://mattermost.atlassian.net/browse/MM-40690

```release-note
NONE
```
Этот коммит содержится в:
Agniva De Sarker
2022-10-25 14:58:13 +05:30
коммит произвёл GitHub
родитель 53b396d325
Коммит b62ee905ae
2 изменённых файлов: 33 добавлений и 32 удалений

Просмотреть файл

@@ -318,89 +318,90 @@ func (u *User) DeepCopy() *User {
// correctly. // correctly.
func (u *User) IsValid() *AppError { func (u *User) IsValid() *AppError {
if !IsValidId(u.Id) { if !IsValidId(u.Id) {
return InvalidUserError("id", "") return InvalidUserError("id", "", u.Id)
} }
if u.CreateAt == 0 { if u.CreateAt == 0 {
return InvalidUserError("create_at", u.Id) return InvalidUserError("create_at", u.Id, u.CreateAt)
} }
if u.UpdateAt == 0 { if u.UpdateAt == 0 {
return InvalidUserError("update_at", u.Id) return InvalidUserError("update_at", u.Id, u.UpdateAt)
} }
if u.IsRemote() { if u.IsRemote() {
if !IsValidUsernameAllowRemote(u.Username) { if !IsValidUsernameAllowRemote(u.Username) {
return InvalidUserError("username", u.Id) return InvalidUserError("username", u.Id, u.Username)
} }
} else { } else {
if !IsValidUsername(u.Username) { if !IsValidUsername(u.Username) {
return InvalidUserError("username", u.Id) return InvalidUserError("username", u.Id, u.Username)
} }
} }
if len(u.Email) > UserEmailMaxLength || u.Email == "" || !IsValidEmail(u.Email) { if len(u.Email) > UserEmailMaxLength || u.Email == "" || !IsValidEmail(u.Email) {
return InvalidUserError("email", u.Id) return InvalidUserError("email", u.Id, u.Email)
} }
if utf8.RuneCountInString(u.Nickname) > UserNicknameMaxRunes { if utf8.RuneCountInString(u.Nickname) > UserNicknameMaxRunes {
return InvalidUserError("nickname", u.Id) return InvalidUserError("nickname", u.Id, u.Nickname)
} }
if utf8.RuneCountInString(u.Position) > UserPositionMaxRunes { if utf8.RuneCountInString(u.Position) > UserPositionMaxRunes {
return InvalidUserError("position", u.Id) return InvalidUserError("position", u.Id, u.Position)
} }
if utf8.RuneCountInString(u.FirstName) > UserFirstNameMaxRunes { if utf8.RuneCountInString(u.FirstName) > UserFirstNameMaxRunes {
return InvalidUserError("first_name", u.Id) return InvalidUserError("first_name", u.Id, u.FirstName)
} }
if utf8.RuneCountInString(u.LastName) > UserLastNameMaxRunes { if utf8.RuneCountInString(u.LastName) > UserLastNameMaxRunes {
return InvalidUserError("last_name", u.Id) return InvalidUserError("last_name", u.Id, u.LastName)
} }
if u.AuthData != nil && len(*u.AuthData) > UserAuthDataMaxLength { if u.AuthData != nil && len(*u.AuthData) > UserAuthDataMaxLength {
return InvalidUserError("auth_data", u.Id) return InvalidUserError("auth_data", u.Id, u.AuthData)
} }
if u.AuthData != nil && *u.AuthData != "" && u.AuthService == "" { if u.AuthData != nil && *u.AuthData != "" && u.AuthService == "" {
return InvalidUserError("auth_data_type", u.Id) return InvalidUserError("auth_data_type", u.Id, *u.AuthData+" "+u.AuthService)
} }
if u.Password != "" && u.AuthData != nil && *u.AuthData != "" { if u.Password != "" && u.AuthData != nil && *u.AuthData != "" {
return InvalidUserError("auth_data_pwd", u.Id) return InvalidUserError("auth_data_pwd", u.Id, *u.AuthData)
} }
if len(u.Password) > UserPasswordMaxLength { if len(u.Password) > UserPasswordMaxLength {
return InvalidUserError("password_limit", u.Id) return InvalidUserError("password_limit", u.Id, "")
} }
if !IsValidLocale(u.Locale) { if !IsValidLocale(u.Locale) {
return InvalidUserError("locale", u.Id) return InvalidUserError("locale", u.Id, u.Locale)
} }
if len(u.Timezone) > 0 { if len(u.Timezone) > 0 {
if tzJSON, err := json.Marshal(u.Timezone); err != nil { if tzJSON, err := json.Marshal(u.Timezone); err != nil {
return NewAppError("User.IsValid", "model.user.is_valid.marshal.app_error", nil, "", http.StatusInternalServerError).Wrap(err) return NewAppError("User.IsValid", "model.user.is_valid.marshal.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
} else if utf8.RuneCount(tzJSON) > UserTimezoneMaxRunes { } else if utf8.RuneCount(tzJSON) > UserTimezoneMaxRunes {
return InvalidUserError("timezone_limit", u.Id) return InvalidUserError("timezone_limit", u.Id, u.Timezone)
} }
} }
if len(u.Roles) > UserRolesMaxLength { if len(u.Roles) > UserRolesMaxLength {
return NewAppError("User.IsValid", "model.user.is_valid.roles_limit.app_error", return NewAppError("User.IsValid", "model.user.is_valid.roles_limit.app_error",
map[string]any{"Limit": UserRolesMaxLength}, "user_id="+u.Id, http.StatusBadRequest) map[string]any{"Limit": UserRolesMaxLength}, "user_id="+u.Id+" roles_limit="+u.Roles, http.StatusBadRequest)
} }
return nil return nil
} }
func InvalidUserError(fieldName string, userId string) *AppError { func InvalidUserError(fieldName, userId string, fieldValue any) *AppError {
id := fmt.Sprintf("model.user.is_valid.%s.app_error", fieldName) id := fmt.Sprintf("model.user.is_valid.%s.app_error", fieldName)
details := "" details := ""
if userId != "" { if userId != "" {
details = "user_id=" + userId details = "user_id=" + userId
} }
details += fmt.Sprintf(" %s=%v", fieldName, fieldValue)
return NewAppError("User.IsValid", id, nil, details, http.StatusBadRequest) return NewAppError("User.IsValid", id, nil, details, http.StatusBadRequest)
} }

Просмотреть файл

@@ -77,37 +77,37 @@ func TestUserUpdateMentionKeysFromUsername(t *testing.T) {
func TestUserIsValid(t *testing.T) { func TestUserIsValid(t *testing.T) {
user := User{} user := User{}
appErr := user.IsValid() appErr := user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "id", ""), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "id", "", user.Id), "expected user is valid error: %s", appErr.Error())
user.Id = NewId() user.Id = NewId()
appErr = user.IsValid() appErr = user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "create_at", user.Id), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "create_at", user.Id, user.CreateAt), "expected user is valid error: %s", appErr.Error())
user.CreateAt = GetMillis() user.CreateAt = GetMillis()
appErr = user.IsValid() appErr = user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "update_at", user.Id), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "update_at", user.Id, user.UpdateAt), "expected user is valid error: %s", appErr.Error())
user.UpdateAt = GetMillis() user.UpdateAt = GetMillis()
appErr = user.IsValid() appErr = user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "username", user.Id), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "username", user.Id, user.Username), "expected user is valid error: %s", appErr.Error())
user.Username = NewId() + "^hello#" user.Username = NewId() + "^hello#"
appErr = user.IsValid() appErr = user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "username", user.Id), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "username", user.Id, user.Username), "expected user is valid error: %s", appErr.Error())
user.Username = NewId() user.Username = NewId()
appErr = user.IsValid() appErr = user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "email", user.Id), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "email", user.Id, user.Email), "expected user is valid error: %s", appErr.Error())
user.Email = strings.Repeat("01234567890", 20) user.Email = strings.Repeat("01234567890", 20)
appErr = user.IsValid() appErr = user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "email", user.Id), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "email", user.Id, user.Email), "expected user is valid error: %s", appErr.Error())
user.Email = "user@example.com" user.Email = "user@example.com"
user.Nickname = strings.Repeat("a", 65) user.Nickname = strings.Repeat("a", 65)
appErr = user.IsValid() appErr = user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "nickname", user.Id), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "nickname", user.Id, user.Nickname), "expected user is valid error: %s", appErr.Error())
user.Nickname = strings.Repeat("a", 64) user.Nickname = strings.Repeat("a", 64)
require.Nil(t, user.IsValid()) require.Nil(t, user.IsValid())
@@ -118,12 +118,12 @@ func TestUserIsValid(t *testing.T) {
user.FirstName = strings.Repeat("a", 65) user.FirstName = strings.Repeat("a", 65)
appErr = user.IsValid() appErr = user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "first_name", user.Id), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "first_name", user.Id, user.FirstName), "expected user is valid error: %s", appErr.Error())
user.FirstName = strings.Repeat("a", 64) user.FirstName = strings.Repeat("a", 64)
user.LastName = strings.Repeat("a", 65) user.LastName = strings.Repeat("a", 65)
appErr = user.IsValid() appErr = user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "last_name", user.Id), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "last_name", user.Id, user.LastName), "expected user is valid error: %s", appErr.Error())
user.LastName = strings.Repeat("a", 64) user.LastName = strings.Repeat("a", 64)
user.Position = strings.Repeat("a", 128) user.Position = strings.Repeat("a", 128)
@@ -131,7 +131,7 @@ func TestUserIsValid(t *testing.T) {
user.Position = strings.Repeat("a", 129) user.Position = strings.Repeat("a", 129)
appErr = user.IsValid() appErr = user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "position", user.Id), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "position", user.Id, user.Position), "expected user is valid error: %s", appErr.Error())
user.Position = "" user.Position = ""
user.Roles = strings.Repeat("a", UserRolesMaxLength) user.Roles = strings.Repeat("a", UserRolesMaxLength)
@@ -140,10 +140,10 @@ func TestUserIsValid(t *testing.T) {
user.Roles = strings.Repeat("a", UserRolesMaxLength+1) user.Roles = strings.Repeat("a", UserRolesMaxLength+1)
appErr = user.IsValid() appErr = user.IsValid()
require.True(t, HasExpectedUserIsValidError(appErr, "roles_limit", user.Id), "expected user is valid error: %s", appErr.Error()) require.True(t, HasExpectedUserIsValidError(appErr, "roles_limit", user.Id, user.Roles), "expected user is valid error: %s", appErr.Error())
} }
func HasExpectedUserIsValidError(err *AppError, fieldName string, userId string) bool { func HasExpectedUserIsValidError(err *AppError, fieldName, userId string, fieldValue any) bool {
if err == nil { if err == nil {
return false return false
} }
@@ -151,7 +151,7 @@ func HasExpectedUserIsValidError(err *AppError, fieldName string, userId string)
return err.Where == "User.IsValid" && return err.Where == "User.IsValid" &&
err.Id == fmt.Sprintf("model.user.is_valid.%s.app_error", fieldName) && err.Id == fmt.Sprintf("model.user.is_valid.%s.app_error", fieldName) &&
err.StatusCode == http.StatusBadRequest && err.StatusCode == http.StatusBadRequest &&
(userId == "" || err.DetailedError == "user_id="+userId) (userId == "" || err.DetailedError == fmt.Sprintf("user_id=%s %s=%v", userId, fieldName, fieldValue))
} }
func TestUserGetFullName(t *testing.T) { func TestUserGetFullName(t *testing.T) {