Skip to content

Commit 3e1bf39

Browse files
committed
fix(projects): restrict delete to admin or project-manager owner
1 parent d256194 commit 3e1bf39

3 files changed

Lines changed: 19 additions & 12 deletions

File tree

src/TaskManagement.Api/Features/Projects/Commands/Handlers/DeleteProjectCommandHandler.cs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,10 @@ public async Task Handle(DeleteProjectCommand request, CancellationToken cancell
4242
var isAdmin = _currentUserService.IsInRole(Roles.Administrator);
4343
var isProjectManager = _currentUserService.IsInRole(Roles.ProjectManager);
4444

45-
if (!isAdmin && !isProjectManager && project.OwnerUserId != currentUserId)
45+
var canDeleteAsProjectManagerOwner =
46+
isProjectManager && string.Equals(project.OwnerUserId, currentUserId, StringComparison.Ordinal);
47+
48+
if (!isAdmin && !canDeleteAsProjectManagerOwner)
4649
{
4750
throw new ForbiddenAccessException("User is not authorized to delete this project.");
4851
}

tests/TaskManagement.Api.Tests/IntegrationTests/Features/Projects/DeleteProjectEndpointTests.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,10 +76,10 @@ private void SetAuthenticatedUser(string userId, string? roles = null)
7676
}
7777

7878
[Fact]
79-
public async Task DeleteProject_WhenUserIsOwner_ShouldReturnNoContentAndDeleteProject()
79+
public async Task DeleteProject_WhenUserIsProjectManagerOwner_ShouldReturnNoContentAndDeleteProject()
8080
{
8181
// Arrange
82-
SetAuthenticatedUser(_ownerUserId);
82+
SetAuthenticatedUser(_ownerUserId, Roles.ProjectManager);
8383
int initialProjectCount;
8484
using (var scope = _factory.Services.CreateScope())
8585
{
@@ -107,7 +107,7 @@ public async Task DeleteProject_WhenUserIsOwner_ShouldReturnNoContentAndDeletePr
107107
}
108108

109109
[Fact]
110-
public async Task DeleteProject_WhenUserIsProjectManagerButNotOwner_ShouldReturnNoContent()
110+
public async Task DeleteProject_WhenUserIsProjectManagerButNotOwner_ShouldReturnForbidden()
111111
{
112112
// Arrange
113113
SetAuthenticatedUser(_otherUserId, Roles.ProjectManager);
@@ -122,15 +122,15 @@ public async Task DeleteProject_WhenUserIsProjectManagerButNotOwner_ShouldReturn
122122
var response = await _client.DeleteAsync($"/api/projects/{_projectToDeleteId}");
123123

124124
// Assert
125-
response.StatusCode.Should().Be(HttpStatusCode.NoContent);
125+
response.StatusCode.Should().Be(HttpStatusCode.Forbidden);
126126

127127
// Assert
128128
using (var scope = _factory.Services.CreateScope())
129129
{
130130
var dbContext = scope.ServiceProvider.GetRequiredService<TaskManagementDbContext>();
131131
var projectInDb = await dbContext.Projects.FindAsync(_projectToDeleteId);
132-
projectInDb.Should().BeNull();
133-
(await dbContext.Projects.CountAsync()).Should().Be(initialProjectCount - 1);
132+
projectInDb.Should().NotBeNull();
133+
(await dbContext.Projects.CountAsync()).Should().Be(initialProjectCount);
134134
}
135135
}
136136

tests/TaskManagement.Api.Tests/UnitTests/Features/Projects/Commands/DeleteProjectCommandHandlerTests.cs

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,12 @@ private void SeedDatabase()
6262
}
6363

6464
[Fact]
65-
public async Task Handle_ShouldRemoveProject_WhenRequestIsValidAndUserIsOwner()
65+
public async Task Handle_ShouldRemoveProject_WhenRequestIsValidAndUserIsProjectManagerOwner()
6666
{
6767
// Arrange
6868
var command = new DeleteProjectCommand { Id = _projectIdToDelete };
6969
_mockCurrentUser.Setup(u => u.Id).Returns(_ownerUserId);
70+
_mockCurrentUser.Setup(u => u.IsInRole(Roles.ProjectManager)).Returns(true);
7071
int initialCount = await _dbContext.Projects.CountAsync();
7172

7273
// Act
@@ -114,19 +115,22 @@ public async Task Handle_ShouldThrowForbiddenAccessException_WhenUserIsNotOwner(
114115
}
115116

116117
[Fact]
117-
public async Task Handle_ShouldRemoveProject_WhenUserIsProjectManager()
118+
public async Task Handle_ShouldThrowForbiddenAccessException_WhenUserIsProjectManagerButNotOwner()
118119
{
119120
// Arrange
120121
var command = new DeleteProjectCommand { Id = _projectIdToDelete };
121122
_mockCurrentUser.Setup(u => u.Id).Returns(_otherUserId);
122123
_mockCurrentUser.Setup(u => u.IsInRole(Roles.ProjectManager)).Returns(true);
123124

124125
// Act
125-
await _handler.Handle(command, CancellationToken.None);
126+
Func<Task> act = async () => await _handler.Handle(command, CancellationToken.None);
126127

127128
// Assert
128-
var deletedProject = await _dbContext.Projects.FindAsync(_projectIdToDelete);
129-
deletedProject.Should().BeNull();
129+
await act.Should().ThrowAsync<ForbiddenAccessException>()
130+
.WithMessage("*not authorized to delete this project*");
131+
132+
var existingProject = await _dbContext.Projects.FindAsync(_projectIdToDelete);
133+
existingProject.Should().NotBeNull();
130134
}
131135

132136
[Fact]

0 commit comments

Comments
 (0)