From 3eea204081205f7d101cde55e88269a02bce0ab5 Mon Sep 17 00:00:00 2001 From: Edi Asllani Date: Wed, 10 Jun 2026 13:02:13 +0200 Subject: [PATCH] fix(security): enforce project-member visibility on standups/commits + lock down /metrics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FR-4.7 says project data is invited-only, enforced backend, but three project-scoped GET handlers only required authentication and filtered by ProjectId — letting any signed-in user read another project's private standups, standup CSV export, and commit history by guessing a project id: - GetStandupFeedQueryHandler - ExportStandupsCsvQueryHandler (previously only checked the project exists) - GetProjectCommitsQueryHandler Apply the same owner-or-accepted-member guard the sibling handlers (GetProjectDashboard/GetProjectSummaries) already use, throwing ForbiddenException (-> 403) for non-members. Document 403/404 on the endpoints. Adds ProjectVisibilityGuardTests covering outsider/owner/member for all three (6 tests). Also closes public exposure of the Prometheus /metrics endpoint. The prod nginx ingress has snippet directives disabled cluster-wide, so block by Host in-app: MapMetrics("/metrics").RequireHost(Metrics:AllowedHosts). In-cluster Prometheus scrapes the pod directly as 'loopless-backend:8080' (allow-listed) while the public api host is not, returning 404 to the internet without breaking scraping. Wired via Helm values config.backend.Metrics__AllowedHosts__0. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Endpoints/ProjectEndpoints.cs | 1 + .../Endpoints/StandupEndpoints.cs | 3 + backend/src/Loopless.Api/Program.cs | 14 +- .../GetProjectCommitsQueryHandler.cs | 28 +++- .../ExportStandupsCsvQueryHandler.cs | 27 +++- .../GetFeed/GetStandupFeedQueryHandler.cs | 29 +++- .../Visibility/ProjectVisibilityGuardTests.cs | 146 ++++++++++++++++++ .../ProjectVisibilityTestDbContext.cs | 93 +++++++++++ devops/helm/loopless/values.yaml | 5 + 9 files changed, 335 insertions(+), 11 deletions(-) create mode 100644 backend/tests/Loopless.UnitTests/Visibility/ProjectVisibilityGuardTests.cs create mode 100644 backend/tests/Loopless.UnitTests/Visibility/ProjectVisibilityTestDbContext.cs diff --git a/backend/src/Loopless.Api/Endpoints/ProjectEndpoints.cs b/backend/src/Loopless.Api/Endpoints/ProjectEndpoints.cs index f605532..cd9fa4a 100644 --- a/backend/src/Loopless.Api/Endpoints/ProjectEndpoints.cs +++ b/backend/src/Loopless.Api/Endpoints/ProjectEndpoints.cs @@ -186,6 +186,7 @@ public static IEndpointRouteBuilder MapProjectEndpoints(this IEndpointRouteBuild .WithName("GetProjectCommits") .Produces>(StatusCodes.Status200OK) .Produces(StatusCodes.Status401Unauthorized) + .Produces(StatusCodes.Status403Forbidden) .Produces(StatusCodes.Status404NotFound) .ProducesValidationProblem(); diff --git a/backend/src/Loopless.Api/Endpoints/StandupEndpoints.cs b/backend/src/Loopless.Api/Endpoints/StandupEndpoints.cs index cefe22e..ec85147 100644 --- a/backend/src/Loopless.Api/Endpoints/StandupEndpoints.cs +++ b/backend/src/Loopless.Api/Endpoints/StandupEndpoints.cs @@ -52,6 +52,8 @@ public static IEndpointRouteBuilder MapStandupEndpoints(this IEndpointRouteBuild .WithName("GetStandupFeed") .Produces>(StatusCodes.Status200OK) .Produces(StatusCodes.Status401Unauthorized) + .Produces(StatusCodes.Status403Forbidden) + .Produces(StatusCodes.Status404NotFound) .ProducesValidationProblem(); group.MapGet("/analytics", async ( @@ -84,6 +86,7 @@ public static IEndpointRouteBuilder MapStandupEndpoints(this IEndpointRouteBuild .WithName("ExportStandupsCsv") .Produces(StatusCodes.Status200OK, contentType: "text/csv") .Produces(StatusCodes.Status401Unauthorized) + .Produces(StatusCodes.Status403Forbidden) .Produces(StatusCodes.Status404NotFound); return app; diff --git a/backend/src/Loopless.Api/Program.cs b/backend/src/Loopless.Api/Program.cs index 0ef4a11..a1e858e 100644 --- a/backend/src/Loopless.Api/Program.cs +++ b/backend/src/Loopless.Api/Program.cs @@ -227,7 +227,19 @@ app.MapGet("/", () => Results.Redirect("/swagger")).ExcludeFromDescription(); } - app.MapMetrics("/metrics"); + // Prometheus scrape endpoint. Snippet-based path blocking is disabled on the prod + // ingress, so restrict /metrics by Host instead: the in-cluster scrape reaches the + // pod directly as "loopless-backend:8080" (allowed), while public traffic arrives as + // the ingress host (api.) which is not in the allowlist and gets 404 — keeping + // metrics off the public internet without breaking scraping. Unset (dev/local) = open. + var metricsEndpoint = app.MapMetrics("/metrics"); + var metricsAllowedHosts = app.Configuration + .GetSection("Metrics:AllowedHosts").Get(); + if (metricsAllowedHosts is { Length: > 0 }) + { + metricsEndpoint.RequireHost(metricsAllowedHosts); + } + app.MapHealthChecks("/health"); app.MapHealthChecks("/health/ready", new HealthCheckOptions { diff --git a/backend/src/Loopless.Application/Features/Projects/GetCommits/GetProjectCommitsQueryHandler.cs b/backend/src/Loopless.Application/Features/Projects/GetCommits/GetProjectCommitsQueryHandler.cs index db811e8..0e0cf12 100644 --- a/backend/src/Loopless.Application/Features/Projects/GetCommits/GetProjectCommitsQueryHandler.cs +++ b/backend/src/Loopless.Application/Features/Projects/GetCommits/GetProjectCommitsQueryHandler.cs @@ -1,21 +1,39 @@ using Loopless.Application.Common.Exceptions; using Loopless.Application.DTOs; using Loopless.Application.Interfaces; +using Loopless.Domain.Enums; using MediatR; using Microsoft.EntityFrameworkCore; namespace Loopless.Application.Features.Projects.GetCommits; public sealed class GetProjectCommitsQueryHandler( - IAppDbContext db) : IRequestHandler> + IAppDbContext db, + ICurrentUser currentUser) : IRequestHandler> { public async Task> Handle(GetProjectCommitsQuery request, CancellationToken cancellationToken) { - var projectExists = await db.Projects - .AnyAsync(p => p.Id == request.ProjectId, cancellationToken); + var keycloakId = currentUser.KeycloakId + ?? throw new UnauthorizedException("Missing subject claim."); - if (!projectExists) - throw new NotFoundException("Project not found."); + var user = await db.Users + .FirstOrDefaultAsync(u => u.KeycloakId == keycloakId, cancellationToken) + ?? throw new NotFoundException("User not found."); + + var project = await db.Projects + .FirstOrDefaultAsync(p => p.Id == request.ProjectId, cancellationToken) + ?? throw new NotFoundException("Project not found."); + + var isOwner = project.OwnerId == user.Id; + var isMember = await db.ProjectInvitations + .AnyAsync( + i => i.ProjectId == project.Id + && (i.EnterpriseId == user.Id || i.FreelancerId == user.Id) + && i.Status == InvitationStatus.Accepted, + cancellationToken); + + if (!isOwner && !isMember) + throw new ForbiddenException("Only project members can view commits."); var baseQuery = db.GitHubCommits.Where(c => c.ProjectId == request.ProjectId); var totalCount = await baseQuery.CountAsync(cancellationToken); diff --git a/backend/src/Loopless.Application/Features/Standups/GetAnalytics/ExportStandupsCsvQueryHandler.cs b/backend/src/Loopless.Application/Features/Standups/GetAnalytics/ExportStandupsCsvQueryHandler.cs index 744b69e..05f1ff3 100644 --- a/backend/src/Loopless.Application/Features/Standups/GetAnalytics/ExportStandupsCsvQueryHandler.cs +++ b/backend/src/Loopless.Application/Features/Standups/GetAnalytics/ExportStandupsCsvQueryHandler.cs @@ -1,18 +1,39 @@ using Loopless.Application.Common.Exceptions; using Loopless.Application.DTOs; using Loopless.Application.Interfaces; +using Loopless.Domain.Enums; using MediatR; using Microsoft.EntityFrameworkCore; namespace Loopless.Application.Features.Standups.GetAnalytics; public sealed class ExportStandupsCsvQueryHandler( - IAppDbContext db) : IRequestHandler> + IAppDbContext db, + ICurrentUser currentUser) : IRequestHandler> { public async Task> Handle(ExportStandupsCsvQuery request, CancellationToken cancellationToken) { - var projectExists = await db.Projects.AnyAsync(p => p.Id == request.ProjectId, cancellationToken); - if (!projectExists) throw new NotFoundException("Project not found."); + var keycloakId = currentUser.KeycloakId + ?? throw new UnauthorizedException("Missing subject claim."); + + var user = await db.Users + .FirstOrDefaultAsync(u => u.KeycloakId == keycloakId, cancellationToken) + ?? throw new NotFoundException("User not found."); + + var project = await db.Projects + .FirstOrDefaultAsync(p => p.Id == request.ProjectId, cancellationToken) + ?? throw new NotFoundException("Project not found."); + + var isOwner = project.OwnerId == user.Id; + var isMember = await db.ProjectInvitations + .AnyAsync( + i => i.ProjectId == project.Id + && (i.EnterpriseId == user.Id || i.FreelancerId == user.Id) + && i.Status == InvitationStatus.Accepted, + cancellationToken); + + if (!isOwner && !isMember) + throw new ForbiddenException("Only project members can export standups."); return await db.Standups .Where(s => s.ProjectId == request.ProjectId) diff --git a/backend/src/Loopless.Application/Features/Standups/GetFeed/GetStandupFeedQueryHandler.cs b/backend/src/Loopless.Application/Features/Standups/GetFeed/GetStandupFeedQueryHandler.cs index ad46e1c..9b84893 100644 --- a/backend/src/Loopless.Application/Features/Standups/GetFeed/GetStandupFeedQueryHandler.cs +++ b/backend/src/Loopless.Application/Features/Standups/GetFeed/GetStandupFeedQueryHandler.cs @@ -1,15 +1,40 @@ -using Loopless.Application.DTOs; +using Loopless.Application.Common.Exceptions; +using Loopless.Application.DTOs; using Loopless.Application.Interfaces; +using Loopless.Domain.Enums; using MediatR; using Microsoft.EntityFrameworkCore; namespace Loopless.Application.Features.Standups.GetFeed; public sealed class GetStandupFeedQueryHandler( - IAppDbContext db) : IRequestHandler> + IAppDbContext db, + ICurrentUser currentUser) : IRequestHandler> { public async Task> Handle(GetStandupFeedQuery request, CancellationToken cancellationToken) { + var keycloakId = currentUser.KeycloakId + ?? throw new UnauthorizedException("Missing subject claim."); + + var user = await db.Users + .FirstOrDefaultAsync(u => u.KeycloakId == keycloakId, cancellationToken) + ?? throw new NotFoundException("User not found."); + + var project = await db.Projects + .FirstOrDefaultAsync(p => p.Id == request.ProjectId, cancellationToken) + ?? throw new NotFoundException("Project not found."); + + var isOwner = project.OwnerId == user.Id; + var isMember = await db.ProjectInvitations + .AnyAsync( + i => i.ProjectId == project.Id + && (i.EnterpriseId == user.Id || i.FreelancerId == user.Id) + && i.Status == InvitationStatus.Accepted, + cancellationToken); + + if (!isOwner && !isMember) + throw new ForbiddenException("Only project members can view the standup feed."); + var baseQuery = db.Standups.Where(s => s.ProjectId == request.ProjectId); var totalCount = await baseQuery.CountAsync(cancellationToken); diff --git a/backend/tests/Loopless.UnitTests/Visibility/ProjectVisibilityGuardTests.cs b/backend/tests/Loopless.UnitTests/Visibility/ProjectVisibilityGuardTests.cs new file mode 100644 index 0000000..b92bfe0 --- /dev/null +++ b/backend/tests/Loopless.UnitTests/Visibility/ProjectVisibilityGuardTests.cs @@ -0,0 +1,146 @@ +using Loopless.Application.Common.Exceptions; +using Loopless.Application.Features.Projects.GetCommits; +using Loopless.Application.Features.Standups.GetAnalytics; +using Loopless.Application.Features.Standups.GetFeed; +using Loopless.Domain.Entities; +using Loopless.Domain.Enums; +using Loopless.UnitTests.Messaging; +using Xunit; + +namespace Loopless.UnitTests.Visibility; + +// Regression tests for FR-4.7 "project visibility invited-only, enforced backend". +// The standup feed, CSV export, and commits endpoints previously returned another +// project's data to ANY authenticated user (no membership check). These assert the +// owner-or-accepted-member guard now blocks non-members and admits owner + members. +public class ProjectVisibilityGuardTests +{ + private static User MakeUser(string keycloakId, UserRole role) + => new() + { + Email = $"{keycloakId}@test.local", + Name = keycloakId, + Role = role, + KeycloakId = keycloakId, + }; + + private static Project MakeProject(Guid ownerId) + => new() { OwnerId = ownerId, Title = "Test Project", Status = ProjectStatus.Active }; + + private sealed record Fixture( + ProjectVisibilityTestDbContext Db, + Project Project, + User Owner, + User Member, + User Outsider); + + // Owner (enterprise) + an accepted freelancer member + an unrelated outsider, with one + // standup and one commit on the project. + private static async Task SeedAsync() + { + var db = ProjectVisibilityTestDbContext.CreateInMemory(); + + var owner = MakeUser("owner", UserRole.Enterprise); + var member = MakeUser("member", UserRole.Freelancer); + var outsider = MakeUser("outsider", UserRole.Freelancer); + db.Users.AddRange(owner, member, outsider); + + var project = MakeProject(owner.Id); + db.Projects.Add(project); + + db.ProjectInvitations.Add(new ProjectInvitation + { + ProjectId = project.Id, + EnterpriseId = owner.Id, + FreelancerId = member.Id, + Status = InvitationStatus.Accepted, + }); + + db.Standups.Add(new Standup + { + ProjectId = project.Id, + FreelancerId = member.Id, + WhatDid = "did", + WhatPlan = "plan", + Blockers = "none", + CreatedAt = DateTimeOffset.UtcNow, + }); + + db.GitHubCommits.Add(new GitHubCommit + { + ProjectId = project.Id, + Sha = "abc123", + Message = "init", + Author = "member", + CommittedAt = DateTimeOffset.UtcNow, + }); + + await db.SaveChangesAsync(); + return new Fixture(db, project, owner, member, outsider); + } + + // ---- Standup feed ------------------------------------------------------- + + [Fact] + public async Task StandupFeed_Outsider_IsForbidden() + { + var f = await SeedAsync(); + var handler = new GetStandupFeedQueryHandler(f.Db, new FakeCurrentUser(f.Outsider.KeycloakId)); + await Assert.ThrowsAsync(() => + handler.Handle(new GetStandupFeedQuery(f.Project.Id, 1, 20), CancellationToken.None)); + } + + [Fact] + public async Task StandupFeed_Owner_AndMember_SeeEntries() + { + var f = await SeedAsync(); + + var asOwner = new GetStandupFeedQueryHandler(f.Db, new FakeCurrentUser(f.Owner.KeycloakId)); + var ownerResult = await asOwner.Handle(new GetStandupFeedQuery(f.Project.Id, 1, 20), CancellationToken.None); + Assert.Equal(1, ownerResult.TotalCount); + + var asMember = new GetStandupFeedQueryHandler(f.Db, new FakeCurrentUser(f.Member.KeycloakId)); + var memberResult = await asMember.Handle(new GetStandupFeedQuery(f.Project.Id, 1, 20), CancellationToken.None); + Assert.Equal(1, memberResult.TotalCount); + } + + // ---- CSV export --------------------------------------------------------- + + [Fact] + public async Task StandupCsvExport_Outsider_IsForbidden() + { + var f = await SeedAsync(); + var handler = new ExportStandupsCsvQueryHandler(f.Db, new FakeCurrentUser(f.Outsider.KeycloakId)); + await Assert.ThrowsAsync(() => + handler.Handle(new ExportStandupsCsvQuery(f.Project.Id), CancellationToken.None)); + } + + [Fact] + public async Task StandupCsvExport_Member_GetsRows() + { + var f = await SeedAsync(); + var handler = new ExportStandupsCsvQueryHandler(f.Db, new FakeCurrentUser(f.Member.KeycloakId)); + var rows = await handler.Handle(new ExportStandupsCsvQuery(f.Project.Id), CancellationToken.None); + Assert.Single(rows); + } + + // ---- Commits ------------------------------------------------------------ + + [Fact] + public async Task Commits_Outsider_IsForbidden() + { + var f = await SeedAsync(); + var handler = new GetProjectCommitsQueryHandler(f.Db, new FakeCurrentUser(f.Outsider.KeycloakId)); + await Assert.ThrowsAsync(() => + handler.Handle(new GetProjectCommitsQuery(f.Project.Id, 1, 20), CancellationToken.None)); + } + + [Fact] + public async Task Commits_Owner_SeesCommits() + { + var f = await SeedAsync(); + var handler = new GetProjectCommitsQueryHandler(f.Db, new FakeCurrentUser(f.Owner.KeycloakId)); + var result = await handler.Handle(new GetProjectCommitsQuery(f.Project.Id, 1, 20), CancellationToken.None); + Assert.Equal(1, result.TotalCount); + } +} diff --git a/backend/tests/Loopless.UnitTests/Visibility/ProjectVisibilityTestDbContext.cs b/backend/tests/Loopless.UnitTests/Visibility/ProjectVisibilityTestDbContext.cs new file mode 100644 index 0000000..0ec6a18 --- /dev/null +++ b/backend/tests/Loopless.UnitTests/Visibility/ProjectVisibilityTestDbContext.cs @@ -0,0 +1,93 @@ +using Loopless.Application.Interfaces; +using Loopless.Domain.Entities; +using Microsoft.EntityFrameworkCore; + +namespace Loopless.UnitTests.Visibility; + +// In-memory IAppDbContext that maps the entities the project-visibility guards touch +// (Users, Projects, ProjectInvitations, Standups, GitHubCommits). The Standups-analytics +// test context ignores ProjectInvitation, so the membership guards need their own. +internal sealed class ProjectVisibilityTestDbContext(DbContextOptions options) + : DbContext(options), IAppDbContext +{ + public DbSet Users => Set(); + public DbSet FreelancerProfiles => Set(); + public DbSet EnterpriseProfiles => Set(); + public DbSet Skills => Set(); + public DbSet Embeddings => Set(); + public DbSet Standups => Set(); + public DbSet Conversations => Set(); + public DbSet Messages => Set(); + public DbSet Projects => Set(); + public DbSet ProjectInvitations => Set(); + public DbSet GitHubCommits => Set(); + public DbSet ProjectTasks => Set(); + public DbSet ProjectTaskAuditLogs => Set(); + public DbSet Notifications => Set(); + public DbSet ProjectSummaries => Set(); + public DbSet AuditTrails => Set(); + public DbSet ProjectFiles => Set(); + public DbSet ContentFlags => Set(); + public DbSet ProjectLinks => Set(); + public DbSet SupportTickets => Set(); + + protected override void OnModelCreating(ModelBuilder modelBuilder) + { + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + modelBuilder.Ignore(); + + modelBuilder.Entity(b => + { + b.HasKey(u => u.Id); + b.Ignore(u => u.FreelancerProfile); + b.Ignore(u => u.EnterpriseProfile); + }); + modelBuilder.Entity(b => + { + b.HasKey(p => p.Id); + b.Ignore(p => p.Owner); + b.Ignore(p => p.Invitations); + }); + modelBuilder.Entity(b => + { + b.HasKey(i => i.Id); + b.Ignore(i => i.Project); + b.Ignore(i => i.Enterprise); + b.Ignore(i => i.Freelancer); + }); + modelBuilder.Entity(b => + { + b.HasKey(s => s.Id); + b.HasOne(s => s.Freelancer).WithMany().HasForeignKey(s => s.FreelancerId); + }); + modelBuilder.Entity(b => + { + b.HasKey(c => c.Id); + b.Ignore(c => c.Project); + }); + + base.OnModelCreating(modelBuilder); + } + + public static ProjectVisibilityTestDbContext CreateInMemory() + { + var options = new DbContextOptionsBuilder() + .UseInMemoryDatabase(Guid.NewGuid().ToString()) + .ConfigureWarnings(w => w.Ignore(Microsoft.EntityFrameworkCore.Diagnostics.InMemoryEventId.TransactionIgnoredWarning)) + .Options; + return new ProjectVisibilityTestDbContext(options); + } +} diff --git a/devops/helm/loopless/values.yaml b/devops/helm/loopless/values.yaml index de2722b..986629a 100644 --- a/devops/helm/loopless/values.yaml +++ b/devops/helm/loopless/values.yaml @@ -113,6 +113,11 @@ config: OpenAI__ChatEndpoint: "https://models.inference.ai.azure.com/chat/completions" OpenAI__ChatModel: "gpt-4o-mini" KeyVault__Uri: "" # no cloud vault on this cluster -> plain k8s Secret only + # Restrict /metrics to the in-cluster Prometheus scrape (Host: loopless-backend:8080). + # Snippet directives are disabled on this cluster's ingress, so path-blocking happens + # in-app by Host: the public api host is not allow-listed -> /metrics returns 404 to + # the internet while the in-cluster scrape still works. See Program.cs MapMetrics. + Metrics__AllowedHosts__0: "loopless-backend:8080" # Object storage (in-cluster MinIO). Access/secret keys come from the Secret. Storage__ServiceUrl: "http://minio:9000" Storage__BucketName: "loopless-uploads"