From 056b13eb986912983b8a7501edb3ab663f9c7302 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 8 Jan 2014 16:29:09 +1100 Subject: [PATCH 1/2] fixes up the double member creation in the createuser method --- .../Security/MembershipProviderBase.cs | 43 +++++++++++++------ .../Security/UmbracoMembershipProviderBase.cs | 9 +++- src/Umbraco.Core/Services/MemberService.cs | 4 ++ src/umbraco.businesslogic/User.cs | 18 ++++++++ 4 files changed, 59 insertions(+), 15 deletions(-) diff --git a/src/Umbraco.Core/Security/MembershipProviderBase.cs b/src/Umbraco.Core/Security/MembershipProviderBase.cs index 71fb729b78..a9475b526f 100644 --- a/src/Umbraco.Core/Security/MembershipProviderBase.cs +++ b/src/Umbraco.Core/Security/MembershipProviderBase.cs @@ -389,52 +389,69 @@ namespace Umbraco.Core.Security /// Ensures the ValidatingPassword event is executed before executing PerformCreateUser and performs basic membership provider validation of values. /// public sealed override MembershipUser CreateUser(string username, string password, string email, string passwordQuestion, string passwordAnswer, bool isApproved, object providerUserKey, out MembershipCreateStatus status) + { + var valStatus = ValidateNewUser(username, password, email, passwordQuestion, passwordAnswer, isApproved, providerUserKey); + if (valStatus != MembershipCreateStatus.Success) + { + status = valStatus; + return null; + } + + return PerformCreateUser(username, password, email, passwordQuestion, passwordAnswer, isApproved, providerUserKey, out status); + } + + /// + /// Performs the validation of the information for creating a new user + /// + /// + /// + /// + /// + /// + /// + /// + /// + protected MembershipCreateStatus ValidateNewUser(string username, string password, string email, string passwordQuestion, string passwordAnswer, bool isApproved, object providerUserKey) { var args = new ValidatePasswordEventArgs(username, password, true); OnValidatingPassword(args); if (args.Cancel) { - status = MembershipCreateStatus.InvalidPassword; - return null; + return MembershipCreateStatus.InvalidPassword; } // Validate password var passwordValidAttempt = IsPasswordValid(password, MinRequiredNonAlphanumericCharacters, PasswordStrengthRegularExpression, MinRequiredPasswordLength); if (passwordValidAttempt.Success == false) { - status = MembershipCreateStatus.InvalidPassword; - return null; + return MembershipCreateStatus.InvalidPassword; } // Validate email if (IsEmailValid(email) == false) { - status = MembershipCreateStatus.InvalidEmail; - return null; + return MembershipCreateStatus.InvalidEmail; } // Make sure username isn't all whitespace if (string.IsNullOrWhiteSpace(username.Trim())) { - status = MembershipCreateStatus.InvalidUserName; - return null; + return MembershipCreateStatus.InvalidUserName; } // Check password question if (string.IsNullOrWhiteSpace(passwordQuestion) && RequiresQuestionAndAnswer) { - status = MembershipCreateStatus.InvalidQuestion; - return null; + return MembershipCreateStatus.InvalidQuestion; } // Check password answer if (string.IsNullOrWhiteSpace(passwordAnswer) && RequiresQuestionAndAnswer) { - status = MembershipCreateStatus.InvalidAnswer; - return null; + return MembershipCreateStatus.InvalidAnswer; } - return PerformCreateUser(username, password, email, passwordQuestion, passwordAnswer, isApproved, providerUserKey, out status); + return MembershipCreateStatus.Success; } /// diff --git a/src/Umbraco.Core/Security/UmbracoMembershipProviderBase.cs b/src/Umbraco.Core/Security/UmbracoMembershipProviderBase.cs index 2b0b128b1c..6cbd12f448 100644 --- a/src/Umbraco.Core/Security/UmbracoMembershipProviderBase.cs +++ b/src/Umbraco.Core/Security/UmbracoMembershipProviderBase.cs @@ -48,8 +48,13 @@ namespace Umbraco.Core.Security public MembershipUser CreateUser(string memberTypeAlias, string username, string password, string email, string passwordQuestion, string passwordAnswer, bool isApproved, object providerUserKey, out MembershipCreateStatus status) { //do the base validation first - base.CreateUser(username, password, email, passwordQuestion, passwordAnswer, isApproved, providerUserKey, out status); - + var valStatus = ValidateNewUser(username, password, email, passwordQuestion, passwordAnswer, isApproved, providerUserKey); + if (valStatus != MembershipCreateStatus.Success) + { + status = valStatus; + return null; + } + return PerformCreateUser(memberTypeAlias, username, password, email, passwordQuestion, passwordAnswer, isApproved, providerUserKey, out status); } diff --git a/src/Umbraco.Core/Services/MemberService.cs b/src/Umbraco.Core/Services/MemberService.cs index 83a1bfc050..d82a429662 100644 --- a/src/Umbraco.Core/Services/MemberService.cs +++ b/src/Umbraco.Core/Services/MemberService.cs @@ -546,6 +546,10 @@ namespace Umbraco.Core.Services { repository.AddOrUpdate(member); uow.Commit(); + + //insert the xml + var xml = member.ToXml(); + CreateAndSaveMemberXml(xml, member.Id, uow.Database); } return member; diff --git a/src/umbraco.businesslogic/User.cs b/src/umbraco.businesslogic/User.cs index ab2f6b110c..e2dd21d104 100644 --- a/src/umbraco.businesslogic/User.cs +++ b/src/umbraco.businesslogic/User.cs @@ -4,6 +4,7 @@ using System.Web.Caching; using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Logging; +using Umbraco.Core.Models.Membership; using Umbraco.Core.Models.Rdbms; using umbraco.DataLayer; using System.Collections.Generic; @@ -39,6 +40,23 @@ namespace umbraco.BusinessLogic get { return Application.SqlHelper; } } + internal User(IUser user) + { + _id = (int)user.Id; + _userNoConsole = user.IsLockedOut; + _userDisabled = user.IsApproved; + _name = user.Name; + _loginname = user.Username; + _email = user.Email; + _language = user.Language; + _startnodeid = user.StartContentId; + _startmediaid = user.StartMediaId; + //this is cached, so should be 'ok' + _usertype = UserType.GetUserType(user.UserType.Id); + + _isInitialized = true; + } + /// /// Initializes a new instance of the class. /// From cdbf89ee570d481f479bf5eb00ae8220c8667457 Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 8 Jan 2014 17:08:13 +1100 Subject: [PATCH 2/2] Fixes up the unit tests --- src/Umbraco.Core/Properties/AssemblyInfo.cs | 5 ++++- src/Umbraco.Tests/BusinessLogic/ApplicationTest.cs | 3 ++- .../UmbracoServiceMembershipProviderTests.cs | 14 +++++++------- .../Persistence/Repositories/UserRepositoryTest.cs | 5 +++-- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/src/Umbraco.Core/Properties/AssemblyInfo.cs b/src/Umbraco.Core/Properties/AssemblyInfo.cs index 641fde43a1..bd7d67f6af 100644 --- a/src/Umbraco.Core/Properties/AssemblyInfo.cs +++ b/src/Umbraco.Core/Properties/AssemblyInfo.cs @@ -44,4 +44,7 @@ using System.Security.Permissions; [assembly: InternalsVisibleTo("Concorde.Sync")] [assembly: InternalsVisibleTo("Umbraco.Belle")] [assembly: InternalsVisibleTo("Umbraco.VisualStudio")] -[assembly: InternalsVisibleTo("umbraco.providers")] \ No newline at end of file +[assembly: InternalsVisibleTo("umbraco.providers")] + +//allow this to be mocked in our unit tests +[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2")] \ No newline at end of file diff --git a/src/Umbraco.Tests/BusinessLogic/ApplicationTest.cs b/src/Umbraco.Tests/BusinessLogic/ApplicationTest.cs index 1bcd405c67..60a316b510 100644 --- a/src/Umbraco.Tests/BusinessLogic/ApplicationTest.cs +++ b/src/Umbraco.Tests/BusinessLogic/ApplicationTest.cs @@ -1,4 +1,5 @@ using NUnit.Framework; +using Umbraco.Tests.TestHelpers; using umbraco.BusinessLogic; using System; using System.Linq; @@ -11,7 +12,7 @@ namespace Umbraco.Tests.BusinessLogic ///to contain all ApplicationTest Unit Tests /// [TestFixture()] - public class ApplicationTest : BaseTest + public class ApplicationTest : BaseDatabaseFactoryTest { /// diff --git a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs index 183e5b0e4f..35092f6bb6 100644 --- a/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs +++ b/src/Umbraco.Tests/Membership/UmbracoServiceMembershipProviderTests.cs @@ -59,7 +59,7 @@ namespace Umbraco.Tests.Membership provider.Initialize("test", new NameValueCollection()); MembershipCreateStatus status; - var user = provider.CreateUser("test", "test", "test", "test@test.com", "test", "test", true, "test", out status); + var user = provider.CreateUser("test", "test", "testtest$1", "test@test.com", "test", "test", true, "test", out status); Assert.IsNull(user); } @@ -75,7 +75,7 @@ namespace Umbraco.Tests.Membership provider.Initialize("test", new NameValueCollection { { "requiresUniqueEmail", "true" } }); MembershipCreateStatus status; - var user = provider.CreateUser("test", "test", "test", "test@test.com", "test", "test", true, "test", out status); + var user = provider.CreateUser("test", "test", "testtest$1", "test@test.com", "test", "test", true, "test", out status); Assert.IsNull(user); } @@ -105,7 +105,7 @@ namespace Umbraco.Tests.Membership MembershipCreateStatus status; - provider.CreateUser("test", "test", "test", "test@test.com", "test", "test", true, "test", out status); + provider.CreateUser("test", "test", "testtest$1", "test@test.com", "test", "test", true, "test", out status); Assert.AreNotEqual("test", createdMember.PasswordAnswer); Assert.AreEqual(provider.EncryptString("test"), createdMember.PasswordAnswer); @@ -137,11 +137,11 @@ namespace Umbraco.Tests.Membership MembershipCreateStatus status; - provider.CreateUser("test", "test", "test", "test@test.com", "test", "test", true, "test", out status); + provider.CreateUser("test", "test", "testtest$1", "test@test.com", "test", "test", true, "test", out status); Assert.AreNotEqual("test", createdMember.Password); var decrypted = provider.DecryptPassword(createdMember.Password); - Assert.AreEqual("test", decrypted); + Assert.AreEqual("testtest$1", decrypted); } [Test] @@ -170,13 +170,13 @@ namespace Umbraco.Tests.Membership MembershipCreateStatus status; - provider.CreateUser("test", "test", "test", "test@test.com", "test", "test", true, "test", out status); + provider.CreateUser("test", "test", "testtest$1", "test@test.com", "test", "test", true, "test", out status); Assert.AreNotEqual("test", createdMember.Password); string salt; var storedPassword = provider.StoredPassword(createdMember.Password, out salt); - var hashedPassword = provider.EncryptOrHashPassword("test", salt); + var hashedPassword = provider.EncryptOrHashPassword("testtest$1", salt); Assert.AreEqual(hashedPassword, storedPassword); } diff --git a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs index 6a4ad7de12..45ae1d4744 100644 --- a/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs +++ b/src/Umbraco.Tests/Persistence/Repositories/UserRepositoryTest.cs @@ -134,7 +134,8 @@ namespace Umbraco.Tests.Persistence.Repositories var resolved = (User)repository.Get((int)user.Id); resolved.Name = "New Name"; - resolved.DefaultPermissions = new[]{"Z", "Y", "X"}; + //the db column is not used, default permissions are taken from the user type's permissions, this is a getter only + //resolved.DefaultPermissions = "ZYX"; resolved.Language = "fr"; resolved.IsApproved = false; resolved.Password = "new"; @@ -153,7 +154,7 @@ namespace Umbraco.Tests.Persistence.Repositories // Assert Assert.That(updatedItem.Id, Is.EqualTo(resolved.Id)); Assert.That(updatedItem.Name, Is.EqualTo(resolved.Name)); - Assert.That(updatedItem.DefaultPermissions, Is.EqualTo(resolved.DefaultPermissions)); + //Assert.That(updatedItem.DefaultPermissions, Is.EqualTo(resolved.DefaultPermissions)); Assert.That(updatedItem.Language, Is.EqualTo(resolved.Language)); Assert.That(updatedItem.IsApproved, Is.EqualTo(resolved.IsApproved)); Assert.That(updatedItem.Password, Is.EqualTo(resolved.Password));