Abstracted account validation (#7944)

* Added a service to abstract some account validation away from the AccountController, so it's easier to upgrade it and use the same validation elsewhere.

* Added a validation context to carry information used for validation of account
information.

* Refactored password validation in the AccountController

* Updated tests

* fixed value read from context.ValidationSuccessful

src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs

@@ -70,6 +70,7 @@ namespace Orchard.Tests.Modules.Users.Controllers {
             builder.RegisterType<UserService>().As<IUserService>();
             builder.RegisterType<UserPartHandler>().As<IContentHandler>();
             builder.RegisterType<OrchardServices>().As<IOrchardServices>();
+            builder.RegisterType<AccountValidationService>().As<IAccountValidationService>();
 
             builder.RegisterInstance(new Work<IEnumerable<IShapeTableEventHandler>>(resolve => _container.Resolve<IEnumerable<IShapeTableEventHandler>>())).AsSelf();
             builder.RegisterType<DefaultShapeTableManager>().As<IShapeTableManager>();

src/Orchard.Tests.Modules/Users/Services/MembershipServiceTests.cs

@@ -103,6 +103,7 @@ namespace Orchard.Tests.Modules.Users.Services
             builder.RegisterInstance(_workContextAccessor.Object).As<IWorkContextAccessor>();
 
             _container = builder.Build();
+            _container.Resolve<IWorkContextAccessor>().GetContext().CurrentSite.ContentItem.Weld(new RegistrationSettingsPart());
             _membershipValidationService = _container.Resolve<IMembershipValidationService>();
             _membershipService = _container.Resolve<IMembershipService>();
         }

src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs

@@ -27,6 +27,7 @@ namespace Orchard.Users.Controllers {
         private readonly IOrchardServices _orchardServices;
         private readonly IUserEventHandler _userEventHandler;
         private readonly IClock _clock;
+        private readonly IAccountValidationService _accountValidationService;
 
         public AccountController(
             IAuthenticationService authenticationService,
@@ -34,7 +35,8 @@ namespace Orchard.Users.Controllers {
             IUserService userService,
             IOrchardServices orchardServices,
             IUserEventHandler userEventHandler,
-            IClock clock) {
+            IClock clock,
+            IAccountValidationService accountValidationService) {
 
             _authenticationService = authenticationService;
             _membershipService = membershipService;
@@ -42,6 +44,7 @@ namespace Orchard.Users.Controllers {
             _orchardServices = orchardServices;
             _userEventHandler = userEventHandler;
             _clock = clock;
+            _accountValidationService = accountValidationService;
 
             Logger = NullLogger.Instance;
             T = NullLocalizer.Instance;
@@ -277,8 +280,7 @@ namespace Orchard.Users.Controllers {
 
                 }
                 return RedirectToAction("ChangePasswordSuccess");
-            }
-            else {
+            } else {
                 return ChangePassword();
             }
         }
@@ -320,8 +322,7 @@ namespace Orchard.Users.Controllers {
 
             if (PasswordChangeIsSuccess(currentPassword, newPassword, username)) {
                 return RedirectToAction("ChangePasswordSuccess");
-            }
-            else {
+            } else {
                 return View(viewModel);
             }
         }
@@ -342,8 +343,7 @@ namespace Orchard.Users.Controllers {
                     return true;
                 }
 
-            }
-            catch {
+            } catch {
                 ModelState.AddModelError("_FORM", T("The current password is incorrect or the new password is invalid."));
 
                 return false;
@@ -383,13 +383,7 @@ namespace Orchard.Users.Controllers {
             ViewData["SpecialCharacterRequirement"] = membershipSettings.GetPasswordSpecialRequirement();
             ViewData["NumberRequirement"] = membershipSettings.GetPasswordNumberRequirement();
 
-            ValidatePassword(newPassword);
-
-            if (!string.Equals(newPassword, confirmPassword, StringComparison.Ordinal)) {
-                ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match."));
-            }
-
-            if (!ModelState.IsValid) {
+            if (!ValidatePassword(newPassword, confirmPassword)) {
                 return View();
             }
 
@@ -452,16 +446,13 @@ namespace Orchard.Users.Controllers {
                 ModelState.AddModelError("newPassword", T("The new password must be different from the current password."));
             }
 
-            ValidatePassword(newPassword);
-
-            if (!string.Equals(newPassword, confirmPassword, StringComparison.Ordinal)) {
-                ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match."));
+            if (!ModelState.IsValid) {
+                return false;
             }
 
-            return ModelState.IsValid;
+            return ValidatePassword(newPassword, confirmPassword);
         }
 
-
         private IUser ValidateLogOn(string userNameOrEmail, string password) {
             bool validate = true;
 
@@ -469,6 +460,8 @@ namespace Orchard.Users.Controllers {
                 ModelState.AddModelError("userNameOrEmail", T("You must specify a username or e-mail."));
                 validate = false;
             }
+            // Here we don't do the "full" validation of the password, because policies may have
+            // changed since its creation and that should not prevent a user from logging in.
             if (string.IsNullOrEmpty(password)) {
                 ModelState.AddModelError("password", T("You must specify a password."));
                 validate = false;
@@ -490,54 +483,61 @@ namespace Orchard.Users.Controllers {
         }
 
         private bool ValidateRegistration(string userName, string email, string password, string confirmPassword) {
-            bool validate = true;
 
-            if (string.IsNullOrEmpty(userName)) {
-                ModelState.AddModelError("username", T("You must specify a username."));
-                validate = false;
-            }
-            else {
-                if (userName.Length >= UserPart.MaxUserNameLength) {
-                    ModelState.AddModelError("username", T("The username you provided is too long."));
-                    validate = false;
+            var context = new AccountValidationContext {
+                UserName = userName,
+                Email = email,
+                Password = password
+            };
+
+            _accountValidationService.ValidateUserName(context);
+            _accountValidationService.ValidateEmail(context);
+            // Don't do the other validations if we already know we failed
+            if (!context.ValidationSuccessful) {
+                foreach (var error in context.ValidationErrors) {
+                    ModelState.AddModelError(error.Key, error.Value);
                 }
-            }
-
-            if (string.IsNullOrEmpty(email)) {
-                ModelState.AddModelError("email", T("You must specify an email address."));
-                validate = false;
-            }
-            else if (email.Length >= UserPart.MaxEmailLength) {
-                ModelState.AddModelError("email", T("The email address you provided is too long."));
-                validate = false;
-            }
-            else if (!Regex.IsMatch(email, UserPart.EmailPattern, RegexOptions.IgnoreCase)) {
-                // http://haacked.com/archive/2007/08/21/i-knew-how-to-validate-an-email-address-until-i.aspx    
-                ModelState.AddModelError("email", T("You must specify a valid email address."));
-                validate = false;
-            }
-
-            if (!validate)
                 return false;
+            }
 
             if (!_userService.VerifyUserUnicity(userName, email)) {
-                ModelState.AddModelError("userExists", T("User with that username and/or email already exists."));
+                context.ValidationErrors.Add("userExists", T("User with that username and/or email already exists."));
             }
 
-            ValidatePassword(password);
+            _accountValidationService.ValidatePassword(context);
 
             if (!string.Equals(password, confirmPassword, StringComparison.Ordinal)) {
-                ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match."));
+                context.ValidationErrors.Add("_FORM", T("The new password and confirmation password do not match."));
             }
-            return ModelState.IsValid;
-        }
 
-        private void ValidatePassword(string password) {
-            if (!_userService.PasswordMeetsPolicies(password, out IDictionary<string, LocalizedString> validationErrors)) {
-                foreach (var error in validationErrors) {
+            if (!context.ValidationSuccessful) {
+                foreach (var error in context.ValidationErrors) {
                     ModelState.AddModelError(error.Key, error.Value);
                 }
             }
+
+            return ModelState.IsValid;
+        }
+
+        private bool ValidatePassword(string password) {
+            var context = new AccountValidationContext {
+                Password = password
+            };
+            var result = _accountValidationService.ValidatePassword(context);
+            if (!result) {
+                foreach (var error in context.ValidationErrors) {
+                    ModelState.AddModelError(error.Key, error.Value);
+                }
+            }
+            return result;
+        }
+
+        private bool ValidatePassword(string password, string confirmPassword) {
+            if (!string.Equals(password, confirmPassword, StringComparison.Ordinal)) {
+                ModelState.AddModelError("_FORM", T("The new password and confirmation password do not match."));
+                return false;
+            }
+            return ValidatePassword(password);
         }
 
         private static string ErrorCodeToString(MembershipCreateStatus createStatus) {

src/Orchard.Web/Modules/Orchard.Users/Orchard.Users.csproj

@@ -131,6 +131,7 @@
     <Compile Include="Permissions.cs" />
     <Compile Include="Properties\AssemblyInfo.cs" />
     <Compile Include="Services\ApproveUserService.cs" />
+    <Compile Include="Services\AccountValidationService.cs" />
     <Compile Include="Services\AuthenticationRedirectionFilter.cs" />
     <Compile Include="Services\IUserService.cs" />
     <Compile Include="Services\MembershipValidationService.cs" />

src/Orchard.Web/Modules/Orchard.Users/Services/AccountValidationService.cs

@@ -0,0 +1,65 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text.RegularExpressions;
+using System.Web;
+using Orchard.Localization;
+using Orchard.Security;
+using Orchard.Users.Models;
+
+namespace Orchard.Users.Services {
+    public class AccountValidationService : IAccountValidationService {
+
+        private readonly IUserService _userService;
+
+        public AccountValidationService(
+            IUserService userService) {
+
+            _userService = userService;
+
+            T = NullLocalizer.Instance;
+        }
+
+        public Localizer T { get; set; }
+
+        public bool ValidatePassword(AccountValidationContext context) {
+            IDictionary<string, LocalizedString> validationErrors;
+            _userService.PasswordMeetsPolicies(context.Password, out validationErrors);
+            if (validationErrors != null && validationErrors.Any()) {
+                foreach (var err in validationErrors) {
+                    if (!context.ValidationErrors.ContainsKey(err.Key)) {
+                        context.ValidationErrors.Add(err);
+                    }
+                }
+            }
+
+            return context.ValidationSuccessful;
+        }
+
+        public bool ValidateUserName(AccountValidationContext context) {
+
+            if (string.IsNullOrWhiteSpace(context.UserName)) {
+                context.ValidationErrors.Add("username", T("You must specify a username."));
+            } else if (context.UserName.Length >= UserPart.MaxUserNameLength) {
+                context.ValidationErrors.Add("username", T("The username you provided is too long."));
+            }
+
+            return context.ValidationSuccessful;
+        }
+
+        public bool ValidateEmail(AccountValidationContext context) {
+
+            if (string.IsNullOrWhiteSpace(context.Email)) {
+                context.ValidationErrors.Add("email", T("You must specify an email address."));
+            } else if (context.Email.Length >= UserPart.MaxEmailLength) {
+                context.ValidationErrors.Add("email", T("The email address you provided is too long."));
+            } else if (!Regex.IsMatch(context.Email, UserPart.EmailPattern, RegexOptions.IgnoreCase)) {
+                // http://haacked.com/archive/2007/08/21/i-knew-how-to-validate-an-email-address-until-i.aspx    
+                context.ValidationErrors.Add("email", T("You must specify a valid email address."));
+            }
+
+            return context.ValidationSuccessful;
+        }
+        
+    }
+}

src/Orchard/Orchard.Framework.csproj

@@ -203,6 +203,8 @@
     <Compile Include="Reports\Services\ReportsCoordinator.cs" />
     <Compile Include="Reports\Services\ReportsManager.cs" />
     <Compile Include="Reports\Services\ReportsPersister.cs" />
+    <Compile Include="Security\AccountValidationContext.cs" />
+    <Compile Include="Security\IAccountValidationService.cs" />
     <Compile Include="Security\IMembershipSettings.cs" />
     <Compile Include="Security\IMembershipValidationService.cs" />
     <Compile Include="Localization\Services\ILocalizationStreamParser.cs" />
@@ -211,6 +213,7 @@
     <Compile Include="Security\ISecurityService.cs" />
     <Compile Include="Security\ISslSettingsProvider.cs" />
     <Compile Include="Security\IUserDataProvider.cs" />
+    <Compile Include="Security\NullAccountValidationService.cs" />
     <Compile Include="Security\NullMembershipService.cs" />
     <Compile Include="Security\Providers\BaseUserDataProvider.cs" />
     <Compile Include="Security\Providers\DefaultSecurityService.cs" />
@@ -1082,4 +1085,4 @@
   </ItemGroup>
   <ItemGroup />
   <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
-</Project>
+</Project>

src/Orchard/Security/AccountValidationContext.cs

@@ -0,0 +1,28 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Orchard.Localization;
+
+namespace Orchard.Security {
+
+    public class AccountValidationContext {
+
+        public AccountValidationContext() {
+            ValidationErrors = new Dictionary<string, LocalizedString>();
+        }
+
+        // Results
+        public IDictionary<string, LocalizedString> ValidationErrors { get; set; }
+        public bool ValidationSuccessful { get { return !ValidationErrors.Any(); } }
+
+        // Things to validate
+        public string UserName { get; set; }
+        public string Email { get; set; }
+        public string Password { get; set; }
+
+        // Additional useful information
+        public IUser User { get; set; }
+    }
+}

src/Orchard/Security/IAccountValidationService.cs

@@ -0,0 +1,32 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Orchard.Localization;
+
+namespace Orchard.Security {
+    public interface IAccountValidationService : IDependency {
+        /// <summary>
+        /// Verifies whether the string is a valid password.
+        /// </summary>
+        /// <param name="context">The object describing the context of the validation.</param>
+        /// <returns>true if the context contains a valid password, false otherwise.</returns>
+        bool ValidatePassword(AccountValidationContext context);
+
+        /// <summary>
+        /// Verifies whether the string is a valid UserName.
+        /// </summary>
+        /// <param name="context">The object describing the context of the validation.</param>
+        /// <returns>true if the context contains a valid UserName, false otherwise.</returns>
+        bool ValidateUserName(AccountValidationContext context);
+
+        /// <summary>
+        /// Verifies whether the string is a valid email.
+        /// </summary>
+        /// <param name="context">The object describing the context of the validation.</param>
+        /// <returns>true if the context contains a valid UserName, false otherwise.</returns>
+        bool ValidateEmail(AccountValidationContext context);
+        
+    }
+}

src/Orchard/Security/NullAccountValidationService.cs

@@ -0,0 +1,27 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Orchard.Localization;
+
+namespace Orchard.Security {
+    /// <summary>
+    /// Provides a default implementation of <c>IAccountValidationService</c> used only for dependency resolution
+    /// in a setup context. No members on this implementation will ever be called; at the time when this
+    /// interface is actually used in a tenant, another implementation is assumed to have suppressed it.
+    /// </summary>
+    public class NullAccountValidationService : IAccountValidationService {
+        public bool ValidateEmail(AccountValidationContext context) {
+            throw new NotImplementedException();
+        }
+
+        public bool ValidatePassword(AccountValidationContext context) {
+            throw new NotImplementedException();
+        }
+
+        public bool ValidateUserName(AccountValidationContext context) {
+            throw new NotImplementedException();
+        }
+    }
+}