From f999b9d859856604233b752a1cf08282e382e5e2 Mon Sep 17 00:00:00 2001 From: Kevin LaBranche Date: Mon, 17 Jan 2011 16:14:05 -0700 Subject: [PATCH 1/5] Fix for http://orchard.codeplex.com/workitem/17054. Email address is not validated for a registered user. Added code to the AccountController's ValidateRegistration to fix. Also added two unit tests and went ahead and added the data annotation's to the UserCreateViewModel and the UserEditViewModel as well since if we are accepting email addresses they might as well be valid (well-formed). :-) --HG-- branch : 1.x --- .../Controllers/AccountControllerTests.cs | 35 +++++++++++++++++++ .../Controllers/AccountController.cs | 8 +++++ .../ViewModels/UserCreateViewModel.cs | 1 + .../ViewModels/UserEditViewModel.cs | 1 + 4 files changed, 45 insertions(+) diff --git a/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs b/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs index 62d644d1d..944525a21 100644 --- a/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs +++ b/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs @@ -152,6 +152,41 @@ namespace Orchard.Tests.Modules.Users.Controllers { Assert.That(result, Is.TypeOf()); } + [Test] + public void UsersShouldNotBeAbleToRegisterIfInvalidEmail() + { + + var registrationSettings = _container.Resolve().GetContext().CurrentSite.As(); + registrationSettings.UsersCanRegister = true; + registrationSettings.UsersAreModerated = false; + registrationSettings.UsersMustValidateEmail = false; + + _session.Flush(); + + _controller.ModelState.Clear(); + var result = _controller.Register("bar", "notanemailaddress", "66554321", "66554321"); + + Assert.That(((ViewResult)result).ViewData.ModelState.Count == 1,"Invalid email address."); + } + + [Test] + public void UsersShouldBeAbleToRegisterIfValidEmail() + { + + var registrationSettings = _container.Resolve().GetContext().CurrentSite.As(); + registrationSettings.UsersCanRegister = true; + registrationSettings.UsersAreModerated = false; + registrationSettings.UsersMustValidateEmail = false; + + _session.Flush(); + + _controller.ModelState.Clear(); + var result = _controller.Register("bar", "t@t.com", "password", "password"); + + Assert.That(result, Is.TypeOf()); + Assert.That(((RedirectResult)result).Url, Is.EqualTo("~/")); + } + [Test] public void RegisteredUserShouldBeRedirectedToHomePage() { diff --git a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs index 7da36e3f0..b135cc68d 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/Controllers/AccountController.cs @@ -1,4 +1,5 @@ using System; +using System.Text.RegularExpressions; using System.Diagnostics.CodeAnalysis; using Orchard.Localization; using System.Security.Principal; @@ -317,6 +318,8 @@ namespace Orchard.Users.Controllers { private bool ValidateRegistration(string userName, string email, string password, string confirmPassword) { bool validate = true; + Regex isValidEmail = new Regex("^[a-z0-9_\\+-]+(\\.[a-z0-9_\\+-]+)*@[a-z0-9-]+(\\.[a-z0-9-]+)*\\.([a-z]{2,4})$"); + if (String.IsNullOrEmpty(userName)) { ModelState.AddModelError("username", T("You must specify a username.")); validate = false; @@ -326,6 +329,11 @@ namespace Orchard.Users.Controllers { validate = false; } + if (!isValidEmail.IsMatch(email)) { + ModelState.AddModelError("email", T("You must specify a valid email address.")); + validate = false; + } + if (!validate) return false; diff --git a/src/Orchard.Web/Modules/Orchard.Users/ViewModels/UserCreateViewModel.cs b/src/Orchard.Web/Modules/Orchard.Users/ViewModels/UserCreateViewModel.cs index d7d640774..a333c1047 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/ViewModels/UserCreateViewModel.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/ViewModels/UserCreateViewModel.cs @@ -7,6 +7,7 @@ namespace Orchard.Users.ViewModels { public string UserName { get; set; } [Required, DataType(DataType.EmailAddress)] + [RegularExpression("^[a-z0-9_\\+-]+(\\.[a-z0-9_\\+-]+)*@[a-z0-9-]+(\\.[a-z0-9-]+)*\\.([a-z]{2,4})$")] public string Email { get; set; } [Required, DataType(DataType.Password)] diff --git a/src/Orchard.Web/Modules/Orchard.Users/ViewModels/UserEditViewModel.cs b/src/Orchard.Web/Modules/Orchard.Users/ViewModels/UserEditViewModel.cs index 991ad0cee..5dd9aa8f4 100644 --- a/src/Orchard.Web/Modules/Orchard.Users/ViewModels/UserEditViewModel.cs +++ b/src/Orchard.Web/Modules/Orchard.Users/ViewModels/UserEditViewModel.cs @@ -11,6 +11,7 @@ namespace Orchard.Users.ViewModels { } [Required] + [RegularExpression("^[a-z0-9_\\+-]+(\\.[a-z0-9_\\+-]+)*@[a-z0-9-]+(\\.[a-z0-9-]+)*\\.([a-z]{2,4})$")] public string Email { get { return User.As().Record.Email; } set { User.As().Record.Email = value; } From ebc9667649b3668782bb105048e045203bf1383c Mon Sep 17 00:00:00 2001 From: Suha Can Date: Tue, 18 Jan 2011 11:06:07 -0800 Subject: [PATCH 2/5] Minor code convention tweak to a recently contributed unit test. --HG-- branch : 1.x --- .../Users/Controllers/AccountControllerTests.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs b/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs index 944525a21..f7d78bee6 100644 --- a/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs +++ b/src/Orchard.Tests.Modules/Users/Controllers/AccountControllerTests.cs @@ -153,8 +153,7 @@ namespace Orchard.Tests.Modules.Users.Controllers { } [Test] - public void UsersShouldNotBeAbleToRegisterIfInvalidEmail() - { + public void UsersShouldNotBeAbleToRegisterIfInvalidEmail() { var registrationSettings = _container.Resolve().GetContext().CurrentSite.As(); registrationSettings.UsersCanRegister = true; @@ -170,8 +169,7 @@ namespace Orchard.Tests.Modules.Users.Controllers { } [Test] - public void UsersShouldBeAbleToRegisterIfValidEmail() - { + public void UsersShouldBeAbleToRegisterIfValidEmail() { var registrationSettings = _container.Resolve().GetContext().CurrentSite.As(); registrationSettings.UsersCanRegister = true; From e5fa2c097fdf6a204aec37463b41cf878d50bf94 Mon Sep 17 00:00:00 2001 From: bertrandleroy Date: Tue, 18 Jan 2011 16:16:26 -0800 Subject: [PATCH 3/5] Fixing typo --HG-- branch : 1.x --- .../Modules/Orchard.Modules/Controllers/AdminController.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs b/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs index 8fe28191c..cbf8d0b68 100644 --- a/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs +++ b/src/Orchard.Web/Modules/Orchard.Modules/Controllers/AdminController.cs @@ -110,7 +110,7 @@ namespace Orchard.Modules.Controllers { try { _reportsCoordinator.Register("Data Migration", "Upgrade " + id, "Orchard installation"); _dataMigrationManager.Update(id); - Services.Notifier.Information(T("The feature {0} was updated succesfuly", id)); + Services.Notifier.Information(T("The feature {0} was updated successfully", id)); } catch (Exception ex) { Services.Notifier.Error(T("An error occured while updating the feature {0}: {1}", id, ex.Message)); From 19f34c0fce157b7b2a11c9aab1b88f4dd2120d7e Mon Sep 17 00:00:00 2001 From: freeflying1222 Date: Sat, 29 Jan 2011 19:37:22 +0800 Subject: [PATCH 4/5] Refactor: There are duplicate code blocks for getting area from "Route" in ShellRoute.cs and OrchardControllerFactory.cs. And I don't think it's a good idea assigning the responsibility to OrchardControllerFactory.cs --HG-- branch : 1.x --- src/Orchard/Mvc/Extensions/RouteExtention.cs | 35 ++++++++++++++++++++ src/Orchard/Mvc/OrchardControllerFactory.cs | 24 +------------- src/Orchard/Mvc/Routes/ShellRoute.cs | 10 +----- src/Orchard/Orchard.Framework.csproj | 1 + 4 files changed, 38 insertions(+), 32 deletions(-) create mode 100644 src/Orchard/Mvc/Extensions/RouteExtention.cs diff --git a/src/Orchard/Mvc/Extensions/RouteExtention.cs b/src/Orchard/Mvc/Extensions/RouteExtention.cs new file mode 100644 index 000000000..882f64989 --- /dev/null +++ b/src/Orchard/Mvc/Extensions/RouteExtention.cs @@ -0,0 +1,35 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Web.Routing; +using System.Web.Mvc; + +namespace Orchard.Mvc{ + + public static class RouteExtention{ + public static string GetAreaName(this RouteBase route){ + var routeWithArea = route as IRouteWithArea; + if (routeWithArea != null) { + return routeWithArea.Area; + } + + var castRoute = route as Route; + if (castRoute != null && castRoute.DataTokens != null) { + return castRoute.DataTokens["area"] as string; + } + + return null; + } + + public static string GetAreaName(this RouteData routeData){ + object area; + if (routeData.DataTokens.TryGetValue("area", out area)) + { + return area as string; + } + + return GetAreaName(routeData.Route); + } + } +} diff --git a/src/Orchard/Mvc/OrchardControllerFactory.cs b/src/Orchard/Mvc/OrchardControllerFactory.cs index e0330a326..5dc828412 100644 --- a/src/Orchard/Mvc/OrchardControllerFactory.cs +++ b/src/Orchard/Mvc/OrchardControllerFactory.cs @@ -30,7 +30,7 @@ namespace Orchard.Mvc { var routeData = requestContext.RouteData; // Determine the area name for the request, and fall back to stock orchard controllers - var areaName = GetAreaName(routeData); + var areaName = routeData.GetAreaName(); // Service name pattern matches the identification strategy var serviceKey = (areaName + "/" + controllerName).ToLowerInvariant(); @@ -57,27 +57,5 @@ namespace Orchard.Mvc { return null; } - public static string GetAreaName(RouteBase route) { - var routeWithArea = route as IRouteWithArea; - if (routeWithArea != null) { - return routeWithArea.Area; - } - - var castRoute = route as Route; - if (castRoute != null && castRoute.DataTokens != null) { - return castRoute.DataTokens["area"] as string; - } - - return null; - } - - public static string GetAreaName(RouteData routeData) { - object area; - if (routeData.DataTokens.TryGetValue("area", out area)) { - return area as string; - } - - return GetAreaName(routeData.Route); - } } } \ No newline at end of file diff --git a/src/Orchard/Mvc/Routes/ShellRoute.cs b/src/Orchard/Mvc/Routes/ShellRoute.cs index f8e5a540b..0e7e50083 100644 --- a/src/Orchard/Mvc/Routes/ShellRoute.cs +++ b/src/Orchard/Mvc/Routes/ShellRoute.cs @@ -25,15 +25,7 @@ namespace Orchard.Mvc.Routes { if (!string.IsNullOrEmpty(_shellSettings.RequestUrlPrefix)) _urlPrefix = new UrlPrefix(_shellSettings.RequestUrlPrefix); - var routeWithArea = route as IRouteWithArea; - if (routeWithArea != null) { - Area = routeWithArea.Area; - } - - var routeWithDataTokens = route as Route; - if ((routeWithDataTokens != null) && (routeWithDataTokens.DataTokens != null)) { - Area = (routeWithDataTokens.DataTokens["area"] as string); - } + Area = route.GetAreaName(); } public string ShellSettingsName { get { return _shellSettings.Name; } } diff --git a/src/Orchard/Orchard.Framework.csproj b/src/Orchard/Orchard.Framework.csproj index be5812871..df5cdec31 100644 --- a/src/Orchard/Orchard.Framework.csproj +++ b/src/Orchard/Orchard.Framework.csproj @@ -183,6 +183,7 @@ + From fdc69727458940ac1fd6c63399a5749b2c85322b Mon Sep 17 00:00:00 2001 From: Suha Can Date: Mon, 31 Jan 2011 10:43:44 -0800 Subject: [PATCH 5/5] Minor alteration to previous contribution --HG-- branch : 1.x --- .../{RouteExtention.cs => RouteExtension.cs} | 14 ++++---------- src/Orchard/Mvc/OrchardControllerFactory.cs | 1 + src/Orchard/Mvc/Routes/ShellRoute.cs | 1 + src/Orchard/Orchard.Framework.csproj | 2 +- 4 files changed, 7 insertions(+), 11 deletions(-) rename src/Orchard/Mvc/Extensions/{RouteExtention.cs => RouteExtension.cs} (76%) diff --git a/src/Orchard/Mvc/Extensions/RouteExtention.cs b/src/Orchard/Mvc/Extensions/RouteExtension.cs similarity index 76% rename from src/Orchard/Mvc/Extensions/RouteExtention.cs rename to src/Orchard/Mvc/Extensions/RouteExtension.cs index 882f64989..cac69c3ce 100644 --- a/src/Orchard/Mvc/Extensions/RouteExtention.cs +++ b/src/Orchard/Mvc/Extensions/RouteExtension.cs @@ -1,13 +1,8 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Web.Routing; +using System.Web.Routing; using System.Web.Mvc; -namespace Orchard.Mvc{ - - public static class RouteExtention{ +namespace Orchard.Mvc.Extensions { + public static class RouteExtension{ public static string GetAreaName(this RouteBase route){ var routeWithArea = route as IRouteWithArea; if (routeWithArea != null) { @@ -24,8 +19,7 @@ namespace Orchard.Mvc{ public static string GetAreaName(this RouteData routeData){ object area; - if (routeData.DataTokens.TryGetValue("area", out area)) - { + if (routeData.DataTokens.TryGetValue("area", out area)) { return area as string; } diff --git a/src/Orchard/Mvc/OrchardControllerFactory.cs b/src/Orchard/Mvc/OrchardControllerFactory.cs index 5dc828412..6bf9c2d78 100644 --- a/src/Orchard/Mvc/OrchardControllerFactory.cs +++ b/src/Orchard/Mvc/OrchardControllerFactory.cs @@ -4,6 +4,7 @@ using System.Web.Routing; using Autofac; using Autofac.Core; using Autofac.Features.Metadata; +using Orchard.Mvc.Extensions; namespace Orchard.Mvc { public interface IControllerType { diff --git a/src/Orchard/Mvc/Routes/ShellRoute.cs b/src/Orchard/Mvc/Routes/ShellRoute.cs index 0e7e50083..54882834e 100644 --- a/src/Orchard/Mvc/Routes/ShellRoute.cs +++ b/src/Orchard/Mvc/Routes/ShellRoute.cs @@ -7,6 +7,7 @@ using System.Web.SessionState; using Autofac; using Orchard.Environment; using Orchard.Environment.Configuration; +using Orchard.Mvc.Extensions; namespace Orchard.Mvc.Routes { diff --git a/src/Orchard/Orchard.Framework.csproj b/src/Orchard/Orchard.Framework.csproj index df5cdec31..0a7324e1e 100644 --- a/src/Orchard/Orchard.Framework.csproj +++ b/src/Orchard/Orchard.Framework.csproj @@ -183,7 +183,7 @@ - +