From 71cf8383a3d527d2e785524dc8b2e45470073e86 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 1 Nov 2026 09:19:32 +0100 Subject: [PATCH 1/4] Check customer e-mails and change passwords in the account's store The storefront validators checked an e-mail or username as typed and in the current store, while the commands store it trimmed and lowercased on an account that belongs to its own store. They now check the stored form in the store of the account being edited, including when a case-only repair applies. A password change finds its account by e-mail, and with per-store identity one e-mail can belong to several stores. The customer's own change and the SCIM credentials reset now use the store of the account they act on, and a sub-account password is changed only when its e-mail resolves to that account. --- .../Services/Scim/ScimAccountLock.cs | 3 +- .../Services/Scim/ScimAccountLockTests.cs | 22 ++++ .../SubAccountEditCommandHandlerTests.cs | 15 +++ .../AccountControllerProviderOnlyTests.cs | 15 ++- .../CustomerValidatorsStoreScopingTests.cs | 103 ++++++++++++++++-- .../Customers/SubAccountEditCommandHandler.cs | 7 ++ .../Controllers/AccountController.cs | 4 +- .../Customer/CustomerInfoValidator.cs | 28 +++-- .../Customer/SubAccountEditValidator.cs | 16 +-- 9 files changed, 180 insertions(+), 33 deletions(-) create mode 100644 src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimAccountLockTests.cs diff --git a/src/Business/Grand.Business.Authentication/Services/Scim/ScimAccountLock.cs b/src/Business/Grand.Business.Authentication/Services/Scim/ScimAccountLock.cs index 6086adbd2..d08df1902 100644 --- a/src/Business/Grand.Business.Authentication/Services/Scim/ScimAccountLock.cs +++ b/src/Business/Grand.Business.Authentication/Services/Scim/ScimAccountLock.cs @@ -62,7 +62,8 @@ 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(new ChangePasswordRequest(customer.Email, PasswordFormat.Hashed, password), + customer.StoreId); logger.LogInformation("SCIM reset the credentials of customer {CustomerId}", customer.Id); } 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..249763ffb --- /dev/null +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimAccountLockTests.cs @@ -0,0 +1,22 @@ +using Grand.Business.Core.Utilities.Customers; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Business.Authentication.Tests.Services.Scim; + +[TestClass] +public class ScimAccountLockTests +{ + [TestMethod] + public async Task ResetCredentials_ChangesThePasswordInTheStoreOfTheAccount() + { + var fixture = new ScimFixture(); + var customer = await fixture.AddCustomer("jan@example.com"); + customer.StoreId = "store-9"; + + await fixture.AccountLock().ResetCredentials(customer); + + fixture.CustomerManager.Verify(x => x.ChangePassword( + It.Is(r => r.Email == "jan@example.com"), "store-9"), Times.Once); + } +} diff --git a/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs b/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs index f5156f3bd..ce5e02c50 100644 --- a/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs +++ b/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs @@ -42,9 +42,24 @@ private Customer SubAccount(string ownerId = "owner") Id = "c1", Email = "sub@x.com", Username = "sub@x.com", StoreId = "store-9", OwnerId = ownerId }; _customerServiceMock.Setup(c => c.GetCustomerById("c1")).ReturnsAsync(customer); + _customerServiceMock.Setup(c => c.GetCustomerByEmail(It.IsAny(), "store-9")).ReturnsAsync(customer); return customer; } + [TestMethod] + public async Task Handle_EmailLeadsToAnotherAccount_DoesNotChangePassword() + { + SubAccount(); + _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.IsFalse(result); + _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" }, diff --git a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs index 355552f57..c1e441033 100644 --- a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs +++ b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs @@ -87,6 +87,19 @@ public void Init() _controller.Url = url.Object; } + [TestMethod] + public async Task ChangePassword_IsScopedToTheStoreOfTheCustomer() + { + _currentCustomer.Email = "john@example.com"; + _currentCustomer.StoreId = "store-9"; + + await _controller.ChangePassword(new ChangePasswordModel { OldPassword = "old", NewPassword = "new" }, + new OidcLoginConfig()); + + _customerManager.Verify(m => m.ChangePassword( + It.Is(r => r.Email == "john@example.com"), "store-9"), Times.Once); + } + [TestMethod] public async Task Login_ProviderOnly_RedirectsToProvider() { @@ -306,7 +319,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(It.IsAny(), It.IsAny()), 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..6198b6b46 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,104 @@ 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); + private static CustomerInfoValidator CustomerInfo(Mock customerService, CustomerSettings settings) + { + 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("old@x.com"), 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..80f3d9536 100644 --- a/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountEditCommandHandler.cs +++ b/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountEditCommandHandler.cs @@ -49,9 +49,16 @@ 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) if (!string.IsNullOrEmpty(request.EditModel.Password)) + { + //the password change finds its account by email, which has to lead back to this account + var emailHolder = await _customerService.GetCustomerByEmail(customer.Email, customer.StoreId); + if (emailHolder?.Id != customer.Id) + return false; + await _customerManagerService.ChangePassword( new ChangePasswordRequest(customer.Email, _customerSettings.DefaultPasswordFormat, request.EditModel.Password), customer.StoreId); + } //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..7e3b17169 100644 --- a/src/Web/Grand.Web/Controllers/AccountController.cs +++ b/src/Web/Grand.Web/Controllers/AccountController.cs @@ -1122,7 +1122,9 @@ public virtual async Task ChangePassword(ChangePasswordModel mode var changePasswordRequest = new ChangePasswordRequest(_contextAccessor.WorkContext.CurrentCustomer.Email, _customerSettings.DefaultPasswordFormat, model.NewPassword, model.OldPassword); - await _customerManagerService.ChangePassword(changePasswordRequest); + //scope the lookup to the customer's store - the same e-mail may exist in another store with per-store identity + await _customerManagerService.ChangePassword(changePasswordRequest, + _contextAccessor.WorkContext.CurrentCustomer.StoreId); 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..413c6dad2 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,28 +122,36 @@ 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); + var customer2 = await customerService.GetCustomerByEmail(email, storeId); if (customer2 != null) 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); + var customer2 = await customerService.GetCustomerByUsername(username, storeId); if (customer2 != null) 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")); From 2ce6a664bd7a12f0fd0d6fb33868147d82bf6937 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 1 Nov 2026 09:49:01 +0100 Subject: [PATCH 2/4] Change a password on the account itself, not through its e-mail These callers already hold the account, but the password change looked it up again by e-mail, a value that can change between the two steps and is unique only as far as the application keeps it so. A new overload takes the customer and the existing method now resolves the e-mail and calls it. The sub-account edit no longer needs its check that the e-mail leads back to the account, nor can it skip the remaining edits without telling the owner. --- .../Services/Scim/ScimAccountLock.cs | 4 +- .../Customers/ICustomerManagerService.cs | 8 ++++ .../Services/CustomerManagerService.cs | 22 +++++++--- .../Services/Scim/ScimAccountLockTests.cs | 9 ++-- .../Services/Scim/ScimFixture.cs | 8 ++-- .../Services/Scim/ScimUserServiceTests.cs | 2 +- .../Services/CustomerManagerServiceTests.cs | 23 ++++++++++ .../SubAccountEditCommandHandlerTests.cs | 44 +++++++++---------- .../AccountControllerProviderOnlyTests.cs | 14 +++--- .../Customers/SubAccountEditCommandHandler.cs | 15 ++----- .../Controllers/AccountController.cs | 8 +--- 11 files changed, 90 insertions(+), 67 deletions(-) diff --git a/src/Business/Grand.Business.Authentication/Services/Scim/ScimAccountLock.cs b/src/Business/Grand.Business.Authentication/Services/Scim/ScimAccountLock.cs index d08df1902..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,8 +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), - customer.StoreId); + 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.Customers/Services/CustomerManagerService.cs b/src/Business/Grand.Business.Customers/Services/CustomerManagerService.cs index 604b8fe92..e65f53cfe 100644 --- a/src/Business/Grand.Business.Customers/Services/CustomerManagerService.cs +++ b/src/Business/Grand.Business.Customers/Services/CustomerManagerService.cs @@ -193,32 +193,44 @@ 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; + customer.PasswordFormatId = passwordFormat; await _customerService.UpdateCustomer(customer); //insert password history await _customerHistoryPasswordService.InsertCustomerPassword(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 index 249763ffb..728ed666e 100644 --- a/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimAccountLockTests.cs +++ b/src/Tests/Grand.Business.Authentication.Tests/Services/Scim/ScimAccountLockTests.cs @@ -1,4 +1,4 @@ -using Grand.Business.Core.Utilities.Customers; +using Grand.Domain.Customers; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; @@ -8,15 +8,14 @@ namespace Grand.Business.Authentication.Tests.Services.Scim; public class ScimAccountLockTests { [TestMethod] - public async Task ResetCredentials_ChangesThePasswordInTheStoreOfTheAccount() + public async Task ResetCredentials_ChangesThePasswordOfThatAccount() { var fixture = new ScimFixture(); var customer = await fixture.AddCustomer("jan@example.com"); - customer.StoreId = "store-9"; await fixture.AccountLock().ResetCredentials(customer); - fixture.CustomerManager.Verify(x => x.ChangePassword( - It.Is(r => r.Email == "jan@example.com"), "store-9"), Times.Once); + 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..b5b7fb890 100644 --- a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs +++ b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs @@ -168,6 +168,29 @@ 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.UpdateCustomer(customer), Times.Once); + _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.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs b/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs index ce5e02c50..3412664ef 100644 --- a/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs +++ b/src/Tests/Grand.Web.Tests/Commands/Customers/SubAccountEditCommandHandlerTests.cs @@ -42,22 +42,16 @@ private Customer SubAccount(string ownerId = "owner") Id = "c1", Email = "sub@x.com", Username = "sub@x.com", StoreId = "store-9", OwnerId = ownerId }; _customerServiceMock.Setup(c => c.GetCustomerById("c1")).ReturnsAsync(customer); - _customerServiceMock.Setup(c => c.GetCustomerByEmail(It.IsAny(), "store-9")).ReturnsAsync(customer); return customer; } - [TestMethod] - public async Task Handle_EmailLeadsToAnotherAccount_DoesNotChangePassword() + private void VerifyPasswordChangedOn(Customer customer) { - SubAccount(); - _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.IsFalse(result); - _customerManagerServiceMock.Verify(m => m.ChangePassword( - It.IsAny(), It.IsAny()), Times.Never); + _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, @@ -78,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(), @@ -144,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] @@ -166,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] @@ -225,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] @@ -268,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 c1e441033..2503757f0 100644 --- a/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs +++ b/src/Tests/Grand.Web.Tests/Controllers/AccountControllerProviderOnlyTests.cs @@ -88,16 +88,13 @@ public void Init() } [TestMethod] - public async Task ChangePassword_IsScopedToTheStoreOfTheCustomer() + public async Task ChangePassword_ChangesThePasswordOfTheSignedInAccount() { - _currentCustomer.Email = "john@example.com"; - _currentCustomer.StoreId = "store-9"; - await _controller.ChangePassword(new ChangePasswordModel { OldPassword = "old", NewPassword = "new" }, new OidcLoginConfig()); - _customerManager.Verify(m => m.ChangePassword( - It.Is(r => r.Email == "john@example.com"), "store-9"), Times.Once); + _customerManager.Verify(m => m.ChangePassword(_currentCustomer, It.IsAny(), "new"), Times.Once); + _customerManager.Verify(m => m.ChangePassword(It.IsAny(), It.IsAny()), Times.Never); } [TestMethod] @@ -308,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] @@ -319,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(), It.IsAny()), Times.Once); + _customerManager.Verify(m => m.ChangePassword(_currentCustomer, It.IsAny(), "b"), Times.Once); } [TestMethod] diff --git a/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountEditCommandHandler.cs b/src/Web/Grand.Web/Commands/Handler/Customers/SubAccountEditCommandHandler.cs index 80f3d9536..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,18 +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)) - { - //the password change finds its account by email, which has to lead back to this account - var emailHolder = await _customerService.GetCustomerByEmail(customer.Email, customer.StoreId); - if (emailHolder?.Id != customer.Id) - return false; - - 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 7e3b17169..c79d8e20f 100644 --- a/src/Web/Grand.Web/Controllers/AccountController.cs +++ b/src/Web/Grand.Web/Controllers/AccountController.cs @@ -1119,12 +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); - - //scope the lookup to the customer's store - the same e-mail may exist in another store with per-store identity - await _customerManagerService.ChangePassword(changePasswordRequest, - _contextAccessor.WorkContext.CurrentCustomer.StoreId); + await _customerManagerService.ChangePassword(_contextAccessor.WorkContext.CurrentCustomer, + _customerSettings.DefaultPasswordFormat, model.NewPassword); var customer = await _customerService.GetCustomerById(_contextAccessor.WorkContext.CurrentCustomer.Id); //sign in From d8d1c714c8db5f2b46fb4562358eed434ca70d94 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 1 Nov 2026 10:28:46 +0100 Subject: [PATCH 3/4] Write only the password fields when changing a password The password change saved the whole customer, including its e-mail, username, store and active flag. With a copy the caller loaded earlier in the request, that could undo an edit made in the meantime, such as a deactivation. A dedicated update now writes the four password fields in one operation, and still refuses a system account and raises the update event. --- .../Interfaces/Customers/ICustomerService.cs | 6 +++ .../Services/CustomerManagerService.cs | 3 +- .../Services/CustomerService.cs | 20 ++++++++++ .../Services/CustomerManagerServiceTests.cs | 4 +- .../Services/CustomerServiceTests.cs | 38 +++++++++++++++++++ 5 files changed, 69 insertions(+), 2 deletions(-) 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 e65f53cfe..5322183a2 100644 --- a/src/Business/Grand.Business.Customers/Services/CustomerManagerService.cs +++ b/src/Business/Grand.Business.Customers/Services/CustomerManagerService.cs @@ -231,7 +231,8 @@ public virtual async Task ChangePassword(Customer customer, PasswordFormat passw customer.PasswordChangeDateUtc = DateTime.UtcNow; customer.PasswordFormatId = passwordFormat; - await _customerService.UpdateCustomer(customer); + //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.Customers.Tests/Services/CustomerManagerServiceTests.cs b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs index b5b7fb890..37b79b143 100644 --- a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs +++ b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerManagerServiceTests.cs @@ -177,7 +177,9 @@ public async Task ChangePassword_ForCustomer_ChangesThatCustomerWithoutLookup() await _customerManagerService.ChangePassword(customer, PasswordFormat.Clear, "zxcvbn"); //Assert Assert.AreEqual("zxcvbn", customer.Password); - _customerServiceMock.Verify(c => c.UpdateCustomer(customer), Times.Once); + _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); } 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() { From c21de3694b4848b693d727e7f73b2d59ae7afa56 Mon Sep 17 00:00:00 2001 From: support Date: Sun, 1 Nov 2026 17:30:10 +0100 Subject: [PATCH 4/4] Do not report a customer's own e-mail or username as taken A lookup can return the customer being edited, for example where it ignores case and the stored value differs only in case. Saving My account then failed with an "already exists" message about the customer's own address. --- .../CustomerValidatorsStoreScopingTests.cs | 23 +++++++++++++++++-- .../Customer/CustomerInfoValidator.cs | 4 ++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs b/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs index 6198b6b46..16afed25a 100644 --- a/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs +++ b/src/Tests/Grand.Web.Tests/Validators/Customer/CustomerValidatorsStoreScopingTests.cs @@ -260,7 +260,26 @@ private static (SubAccountEditValidator validator, Mock custom Id = "c1", Email = email, FirstName = "A", LastName = "B" }; - private static CustomerInfoValidator CustomerInfo(Mock customerService, CustomerSettings settings) + [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())) @@ -271,7 +290,7 @@ private static CustomerInfoValidator CustomerInfo(Mock custome return new CustomerInfoValidator( new List>(), - Context("old@x.com"), customerService.Object, mediator.Object, attrParser.Object, + Context(currentEmail), customerService.Object, mediator.Object, attrParser.Object, Translation().Object, new Mock().Object, settings, PerStoreOn().Config); } } diff --git a/src/Web/Grand.Web/Validators/Customer/CustomerInfoValidator.cs b/src/Web/Grand.Web/Validators/Customer/CustomerInfoValidator.cs index 413c6dad2..cc4a6ab48 100644 --- a/src/Web/Grand.Web/Validators/Customer/CustomerInfoValidator.cs +++ b/src/Web/Grand.Web/Validators/Customer/CustomerInfoValidator.cs @@ -139,7 +139,7 @@ public CustomerInfoValidator( context.AddFailure(translationService.GetResource("Account.EmailUsernameErrors.EmailTooLong")); var customer2 = await customerService.GetCustomerByEmail(email, storeId); - if (customer2 != null) + if (customer2 != null && customer2.Id != currentCustomer.Id) context.AddFailure( translationService.GetResource("Account.EmailUsernameErrors.EmailAlreadyExists")); } @@ -152,7 +152,7 @@ public CustomerInfoValidator( context.AddFailure(translationService.GetResource("Account.EmailUsernameErrors.UsernameTooLong")); var customer2 = await customerService.GetCustomerByUsername(username, storeId); - if (customer2 != null) + if (customer2 != null && customer2.Id != currentCustomer.Id) context.AddFailure( translationService.GetResource("Account.EmailUsernameErrors.UsernameAlreadyExists")); }