From 282550f402f3262bb19744d966dda85013aa768e Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 7 Jan 2015 13:17:44 +1100 Subject: [PATCH] Fixes: U4-6080 Server side email validation doesn't occur on existing members when they are saved --- .../CoreStrings/StringExtensionsTests.cs | 2 +- .../CoreStrings/StringValidationTests.cs | 28 ++++ src/Umbraco.Tests/Umbraco.Tests.csproj | 1 + src/Umbraco.Web/Editors/MemberController.cs | 1 - ...ershipProviderValidationFilterAttribute.cs | 129 ----------------- src/Umbraco.Web/Umbraco.Web.csproj | 1 - .../WebApi/Binders/MemberBinder.cs | 134 ++++++++++++++++++ 7 files changed, 164 insertions(+), 132 deletions(-) create mode 100644 src/Umbraco.Tests/CoreStrings/StringValidationTests.cs delete mode 100644 src/Umbraco.Web/Editors/MembershipProviderValidationFilterAttribute.cs diff --git a/src/Umbraco.Tests/CoreStrings/StringExtensionsTests.cs b/src/Umbraco.Tests/CoreStrings/StringExtensionsTests.cs index c86ee01100..43e5d0fcf9 100644 --- a/src/Umbraco.Tests/CoreStrings/StringExtensionsTests.cs +++ b/src/Umbraco.Tests/CoreStrings/StringExtensionsTests.cs @@ -7,7 +7,7 @@ using Umbraco.Core.ObjectResolution; namespace Umbraco.Tests.CoreStrings { - [TestFixture] + [TestFixture] public class StringExtensionsTests { [SetUp] diff --git a/src/Umbraco.Tests/CoreStrings/StringValidationTests.cs b/src/Umbraco.Tests/CoreStrings/StringValidationTests.cs new file mode 100644 index 0000000000..c1f8b92438 --- /dev/null +++ b/src/Umbraco.Tests/CoreStrings/StringValidationTests.cs @@ -0,0 +1,28 @@ +using System.ComponentModel.DataAnnotations; +using NUnit.Framework; + +namespace Umbraco.Tests.CoreStrings +{ + [TestFixture] + public class StringValidationTests + { + [Test] + public void Validate_Email_Address() + { + var foo = new EmailAddressAttribute(); + + Assert.IsTrue(foo.IsValid("someone@somewhere.com")); + Assert.IsTrue(foo.IsValid("someone@somewhere.co.uk")); + Assert.IsTrue(foo.IsValid("someone+tag@somewhere.net")); + Assert.IsTrue(foo.IsValid("futureTLD@somewhere.fooo")); + + Assert.IsTrue(foo.IsValid("abc@xyz.financial")); + + Assert.IsFalse(foo.IsValid("fdsa")); + Assert.IsFalse(foo.IsValid("fdsa@")); + Assert.IsFalse(foo.IsValid("fdsa@fdsa")); + Assert.IsFalse(foo.IsValid("fdsa@fdsa.")); + + } + } +} \ No newline at end of file diff --git a/src/Umbraco.Tests/Umbraco.Tests.csproj b/src/Umbraco.Tests/Umbraco.Tests.csproj index b6f562f106..60e57f91f0 100644 --- a/src/Umbraco.Tests/Umbraco.Tests.csproj +++ b/src/Umbraco.Tests/Umbraco.Tests.csproj @@ -168,6 +168,7 @@ + diff --git a/src/Umbraco.Web/Editors/MemberController.cs b/src/Umbraco.Web/Editors/MemberController.cs index a696ccf0b1..30320b278e 100644 --- a/src/Umbraco.Web/Editors/MemberController.cs +++ b/src/Umbraco.Web/Editors/MemberController.cs @@ -230,7 +230,6 @@ namespace Umbraco.Web.Editors /// /// [FileUploadCleanupFilter] - [MembershipProviderValidationFilter] public MemberDisplay PostSave( [ModelBinder(typeof(MemberBinder))] MemberSave contentItem) diff --git a/src/Umbraco.Web/Editors/MembershipProviderValidationFilterAttribute.cs b/src/Umbraco.Web/Editors/MembershipProviderValidationFilterAttribute.cs deleted file mode 100644 index aa95af3531..0000000000 --- a/src/Umbraco.Web/Editors/MembershipProviderValidationFilterAttribute.cs +++ /dev/null @@ -1,129 +0,0 @@ -using System; -using System.ComponentModel.DataAnnotations; -using System.Linq; -using System.Net; -using System.Web.Http; -using System.Web.Http.Controllers; -using System.Web.Http.Filters; -using System.Web.Security; -using Umbraco.Core; -using Umbraco.Web.Models.ContentEditing; - -namespace Umbraco.Web.Editors -{ - /// - /// This validates the submitted data in regards to the current membership provider - /// - internal class MembershipProviderValidationFilterAttribute : ActionFilterAttribute - { - public override void OnActionExecuting(HttpActionContext actionContext) - { - base.OnActionExecuting(actionContext); - - //default provider! - var membershipProvider = Core.Security.MembershipProviderExtensions.GetMembersMembershipProvider(); - - var contentItem = (MemberSave) actionContext.ActionArguments["contentItem"]; - - var validEmail = ValidateUniqueEmail(contentItem, membershipProvider, actionContext); - if (validEmail == false) - { - actionContext.ModelState.AddPropertyError( - new ValidationResult("Email address is already in use", new[] {"value"}), - string.Format("{0}email", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); - } - - var validLogin = ValidateUniqueLogin(contentItem, membershipProvider, actionContext); - if (validLogin == false) - { - actionContext.ModelState.AddPropertyError( - new ValidationResult("Username is already in use", new[] { "value" }), - string.Format("{0}login", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); - } - } - - internal bool ValidateUniqueLogin(MemberSave contentItem, MembershipProvider membershipProvider, HttpActionContext actionContext) - { - if (contentItem == null) throw new ArgumentNullException("contentItem"); - if (membershipProvider == null) throw new ArgumentNullException("membershipProvider"); - - int totalRecs; - var existingByName = membershipProvider.FindUsersByName(contentItem.Username.Trim(), 0, int.MaxValue, out totalRecs); - switch (contentItem.Action) - { - case ContentSaveAction.Save: - - //ok, we're updating the member, we need to check if they are changing their login and if so, does it exist already ? - if (contentItem.PersistedContent.Username.InvariantEquals(contentItem.Username.Trim()) == false) - { - //they are changing their login name - if (existingByName.Cast().Select(x => x.UserName) - .Any(x => x == contentItem.Username.Trim())) - { - //the user cannot use this login - return false; - } - } - break; - case ContentSaveAction.SaveNew: - //check if the user's login already exists - if (existingByName.Cast().Select(x => x.UserName) - .Any(x => x == contentItem.Username.Trim())) - { - //the user cannot use this login - return false; - } - break; - default: - //we don't support this for members - throw new HttpResponseException(HttpStatusCode.NotFound); - } - - return true; - } - - internal bool ValidateUniqueEmail(MemberSave contentItem, MembershipProvider membershipProvider, HttpActionContext actionContext) - { - if (contentItem == null) throw new ArgumentNullException("contentItem"); - if (membershipProvider == null) throw new ArgumentNullException("membershipProvider"); - - if (membershipProvider.RequiresUniqueEmail == false) - { - return true; - } - - int totalRecs; - var existingByEmail = membershipProvider.FindUsersByEmail(contentItem.Email.Trim(), 0, int.MaxValue, out totalRecs); - switch (contentItem.Action) - { - case ContentSaveAction.Save: - //ok, we're updating the member, we need to check if they are changing their email and if so, does it exist already ? - if (contentItem.PersistedContent.Email.InvariantEquals(contentItem.Email.Trim()) == false) - { - //they are changing their email - if (existingByEmail.Cast().Select(x => x.Email) - .Any(x => x.InvariantEquals(contentItem.Email.Trim()))) - { - //the user cannot use this email - return false; - } - } - break; - case ContentSaveAction.SaveNew: - //check if the user's email already exists - if (existingByEmail.Cast().Select(x => x.Email) - .Any(x => x.InvariantEquals(contentItem.Email.Trim()))) - { - //the user cannot use this email - return false; - } - break; - default: - //we don't support this for members - throw new HttpResponseException(HttpStatusCode.NotFound); - } - - return true; - } - } -} \ No newline at end of file diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index 21ac991bbc..2dec8c27f6 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -397,7 +397,6 @@ - diff --git a/src/Umbraco.Web/WebApi/Binders/MemberBinder.cs b/src/Umbraco.Web/WebApi/Binders/MemberBinder.cs index 3893167a73..49669fd427 100644 --- a/src/Umbraco.Web/WebApi/Binders/MemberBinder.cs +++ b/src/Umbraco.Web/WebApi/Binders/MemberBinder.cs @@ -1,5 +1,8 @@ using System; using System.Collections.Generic; +using System.ComponentModel.DataAnnotations; +using System.Net; +using System.Web.Http; using System.Web.Http.Controllers; using System.Web.Http.ModelBinding; using System.Web.Security; @@ -12,6 +15,7 @@ using Umbraco.Web.Models.ContentEditing; using Umbraco.Web.WebApi.Filters; using System.Linq; using Umbraco.Core.Models.Membership; +using Umbraco.Web; namespace Umbraco.Web.WebApi.Binders { @@ -194,6 +198,52 @@ namespace Umbraco.Web.WebApi.Binders /// internal class MemberValidationHelper : ContentItemValidationHelper { + /// + /// We need to manually validate a few things here like email and login to make sure they are valid and aren't duplicates + /// + /// + /// + /// + protected override bool ValidatePropertyData(ContentItemBasic postedItem, HttpActionContext actionContext) + { + var memberSave = (MemberSave)postedItem; + + if (memberSave.Username.IsNullOrWhiteSpace()) + { + actionContext.ModelState.AddPropertyError( + new ValidationResult("Invalid user name", new[] { "value" }), + string.Format("{0}login", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); + } + + if (memberSave.Email.IsNullOrWhiteSpace() || new EmailAddressAttribute().IsValid(memberSave.Email) == false) + { + actionContext.ModelState.AddPropertyError( + new ValidationResult("Invalid email", new[] { "value" }), + string.Format("{0}email", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); + } + + //default provider! + var membershipProvider = Core.Security.MembershipProviderExtensions.GetMembersMembershipProvider(); + + var validEmail = ValidateUniqueEmail(memberSave, membershipProvider, actionContext); + if (validEmail == false) + { + actionContext.ModelState.AddPropertyError( + new ValidationResult("Email address is already in use", new[] { "value" }), + string.Format("{0}email", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); + } + + var validLogin = ValidateUniqueLogin(memberSave, membershipProvider, actionContext); + if (validLogin == false) + { + actionContext.ModelState.AddPropertyError( + new ValidationResult("Username is already in use", new[] { "value" }), + string.Format("{0}login", Constants.PropertyEditors.InternalGenericPropertiesPrefix)); + } + + return base.ValidatePropertyData(postedItem, actionContext); + } + protected override bool ValidateProperties(ContentItemBasic postedItem, HttpActionContext actionContext) { var propertiesToValidate = postedItem.Properties.ToList(); @@ -206,6 +256,90 @@ namespace Umbraco.Web.WebApi.Binders return ValidateProperties(propertiesToValidate.ToArray(), postedItem.PersistedContent.Properties.ToArray(), actionContext); } + + internal bool ValidateUniqueLogin(MemberSave contentItem, MembershipProvider membershipProvider, HttpActionContext actionContext) + { + if (contentItem == null) throw new ArgumentNullException("contentItem"); + if (membershipProvider == null) throw new ArgumentNullException("membershipProvider"); + + int totalRecs; + var existingByName = membershipProvider.FindUsersByName(contentItem.Username.Trim(), 0, int.MaxValue, out totalRecs); + switch (contentItem.Action) + { + case ContentSaveAction.Save: + + //ok, we're updating the member, we need to check if they are changing their login and if so, does it exist already ? + if (contentItem.PersistedContent.Username.InvariantEquals(contentItem.Username.Trim()) == false) + { + //they are changing their login name + if (existingByName.Cast().Select(x => x.UserName) + .Any(x => x == contentItem.Username.Trim())) + { + //the user cannot use this login + return false; + } + } + break; + case ContentSaveAction.SaveNew: + //check if the user's login already exists + if (existingByName.Cast().Select(x => x.UserName) + .Any(x => x == contentItem.Username.Trim())) + { + //the user cannot use this login + return false; + } + break; + default: + //we don't support this for members + throw new HttpResponseException(HttpStatusCode.NotFound); + } + + return true; + } + + internal bool ValidateUniqueEmail(MemberSave contentItem, MembershipProvider membershipProvider, HttpActionContext actionContext) + { + if (contentItem == null) throw new ArgumentNullException("contentItem"); + if (membershipProvider == null) throw new ArgumentNullException("membershipProvider"); + + if (membershipProvider.RequiresUniqueEmail == false) + { + return true; + } + + int totalRecs; + var existingByEmail = membershipProvider.FindUsersByEmail(contentItem.Email.Trim(), 0, int.MaxValue, out totalRecs); + switch (contentItem.Action) + { + case ContentSaveAction.Save: + //ok, we're updating the member, we need to check if they are changing their email and if so, does it exist already ? + if (contentItem.PersistedContent.Email.InvariantEquals(contentItem.Email.Trim()) == false) + { + //they are changing their email + if (existingByEmail.Cast().Select(x => x.Email) + .Any(x => x.InvariantEquals(contentItem.Email.Trim()))) + { + //the user cannot use this email + return false; + } + } + break; + case ContentSaveAction.SaveNew: + //check if the user's email already exists + if (existingByEmail.Cast().Select(x => x.Email) + .Any(x => x.InvariantEquals(contentItem.Email.Trim()))) + { + //the user cannot use this email + return false; + } + break; + default: + //we don't support this for members + throw new HttpResponseException(HttpStatusCode.NotFound); + } + + return true; + } } } } \ No newline at end of file