Skip to content

Commit e3d24cd

Browse files
dcl10claude
dcl10
and
claude
authored
Debug/fix project visibility (#53)
* Fix project visibility bug — scope reads to creator and members only All authenticated users could previously read any project because the CanManageProjects policy only checked for a role claim, not membership. GET endpoints now use the default authenticated-user policy; ListAsync and GetAsync filter results to projects the caller created or has a TeamMembership on. Write endpoints retain the CanManageProjects role requirement. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Update project visibility tests to reflect membership-scoped reads Removed ManageProjectsRole from GET test clients (no longer required), replaced the forbidden-without-role assertion with an empty-list check, and added four new cases covering: member can see a project, non-member cannot see a project (for both List and Get endpoints). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 2876a17 commit e3d24cd

3 files changed

Lines changed: 166 additions & 19 deletions

File tree

‎backend/src/SkillMatrixLlm.Api/Controllers/ProjectsController.cs‎

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,20 @@ namespace SkillMatrixLlm.Api.Controllers;
1212
/// <summary>Manages projects and their lifecycle.</summary>
1313
[ApiController]
1414
[Route("api/[controller]")]
15-
[Authorize(nameof(AuthPolicies.CanManageProjects))]
15+
[Authorize]
1616
public class ProjectsController(ProjectService projects, AppUserService appUser, TeamService teams) : ControllerBase
1717
{
18-
/// <summary>Lists projects with an optional status filter.</summary>
18+
/// <summary>Lists projects visible to the caller (created by them or as a team member).</summary>
1919
/// <param name="status">Optional status filter.</param>
2020
/// <returns>Matching projects ordered by creation date descending.</returns>
2121
[HttpGet]
2222
[ProducesResponseType(typeof(List<Project>), StatusCodes.Status200OK)]
2323
public async Task<ActionResult<List<Project>>> List([FromQuery] ProjectStatus? status)
24-
=> Ok(await projects.ListAsync(status));
24+
{
25+
var callerId = await GetCallerAppUserIdAsync();
26+
if (callerId is null) return Ok(new List<Project>());
27+
return Ok(await projects.ListAsync(callerId.Value, status));
28+
}
2529

2630
/// <summary>Returns full project detail including teams and recommendations.</summary>
2731
/// <param name="id">Project ID.</param>
@@ -31,9 +35,11 @@ public async Task<ActionResult<List<Project>>> List([FromQuery] ProjectStatus? s
3135
[ProducesResponseType(StatusCodes.Status404NotFound)]
3236
public async Task<ActionResult<ProjectDetailDto>> Get(Guid id)
3337
{
38+
var callerId = await GetCallerAppUserIdAsync();
39+
if (callerId is null) return NotFound();
3440
try
3541
{
36-
return Ok(await projects.GetAsync(id));
42+
return Ok(await projects.GetAsync(id, callerId.Value));
3743
}
3844
catch (KeyNotFoundException ex)
3945
{
@@ -45,6 +51,7 @@ public async Task<ActionResult<ProjectDetailDto>> Get(Guid id)
4551
/// <param name="request">Project details.</param>
4652
/// <returns>The created project.</returns>
4753
[HttpPost]
54+
[Authorize(nameof(AuthPolicies.CanManageProjects))]
4855
[ProducesResponseType(typeof(Project), StatusCodes.Status201Created)]
4956
[ProducesResponseType(StatusCodes.Status400BadRequest)]
5057
[ProducesResponseType(StatusCodes.Status404NotFound)]
@@ -73,6 +80,7 @@ public async Task<ActionResult<Project>> Create(CreateProjectRequest request)
7380
/// <param name="request">Updated project details.</param>
7481
/// <returns>The updated project.</returns>
7582
[HttpPut("{id:guid}")]
83+
[Authorize(nameof(AuthPolicies.CanManageProjects))]
7684
[ProducesResponseType(typeof(Project), StatusCodes.Status200OK)]
7785
[ProducesResponseType(StatusCodes.Status404NotFound)]
7886
[ProducesResponseType(StatusCodes.Status409Conflict)]
@@ -97,6 +105,7 @@ public async Task<ActionResult<Project>> Update(Guid id, UpdateProjectRequest re
97105
/// <param name="request">Target status.</param>
98106
/// <returns>The updated project.</returns>
99107
[HttpPut("{id:guid}/status")]
108+
[Authorize(nameof(AuthPolicies.CanManageProjects))]
100109
[ProducesResponseType(typeof(Project), StatusCodes.Status200OK)]
101110
[ProducesResponseType(StatusCodes.Status404NotFound)]
102111
[ProducesResponseType(StatusCodes.Status409Conflict)]
@@ -120,6 +129,7 @@ public async Task<ActionResult<Project>> TransitionStatus(Guid id, TransitionSta
120129
/// <param name="id">Project ID.</param>
121130
/// <returns>No content on success.</returns>
122131
[HttpDelete("{id:guid}")]
132+
[Authorize(nameof(AuthPolicies.CanManageProjects))]
123133
[ProducesResponseType(StatusCodes.Status204NoContent)]
124134
[ProducesResponseType(StatusCodes.Status404NotFound)]
125135
[ProducesResponseType(StatusCodes.Status409Conflict)]
@@ -149,6 +159,7 @@ public async Task<ActionResult> Close(Guid id)
149159
/// <param name="request">Team source.</param>
150160
/// <returns>The created team.</returns>
151161
[HttpPost("{projectId:guid}/teams")]
162+
[Authorize(nameof(AuthPolicies.CanManageProjects))]
152163
[ProducesResponseType(typeof(TeamDto), StatusCodes.Status201Created)]
153164
[ProducesResponseType(StatusCodes.Status404NotFound)]
154165
public async Task<ActionResult<TeamDto>> CreateTeam(Guid projectId, CreateTeamRequest request)
@@ -170,6 +181,7 @@ public async Task<ActionResult<TeamDto>> CreateTeam(Guid projectId, CreateTeamRe
170181
/// <param name="request">User and role details.</param>
171182
/// <returns>The created membership record.</returns>
172183
[HttpPost("{projectId:guid}/teams/{teamId:guid}/members")]
184+
[Authorize(nameof(AuthPolicies.CanManageProjects))]
173185
[ProducesResponseType(typeof(TeamMembership), StatusCodes.Status201Created)]
174186
[ProducesResponseType(StatusCodes.Status404NotFound)]
175187
[ProducesResponseType(StatusCodes.Status409Conflict)]
@@ -196,6 +208,7 @@ public async Task<ActionResult<TeamMembership>> AddTeamMember(Guid projectId, Gu
196208
/// <param name="userId">Application user ID of the member to remove.</param>
197209
/// <returns>No content on success.</returns>
198210
[HttpDelete("{projectId:guid}/teams/{teamId:guid}/members/{userId:guid}")]
211+
[Authorize(nameof(AuthPolicies.CanManageProjects))]
199212
[ProducesResponseType(StatusCodes.Status204NoContent)]
200213
[ProducesResponseType(StatusCodes.Status404NotFound)]
201214
public async Task<ActionResult> RemoveTeamMember(Guid projectId, Guid teamId, Guid userId)
@@ -216,6 +229,7 @@ public async Task<ActionResult> RemoveTeamMember(Guid projectId, Guid teamId, Gu
216229
/// <param name="teamId">Team ID.</param>
217230
/// <returns>The confirmed team.</returns>
218231
[HttpPut("{projectId:guid}/teams/{teamId:guid}/confirm")]
232+
[Authorize(nameof(AuthPolicies.CanManageProjects))]
219233
[ProducesResponseType(typeof(TeamDto), StatusCodes.Status200OK)]
220234
[ProducesResponseType(StatusCodes.Status404NotFound)]
221235
[ProducesResponseType(StatusCodes.Status409Conflict)]
@@ -240,6 +254,7 @@ public async Task<ActionResult<TeamDto>> ConfirmTeam(Guid projectId, Guid teamId
240254
/// <param name="teamId">Team ID.</param>
241255
/// <returns>The rejected team.</returns>
242256
[HttpPut("{projectId:guid}/teams/{teamId:guid}/reject")]
257+
[Authorize(nameof(AuthPolicies.CanManageProjects))]
243258
[ProducesResponseType(typeof(TeamDto), StatusCodes.Status200OK)]
244259
[ProducesResponseType(StatusCodes.Status404NotFound)]
245260
[ProducesResponseType(StatusCodes.Status409Conflict)]
@@ -259,4 +274,19 @@ public async Task<ActionResult<TeamDto>> RejectTeam(Guid projectId, Guid teamId)
259274
}
260275
}
261276

277+
private async Task<Guid?> GetCallerAppUserIdAsync()
278+
{
279+
var keycloakId = User.FindFirst("sub")?.Value;
280+
if (string.IsNullOrEmpty(keycloakId)) return null;
281+
try
282+
{
283+
var caller = await appUser.GetProfileByKeycloakId(keycloakId);
284+
return caller.Id;
285+
}
286+
catch (KeyNotFoundException)
287+
{
288+
return null;
289+
}
290+
}
291+
262292
}

‎backend/src/SkillMatrixLlm.Api/Services/ProjectService.cs‎

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,18 @@ public async Task<Project> TransitionStatusAsync(Guid projectId, ProjectStatus n
101101
}
102102

103103
/// <summary>
104-
/// Lists projects with an optional status filter, ordered by creation date descending.
104+
/// Lists projects visible to the caller: projects they created or are a team member of.
105105
/// </summary>
106+
/// <param name="callerAppUserId">Application user ID of the requesting user.</param>
106107
/// <param name="status">Optional status filter.</param>
107-
/// <returns>Matching projects.</returns>
108-
public async Task<List<Project>> ListAsync(ProjectStatus? status)
108+
/// <returns>Matching projects ordered by creation date descending.</returns>
109+
public async Task<List<Project>> ListAsync(Guid callerAppUserId, ProjectStatus? status)
109110
{
110-
var query = db.Projects.Include(p => p.CreatedByUser).AsQueryable();
111+
var query = db.Projects
112+
.Include(p => p.CreatedByUser)
113+
.Where(p => p.CreatedByUserId == callerAppUserId
114+
|| db.TeamMemberships.Any(tm => tm.UserId == callerAppUserId && tm.Team!.ProjectId == p.Id))
115+
.AsQueryable();
111116

112117
if (status.HasValue)
113118
{
@@ -122,15 +127,19 @@ public async Task<List<Project>> ListAsync(ProjectStatus? status)
122127

123128
/// <summary>
124129
/// Returns full project detail including associated teams (with memberships) and recommendations.
130+
/// Only accessible to the project creator or a team member.
125131
/// </summary>
126132
/// <param name="projectId">Project ID.</param>
133+
/// <param name="callerAppUserId">Application user ID of the requesting user.</param>
127134
/// <returns>Project detail.</returns>
128-
/// <exception cref="KeyNotFoundException">Thrown when the project does not exist.</exception>
129-
public async Task<ProjectDetailDto> GetAsync(Guid projectId)
135+
/// <exception cref="KeyNotFoundException">Thrown when the project does not exist or the caller has no access.</exception>
136+
public async Task<ProjectDetailDto> GetAsync(Guid projectId, Guid callerAppUserId)
130137
{
131138
var project = await db.Projects
132139
.Include(p => p.CreatedByUser)
133-
.FirstOrDefaultAsync(p => p.Id == projectId)
140+
.FirstOrDefaultAsync(p => p.Id == projectId
141+
&& (p.CreatedByUserId == callerAppUserId
142+
|| db.TeamMemberships.Any(tm => tm.UserId == callerAppUserId && tm.Team!.ProjectId == p.Id)))
134143
?? throw new KeyNotFoundException($"Project {projectId} not found.");
135144

136145
var teams = await db.Teams

‎backend/tests/SkillMatrixLlm.Api.IntegrationTests/ProjectsControllerTests.cs‎

Lines changed: 116 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ namespace SkillMatrixLlm.Api.Tests;
22

33
using System.Net;
44
using System.Net.Http.Json;
5+
using System.Security.Claims;
56
using System.Text.Json;
67
using System.Text.Json.Serialization;
78
using Constants;
@@ -13,6 +14,8 @@ namespace SkillMatrixLlm.Api.Tests;
1314
using Models.Projects;
1415
using Xunit;
1516
using ProjectEntity = SkillMatrixLlm.Api.Data.Entities.Project;
17+
using TeamEntity = SkillMatrixLlm.Api.Data.Entities.Team;
18+
using TeamMembershipEntity = SkillMatrixLlm.Api.Data.Entities.TeamMembership;
1619
using UserEntity = SkillMatrixLlm.Api.Data.Entities.User;
1720

1821
public class ProjectsControllerTests(ApiFactory factory) : IClassFixture<ApiFactory>, IAsyncLifetime
@@ -71,20 +74,42 @@ private static ProjectEntity SeedProject(AppDbContext db, UserEntity user, Proje
7174
return project;
7275
}
7376

77+
private static void SeedMembership(AppDbContext db, ProjectEntity project, UserEntity member)
78+
{
79+
var team = new TeamEntity
80+
{
81+
ProjectId = project.Id,
82+
Source = ProjectSource.ManuallyAssembled,
83+
Status = TeamStatus.Proposed,
84+
CreatedAt = DateTime.UtcNow,
85+
};
86+
db.Teams.Add(team);
87+
db.SaveChanges();
88+
89+
db.TeamMemberships.Add(new TeamMembershipEntity
90+
{
91+
TeamId = team.Id,
92+
UserId = member.Id,
93+
ProjectRole = "Developer",
94+
MembershipStatus = MembershipStatus.Invited,
95+
});
96+
db.SaveChanges();
97+
}
98+
7499
// -------------------------------------------------------------------------
75100
// GET /api/projects
76101
// -------------------------------------------------------------------------
77102

78103
[Fact]
79-
public async Task List_ReturnsAllProjects_WhenNoFilter()
104+
public async Task List_ReturnsOwnProjects_WhenNoFilter()
80105
{
81106
using var scope = factory.Services.CreateScope();
82107
var db = scope.ServiceProvider.GetRequiredService<AppDbContext>();
83108
var user = SeedUser(db);
84109
SeedProject(db, user, ProjectStatus.Draft);
85110
SeedProject(db, user, ProjectStatus.Open);
86111

87-
var client = factory.CreateAuthenticatedClient([TestClaims.Sub, TestClaims.ManageProjectsRole]);
112+
var client = factory.CreateAuthenticatedClient([TestClaims.Sub]);
88113

89114
var response = await client.GetAsync("/api/projects");
90115

@@ -103,7 +128,7 @@ public async Task List_FiltersByStatus()
103128
SeedProject(db, user, ProjectStatus.Draft);
104129
SeedProject(db, user, ProjectStatus.Open);
105130

106-
var client = factory.CreateAuthenticatedClient([TestClaims.Sub, TestClaims.ManageProjectsRole]);
131+
var client = factory.CreateAuthenticatedClient([TestClaims.Sub]);
107132

108133
var response = await client.GetAsync("/api/projects?status=Draft");
109134

@@ -115,26 +140,69 @@ public async Task List_FiltersByStatus()
115140
}
116141

117142
[Fact]
118-
public async Task List_ReturnsForbidden_WithoutManageProjectsRole()
143+
public async Task List_ReturnsEmpty_WhenCallerHasNoProjects()
119144
{
120145
var response = await factory.CreateAuthenticatedClient([TestClaims.Sub]).GetAsync("/api/projects");
121146

122-
Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode);
147+
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
148+
var list = await response.Content.ReadFromJsonAsync<List<Project>>(JsonOptions);
149+
Assert.NotNull(list);
150+
Assert.Empty(list);
151+
}
152+
153+
[Fact]
154+
public async Task List_IncludesMemberProject_WhenCallerIsTeamMember()
155+
{
156+
using var scope = factory.Services.CreateScope();
157+
var db = scope.ServiceProvider.GetRequiredService<AppDbContext>();
158+
var pm = SeedUser(db, "pm-keycloak-id");
159+
var member = SeedUser(db, "member-keycloak-id");
160+
var project = SeedProject(db, pm);
161+
SeedMembership(db, project, member);
162+
163+
var client = factory.CreateAuthenticatedClient([new Claim("sub", "member-keycloak-id")]);
164+
165+
var response = await client.GetAsync("/api/projects");
166+
167+
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
168+
var list = await response.Content.ReadFromJsonAsync<List<Project>>(JsonOptions);
169+
Assert.NotNull(list);
170+
Assert.Single(list);
171+
Assert.Equal(project.Id, list[0].Id);
172+
}
173+
174+
[Fact]
175+
public async Task List_ExcludesProject_WhenCallerIsNotCreatorOrMember()
176+
{
177+
using var scope = factory.Services.CreateScope();
178+
var db = scope.ServiceProvider.GetRequiredService<AppDbContext>();
179+
var pm = SeedUser(db, "pm-keycloak-id");
180+
SeedProject(db, pm);
181+
SeedUser(db, "other-keycloak-id");
182+
183+
var client = factory.CreateAuthenticatedClient([new Claim("sub", "other-keycloak-id")]);
184+
185+
var response = await client.GetAsync("/api/projects");
186+
187+
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
188+
var list = await response.Content.ReadFromJsonAsync<List<Project>>(JsonOptions);
189+
Assert.NotNull(list);
190+
Assert.Empty(list);
123191
}
124192

125193
// -------------------------------------------------------------------------
126194
// GET /api/projects/{id}
127195
// -------------------------------------------------------------------------
128196

129197
[Fact]
130-
public async Task Get_ReturnsProjectDetail_WhenFound()
198+
public async Task Get_ReturnsProjectDetail_WhenCallerIsCreator()
131199
{
132200
using var scope = factory.Services.CreateScope();
133201
var db = scope.ServiceProvider.GetRequiredService<AppDbContext>();
134202
var user = SeedUser(db);
135203
var project = SeedProject(db, user);
136204

137-
var client = factory.CreateAuthenticatedClient([TestClaims.Sub, TestClaims.ManageProjectsRole]);
205+
var client = factory.CreateAuthenticatedClient([TestClaims.Sub]);
138206

139207
var response = await client.GetAsync($"/api/projects/{project.Id}");
140208

@@ -146,10 +214,50 @@ public async Task Get_ReturnsProjectDetail_WhenFound()
146214
Assert.Empty(detail.Recommendations);
147215
}
148216

217+
[Fact]
218+
public async Task Get_ReturnsProjectDetail_WhenCallerIsTeamMember()
219+
{
220+
using var scope = factory.Services.CreateScope();
221+
var db = scope.ServiceProvider.GetRequiredService<AppDbContext>();
222+
var pm = SeedUser(db, "pm-keycloak-id");
223+
var member = SeedUser(db, "member-keycloak-id");
224+
var project = SeedProject(db, pm);
225+
SeedMembership(db, project, member);
226+
227+
var client = factory.CreateAuthenticatedClient([new Claim("sub", "member-keycloak-id")]);
228+
229+
var response = await client.GetAsync($"/api/projects/{project.Id}");
230+
231+
Assert.Equal(HttpStatusCode.OK, response.StatusCode);
232+
var detail = await response.Content.ReadFromJsonAsync<ProjectDetailDto>(JsonOptions);
233+
Assert.NotNull(detail);
234+
Assert.Equal(project.Id, detail.Id);
235+
}
236+
237+
[Fact]
238+
public async Task Get_ReturnsNotFound_WhenCallerIsNotCreatorOrMember()
239+
{
240+
using var scope = factory.Services.CreateScope();
241+
var db = scope.ServiceProvider.GetRequiredService<AppDbContext>();
242+
var pm = SeedUser(db, "pm-keycloak-id");
243+
var project = SeedProject(db, pm);
244+
SeedUser(db, "other-keycloak-id");
245+
246+
var client = factory.CreateAuthenticatedClient([new Claim("sub", "other-keycloak-id")]);
247+
248+
var response = await client.GetAsync($"/api/projects/{project.Id}");
249+
250+
Assert.Equal(HttpStatusCode.NotFound, response.StatusCode);
251+
}
252+
149253
[Fact]
150254
public async Task Get_ReturnsNotFound_WhenProjectDoesNotExist()
151255
{
152-
var client = factory.CreateAuthenticatedClient([TestClaims.Sub, TestClaims.ManageProjectsRole]);
256+
using var scope = factory.Services.CreateScope();
257+
var db = scope.ServiceProvider.GetRequiredService<AppDbContext>();
258+
SeedUser(db);
259+
260+
var client = factory.CreateAuthenticatedClient([TestClaims.Sub]);
153261

154262
var response = await client.GetAsync($"/api/projects/{Guid.NewGuid()}");
155263

0 commit comments

Comments
 (0)