diff --git a/listenarr.application/Configuration/Core/ConfigurationService.cs b/listenarr.application/Configuration/Core/ConfigurationService.cs index 7adefede2..999de5283 100644 --- a/listenarr.application/Configuration/Core/ConfigurationService.cs +++ b/listenarr.application/Configuration/Core/ConfigurationService.cs @@ -164,6 +164,41 @@ public async Task SaveApplicationSettingsAsync(ApplicationSettings settings) settings.EnabledNotificationTriggers = existing.EnabledNotificationTriggers; if (settings.Webhooks == null) settings.Webhooks = existing.Webhooks; + + // The other fields RedactApplicationSettings covers need the + // same sentinel check as ProwlarrApiKeyEncrypted above. A + // redaction-gated caller is handed RedactedValue for each of + // them by the GET and posts it back verbatim, so without this + // the sentinel overwrites the stored secret. Blank is left + // alone here for the same reason as above: clearing a webhook + // URL or a bot token has to stay possible. + if (string.Equals(settings.WebhookUrl, ApiResponseRedactor.RedactedValue, StringComparison.Ordinal)) + { + settings.WebhookUrl = existing.WebhookUrl; + } + + if (string.Equals(settings.DiscordBotToken, ApiResponseRedactor.RedactedValue, StringComparison.Ordinal)) + { + settings.DiscordBotToken = existing.DiscordBotToken; + } + + if (settings.Webhooks != null && existing.Webhooks != null) + { + foreach (var webhook in settings.Webhooks) + { + if (!string.Equals(webhook.Url, ApiResponseRedactor.RedactedValue, StringComparison.Ordinal)) + { + continue; + } + + var storedWebhook = existing.Webhooks + .FirstOrDefault(candidate => string.Equals(candidate.Id, webhook.Id, StringComparison.Ordinal)); + if (storedWebhook != null) + { + webhook.Url = storedWebhook.Url; + } + } + } } if (!string.IsNullOrWhiteSpace(settings.OutputPath) @@ -357,6 +392,35 @@ public async Task SaveStartupConfigAsync(StartupConfig config) { try { + var currentConfig = startupConfigService.GetConfig(); + + // The GET that populates the settings screen replaces each + // secret with ApiResponseRedactor.RedactedValue for callers the + // redaction gate does not exempt, and the screen posts that same + // document straight back when the operator saves. Treat the + // sentinel as "unchanged" and keep what is already stored, the + // way SaveApplicationSettingsAsync and + // SaveProwlarrImportSettingsAsync already do for the Prowlarr + // key. Without this the literal sentinel lands in config.json + // and the real value is gone. + // + // Only the sentinel is special-cased. A blank or absent field + // still means what it has always meant here, which is clear the + // value, so an operator who empties the API key box can still + // do so. + if (config != null && currentConfig != null) + { + if (string.Equals(config.ApiKey, ApiResponseRedactor.RedactedValue, StringComparison.Ordinal)) + { + config.ApiKey = currentConfig.ApiKey; + } + + if (string.Equals(config.SslCertPassword, ApiResponseRedactor.RedactedValue, StringComparison.Ordinal)) + { + config.SslCertPassword = currentConfig.SslCertPassword; + } + } + // Defense-in-depth backstop against the auth-enable lockout. // SaveApplicationSettingsAsync's throw-on-failure (above) only // covers the case where admin credentials were *supplied* but @@ -378,7 +442,6 @@ public async Task SaveStartupConfigAsync(StartupConfig config) // management path, not here. if (config != null && config.IsAuthenticationEnabled()) { - var currentConfig = startupConfigService.GetConfig(); var wasAuthEnabled = currentConfig?.IsAuthenticationEnabled() == true; if (!wasAuthEnabled) { diff --git a/tests/Features/Application/Configuration/Core/ConfigurationServiceTests.cs b/tests/Features/Application/Configuration/Core/ConfigurationServiceTests.cs index ef292849a..f2e5a91aa 100644 --- a/tests/Features/Application/Configuration/Core/ConfigurationServiceTests.cs +++ b/tests/Features/Application/Configuration/Core/ConfigurationServiceTests.cs @@ -394,6 +394,195 @@ await svc.SaveStartupConfigAsync(new StartupConfig currentConfigEnabled.Verify(s => s.SaveAsync(It.IsAny()), Times.Once); } + [Fact] + public async Task SaveStartupConfig_RedactedSecrets_KeepStoredValues() + { + // GET /configuration/startupconfig hands a redaction-gated caller + // ApiResponseRedactor.RedactedValue in place of the API key and the + // SSL certificate password, and the settings screen posts that same + // document back on save. The sentinel must be read as "unchanged", + // not written to config.json on top of the real secret. + var stored = new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = "stored-api-key", + SslCertPassword = "stored-cert-password", + Port = 5000, + }; + + var startupConfigService = new Mock(); + startupConfigService.Setup(s => s.GetConfig()).Returns(stored); + StartupConfig? saved = null; + startupConfigService.Setup(s => s.SaveAsync(It.IsAny())) + .Callback(c => saved = c) + .Returns(Task.CompletedTask); + + Init(b => b.WithSingleton(startupConfigService.Object)); + var svc = _provider.GetRequiredService(); + + // Build the incoming payload the way the API actually produces it, + // so the test exercises the real redaction sentinel rather than a + // hand-written copy of it. + var incoming = ApiResponseRedactor.RedactStartupConfig(stored); + Assert.Equal(ApiResponseRedactor.RedactedValue, incoming.ApiKey); + Assert.Equal(ApiResponseRedactor.RedactedValue, incoming.SslCertPassword); + + await svc.SaveStartupConfigAsync(incoming); + + Assert.NotNull(saved); + Assert.Equal("stored-api-key", saved!.ApiKey); + Assert.Equal("stored-cert-password", saved.SslCertPassword); + } + + [Fact] + public async Task SaveStartupConfig_NewSecrets_ReplaceStoredValues() + { + // Control for the test above. The sentinel check must not degrade + // into "never update these fields": an operator who types a new API + // key or a new certificate password still has to be able to save it. + var stored = new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = "stored-api-key", + SslCertPassword = "stored-cert-password", + }; + + var startupConfigService = new Mock(); + startupConfigService.Setup(s => s.GetConfig()).Returns(stored); + StartupConfig? saved = null; + startupConfigService.Setup(s => s.SaveAsync(It.IsAny())) + .Callback(c => saved = c) + .Returns(Task.CompletedTask); + + Init(b => b.WithSingleton(startupConfigService.Object)); + var svc = _provider.GetRequiredService(); + + await svc.SaveStartupConfigAsync(new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = "rotated-api-key", + SslCertPassword = "rotated-cert-password", + }); + + Assert.NotNull(saved); + Assert.Equal("rotated-api-key", saved!.ApiKey); + Assert.Equal("rotated-cert-password", saved.SslCertPassword); + } + + [Fact] + public async Task SaveStartupConfig_BlankOrAbsentSecrets_AreWrittenThroughUnchanged() + { + // Second control. Only the sentinel is special-cased; blank keeps + // the meaning it has always had on this path, which is clear the + // value. Preserving on blank as well would take away the operator's + // way of removing an API key or a certificate password. + var stored = new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = "stored-api-key", + SslCertPassword = "stored-cert-password", + }; + + var startupConfigService = new Mock(); + startupConfigService.Setup(s => s.GetConfig()).Returns(stored); + var savedConfigs = new List(); + startupConfigService.Setup(s => s.SaveAsync(It.IsAny())) + .Callback(savedConfigs.Add) + .Returns(Task.CompletedTask); + + Init(b => b.WithSingleton(startupConfigService.Object)); + var svc = _provider.GetRequiredService(); + + await svc.SaveStartupConfigAsync(new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = string.Empty, + SslCertPassword = string.Empty, + }); + await svc.SaveStartupConfigAsync(new StartupConfig + { + AuthenticationRequired = "false", + ApiKey = null, + SslCertPassword = null, + }); + + Assert.Equal(2, savedConfigs.Count); + Assert.Equal(string.Empty, savedConfigs[0].ApiKey); + Assert.Equal(string.Empty, savedConfigs[0].SslCertPassword); + Assert.Null(savedConfigs[1].ApiKey); + Assert.Null(savedConfigs[1].SslCertPassword); + } + + [Fact] + public async Task SaveApplicationSettings_RedactedSecrets_KeepStoredValues() + { + // Same round trip on the settings row. ProwlarrApiKeyEncrypted was + // already guarded; the rest of what RedactApplicationSettings covers + // was not, so the sentinel reached the database. + var svc = _provider.GetRequiredService(); + + var seed = await svc.GetApplicationSettingsAsync(); + seed.WebhookUrl = "https://example.test/global-hook"; + seed.DiscordBotToken = "stored-discord-token"; + seed.Webhooks = + [ + new() { Id = "webhook-1", Name = "First", Url = "https://example.test/one", Type = "Zapier" }, + new() { Id = "webhook-2", Name = "Second", Url = "https://example.test/two", Type = "Slack" }, + ]; + await svc.SaveApplicationSettingsAsync(seed); + + var current = await svc.GetApplicationSettingsAsync(); + var incoming = ApiResponseRedactor.RedactApplicationSettings(current); + Assert.Equal(ApiResponseRedactor.RedactedValue, incoming.WebhookUrl); + Assert.Equal(ApiResponseRedactor.RedactedValue, incoming.DiscordBotToken); + Assert.All(incoming.Webhooks!, w => Assert.Equal(ApiResponseRedactor.RedactedValue, w.Url)); + + await svc.SaveApplicationSettingsAsync(incoming); + + var stored = await _applicationSettingsRepository.GetAsync(); + Assert.NotNull(stored); + Assert.Equal("https://example.test/global-hook", stored!.WebhookUrl); + Assert.Equal("stored-discord-token", stored.DiscordBotToken); + Assert.NotNull(stored.Webhooks); + Assert.Equal( + "https://example.test/one", + stored.Webhooks!.Single(w => w.Id == "webhook-1").Url); + Assert.Equal( + "https://example.test/two", + stored.Webhooks!.Single(w => w.Id == "webhook-2").Url); + } + + [Fact] + public async Task SaveApplicationSettings_NewSecrets_ReplaceStoredValues() + { + // Control for the test above, on the settings row this time. + var svc = _provider.GetRequiredService(); + + var seed = await svc.GetApplicationSettingsAsync(); + seed.WebhookUrl = "https://example.test/global-hook"; + seed.DiscordBotToken = "stored-discord-token"; + seed.Webhooks = + [ + new() { Id = "webhook-1", Name = "First", Url = "https://example.test/one", Type = "Zapier" }, + ]; + await svc.SaveApplicationSettingsAsync(seed); + + var incoming = await svc.GetApplicationSettingsAsync(); + incoming.WebhookUrl = "https://example.test/changed-hook"; + incoming.DiscordBotToken = "rotated-discord-token"; + incoming.Webhooks!.Single(w => w.Id == "webhook-1").Url = "https://example.test/changed-one"; + + await svc.SaveApplicationSettingsAsync(incoming); + + var stored = await _applicationSettingsRepository.GetAsync(); + Assert.NotNull(stored); + Assert.Equal("https://example.test/changed-hook", stored!.WebhookUrl); + Assert.Equal("rotated-discord-token", stored.DiscordBotToken); + Assert.Equal( + "https://example.test/changed-one", + stored.Webhooks!.Single(w => w.Id == "webhook-1").Url); + } + [Fact] public async Task SaveStartupConfig_SkipsAdminCheck_WhenAuthDisabled() {