From 03a8bf92c90102bc390344a1026393fb0b0cae5e Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Fri, 9 Oct 2026 15:17:47 +0200 Subject: [PATCH] App names and serial numbers travel as query values: app details for names with '/' open The host decodes %2F to a path separator before routing, so apps/{appName}/analytics|sessions and their global twins answered 404 for every app whose name contains '/', before any function ran; a name with a literal escape such as '%20' was decoded a second time. The routes are now apps/analytics, apps/sessions, global/apps/analytics and global/apps/sessions with ?appName=, and the unblock route is DELETE devices/block?serialNumber=&tenantId= (serials may contain '/' too). Function names, policies and tenant scoping stay; req.Query decodes exactly once. Tests run names with '/', '%', '+', a space and a literal escape through the catalog and the handlers, and the web builders put the name into the query. Manifest and wire types regenerated (doc text only). Rollout: backend, then web right after; an app-detail tab on the old bundle 404s until reloaded. Co-Authored-By: Claude Opus 5.5 --- .../AppsAnalyticsFailureRateTests.cs | 2 +- .../EndpointPolicyCatalogCompletenessTests.cs | 5 +- .../FreeTextNameQueryParamTests.cs | 226 ++++++++++++++++++ .../Functions/Admin/DeviceBlockFunction.cs | 13 +- .../Functions/Apps/AppsAnalyticsHelper.cs | 4 +- .../Functions/Apps/GetAppAnalyticsFunction.cs | 17 +- .../Functions/Apps/GetAppSessionsFunction.cs | 14 +- .../Apps/GetGlobalAppAnalyticsFunction.cs | 21 +- .../Apps/GetGlobalAppSessionsFunction.cs | 19 +- .../Security/EndpointAccessPolicyCatalog.cs | 12 +- .../src/generated/wire-types.generated.ts | 4 +- .../Models/Metrics/AppsDashboardApiModels.cs | 8 +- .../lib/__tests__/apiPathEncoding.test.ts | 11 + .../lib/__tests__/scopedApi.test.ts | 15 +- src/Web/autopilot-monitor-web/lib/api.ts | 15 +- .../utils/shared-manifests.json | 4 +- .../utils/wire-types.generated.ts | 4 +- 17 files changed, 326 insertions(+), 68 deletions(-) create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/FreeTextNameQueryParamTests.cs diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AppsAnalyticsFailureRateTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AppsAnalyticsFailureRateTests.cs index 640f64e4..02fca62d 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/AppsAnalyticsFailureRateTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AppsAnalyticsFailureRateTests.cs @@ -118,7 +118,7 @@ public class AppsAnalyticsFailureRateTests Assert.False(app.TryGetProperty("trendDelta", out _)); } - // ── /apps/{appName}/analytics ─────────────────────────────────────────── + // ── /apps/analytics ───────────────────────────────────────────────────── private static async Task BuildAnalyticsAsync( List summaries, ISessionRepository? sessionRepo = null) diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs index afc4db76..b96e06c2 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs @@ -137,7 +137,6 @@ public class EndpointPolicyCatalogCompletenessTests [InlineData("PUT", "/api/rules/gather/rule-1", "rules/gather/{ruleId}")] [InlineData("DELETE", "/api/tenants/tid-1/admins/user@contoso.com", "tenants/{tenantId}/admins/{adminUpn}")] [InlineData("PATCH", "/api/tenants/tid-1/admins/user@contoso.com/permissions", "tenants/{tenantId}/admins/{adminUpn}/permissions")] - [InlineData("DELETE", "/api/devices/block/SN123456", "devices/block/{encodedSerialNumber}")] [InlineData("DELETE", "/api/versions/block/v1.0.*", "versions/block/{encodedPattern}")] [InlineData("PATCH", "/api/global/session-reports/report-1/note", "global/session-reports/{reportId}/note")] [InlineData("POST", "/api/rules/analyze/ANALYZE-ID-001/create-from-template", "rules/analyze/{ruleId}/create-from-template")] @@ -284,8 +283,8 @@ public class EndpointPolicyCatalogCompletenessTests /// [Theory] [InlineData("GET", "/api/global/apps/list", "global/apps/list")] - [InlineData("GET", "/api/global/apps/Company%20Portal/analytics", "global/apps/{appName}/analytics")] - [InlineData("GET", "/api/global/apps/Company%20Portal/sessions", "global/apps/{appName}/sessions")] + [InlineData("GET", "/api/global/apps/analytics", "global/apps/analytics")] + [InlineData("GET", "/api/global/apps/sessions", "global/apps/sessions")] public void GlobalAppsRoutes_AreGlobalReadOrAdmin(string method, string path, string expectedTemplate) { var entry = EndpointAccessPolicyCatalog.FindPolicy(method, path); diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/FreeTextNameQueryParamTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/FreeTextNameQueryParamTests.cs new file mode 100644 index 00000000..bf3015cf --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/FreeTextNameQueryParamTests.cs @@ -0,0 +1,226 @@ +using System.Net; +using System.Security.Claims; +using System.Text.Json; +using AutopilotMonitor.Functions.Functions.Admin; +using AutopilotMonitor.Functions.Functions.Apps; +using AutopilotMonitor.Functions.Security; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared.DataAccess; +using AutopilotMonitor.Shared.Models; +using Microsoft.Azure.Functions.Worker.Http; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// App names and serial numbers are free text and may contain '/', '%' and '+'. They travel as query +/// values: in a path segment the host decodes an escaped slash to a separator before routing (a 404 +/// before any function runs), and a catalog placeholder never matches a '/'. The encoded forms below +/// are what the portal's URLSearchParams sends; req.Query decodes them exactly once. +/// +public class FreeTextNameQueryParamTests +{ + private const string TenantId = "00000000-0000-0000-0000-0000000000a7"; + + public static TheoryData Names => new() + { + { "a/b", "a%2Fb" }, + { "100%", "100%25" }, + { "x+y", "x%2By" }, + { "Company Portal", "Company+Portal" }, + // A literal escape sequence survives; the former second decode turned it into "a b". + { "a%20b", "a%2520b" }, + }; + + // ── Policy catalog ─────────────────────────────────────────────────────── + + [Theory] + [InlineData("GET", "/api/apps/analytics", "apps/analytics", EndpointPolicy.MemberRead, TenantScoping.None)] + [InlineData("GET", "/api/apps/sessions", "apps/sessions", EndpointPolicy.MemberRead, TenantScoping.None)] + [InlineData("GET", "/api/global/apps/analytics", "global/apps/analytics", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam)] + [InlineData("GET", "/api/global/apps/sessions", "global/apps/sessions", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam)] + [InlineData("DELETE", "/api/devices/block", "devices/block", EndpointPolicy.GlobalAdminOnly, TenantScoping.None)] + public void Free_name_routes_are_literal_and_keep_their_policy( + string method, string path, string template, EndpointPolicy policy, TenantScoping scoping) + { + var entry = EndpointAccessPolicyCatalog.FindPolicy(method, path); + + Assert.NotNull(entry); + Assert.Equal(template, entry!.RouteTemplate); + Assert.Equal(policy, entry.Policy); + Assert.Equal(scoping, entry.TenantScoping); + } + + [Fact] + public void No_route_carries_an_app_name_or_serial_number_in_the_path() + { + var offenders = EndpointAccessPolicyCatalog.Entries + .Where(e => e.RouteTemplate.Contains("{appName}", StringComparison.OrdinalIgnoreCase) + || e.RouteTemplate.Contains("serialNumber}", StringComparison.OrdinalIgnoreCase)) + .Select(e => $"{e.HttpMethod} {e.RouteTemplate}") + .ToList(); + + Assert.Empty(offenders); + } + + // ── Query path: the name reaches the handler unchanged ────────────────── + + [Theory] + [MemberData(nameof(Names))] + public async Task App_analytics_matches_the_exact_name(string name, string encoded) + { + var function = new GetAppAnalyticsFunction( + NullLogger.Instance, MetricsWith(name), Sessions(), RegressionTracker()); + var req = MemberRequest($"?appName={encoded}&days=30"); + + var res = await function.Run(req); + + Assert.Equal(HttpStatusCode.OK, res.StatusCode); + var root = Json(res); + Assert.Equal(name, root.GetProperty("appName").GetString()); + Assert.Equal(1, root.GetProperty("summary").GetProperty("totalInstalls").GetInt32()); + } + + [Theory] + [MemberData(nameof(Names))] + public async Task App_sessions_match_the_exact_name(string name, string encoded) + { + var function = new GetAppSessionsFunction(NullLogger.Instance, MetricsWith(name), Sessions()); + var req = MemberRequest($"?appName={encoded}&days=30"); + + var res = await function.Run(req); + + Assert.Equal(HttpStatusCode.OK, res.StatusCode); + Assert.Equal(1, Json(res).GetProperty("total").GetInt32()); + } + + [Theory] + [MemberData(nameof(Names))] + public async Task Global_app_analytics_and_sessions_match_the_exact_name(string name, string encoded) + { + var analytics = new GetGlobalAppAnalyticsFunction( + NullLogger.Instance, MetricsWith(name), Sessions(), RegressionTracker()); + var sessions = new GetGlobalAppSessionsFunction(NullLogger.Instance, MetricsWith(name), Sessions()); + var query = $"?appName={encoded}&days=30&tenantId={TenantId}"; + + var analyticsRes = await analytics.Run(EndpointHarness.Request(TenantId, queryString: query).Req); + var sessionsRes = await sessions.Run(EndpointHarness.Request(TenantId, queryString: query).Req); + + Assert.Equal(1, Json(analyticsRes).GetProperty("summary").GetProperty("totalInstalls").GetInt32()); + Assert.Equal(1, Json(sessionsRes).GetProperty("total").GetInt32()); + } + + [Fact] + public async Task App_routes_without_a_name_are_a_bad_request() + { + var analytics = new GetAppAnalyticsFunction( + NullLogger.Instance, MetricsWith("x"), Sessions(), RegressionTracker()); + var globalSessions = new GetGlobalAppSessionsFunction(NullLogger.Instance, MetricsWith("x"), Sessions()); + + Assert.Equal(HttpStatusCode.BadRequest, (await analytics.Run(MemberRequest("?days=30"))).StatusCode); + Assert.Equal(HttpStatusCode.BadRequest, + (await globalSessions.Run(EndpointHarness.Request(TenantId, queryString: "?appName=%20&days=30").Req)).StatusCode); + } + + [Theory] + [MemberData(nameof(Names))] + public async Task Unblock_receives_the_exact_serial_number(string serial, string encoded) + { + var repo = new Mock(); + repo.Setup(r => r.UnblockDeviceAsync(TenantId, serial)).ReturnsAsync(Array.Empty()); + var function = UnblockFunction(repo.Object); + + var res = await function.UnblockDevice( + EndpointHarness.Request(TenantId, queryString: $"?serialNumber={encoded}&tenantId={TenantId}").Req); + + Assert.Equal(HttpStatusCode.OK, res.StatusCode); + repo.Verify(r => r.UnblockDeviceAsync(TenantId, serial), Times.Once); + } + + [Fact] + public async Task Unblock_without_a_serial_number_is_a_bad_request() + { + var repo = new Mock(); + var function = UnblockFunction(repo.Object); + + var res = await function.UnblockDevice(EndpointHarness.Request(TenantId, queryString: $"?tenantId={TenantId}").Req); + + Assert.Equal(HttpStatusCode.BadRequest, res.StatusCode); + repo.Verify(r => r.UnblockDeviceAsync(It.IsAny(), It.IsAny()), Times.Never); + } + + // ── Rig ────────────────────────────────────────────────────────────────── + + /// One finished install of , plus decoys named after its parts. + private static IMetricsRepository MetricsWith(string name) + { + AppInstallSummary Install(string appName, string sessionId) => new() + { + TenantId = TenantId, + SessionId = sessionId, + AppName = appName, + Status = "Succeeded", + StartedAt = DateTime.UtcNow.AddHours(-2), + DurationSeconds = 30, + }; + + var summaries = new List { Install(name, "s-1"), Install("a", "s-2"), Install("b", "s-3"), Install("a b", "s-4") }; + var repo = new Mock(); + repo.Setup(r => r.GetAppsDashboardSummariesAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(() => summaries.ToList()); + return repo.Object; + } + + private static ISessionRepository Sessions() + { + var repo = new Mock(); + repo.Setup(r => r.GetSessionAsync(It.IsAny(), It.IsAny())).ReturnsAsync((SessionSummary?)null); + return repo.Object; + } + + private static IHardwareRejectionNotificationTracker RegressionTracker() + { + var tracker = new Mock(); + tracker.Setup(t => t.GetAppVersionRegressionsAsync(It.IsAny())).ReturnsAsync(new List()); + return tracker.Object; + } + + /// A member request: the JWT tenant comes from the principal the authentication middleware would set. + private static HttpRequestData MemberRequest(string queryString) + { + var (req, context) = EndpointHarness.Request(TenantId, queryString: queryString); + context.Items["ClaimsPrincipal"] = new ClaimsPrincipal(new ClaimsIdentity(new[] { new Claim("tid", TenantId) }, "test")); + return req; + } + + private static DeviceBlockFunction UnblockFunction(IDeviceSecurityRepository securityRepo) + { + var maintenance = new Mock(); + maintenance + .Setup(m => m.LogAuditEntryAsync(It.IsAny(), It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny?>())) + .ReturnsAsync(true); + var adminConfig = new Mock( + Mock.Of(), NullLogger.Instance, + new MemoryCache(new MemoryCacheOptions())) { CallBase = false }; + var opsEvents = new OpsEventService( + Mock.Of(), NullLogger.Instance, + TestNotifications.InertOpsAlertDispatch(adminConfig.Object)); + + return new DeviceBlockFunction( + NullLogger.Instance, + new BlockedDeviceService(securityRepo, Mock.Of(), NullLogger.Instance), + maintenance.Object, + opsEvents); + } + + private static JsonElement Json(HttpResponseData res) + { + res.Body.Position = 0; + using var doc = JsonDocument.Parse(res.Body); + return doc.RootElement.Clone(); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Admin/DeviceBlockFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Admin/DeviceBlockFunction.cs index 12312b66..9838983d 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Admin/DeviceBlockFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Admin/DeviceBlockFunction.cs @@ -135,11 +135,14 @@ namespace AutopilotMonitor.Functions.Functions.Admin } } - /// DELETE /api/devices/block/{encodedSerialNumber}?tenantId={tenantId} — unblock a device + /// + /// DELETE /api/devices/block?serialNumber={serialNumber}&tenantId={tenantId} — unblock a device. The serial + /// is a query value, never a path segment: serial numbers may contain '/', which the host decodes to a path + /// separator before any route matches. + /// [Function("UnblockDevice")] public async Task UnblockDevice( - [HttpTrigger(AuthorizationLevel.Anonymous, "delete", Route = "devices/block/{encodedSerialNumber}")] HttpRequestData req, - string encodedSerialNumber) + [HttpTrigger(AuthorizationLevel.Anonymous, "delete", Route = "devices/block")] HttpRequestData req) { try { @@ -150,9 +153,9 @@ namespace AutopilotMonitor.Functions.Functions.Admin if (string.IsNullOrEmpty(tenantId)) return await req.BadRequestAsync("tenantId query parameter is required"); - var serialNumber = Uri.UnescapeDataString(encodedSerialNumber ?? string.Empty); + var serialNumber = req.Query["serialNumber"]; if (string.IsNullOrEmpty(serialNumber)) - return await req.BadRequestAsync("serialNumber is required"); + return await req.BadRequestAsync("serialNumber query parameter is required"); await _blockedDeviceService.UnblockDeviceAsync(tenantId, serialNumber); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/AppsAnalyticsHelper.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/AppsAnalyticsHelper.cs index 4add5b4d..5fb5e5b8 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/AppsAnalyticsHelper.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/AppsAnalyticsHelper.cs @@ -230,7 +230,7 @@ namespace AutopilotMonitor.Functions.Functions.Apps }; } - // ── /apps/{appName}/analytics ─────────────────────────────────────── + // ── /apps/analytics ───────────────────────────────────────────────── /// /// Builds the analytics response body for a single app. @@ -462,7 +462,7 @@ namespace AutopilotMonitor.Functions.Functions.Apps }; } - // ── /apps/{appName}/sessions ──────────────────────────────────────── + // ── /apps/sessions ────────────────────────────────────────────────── public static async Task BuildSessionsResponseAsync( List allSummaries, diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetAppAnalyticsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetAppAnalyticsFunction.cs index d1c66beb..ecbc9e1b 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetAppAnalyticsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetAppAnalyticsFunction.cs @@ -9,9 +9,11 @@ using Microsoft.Extensions.Logging; namespace AutopilotMonitor.Functions.Functions.Apps { /// - /// GET /api/apps/{appName}/analytics?days=30 + /// GET /api/apps/analytics?appName=...&days=30 /// Per-tenant drill-down for a single app: time series, version breakdown, /// installer phase breakdown, top failure codes, device-model correlation. + /// The name is a query value, never a path segment: app names may contain '/', which the + /// host decodes to a path separator before any route matches. /// public class GetAppAnalyticsFunction { @@ -34,20 +36,19 @@ namespace AutopilotMonitor.Functions.Functions.Apps [Function("GetAppAnalytics")] public async Task Run( - [HttpTrigger(AuthorizationLevel.Anonymous, "get", Route = "apps/{appName}/analytics")] HttpRequestData req, - string appName) + [HttpTrigger(AuthorizationLevel.Anonymous, "get", Route = "apps/analytics")] HttpRequestData req) { try { var tenantId = TenantHelper.GetTenantId(req); - var decodedAppName = Uri.UnescapeDataString(appName ?? string.Empty); - if (string.IsNullOrWhiteSpace(decodedAppName)) + var query = req.Query; + var appName = query["appName"]; + if (string.IsNullOrWhiteSpace(appName)) { return await req.BadRequestAsync("appName is required"); } - var query = req.Query; var days = QueryParams.Int(query["days"], @default: 30, min: 1, max: 365); if (!AppsAnalyticsHelper.TryParseSourceQueryParam(query["source"], out var source)) { @@ -57,11 +58,11 @@ namespace AutopilotMonitor.Functions.Functions.Apps var summaries = await AppsAnalyticsHelper.LoadSummariesAsync(_metricsRepo, tenantId, days); // Active duration-regression episodes for this app (fail-soft: empty on error). var versionRegressions = (await _notificationTracker.GetAppVersionRegressionsAsync(tenantId)) - .Where(a => string.Equals(a.AppName, decodedAppName, StringComparison.OrdinalIgnoreCase) + .Where(a => string.Equals(a.AppName, appName, StringComparison.OrdinalIgnoreCase) && AppInstallSources.Normalize(a.Source) == source) .ToList(); var body = await AppsAnalyticsHelper.BuildAnalyticsResponseAsync( - summaries, _sessionRepo, decodedAppName, source, days, versionRegressions); + summaries, _sessionRepo, appName, source, days, versionRegressions); var response = req.CreateResponse(HttpStatusCode.OK); await response.WriteAsJsonAsync(body); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetAppSessionsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetAppSessionsFunction.cs index e8e0df4d..d5158cf0 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetAppSessionsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetAppSessionsFunction.cs @@ -9,8 +9,9 @@ using Microsoft.Extensions.Logging; namespace AutopilotMonitor.Functions.Functions.Apps { /// - /// GET /api/apps/{appName}/sessions?days=30&status=failed&model=...&version=...&offset=0&limit=50 + /// GET /api/apps/sessions?appName=...&days=30&status=failed&model=...&version=...&offset=0&limit=50 /// Returns paginated sessions that (tried to) install a given app for the caller's tenant. + /// The name is a query value for the reason given on . /// public class GetAppSessionsFunction { @@ -33,20 +34,19 @@ namespace AutopilotMonitor.Functions.Functions.Apps [Function("GetAppSessions")] public async Task Run( - [HttpTrigger(AuthorizationLevel.Anonymous, "get", Route = "apps/{appName}/sessions")] HttpRequestData req, - string appName) + [HttpTrigger(AuthorizationLevel.Anonymous, "get", Route = "apps/sessions")] HttpRequestData req) { try { var tenantId = TenantHelper.GetTenantId(req); - var decodedAppName = Uri.UnescapeDataString(appName ?? string.Empty); - if (string.IsNullOrWhiteSpace(decodedAppName)) + var query = req.Query; + var appName = query["appName"]; + if (string.IsNullOrWhiteSpace(appName)) { return await req.BadRequestAsync("appName is required"); } - var query = req.Query; var days = QueryParams.Int(query["days"], @default: 30, min: 1, max: 365); if (!AppsAnalyticsHelper.TryParseSourceQueryParam(query["source"], out var source)) { @@ -63,7 +63,7 @@ namespace AutopilotMonitor.Functions.Functions.Apps var summaries = await AppsAnalyticsHelper.LoadSummariesAsync(_metricsRepo, tenantId, days); var body = await AppsAnalyticsHelper.BuildSessionsResponseAsync( - summaries, _sessionRepo, decodedAppName, source, days, + summaries, _sessionRepo, appName, source, days, statusFilter, modelFilter, versionFilter, offset, limit); var response = req.CreateResponse(HttpStatusCode.OK); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetGlobalAppAnalyticsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetGlobalAppAnalyticsFunction.cs index 60538d88..9cf8d14d 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetGlobalAppAnalyticsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetGlobalAppAnalyticsFunction.cs @@ -9,9 +9,9 @@ using Microsoft.Extensions.Logging; namespace AutopilotMonitor.Functions.Functions.Apps { /// - /// GET /api/global/apps/{appName}/analytics?days=30[&tenantId=GUID] - /// Global Admin variant of . - /// Authorization: GlobalAdminOnly. + /// GET /api/global/apps/analytics?appName=...&days=30[&tenantId=GUID] + /// Global Admin variant of (the name is a query value for the same reason). + /// Authorization: GlobalReadOrAdmin. /// public class GetGlobalAppAnalyticsFunction { @@ -34,19 +34,18 @@ namespace AutopilotMonitor.Functions.Functions.Apps [Function("GetGlobalAppAnalytics")] public async Task Run( - [HttpTrigger(AuthorizationLevel.Anonymous, "get", Route = "global/apps/{appName}/analytics")] HttpRequestData req, - string appName) + [HttpTrigger(AuthorizationLevel.Anonymous, "get", Route = "global/apps/analytics")] HttpRequestData req) { try { var userEmail = TenantHelper.GetUserIdentifier(req); - var decodedAppName = Uri.UnescapeDataString(appName ?? string.Empty); - if (string.IsNullOrWhiteSpace(decodedAppName)) + var query = req.Query; + var appName = query["appName"]; + if (string.IsNullOrWhiteSpace(appName)) { return await req.BadRequestAsync("appName is required"); } - var query = req.Query; var scopedTenantId = query["tenantId"]; if (!AppsAnalyticsHelper.IsValidOptionalTenantIdQueryParam(scopedTenantId)) { @@ -60,7 +59,7 @@ namespace AutopilotMonitor.Functions.Functions.Apps _logger.LogInformation( "Global apps/{App}/analytics requested (user: {User}, tenantId: {TenantId}, days: {Days})", - decodedAppName, userEmail, scopedTenantId ?? "", days); + appName, userEmail, scopedTenantId ?? "", days); var summaries = await AppsAnalyticsHelper.LoadSummariesAsync(_metricsRepo, scopedTenantId, days); // Episodes are per-tenant tracker rows; without a tenant scope the aggregated @@ -68,11 +67,11 @@ namespace AutopilotMonitor.Functions.Functions.Apps var versionRegressions = string.IsNullOrEmpty(scopedTenantId) ? new List() : (await _notificationTracker.GetAppVersionRegressionsAsync(scopedTenantId!)) - .Where(a => string.Equals(a.AppName, decodedAppName, StringComparison.OrdinalIgnoreCase) + .Where(a => string.Equals(a.AppName, appName, StringComparison.OrdinalIgnoreCase) && AppInstallSources.Normalize(a.Source) == source) .ToList(); var body = await AppsAnalyticsHelper.BuildAnalyticsResponseAsync( - summaries, _sessionRepo, decodedAppName, source, days, versionRegressions); + summaries, _sessionRepo, appName, source, days, versionRegressions); var response = req.CreateResponse(HttpStatusCode.OK); await response.WriteAsJsonAsync(body); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetGlobalAppSessionsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetGlobalAppSessionsFunction.cs index 92b8218a..803e1ff5 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetGlobalAppSessionsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Apps/GetGlobalAppSessionsFunction.cs @@ -9,9 +9,9 @@ using Microsoft.Extensions.Logging; namespace AutopilotMonitor.Functions.Functions.Apps { /// - /// GET /api/global/apps/{appName}/sessions?days=30&status=failed[&tenantId=GUID][&model=X][&version=Y][&offset=N][&limit=N] - /// Global Admin variant of . - /// Authorization: GlobalAdminOnly. + /// GET /api/global/apps/sessions?appName=...&days=30&status=failed[&tenantId=GUID][&model=X][&version=Y][&offset=N][&limit=N] + /// Global Admin variant of (the name is a query value for the same reason). + /// Authorization: GlobalReadOrAdmin. /// public class GetGlobalAppSessionsFunction { @@ -34,19 +34,18 @@ namespace AutopilotMonitor.Functions.Functions.Apps [Function("GetGlobalAppSessions")] public async Task Run( - [HttpTrigger(AuthorizationLevel.Anonymous, "get", Route = "global/apps/{appName}/sessions")] HttpRequestData req, - string appName) + [HttpTrigger(AuthorizationLevel.Anonymous, "get", Route = "global/apps/sessions")] HttpRequestData req) { try { var userEmail = TenantHelper.GetUserIdentifier(req); - var decodedAppName = Uri.UnescapeDataString(appName ?? string.Empty); - if (string.IsNullOrWhiteSpace(decodedAppName)) + var query = req.Query; + var appName = query["appName"]; + if (string.IsNullOrWhiteSpace(appName)) { return await req.BadRequestAsync("appName is required"); } - var query = req.Query; var scopedTenantId = query["tenantId"]; if (!AppsAnalyticsHelper.IsValidOptionalTenantIdQueryParam(scopedTenantId)) { @@ -68,11 +67,11 @@ namespace AutopilotMonitor.Functions.Functions.Apps _logger.LogInformation( "Global apps/{App}/sessions requested (user: {User}, tenantId: {TenantId})", - decodedAppName, userEmail, scopedTenantId ?? ""); + appName, userEmail, scopedTenantId ?? ""); var summaries = await AppsAnalyticsHelper.LoadSummariesAsync(_metricsRepo, scopedTenantId, days); var body = await AppsAnalyticsHelper.BuildSessionsResponseAsync( - summaries, _sessionRepo, decodedAppName, source, days, + summaries, _sessionRepo, appName, source, days, statusFilter, modelFilter, versionFilter, offset, limit); var response = req.CreateResponse(HttpStatusCode.OK); diff --git a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs index 014fa74a..62c2a4b0 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs @@ -313,9 +313,11 @@ public static class EndpointAccessPolicyCatalog // (GA/delegated drill-in) over the SAME route MCP uses — no global duplicate needed. new("GET", "metrics/device-history", EndpointPolicy.MemberRead, TenantScoping.QueryParam), new("GET", "metrics/device-journeys", EndpointPolicy.MemberRead), + // Free-text names (app names, serial numbers) are query values, never {placeholders}: the host decodes + // %2F to a path separator before routing, and a placeholder matches no '/' (FreeTextNameQueryParamTests). new("GET", "apps/list", EndpointPolicy.MemberRead), - new("GET", "apps/{appName}/analytics", EndpointPolicy.MemberRead), - new("GET", "apps/{appName}/sessions", EndpointPolicy.MemberRead), + new("GET", "apps/analytics", EndpointPolicy.MemberRead), + new("GET", "apps/sessions", EndpointPolicy.MemberRead), new("GET", "metrics/usage", EndpointPolicy.MemberRead), new("GET", "metrics/sla", EndpointPolicy.MemberRead), new("GET", "metrics/rule-stats", EndpointPolicy.MemberRead), @@ -547,8 +549,8 @@ public static class EndpointAccessPolicyCatalog new("GET", "global/metrics/device-journeys", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam), new("GET", "global/metrics/verdict-calibration", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam), new("GET", "global/apps/list", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam), - new("GET", "global/apps/{appName}/analytics", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam), - new("GET", "global/apps/{appName}/sessions", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam), + new("GET", "global/apps/analytics", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam), + new("GET", "global/apps/sessions", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam), new("GET", "global/metrics/geographic", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam), new("GET", "global/metrics/geographic/sessions", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam), new("GET", "global/metrics/usage", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.QueryParam), @@ -587,7 +589,7 @@ public static class EndpointAccessPolicyCatalog new("GET", "global/devices/blocked", EndpointPolicy.GlobalReadOrAdmin), new("GET", "devices/blocked", EndpointPolicy.GlobalReadOrAdmin), new("POST", "devices/block", EndpointPolicy.GlobalAdminOnly), - new("DELETE", "devices/block/{encodedSerialNumber}", EndpointPolicy.GlobalAdminOnly), + new("DELETE", "devices/block", EndpointPolicy.GlobalAdminOnly), new("GET", "versions/blocked", EndpointPolicy.GlobalReadOrAdmin), new("POST", "versions/block", EndpointPolicy.GlobalAdminOnly), new("DELETE", "versions/block/{encodedPattern}", EndpointPolicy.GlobalAdminOnly), diff --git a/src/McpServer/autopilot-monitor-mcp/src/generated/wire-types.generated.ts b/src/McpServer/autopilot-monitor-mcp/src/generated/wire-types.generated.ts index 7e07d249..d924de06 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/generated/wire-types.generated.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/generated/wire-types.generated.ts @@ -446,7 +446,7 @@ export interface AppAnalyticsFailureCode { sampleMessage: string; } -/** Response of GET apps/{appName}/analytics and its global variant. */ +/** Response of GET apps/analytics?appName= and its global variant. */ export interface AppAnalyticsResponse { success: boolean; appName: string; @@ -641,7 +641,7 @@ export interface AppSessionItem { installPassCount: number; } -/** Response of GET apps/{appName}/sessions and its global variant (offset-paged). */ +/** Response of GET apps/sessions?appName= and its global variant (offset-paged). */ export interface AppSessionsResponse { success: boolean; total: number; diff --git a/src/Shared/AutopilotMonitor.Shared/Models/Metrics/AppsDashboardApiModels.cs b/src/Shared/AutopilotMonitor.Shared/Models/Metrics/AppsDashboardApiModels.cs index f33845fe..16f67e81 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/Metrics/AppsDashboardApiModels.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/Metrics/AppsDashboardApiModels.cs @@ -3,8 +3,8 @@ using System.Collections.Generic; namespace AutopilotMonitor.Shared.Models { - // Declaration order == wire order. App Dashboard endpoints (apps/list, apps/{app}/analytics, - // apps/{app}/sessions and their global variants) — lifted from the Functions-local + // Declaration order == wire order. App Dashboard endpoints (apps/list, apps/analytics, + // apps/sessions and their global variants) — lifted from the Functions-local // anonymous builders in AppsAnalyticsHelper so the manifest exports them. /// @@ -54,7 +54,7 @@ namespace AutopilotMonitor.Shared.Models public DateTime LastSeenAt { get; set; } } - /// Response of GET apps/{appName}/analytics and its global variant. + /// Response of GET apps/analytics?appName= and its global variant. public class AppAnalyticsResponse : IApiResponse { public bool Success { get; set; } @@ -151,7 +151,7 @@ namespace AutopilotMonitor.Shared.Models public double LiftVsBaseline { get; set; } } - /// Response of GET apps/{appName}/sessions and its global variant (offset-paged). + /// Response of GET apps/sessions?appName= and its global variant (offset-paged). public class AppSessionsResponse : IApiResponse { public bool Success { get; set; } diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/apiPathEncoding.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/apiPathEncoding.test.ts index bf6c0b2d..35039aac 100644 --- a/src/Web/autopilot-monitor-web/lib/__tests__/apiPathEncoding.test.ts +++ b/src/Web/autopilot-monitor-web/lib/__tests__/apiPathEncoding.test.ts @@ -61,6 +61,17 @@ describe("api path ids are percent-encoded", () => { expect(api.sessions.watch(HOSTILE, "t")).toBe(`https://test.example/api/sessions/${ENCODED}/watch?tenantId=t`); }); + it("free-text names (app names, serials) travel as query values", async () => { + const { api } = await apiPromise; + const unblock = new URL(api.devices.unblock("SN/1+2%", "t")); + expect(unblock.pathname).toBe("/api/devices/block"); + expect(unblock.searchParams.get("serialNumber")).toBe("SN/1+2%"); + expect(unblock.searchParams.get("tenantId")).toBe("t"); + const analytics = new URL(api.apps.globalAnalytics("a/b", 30)); + expect(analytics.pathname).toBe("/api/global/apps/analytics"); + expect(analytics.searchParams.get("appName")).toBe("a/b"); + }); + it("no path builder lets an id escape its segment", async () => { const { api } = await apiPromise; type Builder = (...args: string[]) => string; diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/scopedApi.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/scopedApi.test.ts index c8d72a61..a0678161 100644 --- a/src/Web/autopilot-monitor-web/lib/__tests__/scopedApi.test.ts +++ b/src/Web/autopilot-monitor-web/lib/__tests__/scopedApi.test.ts @@ -84,7 +84,7 @@ describe("scopedApi routing", () => { const t = scopedApi.appSessions(tenantMode, "My App", 7, "failed", 20, 10); const g = scopedApi.appSessions(override, "My App", 7, "failed", 20, 10); for (const url of [t, g]) { - expect(url).toContain("My%20App"); + expect(url).toContain("appName=My+App"); expect(url).toContain("days=7"); expect(url).toContain("status=failed"); expect(url).toContain("offset=20"); @@ -93,6 +93,19 @@ describe("scopedApi routing", () => { expect(g).toContain(`tenantId=${OTHER}`); }); + it("per-app URLs carry the name as a query value, never in the path", () => { + // The host decodes %2F to a path separator before routing, so a name in the path 404s. + for (const name of ["a/b", "100%", "x+y", "a%20b"]) { + for (const sel of [tenantMode, override]) { + for (const url of [scopedApi.appAnalytics(sel, name, 30), scopedApi.appSessions(sel, name, 30, "all", 0, 10)]) { + const parsed = new URL(url); + expect(parsed.pathname).toMatch(/^\/api\/(global\/)?apps\/(analytics|sessions)$/); + expect(parsed.searchParams.get("appName")).toBe(name); + } + } + } + }); + it("per-app URLs carry a non-Intune install channel and leave the Intune default out", () => { for (const sel of [tenantMode, override]) { expect(scopedApi.appAnalytics(sel, "7-Zip", 30, { source: "realmjoin" })).toContain("source=realmjoin"); diff --git a/src/Web/autopilot-monitor-web/lib/api.ts b/src/Web/autopilot-monitor-web/lib/api.ts index 7b653424..42685139 100644 --- a/src/Web/autopilot-monitor-web/lib/api.ts +++ b/src/Web/autopilot-monitor-web/lib/api.ts @@ -249,8 +249,9 @@ export const api = { blocked: (tenantId: string) => `${API_BASE_URL}/api/devices/blocked${qs({ tenantId })}`, block: () => `${API_BASE_URL}/api/devices/block`, + // Serials may contain '/': a query value, never a path segment (the host decodes %2F before routing). unblock: (serialNumber: string, tenantId: string) => - `${API_BASE_URL}/api/devices/block/${encodeURIComponent(serialNumber)}${qs({ tenantId })}`, + `${API_BASE_URL}/api/devices/block${qs({ serialNumber, tenantId })}`, allBlocked: () => `${API_BASE_URL}/api/global/devices/blocked`, }, @@ -372,12 +373,14 @@ export const api = { }, // ── Apps Dashboard ──────────────────────────────────────────────────────── + // App names may contain '/': the name is a query value, never a path segment (the host decodes + // %2F before routing). apps: { list: (tenantId: string, days: number) => `${API_BASE_URL}/api/apps/list${qs({ tenantId, days: String(days) })}`, // source = install channel ("realmjoin"); the Intune default is left out of the URL. analytics: (tenantId: string, appName: string, days: number, opts?: { source?: string }) => - `${API_BASE_URL}/api/apps/${encodeURIComponent(appName)}/analytics${qs({ tenantId, days: String(days), source: appInstallSourceParam(opts?.source) })}`, + `${API_BASE_URL}/api/apps/analytics${qs({ appName, tenantId, days: String(days), source: appInstallSourceParam(opts?.source) })}`, // model/version live in an options object: as trailing positional strings they sat // adjacent to other optional strings, so a forgotten argument silently shifted the // next one into its place (compiles fine, queries the wrong thing). @@ -390,7 +393,8 @@ export const api = { limit = 50, opts?: { model?: string; version?: string; source?: string } ) => - `${API_BASE_URL}/api/apps/${encodeURIComponent(appName)}/sessions${qs({ + `${API_BASE_URL}/api/apps/sessions${qs({ + appName, tenantId, days: String(days), status, @@ -407,7 +411,7 @@ export const api = { globalList: (days: number, tenantId?: string) => `${API_BASE_URL}/api/global/apps/list${qs({ days: String(days), tenantId })}`, globalAnalytics: (appName: string, days: number, tenantId?: string, opts?: { source?: string }) => - `${API_BASE_URL}/api/global/apps/${encodeURIComponent(appName)}/analytics${qs({ days: String(days), tenantId, source: appInstallSourceParam(opts?.source) })}`, + `${API_BASE_URL}/api/global/apps/analytics${qs({ appName, days: String(days), tenantId, source: appInstallSourceParam(opts?.source) })}`, globalSessions: ( appName: string, days: number, @@ -417,7 +421,8 @@ export const api = { tenantId?: string, opts?: { model?: string; version?: string; source?: string } ) => - `${API_BASE_URL}/api/global/apps/${encodeURIComponent(appName)}/sessions${qs({ + `${API_BASE_URL}/api/global/apps/sessions${qs({ + appName, days: String(days), status, offset: String(offset), diff --git a/src/Web/autopilot-monitor-web/utils/shared-manifests.json b/src/Web/autopilot-monitor-web/utils/shared-manifests.json index 1bf86f6d..00823d5b 100644 --- a/src/Web/autopilot-monitor-web/utils/shared-manifests.json +++ b/src/Web/autopilot-monitor-web/utils/shared-manifests.json @@ -2826,7 +2826,7 @@ }, "AppAnalyticsResponse": { "kind": "object", - "doc": "Response of GET apps/{appName}/analytics and its global variant.", + "doc": "Response of GET apps/analytics?appName= and its global variant.", "fields": [ { "name": "success", @@ -3774,7 +3774,7 @@ }, "AppSessionsResponse": { "kind": "object", - "doc": "Response of GET apps/{appName}/sessions and its global variant (offset-paged).", + "doc": "Response of GET apps/sessions?appName= and its global variant (offset-paged).", "fields": [ { "name": "success", diff --git a/src/Web/autopilot-monitor-web/utils/wire-types.generated.ts b/src/Web/autopilot-monitor-web/utils/wire-types.generated.ts index 0e541652..553fd5f6 100644 --- a/src/Web/autopilot-monitor-web/utils/wire-types.generated.ts +++ b/src/Web/autopilot-monitor-web/utils/wire-types.generated.ts @@ -446,7 +446,7 @@ export interface AppAnalyticsFailureCode { sampleMessage: string; } -/** Response of GET apps/{appName}/analytics and its global variant. */ +/** Response of GET apps/analytics?appName= and its global variant. */ export interface AppAnalyticsResponse { success: boolean; appName: string; @@ -641,7 +641,7 @@ export interface AppSessionItem { installPassCount: number; } -/** Response of GET apps/{appName}/sessions and its global variant (offset-paged). */ +/** Response of GET apps/sessions?appName= and its global variant (offset-paged). */ export interface AppSessionsResponse { success: boolean; total: number;