From 527332a3acdc16452b24fb89da11656cd342c61f Mon Sep 17 00:00:00 2001 From: Timothy Date: Tue, 30 Jun 2026 18:49:35 +0200 Subject: [PATCH] feat(api): default-deny mutating API writes Refs #43 --- .../Controllers/ApiControllerSecurityTests.cs | 122 ++++++++++++++++++ .../ChannelControllerSecurityTests.cs | 62 --------- .../CollectionControllerSecurityTests.cs | 60 --------- .../FFmpegProfileControllerTests.cs | 11 -- .../Controllers/PlayoutControllerTests.cs | 11 -- .../Controllers/ScheduleControllerTests.cs | 11 -- .../SmartCollectionControllerTests.cs | 11 -- .../Filters/ApiKeyAuthorizationFilterTests.cs | 36 +++++- ErsatzTV/Controllers/Api/ChannelController.cs | 6 - .../Controllers/Api/CollectionController.cs | 2 - .../Api/FFmpegProfileController.cs | 2 - .../Controllers/Api/LibrariesController.cs | 2 - .../Controllers/Api/MaintenanceController.cs | 2 - ErsatzTV/Controllers/Api/PlayoutController.cs | 2 - ErsatzTV/Controllers/Api/ScannerController.cs | 2 + .../Controllers/Api/ScheduleController.cs | 2 - .../Api/SmartCollectionController.cs | 2 - ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs | 33 ++++- .../SkipApiKeyAuthorizationAttribute.cs | 9 ++ ErsatzTV/Startup.cs | 1 + docs/rest-api.md | 10 +- 21 files changed, 205 insertions(+), 194 deletions(-) create mode 100644 ErsatzTV.Tests/Controllers/ApiControllerSecurityTests.cs delete mode 100644 ErsatzTV.Tests/Controllers/ChannelControllerSecurityTests.cs delete mode 100644 ErsatzTV.Tests/Controllers/CollectionControllerSecurityTests.cs create mode 100644 ErsatzTV/Filters/SkipApiKeyAuthorizationAttribute.cs diff --git a/ErsatzTV.Tests/Controllers/ApiControllerSecurityTests.cs b/ErsatzTV.Tests/Controllers/ApiControllerSecurityTests.cs new file mode 100644 index 000000000..4f00c1279 --- /dev/null +++ b/ErsatzTV.Tests/Controllers/ApiControllerSecurityTests.cs @@ -0,0 +1,122 @@ +using System.Reflection; +using ErsatzTV; +using ErsatzTV.Controllers.Api; +using ErsatzTV.Filters; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Routing; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.FileProviders; +using Microsoft.Extensions.Options; +using NSubstitute; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Controllers; + +[TestFixture] +public class ApiControllerSecurityTests +{ + private static readonly bool ApiKeyAuthorizationFilterIsGlobal = IsApiKeyAuthorizationFilterRegisteredGlobally(); + + [Test] + public void Every_Mutating_Api_Action_Should_Be_Globally_Protected_Or_Explicitly_Exempt() + { + Type[] apiControllers = + [ + typeof(ChannelController), + typeof(CollectionController), + typeof(FFmpegProfileController), + typeof(LibrariesController), + typeof(MaintenanceController), + typeof(PlayoutController), + typeof(ScannerController), + typeof(ScheduleController), + typeof(ScriptedScheduleController), + typeof(SessionController), + typeof(SmartCollectionController) + ]; + + foreach (Type controllerType in apiControllers) + { + bool controllerSkipsApiKey = controllerType + .GetCustomAttributes(inherit: true) + .Any(); + + foreach (MethodInfo action in controllerType + .GetMethods(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly)) + { + bool isMutating = action + .GetCustomAttributes(inherit: true) + .SelectMany(a => a.HttpMethods) + .Any(m => m is "POST" or "PUT" or "PATCH" or "DELETE"); + + if (!isMutating) + { + continue; + } + + bool actionSkipsApiKey = action + .GetCustomAttributes(inherit: true) + .Any(); + + (controllerSkipsApiKey || actionSkipsApiKey || IsGloballyProtected()) + .ShouldBeTrue($"{controllerType.Name}.{action.Name} must be covered by global API write auth or explicitly exempt"); + } + } + } + + [Test] + public void ScannerController_Should_Be_Only_Api_Key_Exempt_Api_Controller() + { + Type[] exemptControllers = typeof(ScannerController) + .Assembly + .GetTypes() + .Where(t => t.Namespace == typeof(ScannerController).Namespace) + .Where(t => t.GetCustomAttributes(inherit: true).Any()) + .Where(t => t.GetCustomAttributes(inherit: true).Any()) + .ToArray(); + + exemptControllers.ShouldBe([typeof(ScannerController)]); + } + + [Test] + public void Startup_Should_Register_ApiKeyAuthorizationFilter_Globally() + { + ApiKeyAuthorizationFilterIsGlobal.ShouldBeTrue(); + } + + private static bool IsGloballyProtected() => ApiKeyAuthorizationFilterIsGlobal; + + private static bool IsApiKeyAuthorizationFilterRegisteredGlobally() + { + var settings = new Dictionary + { + ["provider"] = "sqlite", + ["ConnectionStrings:Data"] = "Data Source=:memory:" + }; + + IConfiguration configuration = new ConfigurationBuilder() + .AddInMemoryCollection(settings) + .Build(); + + var environment = Substitute.For(); + environment.ApplicationName.Returns("ErsatzTV"); + environment.EnvironmentName.Returns("Development"); + environment.ContentRootPath.Returns(TestContext.CurrentContext.TestDirectory); + environment.WebRootPath.Returns(TestContext.CurrentContext.TestDirectory); + environment.ContentRootFileProvider.Returns(new NullFileProvider()); + environment.WebRootFileProvider.Returns(new NullFileProvider()); + + var services = new ServiceCollection(); + new Startup(configuration, environment).ConfigureServices(services); + + using ServiceProvider provider = services.BuildServiceProvider(); + MvcOptions options = provider.GetRequiredService>().Value; + + return options.Filters + .OfType() + .Any(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); + } +} diff --git a/ErsatzTV.Tests/Controllers/ChannelControllerSecurityTests.cs b/ErsatzTV.Tests/Controllers/ChannelControllerSecurityTests.cs deleted file mode 100644 index 444051220..000000000 --- a/ErsatzTV.Tests/Controllers/ChannelControllerSecurityTests.cs +++ /dev/null @@ -1,62 +0,0 @@ -using System.Linq; -using System.Reflection; -using ErsatzTV.Controllers.Api; -using ErsatzTV.Filters; -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.Routing; -using NUnit.Framework; -using Shouldly; - -namespace ErsatzTV.Tests.Controllers; - -/// -/// Guards the API-key write-path contract for : every mutating -/// action (POST/PUT/PATCH/DELETE) must be covered by . -/// The filter is applied at the controller level, so this also protects any future write endpoint -/// added to the controller (regression net for the ResetPlayout bypass). -/// -[TestFixture] -public class ChannelControllerSecurityTests -{ - [Test] - public void Controller_Should_Apply_ApiKeyAuthorizationFilter() - { - ServiceFilterAttribute? filter = typeof(ChannelController) - .GetCustomAttributes(inherit: true) - .SingleOrDefault(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); - - filter.ShouldNotBeNull("ChannelController must carry the ApiKeyAuthorizationFilter at the class level"); - } - - [Test] - public void Every_Mutating_Action_Should_Be_Protected() - { - MethodInfo[] actions = typeof(ChannelController) - .GetMethods(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly); - - bool controllerHasFilter = typeof(ChannelController) - .GetCustomAttributes(inherit: true) - .Any(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); - - foreach (MethodInfo action in actions) - { - bool isMutating = action - .GetCustomAttributes(inherit: true) - .SelectMany(a => a.HttpMethods) - .Any(m => - m is "POST" or "PUT" or "PATCH" or "DELETE"); - - if (!isMutating) - { - continue; - } - - bool actionHasFilter = action - .GetCustomAttributes(inherit: true) - .Any(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); - - (controllerHasFilter || actionHasFilter) - .ShouldBeTrue($"Mutating action {action.Name} is not protected by ApiKeyAuthorizationFilter"); - } - } -} diff --git a/ErsatzTV.Tests/Controllers/CollectionControllerSecurityTests.cs b/ErsatzTV.Tests/Controllers/CollectionControllerSecurityTests.cs deleted file mode 100644 index 73555eeff..000000000 --- a/ErsatzTV.Tests/Controllers/CollectionControllerSecurityTests.cs +++ /dev/null @@ -1,60 +0,0 @@ -using System.Reflection; -using ErsatzTV.Controllers.Api; -using ErsatzTV.Filters; -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.Routing; -using NUnit.Framework; -using Shouldly; - -namespace ErsatzTV.Tests.Controllers; - -[TestFixture] -public class CollectionControllerSecurityTests -{ - [TestCase(typeof(CollectionController))] - [TestCase(typeof(LibrariesController))] - [TestCase(typeof(MaintenanceController))] - [TestCase(typeof(SmartCollectionController))] - public void Controller_Should_Apply_ApiKeyAuthorizationFilter(Type controllerType) - { - ServiceFilterAttribute? filter = controllerType - .GetCustomAttributes(inherit: true) - .SingleOrDefault(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); - - filter.ShouldNotBeNull($"{controllerType.Name} must carry ApiKeyAuthorizationFilter at the class level"); - } - - [TestCase(typeof(CollectionController))] - [TestCase(typeof(LibrariesController))] - [TestCase(typeof(MaintenanceController))] - [TestCase(typeof(SmartCollectionController))] - public void Every_Mutating_Action_Should_Be_Protected(Type controllerType) - { - MethodInfo[] actions = controllerType - .GetMethods(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly); - - bool controllerHasFilter = controllerType - .GetCustomAttributes(inherit: true) - .Any(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); - - foreach (MethodInfo action in actions) - { - bool isMutating = action - .GetCustomAttributes(inherit: true) - .SelectMany(a => a.HttpMethods) - .Any(m => m is "POST" or "PUT" or "PATCH" or "DELETE"); - - if (!isMutating) - { - continue; - } - - bool actionHasFilter = action - .GetCustomAttributes(inherit: true) - .Any(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); - - (controllerHasFilter || actionHasFilter) - .ShouldBeTrue($"Mutating action {controllerType.Name}.{action.Name} is not protected"); - } - } -} diff --git a/ErsatzTV.Tests/Controllers/FFmpegProfileControllerTests.cs b/ErsatzTV.Tests/Controllers/FFmpegProfileControllerTests.cs index 6fd2111d2..657815b91 100644 --- a/ErsatzTV.Tests/Controllers/FFmpegProfileControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/FFmpegProfileControllerTests.cs @@ -7,7 +7,6 @@ using ErsatzTV.Core.Api.FFmpegProfiles; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Errors; using ErsatzTV.Core.FFmpeg; -using ErsatzTV.Filters; using LanguageExt; using MediatR; using Microsoft.AspNetCore.Mvc; @@ -43,16 +42,6 @@ public class FFmpegProfileControllerTests ShouldHaveActionRoute("DELETE", "/api/ffmpeg/profiles/{id:int}"); } - [Test] - public void Controller_Should_Apply_ApiKeyAuthorizationFilter() - { - ServiceFilterAttribute? filter = typeof(FFmpegProfileController) - .GetCustomAttributes(inherit: true) - .SingleOrDefault(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); - - filter.ShouldNotBeNull("FFmpegProfileController must carry ApiKeyAuthorizationFilter at the class level"); - } - [Test] public void Mutations_Should_Use_Request_Dtos_For_Wire_Contract() { diff --git a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs index 7125d03dd..0726e9a6f 100644 --- a/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/PlayoutControllerTests.cs @@ -6,7 +6,6 @@ using ErsatzTV.Core; using ErsatzTV.Core.Api.Playouts; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Errors; -using ErsatzTV.Filters; using LanguageExt; using MediatR; using Microsoft.AspNetCore.Mvc; @@ -40,16 +39,6 @@ public class PlayoutControllerTests ShouldHaveActionRoute(nameof(PlayoutController.Delete), "DELETE", "/api/playouts/{id:int}"); } - [Test] - public void Controller_Should_Apply_ApiKeyAuthorizationFilter() - { - ServiceFilterAttribute? filter = typeof(PlayoutController) - .GetCustomAttributes(inherit: true) - .SingleOrDefault(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); - - filter.ShouldNotBeNull("PlayoutController must carry ApiKeyAuthorizationFilter at the class level"); - } - [Test] public void Create_Should_Use_Stable_Request_Dto() { diff --git a/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs b/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs index 4c0c11d36..151674a4d 100644 --- a/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/ScheduleControllerTests.cs @@ -6,7 +6,6 @@ using ErsatzTV.Core; using ErsatzTV.Core.Domain; using ErsatzTV.Core.Errors; using ErsatzTV.Core.Scheduling; -using ErsatzTV.Filters; using LanguageExt; using MediatR; using Microsoft.AspNetCore.Mvc; @@ -49,16 +48,6 @@ public class ScheduleControllerTests "/api/schedules/{id:int}/items/{itemId:int}"); } - [Test] - public void Controller_Should_Apply_ApiKeyAuthorizationFilter() - { - ServiceFilterAttribute? filter = typeof(ScheduleController) - .GetCustomAttributes(inherit: true) - .SingleOrDefault(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); - - filter.ShouldNotBeNull("ScheduleController must carry ApiKeyAuthorizationFilter at the class level"); - } - [Test] public void Update_Should_Use_Stable_Update_Request_Dto() { diff --git a/ErsatzTV.Tests/Controllers/SmartCollectionControllerTests.cs b/ErsatzTV.Tests/Controllers/SmartCollectionControllerTests.cs index 85a887861..860f4da88 100644 --- a/ErsatzTV.Tests/Controllers/SmartCollectionControllerTests.cs +++ b/ErsatzTV.Tests/Controllers/SmartCollectionControllerTests.cs @@ -5,7 +5,6 @@ using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.SmartCollections; using ErsatzTV.Core.Errors; -using ErsatzTV.Filters; using LanguageExt; using MediatR; using Microsoft.AspNetCore.Mvc; @@ -41,16 +40,6 @@ public class SmartCollectionControllerTests ShouldHaveActionRoute(nameof(SmartCollectionController.Delete), "DELETE", "/api/smart-collections/{id:int}"); } - [Test] - public void Controller_Should_Apply_ApiKeyAuthorizationFilter() - { - ServiceFilterAttribute? filter = typeof(SmartCollectionController) - .GetCustomAttributes(inherit: true) - .SingleOrDefault(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); - - filter.ShouldNotBeNull("SmartCollectionController must carry ApiKeyAuthorizationFilter at the class level"); - } - [Test] public async Task Create_Should_Return_201_With_Location_And_Body() { diff --git a/ErsatzTV.Tests/Filters/ApiKeyAuthorizationFilterTests.cs b/ErsatzTV.Tests/Filters/ApiKeyAuthorizationFilterTests.cs index 9ef312e11..3e04cd1fe 100644 --- a/ErsatzTV.Tests/Filters/ApiKeyAuthorizationFilterTests.cs +++ b/ErsatzTV.Tests/Filters/ApiKeyAuthorizationFilterTests.cs @@ -14,16 +14,27 @@ namespace ErsatzTV.Tests.Filters; [TestFixture] public class ApiKeyAuthorizationFilterTests { - private static AuthorizationFilterContext MakeContext(string method, string? apiKeyHeader) + private static AuthorizationFilterContext MakeContext( + string method, + string? apiKeyHeader, + string path = "/api/channels", + bool skipApiKeyAuthorization = false) { var httpContext = new DefaultHttpContext(); httpContext.Request.Method = method; + httpContext.Request.Path = path; if (apiKeyHeader is not null) { httpContext.Request.Headers[ApiKeyAuthorizationFilter.HeaderName] = apiKeyHeader; } - var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor()); + var actionDescriptor = new ActionDescriptor(); + if (skipApiKeyAuthorization) + { + actionDescriptor.EndpointMetadata = [new SkipApiKeyAuthorizationAttribute()]; + } + + var actionContext = new ActionContext(httpContext, new RouteData(), actionDescriptor); return new AuthorizationFilterContext(actionContext, new List()); } @@ -92,4 +103,25 @@ public class ApiKeyAuthorizationFilterTests MakeFilter(configuredKey: "secret").OnAuthorization(context); context.Result.ShouldBeNull(); } + + [Test] + public void Should_Allow_Non_Api_Mutating_Request_Even_When_Key_Configured() + { + AuthorizationFilterContext context = MakeContext("POST", apiKeyHeader: null, path: "/iptv/channels.m3u"); + MakeFilter(configuredKey: "secret").OnAuthorization(context); + context.Result.ShouldBeNull(); + } + + [Test] + public void Should_Allow_Api_Mutating_Request_When_Endpoint_Skips_Api_Key_Authorization() + { + AuthorizationFilterContext context = MakeContext( + "POST", + apiKeyHeader: null, + path: "/api/scan/0f8fad5b-d9cb-469f-a165-70867728950e/progress", + skipApiKeyAuthorization: true); + + MakeFilter(configuredKey: "secret").OnAuthorization(context); + context.Result.ShouldBeNull(); + } } diff --git a/ErsatzTV/Controllers/Api/ChannelController.cs b/ErsatzTV/Controllers/Api/ChannelController.cs index c31682c83..6220adfa5 100644 --- a/ErsatzTV/Controllers/Api/ChannelController.cs +++ b/ErsatzTV/Controllers/Api/ChannelController.cs @@ -8,7 +8,6 @@ using ErsatzTV.Core; using ErsatzTV.Core.Api.Channels; using ErsatzTV.Core.Scheduling; using ErsatzTV.Extensions; -using ErsatzTV.Filters; using MediatR; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -16,11 +15,6 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -// Apply the optional API-key control at the controller level so that EVERY mutating action -// (including future ones) is covered by default; the filter no-ops on read methods (GET) and -// when Api:WriteKey is unset, preserving the open LAN behavior. This is fail-safe: a developer -// adding a new write endpoint here cannot accidentally leave it unauthenticated. -[ServiceFilter(typeof(ApiKeyAuthorizationFilter))] public class ChannelController(ChannelWriter workerChannel, IMediator mediator) { [HttpGet("/api/channels")] diff --git a/ErsatzTV/Controllers/Api/CollectionController.cs b/ErsatzTV/Controllers/Api/CollectionController.cs index b4e1871be..90c3fff48 100644 --- a/ErsatzTV/Controllers/Api/CollectionController.cs +++ b/ErsatzTV/Controllers/Api/CollectionController.cs @@ -3,7 +3,6 @@ using ErsatzTV.Application.MediaCollections; using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Extensions; -using ErsatzTV.Filters; using MediatR; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -11,7 +10,6 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -[ServiceFilter(typeof(ApiKeyAuthorizationFilter))] public class CollectionController(IMediator mediator) : ControllerBase { [HttpGet("/api/collections")] diff --git a/ErsatzTV/Controllers/Api/FFmpegProfileController.cs b/ErsatzTV/Controllers/Api/FFmpegProfileController.cs index 0e9b733fc..386967021 100644 --- a/ErsatzTV/Controllers/Api/FFmpegProfileController.cs +++ b/ErsatzTV/Controllers/Api/FFmpegProfileController.cs @@ -4,7 +4,6 @@ using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.FFmpegProfiles; using ErsatzTV.Extensions; -using ErsatzTV.Filters; using MediatR; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -12,7 +11,6 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -[ServiceFilter(typeof(ApiKeyAuthorizationFilter))] public class FFmpegProfileController(IMediator mediator) : ControllerBase { [HttpGet("/api/ffmpeg/profiles", Name = "GetFFmpegProfiles")] diff --git a/ErsatzTV/Controllers/Api/LibrariesController.cs b/ErsatzTV/Controllers/Api/LibrariesController.cs index ff76280ec..efeadd1ae 100644 --- a/ErsatzTV/Controllers/Api/LibrariesController.cs +++ b/ErsatzTV/Controllers/Api/LibrariesController.cs @@ -1,6 +1,5 @@ using ErsatzTV.Application.Libraries; using ErsatzTV.Core.Interfaces.Repositories; -using ErsatzTV.Filters; using MediatR; using Microsoft.AspNetCore.Mvc; @@ -8,7 +7,6 @@ namespace ErsatzTV.Controllers.Api; [ApiController] [EndpointGroupName("general")] -[ServiceFilter(typeof(ApiKeyAuthorizationFilter))] public class LibrariesController(ITelevisionRepository televisionRepository, IMediator mediator) { [HttpPost("/api/libraries/{id:int}/scan")] diff --git a/ErsatzTV/Controllers/Api/MaintenanceController.cs b/ErsatzTV/Controllers/Api/MaintenanceController.cs index 26977cbac..b6009e32a 100644 --- a/ErsatzTV/Controllers/Api/MaintenanceController.cs +++ b/ErsatzTV/Controllers/Api/MaintenanceController.cs @@ -2,7 +2,6 @@ using System.Threading.Channels; using ErsatzTV.Application; using ErsatzTV.Application.Maintenance; using ErsatzTV.Core; -using ErsatzTV.Filters; using MediatR; using Microsoft.AspNetCore.Mvc; @@ -10,7 +9,6 @@ namespace ErsatzTV.Controllers.Api; [ApiController] [EndpointGroupName("general")] -[ServiceFilter(typeof(ApiKeyAuthorizationFilter))] public class MaintenanceController(IMediator mediator, ChannelWriter workerChannel) { [HttpGet("/api/maintenance/gc")] diff --git a/ErsatzTV/Controllers/Api/PlayoutController.cs b/ErsatzTV/Controllers/Api/PlayoutController.cs index 366257cd0..833ce778b 100644 --- a/ErsatzTV/Controllers/Api/PlayoutController.cs +++ b/ErsatzTV/Controllers/Api/PlayoutController.cs @@ -4,7 +4,6 @@ using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.Playouts; using ErsatzTV.Extensions; -using ErsatzTV.Filters; using MediatR; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -12,7 +11,6 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -[ServiceFilter(typeof(ApiKeyAuthorizationFilter))] public class PlayoutController(IMediator mediator) : ControllerBase { [HttpGet("/api/playouts/{id:int}", Name = "GetPlayoutById")] diff --git a/ErsatzTV/Controllers/Api/ScannerController.cs b/ErsatzTV/Controllers/Api/ScannerController.cs index 8c8404160..fb337cc21 100644 --- a/ErsatzTV/Controllers/Api/ScannerController.cs +++ b/ErsatzTV/Controllers/Api/ScannerController.cs @@ -2,12 +2,14 @@ using System.Threading.Channels; using ErsatzTV.Application; using ErsatzTV.Application.Search; using ErsatzTV.Core.Interfaces.Metadata; +using ErsatzTV.Filters; using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] [ApiExplorerSettings(IgnoreApi = true)] +[SkipApiKeyAuthorization] [Route("api/scan/{scanId:guid}")] public class ScannerController( IScannerProxyService scannerProxyService, diff --git a/ErsatzTV/Controllers/Api/ScheduleController.cs b/ErsatzTV/Controllers/Api/ScheduleController.cs index b66f2f3ce..7592d3562 100644 --- a/ErsatzTV/Controllers/Api/ScheduleController.cs +++ b/ErsatzTV/Controllers/Api/ScheduleController.cs @@ -3,7 +3,6 @@ using ErsatzTV.Application.ProgramSchedules; using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Extensions; -using ErsatzTV.Filters; using MediatR; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -11,7 +10,6 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -[ServiceFilter(typeof(ApiKeyAuthorizationFilter))] public class ScheduleController(IMediator mediator) : ControllerBase { [HttpGet("/api/schedules")] diff --git a/ErsatzTV/Controllers/Api/SmartCollectionController.cs b/ErsatzTV/Controllers/Api/SmartCollectionController.cs index 794873a33..4837a4277 100644 --- a/ErsatzTV/Controllers/Api/SmartCollectionController.cs +++ b/ErsatzTV/Controllers/Api/SmartCollectionController.cs @@ -4,7 +4,6 @@ using ErsatzTV.Controllers.Api.Requests; using ErsatzTV.Core; using ErsatzTV.Core.Api.SmartCollections; using ErsatzTV.Extensions; -using ErsatzTV.Filters; using MediatR; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc; @@ -12,7 +11,6 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -[ServiceFilter(typeof(ApiKeyAuthorizationFilter))] public class SmartCollectionController(IMediator mediator) : ControllerBase { [HttpGet("/api/smart-collections")] diff --git a/ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs b/ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs index e0c33c70e..8c0643d5c 100644 --- a/ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs +++ b/ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs @@ -6,12 +6,12 @@ using Microsoft.Extensions.Primitives; namespace ErsatzTV.Filters; /// -/// Optional API-key authorization for mutating JSON API endpoints (slice #2a). +/// Optional API-key authorization for mutating JSON API endpoints. /// Reads the configured key from Api:WriteKey. When that key is empty the filter is /// a no-op (preserving the current open LAN behavior); when it is set, mutating requests -/// (POST/PUT/PATCH/DELETE) must present a matching X-Api-Key header or receive 401. -/// This is fully independent of and only applies to the actions it -/// decorates — it never affects /iptv/* or any read endpoint. +/// under /api/* (POST/PUT/PATCH/DELETE) must present a matching X-Api-Key +/// header or receive 401. This is fully independent of and never +/// affects /iptv/* or any read endpoint. /// public class ApiKeyAuthorizationFilter(IConfiguration configuration) : IAuthorizationFilter { @@ -20,6 +20,11 @@ public class ApiKeyAuthorizationFilter(IConfiguration configuration) : IAuthoriz public void OnAuthorization(AuthorizationFilterContext context) { + if (ShouldSkipApiKeyAuthorization(context)) + { + return; + } + string configuredKey = configuration[ConfigurationKey]; // empty key => API-key auth disabled, endpoint is open @@ -50,4 +55,24 @@ public class ApiKeyAuthorizationFilter(IConfiguration configuration) : IAuthoriz }); } } + + private static bool ShouldSkipApiKeyAuthorization(AuthorizationFilterContext context) + { + if (context.Filters.OfType().Any() + || context.ActionDescriptor.EndpointMetadata.OfType().Any()) + { + return true; + } + + if (!context.HttpContext.Request.Path.StartsWithSegments("/api", StringComparison.OrdinalIgnoreCase)) + { + return true; + } + + string method = context.HttpContext.Request.Method; + return !HttpMethods.IsPost(method) + && !HttpMethods.IsPut(method) + && !HttpMethods.IsPatch(method) + && !HttpMethods.IsDelete(method); + } } diff --git a/ErsatzTV/Filters/SkipApiKeyAuthorizationAttribute.cs b/ErsatzTV/Filters/SkipApiKeyAuthorizationAttribute.cs new file mode 100644 index 000000000..4a1f7e7ea --- /dev/null +++ b/ErsatzTV/Filters/SkipApiKeyAuthorizationAttribute.cs @@ -0,0 +1,9 @@ +using Microsoft.AspNetCore.Mvc.Filters; + +namespace ErsatzTV.Filters; + +/// +/// Marks an internal API endpoint as exempt from global API-key write authorization. +/// +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] +public sealed class SkipApiKeyAuthorizationAttribute : Attribute, IFilterMetadata; diff --git a/ErsatzTV/Startup.cs b/ErsatzTV/Startup.cs index 0f350fc88..0e333f292 100644 --- a/ErsatzTV/Startup.cs +++ b/ErsatzTV/Startup.cs @@ -338,6 +338,7 @@ public class Startup options.OutputFormatters.Insert(0, new ChannelGuideOutputFormatter()); options.OutputFormatters.Insert(0, new DeviceXmlOutputFormatter()); options.OutputFormatters.Insert(0, new HdhrJsonOutputFormatter()); + options.Filters.AddService(); }) .AddNewtonsoftJson(opt => { diff --git a/docs/rest-api.md b/docs/rest-api.md index b6387e325..972340ff8 100644 --- a/docs/rest-api.md +++ b/docs/rest-api.md @@ -100,11 +100,17 @@ The current `.ToActionResult()` only yields 200/400/404, so we add **richer mapp Reuse handler validation. Port the page-only checks in §2.3 into handlers so the API reaches parity with the Blazor UI. No validation logic in controllers. ### 3.5 Auth — dedicated API key for writes (decoupled from IPTV) -**Mechanism:** a dedicated **API key** for mutations — a new config key (e.g. `Api__WriteKey`) checked by a filter applied **only to POST/PUT/DELETE on `/api/*`**. Enforced only when the key is configured (LAN-open default preserved); reads and all `/iptv/*` stay open. +**Mechanism:** a dedicated **API key** for mutations — `Api:WriteKey` / `Api__WriteKey`, +checked against the `X-Api-Key` request header by a global MVC filter. When the key is configured, +all mutating `/api/*` requests (`POST`/`PUT`/`PATCH`/`DELETE`) require the header; when the key is +unset, the LAN-open default is preserved. Reads and all `/iptv/*` routes stay open. The scanner +callback controller is the designed exemption because scanner child processes call +`/api/scan/{scanId}/...` without `X-Api-Key`; any future exemption must be explicit via +`[SkipApiKeyAuthorization]`. **Why not reuse the JWT scheme (important):** JWT is gated by a single global toggle, `JwtHelper.IsEnabled` ← `JWT:IssuerSigningKey` (`Startup.cs:166`). That **same toggle also gates the IPTV endpoints** (`/iptv/channels.m3u`, `/iptv/xmltv.xml`, streams — `ConditionalIptvAuthorizeFilter:18`) which **Jellyfin and Dispatcharr consume**. Enabling JWT to protect writes would force token auth onto those media feeds — and JWT tokens **expire in 1 day** (`JwtHelper.cs:27`), unsuitable for a standing tuner URL. A dedicated API key **decouples write-auth from media-consumer auth**: turning it on changes **nothing** for Jellyfin/Dispatcharr. -**Properties:** long-lived credential (no 1-day churn), fit for MCP / new-UI write clients; net-new but small (one filter + one config key); a standard machine-API pattern. +**Properties:** long-lived credential (no 1-day churn), fit for MCP / new-UI write clients; net-new but small (one global filter + one config key); a standard machine-API pattern. **Backlog:** tighten CORS for mutation routes if writes are exposed beyond LAN.