diff --git a/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerManagerService.cs b/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerManagerService.cs index 9536c9197c..e3515b3fe2 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerManagerService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerManagerService.cs @@ -39,4 +39,14 @@ public interface ICustomerManagerService /// Request /// Store identifier - used to resolve the customer when per-store customer identity is enabled Task ChangePassword(ChangePasswordRequest request, string storeId = ""); + + /// + /// Change password of this customer, without resolving the account by e-mail + /// + /// Customer + /// Password format + /// New password + /// The default body keeps implementations built for an earlier 2.4 release loading without a rebuild + Task ChangePassword(Customer customer, PasswordFormat passwordFormat, string newPassword) => + ChangePassword(new ChangePasswordRequest(customer.Email, passwordFormat, newPassword), customer.StoreId); } \ No newline at end of file diff --git a/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerService.cs b/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerService.cs index 685404e62b..d1c2919f6c 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerService.cs @@ -190,6 +190,13 @@ Task UpdateCustomerField(string customerId, /// Customer Task UpdateActive(Customer customer); + /// + /// Updates the password fields of the customer, and nothing else + /// + /// Customer + /// The default body keeps implementations built for an earlier 2.4 release loading without a rebuild + Task UpdateCustomerPassword(Customer customer) => UpdateCustomer(customer); + /// /// Update the customer /// diff --git a/src/Business/Grand.Business.Customers/Services/CustomerManagerService.cs b/src/Business/Grand.Business.Customers/Services/CustomerManagerService.cs index 7768df188f..738df793f7 100644 --- a/src/Business/Grand.Business.Customers/Services/CustomerManagerService.cs +++ b/src/Business/Grand.Business.Customers/Services/CustomerManagerService.cs @@ -173,33 +173,46 @@ public virtual async Task ChangePassword(ChangePasswordRequest request, string s ArgumentNullException.ThrowIfNull(request); var customer = await _customerService.GetCustomerByEmail(request.Email, storeId); + + await ChangePassword(customer, request.PasswordFormat, request.NewPassword); + } + + /// + /// Change password of this customer, without resolving the account by e-mail + /// + /// Customer + /// Password format + /// New password + public virtual async Task ChangePassword(Customer customer, PasswordFormat passwordFormat, string newPassword) + { ArgumentNullException.ThrowIfNull(customer); - switch (request.PasswordFormat) + switch (passwordFormat) { case PasswordFormat.Clear: { - customer.Password = request.NewPassword; + customer.Password = newPassword; } break; case PasswordFormat.Encrypted: { customer.PasswordSalt = CommonHelper.GenerateRandomDigitCode(24); - customer.Password = _encryptionService.EncryptText(request.NewPassword, customer.PasswordSalt); + customer.Password = _encryptionService.EncryptText(newPassword, customer.PasswordSalt); } break; case PasswordFormat.Hashed: { //modern self-describing PBKDF2 hash - salt and parameters are embedded in the value itself customer.PasswordSalt = string.Empty; - customer.Password = _encryptionService.HashPassword(request.NewPassword); + customer.Password = _encryptionService.HashPassword(newPassword); } break; } customer.PasswordChangeDateUtc = DateTime.UtcNow; - customer.PasswordFormatId = request.PasswordFormat; - await _customerService.UpdateCustomer(customer); + customer.PasswordFormatId = passwordFormat; + //only the password fields: the caller's copy may be older than the account + await _customerService.UpdateCustomerPassword(customer); //insert password history await _customerHistoryPasswordService.InsertCustomerPassword(customer); diff --git a/src/Business/Grand.Business.Customers/Services/CustomerService.cs b/src/Business/Grand.Business.Customers/Services/CustomerService.cs index 143fe78201..127eb46794 100644 --- a/src/Business/Grand.Business.Customers/Services/CustomerService.cs +++ b/src/Business/Grand.Business.Customers/Services/CustomerService.cs @@ -551,6 +551,26 @@ public virtual async Task UpdateCustomerInAdminPanel(Customer customer) } + public virtual async Task UpdateCustomerPassword(Customer customer) + { + ArgumentNullException.ThrowIfNull(customer); + + if (customer.IsSystemAccount) + throw new GrandException( + $"System customer account ({(string.IsNullOrEmpty(customer.SystemName) ? customer.Email : customer.SystemName)}) could not be updated"); + + var update = UpdateBuilder.Create() + .Set(x => x.Password, customer.Password) + .Set(x => x.PasswordSalt, customer.PasswordSalt) + .Set(x => x.PasswordFormatId, customer.PasswordFormatId) + .Set(x => x.PasswordChangeDateUtc, customer.PasswordChangeDateUtc); + + await _customerRepository.UpdateOneAsync(x => x.Id == customer.Id, update); + + //event notification + await _mediator.EntityUpdated(customer); + } + public virtual async Task UpdateActive(Customer customer) { ArgumentNullException.ThrowIfNull(customer); diff --git a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs index c9137a7e30..6dba653f5b 100644 --- a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs +++ b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs @@ -100,6 +100,74 @@ public async Task ChangePassword_PassesStoreIdToLookup() _customerServiceMock.Verify(c => c.GetCustomerByEmail("admin@admin.com", "store-1"), Times.Once); } + [TestMethod] + public async Task ChangePassword_ForCustomer_ChangesThatCustomerWithoutLookup() + { + //Arrange + var customer = new Customer { Id = "c1", Email = "a@x.com", PasswordFormatId = PasswordFormat.Clear, Password = "123456" }; + //Act + await _customerManagerService.ChangePassword(customer, PasswordFormat.Clear, "zxcvbn"); + //Assert + Assert.AreEqual("zxcvbn", customer.Password); + _customerServiceMock.Verify(c => c.UpdateCustomerPassword(customer), Times.Once); + //the whole-customer update would write back everything else the caller's copy holds + _customerServiceMock.Verify(c => c.UpdateCustomer(It.IsAny()), Times.Never); + _customerServiceMock.Verify(c => c.GetCustomerByEmail(It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task ChangePassword_ImplementationBuiltForAnEarlierRelease_UsesItsExistingMethod() + { + ICustomerManagerService manager = new EarlierReleaseManager(); + + await manager.ChangePassword(new Customer { Email = "a@x.com", StoreId = "store-1" }, PasswordFormat.Hashed, "new"); + + var implementation = (EarlierReleaseManager)manager; + Assert.AreEqual("a@x.com", implementation.Request.Email); + Assert.AreEqual(PasswordFormat.Hashed, implementation.Request.PasswordFormat); + Assert.AreEqual("new", implementation.Request.NewPassword); + Assert.AreEqual("store-1", implementation.StoreId); + } + + [TestMethod] + public void NewInterfaceMembers_HaveDefaultBodies_SoExistingImplementationsKeepLoading() + { + Assert.IsFalse(typeof(ICustomerService).GetMethod(nameof(ICustomerService.UpdateCustomerPassword))!.IsAbstract); + Assert.IsFalse(typeof(ICustomerManagerService).GetMethod(nameof(ICustomerManagerService.ChangePassword), + [typeof(Customer), typeof(PasswordFormat), typeof(string)])!.IsAbstract); + } + + private sealed class EarlierReleaseManager : ICustomerManagerService + { + public ChangePasswordRequest Request { get; private set; } + public string StoreId { get; private set; } + + public Task LoginCustomer(string usernameOrEmail, string password, string storeId = "") => + throw new NotImplementedException(); + + public Task RegisterCustomer(RegistrationRequest request) => throw new NotImplementedException(); + + public bool PasswordMatch(PasswordFormat passwordFormat, string oldPassword, string newPassword, + string passwordSalt) => throw new NotImplementedException(); + + public Task ChangePassword(ChangePasswordRequest request, string storeId = "") + { + Request = request; + StoreId = storeId; + return Task.CompletedTask; + } + } + + [TestMethod] + public async Task ChangePassword_UnknownEmail_Throws() + { + //Arrange + var changepassword = new ChangePasswordRequest("nobody@x.com", PasswordFormat.Clear, "zxcvbn"); + //Act + Assert + await Assert.ThrowsExactlyAsync(() => + _customerManagerService.ChangePassword(changepassword, "store-1")); + } + [TestMethod] public async Task ChangePasswordTest_Success() { diff --git a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs index ee12c930ae..0d755fe0b4 100644 --- a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs +++ b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs @@ -9,6 +9,7 @@ using Grand.Infrastructure.Configuration; using Grand.Infrastructure.Tests.Caching; using Grand.Mediator; +using Grand.SharedKernel; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -474,6 +475,43 @@ public async Task UpdateCustomerinAdminPanelTest() Assert.AreEqual("store-1", updated.StoreId); } + [TestMethod] + public async Task UpdateCustomerPassword_WritesOnlyThePasswordFields() + { + //Arrange + await _repository.InsertAsync(new Customer { + Id = "c1", Email = "current@x.com", Username = "current", StoreId = "store-1", Active = false, + Password = "old", PasswordSalt = "salt", PasswordFormatId = PasswordFormat.Clear + }); + //taken before the account changed, as a caller holds it during a request + var changedAt = new DateTime(2026, 11, 1, 12, 0, 0, DateTimeKind.Utc); + var snapshot = new Customer { + Id = "c1", Email = "stale@x.com", Username = "stale", StoreId = "store-2", Active = true, + Password = "new", PasswordSalt = "", PasswordFormatId = PasswordFormat.Hashed, + PasswordChangeDateUtc = changedAt + }; + //Act + await _customerService.UpdateCustomerPassword(snapshot); + //Assert + var stored = _repository.Table.First(x => x.Id == "c1"); + Assert.AreEqual("new", stored.Password); + Assert.AreEqual("", stored.PasswordSalt); + Assert.AreEqual(PasswordFormat.Hashed, stored.PasswordFormatId); + Assert.AreEqual(changedAt, stored.PasswordChangeDateUtc); + Assert.AreEqual("current@x.com", stored.Email); + Assert.AreEqual("current", stored.Username); + Assert.AreEqual("store-1", stored.StoreId); + Assert.IsFalse(stored.Active); + } + + [TestMethod] + public async Task UpdateCustomerPassword_SystemAccount_Throws() + { + var customer = new Customer { Id = "c1", IsSystemAccount = true, SystemName = "system" }; + + await Assert.ThrowsExactlyAsync(() => _customerService.UpdateCustomerPassword(customer)); + } + [TestMethod] public async Task UpdateActiveTest() { diff --git a/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs b/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs index 39337c109b..73becd1ced 100644 --- a/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs +++ b/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs @@ -6,6 +6,7 @@ using Grand.Web.Models.Customer; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; +using System.Linq.Expressions; namespace Grand.Web.Tests.Commands.Customers; @@ -14,6 +15,7 @@ public class SubAccountEditCommandHandlerTests { private Mock _customerServiceMock; private Mock _customerManagerServiceMock; + private CustomerSettings _customerSettings; private SubAccountEditCommandHandler _handler; [TestInitialize] @@ -21,47 +23,185 @@ public void Init() { _customerServiceMock = new Mock(); _customerManagerServiceMock = new Mock(); + _customerSettings = new CustomerSettings { AllowUsersToChangeEmail = true }; _handler = new SubAccountEditCommandHandler(_customerServiceMock.Object, - _customerManagerServiceMock.Object, new CustomerSettings { AllowUsersToChangeEmail = true }); + _customerManagerServiceMock.Object, _customerSettings); } - [TestMethod] - public async Task Handle_WithPassword_ChangesPasswordScopedToCustomerStore() + private Customer SubAccount(string ownerId = "owner") { - var customer = new Customer { Id = "c1", Email = "sub@x.com", StoreId = "store-9" }; + var customer = new Customer { + Id = "c1", Email = "sub@x.com", Username = "sub@x.com", StoreId = "store-9", OwnerId = ownerId + }; _customerServiceMock.Setup(c => c.GetCustomerById("c1")).ReturnsAsync(customer); + return customer; + } - var command = new SubAccountEditCommand { - CurrentCustomer = new Customer { Id = "owner" }, - EditModel = new SubAccountEditModel { - Id = "c1", Email = "sub@x.com", Password = "newpass", Active = true, - FirstName = "John", LastName = "Doe" - } - }; + private void VerifyPasswordChangedOn(Customer customer) + { + _customerManagerServiceMock.Verify(m => m.ChangePassword(customer, It.IsAny(), "newpass"), + Times.Once); + //never through a lookup by e-mail, which may lead to another account + _customerManagerServiceMock.Verify(m => m.ChangePassword(It.IsAny(), + It.IsAny()), Times.Never); + } + + private static SubAccountEditCommand Command(string email = "sub@x.com", string password = null) => new() { + CurrentCustomer = new Customer { Id = "owner" }, + EditModel = new SubAccountEditModel { + Id = "c1", Email = email, Password = password, Active = true, FirstName = "John", LastName = "Doe" + } + }; + + private void VerifyNothingWritten() + { + _customerServiceMock.Verify(c => c.UpdateCustomerField(It.IsAny(), + It.IsAny>>(), It.IsAny()), Times.Never); + _customerServiceMock.Verify(c => c.UpdateActive(It.IsAny()), Times.Never); + _customerServiceMock.Verify(c => c.UpdateUserField(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny()), Times.Never); + _customerManagerServiceMock.Verify(m => m.ChangePassword(It.IsAny(), + It.IsAny()), Times.Never); + _customerManagerServiceMock.Verify(m => m.ChangePassword(It.IsAny(), It.IsAny(), + It.IsAny()), Times.Never); + } - var result = await _handler.Handle(command, default); + [TestMethod] + public async Task Handle_WithPassword_ChangesPasswordOfTheEditedAccount() + { + var customer = SubAccount(); + //another account holding the same address must not matter + _customerServiceMock.Setup(c => c.GetCustomerByEmail("sub@x.com", "store-9")) + .ReturnsAsync(new Customer { Id = "other", Email = "sub@x.com", StoreId = "store-9" }); + + var result = await _handler.Handle(Command(password: "newpass"), default); Assert.IsTrue(result); - //the sub-account's store must scope the password change (safe with or without per-store identity) - _customerManagerServiceMock.Verify(m => m.ChangePassword( - It.Is(r => r.Email == "sub@x.com"), "store-9"), Times.Once); + VerifyPasswordChangedOn(customer); } [TestMethod] public async Task Handle_WithoutPassword_DoesNotChangePassword() { - var customer = new Customer { Id = "c1", Email = "sub@x.com", StoreId = "store-9" }; - _customerServiceMock.Setup(c => c.GetCustomerById("c1")).ReturnsAsync(customer); + SubAccount(); - var command = new SubAccountEditCommand { - CurrentCustomer = new Customer { Id = "owner" }, - EditModel = new SubAccountEditModel { Id = "c1", Email = "sub@x.com", Active = true } - }; - - var result = await _handler.Handle(command, default); + var result = await _handler.Handle(Command(), default); Assert.IsTrue(result); _customerManagerServiceMock.Verify(m => m.ChangePassword( It.IsAny(), It.IsAny()), Times.Never); + _customerManagerServiceMock.Verify(m => m.ChangePassword( + It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task Handle_AccountOfAnotherOwner_ChangesNothing() + { + SubAccount(ownerId: "another-owner"); + + var result = await _handler.Handle(Command(email: "new@x.com", password: "newpass"), default); + + Assert.IsFalse(result); + VerifyNothingWritten(); + } + + [TestMethod] + public async Task Handle_AccountWithoutOwner_ChangesNothing() + { + SubAccount(ownerId: ""); + + var result = await _handler.Handle(Command(password: "newpass"), default); + + Assert.IsFalse(result); + VerifyNothingWritten(); + } + + [TestMethod] + public async Task Handle_DeletedAccount_ChangesNothing() + { + SubAccount().Deleted = true; + + var result = await _handler.Handle(Command(password: "newpass"), default); + + Assert.IsFalse(result); + VerifyNothingWritten(); + } + + [TestMethod] + public async Task Handle_MissingAccount_ChangesNothing() + { + var result = await _handler.Handle(Command(password: "newpass"), default); + + Assert.IsFalse(result); + VerifyNothingWritten(); + } + + [TestMethod] + public async Task Handle_MixedCaseEmail_IsStoredLowercasedAndTrimmed() + { + var customer = SubAccount(); + + await _handler.Handle(Command(email: " New.Sub@X.com "), default); + + Assert.AreEqual("new.sub@x.com", customer.Email); + _customerServiceMock.Verify(c => c.UpdateCustomerField(customer, + It.IsAny>>(), "new.sub@x.com"), Times.AtLeastOnce); + } + + [TestMethod] + public async Task Handle_NewEmailAndPassword_ChangesPasswordOfTheEditedAccount() + { + var customer = SubAccount(); + + await _handler.Handle(Command(email: "New@X.com", password: "newpass"), default); + + Assert.AreEqual("new@x.com", customer.Email); + VerifyPasswordChangedOn(customer); + } + + [TestMethod] + public async Task Handle_NewEmail_UsernamesDisabled_KeepsUsernameEqualToEmail() + { + var customer = SubAccount(); + + await _handler.Handle(Command(email: "new@x.com"), default); + + Assert.AreEqual("new@x.com", customer.Username); + } + + [TestMethod] + public async Task Handle_NewEmail_UsernamesEnabled_LeavesUsername() + { + _customerSettings.UsernamesEnabled = true; + var customer = SubAccount(); + + await _handler.Handle(Command(email: "new@x.com"), default); + + Assert.AreEqual("new@x.com", customer.Email); + Assert.AreEqual("sub@x.com", customer.Username); + } + + [TestMethod] + public async Task Handle_EmailStoredInMixedCase_IsRepairedEvenWhenChangesAreNotAllowed() + { + _customerSettings.AllowUsersToChangeEmail = false; + var customer = SubAccount(); + customer.Email = "Sub@X.com"; + + await _handler.Handle(Command(email: "Sub@X.com", password: "newpass"), default); + + Assert.AreEqual("sub@x.com", customer.Email); + VerifyPasswordChangedOn(customer); + } + + [TestMethod] + public async Task Handle_EmailChangeNotAllowed_KeepsEmail() + { + _customerSettings.AllowUsersToChangeEmail = false; + var customer = SubAccount(); + + await _handler.Handle(Command(email: "new@x.com"), default); + + Assert.AreEqual("sub@x.com", customer.Email); } } diff --git a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerSubAccountsTests.cs b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerSubAccountsTests.cs new file mode 100644 index 0000000000..47ff3156b5 --- /dev/null +++ b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerSubAccountsTests.cs @@ -0,0 +1,128 @@ +using Grand.Business.Core.Interfaces.Authentication; +using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Business.Core.Interfaces.Common.Localization; +using Grand.Business.Core.Interfaces.Customers; +using Grand.Business.Core.Interfaces.Messages; +using Grand.Business.Core.Utilities.Customers; +using Grand.Domain.Common; +using Grand.Domain.Customers; +using Grand.Infrastructure; +using Grand.Infrastructure.Configuration; +using Grand.Mediator; +using Grand.Web.Commands.Models.Customers; +using Grand.Web.Controllers; +using Grand.Web.Features.Models.Customers; +using Grand.Web.Models.Customer; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Web.Tests.Controllers; + +[TestClass] +public class AccountControllerSubAccountsTests +{ + private readonly Customer _customer = new() { Id = "owner", Email = "owner@x.com", StoreId = "store-9" }; + private Mock _customerManagerServiceMock; + private CustomerSettings _customerSettings; + private Mock _groupServiceMock; + private Mock _mediatorMock; + private AccountController _controller; + + [TestInitialize] + public void Init() + { + _customerManagerServiceMock = new Mock(); + _customerSettings = new CustomerSettings { HideSubAccountsTab = false }; + _groupServiceMock = new Mock(); + _groupServiceMock.Setup(g => g.IsOwner(_customer)).ReturnsAsync(true); + _mediatorMock = new Mock(); + + var workContext = new Mock(); + workContext.Setup(w => w.CurrentCustomer).Returns(_customer); + var contextAccessor = new Mock(); + contextAccessor.Setup(c => c.WorkContext).Returns(workContext.Object); + + _controller = new AccountController( + new Mock().Object, + new Mock().Object, + contextAccessor.Object, + new Mock().Object, + _groupServiceMock.Object, + _customerManagerServiceMock.Object, + new Mock().Object, + _mediatorMock.Object, + new Mock().Object, + new CaptchaSettings(), + _customerSettings, + new CustomerConfig()) { + ControllerContext = new ControllerContext { HttpContext = HtmlRequest() } + }; + } + + private static DefaultHttpContext HtmlRequest() + { + var httpContext = new DefaultHttpContext(); + httpContext.Request.Headers.Accept = "text/html"; + return httpContext; + } + + private void VerifyNoCommand() + { + _mediatorMock.Verify(m => m.Send(It.IsAny(), It.IsAny()), Times.Never); + _mediatorMock.Verify(m => m.Send(It.IsAny(), It.IsAny()), Times.Never); + _mediatorMock.Verify(m => m.Send(It.IsAny(), It.IsAny()), Times.Never); + } + + private static void AssertRedirectsTo(string route, IActionResult result) + { + Assert.IsInstanceOfType(result); + Assert.AreEqual(route, ((RedirectToRouteResult)result).RouteName); + } + + [TestMethod] + public async Task SubAccountActions_TabHidden_RedirectAndChangeNothing() + { + _customerSettings.HideSubAccountsTab = true; + + AssertRedirectsTo("CustomerInfo", await _controller.SubAccounts()); + AssertRedirectsTo("CustomerInfo", await _controller.SubAccountAdd()); + AssertRedirectsTo("CustomerInfo", await _controller.SubAccountAdd(new SubAccountCreateModel())); + AssertRedirectsTo("CustomerInfo", await _controller.SubAccountEdit("c1")); + AssertRedirectsTo("CustomerInfo", await _controller.SubAccountEdit(new SubAccountEditModel())); + AssertRedirectsTo("CustomerInfo", await _controller.SubAccountDelete("c1")); + VerifyNoCommand(); + } + + [TestMethod] + public async Task SubAccountActions_NotAnOwner_ChallengeAndChangeNothing() + { + _groupServiceMock.Setup(g => g.IsOwner(_customer)).ReturnsAsync(false); + + Assert.IsInstanceOfType(await _controller.SubAccountAdd(new SubAccountCreateModel())); + Assert.IsInstanceOfType(await _controller.SubAccountEdit(new SubAccountEditModel())); + Assert.IsInstanceOfType(await _controller.SubAccountDelete("c1")); + VerifyNoCommand(); + } + + [TestMethod] + public async Task SubAccountEdit_AccountNotAvailable_RedirectsToTheList() + { + _mediatorMock.Setup(m => m.Send(It.IsAny(), It.IsAny())) + .ReturnsAsync((SubAccountEditModel)null); + + AssertRedirectsTo("CustomerSubAccounts", await _controller.SubAccountEdit("c1")); + } + + [TestMethod] + public async Task ChangePassword_ChangesThePasswordOfTheSignedInAccount() + { + await _controller.ChangePassword(new ChangePasswordModel { OldPassword = "old", NewPassword = "new" }); + + _customerManagerServiceMock.Verify(m => m.ChangePassword(_customer, It.IsAny(), "new"), + Times.Once); + _customerManagerServiceMock.Verify(m => m.ChangePassword(It.IsAny(), + It.IsAny()), Times.Never); + } +} diff --git a/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs b/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs index 943686c5ed..ef019d7d77 100644 --- a/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs +++ b/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs @@ -27,6 +27,7 @@ namespace Grand.Web.Tests.Validators.Customers; public class CustomerValidatorsStoreScopingTests { private const string StoreId = "store-1"; + private const string OwnerId = "owner"; private static Mock Translation() { @@ -38,7 +39,9 @@ private static Mock Translation() private static IContextAccessor Context(string currentCustomerEmail = "owner@x.com") { var work = new Mock(); - work.Setup(w => w.CurrentCustomer).Returns(new Customer { Email = currentCustomerEmail }); + work.Setup(w => w.CurrentCustomer).Returns(new Customer { + Id = OwnerId, Email = currentCustomerEmail, StoreId = StoreId + }); var store = new Mock(); store.Setup(s => s.CurrentStore).Returns(new Grand.Domain.Stores.Store { Id = StoreId }); var ctx = new Mock(); @@ -170,23 +173,150 @@ await validator.ValidateAsync(new SubAccountCreateModel { } [TestMethod] - public async Task SubAccountEditValidator_ScopesEmailLookupToCurrentStore_WhenEmailChanged() + public async Task SubAccountEditValidator_ScopesEmailLookupToTheAccountStore_WhenEmailChanged() + { + var (validator, customerService) = SubAccountEdit(new Customer { + Id = "c1", Email = "old@x.com", StoreId = "store-2", OwnerId = OwnerId + }); + + await validator.ValidateAsync(SubAccountEditModel("new@x.com")); + + //the account's store, as the current store (store-1) is not where this address has to stay unique + customerService.Verify(c => c.GetCustomerByEmail("new@x.com", "store-2"), Times.Once); + } + + [TestMethod] + public async Task SubAccountEditValidator_ChecksTheAddressAsItWillBeStored() + { + var (validator, customerService) = SubAccountEdit(new Customer { + Id = "c1", Email = "old@x.com", StoreId = StoreId, OwnerId = OwnerId + }); + customerService.Setup(c => c.GetCustomerByEmail("taken@x.com", StoreId)) + .ReturnsAsync(new Customer { Id = "other" }); + + var result = await validator.ValidateAsync(SubAccountEditModel(" Taken@X.com ")); + + Assert.IsTrue(result.Errors.Any(e => e.ErrorMessage == "Account.EmailUsernameErrors.EmailAlreadyExists")); + } + + [TestMethod] + public async Task SubAccountEditValidator_RepairingCaseChecksTheAddress_WhenChangesAreNotAllowed() + { + var (validator, customerService) = SubAccountEdit(new Customer { + Id = "c1", Email = "Taken@X.com", StoreId = StoreId, OwnerId = OwnerId + }, new CustomerSettings { AllowUsersToChangeEmail = false }); + customerService.Setup(c => c.GetCustomerByEmail("taken@x.com", StoreId)) + .ReturnsAsync(new Customer { Id = "other" }); + + var result = await validator.ValidateAsync(SubAccountEditModel("Taken@X.com")); + + Assert.IsTrue(result.Errors.Any(e => e.ErrorMessage == "Account.EmailUsernameErrors.EmailAlreadyExists")); + } + + [TestMethod] + [DataRow("another-owner", false)] + [DataRow("", false)] + [DataRow(OwnerId, true)] + public async Task SubAccountEditValidator_RejectsDeletedOrNotOwnedAccount(string ownerId, bool deleted) + { + var (validator, _) = SubAccountEdit(new Customer { + Id = "c1", Email = "sub@x.com", StoreId = StoreId, OwnerId = ownerId, Deleted = deleted + }); + + var result = await validator.ValidateAsync(SubAccountEditModel("sub@x.com")); + + Assert.IsTrue(result.Errors.Any(e => e.ErrorMessage == "You are not owner of this account")); + } + + [TestMethod] + public async Task SubAccountEditValidator_MissingEmailIsReportedWithoutThrowing() + { + var (validator, _) = SubAccountEdit(new Customer { + Id = "c1", Email = "sub@x.com", StoreId = StoreId, OwnerId = OwnerId + }); + + var result = await validator.ValidateAsync(SubAccountEditModel(null)); + + Assert.IsTrue(result.Errors.Any(e => e.ErrorMessage == "Account.Fields.Email.Required")); + } + + [TestMethod] + public async Task CustomerInfoValidator_ChecksTheAddressAsItWillBeStored() { var customerService = new Mock(); - customerService.Setup(c => c.GetCustomerById("c1")) - .ReturnsAsync(new Customer { Id = "c1", Email = "old@x.com" }); + customerService.Setup(c => c.GetCustomerByEmail("taken@x.com", StoreId)) + .ReturnsAsync(new Customer { Id = "other" }); + + var result = await CustomerInfo(customerService, new CustomerSettings { AllowUsersToChangeEmail = true }) + .ValidateAsync(new CustomerInfoModel { Email = " Taken@X.com " }); + + Assert.IsTrue(result.Errors.Any(e => e.ErrorMessage == "Account.EmailUsernameErrors.EmailAlreadyExists")); + } + + [TestMethod] + public async Task CustomerInfoValidator_ChecksTheUsernameAsItWillBeStored() + { + var customerService = new Mock(); + customerService.Setup(c => c.GetCustomerByUsername("taken", StoreId)) + .ReturnsAsync(new Customer { Id = "other" }); + + var result = await CustomerInfo(customerService, new CustomerSettings { + UsernamesEnabled = true, AllowUsersToChangeUsernames = true + }).ValidateAsync(new CustomerInfoModel { Email = "old@x.com", Username = " Taken " }); + + Assert.IsTrue(result.Errors.Any(e => e.ErrorMessage == "Account.EmailUsernameErrors.UsernameAlreadyExists")); + } + + private static (SubAccountEditValidator validator, Mock customerService) SubAccountEdit( + Customer account, CustomerSettings settings = null) + { + var customerService = new Mock(); + customerService.Setup(c => c.GetCustomerById(account.Id)).ReturnsAsync(account); var groupService = new Mock(); groupService.Setup(g => g.IsRegistered(It.IsAny())).ReturnsAsync(true); - var validator = new SubAccountEditValidator( + return (new SubAccountEditValidator( new List>(), customerService.Object, groupService.Object, Translation().Object, - Context(), new CustomerSettings { AllowUsersToChangeEmail = true }, PerStoreOn().Config); + Context(), settings ?? new CustomerSettings { AllowUsersToChangeEmail = true }, PerStoreOn().Config), + customerService); + } - await validator.ValidateAsync(new SubAccountEditModel { - Id = "c1", Email = "new@x.com", FirstName = "A", LastName = "B" - }); + private static SubAccountEditModel SubAccountEditModel(string email) => new() { + Id = "c1", Email = email, FirstName = "A", LastName = "B" + }; - customerService.Verify(c => c.GetCustomerByEmail(It.IsAny(), StoreId), Times.AtLeastOnce); + [TestMethod] + public async Task CustomerInfoValidator_LookupReturningTheAccountItself_IsNotAConflict() + { + //a lookup that ignores case finds the account whose address differs only in case + var customerService = new Mock(); + customerService.Setup(c => c.GetCustomerByEmail("old@x.com", StoreId)) + .ReturnsAsync(new Customer { Id = OwnerId }); + customerService.Setup(c => c.GetCustomerByUsername("taken", StoreId)) + .ReturnsAsync(new Customer { Id = OwnerId }); + + var result = await CustomerInfo(customerService, new CustomerSettings { + AllowUsersToChangeEmail = false, UsernamesEnabled = true, AllowUsersToChangeUsernames = true + }, "Old@X.com").ValidateAsync(new CustomerInfoModel { Email = "Old@X.com", Username = "Taken" }); + + Assert.IsFalse(result.Errors.Any(e => e.ErrorMessage == "Account.EmailUsernameErrors.EmailAlreadyExists")); + Assert.IsFalse(result.Errors.Any(e => e.ErrorMessage == "Account.EmailUsernameErrors.UsernameAlreadyExists")); + } + + private static CustomerInfoValidator CustomerInfo(Mock customerService, CustomerSettings settings, + string currentEmail = "old@x.com") + { + var mediator = new Mock(); + mediator.Setup(m => m.Send(It.IsAny(), It.IsAny())) + .ReturnsAsync(new List()); + var attrParser = new Mock(); + attrParser.Setup(p => p.GetAttributeWarnings(It.IsAny>(), It.IsAny())) + .ReturnsAsync(new List()); + + return new CustomerInfoValidator( + new List>(), + Context(currentEmail), customerService.Object, mediator.Object, attrParser.Object, + Translation().Object, new Mock().Object, settings, PerStoreOn().Config); } } diff --git a/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountDeleteCommandHandler.cs b/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountDeleteCommandHandler.cs index 31026f5bfc..a1aed5d9f1 100644 --- a/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountDeleteCommandHandler.cs +++ b/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountDeleteCommandHandler.cs @@ -19,7 +19,7 @@ public SubAccountDeleteCommandHandler(ICustomerService customerService) ArgumentNullException.ThrowIfNull(request.CurrentCustomer); var customer = await _customerService.GetCustomerById(request.CustomerId); - if (customer == null || customer.OwnerId != request.CurrentCustomer.Id) + if (customer == null || customer.Deleted || customer.OwnerId != request.CurrentCustomer.Id) return (false, "You are not owner of this account"); //delete customer diff --git a/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountEditCommandHandler.cs b/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountEditCommandHandler.cs index 32555ba9c5..472a2996c8 100644 --- a/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountEditCommandHandler.cs +++ b/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountEditCommandHandler.cs @@ -1,5 +1,4 @@ using Grand.Business.Core.Interfaces.Customers; -using Grand.Business.Core.Utilities.Customers; using Grand.Domain.Customers; using Grand.Web.Commands.Models.Customers; using Grand.Mediator; @@ -26,19 +25,29 @@ public async Task Handle(SubAccountEditCommand request, CancellationToken { ArgumentNullException.ThrowIfNull(request.CurrentCustomer); var customer = await _customerService.GetCustomerById(request.EditModel.Id); - - //update email - if (customer.Email != request.EditModel.Email.ToLower() && _customerSettings.AllowUsersToChangeEmail) + if (customer == null || customer.Deleted || customer.OwnerId != request.CurrentCustomer.Id) + return false; + + //update email - stored lowercased like every other write, or the lowercased lookups miss it. An address + //differing only in case is the same address, so it is repaired even when changing it is not allowed + var email = request.EditModel.Email.Trim().ToLowerInvariant(); + var sameAddress = string.Equals(customer.Email, email, StringComparison.OrdinalIgnoreCase); + if (customer.Email != email && (sameAddress || _customerSettings.AllowUsersToChangeEmail)) { - customer.Email = request.EditModel.Email; - await _customerService.UpdateCustomerField(customer, x => x.Email, request.EditModel.Email); + customer.Email = email; + await _customerService.UpdateCustomerField(customer, x => x.Email, email); + //without usernames the username is the email (see SubAccountAddCommandHandler) + if (!_customerSettings.UsernamesEnabled) + { + customer.Username = email; + await _customerService.UpdateCustomerField(customer, x => x.Username, email); + } } - //update password (scope the lookup to the sub-account's store - safe with or without per-store identity) + //update password - of this account, the one the ownership check above is about if (!string.IsNullOrEmpty(request.EditModel.Password)) - await _customerManagerService.ChangePassword( - new ChangePasswordRequest(customer.Email, _customerSettings.DefaultPasswordFormat, - request.EditModel.Password), customer.StoreId); + await _customerManagerService.ChangePassword(customer, _customerSettings.DefaultPasswordFormat, + request.EditModel.Password); //update active customer.Active = request.EditModel.Active; diff --git a/src/Web/Grand.Web/Commands/Handler/Customers/UpdateCustomerInfoCommandHandler.cs b/src/Web/Grand.Web/Commands/Handler/Customers/UpdateCustomerInfoCommandHandler.cs index 08b64e22c5..967088e3bf 100644 --- a/src/Web/Grand.Web/Commands/Handler/Customers/UpdateCustomerInfoCommandHandler.cs +++ b/src/Web/Grand.Web/Commands/Handler/Customers/UpdateCustomerInfoCommandHandler.cs @@ -38,26 +38,36 @@ public UpdateCustomerInfoCommandHandler( public async Task Handle(UpdateCustomerInfoCommand request, CancellationToken cancellationToken) { - //username + //username - stored lowercased like every other write, or the lowercased lookups miss it if (_customerSettings.UsernamesEnabled && _customerSettings.AllowUsersToChangeUsernames) - if (!request.Customer.Username.Equals(request.Model.Username.Trim(), StringComparison.OrdinalIgnoreCase)) + { + var username = request.Model.Username.Trim().ToLowerInvariant(); + if (request.Customer.Username != username) { //change username - request.Customer.Username = request.Model.Username.Trim(); - await _customerService.UpdateCustomerField(request.Customer, x => x.Username, - request.Model.Username.Trim()); + request.Customer.Username = username; + await _customerService.UpdateCustomerField(request.Customer, x => x.Username, username); //re-authenticate if (request.OriginalCustomerIfImpersonated == null) await _authenticationService.SignIn(request.Customer, true); } + } - //email - if (!request.Customer.Email.Equals(request.Model.Email.Trim(), StringComparison.OrdinalIgnoreCase) && - _customerSettings.AllowUsersToChangeEmail) + //email - stored lowercased for the same reason. An address differing only in case is the same address, + //so it is repaired even where changing it is not allowed + var email = request.Model.Email.Trim().ToLowerInvariant(); + var sameAddress = string.Equals(request.Customer.Email, email, StringComparison.OrdinalIgnoreCase); + if (request.Customer.Email != email && (sameAddress || _customerSettings.AllowUsersToChangeEmail)) { //change email - request.Customer.Email = request.Model.Email.Trim(); - await _customerService.UpdateCustomerField(request.Customer, x => x.Email, request.Model.Email.Trim()); + request.Customer.Email = email; + await _customerService.UpdateCustomerField(request.Customer, x => x.Email, email); + //without usernames the username is the email (as registration sets it) + if (!_customerSettings.UsernamesEnabled) + { + request.Customer.Username = email; + await _customerService.UpdateCustomerField(request.Customer, x => x.Username, email); + } //re-authenticate (if usernames are disabled) //do not authenticate users in impersonation mode if (request.OriginalCustomerIfImpersonated == null) diff --git a/src/Web/Grand.Web/Controllers/AccountController.cs b/src/Web/Grand.Web/Controllers/AccountController.cs index 3317f5fcca..442263ef91 100644 --- a/src/Web/Grand.Web/Controllers/AccountController.cs +++ b/src/Web/Grand.Web/Controllers/AccountController.cs @@ -924,10 +924,8 @@ public virtual async Task ChangePassword(ChangePasswordModel mode { if (!ModelState.IsValid) return View(model); - var changePasswordRequest = new ChangePasswordRequest(_contextAccessor.WorkContext.CurrentCustomer.Email, - _customerSettings.DefaultPasswordFormat, model.NewPassword, model.OldPassword); - - await _customerManagerService.ChangePassword(changePasswordRequest); + await _customerManagerService.ChangePassword(_contextAccessor.WorkContext.CurrentCustomer, + _customerSettings.DefaultPasswordFormat, model.NewPassword); var customer = await _customerService.GetCustomerById(_contextAccessor.WorkContext.CurrentCustomer.Id); //sign in @@ -1111,9 +1109,7 @@ await _customerService.UpdateUserField(_contextAccessor.WorkContext.Curr #region My account / Sub accounts - [HttpGet] - [CustomerGroupAuthorize(SystemCustomerGroupNames.Registered)] - public virtual async Task SubAccounts() + private async Task SubAccountsDenied() { if (!await _groupService.IsOwner(_contextAccessor.WorkContext.CurrentCustomer)) return Challenge(); @@ -1121,6 +1117,16 @@ public virtual async Task SubAccounts() if (_customerSettings.HideSubAccountsTab) return RedirectToRoute("CustomerInfo"); + return null; + } + + [HttpGet] + [CustomerGroupAuthorize(SystemCustomerGroupNames.Registered)] + public virtual async Task SubAccounts() + { + var denied = await SubAccountsDenied(); + if (denied != null) return denied; + var model = await _mediator.Send(new GetSubAccounts { Customer = _contextAccessor.WorkContext.CurrentCustomer }); return View(model); @@ -1130,8 +1136,8 @@ public virtual async Task SubAccounts() [CustomerGroupAuthorize(SystemCustomerGroupNames.Registered)] public virtual async Task SubAccountAdd() { - if (!await _groupService.IsOwner(_contextAccessor.WorkContext.CurrentCustomer)) - return Challenge(); + var denied = await SubAccountsDenied(); + if (denied != null) return denied; var model = new SubAccountCreateModel { Active = true @@ -1143,8 +1149,8 @@ public virtual async Task SubAccountAdd() [CustomerGroupAuthorize(SystemCustomerGroupNames.Registered)] public virtual async Task SubAccountAdd(SubAccountCreateModel model) { - if (!await _groupService.IsOwner(_contextAccessor.WorkContext.CurrentCustomer)) - return Challenge(); + var denied = await SubAccountsDenied(); + if (denied != null) return denied; if (!ModelState.IsValid) return View(model); @@ -1161,11 +1167,13 @@ await _mediator.Send(new SubAccountAddCommand { [CustomerGroupAuthorize(SystemCustomerGroupNames.Registered)] public virtual async Task SubAccountEdit(string id) { - if (!await _groupService.IsOwner(_contextAccessor.WorkContext.CurrentCustomer)) - return Challenge(); + var denied = await SubAccountsDenied(); + if (denied != null) return denied; var model = await _mediator.Send(new GetSubAccount { CustomerId = id, CurrentCustomer = _contextAccessor.WorkContext.CurrentCustomer }); + if (model == null) + return RedirectToRoute("CustomerSubAccounts"); return View(model); } @@ -1174,8 +1182,8 @@ public virtual async Task SubAccountEdit(string id) [CustomerGroupAuthorize(SystemCustomerGroupNames.Registered)] public virtual async Task SubAccountEdit(SubAccountEditModel model) { - if (!await _groupService.IsOwner(_contextAccessor.WorkContext.CurrentCustomer)) - return Challenge(); + var denied = await SubAccountsDenied(); + if (denied != null) return denied; if (!ModelState.IsValid) return View(model); @@ -1192,8 +1200,8 @@ public virtual async Task SubAccountEdit(SubAccountEditModel mode [CustomerGroupAuthorize(SystemCustomerGroupNames.Registered)] public virtual async Task SubAccountDelete(string id) { - if (!await _groupService.IsOwner(_contextAccessor.WorkContext.CurrentCustomer)) - return Challenge(); + var denied = await SubAccountsDenied(); + if (denied != null) return denied; var result = await _mediator.Send(new SubAccountDeleteCommand { CurrentCustomer = _contextAccessor.WorkContext.CurrentCustomer, diff --git a/src/Web/Grand.Web/Features/Handlers/Customers/GetSubAccountHandler.cs b/src/Web/Grand.Web/Features/Handlers/Customers/GetSubAccountHandler.cs index 0be4202e6a..45e11bf93f 100644 --- a/src/Web/Grand.Web/Features/Handlers/Customers/GetSubAccountHandler.cs +++ b/src/Web/Grand.Web/Features/Handlers/Customers/GetSubAccountHandler.cs @@ -20,18 +20,17 @@ public async Task Handle(GetSubAccount request, Cancellatio { ArgumentNullException.ThrowIfNull(request.CurrentCustomer); - var model = new SubAccountEditModel(); - + //null for a missing, deleted or someone else's account, so the caller can tell it from an empty form var subaccount = await _customerService.GetCustomerById(request.CustomerId); - if (subaccount != null && subaccount.OwnerId == request.CurrentCustomer.Id) - model = new SubAccountEditModel { - Id = subaccount.Id, - Email = subaccount.Email, - Active = subaccount.Active, - FirstName = subaccount.GetUserFieldFromEntity(SystemCustomerFieldNames.FirstName), - LastName = subaccount.GetUserFieldFromEntity(SystemCustomerFieldNames.LastName) - }; + if (subaccount == null || subaccount.Deleted || subaccount.OwnerId != request.CurrentCustomer.Id) + return null; - return model; + return new SubAccountEditModel { + Id = subaccount.Id, + Email = subaccount.Email, + Active = subaccount.Active, + FirstName = subaccount.GetUserFieldFromEntity(SystemCustomerFieldNames.FirstName), + LastName = subaccount.GetUserFieldFromEntity(SystemCustomerFieldNames.LastName) + }; } } \ No newline at end of file diff --git a/src/Web/Grand.Web/Validators/Customer/CustomerInfoValidator.cs b/src/Web/Grand.Web/Validators/Customer/CustomerInfoValidator.cs index 528e915db5..ffb205b570 100644 --- a/src/Web/Grand.Web/Validators/Customer/CustomerInfoValidator.cs +++ b/src/Web/Grand.Web/Validators/Customer/CustomerInfoValidator.cs @@ -26,10 +26,6 @@ public CustomerInfoValidator( CustomerConfig customerConfig) : base(validators) { - var storeId = customerConfig.RegisterCustomersPerStore - ? contextAccessor.StoreContext.CurrentStore.Id - : ""; - RuleFor(x => x.Email).NotEmpty() .WithMessage(translationService.GetResource("Account.Fields.Email.Required")); RuleFor(x => x.Email).EmailAddress().WithMessage(translationService.GetResource("Common.WrongEmail")); @@ -126,29 +122,37 @@ public CustomerInfoValidator( contextAccessor.StoreContext.CurrentStore.Id); foreach (var error in customerAttributeWarnings) context.AddFailure(error); - if (contextAccessor.WorkContext.CurrentCustomer.Email != x.Email.ToLower() && customerSettings.AllowUsersToChangeEmail) + //the same values the command stores, checked where the account lives + var currentCustomer = contextAccessor.WorkContext.CurrentCustomer; + var storeId = customerConfig.RegisterCustomersPerStore ? currentCustomer.StoreId : ""; + + var email = x.Email?.Trim().ToLowerInvariant(); + var sameAddress = string.Equals(currentCustomer.Email, email, StringComparison.OrdinalIgnoreCase); + if (!string.IsNullOrEmpty(email) && currentCustomer.Email != email && + (sameAddress || customerSettings.AllowUsersToChangeEmail)) { - if (!CommonHelper.IsValidEmail(x.Email)) + if (!CommonHelper.IsValidEmail(email)) context.AddFailure( translationService.GetResource("Account.EmailUsernameErrors.NewEmailIsNotValid")); - if (x.Email.Length > 100) + if (email.Length > 100) context.AddFailure(translationService.GetResource("Account.EmailUsernameErrors.EmailTooLong")); - var customer2 = await customerService.GetCustomerByEmail(x.Email, storeId); - if (customer2 != null) + var customer2 = await customerService.GetCustomerByEmail(email, storeId); + if (customer2 != null && customer2.Id != currentCustomer.Id) context.AddFailure( translationService.GetResource("Account.EmailUsernameErrors.EmailAlreadyExists")); } - if (customerSettings.UsernamesEnabled && customerSettings.AllowUsersToChangeUsernames && - contextAccessor.WorkContext.CurrentCustomer.Username != x.Username.ToLower()) + var username = x.Username?.Trim().ToLowerInvariant(); + if (!string.IsNullOrEmpty(username) && customerSettings.UsernamesEnabled && + customerSettings.AllowUsersToChangeUsernames && currentCustomer.Username != username) { - if (x.Username.ToLower().Length > 100) + if (username.Length > 100) context.AddFailure(translationService.GetResource("Account.EmailUsernameErrors.UsernameTooLong")); - var customer2 = await customerService.GetCustomerByUsername(x.Username.ToLower(), storeId); - if (customer2 != null) + var customer2 = await customerService.GetCustomerByUsername(username, storeId); + if (customer2 != null && customer2.Id != currentCustomer.Id) context.AddFailure( translationService.GetResource("Account.EmailUsernameErrors.UsernameAlreadyExists")); } diff --git a/src/Web/Grand.Web/Validators/Customer/SubAccountEditValidator.cs b/src/Web/Grand.Web/Validators/Customer/SubAccountEditValidator.cs index c719ff68c2..b96c504530 100644 --- a/src/Web/Grand.Web/Validators/Customer/SubAccountEditValidator.cs +++ b/src/Web/Grand.Web/Validators/Customer/SubAccountEditValidator.cs @@ -22,10 +22,6 @@ public SubAccountEditValidator( CustomerConfig customerConfig) : base(validators) { - var storeId = customerConfig.RegisterCustomersPerStore - ? contextAccessor.StoreContext.CurrentStore.Id - : ""; - RuleFor(x => x.Email).NotEmpty().WithMessage(translationService.GetResource("Account.Fields.Email.Required")); RuleFor(x => x.Email).EmailAddress().WithMessage(translationService.GetResource("Common.WrongEmail")); @@ -43,36 +39,38 @@ public SubAccountEditValidator( { var customer = await customerService.GetCustomerById(x.Id); - switch (customer) + //ownership first: a missing id, a deleted account and someone else's account get the same answer + if (customer == null || customer.Deleted || + customer.OwnerId != contextAccessor.WorkContext.CurrentCustomer.Id) { - case null: - context.AddFailure( - translationService.GetResource("Account.NoExists")); - break; - case { Deleted: true }: - context.AddFailure( - translationService.GetResource("Account.DeleteAccount")); - break; - case not null when !await groupService.IsRegistered(customer): - context.AddFailure( - translationService.GetResource( - "Account is not registered")); - break; + context.AddFailure("You are not owner of this account"); + return; } - if (customer != null && customer.OwnerId != contextAccessor.WorkContext.CurrentCustomer.Id) - context.AddFailure("You are not owner of this account"); + if (!await groupService.IsRegistered(customer)) + { + context.AddFailure(translationService.GetResource("Account is not registered")); + return; + } + + //an empty email is already reported by the NotEmpty rule + if (string.IsNullOrWhiteSpace(x.Email)) + return; - if (customer != null && customer.Email != x.Email.ToLower() && customerSettings.AllowUsersToChangeEmail) + //the same address the command stores, checked where the account lives + var email = x.Email.Trim().ToLowerInvariant(); + var sameAddress = string.Equals(customer.Email, email, StringComparison.OrdinalIgnoreCase); + if (customer.Email != email && (sameAddress || customerSettings.AllowUsersToChangeEmail)) { - if (!CommonHelper.IsValidEmail(x.Email)) + if (!CommonHelper.IsValidEmail(email)) context.AddFailure( translationService.GetResource("Account.EmailUsernameErrors.NewEmailIsNotValid")); - if (x.Email.Length > 100) + if (email.Length > 100) context.AddFailure(translationService.GetResource("Account.EmailUsernameErrors.EmailTooLong")); - var customer2 = await customerService.GetCustomerByEmail(x.Email, storeId); + var storeId = customerConfig.RegisterCustomersPerStore ? customer.StoreId : ""; + var customer2 = await customerService.GetCustomerByEmail(email, storeId); if (customer2 != null && customer.Id != customer2.Id) context.AddFailure( translationService.GetResource("Account.EmailUsernameErrors.EmailAlreadyExists"));