Files
ersatztv/ErsatzTV.Tests/Controllers/ApiControllerSecurityTests.cs
T
timothyandClaude Opus 4.8 ef2bd65c27
Build ErsatzTV Image / decisions.md append-only (pull_request) Successful in 10s
Build ErsatzTV Image / Docs update reminder (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 1m12s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 3m4s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m17s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 10m36s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
feat(api): #286 — mount the whole /api surface at /api/v1
Version every /api route to /api/v1 (251 controller routes + ~24 Location
headers + the scanner callback URL + the Startup request-log literal),
uniform across the machine API, auth, scanner and scripted-build surfaces.

Add ApiVersionRewriteMiddleware: a legacy unversioned /api/* request is
rewritten (NOT redirected) to /api/v1/* in-pipeline — method, body, auth
headers and query survive — carrying RFC 8594 Deprecation/Sunset headers,
so curl / the future MCP server / bookmarks keep working. An already-
versioned path passes through; a future /api/v2 is never forced to v1.

Standardize the route convention (leading-slash absolute route per method,
no class-[Route] — except the two Scanner/Scripted controllers whose ~all
actions share a parametrized {id} prefix), enforced by ApiRouteVersioningTests
(^/api/v\d+/ over the whole Controllers.Api surface; browser-nav
/auth/oidc/login is out of scope).

Regenerate v1.json (160 paths, all /api/v1)/endpoint-index/v1.d.ts; sweep 945
SPA request literals + the test mocks (regex + positional URL parsers). /api/v1
is additive-only after freeze; the legacy-rewrite shim sunsets in ~2 releases
(owner decision) with removal tracked as a Phase-3 follow-up.

Docs: decisions.md 2026-07-13, api-conventions §1/§9, rest-api/spa-conventions/
blazor-route-parity/e2e-local/domain-model.

fixes #286
refs #197

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-13 00:30:20 +02:00

165 lines
7.0 KiB
C#

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 ApiAuthorizationFilterIsGlobal = IsApiAuthorizationFilterRegisteredGlobally();
[Test]
public void Every_Mutating_Api_Action_Should_Be_Globally_Protected_Or_Explicitly_Exempt()
{
// Scan the assembly instead of hand-maintaining a list of controllers here: a
// hardcoded array silently drifts as new controllers are added (9 were missing at
// one point, including two with real mutating actions - ArtworkUploadController POST
// and ChannelTemplateController POST/PUT/DELETE - that were never actually checked).
// Note: not every controller in this namespace derives from ControllerBase (e.g.
// ChannelController, ScannerController, SessionController, MaintenanceController,
// TroubleshootController) - filter on [ApiController] + concrete class only, matching
// the pattern already used by ScannerController_Should_Be_Only_Api_Key_Exempt_Api_Controller
// below, so those controllers stay covered rather than silently dropping out of the scan.
Type[] apiControllers = typeof(CollectionController)
.Assembly
.GetTypes()
.Where(t => t.Namespace == typeof(CollectionController).Namespace)
.Where(t => t is { IsClass: true, IsAbstract: false })
.Where(t => t.GetCustomAttributes<ApiControllerAttribute>(inherit: true).Any())
.ToArray();
// Guard against the scan silently matching nothing (e.g. a namespace rename) and
// giving this test a false pass.
apiControllers.Length.ShouldBeGreaterThanOrEqualTo(20);
foreach (Type controllerType in apiControllers)
{
bool controllerSkipsApiKey = controllerType
.GetCustomAttributes<SkipApiAuthorizationAttribute>(inherit: true)
.Any();
foreach (MethodInfo action in controllerType
.GetMethods(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly))
{
bool isMutating = action
.GetCustomAttributes<HttpMethodAttribute>(inherit: true)
.SelectMany(a => a.HttpMethods)
.Any(m => m is "POST" or "PUT" or "PATCH" or "DELETE");
if (!isMutating)
{
continue;
}
bool actionSkipsApiKey = action
.GetCustomAttributes<SkipApiAuthorizationAttribute>(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 Only_Scanner_And_Auth_Controllers_Should_Be_Auth_Exempt()
{
// ScannerController: internal loopback callback, gated by [LocalhostOnly] instead of a credential.
// AuthController: the /api/v1/auth/* surface itself must be reachable before a caller is authenticated
// (config/session/login/setup) — its sensitive action (password change) self-checks the principal.
// Any OTHER [SkipApiAuthorization] controller is a fail-open hole and must be caught here.
Type[] exemptControllers = typeof(ScannerController)
.Assembly
.GetTypes()
.Where(t => t.Namespace == typeof(ScannerController).Namespace)
.Where(t => t.GetCustomAttributes<ApiControllerAttribute>(inherit: true).Any())
.Where(t => t.GetCustomAttributes<SkipApiAuthorizationAttribute>(inherit: true).Any())
.ToArray();
exemptControllers.ShouldBe([typeof(ScannerController), typeof(AuthController)], ignoreOrder: true);
}
[Test]
public void Startup_Should_Register_ApiAuthorizationFilter_Globally()
{
ApiAuthorizationFilterIsGlobal.ShouldBeTrue();
}
[Test]
public void Sensitive_Read_Controllers_Should_Require_Api_Key()
{
// These GET surfaces disclose secrets/paths or trigger work; they must stay gated even if an
// operator disables Api:RequireKeyForReads (issue #282). Asserted reflectively so the tier
// can't silently drop the attribute.
Type[] sensitiveControllers =
[
typeof(TroubleshootController),
typeof(LogsController),
typeof(SettingsController),
typeof(MaintenanceController)
];
foreach (Type controllerType in sensitiveControllers)
{
controllerType.GetCustomAttributes<RequiresAuthenticationAttribute>(inherit: true).Any()
.ShouldBeTrue($"{controllerType.Name} must carry [RequiresAuthentication]");
}
}
[Test]
public void ScannerController_Should_Be_Localhost_Only()
{
// The scanner callback is exempt from the API key (guessable scan-id GUID); a loopback check
// replaces the GUID as the real gate (issue #285).
typeof(ScannerController)
.GetCustomAttributes<LocalhostOnlyAttribute>(inherit: true)
.Any()
.ShouldBeTrue();
}
private static bool IsGloballyProtected() => ApiAuthorizationFilterIsGlobal;
private static bool IsApiAuthorizationFilterRegisteredGlobally()
{
var settings = new Dictionary<string, string?>
{
["provider"] = "sqlite",
["ConnectionStrings:Data"] = "Data Source=:memory:"
};
IConfiguration configuration = new ConfigurationBuilder()
.AddInMemoryCollection(settings)
.Build();
var environment = Substitute.For<IWebHostEnvironment>();
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<IOptions<MvcOptions>>().Value;
return options.Filters
.OfType<ServiceFilterAttribute>()
.Any(a => a.ServiceType == typeof(ApiAuthorizationFilter));
}
}