fix(store): close AvailableStores cross-store leak in Customer controller - #821
Merged
Merged
Conversation
…ller 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
Follow-up to ARCH-001 Phase 20 (Customer consolidation, #819). Code review of Phase 21 (EmailAccount, #820) caught the same class of bug there and, while auditing the rest of
Grand.Web.AdminSharedfor the same pattern, found it also applies to Customer — already merged todevelop.BaseCustomerController.Create()/Edit(string id)always populatemodel.AvailableStoreswith every store's id/name via the sharedPrepareCustomerModelcall. Store's ownCreateOrUpdate.TabInfoview gates theStaffStoreId/StoreId<select>s onScope.DefaultStoreId is nulland never renders either for Store — but the model still reaches Store'sstore_customer_details_*widget zones viaadditional-data="Model"(confirmed:WidgetZone.InfoTop.cshtml,WidgetZone.InfoBottom.cshtml,WidgetZone.DetailsTabs.cshtml, etc. all passModel). A Store widget readingModel.AvailableStorescould leak other stores' names/ids from a store-scoped screen.Solution
Same fix pattern as
BaseEmailAccountControllerin #820: clearAvailableStoresunder Store scope (scope.DefaultStoreId is not null) right afterPrepareCustomerModel, on all four call sites (Create GET, Create POST redisplay, Edit GET, Edit POST redisplay) via a small private helper. No-op for Admin.Testing
dotnet build src/Web/Grand.Web.AdminShared— 0 errors.dotnet test src/Tests/Grand.Web.Admin.Tests --filter BaseCustomerControllerTests— 35/35 passing (31 existing + 4 new regression tests: Create/Edit GET keepAvailableStoresunder Admin scope, clear it under Store scope).dotnet test src/Tests/Grand.Web.Store.Tests --filter Customer— 13/13 passing.Breaking changes
None.
🤖 Generated with Claude Code