From de46d798e44bac2d5b4589161fdab8698698246c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20Garc=C3=ADa=20Montoro?=
 <alejandro.garciamontoro@gmail.com>
Date: Mon, 21 Apr 2025 19:22:15 +0200
Subject: [PATCH] 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>
---
 server/public/model/utils.go      | 22 ++++++++++++++++------
 server/public/model/utils_test.go |  4 ++++
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/server/public/model/utils.go b/server/public/model/utils.go
index 49fd3326090..527828e26c5 100644
--- a/server/public/model/utils.go
+++ b/server/public/model/utils.go
@@ -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
diff --git a/server/public/model/utils_test.go b/server/public/model/utils_test.go
index 9608d4d9216..e7fd63eae15 100644
--- a/server/public/model/utils_test.go
+++ b/server/public/model/utils_test.go
@@ -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,