From e0887e83d4b36fb722134b9c21ba700ca28e5287 Mon Sep 17 00:00:00 2001
From: KrzysztofPajak <krzysiek@grandnode.com>
Date: Wed, 9 Sep 2026 20:00:40 +0200
Subject: [PATCH] fix(store): close AvailableStores cross-store leak in
 Customer controller

Follow-up to ARCH-001 Phase 20 (Customer consolidation, #819), same
class of issue caught during Phase 21 (EmailAccount, #820) code
review.

BaseCustomerController.Create()/Edit(string) always populate
model.AvailableStores with every store's id/name via the shared
PrepareCustomerModel call. Store's own CreateOrUpdate.TabInfo view
gates the StaffStoreId/StoreId <select>s on Scope.DefaultStoreId is
null and never renders them for Store - but the model still reaches
Store's store_customer_details_* widget zones via
additional-data="Model", so a future Store widget reading
Model.AvailableStores could leak other stores' names/ids from a
store-scoped screen.

Fix in the controller: clear AvailableStores under Store scope
(scope.DefaultStoreId is not null) right after PrepareCustomerModel,
on all four call sites (Create GET, Create POST redisplay, Edit GET,
Edit POST redisplay). No-op for Admin.

Adds regression coverage: Create/Edit GET keep AvailableStores under
Admin scope, clear it under Store scope.
---
 .../BaseCustomerControllerTests.cs            | 76 +++++++++++++++++++
 .../Controllers/BaseCustomerController.cs     | 17 +++++
 2 files changed, 93 insertions(+)

diff --git a/src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCustomerControllerTests.cs b/src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCustomerControllerTests.cs
index 69de8356b..cda632b19 100644
--- a/src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCustomerControllerTests.cs
+++ b/src/Tests/Grand.Web.Admin.Tests/Controllers/BaseCustomerControllerTests.cs
@@ -14,6 +14,7 @@
 using Grand.Web.Common.DataSource;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
+using Microsoft.AspNetCore.Mvc.Rendering;
 using Microsoft.AspNetCore.Mvc.Routing;
 using Microsoft.AspNetCore.Mvc.ViewFeatures;
 using Microsoft.Extensions.Logging;
@@ -186,6 +187,81 @@ public async Task CreateGet_GlobalScope_DoesNotCallApplyPostConstraints_ModelUnt
         Assert.AreEqual("", model.StoreId ?? ""); // never forced for Admin (default no-op hook)
     }
 
+    [TestMethod]
+    public async Task CreateGet_StoreScope_ClearsAvailableStores()
+    {
+        // Regression test for the cross-store leak flagged in code review: PrepareCustomerModel
+        // always populates every store's id/name onto AvailableStores (used for the
+        // StaffStoreId/StoreId pickers), but Store's own view never renders either select and the
+        // model still reaches Store's store_customer_details_* widget zones via
+        // additional-data="Model" - other stores' names/ids must not leak there.
+        ScopeMock.Setup(s => s.DefaultStoreId).Returns("store-1");
+        CustomerViewModelServiceMock
+            .Setup(v => v.PrepareCustomerModel(It.IsAny<CustomerModel>(), null, false))
+            .Callback<CustomerModel, Customer, bool>((m, _, _) =>
+                m.AvailableStores.Add(new SelectListItem { Value = "store-1", Text = "Store 1" }))
+            .Returns(Task.CompletedTask);
+
+        var result = await Controller.Create();
+
+        var model = (CustomerModel)((ViewResult)result).Model;
+        Assert.AreEqual(0, model.AvailableStores.Count);
+    }
+
+    [TestMethod]
+    public async Task CreateGet_GlobalScope_KeepsAvailableStores()
+    {
+        ScopeMock.Setup(s => s.DefaultStoreId).Returns((string)null);
+        CustomerViewModelServiceMock
+            .Setup(v => v.PrepareCustomerModel(It.IsAny<CustomerModel>(), null, false))
+            .Callback<CustomerModel, Customer, bool>((m, _, _) =>
+                m.AvailableStores.Add(new SelectListItem { Value = "store-1", Text = "Store 1" }))
+            .Returns(Task.CompletedTask);
+
+        var result = await Controller.Create();
+
+        var model = (CustomerModel)((ViewResult)result).Model;
+        Assert.AreEqual(1, model.AvailableStores.Count);
+    }
+
+    [TestMethod]
+    public async Task EditGet_StoreScope_ClearsAvailableStores()
+    {
+        var customer = new Customer { Id = "c1" };
+        CustomerServiceMock.Setup(s => s.GetCustomerById("c1")).ReturnsAsync(customer);
+        ScopeMock.Setup(s => s.HasAccess(customer)).ReturnsAsync(true);
+        ScopeMock.Setup(s => s.DefaultStoreId).Returns("store-1");
+        CustomerViewModelServiceMock
+            .Setup(v => v.PrepareCustomerModel(It.IsAny<CustomerModel>(), customer, false))
+            .Callback<CustomerModel, Customer, bool>((m, _, _) =>
+                m.AvailableStores.Add(new SelectListItem { Value = "store-1", Text = "Store 1" }))
+            .Returns(Task.CompletedTask);
+
+        var result = await Controller.Edit("c1");
+
+        var model = (CustomerModel)((ViewResult)result).Model;
+        Assert.AreEqual(0, model.AvailableStores.Count);
+    }
+
+    [TestMethod]
+    public async Task EditGet_GlobalScope_KeepsAvailableStores()
+    {
+        var customer = new Customer { Id = "c1" };
+        CustomerServiceMock.Setup(s => s.GetCustomerById("c1")).ReturnsAsync(customer);
+        ScopeMock.Setup(s => s.HasAccess(customer)).ReturnsAsync(true);
+        ScopeMock.Setup(s => s.DefaultStoreId).Returns((string)null);
+        CustomerViewModelServiceMock
+            .Setup(v => v.PrepareCustomerModel(It.IsAny<CustomerModel>(), customer, false))
+            .Callback<CustomerModel, Customer, bool>((m, _, _) =>
+                m.AvailableStores.Add(new SelectListItem { Value = "store-1", Text = "Store 1" }))
+            .Returns(Task.CompletedTask);
+
+        var result = await Controller.Edit("c1");
+
+        var model = (CustomerModel)((ViewResult)result).Model;
+        Assert.AreEqual(1, model.AvailableStores.Count);
+    }
+
     [TestMethod]
     public async Task CreatePost_Invalid_TwoFactorWarningHookNotCalledOnBase()
     {
diff --git a/src/Web/Grand.Web.AdminShared/Controllers/BaseCustomerController.cs b/src/Web/Grand.Web.AdminShared/Controllers/BaseCustomerController.cs
index e3d1b2106..d51774818 100644
--- a/src/Web/Grand.Web.AdminShared/Controllers/BaseCustomerController.cs
+++ b/src/Web/Grand.Web.AdminShared/Controllers/BaseCustomerController.cs
@@ -74,6 +74,19 @@ public abstract class BaseCustomerController(
     /// (BaseCustomerManagementController never overrides this — Admin never had this method).</summary>
     protected virtual Task ApplyPostConstraints(CustomerModel model) => Task.CompletedTask;
 
+    /// <summary>PrepareCustomerModel always populates every store's id/name onto
+    /// model.AvailableStores (used for the StaffStoreId/StoreId pickers). Store's own
+    /// CreateOrUpdate.TabInfo view gates both selects on Scope.DefaultStoreId is null and never
+    /// renders them - but the model is still handed to Store's store_customer_details_* widget
+    /// zones via additional-data="Model", so a Store widget must not be able to read other
+    /// stores' names/ids off a store-scoped screen. Same fix and rationale as ARCH-001
+    /// EmailAccount's BaseEmailAccountController (code review, 2026-09-09).</summary>
+    private void ClearAvailableStoresForStoreScope(CustomerModel model)
+    {
+        if (scope.DefaultStoreId is not null)
+            model.AvailableStores.Clear();
+    }
+
     /// <summary>Admin-only: warns when a submitted model newly enables two-factor auth. No-op here
     /// — Store's originals never referenced TwoFactorEnabled at all. existingCustomer is null on
     /// Create, which collapses to Admin's original simpler create-time condition automatically.</summary>
@@ -184,6 +197,7 @@ public async Task<IActionResult> Create()
         var model = new CustomerModel();
         await customerViewModelService.PrepareCustomerModel(model, null, false);
         await ApplyPostConstraints(model);
+        ClearAvailableStoresForStoreScope(model);
         model.Active = true;
         return View(model);
     }
@@ -218,6 +232,7 @@ public async Task<IActionResult> Create(CustomerModel model, bool continueEditin
 
         await customerViewModelService.PrepareCustomerModel(model, null, true);
         await ApplyPostConstraints(model);
+        ClearAvailableStoresForStoreScope(model);
         return View(model);
     }
 
@@ -244,6 +259,7 @@ public async Task<IActionResult> Edit(string id)
 
         var model = new CustomerModel();
         await customerViewModelService.PrepareCustomerModel(model, customer, false);
+        ClearAvailableStoresForStoreScope(model);
         return View(model);
     }
 
@@ -287,6 +303,7 @@ public async Task<IActionResult> Edit(CustomerModel model, bool continueEditing)
 
         await customerViewModelService.PrepareCustomerModel(model, customer, true);
         await ApplyPostConstraints(model);
+        ClearAvailableStoresForStoreScope(model);
         return View(model);
     }