From 22461c5f18355c9dc7045ba8fa18a35d1cb57e9a Mon Sep 17 00:00:00 2001 From: support Date: Sun, 1 Nov 2026 21:56:57 +0100 Subject: [PATCH 1/2] Answer 404 from the permission actions popup for an unknown group or permission The popup edits one permission of one customer group, so both must exist. The save checks them before it changes anything. --- .../Controllers/PermissionControllerTests.cs | 125 ++++++++++++++++++ .../Controllers/PermissionController.cs | 30 ++--- 2 files changed, 134 insertions(+), 21 deletions(-) create mode 100644 src/Tests/Grand.Web.Admin.Tests/Controllers/PermissionControllerTests.cs diff --git a/src/Tests/Grand.Web.Admin.Tests/Controllers/PermissionControllerTests.cs b/src/Tests/Grand.Web.Admin.Tests/Controllers/PermissionControllerTests.cs new file mode 100644 index 000000000..7bf4dff2c --- /dev/null +++ b/src/Tests/Grand.Web.Admin.Tests/Controllers/PermissionControllerTests.cs @@ -0,0 +1,125 @@ +using Grand.Business.Core.Interfaces.Common.Directory; +using Grand.Business.Core.Interfaces.Common.Security; +using Grand.Domain.Customers; +using Grand.Domain.Localization; +using Grand.Domain.Permissions; +using Grand.Infrastructure; +using Grand.Web.Admin.Controllers; +using Grand.Web.AdminShared.Models.Permissions; +using Grand.Web.Common.Localization; +using Microsoft.AspNetCore.Mvc; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; + +namespace Grand.Web.Admin.Tests.Controllers; + +[TestClass] +public class PermissionControllerTests +{ + private const string GroupId = "group-1"; + private const string SystemName = "ManageProducts"; + + private readonly Mock _permissionServiceMock = new(); + private readonly Mock _groupServiceMock = new(); + private PermissionController _controller; + + [TestInitialize] + public void Init() + { + var workContextMock = new Mock(); + workContextMock.Setup(w => w.WorkingLanguage).Returns(new Language { Id = "lang-1" }); + var contextAccessorMock = new Mock(); + contextAccessorMock.Setup(c => c.WorkContext).Returns(workContextMock.Object); + + _permissionServiceMock.Setup(p => p.GetPermissionActions(SystemName, GroupId)) + .ReturnsAsync(new List()); + + _controller = new PermissionController(contextAccessorMock.Object, _permissionServiceMock.Object, + _groupServiceMock.Object, new Mock().Object); + } + + private void GroupExists() => + _groupServiceMock.Setup(g => g.GetCustomerGroupById(GroupId)) + .ReturnsAsync(new CustomerGroup { Id = GroupId, Name = "Group" }); + + private void PermissionExists() => + _permissionServiceMock.Setup(p => p.GetPermissionBySystemName(SystemName)) + .ReturnsAsync(new Permission { SystemName = SystemName, Actions = new List { "List", "Edit" } }); + + private void VerifyNothingWritten() + { + _permissionServiceMock.Verify(p => p.DeletePermissionAction(It.IsAny()), Times.Never); + _permissionServiceMock.Verify(p => p.InsertPermissionAction(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task PermissionsActionGet_ShowsTheActionsOfAKnownGroupAndPermission() + { + GroupExists(); + PermissionExists(); + + var result = await _controller.PermissionsAction(SystemName, GroupId); + + var model = (PermissionActionModel)((ViewResult)result).Model; + Assert.AreEqual("Group", model.CustomerGroupName); + CollectionAssert.AreEqual(new List { "List", "Edit" }, model.AvailableActions.ToList()); + } + + [TestMethod] + public async Task PermissionsActionGet_UnknownGroup_IsNotFound() + { + PermissionExists(); + + var result = await _controller.PermissionsAction(SystemName, GroupId); + + Assert.IsInstanceOfType(result); + } + + [TestMethod] + public async Task PermissionsActionGet_UnknownPermission_IsNotFound() + { + GroupExists(); + + var result = await _controller.PermissionsAction(SystemName, GroupId); + + Assert.IsInstanceOfType(result); + } + + [TestMethod] + public async Task PermissionsActionPost_DeniesTheActionsNotSelected() + { + GroupExists(); + PermissionExists(); + + var result = await _controller.PermissionsAction(new PermissionActionSaveModel(SystemName, GroupId, new List { "List" })); + + Assert.IsInstanceOfType(result); + _permissionServiceMock.Verify(p => p.InsertPermissionAction(It.Is(a => + a.Action == "Edit" && a.CustomerGroupId == GroupId && a.SystemName == SystemName)), Times.Once); + _permissionServiceMock.Verify(p => p.InsertPermissionAction(It.IsAny()), Times.Once); + } + + [TestMethod] + public async Task PermissionsActionPost_UnknownGroup_IsNotFoundAndWritesNothing() + { + PermissionExists(); + + var result = await _controller.PermissionsAction(new PermissionActionSaveModel(SystemName, GroupId, null)); + + Assert.IsInstanceOfType(result); + VerifyNothingWritten(); + } + + [TestMethod] + public async Task PermissionsActionPost_UnknownPermission_IsNotFoundAndWritesNothing() + { + GroupExists(); + _permissionServiceMock.Setup(p => p.GetPermissionActions(SystemName, GroupId)) + .ReturnsAsync(new List { new() { Action = "Edit" } }); + + var result = await _controller.PermissionsAction(new PermissionActionSaveModel(SystemName, GroupId, null)); + + Assert.IsInstanceOfType(result); + VerifyNothingWritten(); + } +} diff --git a/src/Web/Grand.Web.Admin/Controllers/PermissionController.cs b/src/Web/Grand.Web.Admin/Controllers/PermissionController.cs index 72bfd7816..975b60932 100644 --- a/src/Web/Grand.Web.Admin/Controllers/PermissionController.cs +++ b/src/Web/Grand.Web.Admin/Controllers/PermissionController.cs @@ -155,27 +155,13 @@ public async Task PermissionsAction(string systemName, string cus }; var customerGroup = await _groupService.GetCustomerGroupById(customeGroupId); - if (customerGroup != null) - { - model.CustomerGroupName = customerGroup.Name; - } - else - { - ViewBag.ClosePage = true; - return await PermissionsAction(systemName, customeGroupId); - } + if (customerGroup == null) return NotFound(); + model.CustomerGroupName = customerGroup.Name; var permissionRecord = await _permissionService.GetPermissionBySystemName(systemName); - if (permissionRecord != null) - { - model.AvailableActions = permissionRecord.Actions.ToList(); - model.PermissionName = _translationService.GetResource(permissionRecord.GetTranslationPermissionName(), _contextAccessor.WorkContext.WorkingLanguage.Id); - } - else - { - ViewBag.ClosePage = true; - return await PermissionsAction(systemName, customeGroupId); - } + if (permissionRecord == null) return NotFound(); + model.AvailableActions = permissionRecord.Actions.ToList(); + model.PermissionName = _translationService.GetResource(permissionRecord.GetTranslationPermissionName(), _contextAccessor.WorkContext.WorkingLanguage.Id); model.DeniedActions = (await _permissionService.GetPermissionActions(systemName, customeGroupId)) .Select(x => x.Action).ToList(); @@ -186,12 +172,15 @@ public async Task PermissionsAction(string systemName, string cus [HttpPost] public async Task PermissionsAction(PermissionActionSaveModel model) { + var permissionRecord = await _permissionService.GetPermissionBySystemName(model.SystemName); + if (permissionRecord == null) return NotFound(); + if (await _groupService.GetCustomerGroupById(model.CustomerGroupId) == null) return NotFound(); + //remove denied actions var deniedActions = await _permissionService.GetPermissionActions(model.SystemName, model.CustomerGroupId); foreach (var action in deniedActions) await _permissionService.DeletePermissionAction(action); //insert denied actions - var permissionRecord = await _permissionService.GetPermissionBySystemName(model.SystemName); var insertActions = permissionRecord.Actions.Except(model.SelectedActions ?? new List()); foreach (var item in insertActions) @@ -201,7 +190,6 @@ await _permissionService.InsertPermissionAction(new PermissionAction { SystemName = model.SystemName }); - ViewBag.ClosePage = true; return await PermissionsAction(model.SystemName, model.CustomerGroupId); } From 9f2c0f994d1563af7cf71b40a1b6e74cb4dc23c2 Mon Sep 17 00:00:00 2001 From: support Date: Mon, 2 Nov 2026 07:01:14 +0100 Subject: [PATCH 2/2] Tell the admin when the permission actions popup cannot save An empty 404 on the POST is re-executed by the status code pages at /page-not-found, still as a POST, where it ran the storefront's BlogController.BlogPost. A 404 with a body is left alone. The popup had no error callback, so a rejected save showed nothing and stayed open. Also cover removing an earlier denial and saving with nothing selected. --- .../Controllers/PermissionControllerTests.cs | 30 +++++++++++++++++-- .../Views/Permission/PermissionsAction.cshtml | 3 ++ .../Controllers/PermissionController.cs | 5 ++-- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/Tests/Grand.Web.Admin.Tests/Controllers/PermissionControllerTests.cs b/src/Tests/Grand.Web.Admin.Tests/Controllers/PermissionControllerTests.cs index 7bf4dff2c..eb4341880 100644 --- a/src/Tests/Grand.Web.Admin.Tests/Controllers/PermissionControllerTests.cs +++ b/src/Tests/Grand.Web.Admin.Tests/Controllers/PermissionControllerTests.cs @@ -99,6 +99,32 @@ public async Task PermissionsActionPost_DeniesTheActionsNotSelected() _permissionServiceMock.Verify(p => p.InsertPermissionAction(It.IsAny()), Times.Once); } + [TestMethod] + public async Task PermissionsActionPost_RemovesTheDenialsOfTheActionsSelected() + { + GroupExists(); + PermissionExists(); + var denied = new PermissionAction { Action = "Edit" }; + _permissionServiceMock.Setup(p => p.GetPermissionActions(SystemName, GroupId)) + .ReturnsAsync(new List { denied }); + + await _controller.PermissionsAction(new PermissionActionSaveModel(SystemName, GroupId, new List { "List", "Edit" })); + + _permissionServiceMock.Verify(p => p.DeletePermissionAction(denied), Times.Once); + _permissionServiceMock.Verify(p => p.InsertPermissionAction(It.IsAny()), Times.Never); + } + + [TestMethod] + public async Task PermissionsActionPost_NothingSelected_DeniesEveryAction() + { + GroupExists(); + PermissionExists(); + + await _controller.PermissionsAction(new PermissionActionSaveModel(SystemName, GroupId, null)); + + _permissionServiceMock.Verify(p => p.InsertPermissionAction(It.IsAny()), Times.Exactly(2)); + } + [TestMethod] public async Task PermissionsActionPost_UnknownGroup_IsNotFoundAndWritesNothing() { @@ -106,7 +132,7 @@ public async Task PermissionsActionPost_UnknownGroup_IsNotFoundAndWritesNothing( var result = await _controller.PermissionsAction(new PermissionActionSaveModel(SystemName, GroupId, null)); - Assert.IsInstanceOfType(result); + Assert.IsInstanceOfType(result); VerifyNothingWritten(); } @@ -119,7 +145,7 @@ public async Task PermissionsActionPost_UnknownPermission_IsNotFoundAndWritesNot var result = await _controller.PermissionsAction(new PermissionActionSaveModel(SystemName, GroupId, null)); - Assert.IsInstanceOfType(result); + Assert.IsInstanceOfType(result); VerifyNothingWritten(); } } diff --git a/src/Web/Grand.Web.Admin/Areas/Admin/Views/Permission/PermissionsAction.cshtml b/src/Web/Grand.Web.Admin/Areas/Admin/Views/Permission/PermissionsAction.cshtml index 5d58ea483..d18cd938f 100644 --- a/src/Web/Grand.Web.Admin/Areas/Admin/Views/Permission/PermissionsAction.cshtml +++ b/src/Web/Grand.Web.Admin/Areas/Admin/Views/Permission/PermissionsAction.cshtml @@ -36,6 +36,9 @@ data: form.serialize(), success: function (data) { mfp.close(); + }, + error: function (xhr, ajaxOptions, thrownError) { + alert(xhr.status === 404 ? xhr.responseText : thrownError); } }); }); diff --git a/src/Web/Grand.Web.Admin/Controllers/PermissionController.cs b/src/Web/Grand.Web.Admin/Controllers/PermissionController.cs index 975b60932..524d667ce 100644 --- a/src/Web/Grand.Web.Admin/Controllers/PermissionController.cs +++ b/src/Web/Grand.Web.Admin/Controllers/PermissionController.cs @@ -172,9 +172,10 @@ public async Task PermissionsAction(string systemName, string cus [HttpPost] public async Task PermissionsAction(PermissionActionSaveModel model) { + //a 404 without a body would be re-executed as a POST on the storefront var permissionRecord = await _permissionService.GetPermissionBySystemName(model.SystemName); - if (permissionRecord == null) return NotFound(); - if (await _groupService.GetCustomerGroupById(model.CustomerGroupId) == null) return NotFound(); + if (permissionRecord == null) return NotFound("Permission not found"); + if (await _groupService.GetCustomerGroupById(model.CustomerGroupId) == null) return NotFound("Customer group not found"); //remove denied actions var deniedActions = await _permissionService.GetPermissionActions(model.SystemName, model.CustomerGroupId);