diff --git a/docs/screenshots/settings-instances.jpg b/docs/screenshots/settings-instances.jpg new file mode 100644 index 0000000..4f106c5 Binary files /dev/null and b/docs/screenshots/settings-instances.jpg differ diff --git a/docs/screenshots/settings-search-advanced.jpg b/docs/screenshots/settings-search-advanced.jpg new file mode 100644 index 0000000..322e862 Binary files /dev/null and b/docs/screenshots/settings-search-advanced.jpg differ diff --git a/docs/screenshots/settings-test-failure.jpg b/docs/screenshots/settings-test-failure.jpg new file mode 100644 index 0000000..8a3821b Binary files /dev/null and b/docs/screenshots/settings-test-failure.jpg differ diff --git a/src/Application/ApplicationServiceExtensions.cs b/src/Application/ApplicationServiceExtensions.cs index e073a8a..55e1571 100644 --- a/src/Application/ApplicationServiceExtensions.cs +++ b/src/Application/ApplicationServiceExtensions.cs @@ -35,6 +35,12 @@ public static IServiceCollection AddApplication(this IServiceCollection services // Settings services.AddScoped(); services.AddScoped(); + // Instance CRUD only needs IArrInstanceRepository, which every host gets from AddInfrastructure, + // so these are safe here. + services.AddScoped(); + services.AddScoped(); + services.AddScoped(); + // NB: TestArrConnectionHandler is registered by the Web host only — it needs IArrClient, which // only a host that calls AddArrClient() provides. Registering it here made every host fail to // start under DI validate-on-build (i.e. in Development). diff --git a/src/Application/Krautwatch.Application.csproj b/src/Application/Krautwatch.Application.csproj index 5734a92..858f4fa 100644 --- a/src/Application/Krautwatch.Application.csproj +++ b/src/Application/Krautwatch.Application.csproj @@ -13,4 +13,11 @@ + + + + <_Parameter1>Krautwatch.Application.Tests + + + diff --git a/src/Application/Settings/ArrInstances.cs b/src/Application/Settings/ArrInstances.cs new file mode 100644 index 0000000..ba6d6eb --- /dev/null +++ b/src/Application/Settings/ArrInstances.cs @@ -0,0 +1,158 @@ +using FluentValidation; +using Krautwatch.Domain.Entities; +using Krautwatch.Domain.Enums; +using Krautwatch.Domain.Interfaces; + +namespace Krautwatch.Application.Settings; + +// ══════════════════════════════════════════════════════════════ +// DTOs +// ══════════════════════════════════════════════════════════════ + +/// +/// A configured instance, shaped for display. Carries a **masked** API key only. +/// +/// +/// The full key is deliberately absent from every read model. It is a credential, and the UI has no reason +/// to hand it back — masking means the settings page cannot be used to harvest keys, only to set them. See +/// #60 for encrypting them at rest, which is a separate concern. +/// +public record ArrInstanceResponse( + Guid Id, + string Name, + ArrKind Kind, + string BaseUrl, + string ApiKeyMasked, + bool Enabled, + DateTimeOffset? LastTestedAt, + bool? LastTestOk, + string? LastTestMessage); + +/// +/// Add or update an instance. A blank on an update means "leave it unchanged", so +/// editing a name does not require re-typing the credential. +/// +public record SaveArrInstanceRequest( + Guid? Id, + string Name, + ArrKind Kind, + string BaseUrl, + string? ApiKey, + bool Enabled); + +// ══════════════════════════════════════════════════════════════ +// Validator +// ══════════════════════════════════════════════════════════════ + +public class SaveArrInstanceRequestValidator : AbstractValidator +{ + public SaveArrInstanceRequestValidator() + { + RuleFor(x => x.Name) + .NotEmpty().WithMessage("Name must not be empty.") + .MaximumLength(100).WithMessage("Name must be 100 characters or fewer."); + + RuleFor(x => x.BaseUrl) + .NotEmpty().WithMessage("Base URL must not be empty.") + .MaximumLength(500).WithMessage("Base URL must be 500 characters or fewer.") + .Must(BeAnAbsoluteHttpUrl) + .WithMessage("Base URL must be absolute and start with http:// or https:// — for example " + + "http://sonarr:8989."); + + // Required on create, optional on update (blank = unchanged). + RuleFor(x => x.ApiKey) + .NotEmpty().When(x => x.Id is null) + .WithMessage("API key is required."); + + RuleFor(x => x.ApiKey) + .MaximumLength(200).WithMessage("API key must be 200 characters or fewer."); + } + + private static bool BeAnAbsoluteHttpUrl(string? url) => + !string.IsNullOrWhiteSpace(url) + && Uri.TryCreate(url.Trim(), UriKind.Absolute, out var parsed) + && (parsed.Scheme == Uri.UriSchemeHttp || parsed.Scheme == Uri.UriSchemeHttps); +} + +// ══════════════════════════════════════════════════════════════ +// Handlers +// ══════════════════════════════════════════════════════════════ + +public class GetArrInstancesHandler(IArrInstanceRepository repository) +{ + public async Task> HandleAsync(CancellationToken ct = default) => + (await repository.GetAllAsync(ct)).Select(ArrInstanceMapper.ToResponse).ToList(); +} + +public class SaveArrInstanceHandler(IArrInstanceRepository repository) +{ + /// Creates or updates an instance. Returns null when the id no longer exists. + public async Task HandleAsync( + SaveArrInstanceRequest request, + CancellationToken ct = default) + { + var baseUrl = request.BaseUrl.Trim().TrimEnd('/'); + + if (request.Id is null) + { + var created = new ArrInstance + { + Name = request.Name.Trim(), + Kind = request.Kind, + BaseUrl = baseUrl, + ApiKey = request.ApiKey!.Trim(), + Enabled = request.Enabled, + }; + await repository.AddAsync(created, ct); + return ArrInstanceMapper.ToResponse(created); + } + + var existing = await repository.GetByIdAsync(request.Id.Value, ct); + if (existing is null) + return null; + + existing.Name = request.Name.Trim(); + existing.Kind = request.Kind; + existing.BaseUrl = baseUrl; + existing.Enabled = request.Enabled; + + // Blank means unchanged — the UI never receives the real key back, so it cannot echo it. + if (!string.IsNullOrWhiteSpace(request.ApiKey)) + existing.ApiKey = request.ApiKey.Trim(); + + await repository.UpdateAsync(existing, ct); + return ArrInstanceMapper.ToResponse(existing); + } +} + +public class DeleteArrInstanceHandler(IArrInstanceRepository repository) +{ + public Task HandleAsync(Guid id, CancellationToken ct = default) => repository.DeleteAsync(id, ct); +} + +internal static class ArrInstanceMapper +{ + public static ArrInstanceResponse ToResponse(ArrInstance i) => new( + Id: i.Id, + Name: i.Name, + Kind: i.Kind, + BaseUrl: i.BaseUrl, + ApiKeyMasked: Mask(i.ApiKey), + Enabled: i.Enabled, + LastTestedAt: i.LastTestedAt, + LastTestOk: i.LastTestOk, + LastTestMessage: i.LastTestMessage); + + /// + /// Shows only the last four characters — enough to tell two keys apart when checking which one is + /// configured, without disclosing anything usable. + /// + internal static string Mask(string? apiKey) + { + if (string.IsNullOrEmpty(apiKey)) + return string.Empty; + + // Too short to reveal any of it and still be a mask. + return apiKey.Length <= 4 ? "••••" : "••••" + apiKey[^4..]; + } +} diff --git a/src/Presentation/Web/Components/Layout/NavMenu.razor b/src/Presentation/Web/Components/Layout/NavMenu.razor index 3cfbbbb..be42a98 100644 --- a/src/Presentation/Web/Components/Layout/NavMenu.razor +++ b/src/Presentation/Web/Components/Layout/NavMenu.razor @@ -8,6 +8,7 @@ Home Search Activity + Settings diff --git a/src/Presentation/Web/Components/Pages/Settings.razor b/src/Presentation/Web/Components/Pages/Settings.razor new file mode 100644 index 0000000..1c6a856 --- /dev/null +++ b/src/Presentation/Web/Components/Pages/Settings.razor @@ -0,0 +1,424 @@ +@page "/settings" +@rendermode InteractiveServer +@attribute [Authorize] +@inject IServiceScopeFactory Scopes +@using Krautwatch.Application.Settings +@using Krautwatch.Domain.Enums + +Settings · Krautwatch + +

Settings

+ +@* ── Sonarr / Radarr instances ─────────────────────────────── *@ +
+

Sonarr / Radarr instances

+

+ Krautwatch calls these to verify connectivity, and optionally to pre-warm the crawl list + (#6). Searching and downloading work + without any instance configured. +

+ + @if (_instances is null) + { +

Loading…

+ } + else if (_instances.Count == 0) + { +

No instances configured.

+ } + else + { + + + + + + + + + @foreach (var i in _instances) + { + + + + + + + + + } + +
NameTypeBase URLAPI keyLast testActions
@i.Name@i.Kind@i.BaseUrl@i.ApiKeyMasked + @if (i.LastTestOk == true) + { + ✓ @i.LastTestMessage + } + else if (i.LastTestOk == false) + { + ✕ @i.LastTestMessage + } + else + { + never tested + } + + + + +
+ } + + @if (_editing is null) + { + + } + else + { +
+

@(_editing.Id is null ? "Add instance" : "Edit instance")

+ + + + + + + + + + + + @if (_instanceErrors.Count > 0) + { + + } + @if (_instanceNotice is not null) + { +

@_instanceNotice

+ } + +
+ + + +
+
+ } +
+ +@* ── Search ────────────────────────────────────────────────── *@ +
+

Search

+

+ When Sonarr searches for a show nothing has crawled yet, Krautwatch resolves it live against the + broadcasters. How the first search behaves is up to you. +

+ + @if (_settings is not null) + { + + + + + @if (_waitMode == SearchWaitMode.ReturnFast) + { +
+ Advanced + +
+ } + } +
+ +@* ── Downloads ─────────────────────────────────────────────── *@ +
+

Downloads

+ + @if (_settings is null) + { +

Loading…

+ } + else + { + + + + + + } +
+ +@if (_settingsErrors.Count > 0) +{ + +} +@if (_settingsNotice is not null) +{ +

@_settingsNotice

+} + + + +@code { + IReadOnlyList? _instances; + SettingsResponse? _settings; + + // Instance editor + InstanceDraft? _editing; + readonly List _instanceErrors = []; + string? _instanceNotice; + bool _instanceNoticeOk; + Guid? _testing; + bool _testingDraft; + + // Settings form — bound separately so a failed validation doesn't lose the user's edits. + string _downloadDirectory = ""; + int _maxConcurrent; + int _refreshHours; + SearchWaitMode _waitMode; + int _waitSeconds; + readonly List _settingsErrors = []; + string? _settingsNotice; + + protected override async Task OnInitializedAsync() + { + await LoadInstancesAsync(); + await LoadSettingsAsync(); + } + + async Task LoadInstancesAsync() + { + using var scope = Scopes.CreateScope(); + _instances = await scope.ServiceProvider + .GetRequiredService().HandleAsync(); + } + + async Task LoadSettingsAsync() + { + using var scope = Scopes.CreateScope(); + _settings = await scope.ServiceProvider.GetRequiredService().HandleAsync(); + + _downloadDirectory = _settings.DownloadDirectory; + _maxConcurrent = _settings.MaxConcurrentDownloads; + _refreshHours = _settings.CatalogRefreshIntervalHours; + _waitMode = _settings.SearchWaitMode; + _waitSeconds = _settings.SearchWaitSeconds; + } + + void AddNew() + { + _instanceErrors.Clear(); + _instanceNotice = null; + _editing = new InstanceDraft(); + } + + void Edit(ArrInstanceResponse i) + { + _instanceErrors.Clear(); + _instanceNotice = null; + // ApiKey deliberately left blank: the real key is never sent to the browser, and blank means + // "unchanged" on save. + _editing = new InstanceDraft + { + Id = i.Id, Name = i.Name, Kind = i.Kind, BaseUrl = i.BaseUrl, Enabled = i.Enabled, + }; + } + + void CancelEdit() + { + _editing = null; + _instanceErrors.Clear(); + _instanceNotice = null; + } + + async Task SaveInstanceAsync() + { + if (_editing is null) return; + _instanceErrors.Clear(); + _instanceNotice = null; + + var request = _editing.ToRequest(); + var validation = new SaveArrInstanceRequestValidator().Validate(request); + if (!validation.IsValid) + { + _instanceErrors.AddRange(validation.Errors.Select(e => e.ErrorMessage)); + return; + } + + using var scope = Scopes.CreateScope(); + var saved = await scope.ServiceProvider + .GetRequiredService().HandleAsync(request); + + if (saved is null) + { + _instanceErrors.Add("That instance no longer exists."); + return; + } + + _editing = null; + await LoadInstancesAsync(); + } + + async Task DeleteAsync(Guid id) + { + using var scope = Scopes.CreateScope(); + await scope.ServiceProvider.GetRequiredService().HandleAsync(id); + if (_editing?.Id == id) _editing = null; + await LoadInstancesAsync(); + } + + async Task TestAsync(Guid id) + { + _testing = id; + try + { + using var scope = Scopes.CreateScope(); + await scope.ServiceProvider.GetRequiredService().HandleAsync(id); + await LoadInstancesAsync(); // the outcome is cached on the row + } + finally + { + _testing = null; + } + } + + async Task TestDraftAsync() + { + if (_editing is null) return; + _instanceErrors.Clear(); + _instanceNotice = null; + + // Testing unsaved details means a wrong key never has to be persisted to discover it is wrong. + // Requires an explicit key, since the stored one is never sent to the browser. + if (string.IsNullOrWhiteSpace(_editing.ApiKey)) + { + _instanceErrors.Add("Enter the API key to test without saving."); + return; + } + + _testingDraft = true; + try + { + using var scope = Scopes.CreateScope(); + var result = await scope.ServiceProvider + .GetRequiredService() + .HandleAsync(_editing.ToEntity()); + + _instanceNoticeOk = result.Ok; + _instanceNotice = result.Ok ? $"✓ {result.Message}" : $"✕ {result.Message}"; + } + finally + { + _testingDraft = false; + } + } + + async Task SaveSettingsAsync() + { + _settingsErrors.Clear(); + _settingsNotice = null; + + var request = new SaveSettingsRequest( + _downloadDirectory, _maxConcurrent, _refreshHours, _waitMode, _waitSeconds); + + var validation = new SaveSettingsRequestValidator().Validate(request); + if (!validation.IsValid) + { + _settingsErrors.AddRange(validation.Errors.Select(e => e.ErrorMessage)); + return; + } + + using var scope = Scopes.CreateScope(); + _settings = await scope.ServiceProvider + .GetRequiredService().HandleAsync(request); + _settingsNotice = "Settings saved."; + } + + sealed class InstanceDraft + { + public Guid? Id { get; set; } + public string Name { get; set; } = ""; + public ArrKind Kind { get; set; } = ArrKind.Sonarr; + public string BaseUrl { get; set; } = ""; + public string? ApiKey { get; set; } + public bool Enabled { get; set; } = true; + + public SaveArrInstanceRequest ToRequest() => new(Id, Name, Kind, BaseUrl, ApiKey, Enabled); + + public Krautwatch.Domain.Entities.ArrInstance ToEntity() => new() + { + Name = Name, Kind = Kind, BaseUrl = BaseUrl.Trim().TrimEnd('/'), ApiKey = ApiKey ?? "", + }; + } +} diff --git a/src/Presentation/Web/wwwroot/app.css b/src/Presentation/Web/wwwroot/app.css index a3824c9..abe843d 100644 --- a/src/Presentation/Web/wwwroot/app.css +++ b/src/Presentation/Web/wwwroot/app.css @@ -115,3 +115,35 @@ button.primary:hover { filter: brightness(1.08); } .error { color: #f87171; font-size: 13px; } ul.error { margin: 0 0 16px; padding-left: 18px; } + +/* ── Settings (#4) ───────────────────────────────────────────── */ +.panel { + background: var(--panel); border: 1px solid var(--line); + border-radius: 10px; padding: 20px 22px; margin-bottom: 22px; +} +.panel h2 { margin: 0 0 4px; font-size: 17px; } +.panel h3 { margin: 0 0 14px; font-size: 15px; } +.panel > p { margin: 0 0 16px; } +.panel table { margin-bottom: 16px; } +.panel label { display: block; margin-bottom: 14px; color: var(--muted); font-size: 13px; } +.panel input[type=text], .panel input[type=password], .panel input[type=number], .panel select, .panel input:not([type]) { + display: block; margin-top: 6px; padding: 8px 10px; min-width: 280px; + background: var(--bg); color: var(--text); + border: 1px solid var(--line); border-radius: 6px; font-size: 14px; +} +.panel input:focus, .panel select:focus { outline: none; border-color: var(--accent); } + +label.checkbox, label.radio { display: flex; gap: 9px; align-items: flex-start; } +label.checkbox input, label.radio input { margin-top: 2px; } +label.radio span { display: flex; flex-direction: column; gap: 2px; color: var(--text); } + +.subform { + border-top: 1px solid var(--line); margin-top: 16px; padding-top: 18px; +} +.subform .actions { display: flex; gap: 10px; align-items: center; } + +.advanced { margin: 4px 0 14px; } +.advanced summary { cursor: pointer; } + +button.narrow { width: auto; padding: 8px 16px; } +tr.disabled td { opacity: 0.45; } diff --git a/tests/Application.Tests/ArrInstanceSettingsTests.cs b/tests/Application.Tests/ArrInstanceSettingsTests.cs new file mode 100644 index 0000000..6dc5615 --- /dev/null +++ b/tests/Application.Tests/ArrInstanceSettingsTests.cs @@ -0,0 +1,183 @@ +using Krautwatch.Application.Settings; +using Krautwatch.Domain.Entities; +using Krautwatch.Domain.Enums; +using Krautwatch.Domain.Interfaces; +using NSubstitute; +using Shouldly; +using Xunit; + +namespace Krautwatch.Application.Tests; + +public class ArrInstanceReadModelTests +{ + [Fact] + public async Task The_read_model_never_carries_the_full_api_key() + { + // The security-relevant assertion for this page: masking is what stops the settings UI being usable + // to harvest credentials. + var repo = Substitute.For(); + repo.GetAllAsync(Arg.Any()).Returns([new ArrInstance + { + Name = "Sonarr", Kind = ArrKind.Sonarr, BaseUrl = "http://sonarr:8989", + ApiKey = "supersecretkey1234", + }]); + + var result = await new GetArrInstancesHandler(repo).HandleAsync(TestContext.Current.CancellationToken); + + var instance = result.ShouldHaveSingleItem(); + instance.ApiKeyMasked.ShouldBe("••••1234"); + instance.ApiKeyMasked.ShouldNotContain("supersecret"); + + // Belt and braces: no property anywhere on the DTO holds the real key. + instance.GetType().GetProperties() + .Select(prop => prop.GetValue(instance) as string) + .ShouldNotContain("supersecretkey1234"); + } + + [Theory] + [InlineData("", "")] + [InlineData("abc", "••••")] // too short to reveal any of it + [InlineData("abcd", "••••")] + [InlineData("abcde", "••••bcde")] + public void Masking_never_reveals_more_than_the_last_four_characters(string key, string expected) + { + ArrInstanceMapper.Mask(key).ShouldBe(expected); + } +} + +public class SaveArrInstanceHandlerTests +{ + private static ArrInstance Existing(string apiKey = "original-key") => new() + { + Id = Guid.NewGuid(), Name = "Sonarr", Kind = ArrKind.Sonarr, + BaseUrl = "http://sonarr:8989", ApiKey = apiKey, Enabled = true, + }; + + [Fact] + public async Task Creates_a_new_instance() + { + var repo = Substitute.For(); + + var saved = await new SaveArrInstanceHandler(repo).HandleAsync( + new SaveArrInstanceRequest(null, " Sonarr ", ArrKind.Sonarr, "http://sonarr:8989/", "key", true), + TestContext.Current.CancellationToken); + + saved.ShouldNotBeNull(); + await repo.Received(1).AddAsync( + Arg.Is(i => i != null + && i.Name == "Sonarr" // trimmed + && i.BaseUrl == "http://sonarr:8989"), // trailing slash normalised away + Arg.Any()); + } + + [Fact] + public async Task A_blank_key_on_update_keeps_the_existing_one() + { + // The UI never receives the real key, so it cannot echo it back — blank has to mean "unchanged", + // otherwise editing a name would wipe the credential. + var repo = Substitute.For(); + var existing = Existing(); + repo.GetByIdAsync(existing.Id, Arg.Any()).Returns(existing); + + await new SaveArrInstanceHandler(repo).HandleAsync( + new SaveArrInstanceRequest(existing.Id, "Renamed", ArrKind.Sonarr, "http://sonarr:8989", "", true), + TestContext.Current.CancellationToken); + + existing.Name.ShouldBe("Renamed"); + existing.ApiKey.ShouldBe("original-key"); + } + + [Fact] + public async Task A_supplied_key_on_update_replaces_the_existing_one() + { + var repo = Substitute.For(); + var existing = Existing(); + repo.GetByIdAsync(existing.Id, Arg.Any()).Returns(existing); + + await new SaveArrInstanceHandler(repo).HandleAsync( + new SaveArrInstanceRequest(existing.Id, "Sonarr", ArrKind.Sonarr, "http://sonarr:8989", + " rotated-key ", true), + TestContext.Current.CancellationToken); + + existing.ApiKey.ShouldBe("rotated-key"); // trimmed + } + + [Fact] + public async Task Updating_something_that_no_longer_exists_reports_rather_than_creating_it() + { + var repo = Substitute.For(); + repo.GetByIdAsync(Arg.Any(), Arg.Any()).Returns((ArrInstance?)null); + + var saved = await new SaveArrInstanceHandler(repo).HandleAsync( + new SaveArrInstanceRequest(Guid.NewGuid(), "Gone", ArrKind.Sonarr, "http://x:1", "key", true), + TestContext.Current.CancellationToken); + + saved.ShouldBeNull(); + await repo.DidNotReceive().AddAsync(Arg.Any(), Arg.Any()); + } +} + +public class SaveArrInstanceRequestValidatorTests +{ + private static readonly SaveArrInstanceRequestValidator Validator = new(); + + private static SaveArrInstanceRequest Request( + Guid? id = null, string name = "Sonarr", string baseUrl = "http://sonarr:8989", string? key = "key") => + new(id, name, ArrKind.Sonarr, baseUrl, key, true); + + [Fact] + public void Accepts_a_well_formed_request() => + Validator.Validate(Request()).IsValid.ShouldBeTrue(); + + [Theory] + [InlineData("")] + [InlineData("sonarr:8989")] // no scheme + [InlineData("//sonarr:8989")] // protocol-relative + [InlineData("ftp://sonarr:8989")] // wrong scheme + [InlineData("not a url")] + public void Rejects_a_base_url_that_is_not_absolute_http(string baseUrl) => + Validator.Validate(Request(baseUrl: baseUrl)).IsValid.ShouldBeFalse(); + + [Fact] + public void Accepts_a_reverse_proxy_subpath() => + Validator.Validate(Request(baseUrl: "https://media.example.com/sonarr")).IsValid.ShouldBeTrue(); + + [Fact] + public void Requires_a_key_when_creating() => + Validator.Validate(Request(key: "")).IsValid.ShouldBeFalse(); + + [Fact] + public void Allows_a_blank_key_when_updating() => + Validator.Validate(Request(id: Guid.NewGuid(), key: "")).IsValid.ShouldBeTrue(); + + [Fact] + public void Requires_a_name() => + Validator.Validate(Request(name: "")).IsValid.ShouldBeFalse(); +} + +public class SearchWaitSettingsTests +{ + [Fact] + public async Task Round_trips_the_search_wait_preference() + { + var repo = Substitute.For(); + var settings = new AppSettings(); + repo.GetAsync(Arg.Any()).Returns(settings); + + var saved = await new SaveSettingsHandler(repo).HandleAsync( + new SaveSettingsRequest("/downloads", 2, 6, SearchWaitMode.WaitForComplete, 42), + TestContext.Current.CancellationToken); + + settings.SearchWaitMode.ShouldBe(SearchWaitMode.WaitForComplete); + settings.SearchWaitSeconds.ShouldBe(42); + saved.SearchWaitMode.ShouldBe(SearchWaitMode.WaitForComplete); + } + + [Theory] + [InlineData(0)] + [InlineData(301)] + public void Rejects_an_out_of_range_wait(int seconds) => + new SaveSettingsRequestValidator() + .Validate(new SaveSettingsRequest("/downloads", 2, 6, SearchWaitMode.ReturnFast, seconds)) + .IsValid.ShouldBeFalse(); +}