diff --git a/src/Business/Grand.Business.Authentication/Services/Scim/ScimAccountLock.cs b/src/Business/Grand.Business.Authentication/Services/Scim/ScimAccountLock.cs index 6086adbd2..bfb002064 100644 --- a/src/Business/Grand.Business.Authentication/Services/Scim/ScimAccountLock.cs +++ b/src/Business/Grand.Business.Authentication/Services/Scim/ScimAccountLock.cs @@ -1,6 +1,5 @@ using Grand.Business.Core.Interfaces.Authentication; using Grand.Business.Core.Interfaces.Customers; -using Grand.Business.Core.Utilities.Customers; using Grand.Data; using Grand.Domain.Customers; using Microsoft.Extensions.Logging; @@ -62,7 +61,7 @@ await links.DeleteManyAsync(x => //also writes a new password token var password = Convert.ToBase64String(RandomNumberGenerator.GetBytes(32)); - await customerManagerService.ChangePassword(new ChangePasswordRequest(customer.Email, PasswordFormat.Hashed, password)); + await customerManagerService.ChangePassword(customer, PasswordFormat.Hashed, password); logger.LogInformation("SCIM reset the credentials of customer {CustomerId}", customer.Id); } diff --git a/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerManagerService.cs b/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerManagerService.cs index bdacc0466..ffcbfac06 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerManagerService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerManagerService.cs @@ -42,4 +42,12 @@ Task LoginCustomer(string usernameOrEmail, string password /// 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 + Task ChangePassword(Customer customer, PasswordFormat passwordFormat, string newPassword); } \ 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 2106a6890..ee811c3b8 100644 --- a/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerService.cs +++ b/src/Business/Grand.Business.Core/Interfaces/Customers/ICustomerService.cs @@ -193,6 +193,12 @@ Task UpdateCustomerField(string customerId, /// Customer Task UpdateActive(Customer customer); + /// + /// Updates the password fields of the customer, and nothing else + /// + /// Customer + Task UpdateCustomerPassword(Customer 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 604b8fe92..5322183a2 100644 --- a/src/Business/Grand.Business.Customers/Services/CustomerManagerService.cs +++ b/src/Business/Grand.Business.Customers/Services/CustomerManagerService.cs @@ -193,33 +193,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 99d37d843..cee0a484d 100644 --- a/src/Business/Grand.Business.Customers/Services/CustomerService.cs +++ b/src/Business/Grand.Business.Customers/Services/CustomerService.cs @@ -574,6 +574,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.Authentication.Tests/Services/Scim/ScimAccountLockTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimAccountLockTests.cs new file mode 100644 index 000000000..728ed666e --- /dev/null +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimAccountLockTests.cs @@ -0,0 +1,21 @@ +using Grand.Domain.Customers; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Business.Authentication.Tests.Services.Scim; + +[TestClass] +public class ScimAccountLockTests +{ + [TestMethod] + public async Task ResetCredentials_ChangesThePasswordOfThatAccount() + { + var fixture = new ScimFixture(); + var customer = await fixture.AddCustomer("jan@example.com"); + + await fixture.AccountLock().ResetCredentials(customer); + + fixture.CustomerManager.Verify(x => x.ChangePassword(customer, PasswordFormat.Hashed, It.IsAny()), + Times.Once); + } +} diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimFixture.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimFixture.cs index 7a9e2c054..2add37669 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimFixture.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimFixture.cs @@ -112,11 +112,11 @@ public ScimFixture() r.Customer.Groups.Add(Registered.Id); await Customers.UpdateAsync(r.Customer); }); - CustomerManager.Setup(x => x.ChangePassword(It.IsAny(), It.IsAny())) - .Returns(async (ChangePasswordRequest r, string _) => + CustomerManager.Setup(x => x.ChangePassword(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(async (Customer c, PasswordFormat _, string newPassword) => { - var stored = await Customers.GetOneAsync(x => x.Email == r.Email); - stored.Password = "hash:" + r.NewPassword; + var stored = await Customers.GetOneAsync(x => x.Id == c.Id); + stored.Password = "hash:" + newPassword; stored.UserFields = stored.UserFields.Where(x => x.Key != SystemCustomerFieldNames.PasswordToken).ToList(); stored.UserFields.Add(new UserField { Key = SystemCustomerFieldNames.PasswordToken, Value = Guid.NewGuid().ToString(), StoreId = "" diff --git a/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimUserServiceTests.cs b/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimUserServiceTests.cs index 17d2900af..19e3675d6 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimUserServiceTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimUserServiceTests.cs @@ -484,7 +484,7 @@ public async Task Create_ExistingStaffAccount_KeepsCredentials() await _f.UserService().Create(ScimFixture.Data()); Assert.AreEqual("hash:staff", (await _f.Reload(staff.Id)).Password); - _f.CustomerManager.Verify(x => x.ChangePassword(It.IsAny(), It.IsAny()), Times.Never); + _f.CustomerManager.Verify(x => x.ChangePassword(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } [TestMethod] diff --git a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs index 74ae4d2cf..37b79b143 100644 --- a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs +++ b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs @@ -168,6 +168,31 @@ 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_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 0eb60a350..a60a203ef 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; @@ -523,6 +524,43 @@ public async Task UpdateCustomerInAdminPanel_SavesTradeCredit() Assert.AreEqual(45, updated.PaymentTermDays); } + [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 f5156f3bd..3412664ef 100644 --- a/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs +++ b/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs @@ -45,6 +45,15 @@ private Customer SubAccount(string ownerId = "owner") return customer; } + 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, SubAccountRole role = SubAccountRole.Buyer, string limit = null) => new() { CurrentCustomer = new Customer { Id = "owner" }, @@ -63,6 +72,8 @@ private void VerifyNothingWritten() 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); _customerServiceMock.Verify(c => c.UpdateCustomerField(It.IsAny(), It.IsAny>>(), It.IsAny()), Times.Never); _customerServiceMock.Verify(c => c.UpdateCustomerField(It.IsAny(), @@ -129,16 +140,17 @@ public async Task Handle_Approver_ClearsLimit() } [TestMethod] - public async Task Handle_WithPassword_ChangesPasswordScopedToCustomerStore() + public async Task Handle_WithPassword_ChangesPasswordOfTheEditedAccount() { - SubAccount(); + 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] @@ -151,6 +163,8 @@ public async Task Handle_WithoutPassword_DoesNotChangePassword() 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] @@ -210,14 +224,14 @@ public async Task Handle_MixedCaseEmail_IsStoredLowercased() } [TestMethod] - public async Task Handle_NewEmailAndPassword_ChangesPasswordOfTheNewEmail() + public async Task Handle_NewEmailAndPassword_ChangesPasswordOfTheEditedAccount() { - SubAccount(); + var customer = SubAccount(); await _handler.Handle(Command(email: "New@X.com", password: "newpass"), default); - _customerManagerServiceMock.Verify(m => m.ChangePassword( - It.Is(r => r.Email == "new@x.com"), "store-9"), Times.Once); + Assert.AreEqual("new@x.com", customer.Email); + VerifyPasswordChangedOn(customer); } [TestMethod] @@ -253,8 +267,7 @@ public async Task Handle_EmailStoredInMixedCaseByOlderEdits_IsRepairedEvenWhenCh await _handler.Handle(Command(email: "Sub@X.com", password: "newpass"), default); Assert.AreEqual("sub@x.com", customer.Email); - _customerManagerServiceMock.Verify(m => m.ChangePassword( - It.Is(r => r.Email == "sub@x.com"), "store-9"), Times.Once); + VerifyPasswordChangedOn(customer); } [TestMethod] diff --git a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs index 355552f57..2503757f0 100644 --- a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs +++ b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs @@ -87,6 +87,16 @@ public void Init() _controller.Url = url.Object; } + [TestMethod] + public async Task ChangePassword_ChangesThePasswordOfTheSignedInAccount() + { + await _controller.ChangePassword(new ChangePasswordModel { OldPassword = "old", NewPassword = "new" }, + new OidcLoginConfig()); + + _customerManager.Verify(m => m.ChangePassword(_currentCustomer, It.IsAny(), "new"), Times.Once); + _customerManager.Verify(m => m.ChangePassword(It.IsAny(), It.IsAny()), Times.Never); + } + [TestMethod] public async Task Login_ProviderOnly_RedirectsToProvider() { @@ -295,7 +305,8 @@ public async Task ChangePassword_ProviderOnly_Redirects() var post = await _controller.ChangePassword( new ChangePasswordModel { OldPassword = "a", NewPassword = "b", ConfirmNewPassword = "b" }, _providerOnly); Assert.AreEqual("CustomerInfo", ((RedirectToRouteResult)post).RouteName); - _customerManager.Verify(m => m.ChangePassword(It.IsAny()), Times.Never); + _customerManager.Verify(m => m.ChangePassword(It.IsAny(), It.IsAny()), Times.Never); + _customerManager.Verify(m => m.ChangePassword(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); } [TestMethod] @@ -306,7 +317,7 @@ public async Task ChangePassword_Mixed_ChangesPassword() new ChangePasswordModel { OldPassword = "a", NewPassword = "b", ConfirmNewPassword = "b" }, new OidcLoginConfig()); Assert.IsInstanceOfType(post); - _customerManager.Verify(m => m.ChangePassword(It.IsAny()), Times.Once); + _customerManager.Verify(m => m.ChangePassword(_currentCustomer, It.IsAny(), "b"), Times.Once); } [TestMethod] diff --git a/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs b/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs index bf2ec099d..16afed25a 100644 --- a/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs +++ b/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs @@ -38,7 +38,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 { Id = "owner", Email = currentCustomerEmail }); + work.Setup(w => w.CurrentCustomer).Returns(new Customer { + Id = "owner", 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(); @@ -172,23 +174,123 @@ 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 = "owner" + }); + + 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 = "owner" + }); + 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 = "owner" + }, 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] + public async Task CustomerInfoValidator_ChecksTheAddressAsItWillBeStored() { var customerService = new Mock(); - customerService.Setup(c => c.GetCustomerById("c1")) - .ReturnsAsync(new Customer { Id = "c1", Email = "old@x.com", OwnerId = "owner" }); + 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 = "owner" }); + customerService.Setup(c => c.GetCustomerByUsername("taken", StoreId)) + .ReturnsAsync(new Customer { Id = "owner" }); + + 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/SubAccountEditCommandHandler.cs b/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountEditCommandHandler.cs index 50ea10861..9cde6c28f 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.Web.Models.Customer; @@ -47,11 +46,10 @@ public async Task Handle(SubAccountEditCommand request, CancellationToken } } - //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/Controllers/AccountController.cs b/src/Web/Grand.Web/Controllers/AccountController.cs index b03f49709..c79d8e20f 100644 --- a/src/Web/Grand.Web/Controllers/AccountController.cs +++ b/src/Web/Grand.Web/Controllers/AccountController.cs @@ -1119,10 +1119,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 diff --git a/src/Web/Grand.Web/Validators/Customer/CustomerInfoValidator.cs b/src/Web/Grand.Web/Validators/Customer/CustomerInfoValidator.cs index 225599329..cc4a6ab48 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 fe2a66aa5..7f5497374 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")); @@ -72,16 +68,20 @@ public SubAccountEditValidator( if (string.IsNullOrWhiteSpace(x.Email)) return; - if (customer.Email != x.Email.Trim().ToLowerInvariant() && 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"));