diff --git a/src/Umbraco.Core/Models/ChangingPasswordModel.cs b/src/Umbraco.Core/Models/ChangingPasswordModel.cs index 3816044c0a..c668475275 100644 --- a/src/Umbraco.Core/Models/ChangingPasswordModel.cs +++ b/src/Umbraco.Core/Models/ChangingPasswordModel.cs @@ -1,6 +1,6 @@ -using System.Runtime.Serialization; +using System.Runtime.Serialization; -namespace Umbraco.Web.Models +namespace Umbraco.Core.Models { /// /// A model representing the data required to set a member/user password depending on the provider installed. @@ -20,9 +20,30 @@ namespace Umbraco.Web.Models public string OldPassword { get; set; } /// - /// The id of the user - required to allow changing password without the entire UserSave model + /// The ID of the current user/member requesting the password change + /// For users, required to allow changing password without the entire UserSave model /// [DataMember(Name = "id")] public int Id { get; set; } + + /// + /// The username of the user/member who is changing the password + /// + public string CurrentUsername { get; set; } + + /// + /// The ID of the user/member whose password is being changed + /// + public int SavingUserId { get; set; } + + /// + /// The username of the user/memeber whose password is being changed + /// + public string SavingUsername { get; set; } + + /// + /// True if the current user has access to change the password for the member/user + /// + public bool CurrentUserHasSectionAccess { get; set; } } } diff --git a/src/Umbraco.Core/Models/ContentEditing/UserSave.cs b/src/Umbraco.Core/Models/ContentEditing/UserSave.cs index 2533ebb105..427340700d 100644 --- a/src/Umbraco.Core/Models/ContentEditing/UserSave.cs +++ b/src/Umbraco.Core/Models/ContentEditing/UserSave.cs @@ -1,7 +1,8 @@ -using System.Collections.Generic; +using System.Collections.Generic; using System.ComponentModel.DataAnnotations; using System.Linq; using System.Runtime.Serialization; +using Umbraco.Core.Models; namespace Umbraco.Web.Models.ContentEditing { diff --git a/src/Umbraco.Core/Models/IMemberUserAdapter.cs b/src/Umbraco.Core/Models/IMemberUserAdapter.cs new file mode 100644 index 0000000000..9238ae7da3 --- /dev/null +++ b/src/Umbraco.Core/Models/IMemberUserAdapter.cs @@ -0,0 +1,10 @@ +using System.Collections.Generic; +using Umbraco.Core.Models.Membership; + +namespace Umbraco.Core.Models +{ + public interface IMemberUserAdapter : IUser + { + IEnumerable AllowedSections { get; } + } +} diff --git a/src/Umbraco.Core/Models/MemberUserAdapter.cs b/src/Umbraco.Core/Models/MemberUserAdapter.cs new file mode 100644 index 0000000000..3d5ee7a33a --- /dev/null +++ b/src/Umbraco.Core/Models/MemberUserAdapter.cs @@ -0,0 +1,96 @@ +using System; +using System.Collections.Generic; +using System.ComponentModel; +using Umbraco.Core.Models.Membership; + +namespace Umbraco.Core.Models +{ + public class MemberUserAdapter : IMemberUserAdapter + { + private readonly IMember _member; + + /// + /// Initializes a new instance of the class. + /// Member adaptor to use existing user change password functionality and other shared functions + /// + /// The member to adapt + public MemberUserAdapter(IMember member) + { + _member = member; + } + + // This is the only reason we currently need this adaptor + public IEnumerable AllowedSections => new List() + { + Constants.Applications.Users + }; + + + public UserState UserState { get; } + public string Name { get; set; } + public int SessionTimeout { get; set; } + public int[] StartContentIds { get; set; } + public int[] StartMediaIds { get; set; } + public string Language { get; set; } + public DateTime? InvitedDate { get; set; } + public IEnumerable Groups { get; } + public void RemoveGroup(string @group) => throw new NotImplementedException(); + + public void ClearGroups() => throw new NotImplementedException(); + + public void AddGroup(IReadOnlyUserGroup @group) => throw new NotImplementedException(); + + public IProfile ProfileData { get; } + public string Avatar { get; set; } + public string TourData { get; set; } + public T FromUserCache(string cacheKey) where T : class => throw new NotImplementedException(); + + public void ToUserCache(string cacheKey, T vals) where T : class => throw new NotImplementedException(); + + public object DeepClone() => throw new NotImplementedException(); + + public int Id { get; set; } + public Guid Key { get; set; } + public DateTime CreateDate { get; set; } + public DateTime UpdateDate { get; set; } + public DateTime? DeleteDate { get; set; } + public bool HasIdentity { get; } + public void ResetIdentity() => throw new NotImplementedException(); + + public string Username { get; set; } + public string Email { get; set; } + public DateTime? EmailConfirmedDate { get; set; } + public string RawPasswordValue { get; set; } + public string PasswordConfiguration { get; set; } + public string Comments { get; set; } + public bool IsApproved { get; set; } + public bool IsLockedOut { get; set; } + public DateTime LastLoginDate { get; set; } + public DateTime LastPasswordChangeDate { get; set; } + public DateTime LastLockoutDate { get; set; } + public int FailedPasswordAttempts { get; set; } + public string SecurityStamp { get; set; } + public bool IsDirty() => throw new NotImplementedException(); + + public bool IsPropertyDirty(string propName) => throw new NotImplementedException(); + + public IEnumerable GetDirtyProperties() => throw new NotImplementedException(); + + public void ResetDirtyProperties() => throw new NotImplementedException(); + + public void DisableChangeTracking() => throw new NotImplementedException(); + + public void EnableChangeTracking() => throw new NotImplementedException(); + + public event PropertyChangedEventHandler PropertyChanged; + public bool WasDirty() => throw new NotImplementedException(); + + public bool WasPropertyDirty(string propertyName) => throw new NotImplementedException(); + + public void ResetWereDirtyProperties() => throw new NotImplementedException(); + + public void ResetDirtyProperties(bool rememberDirty) => throw new NotImplementedException(); + + public IEnumerable GetWereDirtyProperties() => throw new NotImplementedException(); + } +} diff --git a/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs b/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs index 9e44855a4a..587b7cd8c5 100644 --- a/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs +++ b/src/Umbraco.Infrastructure/Security/IUmbracoUserManager.cs @@ -263,14 +263,7 @@ namespace Umbraco.Infrastructure.Security /// /// A generated password string GeneratePassword(); - - /// - /// Hashes a password for a null user based on the default password hasher - /// - /// The password to hash - /// The hashed password - string HashPassword(string password); - + /// /// Used to validate the password without an identity user /// Validation code is based on the default ValidatePasswordAsync code diff --git a/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs b/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs index 04560fe45b..528c24ed46 100644 --- a/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs +++ b/src/Umbraco.Infrastructure/Security/UmbracoUserManager.cs @@ -100,19 +100,7 @@ namespace Umbraco.Infrastructure.Security string password = _passwordGenerator.GeneratePassword(); return password; } - - /// - /// Generates a hashed password based on the default password hasher - /// No existing identity user is required and this does not validate the password - /// - /// The password to hash - /// The hashed password - public string HashPassword(string password) - { - string hashedPassword = PasswordHasher.HashPassword(null, password); - return hashedPassword; - } - + /// /// Used to validate the password without an identity user /// Validation code is based on the default ValidatePasswordAsync code diff --git a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs index c69d1365ed..d1c0633472 100644 --- a/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs +++ b/src/Umbraco.Tests.UnitTests/Umbraco.Web.BackOffice/Controllers/MemberControllerUnitTests.cs @@ -20,8 +20,8 @@ using Umbraco.Core.Events; using Umbraco.Core.Mapping; using Umbraco.Core.Models; using Umbraco.Core.Models.ContentEditing; +using Umbraco.Core.Models.Membership; using Umbraco.Core.PropertyEditors; -using Umbraco.Core.PropertyEditors.Validators; using Umbraco.Core.Security; using Umbraco.Core.Serialization; using Umbraco.Core.Services; @@ -33,6 +33,7 @@ using Umbraco.Tests.UnitTests.Umbraco.Core.ShortStringHelper; using Umbraco.Web; using Umbraco.Web.BackOffice.Controllers; using Umbraco.Web.BackOffice.Mapping; +using Umbraco.Web.BackOffice.Security; using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.ContentApps; using Umbraco.Web.Models; @@ -69,11 +70,12 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers IMemberTypeService memberTypeService, IMemberGroupService memberGroupService, IDataTypeService dataTypeService, - IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, + IPasswordChanger passwordChanger) { // arrange Member member = SetupMemberTestData(out MemberSave fakeMemberData, out MemberDisplay memberDisplay, ContentSaveAction.SaveNew); - MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor); + MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor, passwordChanger); sut.ModelState.AddModelError("key", "Invalid model state"); Mock.Get(umbracoMembersUserManager) @@ -105,7 +107,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers IMemberGroupService memberGroupService, IDataTypeService dataTypeService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, - IBackOfficeSecurity backOfficeSecurity) + IBackOfficeSecurity backOfficeSecurity, + IPasswordChanger passwordChanger) { // arrange Member member = SetupMemberTestData(out MemberSave fakeMemberData, out MemberDisplay memberDisplay, ContentSaveAction.SaveNew); @@ -123,7 +126,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers .Returns(() => member); Mock.Get(memberService).Setup(x => x.GetByUsername(It.IsAny())).Returns(() => member); - MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor); + MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor, passwordChanger); // act ActionResult result = await sut.PostSave(fakeMemberData); @@ -143,7 +146,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers IMemberGroupService memberGroupService, IDataTypeService dataTypeService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, - IBackOfficeSecurity backOfficeSecurity) + IBackOfficeSecurity backOfficeSecurity, + IPasswordChanger passwordChanger) { // arrange Member member = SetupMemberTestData(out MemberSave fakeMemberData, out MemberDisplay memberDisplay, ContentSaveAction.SaveNew); @@ -161,7 +165,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers .Returns(() => member); Mock.Get(memberService).Setup(x => x.GetByUsername(It.IsAny())).Returns(() => member); - MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor); + MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor, passwordChanger); // act ActionResult result = await sut.PostSave(fakeMemberData); @@ -182,7 +186,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers IMemberGroupService memberGroupService, IDataTypeService dataTypeService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, - IBackOfficeSecurity backOfficeSecurity) + IBackOfficeSecurity backOfficeSecurity, + IPasswordChanger passwordChanger) { // arrange Member member = SetupMemberTestData(out MemberSave fakeMemberData, out MemberDisplay memberDisplay, ContentSaveAction.Save); @@ -194,9 +199,9 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers .ReturnsAsync(() => IdentityResult.Success); string password = "fakepassword9aw89rnyco3938cyr^%&*()i8Y"; - Mock.Get(umbracoMembersUserManager) - .Setup(x => x.HashPassword(It.IsAny())) - .Returns(password); + Mock.Get(passwordChanger) + .Setup(x => x.ChangePasswordWithIdentityAsync(It.IsAny(), umbracoMembersUserManager)) + .ReturnsAsync(() => new Attempt()); Mock.Get(umbracoMembersUserManager) .Setup(x => x.UpdateAsync(It.IsAny())) .ReturnsAsync(() => IdentityResult.Success); @@ -208,7 +213,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers .Returns(() => null) .Returns(() => member); - MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor); + MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor, passwordChanger); // act ActionResult result = await sut.PostSave(fakeMemberData); @@ -228,7 +233,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers IMemberGroupService memberGroupService, IDataTypeService dataTypeService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, - IBackOfficeSecurity backOfficeSecurity) + IBackOfficeSecurity backOfficeSecurity, + IPasswordChanger passwordChanger) { // arrange Member member = SetupMemberTestData(out MemberSave fakeMemberData, out MemberDisplay memberDisplay, ContentSaveAction.SaveNew); @@ -245,7 +251,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers x => x.GetByEmail(It.IsAny())) .Returns(() => member); - MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor); + MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor, passwordChanger); string reason = "Validation failed"; // act @@ -267,7 +273,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers IMemberGroupService memberGroupService, IDataTypeService dataTypeService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, - IBackOfficeSecurity backOfficeSecurity) + IBackOfficeSecurity backOfficeSecurity, + IPasswordChanger passwordChanger) { // arrange string password = "fakepassword9aw89rnyco3938cyr^%&*()i8Y"; @@ -277,16 +284,16 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers { roleName }; - var membersIdentityUser = new MembersIdentityUser(); + var membersIdentityUser = new MembersIdentityUser(123); Mock.Get(umbracoMembersUserManager) .Setup(x => x.FindByIdAsync(It.IsAny())) .ReturnsAsync(() => membersIdentityUser); Mock.Get(umbracoMembersUserManager) .Setup(x => x.ValidatePasswordAsync(It.IsAny())) .ReturnsAsync(() => IdentityResult.Success); - Mock.Get(umbracoMembersUserManager) - .Setup(x => x.HashPassword(It.IsAny())) - .Returns(password); + Mock.Get(passwordChanger) + .Setup(x => x.ChangePasswordWithIdentityAsync(It.IsAny(), umbracoMembersUserManager)) + .ReturnsAsync(() => Attempt.Succeed(new PasswordChangedModel())); Mock.Get(umbracoMembersUserManager) .Setup(x => x.UpdateAsync(It.IsAny())) .ReturnsAsync(() => IdentityResult.Success); @@ -299,7 +306,7 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers .Returns(() => null) .Returns(() => member); Mock.Get(memberService).Setup(x => x.GetByUsername(It.IsAny())).Returns(() => member); - MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor); + MemberController sut = CreateSut(memberService, memberTypeService, memberGroupService, umbracoMembersUserManager, dataTypeService, backOfficeSecurityAccessor, passwordChanger); // act ActionResult result = await sut.PostSave(fakeMemberData); @@ -309,8 +316,8 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers Assert.IsNotNull(result.Value); Mock.Get(umbracoMembersUserManager) .Verify(u => u.GetRolesAsync(membersIdentityUser)); - Mock.Get(umbracoMembersUserManager) - .Verify(u => u.AddToRolesAsync(membersIdentityUser, new[] { roleName })); + Mock.Get(umbracoMembersUserManager) + .Verify(u => u.AddToRolesAsync(membersIdentityUser, new[] { roleName })); Mock.Get(memberService) .Verify(m => m.Save(It.IsAny(), true)); AssertMemberDisplayPropertiesAreEqual(memberDisplay, result.Value); @@ -325,17 +332,18 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers /// Members user manager /// Data type service /// Back office security accessor + /// Password changer class /// A member controller for the tests private MemberController CreateSut( IMemberService memberService, IMemberTypeService memberTypeService, IMemberGroupService memberGroupService, - IMemberManager membersUserManager, + IUmbracoUserManager membersUserManager, IDataTypeService dataTypeService, - IBackOfficeSecurityAccessor backOfficeSecurityAccessor) + IBackOfficeSecurityAccessor backOfficeSecurityAccessor, + IPasswordChanger mockPasswordChanger) { var mockShortStringHelper = new MockShortStringHelper(); - var textService = new Mock(); var contentTypeBaseServiceProvider = new Mock(); contentTypeBaseServiceProvider.Setup(x => x.GetContentTypeOf(It.IsAny())).Returns(new ContentType(mockShortStringHelper, 123)); @@ -410,13 +418,13 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers _mapper, memberService, memberTypeService, - membersUserManager, + (IMemberManager) membersUserManager, dataTypeService, backOfficeSecurityAccessor, - new ConfigurationEditorJsonSerializer()); + new ConfigurationEditorJsonSerializer(), + mockPasswordChanger); } - /// /// Setup all standard member data for test /// @@ -428,10 +436,10 @@ namespace Umbraco.Tests.UnitTests.Umbraco.Web.BackOffice.Controllers // arrange MemberType memberType = MemberTypeBuilder.CreateSimpleMemberType(); Member member = MemberBuilder.CreateSimpleMember(memberType, "Test Member", "test@example.com", "123", "test"); - int memberId = 123; + var memberId = 123; member.Id = memberId; - //TODO: replace with builder for MemberSave and MemberDisplay + // TODO: replace with builder for MemberSave and MemberDisplay fakeMemberData = new MemberSave() { Id = memberId, diff --git a/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs b/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs index 70b44db586..3383a7578a 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/CurrentUserController.cs @@ -17,6 +17,7 @@ using Umbraco.Core.IO; using Umbraco.Core.Mapping; using Umbraco.Core.Media; using Umbraco.Core.Models; +using Umbraco.Core.Models.Membership; using Umbraco.Core.Security; using Umbraco.Core.Services; using Umbraco.Core.Strings; @@ -42,7 +43,7 @@ namespace Umbraco.Web.BackOffice.Controllers private readonly ContentSettings _contentSettings; private readonly IHostingEnvironment _hostingEnvironment; private readonly IImageUrlGenerator _imageUrlGenerator; - private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; private readonly IUserService _userService; private readonly UmbracoMapper _umbracoMapper; private readonly IBackOfficeUserManager _backOfficeUserManager; @@ -50,6 +51,7 @@ namespace Umbraco.Web.BackOffice.Controllers private readonly ILocalizedTextService _localizedTextService; private readonly AppCaches _appCaches; private readonly IShortStringHelper _shortStringHelper; + private readonly IPasswordChanger _passwordChanger; public CurrentUserController( IMediaFileSystem mediaFileSystem, @@ -63,13 +65,14 @@ namespace Umbraco.Web.BackOffice.Controllers ILoggerFactory loggerFactory, ILocalizedTextService localizedTextService, AppCaches appCaches, - IShortStringHelper shortStringHelper) + IShortStringHelper shortStringHelper, + IPasswordChanger passwordChanger) { _mediaFileSystem = mediaFileSystem; _contentSettings = contentSettings.Value; _hostingEnvironment = hostingEnvironment; _imageUrlGenerator = imageUrlGenerator; - _backofficeSecurityAccessor = backofficeSecurityAccessor; + _backOfficeSecurityAccessor = backofficeSecurityAccessor; _userService = userService; _umbracoMapper = umbracoMapper; _backOfficeUserManager = backOfficeUserManager; @@ -77,6 +80,7 @@ namespace Umbraco.Web.BackOffice.Controllers _localizedTextService = localizedTextService; _appCaches = appCaches; _shortStringHelper = shortStringHelper; + _passwordChanger = passwordChanger; } @@ -89,7 +93,7 @@ namespace Umbraco.Web.BackOffice.Controllers public Dictionary GetPermissions(int[] nodeIds) { var permissions = _userService - .GetPermissions(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, nodeIds); + .GetPermissions(_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser, nodeIds); var permissionsDictionary = new Dictionary(); foreach (var nodeId in nodeIds) @@ -110,7 +114,7 @@ namespace Umbraco.Web.BackOffice.Controllers [HttpGet] public bool HasPermission(string permissionToCheck, int nodeId) { - var p = _userService.GetPermissions(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, nodeId).GetAllPermissions(); + var p = _userService.GetPermissions(_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser, nodeId).GetAllPermissions(); if (p.Contains(permissionToCheck.ToString(CultureInfo.InvariantCulture))) { return true; @@ -129,15 +133,15 @@ namespace Umbraco.Web.BackOffice.Controllers if (status == null) throw new ArgumentNullException(nameof(status)); List userTours; - if (_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData.IsNullOrWhiteSpace()) + if (_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData.IsNullOrWhiteSpace()) { userTours = new List { status }; - _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData = JsonConvert.SerializeObject(userTours); - _userService.Save(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser); + _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData = JsonConvert.SerializeObject(userTours); + _userService.Save(_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser); return userTours; } - userTours = JsonConvert.DeserializeObject>(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData).ToList(); + userTours = JsonConvert.DeserializeObject>(_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData).ToList(); var found = userTours.FirstOrDefault(x => x.Alias == status.Alias); if (found != null) { @@ -145,8 +149,8 @@ namespace Umbraco.Web.BackOffice.Controllers userTours.Remove(found); } userTours.Add(status); - _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData = JsonConvert.SerializeObject(userTours); - _userService.Save(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser); + _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData = JsonConvert.SerializeObject(userTours); + _userService.Save(_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser); return userTours; } @@ -156,10 +160,10 @@ namespace Umbraco.Web.BackOffice.Controllers /// public IEnumerable GetUserTours() { - if (_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData.IsNullOrWhiteSpace()) + if (_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData.IsNullOrWhiteSpace()) return Enumerable.Empty(); - var userTours = JsonConvert.DeserializeObject>(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData); + var userTours = JsonConvert.DeserializeObject>(_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.TourData); return userTours; } @@ -175,7 +179,7 @@ namespace Umbraco.Web.BackOffice.Controllers [AllowAnonymous] public async Task> PostSetInvitedUserPassword([FromBody]string newPassword) { - var user = await _backOfficeUserManager.FindByIdAsync(_backofficeSecurityAccessor.BackOfficeSecurity.GetUserId().ResultOr(0).ToString()); + var user = await _backOfficeUserManager.FindByIdAsync(_backOfficeSecurityAccessor.BackOfficeSecurity.GetUserId().ResultOr(0).ToString()); if (user == null) throw new InvalidOperationException("Could not find user"); var result = await _backOfficeUserManager.AddPasswordAsync(user, newPassword); @@ -190,13 +194,13 @@ namespace Umbraco.Web.BackOffice.Controllers } //They've successfully set their password, we can now update their user account to be approved - _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.IsApproved = true; + _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.IsApproved = true; //They've successfully set their password, and will now get fully logged into the back office, so the lastlogindate is set so the backoffice shows they have logged in - _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.LastLoginDate = DateTime.UtcNow; - _userService.Save(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser); + _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.LastLoginDate = DateTime.UtcNow; + _userService.Save(_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser); //now we can return their full object since they are now really logged into the back office - var userDisplay = _umbracoMapper.Map(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser); + var userDisplay = _umbracoMapper.Map(_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser); userDisplay.SecondsUntilTimeout = HttpContext.User.GetRemainingAuthSeconds(); return userDisplay; @@ -206,31 +210,38 @@ namespace Umbraco.Web.BackOffice.Controllers public IActionResult PostSetAvatar(IList file) { //borrow the logic from the user controller - return UsersController.PostSetAvatarInternal(file, _userService, _appCaches.RuntimeCache, _mediaFileSystem, _shortStringHelper, _contentSettings, _hostingEnvironment, _imageUrlGenerator, _backofficeSecurityAccessor.BackOfficeSecurity.GetUserId().ResultOr(0)); + return UsersController.PostSetAvatarInternal(file, _userService, _appCaches.RuntimeCache, _mediaFileSystem, _shortStringHelper, _contentSettings, _hostingEnvironment, _imageUrlGenerator, _backOfficeSecurityAccessor.BackOfficeSecurity.GetUserId().ResultOr(0)); } /// /// Changes the users password /// - /// + /// The changing password model /// /// If the password is being reset it will return the newly reset password, otherwise will return an empty value /// - public async Task>> PostChangePassword(ChangingPasswordModel data) + public async Task>> PostChangePassword(ChangingPasswordModel changingPasswordModel) { - // TODO: Why don't we inject this? Then we can just inject a logger - var passwordChanger = new PasswordChanger(_loggerFactory.CreateLogger()); - var passwordChangeResult = await passwordChanger.ChangePasswordWithIdentityAsync(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, data, _backOfficeUserManager); + IUser currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser; + changingPasswordModel.CurrentUserHasSectionAccess = currentUser.HasSectionAccess(Constants.Applications.Users); + + // the current user has access to change their password + changingPasswordModel.CurrentUserHasSectionAccess = true; + changingPasswordModel.CurrentUsername = currentUser.Username; + changingPasswordModel.SavingUsername = currentUser.Username; + changingPasswordModel.SavingUserId = currentUser.Id; + + Attempt passwordChangeResult = await _passwordChanger.ChangePasswordWithIdentityAsync(changingPasswordModel, _backOfficeUserManager); if (passwordChangeResult.Success) { - //even if we weren't resetting this, it is the correct value (null), otherwise if we were resetting then it will contain the new pword + // even if we weren't resetting this, it is the correct value (null), otherwise if we were resetting then it will contain the new pword var result = new ModelWithNotifications(passwordChangeResult.Result.ResetPassword); result.AddSuccessNotification(_localizedTextService.Localize("user/password"), _localizedTextService.Localize("user/passwordChanged")); return result; } - foreach (var memberName in passwordChangeResult.Result.ChangeError.MemberNames) + foreach (string memberName in passwordChangeResult.Result.ChangeError.MemberNames) { ModelState.AddModelError(memberName, passwordChangeResult.Result.ChangeError.ErrorMessage); } @@ -243,7 +254,7 @@ namespace Umbraco.Web.BackOffice.Controllers [ValidateAngularAntiForgeryToken] public async Task> GetCurrentUserLinkedLogins() { - var identityUser = await _backOfficeUserManager.FindByIdAsync(_backofficeSecurityAccessor.BackOfficeSecurity.GetUserId().ResultOr(0).ToString()); + var identityUser = await _backOfficeUserManager.FindByIdAsync(_backOfficeSecurityAccessor.BackOfficeSecurity.GetUserId().ResultOr(0).ToString()); // deduplicate in case there are duplicates (there shouldn't be now since we have a unique constraint on the external logins // but there didn't used to be) diff --git a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs index cf5681d578..8831f7fe00 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/MemberController.cs @@ -28,11 +28,13 @@ using Umbraco.Infrastructure.Security; using Umbraco.Infrastructure.Services.Implement; using Umbraco.Web.BackOffice.Filters; using Umbraco.Web.BackOffice.ModelBinders; +using Umbraco.Web.BackOffice.Security; using Umbraco.Web.Common.ActionsResults; using Umbraco.Web.Common.Attributes; using Umbraco.Web.Common.Authorization; using Umbraco.Web.Common.Filters; using Umbraco.Web.ContentApps; +using Umbraco.Web.Models; using Umbraco.Web.Models.ContentEditing; namespace Umbraco.Web.BackOffice.Controllers @@ -56,6 +58,7 @@ namespace Umbraco.Web.BackOffice.Controllers private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; private readonly IJsonSerializer _jsonSerializer; private readonly IShortStringHelper _shortStringHelper; + private readonly IPasswordChanger _passwordChanger; /// /// Initializes a new instance of the class. @@ -73,6 +76,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// The data-type service /// The back office security accessor /// The JSON serializer + /// The password changer public MemberController( ICultureDictionary cultureDictionary, ILoggerFactory loggerFactory, @@ -86,7 +90,8 @@ namespace Umbraco.Web.BackOffice.Controllers IMemberManager memberManager, IDataTypeService dataTypeService, IBackOfficeSecurityAccessor backOfficeSecurityAccessor, - IJsonSerializer jsonSerializer) + IJsonSerializer jsonSerializer, + IPasswordChanger passwordChanger) : base(cultureDictionary, loggerFactory, shortStringHelper, eventMessages, localizedTextService, jsonSerializer) { _propertyEditors = propertyEditors; @@ -99,6 +104,7 @@ namespace Umbraco.Web.BackOffice.Controllers _backOfficeSecurityAccessor = backOfficeSecurityAccessor; _jsonSerializer = jsonSerializer; _shortStringHelper = shortStringHelper; + _passwordChanger = passwordChanger; } /// @@ -390,7 +396,7 @@ namespace Umbraco.Web.BackOffice.Controllers } //TODO: do we need to resave the key? - //contentItem.PersistedContent.Key = contentItem.Key; + // contentItem.PersistedContent.Key = contentItem.Key; // now the member has been saved via identity, resave the member with mapped content properties _memberService.Save(member); @@ -450,7 +456,7 @@ namespace Umbraco.Web.BackOffice.Controllers MembersIdentityUser identityMember = await _memberManager.FindByIdAsync(contentItem.Id.ToString()); if (identityMember == null) { - return new ValidationErrorResult("Member was not found"); + return new ValidationErrorResult("Identity member was not found"); } if (contentItem.Password != null) @@ -461,14 +467,52 @@ namespace Umbraco.Web.BackOffice.Controllers return new ValidationErrorResult(validatePassword.Errors.ToErrorMessage()); } - string newPassword = _memberManager.HashPassword(contentItem.Password.NewPassword); - identityMember.PasswordHash = newPassword; - contentItem.PersistedContent.RawPasswordValue = identityMember.PasswordHash; - if (identityMember.LastPasswordChangeDateUtc != null) + Attempt intId = identityMember.Id.TryConvertTo(); + if (intId.Success == false) { - contentItem.PersistedContent.LastPasswordChangeDate = DateTime.UtcNow; + return new ValidationErrorResult("Member ID was not valid"); + } + + IMember foundMember = _memberService.GetById(intId.Result); + if (foundMember == null) + { + return new ValidationErrorResult("Member was not found"); + } + + IUser currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser; + var changingPasswordModel = new ChangingPasswordModel + { + Id = intId.Result, + OldPassword = contentItem.Password.OldPassword, + NewPassword = contentItem.Password.NewPassword, + CurrentUsername = currentUser.Username, + SavingUserId = foundMember.Id, + SavingUsername = foundMember.Username, + CurrentUserHasSectionAccess = currentUser.HasSectionAccess(Constants.Applications.Members) + }; + + Attempt passwordChangeResult = await _passwordChanger.ChangePasswordWithIdentityAsync(changingPasswordModel, _memberManager); + + if (passwordChangeResult.Success) + { + contentItem.PersistedContent.RawPasswordValue = passwordChangeResult.Result.ResetPassword; + if (identityMember.LastPasswordChangeDateUtc != null) + { + contentItem.PersistedContent.LastPasswordChangeDate = DateTime.UtcNow; + } + identityMember.LastPasswordChangeDateUtc = contentItem.PersistedContent.LastPasswordChangeDate; } + + if (passwordChangeResult.Result.ChangeError?.MemberNames != null) + { + foreach (string memberName in passwordChangeResult.Result.ChangeError?.MemberNames) + { + ModelState.AddModelError(memberName, passwordChangeResult.Result.ChangeError?.ErrorMessage ?? string.Empty); + } + } + + return new ValidationErrorResult(new SimpleValidationModel(ModelState.ToErrorDictionary())); } IdentityResult updatedResult = await _memberManager.UpdateAsync(identityMember); diff --git a/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs b/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs index d8e4b83ac9..93d1f236a6 100644 --- a/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs +++ b/src/Umbraco.Web.BackOffice/Controllers/UsersController.cs @@ -23,6 +23,7 @@ using Umbraco.Core.Mail; using Umbraco.Core.Mapping; using Umbraco.Core.Media; using Umbraco.Core.Models; +using Umbraco.Core.Models.Identity; using Umbraco.Core.Models.Membership; using Umbraco.Core.Persistence; using Umbraco.Core.Security; @@ -56,7 +57,7 @@ namespace Umbraco.Web.BackOffice.Controllers private readonly IImageUrlGenerator _imageUrlGenerator; private readonly SecuritySettings _securitySettings; private readonly IEmailSender _emailSender; - private readonly IBackOfficeSecurityAccessor _backofficeSecurityAccessor; + private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; private readonly AppCaches _appCaches; private readonly IShortStringHelper _shortStringHelper; private readonly IUserService _userService; @@ -68,6 +69,7 @@ namespace Umbraco.Web.BackOffice.Controllers private readonly LinkGenerator _linkGenerator; private readonly IBackOfficeExternalLoginProviders _externalLogins; private readonly UserEditorAuthorizationHelper _userEditorAuthorizationHelper; + private readonly IPasswordChanger _passwordChanger; private readonly ILogger _logger; public UsersController( @@ -89,7 +91,8 @@ namespace Umbraco.Web.BackOffice.Controllers ILoggerFactory loggerFactory, LinkGenerator linkGenerator, IBackOfficeExternalLoginProviders externalLogins, - UserEditorAuthorizationHelper userEditorAuthorizationHelper) + UserEditorAuthorizationHelper userEditorAuthorizationHelper, + IPasswordChanger passwordChanger) { _mediaFileSystem = mediaFileSystem; _contentSettings = contentSettings.Value; @@ -98,7 +101,7 @@ namespace Umbraco.Web.BackOffice.Controllers _imageUrlGenerator = imageUrlGenerator; _securitySettings = securitySettings.Value; _emailSender = emailSender; - _backofficeSecurityAccessor = backofficeSecurityAccessor; + _backOfficeSecurityAccessor = backofficeSecurityAccessor; _appCaches = appCaches; _shortStringHelper = shortStringHelper; _userService = userService; @@ -110,6 +113,7 @@ namespace Umbraco.Web.BackOffice.Controllers _linkGenerator = linkGenerator; _externalLogins = externalLogins; _userEditorAuthorizationHelper = userEditorAuthorizationHelper; + _passwordChanger = passwordChanger; _logger = _loggerFactory.CreateLogger(); } @@ -119,7 +123,7 @@ namespace Umbraco.Web.BackOffice.Controllers /// public ActionResult GetCurrentUserAvatarUrls() { - var urls = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.GetUserAvatarUrls(_appCaches.RuntimeCache, _mediaFileSystem, _imageUrlGenerator); + var urls = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.GetUserAvatarUrls(_appCaches.RuntimeCache, _mediaFileSystem, _imageUrlGenerator); if (urls == null) return new ValidationErrorResult("Could not access Gravatar endpoint"); @@ -285,7 +289,7 @@ namespace Umbraco.Web.BackOffice.Controllers var hideDisabledUsers = _securitySettings.HideDisabledUsersInBackOffice; var excludeUserGroups = new string[0]; - var isAdmin = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.IsAdmin(); + var isAdmin = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.IsAdmin(); if (isAdmin == false) { //this user is not an admin so in that case we need to exclude all admin users @@ -294,7 +298,7 @@ namespace Umbraco.Web.BackOffice.Controllers var filterQuery = _sqlContext.Query(); - if (!_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.IsSuper()) + if (!_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.IsSuper()) { // only super can see super - but don't use IsSuper, cannot be mapped to SQL //filterQuery.Where(x => !x.IsSuper()); @@ -359,7 +363,7 @@ namespace Umbraco.Web.BackOffice.Controllers } //Perform authorization here to see if the current user can actually save this user with the info being requested - var canSaveUser = _userEditorAuthorizationHelper.IsAuthorized(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, null, null, null, userSave.UserGroups); + var canSaveUser = _userEditorAuthorizationHelper.IsAuthorized(_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser, null, null, null, userSave.UserGroups); if (canSaveUser == false) { return Unauthorized(canSaveUser.Result); @@ -448,7 +452,7 @@ namespace Umbraco.Web.BackOffice.Controllers } //Perform authorization here to see if the current user can actually save this user with the info being requested - var canSaveUser = _userEditorAuthorizationHelper.IsAuthorized(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, user, null, null, userSave.UserGroups); + var canSaveUser = _userEditorAuthorizationHelper.IsAuthorized(_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser, user, null, null, userSave.UserGroups); if (canSaveUser == false) { return new ValidationErrorResult(canSaveUser.Result, StatusCodes.Status401Unauthorized); @@ -511,7 +515,7 @@ namespace Umbraco.Web.BackOffice.Controllers { //send the email - await SendUserInviteEmailAsync(display, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Name, _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Email, user, userSave.Message); + await SendUserInviteEmailAsync(display, _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Name, _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser.Email, user, userSave.Message); } @@ -605,7 +609,7 @@ namespace Umbraco.Web.BackOffice.Controllers return NotFound(); //Perform authorization here to see if the current user can actually save this user with the info being requested - var canSaveUser = _userEditorAuthorizationHelper.IsAuthorized(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, found, userSave.StartContentIds, userSave.StartMediaIds, userSave.UserGroups); + var canSaveUser = _userEditorAuthorizationHelper.IsAuthorized(_backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser, found, userSave.StartContentIds, userSave.StartMediaIds, userSave.UserGroups); if (canSaveUser == false) { return Unauthorized(canSaveUser.Result); @@ -665,7 +669,7 @@ namespace Umbraco.Web.BackOffice.Controllers var display = _umbracoMapper.Map(user); // determine if the user has changed their own language; - var currentUser = _backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser; + var currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser; var userHasChangedOwnLanguage = user.Id == currentUser.Id && currentUser.Language != user.Language; @@ -691,21 +695,25 @@ namespace Umbraco.Web.BackOffice.Controllers return new ValidationErrorResult(new SimpleValidationModel(ModelState.ToErrorDictionary())); } - var intId = changingPasswordModel.Id.TryConvertTo(); + Attempt intId = changingPasswordModel.Id.TryConvertTo(); if (intId.Success == false) { return NotFound(); } - var found = _userService.GetUserById(intId.Result); + IUser found = _userService.GetUserById(intId.Result); if (found == null) { return NotFound(); } - // TODO: Why don't we inject this? Then we can just inject a logger - var passwordChanger = new PasswordChanger(_loggerFactory.CreateLogger()); - var passwordChangeResult = await passwordChanger.ChangePasswordWithIdentityAsync(_backofficeSecurityAccessor.BackOfficeSecurity.CurrentUser, found, changingPasswordModel, _userManager); + IUser currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser; + changingPasswordModel.CurrentUserHasSectionAccess = currentUser.HasSectionAccess(Constants.Applications.Users); + changingPasswordModel.CurrentUsername = currentUser.Username; + changingPasswordModel.SavingUserId = found.Id; + changingPasswordModel.SavingUsername = found.Username; + + Attempt passwordChangeResult = await _passwordChanger.ChangePasswordWithIdentityAsync(changingPasswordModel, _userManager); if (passwordChangeResult.Success) { @@ -714,7 +722,7 @@ namespace Umbraco.Web.BackOffice.Controllers return result; } - foreach (var memberName in passwordChangeResult.Result.ChangeError.MemberNames) + foreach (string memberName in passwordChangeResult.Result.ChangeError.MemberNames) { ModelState.AddModelError(memberName, passwordChangeResult.Result.ChangeError.ErrorMessage); } @@ -730,7 +738,7 @@ namespace Umbraco.Web.BackOffice.Controllers [Authorize(Policy = AuthorizationPolicies.AdminUserEditsRequireAdmin)] public IActionResult PostDisableUsers([FromQuery]int[] userIds) { - var tryGetCurrentUserId = _backofficeSecurityAccessor.BackOfficeSecurity.GetUserId(); + var tryGetCurrentUserId = _backOfficeSecurityAccessor.BackOfficeSecurity.GetUserId(); if (tryGetCurrentUserId && userIds.Contains(tryGetCurrentUserId.Result)) { return ValidationErrorResult.CreateNotificationValidationErrorResult("The current user cannot disable itself"); diff --git a/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs index 32fb491695..fc0edbb2cb 100644 --- a/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs +++ b/src/Umbraco.Web.BackOffice/DependencyInjection/UmbracoBuilderExtensions.cs @@ -6,6 +6,7 @@ using Microsoft.Extensions.Logging; using Umbraco.Core.DependencyInjection; using Umbraco.Core.Hosting; using Umbraco.Core.IO; +using Umbraco.Core.Models.Identity; using Umbraco.Core.Services; using Umbraco.Extensions; using Umbraco.Infrastructure.DependencyInjection; @@ -81,6 +82,7 @@ namespace Umbraco.Web.BackOffice.DependencyInjection builder.Services.AddUnique(); builder.Services.AddUnique(); builder.Services.AddUnique(); + builder.Services.AddUnique, PasswordChanger>(); return builder; } diff --git a/src/Umbraco.Web.BackOffice/Security/IPasswordChanger.cs b/src/Umbraco.Web.BackOffice/Security/IPasswordChanger.cs new file mode 100644 index 0000000000..7292fde344 --- /dev/null +++ b/src/Umbraco.Web.BackOffice/Security/IPasswordChanger.cs @@ -0,0 +1,16 @@ +using System.Threading.Tasks; +using Umbraco.Core; +using Umbraco.Core.Models; +using Umbraco.Core.Models.Identity; +using Umbraco.Infrastructure.Security; +using Umbraco.Web.Models; + +namespace Umbraco.Web.BackOffice.Security +{ + public interface IPasswordChanger where TUser : UmbracoIdentityUser + { + public Task> ChangePasswordWithIdentityAsync( + ChangingPasswordModel passwordModel, + IUmbracoUserManager userMgr); + } +} diff --git a/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs b/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs index 6144e7b63f..e9df385a18 100644 --- a/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs +++ b/src/Umbraco.Web.BackOffice/Security/PasswordChanger.cs @@ -1,22 +1,31 @@ using System; using System.ComponentModel.DataAnnotations; using System.Threading.Tasks; +using Microsoft.AspNetCore.Identity; using Microsoft.Extensions.Logging; using Umbraco.Core; using Umbraco.Core.Models; -using Umbraco.Core.Security; -using Umbraco.Extensions; +using Umbraco.Core.Models.Identity; +using Umbraco.Core.Models.Membership; using Umbraco.Infrastructure.Security; using Umbraco.Web.Models; -using IUser = Umbraco.Core.Models.Membership.IUser; namespace Umbraco.Web.BackOffice.Security { - internal class PasswordChanger + /// + /// Changes the password for an identity user + /// + internal class PasswordChanger : IPasswordChanger + where TUser : UmbracoIdentityUser { - private readonly ILogger _logger; + private readonly ILogger> _logger; - public PasswordChanger(ILogger logger) + /// + /// Initializes a new instance of the class. + /// Password changing functionality + /// + /// Logger for this class + public PasswordChanger(ILogger> logger) { _logger = logger; } @@ -24,55 +33,60 @@ namespace Umbraco.Web.BackOffice.Security /// /// Changes the password for a user based on the many different rules and config options /// - /// The user performing the password save action - /// The user who's password is being changed - /// - /// - /// + /// The changing password model + /// The identity manager to use to update the password + /// Create an adapter to pass through everything - adapting the member into a user for this functionality + /// The outcome of the password changed model public async Task> ChangePasswordWithIdentityAsync( - IUser currentUser, - IUser savingUser, - ChangingPasswordModel passwordModel, - IBackOfficeUserManager userMgr) + ChangingPasswordModel changingPasswordModel, + IUmbracoUserManager userMgr) { - if (passwordModel == null) throw new ArgumentNullException(nameof(passwordModel)); - if (userMgr == null) throw new ArgumentNullException(nameof(userMgr)); + if (changingPasswordModel == null) + { + throw new ArgumentNullException(nameof(changingPasswordModel)); + } - if (passwordModel.NewPassword.IsNullOrWhiteSpace()) + if (userMgr == null) + { + throw new ArgumentNullException(nameof(userMgr)); + } + + if (changingPasswordModel.NewPassword.IsNullOrWhiteSpace()) { return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Cannot set an empty password", new[] { "value" }) }); } - var backOfficeIdentityUser = await userMgr.FindByIdAsync(savingUser.Id.ToString()); - if (backOfficeIdentityUser == null) + TUser identityUser = await userMgr.FindByIdAsync(changingPasswordModel.SavingUserId.ToString()); + if (identityUser == null) { - //this really shouldn't ever happen... but just in case + // this really shouldn't ever happen... but just in case return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Password could not be verified", new[] { "oldPassword" }) }); } - //Are we just changing another user's password? - if (passwordModel.OldPassword.IsNullOrWhiteSpace()) + // Are we just changing another user's password? + if (changingPasswordModel.OldPassword.IsNullOrWhiteSpace()) { - //if it's the current user, the current user cannot reset their own password - if (currentUser.Username == savingUser.Username) + // if it's the current user, the current user cannot reset their own password + // For members, this should not happen + if (changingPasswordModel.CurrentUsername == changingPasswordModel.SavingUsername) { return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Password reset is not allowed", new[] { "value" }) }); } - //if the current user has access to reset/manually change the password - if (currentUser.HasSectionAccess(Umbraco.Core.Constants.Applications.Users) == false) + // if the current user has access to reset/manually change the password + if (changingPasswordModel.CurrentUserHasSectionAccess) { return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("The current user is not authorized", new[] { "value" }) }); } - //ok, we should be able to reset it - var resetToken = await userMgr.GeneratePasswordResetTokenAsync(backOfficeIdentityUser); + // ok, we should be able to reset it + string resetToken = await userMgr.GeneratePasswordResetTokenAsync(identityUser); - var resetResult = await userMgr.ChangePasswordWithResetAsync(savingUser.Id.ToString(), resetToken, passwordModel.NewPassword); + IdentityResult resetResult = await userMgr.ChangePasswordWithResetAsync(changingPasswordModel.SavingUserId.ToString(), resetToken, changingPasswordModel.NewPassword); if (resetResult.Succeeded == false) { - var errors = resetResult.Errors.ToErrorMessage(); + string errors = resetResult.Errors.ToErrorMessage(); _logger.LogWarning("Could not reset user password {PasswordErrors}", errors); return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult(errors, new[] { "value" }) }); } @@ -80,24 +94,25 @@ namespace Umbraco.Web.BackOffice.Security return Attempt.Succeed(new PasswordChangedModel()); } - //is the old password correct? - var validateResult = await userMgr.CheckPasswordAsync(backOfficeIdentityUser, passwordModel.OldPassword); + // is the old password correct? + bool validateResult = await userMgr.CheckPasswordAsync(identityUser, changingPasswordModel.OldPassword); if (validateResult == false) { - //no, fail with an error message for "oldPassword" + // no, fail with an error message for "oldPassword" return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult("Incorrect password", new[] { "oldPassword" }) }); } - //can we change to the new password? - var changeResult = await userMgr.ChangePasswordAsync(backOfficeIdentityUser, passwordModel.OldPassword, passwordModel.NewPassword); + + // can we change to the new password? + IdentityResult changeResult = await userMgr.ChangePasswordAsync(identityUser, changingPasswordModel.OldPassword, changingPasswordModel.NewPassword); if (changeResult.Succeeded == false) { - //no, fail with error messages for "password" - var errors = changeResult.Errors.ToErrorMessage(); + // no, fail with error messages for "password" + string errors = changeResult.Errors.ToErrorMessage(); _logger.LogWarning("Could not change user password {PasswordErrors}", errors); return Attempt.Fail(new PasswordChangedModel { ChangeError = new ValidationResult(errors, new[] { "password" }) }); } + return Attempt.Succeed(new PasswordChangedModel()); } - } }