From 0858e04345b58d183d04f32a03d6ff47cc9dd010 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 15 Jun 2017 00:46:23 +0200 Subject: [PATCH] tidy up how a user is created when invited, ensure the special empty password is used. --- src/Umbraco.Core/Constants-Security.cs | 2 + .../Models/Identity/IdentityModelMappings.cs | 2 +- src/Umbraco.Core/Models/Membership/User.cs | 3 +- .../Security/BackOfficeUserStore.cs | 43 ++++++++----------- .../users/views/users/users.controller.js | 4 +- src/Umbraco.Web/Editors/UsersController.cs | 28 +++++++++--- .../Models/Mapping/UserModelMapper.cs | 7 +-- 7 files changed, 48 insertions(+), 41 deletions(-) diff --git a/src/Umbraco.Core/Constants-Security.cs b/src/Umbraco.Core/Constants-Security.cs index d5e3b9242f..283f1e8315 100644 --- a/src/Umbraco.Core/Constants-Security.cs +++ b/src/Umbraco.Core/Constants-Security.cs @@ -16,6 +16,8 @@ namespace Umbraco.Core public const string BackOfficeTokenAuthenticationType = "UmbracoBackOfficeToken"; public const string BackOfficeTwoFactorAuthenticationType = "UmbracoTwoFactorCookie"; + internal const string EmptyPasswordPrefix = "___UIDEMPTYPWORD__"; + /// /// The prefix used for external identity providers for their authentication type /// diff --git a/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs b/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs index d48e2e6318..c01f1653dd 100644 --- a/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs +++ b/src/Umbraco.Core/Models/Identity/IdentityModelMappings.cs @@ -42,7 +42,7 @@ namespace Umbraco.Core.Models.Identity private string GetPasswordHash(string storedPass) { - return storedPass.StartsWith("___UIDEMPTYPWORD__") ? null : storedPass; + return storedPass.StartsWith(Constants.Security.EmptyPasswordPrefix) ? null : storedPass; } } } \ No newline at end of file diff --git a/src/Umbraco.Core/Models/Membership/User.cs b/src/Umbraco.Core/Models/Membership/User.cs index 70fb643bfb..8014da9b05 100644 --- a/src/Umbraco.Core/Models/Membership/User.cs +++ b/src/Umbraco.Core/Models/Membership/User.cs @@ -46,7 +46,7 @@ namespace Umbraco.Core.Models.Membership if (string.IsNullOrWhiteSpace(name)) throw new ArgumentException("Value cannot be null or whitespace.", "name"); if (string.IsNullOrWhiteSpace(email)) throw new ArgumentException("Value cannot be null or whitespace.", "email"); if (string.IsNullOrWhiteSpace(username)) throw new ArgumentException("Value cannot be null or whitespace.", "username"); - if (string.IsNullOrWhiteSpace(rawPasswordValue)) throw new ArgumentException("Value cannot be null or whitespace.", "rawPasswordValue"); + if (string.IsNullOrEmpty(rawPasswordValue)) throw new ArgumentException("Value cannot be null or empty.", "rawPasswordValue"); _name = name; _email = email; @@ -57,6 +57,7 @@ namespace Umbraco.Core.Models.Membership _isLockedOut = false; _startContentIds = new int[] { }; _startMediaIds = new int[] { }; + } /// diff --git a/src/Umbraco.Core/Security/BackOfficeUserStore.cs b/src/Umbraco.Core/Security/BackOfficeUserStore.cs index 42873a02e2..24477d241a 100644 --- a/src/Umbraco.Core/Security/BackOfficeUserStore.cs +++ b/src/Umbraco.Core/Security/BackOfficeUserStore.cs @@ -68,32 +68,25 @@ namespace Umbraco.Core.Security ThrowIfDisposed(); if (user == null) throw new ArgumentNullException("user"); - var member = new User - { - DefaultToLiveEditing = false, - Email = user.Email, - Language = user.Culture ?? Configuration.GlobalSettings.DefaultUILanguage, - Name = user.Name, - Username = user.UserName, - StartContentIds = user.StartContentIds ?? new int[] { }, - StartMediaIds = user.StartMediaIds ?? new int[] { }, - IsLockedOut = user.IsLockedOut, - IsApproved = true - }; - - UpdateMemberProperties(member, user); - //the password must be 'something' it could be empty if authenticating // with an external provider so we'll just generate one and prefix it, the // prefix will help us determine if the password hasn't actually been specified yet. - if (member.RawPasswordValue.IsNullOrWhiteSpace()) - { - //this will hash the guid with a salt so should be nicely random - var aspHasher = new PasswordHasher(); - member.RawPasswordValue = "___UIDEMPTYPWORD__" + - aspHasher.HashPassword(Guid.NewGuid().ToString("N")); + //this will hash the guid with a salt so should be nicely random + var aspHasher = new PasswordHasher(); + var emptyPasswordValue = Constants.Security.EmptyPasswordPrefix + + aspHasher.HashPassword(Guid.NewGuid().ToString("N")); - } + var member = new User(user.Name, user.Email, user.UserName, emptyPasswordValue) + { + DefaultToLiveEditing = false, + Language = user.Culture ?? Configuration.GlobalSettings.DefaultUILanguage, + StartContentIds = user.StartContentIds ?? new int[] { }, + StartMediaIds = user.StartMediaIds ?? new int[] { }, + IsLockedOut = user.IsLockedOut, + }; + + UpdateMemberProperties(member, user); + _userService.Save(member); if (member.Id == 0) throw new DataException("Could not create the user, check logs for details"); @@ -206,7 +199,7 @@ namespace Umbraco.Core.Security { ThrowIfDisposed(); if (user == null) throw new ArgumentNullException("user"); - if (passwordHash.IsNullOrWhiteSpace()) throw new ArgumentNullException("passwordHash"); + if (string.IsNullOrEmpty(passwordHash)) throw new ArgumentException("Value cannot be null or empty.", "passwordHash"); user.PasswordHash = passwordHash; @@ -222,7 +215,7 @@ namespace Umbraco.Core.Security { ThrowIfDisposed(); if (user == null) throw new ArgumentNullException("user"); - + return Task.FromResult(user.PasswordHash); } @@ -236,7 +229,7 @@ namespace Umbraco.Core.Security ThrowIfDisposed(); if (user == null) throw new ArgumentNullException("user"); - return Task.FromResult(user.PasswordHash.IsNullOrWhiteSpace() == false); + return Task.FromResult(string.IsNullOrEmpty(user.PasswordHash) == false); } /// diff --git a/src/Umbraco.Web.UI.Client/src/views/users/views/users/users.controller.js b/src/Umbraco.Web.UI.Client/src/views/users/views/users/users.controller.js index 4717d2b3c9..2eb0b0a406 100644 --- a/src/Umbraco.Web.UI.Client/src/views/users/views/users/users.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/users/views/users/users.controller.js @@ -280,9 +280,7 @@ contentEditingHelper.contentEditorPerformSave({ statusMessage: localizeSaving, - saveMethod: function(obj, isCreate, files) { - usersResource.inviteUser(a, b, c); - }, + saveMethod: usersResource.inviteUser, scope: $scope, content: vm.newUser, // We do not redirect on failure for users - this is because it is not possible to actually save a user diff --git a/src/Umbraco.Web/Editors/UsersController.cs b/src/Umbraco.Web/Editors/UsersController.cs index 9ed8296b5d..ae20f4fd14 100644 --- a/src/Umbraco.Web/Editors/UsersController.cs +++ b/src/Umbraco.Web/Editors/UsersController.cs @@ -270,7 +270,7 @@ namespace Umbraco.Web.Editors throw new HttpResponseException( Request.CreateNotificationValidationErrorResponse("No Email server is configured")); } - + var user = Services.UserService.GetByEmail(userSave.Email); if (user != null && (user.LastLoginDate != default(DateTime) || user.EmailConfirmedDate.HasValue)) { @@ -278,13 +278,29 @@ namespace Umbraco.Web.Editors throw new HttpResponseException(Request.CreateErrorResponse(HttpStatusCode.BadRequest, ModelState)); } - //map to new user or map to existing user (if they've already been invited but didn't accept it) - user = user == null ? Mapper.Map(userSave) : Mapper.Map(userSave, user); + if (user == null) + { + //we want to create the user with the UserManager, this ensures the 'empty' (special) password + //format is applied without us having to duplicate that logic + var created = await UserManager.CreateAsync(new BackOfficeIdentityUser + { + Email = userSave.Email, + Name = userSave.Name, + UserName = userSave.Email + }); + if (created.Succeeded == false) + { + throw new HttpResponseException( + Request.CreateNotificationValidationErrorResponse(string.Join(", ", created.Errors))); + } - //This is the default but just ensure it's true, until they click on the invite and set their password they will not be approved - //and therefore not authenticated to the back office - user.IsApproved = false; + //now re-look the user back up + user = Services.UserService.GetByEmail(userSave.Email); + } + //map the save info over onto the user + user = Mapper.Map(userSave, user); + //Save the user first Services.UserService.Save(user); var display = Mapper.Map(user); diff --git a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs index 46e60fc8eb..e5baa81fac 100644 --- a/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs +++ b/src/Umbraco.Web/Models/Mapping/UserModelMapper.cs @@ -36,11 +36,8 @@ namespace Umbraco.Web.Models.Mapping } }); - config.CreateMap() - .ConstructUsing(invite => new User(invite.Name, invite.Email, invite.Email, Guid.NewGuid().ToString("N"))) - .IgnoreAllUnmapped() - //generate a new token - .ForMember(user => user.SecurityStamp, expression => expression.MapFrom(x => (DateTime.Now + x.Email).ToSHA1())) + config.CreateMap() + .IgnoreAllUnmapped() //all invited users will not be approved, completing the invite will approve the user .ForMember(user => user.IsApproved, expression => expression.UseValue(false)) .AfterMap((invite, user) =>