diff --git a/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs b/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs index c67e733ad..eed55f7ec 100644 --- a/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs +++ b/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs @@ -70,6 +70,7 @@ namespace Orchard.Tests.Modules.Users.Controllers { builder.RegisterType().As(); builder.RegisterType().As(); builder.RegisterType().As(); + builder.RegisterType().As(); builder.RegisterInstance(new Work>(resolve => _container.Resolve>())).AsSelf(); builder.RegisterType().As(); diff --git a/src/Orchard.Tests.Modules/Users/Services/MembershipServiceTests.cs b/src/Orchard.Tests.Modules/Users/Services/MembershipServiceTests.cs index 54bdb9530..f5060086f 100644 --- a/src/Orchard.Tests.Modules/Users/Services/MembershipServiceTests.cs +++ b/src/Orchard.Tests.Modules/Users/Services/MembershipServiceTests.cs @@ -103,6 +103,7 @@ namespace Orchard.Tests.Modules.Users.Services builder.RegisterInstance(_workContextAccessor.Object).As(); _container = builder.Build(); + _container.Resolve().GetContext().CurrentSite.ContentItem.Weld(new RegistrationSettingsPart()); _membershipValidationService = _container.Resolve(); _membershipService = _container.Resolve(); } diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs index 0bf3b1ddc..83ff318b2 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs +++ b/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 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) { diff --git a/src/Orchard.Web/Modules/Orchard.Users/Orchard.Users.csproj b/src/Orchard.Web/Modules/Orchard.Users/Orchard.Users.csproj index 2311d4617..a61420923 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Orchard.Users.csproj +++ b/src/Orchard.Web/Modules/Orchard.Users/Orchard.Users.csproj @@ -131,6 +131,7 @@ + diff --git a/src/Orchard.Web/Modules/Orchard.Users/Services/AccountValidationService.cs b/src/Orchard.Web/Modules/Orchard.Users/Services/AccountValidationService.cs new file mode 100644 index 000000000..6bde8ab8a --- /dev/null +++ b/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 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; + } + + } +} \ No newline at end of file diff --git a/src/Orchard/Orchard.Framework.csproj b/src/Orchard/Orchard.Framework.csproj index 4014de2ce..6d3e1306d 100644 --- a/src/Orchard/Orchard.Framework.csproj +++ b/src/Orchard/Orchard.Framework.csproj @@ -203,6 +203,8 @@ + + @@ -211,6 +213,7 @@ + @@ -1082,4 +1085,4 @@ - + \ No newline at end of file diff --git a/src/Orchard/Security/AccountValidationContext.cs b/src/Orchard/Security/AccountValidationContext.cs new file mode 100644 index 000000000..aee6b6643 --- /dev/null +++ b/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(); + } + + // Results + public IDictionary 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; } + } +} diff --git a/src/Orchard/Security/IAccountValidationService.cs b/src/Orchard/Security/IAccountValidationService.cs new file mode 100644 index 000000000..ba6fbda66 --- /dev/null +++ b/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 { + /// + /// Verifies whether the string is a valid password. + /// + /// The object describing the context of the validation. + /// true if the context contains a valid password, false otherwise. + bool ValidatePassword(AccountValidationContext context); + + /// + /// Verifies whether the string is a valid UserName. + /// + /// The object describing the context of the validation. + /// true if the context contains a valid UserName, false otherwise. + bool ValidateUserName(AccountValidationContext context); + + /// + /// Verifies whether the string is a valid email. + /// + /// The object describing the context of the validation. + /// true if the context contains a valid UserName, false otherwise. + bool ValidateEmail(AccountValidationContext context); + + } +} diff --git a/src/Orchard/Security/NullAccountValidationService.cs b/src/Orchard/Security/NullAccountValidationService.cs new file mode 100644 index 000000000..5c9945c29 --- /dev/null +++ b/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 { + /// + /// Provides a default implementation of IAccountValidationService 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. + /// + 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(); + } + } +}