MM-60780: Reject emails within angle brackets (#29661)

* Reject emails within angle brackets

mail.ParseAddress is RFC-compliant, which means that it accepts emails
with names, as in "Billy Bob <billy@example.com>". It even accepts this
form *without* a name; e.g. "<billy@example.com>". We want to store the
plain address, so we compare the user input with the Address field of
the result from mail.ParseAddress, which should contain only
"billy@example.com", thus only accepting emails that do not contain
names nor angle brackets.

* Log a warning for admins with clear next steps

* Fix wording of comment

* And a typo

* Add specific command example to log message

* Add input email to log message

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Alejandro García Montoro 2025-04-21 19:22:15 +02:00 committed by GitHub
parent 502506a517
commit de46d798e4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 20 additions and 6 deletions

View file

@ -26,6 +26,7 @@ import (
"github.com/pkg/errors"
"github.com/mattermost/mattermost/server/public/shared/i18n"
"github.com/mattermost/mattermost/server/public/shared/mlog"
)
const (
@ -619,22 +620,31 @@ func isLower(s string) bool {
return strings.ToLower(s) == s
}
func IsValidEmail(email string) bool {
if !isLower(email) {
func IsValidEmail(input string) bool {
if !isLower(input) {
return false
}
if addr, err := mail.ParseAddress(email); err != nil {
if addr, err := mail.ParseAddress(input); err != nil {
return false
} else if addr.Name != "" {
// mail.ParseAddress accepts input of the form "Billy Bob <billy@example.com>" which we don't allow
} else if addr.Address != input {
// mail.ParseAddress accepts input of the form "Billy Bob <billy@example.com>" or "<billy@example.com>",
// which we don't allow. We compare the user input with the parsed addr.Address to ensure we only
// accept plain addresses like "billy@example.com"
// Log a warning for admins in case pre-existing users with emails like <billy@example.com>, which used
// to be valid before https://github.com/mattermost/mattermost/pull/29661, know how to deal with this
// error. We don't need to check for the case addr.Name != "", since that has always been rejected
if addr.Name == "" {
mlog.Warn("email seems to be enclosed in angle brackets, which is not valid; if this relates to an existing user, use the following mmctl command to modify their email: `mmctl user email \"<affecteduser@domain.com>\" affecteduser@domain.com`", mlog.String("email", input))
}
return false
}
// mail.ParseAddress accepts quoted strings for the address
// which can lead to sending to the wrong email address
// check for multiple '@' symbols and invalidate
if strings.Count(email, "@") > 1 {
if strings.Count(input, "@") > 1 {
return false
}
return true

View file

@ -372,6 +372,10 @@ func TestIsValidEmail(t *testing.T) {
Input: "Billy Bob <billy@example.com>",
Expected: false,
},
{
Input: "<billy@example.com>",
Expected: false,
},
{
Input: "email.domain.com",
Expected: false,