From 446e43b2c60669f271166d570fdc422e230164ac Mon Sep 17 00:00:00 2001 From: Arthur Bassi Date: Tue, 11 Aug 2026 09:44:52 -0300 Subject: [PATCH] !fix(users): restrict DeleteUser to Admin [SH-221] Align DeleteUser with AddUser/EditUser: Admin role at controller and service entry, Forbidden for non-Admin, and regression coverage. Co-authored-by: Cursor --- .../UserControllerTests.cs | 39 ++++++++++++++++--- .../UserServiceTests.cs | 27 ++++++++++--- .../Controllers/UserController.cs | 7 +++- .../Implementation/UserService.cs | 13 +++++-- SeaHaven.Services/Interfaces/IUserService.cs | 2 +- 5 files changed, 71 insertions(+), 17 deletions(-) diff --git a/Api.SeaHavenIndustries.Tests/UserControllerTests.cs b/Api.SeaHavenIndustries.Tests/UserControllerTests.cs index 777b12b..c6b06b6 100644 --- a/Api.SeaHavenIndustries.Tests/UserControllerTests.cs +++ b/Api.SeaHavenIndustries.Tests/UserControllerTests.cs @@ -183,10 +183,13 @@ public class UserControllerTests public async Task DeleteUser_NotFound_ReturnsIdNotMatchedMessage() { var service = new Mock(); - service.Setup(s => s.DeleteUserAsync(It.IsAny(), It.IsAny())) - .ReturnsAsync(false); + service.Setup(s => s.DeleteUserAsync( + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(new AddUserOutcomeDTO { Success = false, Error = UserMutationErrors.UserNotFound }); - var controller = NewController(service); + var controller = NewController(service, "admin-1", "Admin"); var result = await controller.DeleteUser(new EditUser_DTO { Id = "missing" }, CancellationToken.None); @@ -199,14 +202,38 @@ public class UserControllerTests public async Task DeleteUser_Found_DelegatesAndReturnsOk() { var service = new Mock(); - service.Setup(s => s.DeleteUserAsync("5", It.IsAny())).ReturnsAsync(true); + service.Setup(s => s.DeleteUserAsync( + "5", + It.IsAny(), + It.IsAny())) + .ReturnsAsync(new AddUserOutcomeDTO { Success = true }); - var controller = NewController(service); + var controller = NewController(service, "admin-1", "Admin"); var result = await controller.DeleteUser(new EditUser_DTO { Id = "5" }, CancellationToken.None); result.Should().BeOfType(); - service.Verify(s => s.DeleteUserAsync("5", It.IsAny()), Times.Once); + service.Verify(s => s.DeleteUserAsync( + "5", + It.IsAny(), + It.IsAny()), Times.Once); + } + + [Fact] + public async Task DeleteUser_Forbidden_ReturnsForbid() + { + var service = new Mock(); + service.Setup(s => s.DeleteUserAsync( + It.IsAny(), + It.IsAny(), + It.IsAny())) + .ReturnsAsync(new AddUserOutcomeDTO { Success = false, Error = UserMutationErrors.Forbidden }); + + var controller = NewController(service, "tech-1", "User"); + + var result = await controller.DeleteUser(new EditUser_DTO { Id = "5" }, CancellationToken.None); + + result.Should().BeOfType(); } [Fact] diff --git a/Api.SeaHavenIndustries.Tests/UserServiceTests.cs b/Api.SeaHavenIndustries.Tests/UserServiceTests.cs index 2e88fd1..d2364bb 100644 --- a/Api.SeaHavenIndustries.Tests/UserServiceTests.cs +++ b/Api.SeaHavenIndustries.Tests/UserServiceTests.cs @@ -61,16 +61,17 @@ public class UserServiceTests } [Fact] - public async Task DeleteUser_NotFound_ReturnsFalseWithoutCascade() + public async Task DeleteUser_NotFound_ReturnsUserNotFoundWithoutCascade() { var userData = new Mock(); userData.Setup(u => u.GetForEditAsync("missing", It.IsAny())).ReturnsAsync((ApplicationUser?)null); var service = NewService(userData, new Mock(), out _); - var found = await service.DeleteUserAsync("missing", CancellationToken.None); + var outcome = await service.DeleteUserAsync("missing", Principal("Admin"), CancellationToken.None); - found.Should().BeFalse(); + outcome.Success.Should().BeFalse(); + outcome.Error.Should().Be(UserMutationErrors.UserNotFound); userData.Verify(u => u.DeleteUserWithCascadeAsync(It.IsAny(), It.IsAny()), Times.Never); } @@ -83,12 +84,28 @@ public class UserServiceTests var service = NewService(userData, new Mock(), out _); - var found = await service.DeleteUserAsync("u1", CancellationToken.None); + var outcome = await service.DeleteUserAsync("u1", Principal("Admin"), CancellationToken.None); - found.Should().BeTrue(); + outcome.Success.Should().BeTrue(); userData.Verify(u => u.DeleteUserWithCascadeAsync(user, It.IsAny()), Times.Once); } + [Fact] + public async Task DeleteUser_NonAdmin_ReturnsForbiddenWithoutCascade() + { + var user = IdentityTestHelpers.User(); + var userData = new Mock(); + userData.Setup(u => u.GetForEditAsync("u1", It.IsAny())).ReturnsAsync(user); + + var service = NewService(userData, new Mock(), out _); + + var outcome = await service.DeleteUserAsync("u1", Principal("User"), CancellationToken.None); + + outcome.Success.Should().BeFalse(); + outcome.Error.Should().Be(UserMutationErrors.Forbidden); + userData.Verify(u => u.DeleteUserWithCascadeAsync(It.IsAny(), It.IsAny()), Times.Never); + } + [Fact] public async Task DeleteCurrentUser_SetsIsDeletedFlag() { diff --git a/Api.SeaHavenIndustries/Controllers/UserController.cs b/Api.SeaHavenIndustries/Controllers/UserController.cs index e92c7cc..6ad653f 100644 --- a/Api.SeaHavenIndustries/Controllers/UserController.cs +++ b/Api.SeaHavenIndustries/Controllers/UserController.cs @@ -94,14 +94,17 @@ namespace Api.SeaHavenIndustries.Controllers [HttpDelete] [Route("DeleteUser")] + [Authorize(Roles = "Admin")] public async Task DeleteUser(EditUser_DTO req, CancellationToken cancellationToken) { string id = req.Id ?? ""; try { - var found = await _userService.DeleteUserAsync(id, cancellationToken); - if (!found) + var outcome = await _userService.DeleteUserAsync(id, User, cancellationToken); + if (!outcome.Success) { + if (string.Equals(outcome.Error, UserMutationErrors.Forbidden, StringComparison.Ordinal)) + return Forbid(); return BadRequest(new Response { Status = "Error", Message = "ID not matched!" }); } return Ok(new DataResponse { Message = "Updated Successfully", Status = "200" }); diff --git a/SeaHaven.Services/Implementation/UserService.cs b/SeaHaven.Services/Implementation/UserService.cs index 2d1f1ba..01b19e9 100644 --- a/SeaHaven.Services/Implementation/UserService.cs +++ b/SeaHaven.Services/Implementation/UserService.cs @@ -161,14 +161,21 @@ namespace SeaHaven.Services.Implementation return new AddUserOutcomeDTO { Success = true }; } - public async Task DeleteUserAsync(string id, CancellationToken cancellationToken) + public async Task DeleteUserAsync( + string id, + ClaimsPrincipal caller, + CancellationToken cancellationToken) { + var authFailure = EnsureAdminCaller(caller); + if (authFailure != null) + return authFailure; + var data = await _userDataService.GetForEditAsync(id, cancellationToken); if (data == null) - return false; + return new AddUserOutcomeDTO { Success = false, Error = UserMutationErrors.UserNotFound }; await _userDataService.DeleteUserWithCascadeAsync(data, cancellationToken); - return true; + return new AddUserOutcomeDTO { Success = true }; } public async Task DeleteCurrentUserAsync(string userId, CancellationToken cancellationToken) diff --git a/SeaHaven.Services/Interfaces/IUserService.cs b/SeaHaven.Services/Interfaces/IUserService.cs index a4a7a7c..56e9214 100644 --- a/SeaHaven.Services/Interfaces/IUserService.cs +++ b/SeaHaven.Services/Interfaces/IUserService.cs @@ -8,7 +8,7 @@ namespace SeaHaven.Services.Interfaces Task> GetUsersAsync(CancellationToken cancellationToken); Task AddUserAsync(AddUserRequestDTO dto, ClaimsPrincipal caller, CancellationToken cancellationToken); Task EditUserAsync(EditUserRequestDTO dto, ClaimsPrincipal caller, CancellationToken cancellationToken); - Task DeleteUserAsync(string id, CancellationToken cancellationToken); + Task DeleteUserAsync(string id, ClaimsPrincipal caller, CancellationToken cancellationToken); Task DeleteCurrentUserAsync(string userId, CancellationToken cancellationToken); Task> GetUserProfileAsync(string userId, CancellationToken cancellationToken); }