Validate email for member models (#17532)

* Validate email for member models * Add null check or more test cases * return invalid when not a valid email * Cleanup * remove private method in favor of extension * Remove non used, using statement --------- Co-authored-by: Elitsa <[email protected]>
umbraco · Nov 25, 2024 · 6b0f8e7 · 6b0f8e7
1 parent 7162efc
commit 6b0f8e7
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 4 deletions.
diff --git a/src/Umbraco.Core/Extensions/StringExtensions.cs b/src/Umbraco.Core/Extensions/StringExtensions.cs
@@ -2,6 +2,7 @@
 // See LICENSE for more details.
 
 using System.ComponentModel;
+using System.ComponentModel.DataAnnotations;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Security.Cryptography;
@@ -1558,6 +1559,14 @@ public static IEnumerable<string> EscapedSplit(this string value, char splitChar
         yield return sb.ToString();
     }
 
+    /// <summary>
+    ///     Checks whether a string is a valid email address.
+    /// </summary>
+    /// <param name="email">The string check</param>
+    /// <returns>Returns a bool indicating whether the string is an email address.</returns>
+    public static bool IsEmail(this string? email) =>
+        string.IsNullOrWhiteSpace(email) is false && new EmailAddressAttribute().IsValid(email);
+
     // having benchmarked various solutions (incl. for/foreach, split and LINQ based ones),
     // this is by far the fastest way to find string needles in a string haystack
     public static int CountOccurrences(this string haystack, string needle)

diff --git a/src/Umbraco.Core/Services/UserService.cs b/src/Umbraco.Core/Services/UserService.cs
@@ -917,7 +917,7 @@ private async Task<UserOperationStatus> ValidateUserCreateModel(UserCreateModel
         {
             return UserOperationStatus.UserNameIsNotEmail;
         }
-        if (!IsEmailValid(model.Email))
+        if (model.Email.IsEmail() is false)
         {
             return UserOperationStatus.InvalidEmail;
         }
@@ -1136,7 +1136,7 @@ private UserOperationStatus ValidateUserUpdateModel(IUser existingUser, UserUpda
             return UserOperationStatus.UserNameIsNotEmail;
         }
 
-        if (IsEmailValid(model.Email) is false)
+        if (model.Email.IsEmail() is false)
         {
             return UserOperationStatus.InvalidEmail;
         }
@@ -1164,8 +1164,6 @@ private UserOperationStatus ValidateUserUpdateModel(IUser existingUser, UserUpda
         return UserOperationStatus.Success;
     }
 
-    private static bool IsEmailValid(string email) => new EmailAddressAttribute().IsValid(email);
-
     private List<int>? GetIdsFromKeys(IEnumerable<Guid>? guids, UmbracoObjectTypes type)
     {
         var keys = guids?

diff --git a/src/Umbraco.Infrastructure/Services/MemberEditingService.cs b/src/Umbraco.Infrastructure/Services/MemberEditingService.cs
@@ -225,6 +225,11 @@ private async Task<MemberEditingOperationStatus> ValidateMemberDataAsync(MemberE
             return MemberEditingOperationStatus.InvalidUsername;
         }
 
+        if (model.Email.IsEmail() is false)
+        {
+            return MemberEditingOperationStatus.InvalidEmail;
+        }
+
         if (password is not null)
         {
             IdentityResult validatePassword = await _memberManager.ValidatePasswordAsync(password);

diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Core/ShortStringHelper/StringExtensionsTests.cs
@@ -364,6 +364,16 @@ public void IsFullPath()
         TryIsFullPath("   ", false, false); // technically, a valid filename on Linux
     }
 
+    [TestCase("[email protected]", true)]
+    [TestCase("test@test", true)]
+    [TestCase("testtest.com", false)]
+    [TestCase("[email protected]", true)]
+    [TestCase("[email protected]", true)]
+    [TestCase(null, false)]
+    [TestCase("", false)]
+    [TestCase(" ", false)]
+    public void IsEmail(string? email, bool isEmail) => Assert.AreEqual(isEmail, email.IsEmail());
+
     private static void TryIsFullPath(string path, bool expectedIsFull, bool expectedIsValid = true)
     {
         Assert.AreEqual(expectedIsFull, path.IsFullPath(), "IsFullPath('" + path + "')");