diff --git a/Directory.Packages.props b/Directory.Packages.props index 7bb487fd9..59e56e359 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -32,6 +32,7 @@ + diff --git a/ErsatzTV.Application/Auth/AuthConstants.cs b/ErsatzTV.Application/Auth/AuthConstants.cs new file mode 100644 index 000000000..19e080aa3 --- /dev/null +++ b/ErsatzTV.Application/Auth/AuthConstants.cs @@ -0,0 +1,28 @@ +namespace ErsatzTV.Application.Auth; + +/// +/// Shared constants for the browser-SPA session authentication (issue #295): the cookie scheme name, +/// the custom claim types the local-login path stamps onto the principal, and the auth-method marker +/// values. The web host (cookie OnValidatePrincipal, AuthController) and the Application +/// handlers both reference these so the claim contract has a single definition. +/// +public static class AuthConstants +{ + /// The cookie authentication scheme name shared by local login and the OIDC callback. + public const string CookieScheme = "cookie"; + + /// The OIDC challenge scheme name. + public const string OidcScheme = "oidc"; + + /// Claim type recording how the principal signed in ( / ). + public const string AuthMethodClaim = "etv:auth_method"; + + /// Claim type carrying the local admin's security stamp (checked on every request to revoke sessions). + public const string SecurityStampClaim = "etv:security_stamp"; + + public const string MethodLocal = "local"; + public const string MethodOidc = "oidc"; + + /// Minimum length for a local admin password. + public const int MinPasswordLength = 8; +} diff --git a/ErsatzTV.Application/Auth/ChangeLocalAdminPassword.cs b/ErsatzTV.Application/Auth/ChangeLocalAdminPassword.cs new file mode 100644 index 000000000..5b35f8525 --- /dev/null +++ b/ErsatzTV.Application/Auth/ChangeLocalAdminPassword.cs @@ -0,0 +1,10 @@ +using ErsatzTV.Core; + +namespace ErsatzTV.Application.Auth; + +/// +/// Changes the local admin password after verifying the current one. Rotates the security stamp so all +/// other sessions are revoked. is the signed-in principal's name. +/// +public record ChangeLocalAdminPassword(string Username, string CurrentPassword, string NewPassword) + : IRequest>; diff --git a/ErsatzTV.Application/Auth/ChangeLocalAdminPasswordHandler.cs b/ErsatzTV.Application/Auth/ChangeLocalAdminPasswordHandler.cs new file mode 100644 index 000000000..fe96c5472 --- /dev/null +++ b/ErsatzTV.Application/Auth/ChangeLocalAdminPasswordHandler.cs @@ -0,0 +1,68 @@ +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Infrastructure.Data; +using Microsoft.EntityFrameworkCore; + +namespace ErsatzTV.Application.Auth; + +public class ChangeLocalAdminPasswordHandler( + IDbContextFactory dbContextFactory, + ILocalPasswordHasher passwordHasher) + : IRequestHandler> +{ + public async Task> Handle( + ChangeLocalAdminPassword request, + CancellationToken cancellationToken) + { + foreach (BaseError error in LocalAdminHelpers.ValidatePassword(request.NewPassword)) + { + return error; + } + + await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); + + List rows = await dbContext.ConfigElements + .Where(c => c.Key == ConfigElementKey.AuthLocalAdminUsername.Key + || c.Key == ConfigElementKey.AuthLocalAdminPasswordHash.Key + || c.Key == ConfigElementKey.AuthSecurityStamp.Key) + .ToListAsync(cancellationToken); + + ConfigElement userRow = rows.Find(r => r.Key == ConfigElementKey.AuthLocalAdminUsername.Key); + ConfigElement hashRow = rows.Find(r => r.Key == ConfigElementKey.AuthLocalAdminPasswordHash.Key); + ConfigElement stampRow = rows.Find(r => r.Key == ConfigElementKey.AuthSecurityStamp.Key); + + if (hashRow is null) + { + return BaseError.New("No local administrator is configured"); + } + + string username = (request.Username ?? string.Empty).Trim(); + bool userMatches = userRow is not null + && string.Equals(userRow.Value, username, StringComparison.OrdinalIgnoreCase); + + LocalPasswordVerification result = + passwordHasher.Verify(hashRow.Value, request.CurrentPassword ?? string.Empty); + + if (!userMatches || result == LocalPasswordVerification.Failed) + { + return BaseError.New("Current password is incorrect"); + } + + // Atomic: the new hash and rotated stamp commit together, so a crash can't leave the new password + // active with the old stamp still authorizing revoked sessions. + string stamp = LocalAdminHelpers.NewSecurityStamp(); + hashRow.Value = passwordHasher.Hash(request.NewPassword); + if (stampRow is null) + { + dbContext.ConfigElements.Add(new ConfigElement { Key = ConfigElementKey.AuthSecurityStamp.Key, Value = stamp }); + } + else + { + stampRow.Value = stamp; + } + + await dbContext.SaveChangesAsync(cancellationToken); + + return new LocalAdminPrincipal(userRow.Value, stamp); + } +} diff --git a/ErsatzTV.Application/Auth/ClaimLocalAdmin.cs b/ErsatzTV.Application/Auth/ClaimLocalAdmin.cs new file mode 100644 index 000000000..d35a3d57b --- /dev/null +++ b/ErsatzTV.Application/Auth/ClaimLocalAdmin.cs @@ -0,0 +1,9 @@ +using ErsatzTV.Core; + +namespace ErsatzTV.Application.Auth; + +/// +/// First-run setup-claim: creates the single local administrator. Fails if one already exists +/// (first-claim-wins), so a later anonymous call cannot take over the account. +/// +public record ClaimLocalAdmin(string Username, string Password) : IRequest>; diff --git a/ErsatzTV.Application/Auth/ClaimLocalAdminHandler.cs b/ErsatzTV.Application/Auth/ClaimLocalAdminHandler.cs new file mode 100644 index 000000000..550fc1cdb --- /dev/null +++ b/ErsatzTV.Application/Auth/ClaimLocalAdminHandler.cs @@ -0,0 +1,68 @@ +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Infrastructure.Data; +using Microsoft.EntityFrameworkCore; + +namespace ErsatzTV.Application.Auth; + +public class ClaimLocalAdminHandler(IDbContextFactory dbContextFactory, ILocalPasswordHasher passwordHasher) + : IRequestHandler> +{ + public async Task> Handle( + ClaimLocalAdmin request, + CancellationToken cancellationToken) + { + foreach (BaseError error in LocalAdminHelpers.ValidateNewCredentials(request.Username, request.Password)) + { + return error; + } + + string username = request.Username.Trim(); + + await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); + + // Fast path for the common already-configured case (clean 409). The real first-claim-wins guard is + // the unique index on ConfigElement.Key + the single atomic SaveChanges below: two concurrent claims + // both pass this check, but only one INSERT of the three credential rows commits — the loser's + // SaveChanges violates the unique Key index and rolls back wholesale (no mixed-state credential). + bool alreadyConfigured = await dbContext.ConfigElements + .AnyAsync(c => c.Key == ConfigElementKey.AuthLocalAdminPasswordHash.Key, cancellationToken); + if (alreadyConfigured) + { + return BaseError.New("A local administrator has already been configured"); + } + + string stamp = LocalAdminHelpers.NewSecurityStamp(); + dbContext.ConfigElements.AddRange( + new ConfigElement { Key = ConfigElementKey.AuthLocalAdminUsername.Key, Value = username }, + new ConfigElement + { + Key = ConfigElementKey.AuthLocalAdminPasswordHash.Key, + Value = passwordHasher.Hash(request.Password) + }, + new ConfigElement { Key = ConfigElementKey.AuthSecurityStamp.Key, Value = stamp }); + + try + { + await dbContext.SaveChangesAsync(cancellationToken); + } + catch (DbUpdateException) + { + // A write conflict here is (almost always) a lost first-claim race — a concurrent claim inserted + // these keys first (unique Key index). Confirm the row now exists on a fresh context before + // reporting "already configured"; otherwise this was a genuine/transient DB error → rethrow rather + // than mask it. + await using TvContext verifyContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); + bool nowConfigured = await verifyContext.ConfigElements + .AnyAsync(c => c.Key == ConfigElementKey.AuthLocalAdminPasswordHash.Key, cancellationToken); + if (nowConfigured) + { + return BaseError.New("A local administrator has already been configured"); + } + + throw; + } + + return new LocalAdminPrincipal(username, stamp); + } +} diff --git a/ErsatzTV.Application/Auth/GetLocalAdminSecurityStamp.cs b/ErsatzTV.Application/Auth/GetLocalAdminSecurityStamp.cs new file mode 100644 index 000000000..f9fcbf180 --- /dev/null +++ b/ErsatzTV.Application/Auth/GetLocalAdminSecurityStamp.cs @@ -0,0 +1,8 @@ +namespace ErsatzTV.Application.Auth; + +/// +/// The current local-admin security stamp, or None if no local admin is configured. The cookie +/// OnValidatePrincipal compares this to the principal's stamp claim on every request; a mismatch +/// (i.e. the password was changed) rejects the session. +/// +public record GetLocalAdminSecurityStamp : IRequest>; diff --git a/ErsatzTV.Application/Auth/GetLocalAdminSecurityStampHandler.cs b/ErsatzTV.Application/Auth/GetLocalAdminSecurityStampHandler.cs new file mode 100644 index 000000000..f86c0b2b9 --- /dev/null +++ b/ErsatzTV.Application/Auth/GetLocalAdminSecurityStampHandler.cs @@ -0,0 +1,11 @@ +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Repositories; + +namespace ErsatzTV.Application.Auth; + +public class GetLocalAdminSecurityStampHandler(IConfigElementRepository configElementRepository) + : IRequestHandler> +{ + public async Task> Handle(GetLocalAdminSecurityStamp request, CancellationToken cancellationToken) => + await configElementRepository.GetValue(ConfigElementKey.AuthSecurityStamp, cancellationToken); +} diff --git a/ErsatzTV.Application/Auth/ILocalPasswordHasher.cs b/ErsatzTV.Application/Auth/ILocalPasswordHasher.cs new file mode 100644 index 000000000..5fd772887 --- /dev/null +++ b/ErsatzTV.Application/Auth/ILocalPasswordHasher.cs @@ -0,0 +1,27 @@ +namespace ErsatzTV.Application.Auth; + +public enum LocalPasswordVerification +{ + Failed, + Success, + SuccessRehashNeeded +} + +/// +/// Wraps ASP.NET Core Identity's PasswordHasher (PBKDF2) behind a minimal, framework-agnostic +/// surface so the Auth handlers don't depend on Identity types directly. +/// +public interface ILocalPasswordHasher +{ + /// Hashes a password for storage (random per-hash salt embedded in the returned string). + string Hash(string password); + + /// Verifies a password against a stored hash in constant time (delegated to Identity). + LocalPasswordVerification Verify(string hash, string password); + + /// + /// A stable, valid hash of a throwaway password. Verify against this when no real credential exists + /// so an unknown-username / unconfigured login costs the same as a real one (no user enumeration). + /// + string DummyHash { get; } +} diff --git a/ErsatzTV.Application/Auth/IsLocalAdminConfigured.cs b/ErsatzTV.Application/Auth/IsLocalAdminConfigured.cs new file mode 100644 index 000000000..463c16b24 --- /dev/null +++ b/ErsatzTV.Application/Auth/IsLocalAdminConfigured.cs @@ -0,0 +1,4 @@ +namespace ErsatzTV.Application.Auth; + +/// True once a local administrator credential has been set (first-run setup is complete). +public record IsLocalAdminConfigured : IRequest; diff --git a/ErsatzTV.Application/Auth/IsLocalAdminConfiguredHandler.cs b/ErsatzTV.Application/Auth/IsLocalAdminConfiguredHandler.cs new file mode 100644 index 000000000..2dd484fd9 --- /dev/null +++ b/ErsatzTV.Application/Auth/IsLocalAdminConfiguredHandler.cs @@ -0,0 +1,15 @@ +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Repositories; + +namespace ErsatzTV.Application.Auth; + +public class IsLocalAdminConfiguredHandler(IConfigElementRepository configElementRepository) + : IRequestHandler +{ + public async Task Handle(IsLocalAdminConfigured request, CancellationToken cancellationToken) + { + Option hash = + await configElementRepository.GetConfigElement(ConfigElementKey.AuthLocalAdminPasswordHash, cancellationToken); + return hash.IsSome; + } +} diff --git a/ErsatzTV.Application/Auth/LocalAdminHelpers.cs b/ErsatzTV.Application/Auth/LocalAdminHelpers.cs new file mode 100644 index 000000000..2cf753c96 --- /dev/null +++ b/ErsatzTV.Application/Auth/LocalAdminHelpers.cs @@ -0,0 +1,49 @@ +using System.Security.Cryptography; +using ErsatzTV.Core; + +namespace ErsatzTV.Application.Auth; + +internal static class LocalAdminHelpers +{ + public const int MaxUsernameLength = 256; + + // Upper bound so an absurdly long password can't burn CPU in PBKDF2 (the request body is also capped + // by Kestrel, #283; this is defense-in-depth on the field itself). + public const int MaxPasswordLength = 1024; + + /// 128 bits of random, lowercase hex. Rotated on every password change to revoke sessions. + public static string NewSecurityStamp() => + Convert.ToHexString(RandomNumberGenerator.GetBytes(16)).ToLowerInvariant(); + + /// Validates a new username + password. Returns the error, or None if valid. + public static Option ValidateNewCredentials(string username, string password) + { + string trimmed = (username ?? string.Empty).Trim(); + if (trimmed.Length == 0) + { + return BaseError.New("Username is required"); + } + + if (trimmed.Length > MaxUsernameLength) + { + return BaseError.New("Username is too long"); + } + + return ValidatePassword(password); + } + + public static Option ValidatePassword(string password) + { + if (string.IsNullOrEmpty(password) || password.Length < AuthConstants.MinPasswordLength) + { + return BaseError.New($"Password must be at least {AuthConstants.MinPasswordLength} characters"); + } + + if (password.Length > MaxPasswordLength) + { + return BaseError.New($"Password must be at most {MaxPasswordLength} characters"); + } + + return Option.None; + } +} diff --git a/ErsatzTV.Application/Auth/LocalAdminPrincipal.cs b/ErsatzTV.Application/Auth/LocalAdminPrincipal.cs new file mode 100644 index 000000000..26656b0b9 --- /dev/null +++ b/ErsatzTV.Application/Auth/LocalAdminPrincipal.cs @@ -0,0 +1,9 @@ +namespace ErsatzTV.Application.Auth; + +/// +/// The identity of the single local administrator, as returned by a successful claim / login / password +/// change. The web host turns this into a cookie principal: becomes the name claim +/// and is stamped as so a later +/// password change (which rotates the stamp) revokes the session. +/// +public record LocalAdminPrincipal(string Username, string SecurityStamp); diff --git a/ErsatzTV.Application/Auth/LocalPasswordHasher.cs b/ErsatzTV.Application/Auth/LocalPasswordHasher.cs new file mode 100644 index 000000000..7805fc526 --- /dev/null +++ b/ErsatzTV.Application/Auth/LocalPasswordHasher.cs @@ -0,0 +1,33 @@ +using Microsoft.AspNetCore.Identity; + +namespace ErsatzTV.Application.Auth; + +/// +/// backed by ASP.NET Core Identity's +/// (PBKDF2-HMAC-SHA512, per-hash random salt, format-versioned so a future work-factor bump is a +/// transparent rehash-on-verify). Stateless and thread-safe → registered as a singleton. +/// +public sealed class LocalPasswordHasher : ILocalPasswordHasher +{ + // The generic user parameter is unused by the hasher (it takes no per-user data), so a shared sentinel + // is fine. + private static readonly object Sentinel = new(); + + private readonly PasswordHasher _hasher = new(); + private readonly Lazy _dummyHash; + + public LocalPasswordHasher() => + _dummyHash = new Lazy(() => _hasher.HashPassword(Sentinel, "not-a-real-password")); + + public string DummyHash => _dummyHash.Value; + + public string Hash(string password) => _hasher.HashPassword(Sentinel, password); + + public LocalPasswordVerification Verify(string hash, string password) => + _hasher.VerifyHashedPassword(Sentinel, hash, password) switch + { + PasswordVerificationResult.Success => LocalPasswordVerification.Success, + PasswordVerificationResult.SuccessRehashNeeded => LocalPasswordVerification.SuccessRehashNeeded, + _ => LocalPasswordVerification.Failed + }; +} diff --git a/ErsatzTV.Application/Auth/RotateLocalAdminSecurityStamp.cs b/ErsatzTV.Application/Auth/RotateLocalAdminSecurityStamp.cs new file mode 100644 index 000000000..d083e7058 --- /dev/null +++ b/ErsatzTV.Application/Auth/RotateLocalAdminSecurityStamp.cs @@ -0,0 +1,9 @@ +namespace ErsatzTV.Application.Auth; + +/// +/// Rotates the local admin security stamp, revoking every outstanding local session server-side (their +/// cookies carry the old stamp and fail OnValidatePrincipal on their next request). Used by logout +/// so signing out actually ends the session server-side, not just client-side. A no-op when no local +/// admin is configured. OIDC sessions are unaffected (they carry no stamp). +/// +public record RotateLocalAdminSecurityStamp : IRequest; diff --git a/ErsatzTV.Application/Auth/RotateLocalAdminSecurityStampHandler.cs b/ErsatzTV.Application/Auth/RotateLocalAdminSecurityStampHandler.cs new file mode 100644 index 000000000..4f01ee161 --- /dev/null +++ b/ErsatzTV.Application/Auth/RotateLocalAdminSecurityStampHandler.cs @@ -0,0 +1,28 @@ +using ErsatzTV.Core.Domain; +using ErsatzTV.Infrastructure.Data; +using Microsoft.EntityFrameworkCore; + +namespace ErsatzTV.Application.Auth; + +public class RotateLocalAdminSecurityStampHandler(IDbContextFactory dbContextFactory) + : IRequestHandler +{ + public async Task Handle(RotateLocalAdminSecurityStamp request, CancellationToken cancellationToken) + { + await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); + + ConfigElement stampRow = await dbContext.ConfigElements + .FirstOrDefaultAsync(c => c.Key == ConfigElementKey.AuthSecurityStamp.Key, cancellationToken); + + // No local admin configured → nothing to revoke. + if (stampRow is null) + { + return Unit.Default; + } + + stampRow.Value = LocalAdminHelpers.NewSecurityStamp(); + await dbContext.SaveChangesAsync(cancellationToken); + + return Unit.Default; + } +} diff --git a/ErsatzTV.Application/Auth/SeedLocalAdminFromEnvironment.cs b/ErsatzTV.Application/Auth/SeedLocalAdminFromEnvironment.cs new file mode 100644 index 000000000..e960e0bcd --- /dev/null +++ b/ErsatzTV.Application/Auth/SeedLocalAdminFromEnvironment.cs @@ -0,0 +1,11 @@ +using ErsatzTV.Core; + +namespace ErsatzTV.Application.Auth; + +/// +/// Recovery/bootstrap path: (re)sets the local admin from configuration (env +/// Auth:LocalAdmin:Username/Password). Overwrites any existing credential and rotates the +/// stamp (revoking sessions), so an operator who is locked out can reset by setting the env and +/// restarting. Runs at startup only when a password is configured. +/// +public record SeedLocalAdminFromEnvironment(string Username, string Password) : IRequest>; diff --git a/ErsatzTV.Application/Auth/SeedLocalAdminFromEnvironmentHandler.cs b/ErsatzTV.Application/Auth/SeedLocalAdminFromEnvironmentHandler.cs new file mode 100644 index 000000000..3d793cebe --- /dev/null +++ b/ErsatzTV.Application/Auth/SeedLocalAdminFromEnvironmentHandler.cs @@ -0,0 +1,63 @@ +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Infrastructure.Data; +using Microsoft.EntityFrameworkCore; + +namespace ErsatzTV.Application.Auth; + +public class SeedLocalAdminFromEnvironmentHandler( + IDbContextFactory dbContextFactory, + ILocalPasswordHasher passwordHasher) + : IRequestHandler> +{ + public async Task> Handle( + SeedLocalAdminFromEnvironment request, + CancellationToken cancellationToken) + { + string username = (request.Username ?? string.Empty).Trim(); + if (username.Length == 0) + { + username = "admin"; + } + + if (username.Length > LocalAdminHelpers.MaxUsernameLength) + { + return BaseError.New("Seed username is too long"); + } + + foreach (BaseError error in LocalAdminHelpers.ValidatePassword(request.Password)) + { + return error; + } + + await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); + + List rows = await dbContext.ConfigElements + .Where(c => c.Key == ConfigElementKey.AuthLocalAdminUsername.Key + || c.Key == ConfigElementKey.AuthLocalAdminPasswordHash.Key + || c.Key == ConfigElementKey.AuthSecurityStamp.Key) + .ToListAsync(cancellationToken); + + // Overwrite (recovery/bootstrap) atomically: username + new hash + rotated stamp commit together. + Upsert(dbContext, rows, ConfigElementKey.AuthLocalAdminUsername.Key, username); + Upsert(dbContext, rows, ConfigElementKey.AuthLocalAdminPasswordHash.Key, passwordHasher.Hash(request.Password)); + Upsert(dbContext, rows, ConfigElementKey.AuthSecurityStamp.Key, LocalAdminHelpers.NewSecurityStamp()); + + await dbContext.SaveChangesAsync(cancellationToken); + + return Unit.Default; + } + + private static void Upsert(TvContext dbContext, List existing, string key, string value) + { + ConfigElement row = existing.Find(r => r.Key == key); + if (row is null) + { + dbContext.ConfigElements.Add(new ConfigElement { Key = key, Value = value }); + } + else + { + row.Value = value; + } + } +} diff --git a/ErsatzTV.Application/Auth/VerifyLocalAdminLogin.cs b/ErsatzTV.Application/Auth/VerifyLocalAdminLogin.cs new file mode 100644 index 000000000..18ace3b6f --- /dev/null +++ b/ErsatzTV.Application/Auth/VerifyLocalAdminLogin.cs @@ -0,0 +1,9 @@ +using ErsatzTV.Core; + +namespace ErsatzTV.Application.Auth; + +/// +/// Verifies a local-login username/password. On success returns the principal (username + current +/// security stamp) to sign into a cookie. A generic error (no username enumeration) on any failure. +/// +public record VerifyLocalAdminLogin(string Username, string Password) : IRequest>; diff --git a/ErsatzTV.Application/Auth/VerifyLocalAdminLoginHandler.cs b/ErsatzTV.Application/Auth/VerifyLocalAdminLoginHandler.cs new file mode 100644 index 000000000..e1d5ecadb --- /dev/null +++ b/ErsatzTV.Application/Auth/VerifyLocalAdminLoginHandler.cs @@ -0,0 +1,53 @@ +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Infrastructure.Data; +using Microsoft.EntityFrameworkCore; + +namespace ErsatzTV.Application.Auth; + +public class VerifyLocalAdminLoginHandler( + IDbContextFactory dbContextFactory, + ILocalPasswordHasher passwordHasher) + : IRequestHandler> +{ + private static readonly BaseError InvalidCredentials = BaseError.New("Invalid username or password"); + + public async Task> Handle( + VerifyLocalAdminLogin request, + CancellationToken cancellationToken) + { + string username = (request.Username ?? string.Empty).Trim(); + + await using TvContext dbContext = await dbContextFactory.CreateDbContextAsync(cancellationToken); + + // Read the hash and stamp in ONE snapshot so they are consistent (issue: a login racing a password + // change must not return a stamp newer than the hash it verified). A concurrent change is then either + // wholly before this read (the old password fails to verify) or wholly after it (we return the + // pre-change stamp, so the cookie AuthController issues is revoked on its very next request by + // CookieSecurityStampValidator). No writes happen here, so there is nothing to clobber. + Dictionary config = await dbContext.ConfigElements + .Where(c => c.Key == ConfigElementKey.AuthLocalAdminUsername.Key + || c.Key == ConfigElementKey.AuthLocalAdminPasswordHash.Key + || c.Key == ConfigElementKey.AuthSecurityStamp.Key) + .ToDictionaryAsync(c => c.Key, c => c.Value, cancellationToken); + + config.TryGetValue(ConfigElementKey.AuthLocalAdminUsername.Key, out string storedUser); + config.TryGetValue(ConfigElementKey.AuthLocalAdminPasswordHash.Key, out string storedHash); + config.TryGetValue(ConfigElementKey.AuthSecurityStamp.Key, out string stamp); + + // Always run exactly one PBKDF2 verify — against a dummy hash when unconfigured/unknown — so response + // timing does not reveal whether the account exists (no user enumeration). + string candidateHash = storedHash ?? passwordHasher.DummyHash; + LocalPasswordVerification result = passwordHasher.Verify(candidateHash, request.Password ?? string.Empty); + + bool userMatches = storedUser is not null + && string.Equals(storedUser, username, StringComparison.OrdinalIgnoreCase); + + if (storedHash is null || !userMatches || result == LocalPasswordVerification.Failed) + { + return InvalidCredentials; + } + + return new LocalAdminPrincipal(storedUser, stamp ?? string.Empty); + } +} diff --git a/ErsatzTV.Application/ErsatzTV.Application.csproj b/ErsatzTV.Application/ErsatzTV.Application.csproj index 8d4a2baeb..73d35a499 100644 --- a/ErsatzTV.Application/ErsatzTV.Application.csproj +++ b/ErsatzTV.Application/ErsatzTV.Application.csproj @@ -15,6 +15,7 @@ + diff --git a/ErsatzTV.Core/Domain/ConfigElementKey.cs b/ErsatzTV.Core/Domain/ConfigElementKey.cs index 2b7f5fdc8..42de2398e 100644 --- a/ErsatzTV.Core/Domain/ConfigElementKey.cs +++ b/ErsatzTV.Core/Domain/ConfigElementKey.cs @@ -61,4 +61,11 @@ public class ConfigElementKey public static ConfigElementKey XmltvTimeZone => new("xmltv.time_zone"); public static ConfigElementKey XmltvDaysToBuild => new("xmltv.days_to_build"); public static ConfigElementKey XmltvBlockBehavior => new("xmltv.block_behavior"); + + // Browser SPA authentication (issue #295). The single local-admin credential lives in ConfigElement + // rows (no DB migration): a username, a PBKDF2 password hash, and a security stamp that is rotated on + // every password change so a stamp mismatch in OnValidatePrincipal revokes all outstanding sessions. + public static ConfigElementKey AuthLocalAdminUsername => new("auth.local_admin.username"); + public static ConfigElementKey AuthLocalAdminPasswordHash => new("auth.local_admin.password_hash"); + public static ConfigElementKey AuthSecurityStamp => new("auth.security_stamp"); } diff --git a/ErsatzTV.Tests/Application/Auth/ChangeLocalAdminPasswordHandlerTests.cs b/ErsatzTV.Tests/Application/Auth/ChangeLocalAdminPasswordHandlerTests.cs new file mode 100644 index 000000000..8f132cf4c --- /dev/null +++ b/ErsatzTV.Tests/Application/Auth/ChangeLocalAdminPasswordHandlerTests.cs @@ -0,0 +1,122 @@ +using ErsatzTV.Application.Auth; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Repositories; +using ErsatzTV.Infrastructure.Data.Repositories; +using ErsatzTV.Tests.Support; +using LanguageExt; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Application.Auth; + +[TestFixture] +public class ChangeLocalAdminPasswordHandlerTests +{ + private InMemoryTvContext _db = null!; + private IConfigElementRepository _configElementRepository = null!; + private ILocalPasswordHasher _passwordHasher = null!; + + private const string Username = "Operator"; + private const string CurrentPassword = "supersecret"; + private const string NewPassword = "evenbettersecret"; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _configElementRepository = new ConfigElementRepository(_db.Factory); + _passwordHasher = new LocalPasswordHasher(); + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private ChangeLocalAdminPasswordHandler MakeHandler() => + new(_db.Factory, _passwordHasher); + + private async Task SeedAdmin() + { + var claim = new ClaimLocalAdminHandler(_db.Factory, _passwordHasher); + (await claim.Handle(new ClaimLocalAdmin(Username, CurrentPassword), CancellationToken.None)) + .IsRight.ShouldBeTrue(); + } + + private async Task StoredHash() => + (await _configElementRepository.GetValue( + ConfigElementKey.AuthLocalAdminPasswordHash, + CancellationToken.None)).IfNone(""); + + private async Task StoredStamp() => + (await _configElementRepository.GetValue( + ConfigElementKey.AuthSecurityStamp, + CancellationToken.None)).IfNone(""); + + [Test] + public async Task Handle_Should_Change_Password_And_Rotate_Stamp_On_Valid_Current_Password() + { + await SeedAdmin(); + string originalStamp = await StoredStamp(); + ChangeLocalAdminPasswordHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new ChangeLocalAdminPassword(Username, CurrentPassword, NewPassword), + CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + + string storedHash = await StoredHash(); + _passwordHasher.Verify(storedHash, NewPassword).ShouldNotBe(LocalPasswordVerification.Failed); + _passwordHasher.Verify(storedHash, CurrentPassword).ShouldBe(LocalPasswordVerification.Failed); + + string newStamp = await StoredStamp(); + newStamp.ShouldNotBe(originalStamp); + LocalAdminPrincipal principal = result.Match( + Left: e => throw new ShouldAssertException(e.ToString()), + Right: p => p); + principal.SecurityStamp.ShouldBe(newStamp); + } + + [Test] + public async Task Handle_Should_Fail_On_Wrong_Current_Password_And_Change_Nothing() + { + await SeedAdmin(); + string originalHash = await StoredHash(); + string originalStamp = await StoredStamp(); + ChangeLocalAdminPasswordHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new ChangeLocalAdminPassword(Username, "notthecurrentpassword", NewPassword), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + (await StoredHash()).ShouldBe(originalHash); + (await StoredStamp()).ShouldBe(originalStamp); + } + + [Test] + public async Task Handle_Should_Reject_Short_New_Password() + { + await SeedAdmin(); + string shortPassword = new('a', AuthConstants.MinPasswordLength - 1); + ChangeLocalAdminPasswordHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new ChangeLocalAdminPassword(Username, CurrentPassword, shortPassword), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + } + + [Test] + public async Task Handle_Should_Fail_On_Unconfigured_Db() + { + ChangeLocalAdminPasswordHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new ChangeLocalAdminPassword(Username, CurrentPassword, NewPassword), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + } +} diff --git a/ErsatzTV.Tests/Application/Auth/ClaimLocalAdminHandlerTests.cs b/ErsatzTV.Tests/Application/Auth/ClaimLocalAdminHandlerTests.cs new file mode 100644 index 000000000..10deab78f --- /dev/null +++ b/ErsatzTV.Tests/Application/Auth/ClaimLocalAdminHandlerTests.cs @@ -0,0 +1,142 @@ +using ErsatzTV.Application.Auth; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Repositories; +using ErsatzTV.Infrastructure.Data.Repositories; +using ErsatzTV.Tests.Support; +using LanguageExt; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Application.Auth; + +[TestFixture] +public class ClaimLocalAdminHandlerTests +{ + private InMemoryTvContext _db = null!; + private IConfigElementRepository _configElementRepository = null!; + private ILocalPasswordHasher _passwordHasher = null!; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _configElementRepository = new ConfigElementRepository(_db.Factory); + _passwordHasher = new LocalPasswordHasher(); + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private ClaimLocalAdminHandler MakeHandler() => + new(_db.Factory, _passwordHasher); + + [Test] + public async Task Handle_Should_Claim_Fresh_Admin_And_Persist_All_Config_Elements() + { + ClaimLocalAdminHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new ClaimLocalAdmin("Operator", "supersecret"), + CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + LocalAdminPrincipal principal = result.Match( + Left: e => throw new ShouldAssertException(e.ToString()), + Right: p => p); + principal.Username.ShouldBe("Operator"); + principal.SecurityStamp.ShouldNotBeNullOrEmpty(); + + Option storedUser = await _configElementRepository.GetValue( + ConfigElementKey.AuthLocalAdminUsername, + CancellationToken.None); + storedUser.IfNone("").ShouldBe("Operator"); + + Option storedStamp = await _configElementRepository.GetValue( + ConfigElementKey.AuthSecurityStamp, + CancellationToken.None); + storedStamp.IfNone("").ShouldBe(principal.SecurityStamp); + + Option storedHash = await _configElementRepository.GetValue( + ConfigElementKey.AuthLocalAdminPasswordHash, + CancellationToken.None); + storedHash.IsSome.ShouldBeTrue(); + _passwordHasher.Verify(storedHash.IfNone(""), "supersecret") + .ShouldNotBe(LocalPasswordVerification.Failed); + } + + [Test] + public async Task Handle_Should_Refuse_Second_Claim_When_Admin_Already_Configured() + { + ClaimLocalAdminHandler handler = MakeHandler(); + (await handler.Handle(new ClaimLocalAdmin("First", "supersecret"), CancellationToken.None)) + .IsRight.ShouldBeTrue(); + + Either result = await handler.Handle( + new ClaimLocalAdmin("Second", "anothersecret"), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + + // The original credential is untouched. + Option storedUser = await _configElementRepository.GetValue( + ConfigElementKey.AuthLocalAdminUsername, + CancellationToken.None); + storedUser.IfNone("").ShouldBe("First"); + } + + [Test] + public async Task Handle_Should_Reject_Whitespace_Username_And_Persist_Nothing() + { + ClaimLocalAdminHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new ClaimLocalAdmin(" ", "supersecret"), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + await AssertNothingPersisted(); + } + + [Test] + public async Task Handle_Should_Reject_Short_Password_And_Persist_Nothing() + { + string shortPassword = new('a', AuthConstants.MinPasswordLength - 1); + ClaimLocalAdminHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new ClaimLocalAdmin("Operator", shortPassword), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + await AssertNothingPersisted(); + } + + [Test] + public async Task Handle_Should_Reject_Over_Long_Password_And_Persist_Nothing() + { + // LocalAdminHelpers.MaxPasswordLength (1024) is internal; use the documented bound directly. + string longPassword = new('a', 1024 + 1); + ClaimLocalAdminHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new ClaimLocalAdmin("Operator", longPassword), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + await AssertNothingPersisted(); + } + + private async Task AssertNothingPersisted() + { + (await _configElementRepository.GetConfigElement( + ConfigElementKey.AuthLocalAdminUsername, + CancellationToken.None)).IsNone.ShouldBeTrue(); + (await _configElementRepository.GetConfigElement( + ConfigElementKey.AuthLocalAdminPasswordHash, + CancellationToken.None)).IsNone.ShouldBeTrue(); + (await _configElementRepository.GetConfigElement( + ConfigElementKey.AuthSecurityStamp, + CancellationToken.None)).IsNone.ShouldBeTrue(); + } +} diff --git a/ErsatzTV.Tests/Application/Auth/RotateLocalAdminSecurityStampHandlerTests.cs b/ErsatzTV.Tests/Application/Auth/RotateLocalAdminSecurityStampHandlerTests.cs new file mode 100644 index 000000000..7b1209e6f --- /dev/null +++ b/ErsatzTV.Tests/Application/Auth/RotateLocalAdminSecurityStampHandlerTests.cs @@ -0,0 +1,65 @@ +using ErsatzTV.Application.Auth; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Repositories; +using ErsatzTV.Infrastructure.Data.Repositories; +using ErsatzTV.Tests.Support; +using LanguageExt; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Application.Auth; + +[TestFixture] +public class RotateLocalAdminSecurityStampHandlerTests +{ + private InMemoryTvContext _db = null!; + private IConfigElementRepository _configElementRepository = null!; + private ILocalPasswordHasher _passwordHasher = null!; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _configElementRepository = new ConfigElementRepository(_db.Factory); + _passwordHasher = new LocalPasswordHasher(); + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private RotateLocalAdminSecurityStampHandler MakeHandler() => new(_db.Factory); + + [Test] + public async Task Handle_Should_Rotate_The_Stamp_When_Configured() + { + // Arrange: claim an admin so a stamp exists. + Either claim = await new ClaimLocalAdminHandler(_db.Factory, _passwordHasher) + .Handle(new ClaimLocalAdmin("admin", "supersecret"), CancellationToken.None); + claim.IsRight.ShouldBeTrue(); + + Option before = + await _configElementRepository.GetValue(ConfigElementKey.AuthSecurityStamp, CancellationToken.None); + string originalStamp = before.Match(s => s, () => throw new ShouldAssertException("expected a stamp")); + + // Act + await MakeHandler().Handle(new RotateLocalAdminSecurityStamp(), CancellationToken.None); + + // Assert: the stamp changed (all outstanding sessions carrying the old stamp are now stale). + Option after = + await _configElementRepository.GetValue(ConfigElementKey.AuthSecurityStamp, CancellationToken.None); + string rotatedStamp = after.Match(s => s, () => throw new ShouldAssertException("expected a stamp")); + rotatedStamp.ShouldNotBe(originalStamp); + rotatedStamp.ShouldNotBeNullOrEmpty(); + } + + [Test] + public async Task Handle_Should_Be_A_No_Op_When_Unconfigured() + { + await MakeHandler().Handle(new RotateLocalAdminSecurityStamp(), CancellationToken.None); + + Option stamp = + await _configElementRepository.GetValue(ConfigElementKey.AuthSecurityStamp, CancellationToken.None); + stamp.IsNone.ShouldBeTrue(); + } +} diff --git a/ErsatzTV.Tests/Application/Auth/SeedLocalAdminFromEnvironmentHandlerTests.cs b/ErsatzTV.Tests/Application/Auth/SeedLocalAdminFromEnvironmentHandlerTests.cs new file mode 100644 index 000000000..f70af03b4 --- /dev/null +++ b/ErsatzTV.Tests/Application/Auth/SeedLocalAdminFromEnvironmentHandlerTests.cs @@ -0,0 +1,120 @@ +using ErsatzTV.Application.Auth; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Repositories; +using ErsatzTV.Infrastructure.Data.Repositories; +using ErsatzTV.Tests.Support; +using LanguageExt; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Application.Auth; + +[TestFixture] +public class SeedLocalAdminFromEnvironmentHandlerTests +{ + private InMemoryTvContext _db = null!; + private IConfigElementRepository _configElementRepository = null!; + private ILocalPasswordHasher _passwordHasher = null!; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _configElementRepository = new ConfigElementRepository(_db.Factory); + _passwordHasher = new LocalPasswordHasher(); + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private SeedLocalAdminFromEnvironmentHandler MakeHandler() => + new(_db.Factory, _passwordHasher); + + private async Task StoredValue(ConfigElementKey key) => + (await _configElementRepository.GetValue(key, CancellationToken.None)).IfNone(""); + + [Test] + public async Task Handle_Should_Seed_Username_Hash_And_Stamp_On_Fresh_Db() + { + SeedLocalAdminFromEnvironmentHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new SeedLocalAdminFromEnvironment("Operator", "supersecret"), + CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await StoredValue(ConfigElementKey.AuthLocalAdminUsername)).ShouldBe("Operator"); + (await StoredValue(ConfigElementKey.AuthSecurityStamp)).ShouldNotBeNullOrEmpty(); + _passwordHasher.Verify(await StoredValue(ConfigElementKey.AuthLocalAdminPasswordHash), "supersecret") + .ShouldNotBe(LocalPasswordVerification.Failed); + } + + [Test] + public async Task Handle_Should_Overwrite_Existing_Credential_And_Rotate_Stamp() + { + var claim = new ClaimLocalAdminHandler(_db.Factory, _passwordHasher); + (await claim.Handle(new ClaimLocalAdmin("Original", "originalsecret"), CancellationToken.None)) + .IsRight.ShouldBeTrue(); + string originalStamp = await StoredValue(ConfigElementKey.AuthSecurityStamp); + + SeedLocalAdminFromEnvironmentHandler handler = MakeHandler(); + Either result = await handler.Handle( + new SeedLocalAdminFromEnvironment("Replacement", "replacementsecret"), + CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await StoredValue(ConfigElementKey.AuthLocalAdminUsername)).ShouldBe("Replacement"); + (await StoredValue(ConfigElementKey.AuthSecurityStamp)).ShouldNotBe(originalStamp); + + string storedHash = await StoredValue(ConfigElementKey.AuthLocalAdminPasswordHash); + _passwordHasher.Verify(storedHash, "replacementsecret").ShouldNotBe(LocalPasswordVerification.Failed); + _passwordHasher.Verify(storedHash, "originalsecret").ShouldBe(LocalPasswordVerification.Failed); + } + + [Test] + public async Task Handle_Should_Default_Empty_Username_To_Admin() + { + SeedLocalAdminFromEnvironmentHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new SeedLocalAdminFromEnvironment(" ", "supersecret"), + CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + (await StoredValue(ConfigElementKey.AuthLocalAdminUsername)).ShouldBe("admin"); + } + + [Test] + public async Task Handle_Should_Reject_Over_Long_Username() + { + // LocalAdminHelpers.MaxUsernameLength (256) is internal; use the documented bound directly. + string longUsername = new('u', 256 + 1); + SeedLocalAdminFromEnvironmentHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new SeedLocalAdminFromEnvironment(longUsername, "supersecret"), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + (await _configElementRepository.GetConfigElement( + ConfigElementKey.AuthLocalAdminUsername, + CancellationToken.None)).IsNone.ShouldBeTrue(); + } + + [Test] + public async Task Handle_Should_Reject_Short_Password() + { + string shortPassword = new('a', AuthConstants.MinPasswordLength - 1); + SeedLocalAdminFromEnvironmentHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new SeedLocalAdminFromEnvironment("Operator", shortPassword), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + (await _configElementRepository.GetConfigElement( + ConfigElementKey.AuthLocalAdminPasswordHash, + CancellationToken.None)).IsNone.ShouldBeTrue(); + } +} diff --git a/ErsatzTV.Tests/Application/Auth/VerifyLocalAdminLoginHandlerTests.cs b/ErsatzTV.Tests/Application/Auth/VerifyLocalAdminLoginHandlerTests.cs new file mode 100644 index 000000000..9a08680d4 --- /dev/null +++ b/ErsatzTV.Tests/Application/Auth/VerifyLocalAdminLoginHandlerTests.cs @@ -0,0 +1,122 @@ +using ErsatzTV.Application.Auth; +using ErsatzTV.Core; +using ErsatzTV.Core.Domain; +using ErsatzTV.Core.Interfaces.Repositories; +using ErsatzTV.Infrastructure.Data.Repositories; +using ErsatzTV.Tests.Support; +using LanguageExt; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Application.Auth; + +[TestFixture] +public class VerifyLocalAdminLoginHandlerTests +{ + private InMemoryTvContext _db = null!; + private IConfigElementRepository _configElementRepository = null!; + private ILocalPasswordHasher _passwordHasher = null!; + + private const string Username = "Operator"; + private const string Password = "supersecret"; + + [SetUp] + public async Task SetUp() + { + _db = await InMemoryTvContext.CreateAsync(); + _configElementRepository = new ConfigElementRepository(_db.Factory); + _passwordHasher = new LocalPasswordHasher(); + } + + [TearDown] + public async Task TearDown() => await _db.DisposeAsync(); + + private VerifyLocalAdminLoginHandler MakeHandler() => + new(_db.Factory, _passwordHasher); + + private async Task SeedAdmin() + { + var claim = new ClaimLocalAdminHandler(_db.Factory, _passwordHasher); + (await claim.Handle(new ClaimLocalAdmin(Username, Password), CancellationToken.None)) + .IsRight.ShouldBeTrue(); + } + + private async Task StoredStamp() => + (await _configElementRepository.GetValue( + ConfigElementKey.AuthSecurityStamp, + CancellationToken.None)).IfNone(""); + + [Test] + public async Task Handle_Should_Return_Principal_With_Stored_Stamp_On_Valid_Credentials() + { + await SeedAdmin(); + VerifyLocalAdminLoginHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new VerifyLocalAdminLogin(Username, Password), + CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + LocalAdminPrincipal principal = result.Match( + Left: e => throw new ShouldAssertException(e.ToString()), + Right: p => p); + principal.Username.ShouldBe(Username); + principal.SecurityStamp.ShouldBe(await StoredStamp()); + } + + [Test] + public async Task Handle_Should_Fail_On_Wrong_Password() + { + await SeedAdmin(); + VerifyLocalAdminLoginHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new VerifyLocalAdminLogin(Username, "wrongpassword"), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + } + + [Test] + public async Task Handle_Should_Fail_On_Wrong_Username() + { + await SeedAdmin(); + VerifyLocalAdminLoginHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new VerifyLocalAdminLogin("SomebodyElse", Password), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + } + + [Test] + public async Task Handle_Should_Fail_On_Unconfigured_Db() + { + VerifyLocalAdminLoginHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new VerifyLocalAdminLogin(Username, Password), + CancellationToken.None); + + result.IsLeft.ShouldBeTrue(); + } + + [Test] + public async Task Handle_Should_Match_Username_Case_Insensitively() + { + await SeedAdmin(); + VerifyLocalAdminLoginHandler handler = MakeHandler(); + + Either result = await handler.Handle( + new VerifyLocalAdminLogin(Username.ToUpperInvariant(), Password), + CancellationToken.None); + + result.IsRight.ShouldBeTrue(); + LocalAdminPrincipal principal = result.Match( + Left: e => throw new ShouldAssertException(e.ToString()), + Right: p => p); + // The stored (canonical) username is returned, not the differently-cased input. + principal.Username.ShouldBe(Username); + } +} diff --git a/ErsatzTV.Tests/Controllers/ApiControllerSecurityTests.cs b/ErsatzTV.Tests/Controllers/ApiControllerSecurityTests.cs index 8b335b2b7..46775e5e7 100644 --- a/ErsatzTV.Tests/Controllers/ApiControllerSecurityTests.cs +++ b/ErsatzTV.Tests/Controllers/ApiControllerSecurityTests.cs @@ -18,7 +18,7 @@ namespace ErsatzTV.Tests.Controllers; [TestFixture] public class ApiControllerSecurityTests { - private static readonly bool ApiKeyAuthorizationFilterIsGlobal = IsApiKeyAuthorizationFilterRegisteredGlobally(); + private static readonly bool ApiAuthorizationFilterIsGlobal = IsApiAuthorizationFilterRegisteredGlobally(); [Test] public void Every_Mutating_Api_Action_Should_Be_Globally_Protected_Or_Explicitly_Exempt() @@ -47,7 +47,7 @@ public class ApiControllerSecurityTests foreach (Type controllerType in apiControllers) { bool controllerSkipsApiKey = controllerType - .GetCustomAttributes(inherit: true) + .GetCustomAttributes(inherit: true) .Any(); foreach (MethodInfo action in controllerType @@ -64,7 +64,7 @@ public class ApiControllerSecurityTests } bool actionSkipsApiKey = action - .GetCustomAttributes(inherit: true) + .GetCustomAttributes(inherit: true) .Any(); (controllerSkipsApiKey || actionSkipsApiKey || IsGloballyProtected()) @@ -74,23 +74,27 @@ public class ApiControllerSecurityTests } [Test] - public void ScannerController_Should_Be_Only_Api_Key_Exempt_Api_Controller() + 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/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(inherit: true).Any()) - .Where(t => t.GetCustomAttributes(inherit: true).Any()) + .Where(t => t.GetCustomAttributes(inherit: true).Any()) .ToArray(); - exemptControllers.ShouldBe([typeof(ScannerController)]); + exemptControllers.ShouldBe([typeof(ScannerController), typeof(AuthController)], ignoreOrder: true); } [Test] - public void Startup_Should_Register_ApiKeyAuthorizationFilter_Globally() + public void Startup_Should_Register_ApiAuthorizationFilter_Globally() { - ApiKeyAuthorizationFilterIsGlobal.ShouldBeTrue(); + ApiAuthorizationFilterIsGlobal.ShouldBeTrue(); } [Test] @@ -109,8 +113,8 @@ public class ApiControllerSecurityTests foreach (Type controllerType in sensitiveControllers) { - controllerType.GetCustomAttributes(inherit: true).Any() - .ShouldBeTrue($"{controllerType.Name} must carry [RequiresApiKey]"); + controllerType.GetCustomAttributes(inherit: true).Any() + .ShouldBeTrue($"{controllerType.Name} must carry [RequiresAuthentication]"); } } @@ -125,9 +129,9 @@ public class ApiControllerSecurityTests .ShouldBeTrue(); } - private static bool IsGloballyProtected() => ApiKeyAuthorizationFilterIsGlobal; + private static bool IsGloballyProtected() => ApiAuthorizationFilterIsGlobal; - private static bool IsApiKeyAuthorizationFilterRegisteredGlobally() + private static bool IsApiAuthorizationFilterRegisteredGlobally() { var settings = new Dictionary { @@ -155,6 +159,6 @@ public class ApiControllerSecurityTests return options.Filters .OfType() - .Any(a => a.ServiceType == typeof(ApiKeyAuthorizationFilter)); + .Any(a => a.ServiceType == typeof(ApiAuthorizationFilter)); } } diff --git a/ErsatzTV.Tests/Controllers/AuthControllerTests.cs b/ErsatzTV.Tests/Controllers/AuthControllerTests.cs new file mode 100644 index 000000000..84d34c320 --- /dev/null +++ b/ErsatzTV.Tests/Controllers/AuthControllerTests.cs @@ -0,0 +1,71 @@ +using System.Collections.Generic; +using ErsatzTV.Application.Auth; +using ErsatzTV.Controllers.Api; +using ErsatzTV.Controllers.Api.Requests; +using MediatR; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Configuration; +using NSubstitute; +using NUnit.Framework; +using Shouldly; + +namespace ErsatzTV.Tests.Controllers; + +[TestFixture] +public class AuthControllerTests +{ + private static IConfiguration Config(bool envSeed) => + new ConfigurationBuilder() + .AddInMemoryCollection( + envSeed + ? new Dictionary { ["Auth:LocalAdmin:Password"] = "seed-password" } + : new Dictionary()) + .Build(); + + [Test] + public async Task Config_Reports_Setup_Not_Required_When_Env_Seed_Configured() + { + var mediator = Substitute.For(); + mediator.Send(Arg.Any(), Arg.Any()).Returns(false); + + var controller = new AuthController(mediator, Config(envSeed: true)); + + var result = await controller.Config(CancellationToken.None) as OkObjectResult; + var body = result!.Value.ShouldBeOfType(); + + // Env seed owns the credential → the SPA must not offer the browser setup-claim. + body.SetupRequired.ShouldBeFalse(); + } + + [Test] + public async Task Config_Reports_Setup_Required_When_Unconfigured_And_No_Env_Seed() + { + var mediator = Substitute.For(); + mediator.Send(Arg.Any(), Arg.Any()).Returns(false); + + var controller = new AuthController(mediator, Config(envSeed: false)); + + var result = await controller.Config(CancellationToken.None) as OkObjectResult; + var body = result!.Value.ShouldBeOfType(); + + body.SetupRequired.ShouldBeTrue(); + } + + [Test] + public async Task Setup_Is_Closed_With_409_When_Env_Seed_Configured() + { + var mediator = Substitute.For(); + var controller = new AuthController(mediator, Config(envSeed: true)) + { + ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() } + }; + + var result = await controller.Setup(new SetupRequest("admin", "hunter2pw"), CancellationToken.None); + + var problem = result.ShouldBeOfType(); + problem.StatusCode.ShouldBe(StatusCodes.Status409Conflict); + // The claim must never be attempted while the env seed owns the credential. + await mediator.DidNotReceive().Send(Arg.Any(), Arg.Any()); + } +} diff --git a/ErsatzTV.Tests/Filters/ApiKeyAuthorizationFilterTests.cs b/ErsatzTV.Tests/Filters/ApiAuthorizationFilterTests.cs similarity index 63% rename from ErsatzTV.Tests/Filters/ApiKeyAuthorizationFilterTests.cs rename to ErsatzTV.Tests/Filters/ApiAuthorizationFilterTests.cs index 61d0cb1b5..a700c27db 100644 --- a/ErsatzTV.Tests/Filters/ApiKeyAuthorizationFilterTests.cs +++ b/ErsatzTV.Tests/Filters/ApiAuthorizationFilterTests.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Security.Claims; using ErsatzTV.Filters; using ErsatzTV.Services; using Microsoft.AspNetCore.Http; @@ -12,7 +13,7 @@ using Shouldly; namespace ErsatzTV.Tests.Filters; [TestFixture] -public class ApiKeyAuthorizationFilterTests +public class ApiAuthorizationFilterTests { private const string Key = "secret"; @@ -27,25 +28,38 @@ public class ApiKeyAuthorizationFilterTests string? apiKeyHeader, string path = "/api/channels", bool skipApiKeyAuthorization = false, - bool requiresApiKey = false) + bool requiresApiKey = false, + bool authenticatedSession = false, + bool csrfHeader = false) { var httpContext = new DefaultHttpContext(); httpContext.Request.Method = method; httpContext.Request.Path = path; if (apiKeyHeader is not null) { - httpContext.Request.Headers[ApiKeyAuthorizationFilter.HeaderName] = apiKeyHeader; + httpContext.Request.Headers[ApiAuthorizationFilter.HeaderName] = apiKeyHeader; + } + + if (csrfHeader) + { + httpContext.Request.Headers[ApiAuthorizationFilter.CsrfHeaderName] = "1"; + } + + if (authenticatedSession) + { + // A ClaimsIdentity with an authentication type reports IsAuthenticated == true. + httpContext.User = new ClaimsPrincipal(new ClaimsIdentity(authenticationType: "cookie")); } var metadata = new List(); if (skipApiKeyAuthorization) { - metadata.Add(new SkipApiKeyAuthorizationAttribute()); + metadata.Add(new SkipApiAuthorizationAttribute()); } if (requiresApiKey) { - metadata.Add(new RequiresApiKeyAttribute()); + metadata.Add(new RequiresAuthenticationAttribute()); } var actionDescriptor = new ActionDescriptor { EndpointMetadata = metadata }; @@ -53,7 +67,7 @@ public class ApiKeyAuthorizationFilterTests return new AuthorizationFilterContext(actionContext, new List()); } - private static ApiKeyAuthorizationFilter MakeFilter(bool requireKeyForReads = true) => + private static ApiAuthorizationFilter MakeFilter(bool requireKeyForReads = true) => new(new FakeApiKeyProvider(Key, requireKeyForReads)); private static void ShouldBeUnauthorized(AuthorizationFilterContext context) @@ -124,10 +138,10 @@ public class ApiKeyAuthorizationFilterTests context.Result.ShouldBeNull(); } - // ---- the sensitive-read tier stays gated even with reads open ([RequiresApiKey]) ---- + // ---- the sensitive-read tier stays gated even with reads open ([RequiresAuthentication]) ---- [Test] - public void Should_Reject_Read_On_RequiresApiKey_Endpoint_Even_When_Reads_Not_Required() + public void Should_Reject_Read_On_RequiresAuthentication_Endpoint_Even_When_Reads_Not_Required() { AuthorizationFilterContext context = MakeContext("GET", apiKeyHeader: null, requiresApiKey: true); MakeFilter(requireKeyForReads: false).OnAuthorization(context); @@ -135,7 +149,7 @@ public class ApiKeyAuthorizationFilterTests } [Test] - public void Should_Allow_Read_On_RequiresApiKey_Endpoint_When_Key_Correct() + public void Should_Allow_Read_On_RequiresAuthentication_Endpoint_When_Key_Correct() { AuthorizationFilterContext context = MakeContext("GET", apiKeyHeader: Key, requiresApiKey: true); MakeFilter(requireKeyForReads: false).OnAuthorization(context); @@ -172,4 +186,54 @@ public class ApiKeyAuthorizationFilterTests MakeFilter(requireKeyForReads: true).OnAuthorization(context); context.Result.ShouldBeNull(); } + + // ---- session (cookie) authentication accepted as an alternative credential (issue #295) ---- + + [Test] + public void Should_Allow_Read_When_Session_Authenticated_Without_Key() + { + AuthorizationFilterContext context = MakeContext("GET", apiKeyHeader: null, authenticatedSession: true); + MakeFilter(requireKeyForReads: true).OnAuthorization(context); + context.Result.ShouldBeNull(); + } + + [Test] + public void Should_Allow_Session_Mutation_When_Csrf_Header_Present() + { + AuthorizationFilterContext context = + MakeContext("POST", apiKeyHeader: null, authenticatedSession: true, csrfHeader: true); + MakeFilter().OnAuthorization(context); + context.Result.ShouldBeNull(); + } + + [Test] + public void Should_Reject_Session_Mutation_When_Csrf_Header_Missing() + { + AuthorizationFilterContext context = + MakeContext("POST", apiKeyHeader: null, authenticatedSession: true, csrfHeader: false); + MakeFilter().OnAuthorization(context); + + var result = context.Result.ShouldBeOfType(); + result.StatusCode.ShouldBe(StatusCodes.Status403Forbidden); + var problemDetails = result.Value.ShouldBeOfType(); + problemDetails.Status.ShouldBe(StatusCodes.Status403Forbidden); + } + + [Test] + public void Should_Reject_When_Neither_Key_Nor_Session_Present() + { + AuthorizationFilterContext context = MakeContext("GET", apiKeyHeader: null, authenticatedSession: false); + MakeFilter(requireKeyForReads: true).OnAuthorization(context); + ShouldBeUnauthorized(context); + } + + [Test] + public void Should_Prefer_Machine_Key_Over_Session_And_Skip_Csrf() + { + // A valid X-Api-Key is CSRF-immune: a session cookie riding along must not force a CSRF check. + AuthorizationFilterContext context = + MakeContext("POST", apiKeyHeader: Key, authenticatedSession: true, csrfHeader: false); + MakeFilter().OnAuthorization(context); + context.Result.ShouldBeNull(); + } } diff --git a/ErsatzTV.Tests/Filters/ApiKeyEndpointRequiresKeyTests.cs b/ErsatzTV.Tests/Filters/ApiKeyEndpointRequiresKeyTests.cs index 5aaf8f39d..fbcc5b190 100644 --- a/ErsatzTV.Tests/Filters/ApiKeyEndpointRequiresKeyTests.cs +++ b/ErsatzTV.Tests/Filters/ApiKeyEndpointRequiresKeyTests.cs @@ -12,7 +12,7 @@ using Shouldly; namespace ErsatzTV.Tests.Filters; /// -/// Proves the shared predicate — the one +/// Proves the shared predicate — the one /// the OpenAPI security/401 transformer consumes — agrees with what the filter actually enforces at /// runtime, across a representative matrix. If the two ever diverged, the spec could claim an endpoint /// is open while the filter gates it (or vice-versa); this test is the anti-drift guard (#287). @@ -37,10 +37,10 @@ public class ApiKeyEndpointRequiresKeyTests new object[] { "DELETE", false, false, false, true }, new object[] { "GET", false, false, true, true }, // read gated when reads-required new object[] { "GET", false, false, false, false }, // read open when reads-not-required - new object[] { "GET", true, false, false, true }, // [RequiresApiKey] gates read even so + new object[] { "GET", true, false, false, true }, // [RequiresAuthentication] gates read even so new object[] { "HEAD", false, false, false, false }, // read verb, open new object[] { "OPTIONS", false, false, true, false },// preflight always exempt - new object[] { "POST", false, true, false, false } // [SkipApiKeyAuthorization] exempt + new object[] { "POST", false, true, false, false } // [SkipApiAuthorization] exempt ]; [TestCaseSource(nameof(Matrix))] @@ -54,15 +54,15 @@ public class ApiKeyEndpointRequiresKeyTests var metadata = new List(); if (requiresApiKey) { - metadata.Add(new RequiresApiKeyAttribute()); + metadata.Add(new RequiresAuthenticationAttribute()); } if (skip) { - metadata.Add(new SkipApiKeyAuthorizationAttribute()); + metadata.Add(new SkipApiAuthorizationAttribute()); } - ApiKeyAuthorizationFilter.EndpointRequiresKey(method, metadata, requireKeyForReads) + ApiAuthorizationFilter.EndpointRequiresKey(method, metadata, requireKeyForReads) .ShouldBe(expected); } @@ -77,12 +77,12 @@ public class ApiKeyEndpointRequiresKeyTests // The filter, on an /api path with the header MISSING, produces a 401 exactly when the predicate // says the endpoint requires a key. Drive the real filter and compare its decision to the predicate. AuthorizationFilterContext context = MakeContext(method, requiresApiKey, skip); - new ApiKeyAuthorizationFilter(new FakeApiKeyProvider(requireKeyForReads)).OnAuthorization(context); + new ApiAuthorizationFilter(new FakeApiKeyProvider(requireKeyForReads)).OnAuthorization(context); bool filterGated = context.Result is not null; filterGated.ShouldBe(expected); filterGated.ShouldBe( - ApiKeyAuthorizationFilter.EndpointRequiresKey(method, Metadata(requiresApiKey, skip), requireKeyForReads)); + ApiAuthorizationFilter.EndpointRequiresKey(method, Metadata(requiresApiKey, skip), requireKeyForReads)); } [Test] @@ -90,11 +90,11 @@ public class ApiKeyEndpointRequiresKeyTests { // The predicate assumes an /api endpoint; the filter's path scoping precedes it. A mutating request // outside /api must pass untouched even though the predicate (given the same method) returns true. - ApiKeyAuthorizationFilter.EndpointRequiresKey("POST", new List(), requireKeyForReads: true) + ApiAuthorizationFilter.EndpointRequiresKey("POST", new List(), requireKeyForReads: true) .ShouldBeTrue(); AuthorizationFilterContext context = MakeContext("POST", requiresApiKey: false, skip: false, path: "/iptv/x.m3u"); - new ApiKeyAuthorizationFilter(new FakeApiKeyProvider(requireKeyForReads: true)).OnAuthorization(context); + new ApiAuthorizationFilter(new FakeApiKeyProvider(requireKeyForReads: true)).OnAuthorization(context); context.Result.ShouldBeNull(); } @@ -104,12 +104,12 @@ public class ApiKeyEndpointRequiresKeyTests var metadata = new List(); if (requiresApiKey) { - metadata.Add(new RequiresApiKeyAttribute()); + metadata.Add(new RequiresAuthenticationAttribute()); } if (skip) { - metadata.Add(new SkipApiKeyAuthorizationAttribute()); + metadata.Add(new SkipApiAuthorizationAttribute()); } return metadata; diff --git a/ErsatzTV/Auth/CookieSecurityStampValidator.cs b/ErsatzTV/Auth/CookieSecurityStampValidator.cs new file mode 100644 index 000000000..354817f98 --- /dev/null +++ b/ErsatzTV/Auth/CookieSecurityStampValidator.cs @@ -0,0 +1,38 @@ +using System.Security.Claims; +using ErsatzTV.Application.Auth; +using LanguageExt; +using MediatR; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Authentication.Cookies; + +namespace ErsatzTV.Auth; + +/// +/// Cookie OnValidatePrincipal handler enforcing local-admin security-stamp revocation (#295): +/// a local-login session carries the admin's security stamp as a claim; a password change rotates the +/// stored stamp, so a mismatch here rejects the (now stale) session on its next request. OIDC sessions +/// carry no stamp claim and are governed by the IdP, so they are skipped. +/// +public static class CookieSecurityStampValidator +{ + public static async Task ValidateAsync(CookieValidatePrincipalContext context) + { + ClaimsPrincipal principal = context.Principal; + string method = principal?.FindFirst(AuthConstants.AuthMethodClaim)?.Value; + if (!string.Equals(method, AuthConstants.MethodLocal, StringComparison.Ordinal)) + { + return; + } + + string presented = principal.FindFirst(AuthConstants.SecurityStampClaim)?.Value; + IMediator mediator = context.HttpContext.RequestServices.GetRequiredService(); + Option stored = await mediator.Send(new GetLocalAdminSecurityStamp()); + string current = stored.IfNone(string.Empty); + + if (string.IsNullOrEmpty(presented) || !string.Equals(presented, current, StringComparison.Ordinal)) + { + context.RejectPrincipal(); + await context.HttpContext.SignOutAsync(AuthConstants.CookieScheme); + } + } +} diff --git a/ErsatzTV/Controllers/Api/AuthController.cs b/ErsatzTV/Controllers/Api/AuthController.cs new file mode 100644 index 000000000..cc633a61d --- /dev/null +++ b/ErsatzTV/Controllers/Api/AuthController.cs @@ -0,0 +1,234 @@ +using System.Security.Claims; +using ErsatzTV.Application.Auth; +using ErsatzTV.Controllers.Api.Requests; +using ErsatzTV.Core; +using ErsatzTV.Extensions; +using ErsatzTV.Filters; +using MediatR; +using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.RateLimiting; + +namespace ErsatzTV.Controllers.Api; + +/// +/// Browser SPA session authentication (issue #295). Excluded from the OpenAPI document — the spec's +/// audience is machine X-Api-Key clients, and a browser-interactive cookie login is not something a +/// generated client drives — and exempt from the global (this surface +/// must be reachable before a caller has a session). Sensitive operations self-check the principal. +/// +[ApiController] +[ApiExplorerSettings(IgnoreApi = true)] +[SkipApiAuthorization] +public class AuthController(IMediator mediator, IConfiguration configuration) : ControllerBase +{ + // When the operator has set an env seed, the local admin is managed via configuration — the browser + // setup-claim is closed. This also eliminates the startup race where an attacker could claim admin in the + // window before LocalAdminSeedService runs (the seed would then lose and its insert be swallowed). + private bool EnvSeedConfigured => !string.IsNullOrWhiteSpace(configuration["Auth:LocalAdmin:Password"]); + + /// Public: what auth options exist + whether first-run setup is still needed (drives the SPA boot gate). + [HttpGet("/api/auth/config")] + public async Task Config(CancellationToken cancellationToken) + { + bool configured = await mediator.Send(new IsLocalAdminConfigured(), cancellationToken); + return Ok(new AuthConfigResponse(OidcHelper.IsEnabled, true, !configured && !EnvSeedConfigured)); + } + + /// The current session (anonymous is a 200 with authenticated=false, never a 401). + [HttpGet("/api/auth/session")] + public IActionResult Session() + { + if (User.Identity?.IsAuthenticated != true) + { + return Ok(new AuthSessionResponse(false, null, null)); + } + + return Ok(new AuthSessionResponse(true, User.Identity?.Name, User.FindFirst(AuthConstants.AuthMethodClaim)?.Value)); + } + + /// First-run setup-claim: create the local admin. Fails 409 if one already exists. + [HttpPost("/api/auth/setup")] + [EnableRateLimiting("auth")] + public async Task Setup([FromBody] SetupRequest request, CancellationToken cancellationToken) + { + if (EnvSeedConfigured) + { + return ApiResults.ConflictProblem( + "Managed via configuration", + "The local administrator is provisioned from Auth:LocalAdmin:* configuration; browser setup is disabled."); + } + + if (await mediator.Send(new IsLocalAdminConfigured(), cancellationToken)) + { + return ApiResults.ConflictProblem("Already configured", "A local administrator already exists."); + } + + Either result = + await mediator.Send(new ClaimLocalAdmin(request.Username, request.Password), cancellationToken); + + return await result.Match( + Left: async error => + { + // A concurrent claim that lost the race reports 409 (the record now exists), not the 422 a + // validation error gets. + if (await mediator.Send(new IsLocalAdminConfigured(), cancellationToken)) + { + return ApiResults.ConflictProblem("Already configured", "A local administrator already exists."); + } + + return error.ToErrorResult(); + }, + Right: async principal => + { + await IssueLocalCookieAsync(principal); + return (IActionResult)Ok(new AuthSessionResponse(true, principal.Username, AuthConstants.MethodLocal)); + }); + } + + /// Local username/password login. A generic 401 on any failure (no username enumeration). + [HttpPost("/api/auth/login")] + [EnableRateLimiting("auth")] + public async Task Login([FromBody] LoginRequest request, CancellationToken cancellationToken) + { + Either result = + await mediator.Send(new VerifyLocalAdminLogin(request.Username, request.Password), cancellationToken); + + return await result.Match( + Left: _ => Task.FromResult((IActionResult)Unauthorized(new ProblemDetails + { + Status = StatusCodes.Status401Unauthorized, + Title = "Unauthorized", + Detail = "Invalid username or password." + })), + Right: async principal => + { + await IssueLocalCookieAsync(principal); + return (IActionResult)Ok(new AuthSessionResponse(true, principal.Username, AuthConstants.MethodLocal)); + }); + } + + /// + /// Sign out of the cookie session. Requires the CSRF header (this controller is filter-exempt). For a + /// local session this rotates the security stamp, ending the session server-side (a captured + /// cookie can't be replayed after logout) — which, for the single local admin, revokes all local + /// sessions ("log out everywhere"). Gated on an authenticated session so an unauthenticated caller + /// can't force-revoke the admin. + /// + [HttpPost("/api/auth/logout")] + public async Task Logout(CancellationToken cancellationToken) + { + // Prevent forced-logout CSRF: a same-site form POST carries the Lax cookie but can't set a custom + // header. The whole controller is [SkipApiAuthorization], so the filter's CSRF check doesn't apply. + if (RequiresCsrf(out IActionResult csrfError)) + { + return csrfError; + } + + if (User.Identity?.IsAuthenticated == true && + string.Equals( + User.FindFirst(AuthConstants.AuthMethodClaim)?.Value, + AuthConstants.MethodLocal, + StringComparison.Ordinal)) + { + await mediator.Send(new RotateLocalAdminSecurityStamp(), cancellationToken); + } + + await HttpContext.SignOutAsync(AuthConstants.CookieScheme); + return NoContent(); + } + + /// Change the local admin password (requires a local-login session); rotates the stamp, revoking other sessions. + [HttpPost("/api/auth/password")] + [EnableRateLimiting("auth")] + public async Task ChangePassword([FromBody] ChangePasswordRequest request, CancellationToken cancellationToken) + { + // Filter-exempt controller, so apply the session-mutation CSRF check here explicitly. + if (RequiresCsrf(out IActionResult csrfError)) + { + return csrfError; + } + + // A machine key must not be able to rotate the local admin's password — require a local session. + if (User.Identity?.IsAuthenticated != true || + !string.Equals( + User.FindFirst(AuthConstants.AuthMethodClaim)?.Value, + AuthConstants.MethodLocal, + StringComparison.Ordinal)) + { + return Unauthorized(new ProblemDetails + { + Status = StatusCodes.Status401Unauthorized, + Title = "Unauthorized", + Detail = "A local-login session is required to change the password." + }); + } + + Either result = await mediator.Send( + new ChangeLocalAdminPassword(User.Identity?.Name, request.CurrentPassword, request.NewPassword), + cancellationToken); + + return await result.Match( + Left: error => Task.FromResult(error.ToErrorResult()), + Right: async principal => + { + // Re-issue the cookie with the rotated stamp so THIS session survives while others are revoked. + await IssueLocalCookieAsync(principal); + return (IActionResult)NoContent(); + }); + } + + /// Browser-navigation OIDC challenge. Outside /api (a top-level GET redirect). 404 when OIDC is off. + [HttpGet("/auth/oidc/login")] + public IActionResult OidcLogin() + { + if (!OidcHelper.IsEnabled) + { + return NotFound(); + } + + return Challenge( + new AuthenticationProperties { RedirectUri = $"{Request.PathBase}/app" }, + AuthConstants.OidcScheme); + } + + // Session-mutation CSRF gate for the [SkipApiAuthorization] auth surface (the global filter doesn't see + // it). Presence-only: a custom header can't be set by a cross-site form/navigation. Returns true (with a + // 403 result) when the header is missing. + private bool RequiresCsrf(out IActionResult error) + { + if (Request.Headers.ContainsKey(ApiAuthorizationFilter.CsrfHeaderName)) + { + error = null; + return false; + } + + error = new ObjectResult(new ProblemDetails + { + Status = StatusCodes.Status403Forbidden, + Title = "Forbidden", + Detail = $"This request requires the '{ApiAuthorizationFilter.CsrfHeaderName}' header." + }) + { + StatusCode = StatusCodes.Status403Forbidden + }; + return true; + } + + private Task IssueLocalCookieAsync(LocalAdminPrincipal principal) + { + var claims = new List + { + new(ClaimTypes.Name, principal.Username), + new(AuthConstants.AuthMethodClaim, AuthConstants.MethodLocal), + new(AuthConstants.SecurityStampClaim, principal.SecurityStamp) + }; + var identity = new ClaimsIdentity(claims, AuthConstants.CookieScheme); + return HttpContext.SignInAsync(AuthConstants.CookieScheme, new ClaimsPrincipal(identity)); + } +} + +public record AuthConfigResponse(bool OidcEnabled, bool LocalLoginEnabled, bool SetupRequired); + +public record AuthSessionResponse(bool Authenticated, string Username, string Method); diff --git a/ErsatzTV/Controllers/Api/LogsController.cs b/ErsatzTV/Controllers/Api/LogsController.cs index 301e4b65b..55746db02 100644 --- a/ErsatzTV/Controllers/Api/LogsController.cs +++ b/ErsatzTV/Controllers/Api/LogsController.cs @@ -9,7 +9,7 @@ using Microsoft.AspNetCore.Mvc; namespace ErsatzTV.Controllers.Api; [ApiController] -[RequiresApiKey] +[RequiresAuthentication] public class LogsController(IMediator mediator) : ControllerBase { private const int MaxPageSize = 100; diff --git a/ErsatzTV/Controllers/Api/MaintenanceController.cs b/ErsatzTV/Controllers/Api/MaintenanceController.cs index 8330a43d1..e8d66527a 100644 --- a/ErsatzTV/Controllers/Api/MaintenanceController.cs +++ b/ErsatzTV/Controllers/Api/MaintenanceController.cs @@ -11,7 +11,7 @@ namespace ErsatzTV.Controllers.Api; [ApiController] [EndpointGroupName("general")] -[RequiresApiKey] +[RequiresAuthentication] public class MaintenanceController(IMediator mediator, ChannelWriter workerChannel) { [HttpPost("/api/maintenance/gc")] diff --git a/ErsatzTV/Controllers/Api/Requests/AuthRequests.cs b/ErsatzTV/Controllers/Api/Requests/AuthRequests.cs new file mode 100644 index 000000000..82d7028f5 --- /dev/null +++ b/ErsatzTV/Controllers/Api/Requests/AuthRequests.cs @@ -0,0 +1,7 @@ +namespace ErsatzTV.Controllers.Api.Requests; + +public record LoginRequest(string Username, string Password); + +public record SetupRequest(string Username, string Password); + +public record ChangePasswordRequest(string CurrentPassword, string NewPassword); diff --git a/ErsatzTV/Controllers/Api/ScannerController.cs b/ErsatzTV/Controllers/Api/ScannerController.cs index a466cf58d..cc428b669 100644 --- a/ErsatzTV/Controllers/Api/ScannerController.cs +++ b/ErsatzTV/Controllers/Api/ScannerController.cs @@ -9,7 +9,7 @@ namespace ErsatzTV.Controllers.Api; [ApiController] [ApiExplorerSettings(IgnoreApi = true)] -[SkipApiKeyAuthorization] +[SkipApiAuthorization] [LocalhostOnly] [Route("api/scan/{scanId:guid}")] public class ScannerController( diff --git a/ErsatzTV/Controllers/Api/SettingsController.cs b/ErsatzTV/Controllers/Api/SettingsController.cs index a29063b3d..5d16159b7 100644 --- a/ErsatzTV/Controllers/Api/SettingsController.cs +++ b/ErsatzTV/Controllers/Api/SettingsController.cs @@ -19,7 +19,7 @@ namespace ErsatzTV.Controllers.Api; [ApiController] [EndpointGroupName("general")] -[RequiresApiKey] +[RequiresAuthentication] public class SettingsController(IMediator mediator) : ControllerBase { // FFmpeg settings diff --git a/ErsatzTV/Controllers/Api/TroubleshootController.cs b/ErsatzTV/Controllers/Api/TroubleshootController.cs index ca313de74..f2b2158b0 100644 --- a/ErsatzTV/Controllers/Api/TroubleshootController.cs +++ b/ErsatzTV/Controllers/Api/TroubleshootController.cs @@ -27,7 +27,7 @@ using Serilog.Context; namespace ErsatzTV.Controllers.Api; [ApiController] -[RequiresApiKey] +[RequiresAuthentication] public class TroubleshootController( ChannelWriter channelWriter, IFileSystem fileSystem, @@ -92,7 +92,8 @@ public class TroubleshootController( [ProducesResponseType(typeof(ValidateSequentialScheduleResponseModel), StatusCodes.Status200OK)] [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status400BadRequest)] public async Task ValidateSchedule( - [Required] [FromBody] ValidateSequentialScheduleRequest request, + [Required] [FromBody] + ValidateSequentialScheduleRequest request, CancellationToken cancellationToken) { if (string.IsNullOrWhiteSpace(request.Yaml)) diff --git a/ErsatzTV/Filters/ApiAuthorizationFilter.cs b/ErsatzTV/Filters/ApiAuthorizationFilter.cs new file mode 100644 index 000000000..3c0b87bb1 --- /dev/null +++ b/ErsatzTV/Filters/ApiAuthorizationFilter.cs @@ -0,0 +1,144 @@ +using System.Security.Cryptography; +using System.Text; +using ErsatzTV.Services; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; +using Microsoft.Extensions.Primitives; + +namespace ErsatzTV.Filters; + +/// +/// Authorization for the JSON API. A request under /api/* that requires authentication is +/// accepted on either of two credentials (issue #295): +/// +/// a matching machine X-Api-Key header (MCP / external clients), or +/// an authenticated session principal (browser cookie, from OIDC or local login). +/// +/// Which endpoints require authentication is decided by — the +/// single predicate shared with OpenAPI generation — so the spec can never drift from enforcement: +/// mutating verbs are always fail-closed; reads are gated when Api:RequireKeyForReads is set +/// (the default) or the endpoint carries . Endpoints +/// marked and anything outside /api (e.g. +/// /iptv/*, /artwork/*, the SPA) are never affected. +/// +/// The machine key is CSRF-immune (a browser cannot set a custom header cross-origin without a +/// credentialed CORS grant, which is never issued). A cookie session is not: session-authenticated +/// mutations must additionally carry the header, which — being a +/// custom header — forces a CORS preflight that a cross-site attacker page cannot satisfy. +/// +/// +public class ApiAuthorizationFilter(IApiKeyProvider apiKeyProvider) : IAuthorizationFilter +{ + public const string HeaderName = "X-Api-Key"; + + /// + /// Required on session-authenticated mutating requests as a CSRF defense. Presence is the whole + /// check — a custom request header cannot be set by a cross-site form/navigation and forces a CORS + /// preflight for cross-origin XHR, so only same-origin (the SPA) can send it. + /// + public const string CsrfHeaderName = "X-CSRF"; + + public void OnAuthorization(AuthorizationFilterContext context) + { + HttpRequest request = context.HttpContext.Request; + + // Never gate anything outside the JSON API surface. + if (!request.Path.StartsWithSegments("/api", StringComparison.OrdinalIgnoreCase)) + { + return; + } + + if (!EndpointRequiresKey(request.Method, context.ActionDescriptor.EndpointMetadata, apiKeyProvider.RequireKeyForReads)) + { + return; + } + + // 1. Machine key wins and is CSRF-immune. Check it first so a browser that happens to hold both + // a cookie and a key is still treated as a machine caller (no CSRF header required). + if (request.Headers.TryGetValue(HeaderName, out StringValues provided) + && KeysMatch(provided.ToString(), apiKeyProvider.ApiKey)) + { + return; + } + + // 2. Authenticated session (cookie principal from OIDC or local login). + if (context.HttpContext.User?.Identity?.IsAuthenticated == true) + { + // Session-authenticated mutations require the CSRF header. Reads are safe (SameSite=Lax + + // no credentialed CORS means a cross-site read can't be issued with the cookie either). + if (IsMutating(request.Method) && !request.Headers.ContainsKey(CsrfHeaderName)) + { + context.Result = new ObjectResult(new ProblemDetails + { + Status = StatusCodes.Status403Forbidden, + Title = "Forbidden", + Detail = $"Session-authenticated writes require the '{CsrfHeaderName}' header." + }) + { + StatusCode = StatusCodes.Status403Forbidden + }; + } + + return; + } + + // 3. Neither credential presented. + context.Result = new UnauthorizedObjectResult(new ProblemDetails + { + Status = StatusCodes.Status401Unauthorized, + Title = "Unauthorized", + Detail = $"Authentication is required. Provide a valid '{HeaderName}' header (machine clients) " + + "or sign in to obtain a session (browser)." + }); + } + + /// + /// The single decision shared between runtime enforcement (this filter) and the OpenAPI document + /// generation (ApiSecurityOperationTransformer), so the spec's declared + /// security/401 can never drift from what is actually enforced. Assumes the endpoint + /// is already known to be under /api (the caller's responsibility). Returns + /// when the endpoint requires authentication: any mutating verb + /// (POST/PUT/PATCH/DELETE) is always fail-closed; reads are gated when + /// is set or the endpoint carries + /// . OPTIONS preflight and endpoints marked + /// are exempt. (The name is retained from the + /// API-key-only era for spec-generation stability; "key" here means "credential".) + /// + public static bool EndpointRequiresKey( + string httpMethod, + IEnumerable endpointMetadata, + bool requireKeyForReads) + { + // CORS preflight carries no custom headers and is handled by the CORS middleware. + if (HttpMethods.IsOptions(httpMethod)) + { + return false; + } + + // Explicit opt-out for internal, separately-guarded endpoints (scanner callback, /api/auth/*). + if (endpointMetadata.OfType().Any()) + { + return false; + } + + // Writes are always fail-closed; reads are gated by the global flag or a per-endpoint opt-in. + return IsMutating(httpMethod) + || requireKeyForReads + || endpointMetadata.OfType().Any(); + } + + private static bool IsMutating(string httpMethod) => + HttpMethods.IsPost(httpMethod) + || HttpMethods.IsPut(httpMethod) + || HttpMethods.IsPatch(httpMethod) + || HttpMethods.IsDelete(httpMethod); + + // Compare in constant time so a remote attacker cannot use response-timing to recover the + // key prefix-by-prefix. FixedTimeEquals also short-circuits length differences without + // leaking anything beyond "lengths differ" (still not the matching-prefix length). + private static bool KeysMatch(string provided, string configured) => + CryptographicOperations.FixedTimeEquals( + Encoding.UTF8.GetBytes(provided), + Encoding.UTF8.GetBytes(configured)); +} diff --git a/ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs b/ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs deleted file mode 100644 index 419b2af27..000000000 --- a/ErsatzTV/Filters/ApiKeyAuthorizationFilter.cs +++ /dev/null @@ -1,98 +0,0 @@ -using System.Security.Cryptography; -using System.Text; -using ErsatzTV.Services; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.Filters; -using Microsoft.Extensions.Primitives; - -namespace ErsatzTV.Filters; - -/// -/// API-key authorization for the JSON API. The effective key (from ) -/// is never empty, so this is fail-closed by construction (issue #280): every mutating request -/// under /api/* must present a matching X-Api-Key header. Read (GET/HEAD) requests -/// under /api/* are also gated when Api:RequireKeyForReads is enabled (the default) -/// or the endpoint carries . Endpoints marked -/// and anything outside /api (e.g. -/// /iptv/*, /artwork/*, the SPA) are never affected. Independent of -/// . -/// -public class ApiKeyAuthorizationFilter(IApiKeyProvider apiKeyProvider) : IAuthorizationFilter -{ - public const string HeaderName = "X-Api-Key"; - - public void OnAuthorization(AuthorizationFilterContext context) - { - HttpRequest request = context.HttpContext.Request; - - // Never gate anything outside the JSON API surface. - if (!request.Path.StartsWithSegments("/api", StringComparison.OrdinalIgnoreCase)) - { - return; - } - - if (!EndpointRequiresKey(request.Method, context.ActionDescriptor.EndpointMetadata, apiKeyProvider.RequireKeyForReads)) - { - return; - } - - if (!request.Headers.TryGetValue(HeaderName, out StringValues provided) - || !KeysMatch(provided.ToString(), apiKeyProvider.ApiKey)) - { - context.Result = new UnauthorizedObjectResult(new ProblemDetails - { - Status = StatusCodes.Status401Unauthorized, - Title = "Unauthorized", - Detail = $"A valid API key is required. Provide it via the '{HeaderName}' header." - }); - } - } - - /// - /// The single decision shared between runtime enforcement (this filter) and the OpenAPI - /// document generation (ApiSecurityOperationTransformer), so the spec's declared - /// security/401 can never drift from what is actually enforced. Assumes the - /// endpoint is already known to be under /api (the caller's responsibility). Returns - /// when the endpoint requires a valid X-Api-Key: any mutating - /// verb (POST/PUT/PATCH/DELETE) is always fail-closed; reads are gated when - /// is set or the endpoint carries - /// . OPTIONS preflight and endpoints marked - /// are exempt. - /// - public static bool EndpointRequiresKey( - string httpMethod, - IEnumerable endpointMetadata, - bool requireKeyForReads) - { - // CORS preflight carries no custom headers and is handled by the CORS middleware. - if (HttpMethods.IsOptions(httpMethod)) - { - return false; - } - - // Explicit opt-out for internal, separately-guarded endpoints (e.g. the scanner callback). - if (endpointMetadata.OfType().Any()) - { - return false; - } - - bool isMutating = HttpMethods.IsPost(httpMethod) - || HttpMethods.IsPut(httpMethod) - || HttpMethods.IsPatch(httpMethod) - || HttpMethods.IsDelete(httpMethod); - - // Writes are always fail-closed; reads are gated by the global flag or a per-endpoint opt-in. - return isMutating - || requireKeyForReads - || endpointMetadata.OfType().Any(); - } - - // Compare in constant time so a remote attacker cannot use response-timing to recover the - // key prefix-by-prefix. FixedTimeEquals also short-circuits length differences without - // leaking anything beyond "lengths differ" (still not the matching-prefix length). - private static bool KeysMatch(string provided, string configured) => - CryptographicOperations.FixedTimeEquals( - Encoding.UTF8.GetBytes(provided), - Encoding.UTF8.GetBytes(configured)); -} diff --git a/ErsatzTV/Filters/RequiresApiKeyAttribute.cs b/ErsatzTV/Filters/RequiresApiKeyAttribute.cs deleted file mode 100644 index 4e444d29d..000000000 --- a/ErsatzTV/Filters/RequiresApiKeyAttribute.cs +++ /dev/null @@ -1,12 +0,0 @@ -using Microsoft.AspNetCore.Mvc.Filters; - -namespace ErsatzTV.Filters; - -/// -/// Marks a read (GET/HEAD) API endpoint as always requiring the X-Api-Key header, even -/// when Api:RequireKeyForReads is disabled. Applied to the sensitive-read tier -/// (troubleshoot, logs, settings, maintenance) that discloses secrets/paths or triggers work. -/// The mirror of . See issue #282. -/// -[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] -public sealed class RequiresApiKeyAttribute : Attribute, IFilterMetadata; diff --git a/ErsatzTV/Filters/RequiresAuthenticationAttribute.cs b/ErsatzTV/Filters/RequiresAuthenticationAttribute.cs new file mode 100644 index 000000000..7ab24596c --- /dev/null +++ b/ErsatzTV/Filters/RequiresAuthenticationAttribute.cs @@ -0,0 +1,14 @@ +using Microsoft.AspNetCore.Mvc.Filters; + +namespace ErsatzTV.Filters; + +/// +/// Marks a read (GET/HEAD) API endpoint as always requiring authentication (a valid machine +/// X-Api-Key header or an authenticated session), even when Api:RequireKeyForReads +/// is disabled. Applied to the sensitive-read tier (troubleshoot, logs, settings, maintenance) +/// that discloses secrets/paths or triggers work. An authenticated session satisfies this tier +/// just as the machine key does. The mirror of . +/// See issues #282, #295. +/// +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] +public sealed class RequiresAuthenticationAttribute : Attribute, IFilterMetadata; diff --git a/ErsatzTV/Filters/SkipApiAuthorizationAttribute.cs b/ErsatzTV/Filters/SkipApiAuthorizationAttribute.cs new file mode 100644 index 000000000..a9d428855 --- /dev/null +++ b/ErsatzTV/Filters/SkipApiAuthorizationAttribute.cs @@ -0,0 +1,12 @@ +using Microsoft.AspNetCore.Mvc.Filters; + +namespace ErsatzTV.Filters; + +/// +/// Marks an internal API endpoint as exempt from the global +/// (neither a machine key nor a session is required). Used for endpoints that are guarded by a +/// different mechanism (e.g. the scanner callback's localhost-only check) and for the +/// /api/auth/* surface itself, which must be reachable before a caller is authenticated. +/// +[AttributeUsage(AttributeTargets.Class | AttributeTargets.Method)] +public sealed class SkipApiAuthorizationAttribute : Attribute, IFilterMetadata; diff --git a/ErsatzTV/Filters/SkipApiKeyAuthorizationAttribute.cs b/ErsatzTV/Filters/SkipApiKeyAuthorizationAttribute.cs deleted file mode 100644 index 4a1f7e7ea..000000000 --- a/ErsatzTV/Filters/SkipApiKeyAuthorizationAttribute.cs +++ /dev/null @@ -1,9 +0,0 @@ -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/Serialization/ApiSecurityOperationTransformer.cs b/ErsatzTV/Serialization/ApiSecurityOperationTransformer.cs index 8ee72f88b..4fef0c4d5 100644 --- a/ErsatzTV/Serialization/ApiSecurityOperationTransformer.cs +++ b/ErsatzTV/Serialization/ApiSecurityOperationTransformer.cs @@ -11,7 +11,7 @@ namespace ErsatzTV.Serialization; /// every operation that actually requires the X-Api-Key header it injects the /// ApiKey security requirement and a documented 401 response. The "requires a key" /// decision is the exact same predicate the runtime filter enforces -/// (), so the spec can never claim an +/// (), so the spec can never claim an /// endpoint is open when it is gated (or vice-versa). The ApiKey scheme itself and the /// ProblemDetails schema the 401 references are declared by /// . See issues #286/#287. @@ -28,7 +28,7 @@ public sealed class ApiSecurityOperationTransformer(IApiKeyProvider apiKeyProvid string method = context.Description.HttpMethod ?? string.Empty; IEnumerable metadata = context.Description.ActionDescriptor.EndpointMetadata; - if (!ApiKeyAuthorizationFilter.EndpointRequiresKey(method, metadata, apiKeyProvider.RequireKeyForReads)) + if (!ApiAuthorizationFilter.EndpointRequiresKey(method, metadata, apiKeyProvider.RequireKeyForReads)) { return Task.CompletedTask; } diff --git a/ErsatzTV/Serialization/ApiSecuritySchemeDocumentTransformer.cs b/ErsatzTV/Serialization/ApiSecuritySchemeDocumentTransformer.cs index 6b00164f1..254befc4d 100644 --- a/ErsatzTV/Serialization/ApiSecuritySchemeDocumentTransformer.cs +++ b/ErsatzTV/Serialization/ApiSecuritySchemeDocumentTransformer.cs @@ -28,7 +28,7 @@ public static class ApiSecuritySchemeDocumentTransformer document.Components.SecuritySchemes[ApiSecurityOperationTransformer.SchemeName] = new OpenApiSecurityScheme { Type = SecuritySchemeType.ApiKey, - Name = ApiKeyAuthorizationFilter.HeaderName, + Name = ApiAuthorizationFilter.HeaderName, In = ParameterLocation.Header, Description = "API key sent in the 'X-Api-Key' request header. Required for all mutating requests and, " + diff --git a/ErsatzTV/Services/ApiKeyProvider.cs b/ErsatzTV/Services/ApiKeyProvider.cs index e2ef782e6..91b1c1dbe 100644 --- a/ErsatzTV/Services/ApiKeyProvider.cs +++ b/ErsatzTV/Services/ApiKeyProvider.cs @@ -5,7 +5,7 @@ using ErsatzTV.Core; namespace ErsatzTV.Services; /// -/// Resolves the effective API key used by and +/// Resolves the effective API key used by and /// the read-gating policy. Resolved once at startup: the configured Api:WriteKey wins; /// otherwise a previously-persisted key is loaded from the config volume; otherwise a fresh /// 256-bit key is generated and persisted. The key is never empty, so write authorization is @@ -136,7 +136,7 @@ public sealed class ApiKeyProvider : IApiKeyProvider "Generated a new API key and saved it to {Path}. Send it via the '{HeaderName}' header " + "(ChicoryTV: Settings → API Key). The key is required for API requests.", path, - Filters.ApiKeyAuthorizationFilter.HeaderName); + Filters.ApiAuthorizationFilter.HeaderName); } catch (Exception ex) { diff --git a/ErsatzTV/Services/RunOnce/LocalAdminSeedService.cs b/ErsatzTV/Services/RunOnce/LocalAdminSeedService.cs new file mode 100644 index 000000000..45d7b23da --- /dev/null +++ b/ErsatzTV/Services/RunOnce/LocalAdminSeedService.cs @@ -0,0 +1,60 @@ +using ErsatzTV.Application.Auth; +using ErsatzTV.Core; +using LanguageExt; +using MediatR; + +namespace ErsatzTV.Services.RunOnce; + +/// +/// Recovery/bootstrap: when Auth:LocalAdmin:Password is configured, (re)seeds the single local +/// administrator at startup (issue #295). Overwrites any existing credential and rotates the security +/// stamp, so an operator locked out of the browser UI can reset by setting the env and restarting. A +/// no-op when unset. Follows the RunOnce pattern (waits for the database to be ready — the migrator is a +/// BackgroundService, so registration order alone does not guarantee the schema exists). +/// +public class LocalAdminSeedService( + IServiceScopeFactory serviceScopeFactory, + IConfiguration configuration, + SystemStartup systemStartup, + ILogger logger) : BackgroundService +{ + protected override async Task ExecuteAsync(CancellationToken stoppingToken) + { + await Task.Yield(); + + string password = configuration["Auth:LocalAdmin:Password"]; + if (string.IsNullOrWhiteSpace(password)) + { + return; + } + + await systemStartup.WaitForDatabase(stoppingToken); + if (stoppingToken.IsCancellationRequested) + { + return; + } + + string username = configuration["Auth:LocalAdmin:Username"]; + + try + { + using IServiceScope scope = serviceScopeFactory.CreateScope(); + IMediator mediator = scope.ServiceProvider.GetRequiredService(); + Either result = + await mediator.Send(new SeedLocalAdminFromEnvironment(username, password), stoppingToken); + + result.Match( + Right: _ => logger.LogWarning( + "Seeded the local administrator from Auth:LocalAdmin:* configuration (any existing " + + "credential was overwritten and all sessions revoked). Unset Auth:LocalAdmin:Password " + + "after signing in."), + Left: error => logger.LogError( + "Failed to seed the local administrator from configuration: {Error}", + error.Value)); + } + catch (Exception ex) + { + logger.LogError(ex, "Failed to seed the local administrator from configuration"); + } + } +} diff --git a/ErsatzTV/Startup.cs b/ErsatzTV/Startup.cs index ab2c76e8d..6c65141d4 100644 --- a/ErsatzTV/Startup.cs +++ b/ErsatzTV/Startup.cs @@ -7,11 +7,15 @@ using System.Runtime.InteropServices; using System.Text; using System.Text.Json.Nodes; using System.Text.Json.Serialization; +using System.Security.Claims; using System.Threading.Channels; +using System.Threading.RateLimiting; using Dapper; using ErsatzTV.Application; +using ErsatzTV.Application.Auth; using ErsatzTV.Application.Channels; using ErsatzTV.Application.Streaming; +using ErsatzTV.Auth; using ErsatzTV.Core; using ErsatzTV.Core.Emby; using ErsatzTV.Core.Errors; @@ -81,6 +85,7 @@ using ErsatzTV.Services.RunOnce; using ErsatzTV.Services.Validators; using FluentValidation; using FluentValidation.AspNetCore; +using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Authentication.Cookies; using Microsoft.AspNetCore.Authentication.JwtBearer; using Microsoft.AspNetCore.Authentication.OpenIdConnect; @@ -88,6 +93,7 @@ using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.DataProtection; using Microsoft.AspNetCore.HttpOverrides; using Microsoft.AspNetCore.Localization; +using Microsoft.AspNetCore.RateLimiting; using Microsoft.AspNetCore.StaticFiles; using Microsoft.EntityFrameworkCore; using Microsoft.Extensions.FileProviders; @@ -210,6 +216,13 @@ public class Startup if (!trustRestricted) { + // Kept as-is from #285 (trust any peer, warn) rather than flipped to ForwardedHeaders.None: + // the forwarded scheme/host feed /iptv M3U/XMLTV/HLS absolute-URL generation + // (Request.Scheme in GetChannelGuideHandler / IptvController), so ignoring them would regress + // stream URLs to http/internal-host for a proxied deployment that hasn't set KnownProxies. + // Setting ForwardedHeaders:KnownProxies/:KnownNetworks is still strongly recommended when + // exposing ErsatzTV beyond a trusted LAN — it also gives the #295 login rate limiter an + // unspoofable client IP and lets the session cookie be marked Secure behind TLS. Log.Warning( "ForwardedHeaders trusts X-Forwarded-* from any peer (spoofable). Set " + "ForwardedHeaders:KnownProxies and/or ForwardedHeaders:KnownNetworks to restrict " + @@ -277,64 +290,121 @@ public class Startup JwtHelper.Init(Configuration); SearchHelper.Init(Configuration); + // Browser SPA authentication (#295). A cookie session is ALWAYS registered — local username/password + // login and the OIDC callback both sign into it. OIDC is added only when configured; the /iptv JWT + // bearer is added only when configured. The /api surface accepts a session OR the machine X-Api-Key + // (see ApiAuthorizationFilter); real enforcement is that filter, not a DefaultPolicy. + AuthenticationBuilder authenticationBuilder = services.AddAuthentication(options => + { + options.DefaultScheme = AuthConstants.CookieScheme; + if (OidcHelper.IsEnabled) + { + options.DefaultChallengeScheme = AuthConstants.OidcScheme; + } + }) + .AddCookie( + AuthConstants.CookieScheme, + options => + { + options.CookieManager = new ChunkingCookieManager(); + + options.Cookie.Name = "ctv-session"; + options.Cookie.HttpOnly = true; + // Lax + no credentialed CORS keeps the cookie same-origin (the SPA is served from /app); + // this is a core CSRF defense alongside the required X-CSRF header on session mutations. + options.Cookie.SameSite = SameSiteMode.Lax; + // SameAsRequest (not Always) so a plain-HTTP LAN deployment is not locked out; behind a + // TLS-terminating proxy the app sees https once ForwardedHeaders:KnownProxies is set. + options.Cookie.SecurePolicy = CookieSecurePolicy.SameAsRequest; + + options.ExpireTimeSpan = TimeSpan.FromDays(14); + options.SlidingExpiration = true; + + options.Events = new CookieAuthenticationEvents + { + // /api is an XHR surface — answer 401/403 rather than redirecting to a login page. + OnRedirectToLogin = context => + { + if (context.Request.Path.StartsWithSegments("/api")) + { + context.Response.StatusCode = StatusCodes.Status401Unauthorized; + return Task.CompletedTask; + } + + context.Response.Redirect(context.RedirectUri); + return Task.CompletedTask; + }, + OnRedirectToAccessDenied = context => + { + if (context.Request.Path.StartsWithSegments("/api")) + { + context.Response.StatusCode = StatusCodes.Status403Forbidden; + return Task.CompletedTask; + } + + context.Response.Redirect(context.RedirectUri); + return Task.CompletedTask; + }, + OnValidatePrincipal = CookieSecurityStampValidator.ValidateAsync + }; + }); + if (OidcHelper.IsEnabled) { - services.AddAuthentication(options => + authenticationBuilder.AddOpenIdConnect( + AuthConstants.OidcScheme, + options => { - options.DefaultScheme = "cookie"; - options.DefaultChallengeScheme = "oidc"; - }) - .AddCookie( - "cookie", - options => + options.Authority = OidcHelper.Authority; + options.ClientId = OidcHelper.ClientId; + options.ClientSecret = OidcHelper.ClientSecret; + + options.ResponseType = OpenIdConnectResponseType.Code; + options.UsePkce = true; + options.ResponseMode = OpenIdConnectResponseMode.Query; + + options.Scope.Clear(); + options.Scope.Add("openid"); + options.Scope.Add("profile"); + options.GetClaimsFromUserInfoEndpoint = true; + + options.CallbackPath = new PathString("/callback"); + + options.SaveTokens = true; + + options.NonceCookie.SecurePolicy = CookieSecurePolicy.SameAsRequest; + options.CorrelationCookie.SecurePolicy = CookieSecurePolicy.SameAsRequest; + + options.Events = new OpenIdConnectEvents { - options.CookieManager = new ChunkingCookieManager(); - - options.Cookie.HttpOnly = true; - options.Cookie.SameSite = SameSiteMode.None; - options.Cookie.SecurePolicy = CookieSecurePolicy.Always; - }) - .AddOpenIdConnect( - "oidc", - options => - { - options.Authority = OidcHelper.Authority; - options.ClientId = OidcHelper.ClientId; - options.ClientSecret = OidcHelper.ClientSecret; - - options.ResponseType = OpenIdConnectResponseType.Code; - options.UsePkce = true; - options.ResponseMode = OpenIdConnectResponseMode.Query; - - options.Scope.Clear(); - options.Scope.Add("openid"); - - options.CallbackPath = new PathString("/callback"); - - options.SaveTokens = true; - - options.NonceCookie.SecurePolicy = CookieSecurePolicy.Always; - options.CorrelationCookie.SecurePolicy = CookieSecurePolicy.Always; - - if (!string.IsNullOrWhiteSpace(OidcHelper.LogoutUri)) + // Mark the session as OIDC so the cookie stamp validator skips it (the stamp is a + // local-login concept; OIDC sessions are governed by the IdP). + OnTokenValidated = context => { - options.Events = new OpenIdConnectEvents + if (context.Principal?.Identity is ClaimsIdentity identity) { - OnRedirectToIdentityProviderForSignOut = context => - { - context.Response.Redirect(OidcHelper.LogoutUri); - context.HandleResponse(); + identity.AddClaim(new Claim(AuthConstants.AuthMethodClaim, AuthConstants.MethodOidc)); + } - return Task.CompletedTask; - } - }; + return Task.CompletedTask; + }, + OnRedirectToIdentityProviderForSignOut = context => + { + if (!string.IsNullOrWhiteSpace(OidcHelper.LogoutUri)) + { + context.Response.Redirect(OidcHelper.LogoutUri); + context.HandleResponse(); + } + + return Task.CompletedTask; } - }); + }; + }); } if (JwtHelper.IsEnabled) { - services.AddAuthentication().AddJwtBearer( + authenticationBuilder.AddJwtBearer( "jwt", options => { @@ -362,34 +432,38 @@ public class Startup }); } - if (OidcHelper.IsEnabled || JwtHelper.IsEnabled) + // Authorization is always registered now that the pipeline always runs UseAuthorization (the cookie + // scheme is always present). No DefaultPolicy: /api is gated by ApiAuthorizationFilter, and no + // endpoint carries [Authorize]. The JWT-only policy stays for /iptv's ConditionalIptvAuthorizeFilter. + services.AddAuthorization(options => { - services.AddAuthorization(options => - { - if (OidcHelper.IsEnabled) + if (JwtHelper.IsEnabled) + { + options.AddPolicy( + "JwtOnlyScheme", + new AuthorizationPolicyBuilder("jwt") + .RequireAuthenticatedUser() + .Build()); + } + }); + + // Per-IP rate limit for the unauthenticated auth surface (login/setup/password) — blunts local + // password brute-force. Keyed on the connection remote IP (accurate only when + // ForwardedHeaders:KnownProxies is configured behind a proxy — see UseForwardedHeaders below). + services.AddRateLimiter(options => + { + options.RejectionStatusCode = StatusCodes.Status429TooManyRequests; + options.AddPolicy( + "auth", + httpContext => RateLimitPartition.GetFixedWindowLimiter( + httpContext.Connection.RemoteIpAddress?.ToString() ?? "unknown", + _ => new FixedWindowRateLimiterOptions { - var defaultAuthorizationPolicyBuilder = new AuthorizationPolicyBuilder( - "cookie", - "oidc"); - - defaultAuthorizationPolicyBuilder = - defaultAuthorizationPolicyBuilder.RequireAuthenticatedUser(); - - options.DefaultPolicy = defaultAuthorizationPolicyBuilder.Build(); - } - - if (JwtHelper.IsEnabled) - { - var onlyJwtSchemePolicyBuilder = new AuthorizationPolicyBuilder("jwt"); - options.AddPolicy( - "JwtOnlyScheme", - onlyJwtSchemePolicyBuilder - .RequireAuthenticatedUser() - .Build()); - } - } - ); - } + PermitLimit = 10, + Window = TimeSpan.FromMinutes(5), + QueueLimit = 0 + })); + }); services.AddCors(o => o.AddPolicy( "ApiCors", @@ -406,9 +480,12 @@ public class Startup "Accept", "Content-Type", "Authorization", - ApiKeyAuthorizationFilter.HeaderName, + ApiAuthorizationFilter.HeaderName, + ApiAuthorizationFilter.CsrfHeaderName, "If-Match") .WithExposedHeaders("ETag"); + // Note: AllowCredentials is deliberately NOT set — cross-origin cookie auth is impossible + // by design (a CSRF defense). Cross-origin clients authenticate with X-Api-Key. } // No configured origins => no cross-origin access (the SPA is same-origin from /app). @@ -423,7 +500,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(); + options.Filters.AddService(); }) .AddNewtonsoftJson(opt => { @@ -438,7 +515,10 @@ public class Startup // API-key authorization for the JSON API (independent of JWT/OIDC). The provider resolves the // effective key once (config, else persisted, else generated) so writes are fail-closed. services.AddSingleton(); - services.AddScoped(); + services.AddScoped(); + + // Local-admin password hashing (browser SPA session auth, #295). Stateless → singleton. + services.AddSingleton(); services.AddFluentValidationAutoValidation(); services.AddValidatorsFromAssemblyContaining(); @@ -820,12 +900,16 @@ public class Startup legacy.UseRouting(); - // Blazor removal (#91 phase b, #206): the OIDC challenge's only attachment point - // was the now-deleted Razor Pages / Blazor UI (AuthorizeFolder("/") + the Blazor - // hub). The OIDC/JWT *service* wiring stays registered (inert unless configured); - // real SPA/API auth is #197. /iptv keeps its own ConditionalIptvAuthorizeFilter - // and mutating /api/* its ApiKeyAuthorizationFilter — both independent of this - // middleware. + // Browser SPA / API authentication (#295). This branch hosts /api, the OIDC /callback, and + // /docs. UseAuthentication populates HttpContext.User from the cookie (default scheme) — the + // credential ApiAuthorizationFilter accepts alongside the machine X-Api-Key — and lets the + // OIDC middleware intercept /callback. UseAuthorization is required for the middleware to run + // (no [Authorize] endpoints; /api is gated by ApiAuthorizationFilter, /iptv by its own + // ConditionalIptvAuthorizeFilter). UseRateLimiter enforces the "auth" per-IP policy on the + // login/setup/password endpoints. + legacy.UseAuthentication(); + legacy.UseAuthorization(); + legacy.UseRateLimiter(); legacy.UseEndpoints(endpoints => { @@ -1061,6 +1145,9 @@ public class Startup // run-once/blocking startup services services.AddHostedService(); services.AddHostedService(); + // Waits on SystemStartup.WaitForDatabase before seeding (#295 env seed) — the migrator is a + // BackgroundService, so registration order alone does not guarantee the schema exists. + services.AddHostedService(); services.AddHostedService(); services.AddHostedService(); services.AddHostedService(); diff --git a/docs/api-conventions.md b/docs/api-conventions.md index b8e301bc4..4920318e3 100644 --- a/docs/api-conventions.md +++ b/docs/api-conventions.md @@ -512,38 +512,82 @@ for items that have no group, so the SPA can render an "ungrouped" bucket concept elsewhere, this is the established pattern to follow — but be aware it means `Id` is not a reliable real-entity id for those synthetic rows. -## 9. Authentication — API key posture (fail-closed) +## 9. Authentication — session-or-key posture (fail-closed) -The whole `/api` surface is gated by the `X-Api-Key` header via the global `ApiKeyAuthorizationFilter` -(issue #197 Bundle A). When you add an endpoint: +The whole `/api` surface is gated by the global `ApiAuthorizationFilter` (renamed from +`ApiKeyAuthorizationFilter` in #295). A request that requires authentication is accepted on **either** +credential: -- **Do nothing** for the common case. Writes (POST/PUT/PATCH/DELETE) always require the key - (fail-closed — there is no "open" mode). Reads (GET/HEAD) require the key when - `Api:RequireKeyForReads` is enabled, which is the **default** (`true`). `OPTIONS` preflight is exempt. -- The effective key comes from `IApiKeyProvider` (`ErsatzTV/Services/ApiKeyProvider.cs`): `Api:WriteKey` - if configured, else a key persisted at `FileSystemLayout.ApiKeyPath` (`/config/api.key`, `0600`), else - a freshly generated 256-bit key. It is never empty. -- **Sensitive-read GETs** that disclose secrets/paths or trigger work must carry `[RequiresApiKey]` so - they stay gated even if an operator sets `Api:RequireKeyForReads=false`. Current tier: - `Troubleshoot`/`Logs`/`Settings`/`Maintenance`. `ApiControllerSecurityTests` asserts this reflectively. -- **Internal loopback callbacks** (the scanner's `/api/scan/*`) use `[SkipApiKeyAuthorization]` + - `[LocalhostOnly]` — the API key is a poor fit for a co-located child process, so the gate is the - loopback check (sound only because `ForwardedHeaders` trust is restricted via - `ForwardedHeaders:KnownProxies`/`KnownNetworks`). +1. a matching machine **`X-Api-Key`** header (MCP / external clients — issue #197 Bundle A), or +2. an **authenticated session** principal (browser cookie `ctv-session`, from local login or OIDC — #295). + +**Which endpoints require authentication is unchanged** and still decided by the single shared predicate +`ApiAuthorizationFilter.EndpointRequiresKey(httpMethod, endpointMetadata, requireKeyForReads)` (also used by +OpenAPI generation, so the spec can't drift). When you add an endpoint: + +- **Do nothing** for the common case. Writes (POST/PUT/PATCH/DELETE) always require a credential + (fail-closed — there is no "open" mode). Reads (GET/HEAD) require one when `Api:RequireKeyForReads` is + enabled, the **default** (`true`). `OPTIONS` preflight is exempt. +- The effective machine key comes from `IApiKeyProvider` (`ErsatzTV/Services/ApiKeyProvider.cs`): + `Api:WriteKey` if configured, else a key persisted at `FileSystemLayout.ApiKeyPath` (`/config/api.key`, + `0600`), else a freshly generated 256-bit key. It is never empty. +- **CSRF (session credential only).** The machine key is CSRF-immune (a browser can't set a custom header + cross-origin without a credentialed CORS grant, which is never issued). A cookie session is not: a + **session-authenticated mutation** must additionally carry the `X-CSRF` header (`ApiAuthorizationFilter.CsrfHeaderName`) + or it is rejected **403** — presence is the whole check (a custom header forces a CORS preflight a + cross-site page can't satisfy; reinforced by `SameSite=Lax` + CORS without `AllowCredentials`). Key-authed + requests are exempt. When you add a SPA mutation, send `X-CSRF: 1` (the SPA client does this centrally). +- **Sensitive-read GETs** that disclose secrets/paths or trigger work must carry `[RequiresAuthentication]` + (renamed from `[RequiresApiKey]`) so they stay gated even if an operator sets `Api:RequireKeyForReads=false`. + A valid session satisfies this tier just as the key does. Current tier: `Troubleshoot`/`Logs`/`Settings`/ + `Maintenance`. `ApiControllerSecurityTests` asserts this reflectively. +- **Internal loopback callbacks** (the scanner's `/api/scan/*`) and the **`/api/auth/*` surface itself** use + `[SkipApiAuthorization]` (renamed from `[SkipApiKeyAuthorization]`). The scanner adds `[LocalhostOnly]`; the + auth surface must be reachable before a caller is authenticated, and its one sensitive action + (`POST /api/auth/password`) self-checks the principal. `ApiControllerSecurityTests` asserts these two are the + **only** auth-exempt controllers. +- **The `/api/auth/*` surface** (`AuthController`, `[ApiExplorerSettings(IgnoreApi = true)]` → excluded from the + OpenAPI doc, whose audience is machine clients): `GET config` (what auth options exist + `setupRequired`), + `GET session`, `POST setup` (first-run claim), `POST login`, `POST logout`, `POST password`. The browser-nav + OIDC challenge is `GET /auth/oidc/login` (outside `/api`). `login`/`setup`/`password` carry a per-IP rate + limit (`[EnableRateLimiting("auth")]`, 10 / 5 min). The local admin is a single credential in `ConfigElement` + rows (`AuthLocalAdminUsername`/`AuthLocalAdminPasswordHash` (PBKDF2) / `AuthSecurityStamp`) — **no DB + migration**; a password change rotates the stamp, revoking sessions via the cookie `OnValidatePrincipal`. + Recovery/bootstrap without the browser: set `Auth:LocalAdmin:Password` (+ optional `…:Username`, default + `admin`) and restart (`LocalAdminSeedService` reseeds + rotates the stamp). While that env is set the + **browser setup-claim is disabled** (409) — the env seed owns the credential, which also removes the + startup setup-vs-seed race. Logout rotates the security stamp for a local session (ends it server-side, + "log out everywhere"), gated on an authenticated session. - **CORS** is opt-in: no cross-origin access by default (the SPA is same-origin from `/app`); set - `Api:CorsAllowedOrigins` (semicolon-separated exact origins) to allow specific browser origins — the - policy already permits `X-Api-Key`/`If-Match` and exposes `ETag`. -- The SPA sends the stored key (`ctv-api-key`) on **every** request; users enter it on the keyless - **API Key** screen (`web/src/screens/ApiKeyScreen.tsx`, route `/app/api-key`). See spa-conventions §5e. + `Api:CorsAllowedOrigins` (semicolon-separated exact origins). `AllowCredentials` is deliberately **not** + set — cross-origin cookie auth is impossible by design (a CSRF defense); cross-origin machine clients use + `X-Api-Key`. The policy permits `X-Api-Key`/`X-CSRF`/`If-Match` and exposes `ETag`. +- **ForwardedHeaders trust is unchanged from #285** (trust `X-Forwarded-*` from any peer by default, with a + warning; restrict via `ForwardedHeaders:KnownProxies`/`:KnownNetworks`). A stricter "ignore unless a proxy is + configured" default was considered for #295 but **reverted** — it would regress `/iptv` M3U/XMLTV/HLS + absolute-URL generation (which reads `Request.Scheme`/`Host`) for a proxied deployment that hasn't set + `KnownProxies`. **Strongly set `KnownProxies`/`:KnownNetworks`** when exposing ErsatzTV behind a proxy — it + also gives the #295 login rate limiter an unspoofable client IP and lets the session cookie be marked + `Secure` behind TLS. +- **Side-effecting GET endpoints are not CSRF-covered by the filter** (it only checks mutating verbs). A few + `[RequiresAuthentication]` GETs have side effects (e.g. `GET /api/troubleshoot/playback.m3u8` starts playback). + Once a session cookie is a normal SPA credential (PR2), those need POST-ification or an explicit `X-CSRF` gate + — tracked as **#301**, which gates PR2 (latent in PR1: the SPA still uses the machine key, so no session + reaches them in normal use). +- **PR1 scope note (this change is server-only).** The SPA still authenticates with the stored key + (`ctv-api-key`, `X-Api-Key`) until the SPA login flow lands (PR2) — PR1 is backward compatible. The + `ApiKeyScreen` → machine-key-management repurpose, the SPA login/setup screens, and `spa-conventions §5e` + update all land in PR2. -**The OpenAPI "v1" document now declares this posture by construction (#287).** An `ApiKey` security +**The OpenAPI "v1" document declares the machine posture by construction (#287).** An `ApiKey` security scheme (`X-Api-Key`, `in: header`) is declared in `components.securitySchemes`, and `ApiSecurityOperationTransformer` injects a per-operation `security` requirement + a documented `401` -for exactly the operations that require the key — using the **same** shared predicate -`ApiKeyAuthorizationFilter.EndpointRequiresKey(httpMethod, endpointMetadata, requireKeyForReads)` that -the runtime filter enforces, so the spec can never drift from enforcement. The document is generated -against the effective default (`Api:RequireKeyForReads=true`), under which every documented operation -requires the key. Two companion transformers run on the "v1" document only: +for exactly the operations that require a credential — using the **same** shared predicate +`ApiAuthorizationFilter.EndpointRequiresKey(...)` the runtime filter enforces, so the spec can never drift from +enforcement. The document is generated against the effective default (`Api:RequireKeyForReads=true`), under +which every documented operation requires the credential; the browser-session path is an additional accepted +credential the spec (machine audience) needn't express. Two companion transformers run on the "v1" document +only: `OperationIdOpenApiTransformer` synthesizes a stable `operationId` (from controller+action) for the ~90 operations that lacked a `Name=` — and disambiguates the HEAD/GET pairs that share a controller+action **structurally, independent of ApiExplorer visitation order** (#197 Bundle C): when 2+ synthesized ops diff --git a/docs/decisions.md b/docs/decisions.md index b81f80e16..01f7e710f 100644 --- a/docs/decisions.md +++ b/docs/decisions.md @@ -53,6 +53,7 @@ below (or that establishes a new convention worth recording).** - [2026-07-12 — Artwork content-type is sniffed, never reflected (#283, S4/S9 stored XSS)](#2026-07-12--artwork-content-type-is-sniffed-never-reflected-283-s4s9-stored-xss) - [2026-07-12 — Fail-closed API auth + sensitive-read tier + CORS/ForwardedHeaders lockdown (#197 Bundle A, PR #292)](#2026-07-12--fail-closed-api-auth--sensitive-read-tier--corsforwardedheaders-lockdown-197-bundle-a-pr-292) - [2026-07-12 (#197 Bundle C — contract-freeze honesty)](#2026-07-12-197-bundle-c--contract-freeze-honesty) +- [2026-07-12 — Browser SPA session auth: `/api` accepts session OR machine key (#295 PR1, server-only)](#2026-07-12--browser-spa-session-auth-api-accepts-session-or-machine-key-295-pr1-server-only) --- @@ -1133,3 +1134,102 @@ so the immutable int PK is the canonical key for all `/api/channels/*` single-it headers. `Number` remains the identity on broadcast surfaces only (IPTV/M3U/XMLTV), a separate contract. A number-based lookup endpoint may be added additively later; `UniqueId` (Guid) stays out of the REST contract absent a federation requirement. + +## 2026-07-12 — Browser SPA session auth: `/api` accepts session OR machine key (#295 PR1, server-only) + +Implements the ratified #295 design (Fable [PLAN-MODE] pass, issue comment 9548). Supersedes the #206 +"OIDC wiring stays inert until #197" note: the retained OIDC service registration is now **revived**, and a +cookie session becomes a first-class `/api` credential alongside the machine `X-Api-Key`. **PR1 is +server-only and backward compatible** — the SPA keeps sending its stored key; the SPA login flow, the +`ApiKeyScreen`→machine-key repurpose, and `spa-conventions §5e` land in **PR2**. + +**One gate, evolved (not `[Authorize]`-per-controller).** `ApiKeyAuthorizationFilter` → `ApiAuthorizationFilter`, +same fail-closed-by-omission logic (a forgotten `[Authorize]` fails *open* — the #280 failure mode — so the +global filter stays the gate). It now accepts a request when a valid `X-Api-Key` matches **OR** the principal +is an authenticated session; the "does this endpoint need auth?" decision is still the single shared +`EndpointRequiresKey(...)` predicate (also drives OpenAPI, so the spec can't drift). Attributes renamed to +match the widened meaning: `[RequiresApiKey]`→`[RequiresAuthentication]`, `[SkipApiKeyAuthorization]`→ +`[SkipApiAuthorization]`. `IApiKeyProvider`, the `X-Api-Key` header, and `Api:WriteKey`/`Api:RequireKeyForReads` +are unchanged — **machine/key behavior is byte-identical** (verified: no OpenAPI drift, existing filter tests +still green). + +**CSRF (session only).** The machine key is CSRF-immune (a browser can't set a custom header cross-origin +without a credentialed CORS grant we never issue). A cookie session is not: a session-authenticated **mutation** +must carry the `X-CSRF` header (presence-only — a custom header forces a CORS preflight a cross-site page can't +satisfy) or is rejected **403**. Reinforced by `SameSite=Lax` + CORS without `AllowCredentials` (cross-origin +cookie auth is impossible by design). No antiforgery-token machinery. + +**Cookie `ctv-session`.** Always registered (local login works with no IdP); OIDC handler added only when +`OIDC:*` is configured. `HttpOnly`, `SameSite=Lax`, `SecurePolicy=SameAsRequest` (so a plain-HTTP LAN isn't +bricked), 14-day sliding. `/api` XHR gets **401/403, not a redirect** (`OnRedirectToLogin`/`AccessDenied`). +The `UseAuthentication`/`UseAuthorization` middleware — deleted with Blazor in #91b — is **revived in the +`legacy` `MapWhen` branch only** (hosts `/api` + OIDC `/callback` + `/docs`; `/iptv` and `/app` untouched). + +**Local store = `ConfigElement` rows, single admin, NO migration** (owner ruling F2): +`AuthLocalAdminUsername`, `AuthLocalAdminPasswordHash` (ASP.NET `PasswordHasher`, PBKDF2, via +`Microsoft.Extensions.Identity.Core`), `AuthSecurityStamp`. A password change rotates the stamp; the cookie +`OnValidatePrincipal` (`CookieSecurityStampValidator`) compares the claim to the stored stamp and rejects a +stale session (revocation). OIDC sessions carry an `etv:auth_method=oidc` claim and skip the stamp check +(governed by the IdP). + +**Fail-closed out of the box + recovery.** An unconfigured instance keeps `/api` gated (the key still works); +first-run is a **setup-claim** (`POST /api/auth/setup`, first-claim-wins, only valid while unconfigured — +owner ruling F1). Recovery without the browser: `Auth:LocalAdmin:Password` env seed (`LocalAdminSeedService`, +overwrites + rotates the stamp on startup) or the machine key. Login hardening: per-IP rate limit +(`[EnableRateLimiting("auth")]`, 10 / 5 min) on login/setup/password, dummy-hash verify on unknown/unconfigured +user (no enumeration). + +**Authelia = app-owned OIDC session; never trust proxy identity headers** (owner ruling F3): the container is +LAN-reachable bypassing the proxy, so `Remote-User`/`Remote-Email` header trust is spoofable. OIDC→Authelia +gives SSO without a double login. **`ForwardedHeaders` behaviour is kept unchanged from #285** (trust any peer +with a warning; restrict via `KnownProxies`/`:KnownNetworks`). A stricter "ignore `X-Forwarded-*` unless a proxy +is configured" default was implemented and then **reverted** after review (cold fork M1): the forwarded +scheme/host feed `/iptv` M3U/XMLTV/HLS absolute-URL generation (`Request.Scheme` in `GetChannelGuideHandler`/ +`IptvController`), so ignoring them would regress stream URLs to `http`/internal-host for a proxied deployment +that hasn't set `KnownProxies`. **Deployment coordination:** operators behind a proxy should set +`ForwardedHeaders:KnownProxies`/`:KnownNetworks` — it gives the login rate limiter an unspoofable client IP and +marks the session cookie `Secure` behind TLS. The residual (a direct LAN peer can spoof `X-Forwarded-For` to +evade the per-IP login limit when unrestricted) is accepted defense-in-depth loss, mitigated by PBKDF2 + +no-enumeration. + +**Review hardening (fork + independent Codex pass, folded into PR1).** Codex caught concurrency defects the +fork missed — folded in: (a) **atomic first-claim-wins** — setup writes the three credential rows in one +transaction guarded by the unique `ConfigElement.Key` index (a lost race → `DbUpdateException` → 409), so a +concurrent claim can't produce a mixed-state credential; (b) **consistent login snapshot** — login reads the +hash + stamp in one query and no longer rehashes-on-verify, so a login racing a password change can't capture a +newer stamp than the hash it verified (a concurrent change either fails the old password or leaves the issued +cookie carrying the pre-change stamp → revoked next request); (c) **env-seed waits on +`SystemStartup.WaitForDatabase`** (the migrator is a `BackgroundService`, so registration order alone didn't +guarantee the schema existed) — moved to `Services/RunOnce/`. Also: **logout + password require `X-CSRF`** +(the `[SkipApiAuthorization]` auth surface isn't covered by the filter's CSRF check → forced-logout CSRF), and +input length caps on username/password. **Logout rotates the security stamp** when called from a local session +(E2E-caught: `SignOutAsync` alone only clears the *client* cookie, leaving the stateless encrypted ticket +replayable server-side) — so signing out actually ends the session server-side; for the single admin this +revokes all local sessions ("log out everywhere"). Gated on an authenticated session so an unauthenticated +caller can't force-revoke the admin. **Deferred with a tracked gate:** side-effecting `[RequiresAuthentication]` +GETs (troubleshoot playback/archive) aren't CSRF-covered — **#301**, gating PR2 (latent in PR1: the SPA still +uses the machine key). OIDC-session revocation lever (no local stamp) noted for PR3 operator docs. + +A **fix-commit re-review** (Codex, #242 discipline) then confirmed the above resolved and caught a second round: +(a) **HIGH — env-seed vs. setup race**: an attacker could claim admin in the startup window before +`LocalAdminSeedService` runs, and the seed's insert would then be swallowed (attacker's credential persists, +defeating the env recovery path). Fixed structurally: **the setup-claim endpoint is closed whenever +`Auth:LocalAdmin:Password` is configured** — the env seed owns the credential, so there is no claim to race +(this also strengthens the setup-claim TOFU posture: an operator on an untrusted network sets the env password +and browser setup is disabled). (b) **LOW**: a concurrent setup race-loser now returns **409** (not 422), and +`ClaimLocalAdmin`'s `DbUpdateException` catch re-checks existence and **rethrows genuine/transient DB errors** +rather than masking them as "already configured". (c) **MEDIUM — accepted**: two *simultaneous* authenticated +password changes are a non-serializable lost-update (last-write-wins; the loser's cookie may be immediately +revoked). Accepted for a **single-admin** system: it needs two concurrent authenticated sessions both submitting +the correct current password at the same instant, and the outcome is self-healing (re-login). Adding EF +optimistic concurrency to the credential rows is disproportionate here. + +**OpenAPI = `ApiKey`-only; `/api/auth/*` excluded** (owner ruling F4): the spec's audience is machine/MCP +clients, and a browser-interactive cookie login isn't something a generated client drives, so the cookie path +is an additional accepted credential the doc needn't express. `AuthController` is `[ApiExplorerSettings(IgnoreApi += true)]`. Verified: no `v1.json`/`v1.d.ts`/`endpoint-index` drift from this PR. + +**Phasing.** PR1 = this (server only, no migration). PR2 = SPA (drop the key header for browser calls + add +`X-CSRF`, `AuthContext` + boot gate, login/setup screens, `ApiKeyScreen`→machine-key management, E2E, +`spa-conventions §5e`). PR3 = key rotation + operator docs (Authelia client + env reference). Rollout: PR1→PR2 +same release, then a manual Authelia round-trip checklist before the prod pin bump.