gsadmin/AutopilotMonitor
1
0
Fork 0
mirror of https://github.com/okieselbach/AutopilotMonitor.git synced 2026-10-24 12:42:38 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(backend/security): enforce body-tenant for non-GA + lift query-tenant guards to middleware

Browse Source 
Closes a tenant-boundary write-side hole and removes two manual cross-tenant
checks in favor of catalog-driven middleware enforcement.

SubmitSessionReport + SubmitDiagFilesReport (the violation):
The functions previously fell back to the JWT tenantId only when the body's
TenantId was empty, otherwise trusting the body verbatim. A Tenant Admin of
Tenant-X could POST a body with tenantId=Y and have the report blob, the
SessionReports row, and the audit-log entry all attributed to Tenant-Y
(integrity / non-repudiation issue, no read-side leak).

New behavior: GA-aware. Non-GAs who send a body tenantId that disagrees with
their JWT get a 403 + warning log. GAs may legitimately submit cross-tenant
reports (the UI sends the session's tenantId), but MUST set the body field
explicitly (no silent fallback that would mis-file under the GA's home tenant).

Diagnostics + Progress middleware elevation (defense-in-depth):
diagnostics/download-url and progress/sessions/{sessionId}/events lifted from
TenantScoping.None (in-function guard) to TenantScoping.QueryParam (catalog +
middleware). Same GA bypass semantics — the in-function check was correct but
copy-paste fragile. Functions now read requestCtx.TargetTenantId, which the
middleware has already validated against the JWT.

Test coverage:
CrossTenantAccessTests gains 10 new InlineData rows across the 5 QueryParam
test methods (no-query / same-tenant / cross-non-GA blocked / cross-GA allowed
/ scoping correctness) for both lifted routes. diagnostics/download-url moved
out of JwtImplicitRoutes_NoCrossTenantCheck since it's no longer JWT-implicit.

Verified: dotnet build clean, 1067/1067 backend tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Oliver Kieselbach
2026-05-08 14:12:39 +02:00
parent 592788be99
commit b6178db8e6
6 changed files with 84 additions and 34 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Backend/AutopilotMonitor.Functions.Tests/CrossTenantAccessTests.cs
+10 -1
View File
@@ -137,7 +137,6 @@ public class CrossTenantAccessTests
[InlineData("GET", "/api/metrics/app")]
[InlineData("GET", "/api/audit/logs")]
[InlineData("GET", "/api/rules/gather")]
[InlineData("GET", "/api/diagnostics/download-url")]
public void JwtImplicitRoutes_NoCrossTenantCheck(string httpMethod, string path)
{
var (isBlocked, routeTenantId) = SimulateCrossTenantCheck(httpMethod, path, TenantA, isGlobalAdmin: false);
@@ -205,6 +204,8 @@ public class CrossTenantAccessTests
[InlineData("GET", "/api/sessions/abc-123/analysis")]
[InlineData("GET", "/api/sessions/abc-123/vulnerability-report")]
[InlineData("GET", "/api/bootstrap/sessions")]
[InlineData("GET", "/api/diagnostics/download-url")]
[InlineData("GET", "/api/progress/sessions/abc-123/events")]
public void QueryParam_NoQueryTenant_DefaultsToJwt(string httpMethod, string path)
{
var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, isGlobalAdmin: false, queryTenantId: null);
@@ -219,6 +220,8 @@ public class CrossTenantAccessTests
[InlineData("GET", "/api/sessions/abc-123")]
[InlineData("GET", "/api/sessions/abc-123/events")]
[InlineData("GET", "/api/bootstrap/sessions")]
[InlineData("GET", "/api/diagnostics/download-url")]
[InlineData("GET", "/api/progress/sessions/abc-123/events")]
public void QueryParam_SameTenant_IsAllowed(string httpMethod, string path)
{
var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, isGlobalAdmin: false, queryTenantId: TenantA);
@@ -236,6 +239,8 @@ public class CrossTenantAccessTests
[InlineData("GET", "/api/sessions/abc-123/vulnerability-report")]
[InlineData("GET", "/api/bootstrap/sessions")]
[InlineData("DELETE", "/api/bootstrap/sessions/CODE123")]
[InlineData("GET", "/api/diagnostics/download-url")]
[InlineData("GET", "/api/progress/sessions/abc-123/events")]
public void QueryParam_CrossTenant_NonGA_IsBlocked(string httpMethod, string path)
{
var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, isGlobalAdmin: false, queryTenantId: TenantB);
@@ -250,6 +255,8 @@ public class CrossTenantAccessTests
[InlineData("GET", "/api/sessions/abc-123")]
[InlineData("GET", "/api/sessions/abc-123/events")]
[InlineData("GET", "/api/bootstrap/sessions")]
[InlineData("GET", "/api/diagnostics/download-url")]
[InlineData("GET", "/api/progress/sessions/abc-123/events")]
public void QueryParam_CrossTenant_GlobalAdmin_IsAllowed(string httpMethod, string path)
{
var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, isGlobalAdmin: true, queryTenantId: TenantB);
@@ -268,6 +275,8 @@ public class CrossTenantAccessTests
[InlineData("GET", "bootstrap/sessions")]
[InlineData("POST", "bootstrap/sessions")]
[InlineData("DELETE", "bootstrap/sessions/{code}")]
[InlineData("GET", "diagnostics/download-url")]
[InlineData("GET", "progress/sessions/{sessionId}/events")]
public void QueryParamRoutes_HaveCorrectScoping(string httpMethod, string routeTemplate)
{
var entry = EndpointAccessPolicyCatalog.Entries
src/Backend/AutopilotMonitor.Functions/Functions/Diagnostics/DiagnosticsDownloadFunction.cs
+6 -10
View File
@@ -44,27 +44,23 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
{
try
{
// Authentication + MemberRead authorization enforced by PolicyEnforcementMiddleware
// Authentication, MemberRead authz, AND cross-tenant access enforced by
// PolicyEnforcementMiddleware (catalog: TenantScoping.QueryParam).
// requestCtx.TargetTenantId is the middleware-validated tenantId from the
// ?tenantId= query param (GA bypass already applied).
var requestCtx = req.GetRequestContext();
var query = HttpUtility.ParseQueryString(req.Url.Query);
var tenantId = query["tenantId"];
var blobName = query["blobName"];
if (string.IsNullOrEmpty(tenantId) || string.IsNullOrEmpty(blobName))
if (string.IsNullOrEmpty(query["tenantId"]) || string.IsNullOrEmpty(blobName))
{
var badRequest = req.CreateResponse(HttpStatusCode.BadRequest);
await badRequest.WriteAsJsonAsync(new { success = false, message = "tenantId and blobName query parameters are required." });
return badRequest;
}
// Validate tenant access
if (!requestCtx.IsGlobalAdmin && tenantId != requestCtx.TenantId)
{
var forbiddenResponse = req.CreateResponse(HttpStatusCode.Forbidden);
await forbiddenResponse.WriteAsJsonAsync(new { success = false, message = "Access denied." });
return forbiddenResponse;
}
var tenantId = requestCtx.TargetTenantId;
// Validate blob name (prevent path traversal, double-encoding, and null byte attacks)
var decodedBlobName = Uri.UnescapeDataString(blobName);
src/Backend/AutopilotMonitor.Functions/Functions/Progress/ProgressPortalFunction.cs
+8 -19
View File
@@ -92,14 +92,15 @@ public class ProgressPortalFunction
try
{
// Authentication + AuthenticatedUser authorization enforced by PolicyEnforcementMiddleware
// Authentication, AuthenticatedUser authz, AND cross-tenant access enforced by
// PolicyEnforcementMiddleware (catalog: TenantScoping.QueryParam).
// requestCtx.TargetTenantId is the middleware-validated tenantId from the
// ?tenantId= query param (GA bypass already applied).
var requestCtx = req.GetRequestContext();
var userIdentifier = requestCtx.UserPrincipalName;
var query = HttpUtility.ParseQueryString(req.Url.Query);
var requestedTenantId = query["tenantId"];
if (string.IsNullOrEmpty(requestedTenantId))
if (string.IsNullOrEmpty(query["tenantId"]))
{
var badRequest = req.CreateResponse(HttpStatusCode.BadRequest);
await badRequest.WriteAsJsonAsync(new
@@ -113,22 +114,10 @@ public class ProgressPortalFunction
return badRequest;
}
// Cross-tenant access only for Global Admins
if (!requestCtx.IsGlobalAdmin && requestedTenantId != requestCtx.TenantId)
{
var forbiddenResponse = req.CreateResponse(HttpStatusCode.Forbidden);
await forbiddenResponse.WriteAsJsonAsync(new
{
success = false,
message = "Access denied. You can only view events for sessions in your own tenant.",
sessionId = sessionId,
count = 0,
events = Array.Empty<object>()
});
return forbiddenResponse;
}
var requestedTenantId = requestCtx.TargetTenantId;
if (requestCtx.IsGlobalAdmin && requestedTenantId != requestCtx.TenantId)
// Audit log for GA cross-tenant access (middleware allowed it; logged here for visibility).
if (requestCtx.IsGlobalAdmin && !string.Equals(requestedTenantId, requestCtx.TenantId, StringComparison.OrdinalIgnoreCase))
{
_logger.LogInformation("{SessionPrefix} Global Admin {User} accessing cross-tenant progress events (tenant: {TenantId})",
sessionPrefix, userIdentifier, requestedTenantId);
src/Backend/AutopilotMonitor.Functions/Functions/Reports/SubmitDiagFilesReportFunction.cs
+28 -1
View File
@@ -74,10 +74,37 @@ namespace AutopilotMonitor.Functions.Functions.Reports
return badRequestResponse;
}
if (string.IsNullOrEmpty(request.TenantId))
// Tenant identity: enforce JWT tenantId for non-GAs (prevents body
// tampering / horizontal escalation). Global Admins MAY submit reports
// on behalf of foreign tenants and MUST specify the target tenantId.
if (!requestCtx.IsGlobalAdmin)
{
if (!string.IsNullOrEmpty(request.TenantId)
&& !string.Equals(request.TenantId, tenantId, StringComparison.OrdinalIgnoreCase))
{
_logger.LogWarning(
"SubmitDiagFilesReport: BLOCKED cross-tenant body for non-GA user={User} jwtTenant={JwtTenant} bodyTenant={BodyTenant}",
userIdentifier, tenantId, request.TenantId);
var forbidden = req.CreateResponse(HttpStatusCode.Forbidden);
await forbidden.WriteAsJsonAsync(new
{
success = false,
message = "Body tenantId must match your authenticated tenant."
});
return forbidden;
}
request.TenantId = tenantId;
}
else if (string.IsNullOrEmpty(request.TenantId))
{
var bad = req.CreateResponse(HttpStatusCode.BadRequest);
await bad.WriteAsJsonAsync(new
{
success = false,
message = "tenantId is required in body for Global Admin submissions."
});
return bad;
}
var metadata = await _sessionReportService.SubmitDiagFilesReportAsync(request, userIdentifier);
src/Backend/AutopilotMonitor.Functions/Functions/Reports/SubmitSessionReportFunction.cs
+30 -1
View File
@@ -71,10 +71,39 @@ namespace AutopilotMonitor.Functions.Functions.Reports
// Ensure sessionId consistency
request.SessionId = sessionId;
if (string.IsNullOrEmpty(request.TenantId))
// Tenant identity: enforce JWT tenantId for non-GAs (prevents body
// tampering / horizontal escalation). Global Admins MAY submit reports
// against foreign tenants — the UI sends the session's tenantId, which
// for cross-tenant GA views is the foreign tenant.
if (!requestCtx.IsGlobalAdmin)
{
if (!string.IsNullOrEmpty(request.TenantId)
&& !string.Equals(request.TenantId, tenantId, StringComparison.OrdinalIgnoreCase))
{
_logger.LogWarning(
"SubmitSessionReport: BLOCKED cross-tenant body for non-GA user={User} jwtTenant={JwtTenant} bodyTenant={BodyTenant}",
userIdentifier, tenantId, request.TenantId);
var forbidden = req.CreateResponse(HttpStatusCode.Forbidden);
await forbidden.WriteAsJsonAsync(new
{
success = false,
message = "Body tenantId must match your authenticated tenant."
});
return forbidden;
}
request.TenantId = tenantId;
}
else if (string.IsNullOrEmpty(request.TenantId))
{
var bad = req.CreateResponse(HttpStatusCode.BadRequest);
await bad.WriteAsJsonAsync(new
{
success = false,
message = "tenantId is required in body for Global Admin submissions."
});
return bad;
}
// Submit report
var metadata = await _sessionReportService.SubmitReportAsync(request, userIdentifier);
src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs
+2 -2
View File
@@ -121,7 +121,7 @@ public static class EndpointAccessPolicyCatalog
new("POST", "realtime/groups/join", EndpointPolicy.MemberRead),
new("POST", "realtime/groups/leave", EndpointPolicy.MemberRead),
new("GET", "progress/sessions", EndpointPolicy.AuthenticatedUser),
new("GET", "progress/sessions/{sessionId}/events", EndpointPolicy.AuthenticatedUser),
new("GET", "progress/sessions/{sessionId}/events", EndpointPolicy.AuthenticatedUser, TenantScoping.QueryParam),
new("PUT", "preview/notification-email", EndpointPolicy.AuthenticatedUser),
new("GET", "feedback/status", EndpointPolicy.AuthenticatedUser),
new("POST", "feedback", EndpointPolicy.AuthenticatedUser),
@@ -156,7 +156,7 @@ public static class EndpointAccessPolicyCatalog
new("GET", "metrics/ime-versions", EndpointPolicy.MemberRead),
new("GET", "audit/logs", EndpointPolicy.MemberRead),
new("GET", "audit/hardware-rejected", EndpointPolicy.MemberRead),
new("GET", "diagnostics/download-url", EndpointPolicy.MemberRead),
new("GET", "diagnostics/download-url", EndpointPolicy.MemberRead, TenantScoping.QueryParam),
new("GET", "rules/gather", EndpointPolicy.MemberRead),
new("GET", "rules/analyze", EndpointPolicy.MemberRead),
new("GET", "rules/ime-log-patterns", EndpointPolicy.MemberRead),