diff --git a/Directory.Packages.props b/Directory.Packages.props index ee3d99be6a..2f21372435 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -57,7 +57,6 @@ - diff --git a/src/Business/Grand.Business.Customers/Services/CustomerService.cs b/src/Business/Grand.Business.Customers/Services/CustomerService.cs index 143fe78201..5ab0d10a1a 100644 --- a/src/Business/Grand.Business.Customers/Services/CustomerService.cs +++ b/src/Business/Grand.Business.Customers/Services/CustomerService.cs @@ -327,7 +327,8 @@ public virtual async Task InsertGuestCustomer(Customer customer) { SystemName = SystemCustomerGroupNames.Guests }); if (guestGroup == null) throw new GrandException("'Guests' group could not be loaded"); - customer.Groups.Add(guestGroup.Id); + if (!customer.Groups.Contains(guestGroup.Id)) + customer.Groups.Add(guestGroup.Id); await _customerRepository.InsertAsync(customer); diff --git a/src/Modules/Grand.Module.Installer/Services/InstallationService.cs b/src/Modules/Grand.Module.Installer/Services/InstallationService.cs index e61e4557ba..b7813a5c39 100644 --- a/src/Modules/Grand.Module.Installer/Services/InstallationService.cs +++ b/src/Modules/Grand.Module.Installer/Services/InstallationService.cs @@ -482,7 +482,7 @@ await dbContext.CreateIndex(_customerRepository, await dbContext.CreateIndex(_customerRepository, OrderBuilder.Create().Ascending(x => x.LastActivityDateUtc), "LastActivityDateUtc_1"); await dbContext.CreateIndex(_customerRepository, OrderBuilder.Create().Ascending(x => x.CustomerGuid), - "CustomerGuid_1"); + "CustomerGuid_1", true); await dbContext.CreateIndex(_customerRepository, OrderBuilder.Create().Ascending(x => x.Email), "Email_1"); //compound lookup indexes supporting per-store customer identity (uniqueness of Email/Username + StoreId diff --git a/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs new file mode 100644 index 0000000000..6698da3813 --- /dev/null +++ b/src/Modules/Grand.Module.Migration/Migrations/2.5/MigrationCustomerGuidUniqueIndex.cs @@ -0,0 +1,91 @@ +using Grand.Data; +using Grand.Data.Mongo; +using Grand.Domain.Customers; +using Grand.Infrastructure.Migrations; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using MongoDB.Driver; + +namespace Grand.Module.Migration.Migrations._2._5; + +public class MigrationCustomerGuidUniqueIndex : IMigration +{ + private const string IndexName = "CustomerGuid_1"; + + //LiteDB names an index after its field and ignores the given name + private const string LiteDbIndexName = "CustomerGuid"; + + public int Priority => 0; + public DbVersion Version => new(2, 5); + public Guid Identity => new("f0c98624-441d-42b6-95ae-429bfb64d266"); + public string Name => "Unique index on Customer.CustomerGuid"; + + public bool UpgradeProcess(IServiceProvider serviceProvider) + { + var repository = serviceProvider.GetRequiredService>(); + var databaseContext = serviceProvider.GetRequiredService(); + var logService = serviceProvider.GetRequiredService>(); + + try + { + MakeUnique(repository, databaseContext, logService); + } + catch (Exception ex) + { + logService.LogError(ex, "UpgradeProcess - CustomerGuidUniqueIndex (2.5)"); + } + + return true; + } + + /// + /// Recreates the CustomerGuid index as unique. Returns false when it stays non-unique + /// + public static bool MakeUnique(IRepository repository, IDatabaseContext databaseContext, ILogger logService) + { + var duplicates = repository.Table + .GroupBy(x => x.CustomerGuid) + .Where(x => x.Count() > 1) + .Select(x => x.Key) + .Take(20) + .ToList(); + + //duplicates must not stop the upgrade - keep the non-unique index for the lookups and report them + if (duplicates.Count != 0) + { + logService.LogError( + "The index {IndexName} stays non-unique - these customer GUIDs are used more than once: {Guids}. Remove the duplicates and recreate the index as unique", + IndexName, string.Join(", ", duplicates)); + return false; + } + + var mongoRepository = repository as MongoRepository; + var keys = OrderBuilder.Create().Ascending(x => x.CustomerGuid); + + databaseContext.DeleteIndex(repository, mongoRepository != null ? IndexName : LiteDbIndexName) + .GetAwaiter().GetResult(); + databaseContext.CreateIndex(repository, keys, IndexName, true).GetAwaiter().GetResult(); + + //when the unique index exists this is a rejected, swallowed no-op; when the unique create failed + //it restores the lookup index + databaseContext.CreateIndex(repository, keys, IndexName).GetAwaiter().GetResult(); + + //CreateIndex swallows its errors: a duplicate saved by a running instance after the check above + //would leave the index non-unique without a trace + if (mongoRepository != null && !IsUnique(mongoRepository)) + { + logService.LogError( + "The index {IndexName} could not be created unique and stays non-unique. Remove duplicate customer GUIDs and recreate the index as unique", + IndexName); + return false; + } + + return true; + } + + private static bool IsUnique(MongoRepository repository) + { + return repository.Collection.Indexes.List().ToList() + .Any(x => x["name"] == IndexName && x.Contains("unique") && x["unique"].ToBoolean()); + } +} diff --git a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs index ee12c930ae..c592b7e3c5 100644 --- a/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs +++ b/src/Tests/Grand.Business.Customers.Tests/Services/CustomerServiceTests.cs @@ -333,6 +333,19 @@ public async Task InsertGuestCustomerTest() Assert.IsTrue(_repository.Table.Any(x => x.StoreId == "1")); } + [TestMethod] + public async Task InsertGuestCustomer_GroupAlreadyPresent_AddedOnce() + { + _mediatorMock.Setup(x => x.Send(It.IsAny(), default)) + .Returns(Task.FromResult(new CustomerGroup { Id = "guests" })); + var customer = new Customer { CustomerGuid = Guid.NewGuid(), Active = true }; + customer.Groups.Add("guests"); + + await _customerService.InsertGuestCustomer(customer); + + CollectionAssert.AreEqual(new[] { "guests" }, customer.Groups.ToArray()); + } + [TestMethod] public async Task InsertCustomerTest() { diff --git a/src/Tests/Grand.Modules.Tests/Services/Migrations/MigrationCustomerGuidUniqueIndexTests.cs b/src/Tests/Grand.Modules.Tests/Services/Migrations/MigrationCustomerGuidUniqueIndexTests.cs new file mode 100644 index 0000000000..2643ec598b --- /dev/null +++ b/src/Tests/Grand.Modules.Tests/Services/Migrations/MigrationCustomerGuidUniqueIndexTests.cs @@ -0,0 +1,55 @@ +using Grand.Data.LiteDb; +using Grand.Data.Tests; +using Grand.Domain.Customers; +using Grand.Module.Migration.Migrations._2._5; +using Grand.SharedKernel; +using LiteDB; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace Grand.Modules.Tests.Services.Migrations; + +[TestClass] +public class MigrationCustomerGuidUniqueIndexTests +{ + private LiteDatabase _database; + private LiteDBRepository _repository; + private LiteDBContext _context; + + [TestInitialize] + public void Setup() + { + _database = new LiteDatabase(new MemoryStream()); + _repository = new LiteDBRepository(_database, new AuditInfoProvider()); + _context = new LiteDBContext(_database); + //an upgraded install: the lookup index exists, not unique + _database.GetCollection(nameof(Customer)).EnsureIndex(x => x.CustomerGuid); + } + + [TestCleanup] + public void Cleanup() => _database.Dispose(); + + [TestMethod] + public async Task LiteDb_RecreatesTheIndexUnique() + { + var guid = Guid.NewGuid(); + await _repository.InsertAsync(new Customer { CustomerGuid = guid }); + + Assert.IsTrue(MigrationCustomerGuidUniqueIndex.MakeUnique(_repository, _context, NullLogger.Instance)); + + await Assert.ThrowsExactlyAsync(() => _repository.InsertAsync(new Customer { CustomerGuid = guid })); + } + + [TestMethod] + public async Task LiteDb_Duplicates_KeepTheIndexNonUnique() + { + var guid = Guid.NewGuid(); + await _repository.InsertAsync(new Customer { CustomerGuid = guid }); + await _repository.InsertAsync(new Customer { CustomerGuid = guid }); + + Assert.IsFalse(MigrationCustomerGuidUniqueIndex.MakeUnique(_repository, _context, NullLogger.Instance)); + + await _repository.InsertAsync(new Customer { CustomerGuid = guid }); + Assert.AreEqual(3, _repository.Table.Count(x => x.CustomerGuid == guid)); + } +} diff --git a/src/Tests/Grand.Web.Common.Tests/Filters/CustomerActivityAttributeTests.cs b/src/Tests/Grand.Web.Common.Tests/Filters/CustomerActivityAttributeTests.cs new file mode 100644 index 0000000000..4b6a307c9c --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/Filters/CustomerActivityAttributeTests.cs @@ -0,0 +1,89 @@ +using Grand.Business.Core.Interfaces.Customers; +using Grand.Data; +using Grand.Domain.Customers; +using Grand.Infrastructure; +using Grand.SharedKernel.Extensions; +using Grand.Web.Common.Filters; +using Grand.Web.Common.RequestCustomerResolvers; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using System.Linq.Expressions; +using System.Net; + +namespace Grand.Web.Common.Tests.Filters; + +[TestClass] +public class CustomerActivityAttributeTests +{ + private Mock _customerService; + private Mock _contextAccessor; + private Mock _workContext; + private Mock _persistence; + + [TestInitialize] + public void Setup() + { + //the test App_Data/Settings.cfg has a connection string, so the filter sees an installed database + DataSettingsManager.Initialize(Path.Combine("", CommonPath.AppData, CommonPath.SettingsFile)); + _customerService = new Mock(); + _workContext = new Mock(); + _persistence = new Mock(); + _contextAccessor = new Mock(); + _contextAccessor.SetupGet(c => c.WorkContext).Returns(_workContext.Object); + } + + private async Task RunGet(Customer current) + { + _workContext.SetupGet(w => w.CurrentCustomer).Returns(current); + var httpContext = new DefaultHttpContext(); + httpContext.Request.Method = HttpMethods.Get; + httpContext.Request.Scheme = "https"; + httpContext.Request.Host = new HostString("shop.test"); + httpContext.Request.Path = "/product"; + httpContext.Connection.RemoteIpAddress = IPAddress.Parse("10.0.0.1"); + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var executing = new ActionExecutingContext(actionContext, new List(), + new Dictionary(), null); + var filter = new CustomerActivityAttribute.CustomerActivityFilter(_customerService.Object, + _contextAccessor.Object, new CustomerSettings { StoreLastVisitedPage = true }, _persistence.Object); + + await filter.OnActionExecutionAsync(executing, + () => Task.FromResult(new ActionExecutedContext(actionContext, new List(), null))); + } + + [TestMethod] + public async Task SystemAccount_NothingWritten() + { + await RunGet(new Customer { Id = "searchengine", IsSystemAccount = true }); + + _customerService.Verify(c => c.UpdateCustomerField(It.IsAny(), + It.IsAny>>(), It.IsAny()), Times.Never); + _customerService.Verify(c => c.UpdateCustomerField(It.IsAny(), + It.IsAny>>(), It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task Guest_ActivityWritten() + { + await RunGet(new Customer { Id = "guest", Active = true }); + + _customerService.Verify(c => c.UpdateCustomerField(It.IsAny(), + It.IsAny>>(), It.IsAny()), Times.AtLeastOnce); + } + + [TestMethod] + public async Task TransientGuest_NothingWritten() + { + _persistence.Setup(p => p.IsTransient("unsaved")).Returns(true); + + await RunGet(new Customer { Id = "unsaved", Active = true }); + + _customerService.Verify(c => c.UpdateCustomerField(It.IsAny(), + It.IsAny>>(), It.IsAny()), Times.Never); + } +} diff --git a/src/Tests/Grand.Web.Common.Tests/Filters/PersistGuestCustomerAttributeTests.cs b/src/Tests/Grand.Web.Common.Tests/Filters/PersistGuestCustomerAttributeTests.cs new file mode 100644 index 0000000000..c4157d811c --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/Filters/PersistGuestCustomerAttributeTests.cs @@ -0,0 +1,102 @@ +using Grand.Business.Core.Interfaces.Customers; +using Grand.Data; +using Grand.Domain.Affiliates; +using Grand.Domain.Customers; +using Grand.Infrastructure; +using Grand.SharedKernel.Extensions; +using Grand.Web.Common.Filters; +using Grand.Web.Common.RequestCustomerResolvers; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.AspNetCore.Routing; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using System.Linq.Expressions; + +namespace Grand.Web.Common.Tests.Filters; + +[TestClass] +public class PersistGuestCustomerAttributeTests +{ + private readonly List _order = []; + private Mock _persistence; + + [TestInitialize] + public void Setup() + { + //the test App_Data/Settings.cfg has a connection string, so the filters see an installed database + DataSettingsManager.Initialize(Path.Combine("", CommonPath.AppData, CommonPath.SettingsFile)); + _persistence = new Mock(); + _persistence.Setup(p => p.EnsurePersisted()).Callback(() => _order.Add("persist")) + .Returns(Task.CompletedTask); + } + + private static (ActionExecutingContext, ActionExecutionDelegate) Context(string query, Action onAction) + { + var httpContext = new DefaultHttpContext(); + httpContext.Request.Method = HttpMethods.Get; + httpContext.Request.QueryString = new QueryString(query); + var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var executing = new ActionExecutingContext(actionContext, new List(), + new Dictionary(), null); + return (executing, () => + { + onAction(); + return Task.FromResult(new ActionExecutedContext(actionContext, new List(), null)); + }); + } + + [TestMethod] + public async Task PersistsTheGuestBeforeTheAction() + { + var (executing, next) = Context("", () => _order.Add("action")); + var filter = new PersistGuestCustomerAttribute.PersistGuestCustomerFilter(_persistence.Object); + + await filter.OnActionExecutionAsync(executing, next); + + CollectionAssert.AreEqual(new[] { "persist", "action" }, _order); + } + + [TestMethod] + public async Task Affiliate_LandingWithAffiliateId_PersistsThenWrites() + { + var customer = new Customer { Id = "g1" }; + var (filter, customerService) = AffiliateFilter(customer, + new Affiliate { Id = "aff", Active = true }); + customerService.Setup(c => c.UpdateCustomerField("g1", It.IsAny>>(), "aff")) + .Callback(() => _order.Add("write")).Returns(Task.CompletedTask); + var (executing, next) = Context("?affiliateid=aff", () => { }); + + await filter.OnActionExecutionAsync(executing, next); + + CollectionAssert.AreEqual(new[] { "persist", "write" }, _order); + } + + [TestMethod] + public async Task Affiliate_InactiveAffiliate_NothingPersisted() + { + var (filter, _) = AffiliateFilter(new Customer { Id = "g1" }, new Affiliate { Id = "aff", Active = false }); + var (executing, next) = Context("?affiliateid=aff", () => { }); + + await filter.OnActionExecutionAsync(executing, next); + + _persistence.Verify(p => p.EnsurePersisted(), Times.Never); + } + + private (AffiliateAttribute.AffiliateFilter, Mock) AffiliateFilter(Customer current, + Affiliate affiliate) + { + var workContext = new Mock(); + workContext.SetupGet(w => w.CurrentCustomer).Returns(current); + var contextAccessor = new Mock(); + contextAccessor.SetupGet(c => c.WorkContext).Returns(workContext.Object); + var affiliateService = new Mock(); + affiliateService.Setup(a => a.GetAffiliateById(affiliate.Id)).ReturnsAsync(affiliate); + var customerService = new Mock(); + + return (new AffiliateAttribute.AffiliateFilter(affiliateService.Object, customerService.Object, + contextAccessor.Object, _persistence.Object), customerService); + } +} diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs new file mode 100644 index 0000000000..1adbff6e4c --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolvers/GuestCustomerPersistenceTests.cs @@ -0,0 +1,169 @@ +using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Business.Core.Interfaces.Customers; +using Grand.Domain.Customers; +using Grand.Web.Common.RequestCustomerResolvers; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Web.Common.Tests.RequestCustomerResolvers; + +[TestClass] +public class GuestCustomerPersistenceTests +{ + private Mock _customers; + private Mock _groups; + private GuestCustomerPersistence _persistence; + + [TestInitialize] + public void Setup() + { + _customers = new Mock(); + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())).ReturnsAsync((Customer c) => c); + _groups = new Mock(); + _persistence = new GuestCustomerPersistence(_customers.Object, _groups.Object, + NullLogger.Instance); + } + + [TestMethod] + public async Task IsTransient_OnlyForTheTransientId() + { + Assert.IsFalse(_persistence.IsTransient("g1")); + + await _persistence.SetTransient(new Customer { Id = "g1" }); + + Assert.IsTrue(_persistence.IsTransient("g1")); + Assert.IsFalse(_persistence.IsTransient("other")); + Assert.IsFalse(_persistence.IsTransient(null)); + Assert.IsFalse(_persistence.IsTransient("")); + } + + [TestMethod] + public async Task SetTransient_AddsGuestsGroup() + { + _groups.Setup(g => g.GetCustomerGroupBySystemName(SystemCustomerGroupNames.Guests)) + .ReturnsAsync(new CustomerGroup { Id = "guests" }); + var fresh = new Customer { Id = "g1" }; + var already = new Customer { Id = "g2" }; + already.Groups.Add("guests"); + + await _persistence.SetTransient(fresh); + await _persistence.SetTransient(already); + + CollectionAssert.AreEqual(new[] { "guests" }, fresh.Groups.ToArray()); + CollectionAssert.AreEqual(new[] { "guests" }, already.Groups.ToArray()); + } + + [TestMethod] + public async Task EnsurePersisted_InsertsOnceAndClearsTransient() + { + var guest = new Customer { Id = "g1" }; + await _persistence.SetTransient(guest); + + await _persistence.EnsurePersisted(); + await _persistence.EnsurePersisted(); + + _customers.Verify(c => c.InsertGuestCustomer(guest), Times.Once); + Assert.IsNull(_persistence.Transient); + Assert.IsFalse(_persistence.IsTransient("g1")); + } + + [TestMethod] + public async Task EnsurePersisted_NoTransient_DoesNothing() + { + await _persistence.EnsurePersisted(); + + _customers.Verify(c => c.InsertGuestCustomer(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task EnsurePersisted_ConcurrentCalls_InsertOnce() + { + var gate = new TaskCompletionSource(); + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())).Returns(gate.Task); + await _persistence.SetTransient(new Customer { Id = "g1" }); + + var first = _persistence.EnsurePersisted(); + var second = _persistence.EnsurePersisted(); + gate.SetResult(new Customer { Id = "g1" }); + await Task.WhenAll(first, second); + + _customers.Verify(c => c.InsertGuestCustomer(It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task EnsurePersisted_AfterFailedInsert_RetriesOnTheNextWrite() + { + var guest = new Customer { Id = "g1" }; + _customers.SetupSequence(c => c.InsertGuestCustomer(guest)) + .ThrowsAsync(new TimeoutException("db blip")) + .ReturnsAsync(guest); + await _persistence.SetTransient(guest); + + await Assert.ThrowsExactlyAsync(() => _persistence.EnsurePersisted()); + Assert.IsTrue(_persistence.IsTransient("g1")); + + await _persistence.EnsurePersisted(); + + _customers.Verify(c => c.InsertGuestCustomer(guest), Times.Exactly(2)); + Assert.IsNull(_persistence.Transient); + } + + [TestMethod] + public async Task Promote_Inserts() + { + var guest = new Customer { Id = "g1", CustomerGuid = Guid.NewGuid() }; + + Assert.AreSame(guest, await _persistence.Promote(guest)); + } + + [TestMethod] + public async Task Promote_ParallelRequestWon_ReturnsTheExistingGuest() + { + //the unique CustomerGuid index rejected the second insert of the same cookie + var guid = Guid.NewGuid(); + var existing = new Customer { Id = "first", CustomerGuid = guid, Active = true }; + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())) + .ThrowsAsync(new InvalidOperationException("duplicate key")); + _customers.Setup(c => c.GetCustomerByGuid(guid)).ReturnsAsync(existing); + + var result = await _persistence.Promote(new Customer { Id = "second", CustomerGuid = guid }); + + Assert.AreSame(existing, result); + } + + [TestMethod] + public async Task Promote_GuidTurnedOutToBeRegistered_ReturnsNull() + { + var guid = Guid.NewGuid(); + var admin = new Customer { Id = "admin", CustomerGuid = guid, Active = true }; + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())) + .ThrowsAsync(new InvalidOperationException("duplicate key")); + _customers.Setup(c => c.GetCustomerByGuid(guid)).ReturnsAsync(admin); + _groups.Setup(g => g.IsRegistered(admin)).ReturnsAsync(true); + + Assert.IsNull(await _persistence.Promote(new Customer { Id = "x", CustomerGuid = guid })); + } + + [TestMethod] + public async Task Promote_GuidTurnedOutToBeSystemAccount_ReturnsNull() + { + var guid = Guid.NewGuid(); + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())) + .ThrowsAsync(new InvalidOperationException("duplicate key")); + _customers.Setup(c => c.GetCustomerByGuid(guid)).ReturnsAsync( + new Customer { Id = "searchengine", CustomerGuid = guid, Active = true, IsSystemAccount = true }); + + Assert.IsNull(await _persistence.Promote(new Customer { Id = "x", CustomerGuid = guid })); + } + + [TestMethod] + public async Task Promote_InsertFailsForAnotherReason_Throws() + { + _customers.Setup(c => c.InsertGuestCustomer(It.IsAny())) + .ThrowsAsync(new InvalidOperationException("db down")); + + await Assert.ThrowsExactlyAsync(() => + _persistence.Promote(new Customer { Id = "x", CustomerGuid = Guid.NewGuid() })); + } +} diff --git a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs index 3c040570c5..bd2683160a 100644 --- a/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/RequestCustomerResolversTests.cs @@ -3,11 +3,11 @@ using Grand.Business.Core.Interfaces.Customers; using Grand.Domain.Customers; using Grand.Web.Common.RequestCustomerResolvers; +using Grand.Web.Common.UserAgent; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using Wangkanai.Detection.Services; namespace Grand.Web.Common.Tests; @@ -132,35 +132,44 @@ public async Task GuestCookie_RegisteredDeletedOrInactive_ReturnsNull() [TestMethod] public async Task SearchEngine_NotACrawler_ReturnsNull() { - var detection = new Mock(); - var resolver = new SearchEngineCustomerResolver(detection.Object, Mock.Of()); + var classifier = new Mock(); + classifier.SetupGet(c => c.IsCrawler).Returns(false); + var resolver = new SearchEngineCustomerResolver(classifier.Object, Mock.Of(), + AccessorWith(new DefaultHttpContext())); - //Crawler is null: the null-safe read must not throw Assert.IsNull(await resolver.Resolve()); Assert.AreEqual(RequestCustomerResolverOrder.SearchEngine, resolver.Order); - - var crawler = new Mock(); - crawler.SetupGet(c => c.IsCrawler).Returns(false); - detection.SetupGet(d => d.Crawler).Returns(crawler.Object); - Assert.IsNull(await resolver.Resolve()); } [TestMethod] public async Task SearchEngine_Crawler_ReturnsSearchEngineCustomer() { - var crawler = new Mock(); - crawler.SetupGet(c => c.IsCrawler).Returns(true); - var detection = new Mock(); - detection.SetupGet(d => d.Crawler).Returns(crawler.Object); + var classifier = new Mock(); + classifier.SetupGet(c => c.IsCrawler).Returns(true); var customers = new Mock(); customers.Setup(c => c.GetCustomerBySystemName(SystemCustomerNames.SearchEngine)) .ReturnsAsync(new Customer { Id = "crawler" }); - var resolver = new SearchEngineCustomerResolver(detection.Object, customers.Object); + var resolver = new SearchEngineCustomerResolver(classifier.Object, customers.Object, + AccessorWith(new DefaultHttpContext())); Assert.AreEqual("crawler", (await resolver.Resolve()).Id); } + [TestMethod] + public async Task SearchEngine_AuthorizationHeader_ReturnsNull() + { + var classifier = new Mock(); + classifier.SetupGet(c => c.IsCrawler).Returns(true); + var customers = new Mock(); + var context = new DefaultHttpContext(); + context.Request.Headers.Authorization = "Bearer token"; + var resolver = new SearchEngineCustomerResolver(classifier.Object, customers.Object, AccessorWith(context)); + + Assert.IsNull(await resolver.Resolve()); + customers.Verify(c => c.GetCustomerBySystemName(It.IsAny()), Times.Never); + } + [TestMethod] public async Task ApiUser_NoApiCustomer_ReturnsNull() { @@ -214,6 +223,40 @@ public async Task ApiUser_Authenticated_ReturnsCustomer() Assert.AreEqual("api", (await resolver.Resolve()).Id); } + [TestMethod] + public async Task GuestCookie_SystemAccountGuid_ReturnsNull() + { + //Anonymous and SearchEngine belong to the Guests group + var guid = Guid.NewGuid(); + var authentication = new Mock(); + authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(guid.ToString()); + var customers = new Mock(); + customers.Setup(c => c.GetCustomerByGuid(guid)) + .ReturnsAsync(new Customer { Id = "anonymous", Active = true, IsSystemAccount = true }); + + var resolver = new GuestCookieCustomerResolver(authentication.Object, customers.Object, + Mock.Of()); + + Assert.IsNull(await resolver.Resolve()); + } + + [TestMethod] + [DataRow("00000000-0000-0000-0000-000000000000")] + [DataRow("0f8fad5bd9cb469fa16570867728950e")] + [DataRow("{0f8fad5b-d9cb-469f-a165-70867728950e}")] + public async Task GuestCookie_EmptyOrNonCanonicalGuid_NotLookedUp(string cookie) + { + var authentication = new Mock(); + authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(cookie); + var customers = new Mock(); + + var resolver = new GuestCookieCustomerResolver(authentication.Object, customers.Object, + Mock.Of()); + + Assert.IsNull(await resolver.Resolve()); + customers.Verify(c => c.GetCustomerByGuid(It.IsAny()), Times.Never); + } + private static IHttpContextAccessor AccessorWith(HttpContext context) { var accessor = new Mock(); diff --git a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs index d1cee850de..1257bec63a 100644 --- a/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/Security/Oidc/OidcWorkContextTests.cs @@ -11,6 +11,7 @@ using Grand.Infrastructure.Configuration; using Grand.Web.Common.RequestCustomerResolvers; using Grand.Web.Common.Security.Oidc; +using Grand.Web.Common.UserAgent; using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; @@ -20,7 +21,6 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using System.Security.Claims; -using Wangkanai.Detection.Services; namespace Grand.Web.Common.Tests.Security.Oidc; @@ -91,7 +91,7 @@ private TestableSetter CreateSetter() services.AddSingleton( new GuestCookieCustomerResolver(_authentication.Object, _customerService.Object, Mock.Of())); services.AddSingleton( - new SearchEngineCustomerResolver(Mock.Of(), _customerService.Object)); + new SearchEngineCustomerResolver(Mock.Of(), _customerService.Object, _accessor.Object)); services.AddSingleton( new AllowAnonymousCustomerResolver(_accessor.Object, _customerService.Object)); return new TestableSetter(_accessor.Object, _authentication.Object, Mock.Of(), diff --git a/src/Tests/Grand.Web.Common.Tests/TagHelpers/AdminTabStripTagHelperTests.cs b/src/Tests/Grand.Web.Common.Tests/TagHelpers/AdminTabStripTagHelperTests.cs index 465b9170c1..69fb25a1ed 100644 --- a/src/Tests/Grand.Web.Common.Tests/TagHelpers/AdminTabStripTagHelperTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/TagHelpers/AdminTabStripTagHelperTests.cs @@ -12,8 +12,7 @@ using Microsoft.AspNetCore.Routing; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using Wangkanai.Detection.Models; -using Wangkanai.Detection.Services; +using Grand.Web.Common.UserAgent; namespace Grand.Web.Common.Tests.TagHelpers; @@ -21,8 +20,7 @@ namespace Grand.Web.Common.Tests.TagHelpers; public class AdminTabStripTagHelperTests { private Mock _mediatorMock; - private Mock _detectionServiceMock; - private Mock _deviceServiceMock; + private Mock _userAgentMock; private ViewContext _viewContext; private Action _onPublish; @@ -37,10 +35,8 @@ public void Init() _onPublish?.Invoke(message); return Task.CompletedTask; }); - _deviceServiceMock = new Mock(); - _deviceServiceMock.Setup(x => x.Type).Returns(Device.Desktop); - _detectionServiceMock = new Mock(); - _detectionServiceMock.Setup(x => x.Device).Returns(_deviceServiceMock.Object); + _userAgentMock = new Mock(); + _userAgentMock.SetupGet(x => x.IsMobileOrTablet).Returns(false); _viewContext = new ViewContext { RouteData = new RouteData(), ViewData = new ViewDataDictionary(new EmptyModelMetadataProvider(), new ModelStateDictionary()), @@ -51,7 +47,7 @@ public void Init() private AdminTabStripTagHelper CreateStrip(string name = "product-edit", bool bindGrid = true, bool setTabPos = false) { - return new AdminTabStripTagHelper(_mediatorMock.Object, _detectionServiceMock.Object) { + return new AdminTabStripTagHelper(_mediatorMock.Object, _userAgentMock.Object) { Name = name, BindGrid = bindGrid, SetTabPos = setTabPos, @@ -166,7 +162,7 @@ public async Task Process_SetTabPos_AddsTheLeftLayoutClass() [TestMethod] public async Task Process_OnAMobileDevice_IgnoresSetTabPos() { - _deviceServiceMock.Setup(x => x.Type).Returns(Device.Mobile); + _userAgentMock.SetupGet(x => x.IsMobileOrTablet).Returns(true); var output = await Run(CreateStrip(setTabPos: true), "
a
"); Assert.AreEqual("grand-tabstrip", output.Attributes["class"].Value); diff --git a/src/Tests/Grand.Web.Common.Tests/UserAgent/UserAgentClassifierTests.cs b/src/Tests/Grand.Web.Common.Tests/UserAgent/UserAgentClassifierTests.cs new file mode 100644 index 0000000000..a46ad8d821 --- /dev/null +++ b/src/Tests/Grand.Web.Common.Tests/UserAgent/UserAgentClassifierTests.cs @@ -0,0 +1,84 @@ +using Grand.Web.Common.UserAgent; +using Microsoft.AspNetCore.Http; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Web.Common.Tests.UserAgent; + +[TestClass] +public class UserAgentClassifierTests +{ + private static UserAgentClassifier ClassifierFor(string userAgent) + { + var context = new DefaultHttpContext(); + if (userAgent != null) context.Request.Headers.UserAgent = userAgent; + var accessor = new Mock(); + accessor.Setup(a => a.HttpContext).Returns(context); + return new UserAgentClassifier(accessor.Object); + } + + [TestMethod] + [DataRow("Mozilla/5.0 (compatible; Googlebot/2.1; +http://www.google.com/bot.html)")] + [DataRow("Mozilla/5.0 (compatible; bingbot/2.0; +http://www.bing.com/bingbot.htm)")] + [DataRow("Mozilla/5.0 (compatible; AhrefsBot/7.0; +http://ahrefs.com/robot/)")] + [DataRow("Mozilla/5.0 (compatible; SemrushBot/7~bl; +http://www.semrush.com/bot.html)")] + [DataRow("Mozilla/5.0 AppleWebKit/537.36 (KHTML, like Gecko; compatible; GPTBot/1.2; +https://openai.com/gptbot)")] + [DataRow("Mozilla/5.0 AppleWebKit/537.36 (KHTML, like Gecko; compatible; ClaudeBot/1.0; +claudebot@anthropic.com)")] + [DataRow("Mozilla/5.0 (compatible; YandexBot/3.0; +http://yandex.com/bots)")] + [DataRow("Mozilla/5.0 (Linux; Android 5.0) AppleWebKit/537.36 (KHTML, like Gecko) Mobile Safari/537.36 (compatible; Bytespider; spider-feedback@bytedance.com)")] + [DataRow("Mozilla/5.0 (compatible; Baiduspider/2.0; +http://www.baidu.com/search/spider.html)")] + [DataRow("facebookexternalhit/1.1 (+http://www.facebook.com/externalhit_uatext.php)")] + [DataRow("meta-externalagent/1.1 (+https://developers.facebook.com/docs/sharing/webmasters/crawler)")] + [DataRow("Mozilla/5.0 (compatible; crawler)")] + [DataRow("Mozilla/5.0 (compatible; SomeSpider/1.0)")] + public void IsCrawler_NamedCrawlers_True(string userAgent) + { + Assert.IsTrue(ClassifierFor(userAgent).IsCrawler); + } + + [TestMethod] + [DataRow("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36")] + [DataRow("Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Safari/605.1.15")] + [DataRow("Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Mobile/15E148 Safari/604.1")] + [DataRow("Mozilla/5.0 (Linux; Android 10; CUBOT X30) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36")] + [DataRow("Mozilla/5.0 (X11; Linux x86_64; rv:121.0) Gecko/20100101 Firefox/121.0")] + [DataRow("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) HeadlessChrome/120.0.0.0 Safari/537.36")] + [DataRow("Scrapy/2.11.0 (+https://scrapy.org)")] + [DataRow("curl/8.4.0")] + [DataRow("Wget/1.21")] + [DataRow("python-requests/2.31.0")] + [DataRow("Go-http-client/1.1")] + [DataRow("okhttp/4.12.0")] + [DataRow("Java/17.0.2")] + [DataRow("Apache-HttpClient/4.5.14 (Java/17.0.2)")] + [DataRow("axios/1.6.0")] + [DataRow("node-fetch/1.0 (+https://github.com/bitinn/node-fetch)")] + [DataRow("")] + [DataRow(null)] + public void IsCrawler_BrowsersAndHttpClients_False(string userAgent) + { + Assert.IsFalse(ClassifierFor(userAgent).IsCrawler); + } + + [TestMethod] + [DataRow("Mozilla/5.0 (iPhone; CPU iPhone OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Mobile/15E148", true)] + [DataRow("Mozilla/5.0 (iPad; CPU OS 17_0 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/17.0 Safari/604.1", true)] + [DataRow("Mozilla/5.0 (Linux; Android 14; Pixel 8) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Mobile Safari/537.36", true)] + [DataRow("Mozilla/5.0 (Linux; Android 13; SM-X710) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36", true)] + [DataRow("Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36", false)] + [DataRow("", false)] + public void IsMobileOrTablet(string userAgent, bool expected) + { + Assert.AreEqual(expected, ClassifierFor(userAgent).IsMobileOrTablet); + } + + [TestMethod] + public void NoHttpContext_NotACrawlerNotMobile() + { + //a scope without a request (scheduled task) is never a crawler + var classifier = new UserAgentClassifier(Mock.Of()); + + Assert.IsFalse(classifier.IsCrawler); + Assert.IsFalse(classifier.IsMobileOrTablet); + } +} diff --git a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs index 1e586ead6d..42297678e8 100644 --- a/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs +++ b/src/Tests/Grand.Web.Common.Tests/WorkContextSetterRequestResolverTests.cs @@ -11,13 +11,13 @@ using Grand.Domain.Tax; using Grand.Infrastructure.Configuration; using Grand.Web.Common.RequestCustomerResolvers; +using Grand.Web.Common.UserAgent; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; -using Wangkanai.Detection.Services; namespace Grand.Web.Common.Tests; @@ -29,8 +29,9 @@ public class WorkContextSetterRequestResolverTests private DefaultHttpContext _httpContext; private Mock _authentication; private Mock _apiAuthentication; - private Mock _detection; + private Mock _userAgent; private Mock _accessor; + private Mock _persistence; [TestInitialize] public void Setup() @@ -39,9 +40,11 @@ public void Setup() _groupService = new Mock(); _authentication = new Mock(); _apiAuthentication = new Mock(); - _detection = new Mock(); + _userAgent = new Mock(); _httpContext = new DefaultHttpContext(); _accessor = new Mock(); + _persistence = new Mock(); + _persistence.Setup(p => p.SetTransient(It.IsAny())).Returns(Task.CompletedTask); _accessor.Setup(a => a.HttpContext).Returns(_httpContext); } @@ -56,6 +59,16 @@ private TestableSetter CreateSetter(params IRequestCustomerResolver[] resolvers) new TaxSettings(), new AppConfig(), services.BuildServiceProvider()); } + private TestableSetter CreateSetterWithPersistence() + { + var services = new ServiceCollection(); + services.AddSingleton(_persistence.Object); + return new TestableSetter(_accessor.Object, _authentication.Object, Mock.Of(), + _customerService.Object, Mock.Of(), + Mock.Of(), Mock.Of(), Mock.Of(), + new TaxSettings(), new AppConfig(), services.BuildServiceProvider()); + } + private static Mock ResolverMock(int order, Customer result) { var mock = new Mock(); @@ -65,11 +78,11 @@ private static Mock ResolverMock(int order, Customer r } //registered in a scrambled order on purpose: the setter must sort by Order - private IRequestCustomerResolver[] BuiltIns() + private IRequestCustomerResolver[] BuiltIns(IUserAgentClassifier classifier = null) { return [ new ApiUserCustomerResolver(_apiAuthentication.Object, _accessor.Object, _customerService.Object), - new SearchEngineCustomerResolver(_detection.Object, _customerService.Object), + new SearchEngineCustomerResolver(classifier ?? _userAgent.Object, _customerService.Object, _accessor.Object), new GuestCookieCustomerResolver(_authentication.Object, _customerService.Object, _groupService.Object), new CookieCustomerResolver(_authentication.Object), new AllowAnonymousCustomerResolver(_accessor.Object, _customerService.Object) @@ -87,9 +100,7 @@ private void ArrangeAllBuiltInsMatch() _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(guid.ToString()); _customerService.Setup(c => c.GetCustomerByGuid(guid)) .ReturnsAsync(new Customer { Id = "guest", Active = true }); - var crawler = new Mock(); - crawler.SetupGet(c => c.IsCrawler).Returns(true); - _detection.SetupGet(d => d.Crawler).Returns(crawler.Object); + _userAgent.SetupGet(u => u.IsCrawler).Returns(true); _customerService.Setup(c => c.GetCustomerBySystemName(SystemCustomerNames.SearchEngine)) .ReturnsAsync(new Customer { Id = "crawler", IsSystemAccount = true }); _apiAuthentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync(new Customer { Id = "api", Active = true }); @@ -122,6 +133,23 @@ public async Task BuiltIns_GuestCookie_BeatsSearchEngine() Assert.AreEqual("guest", customer.Id); } + [TestMethod] + [DataRow("python-requests/2.31.0")] + [DataRow("")] + public async Task BuiltIns_ApiClientWithRealClassifier_ResolvesToApiCustomer(string userAgent) + { + _httpContext.Request.Headers.UserAgent = userAgent; + _httpContext.Request.Headers.Authorization = "Bearer token"; + _customerService.Setup(c => c.GetCustomerBySystemName(SystemCustomerNames.SearchEngine)) + .ReturnsAsync(new Customer { Id = "crawler", IsSystemAccount = true }); + _apiAuthentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync(new Customer { Id = "api", Active = true }); + + var customer = await CreateSetter(BuiltIns(new UserAgentClassifier(_accessor.Object))) + .CurrentCustomerForTest(new Store()); + + Assert.AreEqual("api", customer.Id); + } + [TestMethod] public async Task BuiltIns_SearchEngine_BeatsApiUser() { @@ -140,7 +168,7 @@ public async Task BuiltIns_ApiUser_WinsWhenNothingElseMatches() _httpContext.SetEndpoint(null); _authentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync((Customer)null); _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync((string)null); - _detection.SetupGet(d => d.Crawler).Returns((ICrawlerService)null); + _userAgent.SetupGet(u => u.IsCrawler).Returns(false); var customer = await CreateSetter(BuiltIns()).CurrentCustomerForTest(new Store()); Assert.AreEqual("api", customer.Id); } @@ -150,7 +178,7 @@ public async Task BuiltIns_NothingMatches_CreatesNewGuest() { _authentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync((Customer)null); _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync((string)null); - _detection.SetupGet(d => d.Crawler).Returns((ICrawlerService)null); + _userAgent.SetupGet(u => u.IsCrawler).Returns(false); _apiAuthentication.Setup(a => a.GetAuthenticatedCustomer()).ReturnsAsync((Customer)null); _customerService.Setup(c => c.InsertGuestCustomer(It.IsAny())) .ReturnsAsync((Customer c) => c); @@ -162,6 +190,93 @@ public async Task BuiltIns_NothingMatches_CreatesNewGuest() _authentication.Verify(a => a.SetCustomerGuid(customer.CustomerGuid), Times.Once); } + [TestMethod] + public async Task Get_NoCookie_TransientGuestNotInserted() + { + _httpContext.Request.Method = HttpMethods.Get; + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store { Id = "s1" }); + + Assert.AreEqual("s1", customer.StoreId); + _persistence.Verify(p => p.SetTransient(customer), Times.Once); + _customerService.Verify(c => c.InsertGuestCustomer(It.IsAny()), Times.Never); + _authentication.Verify(a => a.SetCustomerGuid(customer.CustomerGuid), Times.Once); + } + + [TestMethod] + public async Task Post_NoCookie_GuestInserted() + { + _httpContext.Request.Method = HttpMethods.Post; + _customerService.Setup(c => c.InsertGuestCustomer(It.IsAny())).ReturnsAsync((Customer c) => c); + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store { Id = "s1" }); + + _customerService.Verify(c => c.InsertGuestCustomer(customer), Times.Once); + _persistence.Verify(p => p.SetTransient(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task ReturnedCookieOfUnsavedGuest_Promoted() + { + _httpContext.Request.Method = HttpMethods.Get; + var guid = Guid.NewGuid(); + _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(guid.ToString()); + _customerService.Setup(c => c.GetCustomerByGuid(guid)).ReturnsAsync((Customer)null); + var promoted = new Customer { Id = "promoted", CustomerGuid = guid, Active = true }; + _persistence.Setup(p => p.Promote(It.Is(c => c.CustomerGuid == guid && c.StoreId == "s1"))) + .ReturnsAsync(promoted); + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store { Id = "s1" }); + + Assert.AreSame(promoted, customer); + _authentication.Verify(a => a.SetCustomerGuid(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task CookieOfExistingCustomer_NeverPromoted_NewGuestIssued() + { + //an administrator's GUID planted in the guest cookie + _httpContext.Request.Method = HttpMethods.Get; + var adminGuid = Guid.NewGuid(); + _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(adminGuid.ToString()); + _customerService.Setup(c => c.GetCustomerByGuid(adminGuid)) + .ReturnsAsync(new Customer { Id = "admin", CustomerGuid = adminGuid, Active = true }); + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store { Id = "s1" }); + + Assert.AreNotEqual("admin", customer.Id); + Assert.AreNotEqual(adminGuid, customer.CustomerGuid); + _persistence.Verify(p => p.Promote(It.IsAny()), Times.Never); + _authentication.Verify(a => a.SetCustomerGuid(customer.CustomerGuid), Times.Once); + } + + [TestMethod] + public async Task PromotionRefused_NewGuestIssued() + { + _httpContext.Request.Method = HttpMethods.Get; + var guid = Guid.NewGuid(); + _authentication.Setup(a => a.GetCustomerGuid()).ReturnsAsync(guid.ToString()); + _persistence.Setup(p => p.Promote(It.IsAny())).ReturnsAsync((Customer)null); + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store { Id = "s1" }); + + Assert.AreNotEqual(guid, customer.CustomerGuid); + _persistence.Verify(p => p.SetTransient(customer), Times.Once); + } + + [TestMethod] + public async Task BackgroundTask_NeverTransient() + { + _accessor.Setup(a => a.HttpContext).Returns((HttpContext)null); + _customerService.Setup(c => c.GetCustomerBySystemName(SystemCustomerNames.BackgroundTask)) + .ReturnsAsync(new Customer { Id = "task", IsSystemAccount = true }); + + var customer = await CreateSetterWithPersistence().CurrentCustomerForTest(new Store()); + + Assert.AreEqual("task", customer.Id); + _persistence.Verify(p => p.SetTransient(It.IsAny()), Times.Never); + } + [TestMethod] public async Task BackgroundTask_NoHttpContext_ResolversAreNotAsked() { diff --git a/src/Tests/Grand.Web.Tests/Controllers/ProductControllerPreviewTests.cs b/src/Tests/Grand.Web.Tests/Controllers/ProductControllerPreviewTests.cs index 40fa9564e7..6c1854c494 100644 --- a/src/Tests/Grand.Web.Tests/Controllers/ProductControllerPreviewTests.cs +++ b/src/Tests/Grand.Web.Tests/Controllers/ProductControllerPreviewTests.cs @@ -9,6 +9,7 @@ using Grand.Domain.Permissions; using Grand.Infrastructure; using Grand.Mediator; +using Grand.Web.Common.RequestCustomerResolvers; using Grand.Web.Controllers; using Grand.Web.Features.Models.Products; using Grand.Web.Models.Catalog; @@ -33,6 +34,8 @@ public class ProductControllerPreviewTests private Mock _mediatorMock; private Mock _permissionServiceMock; private Mock _productServiceMock; + private Mock _guestPersistenceMock; + private Mock _recentlyViewedMock; [TestInitialize] public void Init() @@ -45,6 +48,8 @@ public void Init() _aclServiceMock = new Mock(); _permissionServiceMock = new Mock(); _mediatorMock = new Mock(); + _guestPersistenceMock = new Mock(); + _recentlyViewedMock = new Mock(); var workContextMock = new Mock(); workContextMock.Setup(x => x.CurrentCustomer).Returns(_customer); @@ -67,13 +72,14 @@ public void Init() _productServiceMock.Object, contextAccessorMock.Object, new Mock().Object, - new Mock().Object, + _recentlyViewedMock.Object, new Mock().Object, _aclServiceMock.Object, _permissionServiceMock.Object, _mediatorMock.Object, new CatalogSettings { AllowViewUnpublishedProductPage = false }, - new CaptchaSettings()) { + new CaptchaSettings(), + _guestPersistenceMock.Object) { ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext { Request = { Method = "GET" } } } @@ -137,4 +143,27 @@ public async Task ProductDetails_AvailabilityEnded_CatalogManagerCanPreview() Assert.IsInstanceOfType(result.Result); } + + [TestMethod] + public async Task ProductDetails_UnsavedGuest_DoesNotRecordRecentlyViewed() + { + GivenProduct(new Product { Id = "p1", Published = true, VisibleIndividually = true }); + _guestPersistenceMock.Setup(x => x.IsTransient(_customer.Id)).Returns(true); + + await _controller.ProductDetails("p1"); + + _recentlyViewedMock.Verify(x => x.AddProductToRecentlyViewedList(It.IsAny(), It.IsAny()), + Times.Never); + } + + [TestMethod] + public async Task ProductDetails_SavedCustomer_RecordsRecentlyViewed() + { + GivenProduct(new Product { Id = "p1", Published = true, VisibleIndividually = true }); + _guestPersistenceMock.Setup(x => x.IsTransient(_customer.Id)).Returns(false); + + await _controller.ProductDetails("p1"); + + _recentlyViewedMock.Verify(x => x.AddProductToRecentlyViewedList(_customer.Id, "p1"), Times.Once); + } } diff --git a/src/Web/Grand.Web.Common/Filters/AffiliateAttribute.cs b/src/Web/Grand.Web.Common/Filters/AffiliateAttribute.cs index 847267932c..2ce94c6dbe 100644 --- a/src/Web/Grand.Web.Common/Filters/AffiliateAttribute.cs +++ b/src/Web/Grand.Web.Common/Filters/AffiliateAttribute.cs @@ -2,6 +2,7 @@ using Grand.Data; using Grand.Domain.Affiliates; using Grand.Infrastructure; +using Grand.Web.Common.RequestCustomerResolvers; using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Filters; @@ -24,9 +25,10 @@ public AffiliateAttribute() : base(typeof(AffiliateFilter)) /// /// Represents a filter that checks and updates affiliate of customer /// - private class AffiliateFilter(IAffiliateService affiliateService, + internal class AffiliateFilter(IAffiliateService affiliateService, ICustomerService customerService, - IContextAccessor contextAccessor) : IAsyncActionFilter + IContextAccessor contextAccessor, + IGuestCustomerPersistence guestCustomerPersistence) : IAsyncActionFilter { #region Methods @@ -84,6 +86,9 @@ private async Task SetCustomerAffiliateId(Affiliate affiliate) if (affiliate.Id == contextAccessor.WorkContext.CurrentCustomer.AffiliateId) return; + //a landing page with ?affiliateid= is usually the guest's first, unsaved request + await guestCustomerPersistence.EnsurePersisted(); + //update affiliate identifier contextAccessor.WorkContext.CurrentCustomer.AffiliateId = affiliate.Id; await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCustomer.Id, x => x.AffiliateId, diff --git a/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs b/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs index f5ae2d36d4..143ba5286e 100644 --- a/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs +++ b/src/Web/Grand.Web.Common/Filters/CustomerActivityAttribute.cs @@ -2,6 +2,7 @@ using Grand.Data; using Grand.Domain.Customers; using Grand.Infrastructure; +using Grand.Web.Common.RequestCustomerResolvers; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Extensions; using Microsoft.AspNetCore.Mvc; @@ -26,10 +27,11 @@ public CustomerActivityAttribute() : base(typeof(CustomerActivityFilter)) /// /// Represents a filter that saves last customer activity date /// - private class CustomerActivityFilter( + internal class CustomerActivityFilter( ICustomerService customerService, IContextAccessor contextAccessor, - CustomerSettings customerSettings) : IAsyncActionFilter + CustomerSettings customerSettings, + IGuestCustomerPersistence guestCustomerPersistence) : IAsyncActionFilter { #region Methods @@ -53,9 +55,15 @@ public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionE if (!customerSettings.StoreLastVisitedPage) return; + //system accounts are shared by every request they serve; an unsaved guest is saved by a write, + //which activity tracking must not trigger + var customer = contextAccessor.WorkContext.CurrentCustomer; + if (customer.IsSystemAccount || guestCustomerPersistence.IsTransient(customer.Id)) + return; + //update last activity date - if (contextAccessor.WorkContext.CurrentCustomer.LastActivityDateUtc.AddMinutes(3.0) < DateTime.UtcNow) - await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCustomer, x => x.LastActivityDateUtc, + if (customer.LastActivityDateUtc.AddMinutes(3.0) < DateTime.UtcNow) + await customerService.UpdateCustomerField(customer, x => x.LastActivityDateUtc, DateTime.UtcNow); //get current IP address @@ -63,11 +71,11 @@ await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCus //update customer's IP address if (!string.IsNullOrEmpty(currentIpAddress) && - !currentIpAddress.Equals(contextAccessor.WorkContext.CurrentCustomer.LastIpAddress, + !currentIpAddress.Equals(customer.LastIpAddress, StringComparison.OrdinalIgnoreCase)) { - contextAccessor.WorkContext.CurrentCustomer.LastIpAddress = currentIpAddress; - await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCustomer, x => x.LastIpAddress, + customer.LastIpAddress = currentIpAddress; + await customerService.UpdateCustomerField(customer, x => x.LastIpAddress, currentIpAddress); } @@ -77,10 +85,10 @@ await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCus return; //save new one if don't match - if (!pageUrl.Equals(contextAccessor.WorkContext.CurrentCustomer.LastVisitedPage, StringComparison.OrdinalIgnoreCase)) + if (!pageUrl.Equals(customer.LastVisitedPage, StringComparison.OrdinalIgnoreCase)) { - contextAccessor.WorkContext.CurrentCustomer.LastVisitedPage = pageUrl; - await customerService.UpdateCustomerField(contextAccessor.WorkContext.CurrentCustomer, x => x.LastVisitedPage, pageUrl); + customer.LastVisitedPage = pageUrl; + await customerService.UpdateCustomerField(customer, x => x.LastVisitedPage, pageUrl); } } diff --git a/src/Web/Grand.Web.Common/Filters/PersistGuestCustomerAttribute.cs b/src/Web/Grand.Web.Common/Filters/PersistGuestCustomerAttribute.cs new file mode 100644 index 0000000000..4bb4bfe8b2 --- /dev/null +++ b/src/Web/Grand.Web.Common/Filters/PersistGuestCustomerAttribute.cs @@ -0,0 +1,31 @@ +using Grand.Web.Common.RequestCustomerResolvers; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; + +namespace Grand.Web.Common.Filters; + +/// +/// Saves the request's unsaved guest before a GET action that writes to the current customer. A guest of a +/// GET request is not in the database yet, and a customer write is a partial update by id that silently does +/// nothing on a document that does not exist. Requests of other methods save the guest when it is created +/// +public class PersistGuestCustomerAttribute : TypeFilterAttribute +{ + public PersistGuestCustomerAttribute() : base(typeof(PersistGuestCustomerFilter)) + { + } + + #region Filter + + internal class PersistGuestCustomerFilter(IGuestCustomerPersistence guestCustomerPersistence) + : IAsyncActionFilter + { + public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next) + { + await guestCustomerPersistence.EnsurePersisted(); + await next(); + } + } + + #endregion +} diff --git a/src/Web/Grand.Web.Common/Grand.Web.Common.csproj b/src/Web/Grand.Web.Common/Grand.Web.Common.csproj index 53d8c644ad..f2542a14d6 100644 --- a/src/Web/Grand.Web.Common/Grand.Web.Common.csproj +++ b/src/Web/Grand.Web.Common/Grand.Web.Common.csproj @@ -11,7 +11,6 @@ - diff --git a/src/Web/Grand.Web.Common/Infrastructure/ServiceCollectionExtensions.cs b/src/Web/Grand.Web.Common/Infrastructure/ServiceCollectionExtensions.cs index 7c6dd6fbee..f1757c054a 100644 --- a/src/Web/Grand.Web.Common/Infrastructure/ServiceCollectionExtensions.cs +++ b/src/Web/Grand.Web.Common/Infrastructure/ServiceCollectionExtensions.cs @@ -302,13 +302,4 @@ public static void AddWebEncoder(this IServiceCollection services) }); } - - /// - /// Adds services for detection device - /// - /// Collection of service descriptors - public static void AddDetectionDevice(this IServiceCollection services) - { - services.AddDetection(); - } } \ No newline at end of file diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs index 9b0962899d..66ea5f1c64 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCookieCustomerResolver.cs @@ -17,11 +17,11 @@ public class GuestCookieCustomerResolver( public async Task Resolve() { - var guid = await authenticationService.GetCustomerGuid(); - if (string.IsNullOrEmpty(guid) || !Guid.TryParse(guid, out var customerGuid)) return null; + var cookie = await authenticationService.GetCustomerGuid(); + if (!GuestCustomerRules.TryParseCookie(cookie, out var customerGuid)) return null; var customerByGuid = await customerService.GetCustomerByGuid(customerGuid); - if (customerByGuid is { Deleted: false, Active: true } && !await groupService.IsRegistered(customerByGuid)) + if (customerByGuid != null && await GuestCustomerRules.IsAcceptableGuest(customerByGuid, groupService)) return customerByGuid; return null; diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs new file mode 100644 index 0000000000..de5964e999 --- /dev/null +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerPersistence.cs @@ -0,0 +1,69 @@ +using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Business.Core.Interfaces.Customers; +using Grand.Domain.Customers; +using Microsoft.Extensions.Logging; + +namespace Grand.Web.Common.RequestCustomerResolvers; + +public class GuestCustomerPersistence( + ICustomerService customerService, + IGroupService groupService, + ILogger logger) : IGuestCustomerPersistence +{ + private Task _persisting; + + public Customer Transient { get; private set; } + + public async Task SetTransient(Customer customer) + { + //the Guests group is normally added on insert; permission checks read it from the unsaved customer + var guestsGroup = await groupService.GetCustomerGroupBySystemName(SystemCustomerGroupNames.Guests); + if (guestsGroup != null && !customer.Groups.Contains(guestsGroup.Id)) + customer.Groups.Add(guestsGroup.Id); + + Transient = customer; + _persisting = null; + } + + public bool IsTransient(string customerId) + { + return Transient != null && !string.IsNullOrEmpty(customerId) && Transient.Id == customerId; + } + + public Task EnsurePersisted() + { + if (Transient == null) return Task.CompletedTask; + + //a failed insert is retried by the next write instead of rethrowing the cached failure + if (_persisting is { IsFaulted: true } or { IsCanceled: true }) _persisting = null; + + //writes of one request may run concurrently: they share the single insert + return _persisting ??= Persist(Transient); + } + + private async Task Persist(Customer guest) + { + await customerService.InsertGuestCustomer(guest); + Transient = null; + } + + public async Task Promote(Customer guest) + { + try + { + return await customerService.InsertGuestCustomer(guest); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + //the unique CustomerGuid index: a parallel request of the same browser saved this cookie first + var existing = await customerService.GetCustomerByGuid(guest.CustomerGuid); + if (existing == null) throw; + + if (await GuestCustomerRules.IsAcceptableGuest(existing, groupService)) return existing; + + logger.LogWarning(ex, "Guest cookie {CustomerGuid} belongs to a customer that is not a guest; a new guest is issued", + guest.CustomerGuid); + return null; + } + } +} diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRules.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRules.cs new file mode 100644 index 0000000000..bb175ca7f4 --- /dev/null +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/GuestCustomerRules.cs @@ -0,0 +1,28 @@ +using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Domain.Customers; + +namespace Grand.Web.Common.RequestCustomerResolvers; + +/// +/// What a guest cookie may resolve to +/// +public static class GuestCustomerRules +{ + /// + /// The cookie holds a GUID in the format the store writes ("D"), never Guid.Empty + /// + public static bool TryParseCookie(string value, out Guid customerGuid) + { + return Guid.TryParseExact(value, "D", out customerGuid) && customerGuid != Guid.Empty; + } + + /// + /// An active guest: not deleted, not registered and not a system account (Anonymous and SearchEngine are + /// in the Guests group too) + /// + public static async Task IsAcceptableGuest(Customer customer, IGroupService groupService) + { + return customer is { Deleted: false, Active: true, IsSystemAccount: false } + && !await groupService.IsRegistered(customer); + } +} diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/IGuestCustomerPersistence.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/IGuestCustomerPersistence.cs new file mode 100644 index 0000000000..d6d1f9ffc7 --- /dev/null +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/IGuestCustomerPersistence.cs @@ -0,0 +1,30 @@ +using Grand.Domain.Customers; + +namespace Grand.Web.Common.RequestCustomerResolvers; + +/// +/// The guest of this request that is not saved yet: saved on its first write, or by a later request that +/// returns its cookie +/// +public interface IGuestCustomerPersistence +{ + /// + /// The unsaved guest of this request; null once saved or when the customer came from the database + /// + Customer Transient { get; } + + Task SetTransient(Customer customer); + + bool IsTransient(string customerId); + + /// + /// Saves the transient guest once; later calls do nothing + /// + Task EnsurePersisted(); + + /// + /// Saves a guest for a returned cookie whose GUID no customer has. Returns the saved guest, the guest a + /// parallel request saved first, or null when the GUID turned out to belong to a customer that is not a guest + /// + Task Promote(Customer guest); +} diff --git a/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs b/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs index 14ed77830e..9d68f737e5 100644 --- a/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs +++ b/src/Web/Grand.Web.Common/RequestCustomerResolvers/SearchEngineCustomerResolver.cs @@ -1,6 +1,7 @@ using Grand.Business.Core.Interfaces.Customers; using Grand.Domain.Customers; -using Wangkanai.Detection.Services; +using Grand.Web.Common.UserAgent; +using Microsoft.AspNetCore.Http; namespace Grand.Web.Common.RequestCustomerResolvers; @@ -8,15 +9,19 @@ namespace Grand.Web.Common.RequestCustomerResolvers; /// Crawlers run as the SearchEngine system customer /// public class SearchEngineCustomerResolver( - IDetectionService detectionService, - ICustomerService customerService) : IRequestCustomerResolver + IUserAgentClassifier userAgentClassifier, + ICustomerService customerService, + IHttpContextAccessor httpContextAccessor) : IRequestCustomerResolver { public int Order => RequestCustomerResolverOrder.SearchEngine; public async Task Resolve() { - var isCrawler = detectionService.Crawler?.IsCrawler; - if (!isCrawler.GetValueOrDefault()) return null; + //an authenticated API client is never treated as a crawler + var request = httpContextAccessor.HttpContext?.Request; + if (request != null && !string.IsNullOrEmpty(request.Headers.Authorization.ToString())) return null; + + if (!userAgentClassifier.IsCrawler) return null; return await customerService.GetCustomerBySystemName(SystemCustomerNames.SearchEngine); } diff --git a/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs b/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs index 45815a79cd..569669d024 100644 --- a/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs +++ b/src/Web/Grand.Web.Common/Startup/GrandCommonStartup.cs @@ -1,7 +1,9 @@ using Grand.Infrastructure; using Grand.Infrastructure.Configuration; using Grand.Web.Common.Infrastructure; +using Grand.Web.Common.RequestCustomerResolvers; using Grand.Web.Common.Routing; +using Grand.Web.Common.UserAgent; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.ResponseCompression; @@ -68,8 +70,11 @@ public void ConfigureServices(IServiceCollection services, IConfiguration config //add WebEncoderOptions services.AddWebEncoder(); - //add detection device - services.AddDetectionDevice(); + //classify the request's user agent (crawler, mobile) + services.AddScoped(); + + //a guest is saved only once it returns its cookie or an action marked [PersistGuestCustomer] writes to it + services.AddScoped(); //add routing services.AddRouting(options => @@ -122,9 +127,6 @@ public void Configure(WebApplication application, IWebHostEnvironment webHostEnv if (!performanceConfig.IgnoreUsePoweredByMiddleware) application.UsePoweredBy(); - //add responsive middleware (for detection) - application.UseDetection(); - //use routing application.UseRouting(); } diff --git a/src/Web/Grand.Web.Common/TagHelpers/Admin/AdminTabStripTagHelper.cs b/src/Web/Grand.Web.Common/TagHelpers/Admin/AdminTabStripTagHelper.cs index ae00c72220..31e66c0ebb 100644 --- a/src/Web/Grand.Web.Common/TagHelpers/Admin/AdminTabStripTagHelper.cs +++ b/src/Web/Grand.Web.Common/TagHelpers/Admin/AdminTabStripTagHelper.cs @@ -1,12 +1,11 @@ using Grand.Mediator; using Grand.Web.Common.Events; +using Grand.Web.Common.UserAgent; using Microsoft.AspNetCore.Html; using Microsoft.AspNetCore.Mvc.Rendering; using Microsoft.AspNetCore.Mvc.ViewFeatures; using Microsoft.AspNetCore.Razor.TagHelpers; using System.Text.Json; -using Wangkanai.Detection.Models; -using Wangkanai.Detection.Services; namespace Grand.Web.Common.TagHelpers.Admin; @@ -19,13 +18,13 @@ namespace Grand.Web.Common.TagHelpers.Admin; [HtmlTargetElement("admin-tabstrip")] public class AdminTabStripTagHelper : TagHelper { - private readonly IDetectionService _detectionService; + private readonly IUserAgentClassifier _userAgentClassifier; private readonly IMediator _mediator; - public AdminTabStripTagHelper(IMediator mediator, IDetectionService detectionService) + public AdminTabStripTagHelper(IMediator mediator, IUserAgentClassifier userAgentClassifier) { _mediator = mediator; - _detectionService = detectionService; + _userAgentClassifier = userAgentClassifier; } [HtmlAttributeName("SetTabPos")] public bool SetTabPos { get; set; } = false; @@ -41,7 +40,7 @@ public override async Task ProcessAsync(TagHelperContext context, TagHelperOutpu ViewContext.ViewData[typeof(AdminTabContentTagHelper).FullName] = new List(); _ = await output.GetChildContentAsync(); var list = (List)ViewContext.ViewData[typeof(AdminTabContentTagHelper).FullName]; - if (_detectionService.Device.Type == Device.Mobile || _detectionService.Device.Type == Device.Tablet) + if (_userAgentClassifier.IsMobileOrTablet) SetTabPos = false; var selectedTabIndex = GetSelectedTabIndex(); diff --git a/src/Web/Grand.Web.Common/UserAgent/IUserAgentClassifier.cs b/src/Web/Grand.Web.Common/UserAgent/IUserAgentClassifier.cs new file mode 100644 index 0000000000..bdbda0dc70 --- /dev/null +++ b/src/Web/Grand.Web.Common/UserAgent/IUserAgentClassifier.cs @@ -0,0 +1,17 @@ +namespace Grand.Web.Common.UserAgent; + +/// +/// What the current request's User-Agent says about the client +/// +public interface IUserAgentClassifier +{ + /// + /// A crawler identified by its User-Agent. A missing User-Agent or a plain HTTP client is not a crawler + /// + bool IsCrawler { get; } + + /// + /// A phone or a tablet + /// + bool IsMobileOrTablet { get; } +} diff --git a/src/Web/Grand.Web.Common/UserAgent/UserAgentClassifier.cs b/src/Web/Grand.Web.Common/UserAgent/UserAgentClassifier.cs new file mode 100644 index 0000000000..920e2024ec --- /dev/null +++ b/src/Web/Grand.Web.Common/UserAgent/UserAgentClassifier.cs @@ -0,0 +1,41 @@ +using System.Text.RegularExpressions; +using Microsoft.AspNetCore.Http; + +namespace Grand.Web.Common.UserAgent; + +/// +/// Classifies the current request's User-Agent once per request +/// +public class UserAgentClassifier(IHttpContextAccessor httpContextAccessor) : IUserAgentClassifier +{ + //user agents longer than this are cut before matching + private const int MaxLength = 512; + + //"bot" not preceded by "cu": CUBOT is an Android phone brand + private static readonly Regex CrawlerPattern = new( + "(? _isCrawler ??= Classify(userAgent => + !string.IsNullOrWhiteSpace(userAgent) && CrawlerPattern.IsMatch(userAgent)); + + public bool IsMobileOrTablet => _isMobileOrTablet ??= Classify(userAgent => + !string.IsNullOrWhiteSpace(userAgent) && MobileOrTabletPattern.IsMatch(userAgent)); + + private bool Classify(Func rule) + { + var request = httpContextAccessor.HttpContext?.Request; + if (request == null) return false; + + var userAgent = request.Headers.UserAgent.ToString(); + if (userAgent.Length > MaxLength) userAgent = userAgent[..MaxLength]; + return rule(userAgent); + } +} diff --git a/src/Web/Grand.Web.Common/WorkContextSetter.cs b/src/Web/Grand.Web.Common/WorkContextSetter.cs index 6f4777d571..aeba6b8a8c 100644 --- a/src/Web/Grand.Web.Common/WorkContextSetter.cs +++ b/src/Web/Grand.Web.Common/WorkContextSetter.cs @@ -217,6 +217,49 @@ private async Task GetBackgroundTaskCustomer() } private async Task CreateCustomerGuest(Store store) + { + //resolved here, not injected: it belongs to the request scope + var persistence = _serviceProvider.GetService(); + + //a browser returning the cookie of a guest that was never saved: save it now under that GUID + var returningGuid = persistence != null ? await ReturningGuestGuid() : null; + if (returningGuid.HasValue) + { + var promoted = await persistence.Promote(BuildGuest(store, returningGuid.Value)); + if (promoted != null) return promoted; + } + + var customer = BuildGuest(store, Guid.NewGuid()); + if (persistence != null && IsSafeRequest()) + //saved on its first write, or by the next request that returns the cookie + await persistence.SetTransient(customer); + else + customer = await _customerService.InsertGuestCustomer(customer); + + await _authenticationService.SetCustomerGuid(customer.CustomerGuid); + return customer; + } + + /// + /// The guest cookie's GUID when no customer has it; an existing customer (registered, administrator, system, + /// deleted) is never taken over + /// + private async Task ReturningGuestGuid() + { + var cookie = await _authenticationService.GetCustomerGuid(); + if (!GuestCustomerRules.TryParseCookie(cookie, out var customerGuid)) return null; + + return await _customerService.GetCustomerByGuid(customerGuid) == null ? customerGuid : null; + } + + private bool IsSafeRequest() + { + var method = _httpContextAccessor.HttpContext?.Request.Method; + return method != null && (HttpMethods.IsGet(method) || HttpMethods.IsHead(method) || + HttpMethods.IsOptions(method)); + } + + private Customer BuildGuest(Store store, Guid customerGuid) { var userFields = new List(); @@ -234,20 +277,14 @@ private async Task CreateCustomerGuest(Store store) StoreId = store.Id }); - var customer = new Customer { - CustomerGuid = Guid.NewGuid(), + return new Customer { + CustomerGuid = customerGuid, Active = true, StoreId = store.Id, LastActivityDateUtc = DateTime.UtcNow, LastIpAddress = _httpContextAccessor?.HttpContext?.Connection?.RemoteIpAddress?.ToString(), UserFields = userFields }; - - customer = await _customerService.InsertGuestCustomer(customer); - - //set customer cookie - await _authenticationService.SetCustomerGuid(customer.CustomerGuid); - return customer; } /// diff --git a/src/Web/Grand.Web/Controllers/CommonController.cs b/src/Web/Grand.Web/Controllers/CommonController.cs index e0a36bd6cd..139d23312e 100644 --- a/src/Web/Grand.Web/Controllers/CommonController.cs +++ b/src/Web/Grand.Web/Controllers/CommonController.cs @@ -144,6 +144,7 @@ public virtual IActionResult ExternalAuthenticationError(IEnumerable err [HttpGet] [PublicStore(true)] [DenySystemAccount] + [PersistGuestCustomer] public virtual async Task SetLanguage( [FromServices] AppConfig config, [FromServices] ICustomerService customerService, @@ -211,6 +212,7 @@ public virtual IActionResult InternalRedirect(string url, bool permanentRedirect [DenySystemAccount] [PublicStore(true)] [HttpGet] + [PersistGuestCustomer] public virtual async Task SetCurrency( [FromServices] ICurrencyService currencyService, [FromServices] ICustomerService customerService, @@ -289,6 +291,7 @@ void SetStoreCookie(Domain.Stores.Store store) //available even when navigation is not allowed [PublicStore(true)] [HttpGet] + [PersistGuestCustomer] public virtual async Task SetTaxType( [FromServices] TaxSettings taxSettings, [FromServices] ICustomerService customerService, diff --git a/src/Web/Grand.Web/Controllers/ProductController.cs b/src/Web/Grand.Web/Controllers/ProductController.cs index 4d7769ccdb..4f7ac0c1be 100644 --- a/src/Web/Grand.Web/Controllers/ProductController.cs +++ b/src/Web/Grand.Web/Controllers/ProductController.cs @@ -16,6 +16,7 @@ using Grand.Web.Common.Controllers; using Grand.Web.Common.Extensions; using Grand.Web.Common.Filters; +using Grand.Web.Common.RequestCustomerResolvers; using Grand.Web.Common.Security.Captcha; using Grand.Web.Events; using Grand.Web.Extensions; @@ -44,7 +45,8 @@ public ProductController( IPermissionService permissionService, IMediator mediator, CatalogSettings catalogSettings, - CaptchaSettings captchaSettings + CaptchaSettings captchaSettings, + IGuestCustomerPersistence guestCustomerPersistence ) { _productService = productService; @@ -57,6 +59,7 @@ CaptchaSettings captchaSettings _mediator = mediator; _catalogSettings = catalogSettings; _captchaSettings = captchaSettings; + _guestCustomerPersistence = guestCustomerPersistence; } #endregion @@ -262,6 +265,7 @@ public virtual async Task GetDatesForMonth(string productId, int private readonly IMediator _mediator; private readonly CatalogSettings _catalogSettings; private readonly CaptchaSettings _captchaSettings; + private readonly IGuestCustomerPersistence _guestCustomerPersistence; #endregion @@ -320,8 +324,9 @@ public virtual async Task> ProductDetails(stri var productLayoutViewPath = await _mediator.Send(new GetProductLayoutViewPath { ProductLayoutId = product.ProductLayoutId }); - //save as recently viewed - await _recentlyViewedProductsService.AddProductToRecentlyViewedList(customer.Id, product.Id); + //save as recently viewed - not for the guest that is not saved yet: its rows would stay orphaned + if (!_guestCustomerPersistence.IsTransient(customer.Id)) + await _recentlyViewedProductsService.AddProductToRecentlyViewedList(customer.Id, product.Id); //display "edit" (manage) link if (await _permissionService.Authorize(StandardPermission.ManageAccessAdminPanel, customer) && @@ -535,8 +540,9 @@ public virtual async Task QuickView(string productId) var productLayoutViewPath = await _mediator.Send(new GetProductLayoutViewPath { ProductLayoutId = product.ProductLayoutId }); - //save as recently viewed - await _recentlyViewedProductsService.AddProductToRecentlyViewedList(customer.Id, product.Id); + //save as recently viewed - not for the guest that is not saved yet: its rows would stay orphaned + if (!_guestCustomerPersistence.IsTransient(customer.Id)) + await _recentlyViewedProductsService.AddProductToRecentlyViewedList(customer.Id, product.Id); _ = _productService.IncrementProductField(product, x => x.Viewed, 1); diff --git a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Currency.cshtml b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Currency.cshtml index e10689880a..c715f0ce8c 100644 --- a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Currency.cshtml +++ b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Currency.cshtml @@ -38,7 +38,7 @@ @foreach (var item in currencies) { var className = item.Selected ? "active" : ""; -
  • @item.Text
    • +
  • @item.Text
    • } diff --git a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Language.cshtml b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Language.cshtml index a776ea7c09..f9be99364b 100644 --- a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Language.cshtml +++ b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Language.cshtml @@ -22,7 +22,7 @@ @foreach (var lang in availableLanguages) { var className = lang.Id == contextAccessor.WorkContext.WorkingLanguage.Id ? "active" : ""; -
  • +
  • @lang.Name
    • } diff --git a/src/Web/Grand.Web/Views/Shared/Partials/Selector_TaxType.cshtml b/src/Web/Grand.Web/Views/Shared/Partials/Selector_TaxType.cshtml index 51075fb068..62e8562b0f 100644 --- a/src/Web/Grand.Web/Views/Shared/Partials/Selector_TaxType.cshtml +++ b/src/Web/Grand.Web/Views/Shared/Partials/Selector_TaxType.cshtml @@ -19,10 +19,10 @@
    diff --git a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Theme.cshtml b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Theme.cshtml index 78111a5c88..b3449d9c37 100644 --- a/src/Web/Grand.Web/Views/Shared/Partials/Selector_Theme.cshtml +++ b/src/Web/Grand.Web/Views/Shared/Partials/Selector_Theme.cshtml @@ -25,7 +25,7 @@ @foreach (var theme in availablThemes) { var className = theme.Name == themeContext?.GetCurrentTheme() ? "active" : ""; -
  • +
  • @(theme.Title ?? theme.Name)
    • }