diff --git a/code/backend/Cleanuparr.Api.Tests/Features/DownloadClient/DownloadClientControllerTests.cs b/code/backend/Cleanuparr.Api.Tests/Features/DownloadClient/DownloadClientControllerTests.cs index 602445b8..be3de7e6 100644 --- a/code/backend/Cleanuparr.Api.Tests/Features/DownloadClient/DownloadClientControllerTests.cs +++ b/code/backend/Cleanuparr.Api.Tests/Features/DownloadClient/DownloadClientControllerTests.cs @@ -270,6 +270,41 @@ public class DownloadClientControllerTests : IDisposable () => _controller.CreateDownloadClientConfig(request)); } + [Fact] + public async Task CreateDownloadClientConfig_HostWithUserInfo_PropagatesValidationException() + { + // Arrange + var request = new CreateDownloadClientRequest + { + Name = "x", + Host = "http://user:pass@localhost:8080", + TypeName = DownloadClientTypeName.qBittorrent, + }; + + // Act / Assert + Cleanuparr.Domain.Exceptions.ValidationException exception = await Should.ThrowAsync( + () => _controller.CreateDownloadClientConfig(request)); + exception.Message.ShouldBe("Credentials in the URL are not supported. Use the HTTP authentication fields instead."); + } + + [Fact] + public async Task CreateDownloadClientConfig_RelativeHost_ReturnsCreated() + { + // Arrange + var request = new CreateDownloadClientRequest + { + Name = "x", + Host = "192.168.1.5", + TypeName = DownloadClientTypeName.qBittorrent, + }; + + // Act + IActionResult result = await _controller.CreateDownloadClientConfig(request); + + // Assert + result.ShouldBeOfType(); + } + [Fact] public async Task CreateDownloadClientConfig_UnknownTypeName_PropagatesValidationException() { @@ -406,6 +441,27 @@ public class DownloadClientControllerTests : IDisposable () => _controller.UpdateDownloadClientConfig(client.Id, request)); } + [Fact] + public async Task UpdateDownloadClientConfig_HostWithUserInfo_PropagatesValidationException() + { + // Arrange + DownloadClientConfig client = NewClient("orig", DownloadClientTypeName.qBittorrent); + _dataContext.DownloadClients.Add(client); + await _dataContext.SaveChangesAsync(); + + UpdateDownloadClientRequest request = new() + { + Name = "renamed", + Host = "http://user:pass@newhost:9090", + TypeName = DownloadClientTypeName.qBittorrent, + }; + + // Act / Assert + Cleanuparr.Domain.Exceptions.ValidationException exception = await Should.ThrowAsync( + () => _controller.UpdateDownloadClientConfig(client.Id, request)); + exception.Message.ShouldBe("Credentials in the URL are not supported. Use the HTTP authentication fields instead."); + } + [Fact] public async Task UpdateDownloadClientConfig_UnknownTypeName_PropagatesValidationException() { @@ -706,6 +762,25 @@ public class DownloadClientControllerTests : IDisposable result.ShouldBeOfType().StatusCode.ShouldBe(400); } + [Fact] + public async Task TestDownloadClient_HostWithUserInfo_ReturnsBadRequest() + { + // Arrange + TestDownloadClientRequest request = new() + { + TypeName = DownloadClientTypeName.qBittorrent, + Host = "http://user:pass@localhost:8080", + Password = "pass", + }; + + // Act + IActionResult result = await _controller.TestDownloadClient(request); + + // Assert + result.ShouldBeOfType().StatusCode.ShouldBe(400); + _downloadServiceFactory.DidNotReceive().GetDownloadService(Arg.Any()); + } + [Fact] public async Task TestDownloadClient_UnknownTypeName_ReturnsBadRequest() { diff --git a/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/CreateDownloadClientRequest.cs b/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/CreateDownloadClientRequest.cs index 81a96b74..7278d1ba 100644 --- a/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/CreateDownloadClientRequest.cs +++ b/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/CreateDownloadClientRequest.cs @@ -54,11 +54,17 @@ public sealed record CreateDownloadClientRequest throw new CustomValidationException("Host cannot be empty"); } - if (!Uri.TryCreate(Host, UriKind.RelativeOrAbsolute, out _)) + if (!Uri.TryCreate(Host, UriKind.RelativeOrAbsolute, out Uri? parsedHost)) { throw new CustomValidationException("Host is not a valid URL"); } + if (parsedHost.IsAbsoluteUri && parsedHost.UserInfo.Length > 0) + { + throw new ValidationException( + "Credentials in the URL are not supported. Use the HTTP authentication fields instead."); + } + if (!string.IsNullOrWhiteSpace(ExternalUrl) && !Uri.TryCreate(ExternalUrl, UriKind.RelativeOrAbsolute, out _)) { throw new CustomValidationException("External URL is not a valid URL"); diff --git a/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/TestDownloadClientRequest.cs b/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/TestDownloadClientRequest.cs index 6b3eb200..f57ff6ba 100644 --- a/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/TestDownloadClientRequest.cs +++ b/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/TestDownloadClientRequest.cs @@ -41,11 +41,17 @@ public sealed record TestDownloadClientRequest throw new CustomValidationException("Host cannot be empty"); } - if (!Uri.TryCreate(Host, UriKind.RelativeOrAbsolute, out _)) + if (!Uri.TryCreate(Host, UriKind.RelativeOrAbsolute, out Uri? parsedHost)) { throw new CustomValidationException("Host is not a valid URL"); } + if (parsedHost.IsAbsoluteUri && parsedHost.UserInfo.Length > 0) + { + throw new ValidationException( + "Credentials in the URL are not supported. Use the HTTP authentication fields instead."); + } + if (TypeName.RequiresAuthField(DownloadClientAuthField.ApiKey) && string.IsNullOrWhiteSpace(ApiKey) && !ApiKey.IsPlaceholder()) { throw new CustomValidationException("API key cannot be empty"); diff --git a/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/UpdateDownloadClientRequest.cs b/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/UpdateDownloadClientRequest.cs index 31a67a2e..056aefa6 100644 --- a/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/UpdateDownloadClientRequest.cs +++ b/code/backend/Cleanuparr.Api/Features/DownloadClient/Contracts/Requests/UpdateDownloadClientRequest.cs @@ -54,11 +54,17 @@ public sealed record UpdateDownloadClientRequest throw new CustomValidationException("Host cannot be empty"); } - if (!Uri.TryCreate(Host, UriKind.RelativeOrAbsolute, out _)) + if (!Uri.TryCreate(Host, UriKind.RelativeOrAbsolute, out Uri? parsedHost)) { throw new CustomValidationException("Host is not a valid URL"); } + if (parsedHost.IsAbsoluteUri && parsedHost.UserInfo.Length > 0) + { + throw new ValidationException( + "Credentials in the URL are not supported. Use the HTTP authentication fields instead."); + } + if (!string.IsNullOrWhiteSpace(ExternalUrl) && !Uri.TryCreate(ExternalUrl, UriKind.RelativeOrAbsolute, out _)) { throw new CustomValidationException("External URL is not a valid URL"); diff --git a/code/backend/Cleanuparr.Api/HostExtensions.cs b/code/backend/Cleanuparr.Api/HostExtensions.cs index fb542932..d517bc9c 100644 --- a/code/backend/Cleanuparr.Api/HostExtensions.cs +++ b/code/backend/Cleanuparr.Api/HostExtensions.cs @@ -2,7 +2,9 @@ using Cleanuparr.Infrastructure.Services; using Cleanuparr.Persistence; using Cleanuparr.Persistence.Providers; +using Cleanuparr.Persistence.Startup; using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; namespace Cleanuparr.Api; @@ -77,6 +79,7 @@ public static class HostExtensions await configContext.Database.MigrateAsync(); } await provider.OnPostMigrateAsync(configContext.Database, DbContextKind.Data, CancellationToken.None); + await MigrateDownloadClientUserInfoAsync(configContext); await using EventsContext eventsContext = EventsContext.CreateStaticInstance(); if ((await eventsContext.Database.GetPendingMigrationsAsync()).Any()) @@ -94,4 +97,15 @@ public static class HostExtensions return builder; } + + /// + /// Moves URL userinfo out of download client hosts. + /// DI does not exist yet, so the migrator gets its own console logger. + /// + private static async Task MigrateDownloadClientUserInfoAsync(DataContext context) + { + using ILoggerFactory loggerFactory = LoggerFactory.Create(builder => builder.AddConsole()); + DownloadClientUserInfoMigrator migrator = new(loggerFactory.CreateLogger()); + await migrator.MigrateAsync(context, CancellationToken.None); + } } \ No newline at end of file diff --git a/code/backend/Cleanuparr.Infrastructure.Tests/Persistence/DownloadClientUserInfoMigrationHeaderProofTests.cs b/code/backend/Cleanuparr.Infrastructure.Tests/Persistence/DownloadClientUserInfoMigrationHeaderProofTests.cs new file mode 100644 index 00000000..a27f8271 --- /dev/null +++ b/code/backend/Cleanuparr.Infrastructure.Tests/Persistence/DownloadClientUserInfoMigrationHeaderProofTests.cs @@ -0,0 +1,117 @@ +using System.Net; +using System.Text; +using Cleanuparr.Domain.Entities.UTorrent.Request; +using Cleanuparr.Domain.Enums; +using Cleanuparr.Infrastructure.Features.DownloadClient.RTorrent; +using Cleanuparr.Infrastructure.Features.DownloadClient.UTorrent; +using Cleanuparr.Infrastructure.Http.DynamicHttpClientSystem; +using Cleanuparr.Infrastructure.Tests.TestHelpers; +using Cleanuparr.Persistence.Models.Configuration; +using Cleanuparr.Persistence.Startup; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; +using Xunit; + +namespace Cleanuparr.Infrastructure.Tests.Persistence; + +/// +/// After migration, each client sends the Basic header its old userinfo encoded. +/// +public sealed class DownloadClientUserInfoMigrationHeaderProofTests +{ + private const string Username = "user"; + + private const string Password = "pass"; + + private static readonly string ExpectedHeader = + Convert.ToBase64String(Encoding.UTF8.GetBytes($"{Username}:{Password}")); + + [Fact] + public async Task RTorrent_AfterMigration_SendsTheSameBasicHeader() + { + DownloadClientConfig config = MigratedConfig(DownloadClientTypeName.rTorrent); + + FakeHttpMessageHandler handler = new(); + handler.SetupResponse((_, _) => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("""0""") + })); + RTorrentClient client = new(config, new HttpClient(handler)); + + await client.StopTorrentAsync("HASH123"); + + handler.CapturedRequests[0].Headers.Authorization!.Scheme.ShouldBe("Basic"); + handler.CapturedRequests[0].Headers.Authorization!.Parameter.ShouldBe(ExpectedHeader); + } + + [Fact] + public async Task UTorrent_AfterMigration_SendsTheSameBasicHeader() + { + DownloadClientConfig config = MigratedConfig(DownloadClientTypeName.uTorrent); + + FakeHttpMessageHandler handler = new(); + handler.SetupResponse((_, _) => Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{}") + })); + UTorrentHttpService service = new(new HttpClient(handler), config, NullLogger.Instance); + + await service.SendRawRequestAsync(new UTorrentRequest { Action = "list" }, "GUID=abc"); + + handler.CapturedRequests[0].Headers.Authorization!.Scheme.ShouldBe("Basic"); + handler.CapturedRequests[0].Headers.Authorization!.Parameter.ShouldBe(ExpectedHeader); + } + + [Theory] + [InlineData(DownloadClientTypeName.qBittorrent)] + [InlineData(DownloadClientTypeName.Deluge)] + [InlineData(DownloadClientTypeName.Sabnzbd)] + public async Task AppLoginClient_AfterMigration_PreemptiveHandlerSendsTheSameBasicHeader(DownloadClientTypeName typeName) + { + DownloadClientConfig config = MigratedConfig(typeName); + + NetworkCredential? credential = config.GetHandlerHttpAuthCredential(); + credential.ShouldNotBeNull(); + + FakeHttpMessageHandler inner = new(); + BasicAuthHttpMessageHandler handler = new(credential) { InnerHandler = inner }; + using HttpClient client = new(handler); + + await client.GetAsync("http://localhost/"); + + inner.CapturedRequests[0].Headers.Authorization!.Scheme.ShouldBe("Basic"); + inner.CapturedRequests[0].Headers.Authorization!.Parameter.ShouldBe(ExpectedHeader); + } + + /// + /// Builds the migrated config through the parser helpers, so no database is needed. + /// + private static DownloadClientConfig MigratedConfig(DownloadClientTypeName typeName) + { + Uri originalHost = new($"http://{Username}:{Password}@original-host/"); + UserInfoCredential credential = UserInfoMigration.ParseCredential(originalHost.UserInfo); + Uri strippedHost = UserInfoMigration.StripUserInfo(originalHost); + + DownloadClientConfig config = new() + { + Name = "client", + TypeName = typeName, + Type = typeName.ClientType(), + Host = strippedHost, + HttpAuthType = HttpAuthType.Basic, + }; + + if (typeName.UsesDedicatedHttpAuthFields()) + { + config.HttpAuthUsername = credential.Username; + config.HttpAuthPassword = credential.Password; + } + else + { + config.Username = credential.Username; + config.Password = credential.Password; + } + + return config; + } +} diff --git a/code/backend/Cleanuparr.Infrastructure.Tests/Persistence/DownloadClientUserInfoMigratorTests.cs b/code/backend/Cleanuparr.Infrastructure.Tests/Persistence/DownloadClientUserInfoMigratorTests.cs new file mode 100644 index 00000000..a5d76383 --- /dev/null +++ b/code/backend/Cleanuparr.Infrastructure.Tests/Persistence/DownloadClientUserInfoMigratorTests.cs @@ -0,0 +1,366 @@ +using Cleanuparr.Domain.Enums; +using Cleanuparr.Infrastructure.Tests.TestHelpers; +using Cleanuparr.Persistence; +using Cleanuparr.Persistence.Models.Configuration; +using Cleanuparr.Persistence.Providers; +using Cleanuparr.Persistence.Startup; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging; +using NSubstitute; +using Shouldly; +using Testcontainers.PostgreSql; +using Xunit; + +namespace Cleanuparr.Infrastructure.Tests.Persistence; + +[Collection("SeedParity")] +public sealed class DownloadClientUserInfoMigratorTests +{ + [Fact] + public async Task Mixed_batch_migrates_on_sqlite() + { + using SqliteTestDatabase sqlite = SqliteTestDatabase.Create("userinfo-migration"); + + await RunAsync(sqlite.CreateContext); + } + + [SkippableFact] + public async Task Mixed_batch_migrates_on_postgres() + { + PostgreSqlContainer postgresContainer; + + try + { + postgresContainer = new PostgreSqlBuilder() + .WithImage("postgres:17") + .Build(); + + await postgresContainer.StartAsync(); + } + catch (Exception exception) + { + throw new SkipException($"Docker is unavailable, skipping userinfo migration test: {exception.Message}"); + } + + try + { + PostgresDatabaseProvider provider = new(); + string connectionString = postgresContainer.GetConnectionString(); + + await RunAsync(() => CreatePostgresContext(connectionString, provider)); + } + finally + { + await postgresContainer.DisposeAsync(); + } + } + + [Fact] + public async Task Bad_row_rolls_back_the_whole_transaction_and_logs_an_error() + { + using SqliteTestDatabase sqlite = SqliteTestDatabase.Create("userinfo-migration-bad-row"); + + await using (DataContext migrate = sqlite.CreateContext()) + { + await migrate.Database.MigrateAsync(); + } + + Guid goodId = Guid.NewGuid(); + Guid badId = Guid.NewGuid(); + + await using (DataContext seed = sqlite.CreateContext()) + { + seed.DownloadClients.Add(new DownloadClientConfig + { + Id = goodId, + Name = "good-client", + TypeName = DownloadClientTypeName.qBittorrent, + Type = DownloadClientType.Torrent, + Host = new Uri("http://user:pass@good-host/"), + }); + await seed.SaveChangesAsync(); + + // Uri rejects a raw '@' in the password, so raw SQL plants this row. + // Reading it throws, which must roll back the whole run. + await seed.Database.ExecuteSqlRawAsync( + """ + INSERT INTO download_clients (id, enabled, name, type_name, type, host, http_auth_type) + VALUES ({0}, 0, 'bad-client', 'qbittorrent', 'torrent', 'http://u:p@ss@bad-host/', 'basic') + """, + badId); + } + + ILogger logger = Substitute.For>(); + DownloadClientUserInfoMigrator migrator = new(logger); + + await using (DataContext run = sqlite.CreateContext()) + { + await Should.NotThrowAsync(() => migrator.MigrateAsync(run, CancellationToken.None)); + } + + logger.Received(1).Log( + LogLevel.Error, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + + // The failed SELECT never reached the good row. + await using DataContext assert = sqlite.CreateContext(); + List rawHosts = await assert.Database + .SqlQueryRaw("SELECT host AS \"Value\" FROM download_clients ORDER BY name") + .ToListAsync(); + rawHosts.ShouldBe(["http://u:p@ss@bad-host/", "http://user:pass@good-host/"]); + } + + [Fact] + public async Task Relative_host_row_is_skipped_and_the_rest_still_migrate() + { + using SqliteTestDatabase sqlite = SqliteTestDatabase.Create("userinfo-migration-relative-host"); + + await using (DataContext migrate = sqlite.CreateContext()) + { + await migrate.Database.MigrateAsync(); + } + + Guid relativeId = Guid.NewGuid(); + Guid credentialsId = Guid.NewGuid(); + + await using (DataContext seed = sqlite.CreateContext()) + { + seed.DownloadClients.Add(new DownloadClientConfig + { + Id = relativeId, + Name = "relative-client", + TypeName = DownloadClientTypeName.qBittorrent, + Type = DownloadClientType.Torrent, + Host = new Uri("192.168.1.5", UriKind.RelativeOrAbsolute), + }); + seed.DownloadClients.Add(new DownloadClientConfig + { + Id = credentialsId, + Name = "credentials-client", + TypeName = DownloadClientTypeName.qBittorrent, + Type = DownloadClientType.Torrent, + Host = new Uri("http://user:pass@good-host/"), + }); + await seed.SaveChangesAsync(); + } + + DownloadClientUserInfoMigrator migrator = new(Substitute.For>()); + + await using (DataContext run = sqlite.CreateContext()) + { + await migrator.MigrateAsync(run, CancellationToken.None); + } + + await using DataContext assert = sqlite.CreateContext(); + DownloadClientConfig relative = await assert.DownloadClients.SingleAsync(c => c.Id == relativeId); + DownloadClientConfig migrated = await assert.DownloadClients.SingleAsync(c => c.Id == credentialsId); + + relative.Host!.OriginalString.ShouldBe("192.168.1.5"); + migrated.Host!.UserInfo.ShouldBeEmpty(); + migrated.HttpAuthUsername.ShouldBe("user"); + migrated.HttpAuthPassword.ShouldBe("pass"); + } + + private static DataContext CreatePostgresContext(string connectionString, PostgresDatabaseProvider provider) + { + DbContextOptionsBuilder builder = new(); + builder + .UseNpgsql(connectionString, options => options.MigrationsAssembly(PostgresDatabaseProvider.MigrationsAssembly)) + .UseLowerCaseNamingConvention() + .UseSnakeCaseNamingConvention(); + + return new DataContext(builder.Options, provider); + } + + private static async Task RunAsync(Func contextFactory) + { + Guid qbitId = Guid.NewGuid(); + Guid delugeId = Guid.NewGuid(); + Guid sabnzbdId = Guid.NewGuid(); + Guid rTorrentId = Guid.NewGuid(); + Guid transmissionId = Guid.NewGuid(); + Guid uTorrentNoCredsId = Guid.NewGuid(); + Guid uTorrentExistingCredsId = Guid.NewGuid(); + Guid noUserInfoId = Guid.NewGuid(); + Guid encodedId = Guid.NewGuid(); + Guid colonPasswordId = Guid.NewGuid(); + Guid ipv6Id = Guid.NewGuid(); + Guid uppercaseSchemeId = Guid.NewGuid(); + Guid usernameOnlyId = Guid.NewGuid(); + Guid passwordOnlyId = Guid.NewGuid(); + Guid emptyUserInfoId = Guid.NewGuid(); + + await using (DataContext migrate = contextFactory()) + { + await migrate.Database.MigrateAsync(); + } + + await using (DataContext seed = contextFactory()) + { + seed.DownloadClients.AddRange( + NewClient(qbitId, "qbit", DownloadClientTypeName.qBittorrent, "http://quser:qpass@qbit-host/"), + NewClient(delugeId, "deluge", DownloadClientTypeName.Deluge, "http://duser:dpass@deluge-host/"), + NewClient(sabnzbdId, "sabnzbd", DownloadClientTypeName.Sabnzbd, "http://suser:spass@sab-host/"), + NewClient(rTorrentId, "rtorrent", DownloadClientTypeName.rTorrent, "http://ruser:rpass@rtorrent-host/"), + NewClient(transmissionId, "transmission", DownloadClientTypeName.Transmission, "http://tuser:tpass@transmission-host/"), + NewClient(uTorrentNoCredsId, "utorrent-no-creds", DownloadClientTypeName.uTorrent, "http://uuser:upass@utorrent-host/"), + NewClient(uTorrentExistingCredsId, "utorrent-existing-creds", DownloadClientTypeName.uTorrent, "http://xuser:xpass@utorrent-existing-host/", username: "existing", password: "existingpass"), + NewClient(noUserInfoId, "no-userinfo", DownloadClientTypeName.qBittorrent, "http://plain-host/unchanged", username: "keepme"), + NewClient(encodedId, "encoded", DownloadClientTypeName.qBittorrent, "http://u%3Ap:p%25q@encoded-host/a%20b?x=%2F"), + NewClient(colonPasswordId, "colon-password", DownloadClientTypeName.qBittorrent, "http://u:p:q@colon-host/"), + NewClient(ipv6Id, "ipv6", DownloadClientTypeName.qBittorrent, "http://user:pass@[::1]:8080/path/"), + NewClient(uppercaseSchemeId, "uppercase-scheme", DownloadClientTypeName.qBittorrent, "HTTP://user:pass@upper-host:80/"), + NewClient(usernameOnlyId, "username-only", DownloadClientTypeName.qBittorrent, "http://justuser@username-only-host/"), + NewClient(passwordOnlyId, "password-only", DownloadClientTypeName.qBittorrent, "http://:justpass@password-only-host/"), + NewClient(emptyUserInfoId, "empty-userinfo", DownloadClientTypeName.qBittorrent, "http://@empty-userinfo-host/")); + + await seed.SaveChangesAsync(); + } + + ILogger logger = Substitute.For>(); + DownloadClientUserInfoMigrator migrator = new(logger); + + await using (DataContext run = contextFactory()) + { + await migrator.MigrateAsync(run, CancellationToken.None); + } + + await using (DataContext assert = contextFactory()) + { + Dictionary clients = await assert.DownloadClients + .AsNoTracking() + .ToDictionaryAsync(c => c.Id); + + DownloadClientConfig qbit = clients[qbitId]; + qbit.HttpAuthUsername.ShouldBe("quser"); + qbit.HttpAuthPassword.ShouldBe("qpass"); + qbit.HttpAuthType.ShouldBe(HttpAuthType.Basic); + qbit.Username.ShouldBeNull(); + qbit.Password.ShouldBeNull(); + qbit.Host!.OriginalString.ShouldBe("http://qbit-host/"); + + DownloadClientConfig deluge = clients[delugeId]; + deluge.HttpAuthUsername.ShouldBe("duser"); + deluge.HttpAuthPassword.ShouldBe("dpass"); + deluge.Host!.OriginalString.ShouldBe("http://deluge-host/"); + + DownloadClientConfig sabnzbd = clients[sabnzbdId]; + sabnzbd.HttpAuthUsername.ShouldBe("suser"); + sabnzbd.HttpAuthPassword.ShouldBe("spass"); + sabnzbd.Host!.OriginalString.ShouldBe("http://sab-host/"); + + DownloadClientConfig rTorrent = clients[rTorrentId]; + rTorrent.Username.ShouldBe("ruser"); + rTorrent.Password.ShouldBe("rpass"); + rTorrent.HttpAuthUsername.ShouldBeNull(); + rTorrent.Host!.OriginalString.ShouldBe("http://rtorrent-host/"); + + DownloadClientConfig transmission = clients[transmissionId]; + transmission.Username.ShouldBe("tuser"); + transmission.Password.ShouldBe("tpass"); + transmission.Host!.OriginalString.ShouldBe("http://transmission-host/"); + + DownloadClientConfig uTorrentNoCreds = clients[uTorrentNoCredsId]; + uTorrentNoCreds.Username.ShouldBe("uuser"); + uTorrentNoCreds.Password.ShouldBe("upass"); + uTorrentNoCreds.Host!.OriginalString.ShouldBe("http://utorrent-host/"); + + // Existing creds win; only the userinfo goes. + DownloadClientConfig uTorrentExistingCreds = clients[uTorrentExistingCredsId]; + uTorrentExistingCreds.Username.ShouldBe("existing"); + uTorrentExistingCreds.Password.ShouldBe("existingpass"); + uTorrentExistingCreds.Host!.OriginalString.ShouldBe("http://utorrent-existing-host/"); + + DownloadClientConfig noUserInfo = clients[noUserInfoId]; + noUserInfo.Username.ShouldBe("keepme"); + noUserInfo.Host!.OriginalString.ShouldBe("http://plain-host/unchanged"); + + DownloadClientConfig encoded = clients[encodedId]; + encoded.HttpAuthUsername.ShouldBe("u:p"); + encoded.HttpAuthPassword.ShouldBe("p%q"); + encoded.Host!.OriginalString.ShouldBe("http://encoded-host/a%20b?x=%2F"); + + DownloadClientConfig colonPassword = clients[colonPasswordId]; + colonPassword.HttpAuthUsername.ShouldBe("u"); + colonPassword.HttpAuthPassword.ShouldBe("p:q"); + + DownloadClientConfig ipv6 = clients[ipv6Id]; + ipv6.HttpAuthUsername.ShouldBe("user"); + ipv6.HttpAuthPassword.ShouldBe("pass"); + ipv6.Host!.OriginalString.ShouldBe("http://[::1]:8080/path/"); + + DownloadClientConfig uppercaseScheme = clients[uppercaseSchemeId]; + uppercaseScheme.HttpAuthUsername.ShouldBe("user"); + uppercaseScheme.Host!.OriginalString.ShouldBe("HTTP://upper-host:80/"); + + DownloadClientConfig usernameOnly = clients[usernameOnlyId]; + usernameOnly.HttpAuthUsername.ShouldBe("justuser"); + usernameOnly.HttpAuthPassword.ShouldBeNull(); + usernameOnly.Host!.OriginalString.ShouldBe("http://username-only-host/"); + + DownloadClientConfig passwordOnly = clients[passwordOnlyId]; + passwordOnly.HttpAuthUsername.ShouldBeNull(); + passwordOnly.HttpAuthPassword.ShouldBe("justpass"); + passwordOnly.Host!.OriginalString.ShouldBe("http://password-only-host/"); + + // Empty userinfo carries no credential, so the row stays as is. + DownloadClientConfig emptyUserInfo = clients[emptyUserInfoId]; + emptyUserInfo.HttpAuthUsername.ShouldBeNull(); + emptyUserInfo.HttpAuthPassword.ShouldBeNull(); + emptyUserInfo.Host!.OriginalString.ShouldBe("http://@empty-userinfo-host/"); + } + + logger.Received(1).Log( + LogLevel.Warning, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + + await using (DataContext secondRun = contextFactory()) + { + await migrator.MigrateAsync(secondRun, CancellationToken.None); + } + + await using (DataContext assertAgain = contextFactory()) + { + DownloadClientConfig qbitAgain = await assertAgain.DownloadClients.AsNoTracking().SingleAsync(c => c.Id == qbitId); + qbitAgain.HttpAuthUsername.ShouldBe("quser"); + qbitAgain.HttpAuthPassword.ShouldBe("qpass"); + qbitAgain.Host!.OriginalString.ShouldBe("http://qbit-host/"); + + DownloadClientConfig uTorrentExistingCredsAgain = await assertAgain.DownloadClients + .AsNoTracking() + .SingleAsync(c => c.Id == uTorrentExistingCredsId); + uTorrentExistingCredsAgain.Username.ShouldBe("existing"); + uTorrentExistingCredsAgain.Password.ShouldBe("existingpass"); + } + + logger.Received(1).Log( + LogLevel.Warning, + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any>()); + } + + private static DownloadClientConfig NewClient( + Guid id, + string name, + DownloadClientTypeName typeName, + string host, + string? username = null, + string? password = null) => new() + { + Id = id, + Name = name, + TypeName = typeName, + Type = typeName.ClientType(), + Host = new Uri(host, UriKind.RelativeOrAbsolute), + Username = username, + Password = password, + }; +} diff --git a/code/backend/Cleanuparr.Persistence.Tests/Models/Configuration/DownloadClientConfigTests.cs b/code/backend/Cleanuparr.Persistence.Tests/Models/Configuration/DownloadClientConfigTests.cs index 64e1a880..a097b086 100644 --- a/code/backend/Cleanuparr.Persistence.Tests/Models/Configuration/DownloadClientConfigTests.cs +++ b/code/backend/Cleanuparr.Persistence.Tests/Models/Configuration/DownloadClientConfigTests.cs @@ -106,6 +106,35 @@ public sealed class DownloadClientConfigTests exception.Message.ShouldBe("Host cannot be empty"); } + [Fact] + public void Validate_WithUserInfoInHost_ThrowsValidationException() + { + var config = new DownloadClientConfig + { + Name = "My Client", + TypeName = DownloadClientTypeName.qBittorrent, + Type = DownloadClientType.Torrent, + Host = new Uri("http://user:pass@localhost:8080") + }; + + var exception = Should.Throw(() => config.Validate()); + exception.Message.ShouldBe("Credentials in the URL are not supported. Use the HTTP authentication fields instead."); + } + + [Fact] + public void Validate_WithRelativeHost_DoesNotThrow() + { + var config = new DownloadClientConfig + { + Name = "My Client", + TypeName = DownloadClientTypeName.qBittorrent, + Type = DownloadClientType.Torrent, + Host = new Uri("192.168.1.5", UriKind.RelativeOrAbsolute) + }; + + Should.NotThrow(() => config.Validate()); + } + #endregion #region Validate - Directory Mapping diff --git a/code/backend/Cleanuparr.Persistence.Tests/Startup/UserInfoMigrationTests.cs b/code/backend/Cleanuparr.Persistence.Tests/Startup/UserInfoMigrationTests.cs new file mode 100644 index 00000000..eec00a86 --- /dev/null +++ b/code/backend/Cleanuparr.Persistence.Tests/Startup/UserInfoMigrationTests.cs @@ -0,0 +1,106 @@ +using Cleanuparr.Persistence.Startup; +using Shouldly; +using Xunit; + +namespace Cleanuparr.Persistence.Tests.Startup; + +public sealed class UserInfoMigrationTests +{ + #region ParseCredential + + [Fact] + public void ParseCredential_UsernameAndPassword_SplitsOnFirstColon() + { + UserInfoCredential credential = UserInfoMigration.ParseCredential("user:pass"); + + credential.Username.ShouldBe("user"); + credential.Password.ShouldBe("pass"); + } + + [Fact] + public void ParseCredential_NoColon_IsUsernameOnly() + { + UserInfoCredential credential = UserInfoMigration.ParseCredential("user"); + + credential.Username.ShouldBe("user"); + credential.Password.ShouldBeNull(); + } + + [Fact] + public void ParseCredential_EmptyUsername_DecodesToNull() + { + UserInfoCredential credential = UserInfoMigration.ParseCredential(":pass"); + + credential.Username.ShouldBeNull(); + credential.Password.ShouldBe("pass"); + } + + [Fact] + public void ParseCredential_PasswordWithColon_KeepsItInThePassword() + { + UserInfoCredential credential = UserInfoMigration.ParseCredential("u:p:q"); + + credential.Username.ShouldBe("u"); + credential.Password.ShouldBe("p:q"); + } + + [Fact] + public void ParseCredential_EncodedSpecialCharacters_AreDecoded() + { + UserInfoCredential credential = UserInfoMigration.ParseCredential("u%3Ap:p%25q"); + + credential.Username.ShouldBe("u:p"); + credential.Password.ShouldBe("p%q"); + } + + [Fact] + public void ParseCredential_EncodedSlashSpaceAndAt_AreDecoded() + { + UserInfoCredential credential = UserInfoMigration.ParseCredential("us%2Fer:p%40ss%20word"); + + credential.Username.ShouldBe("us/er"); + credential.Password.ShouldBe("p@ss word"); + } + + [Fact] + public void ParseCredential_NonAsciiEncoded_IsDecoded() + { + UserInfoCredential credential = UserInfoMigration.ParseCredential("u:caf%C3%A9"); + + credential.Password.ShouldBe("café"); + } + + #endregion + + #region StripUserInfo + + [Theory] + [InlineData("http://user:pass@host:8080/path/?q=1", "http://host:8080/path/?q=1")] + [InlineData("http://user:pass@host/path/", "http://host/path/")] + [InlineData("http://user:pass@host", "http://host")] + [InlineData("https://user:pass@HOST:443/", "https://HOST:443/")] + [InlineData("HTTP://user:pass@host:80/", "HTTP://host:80/")] + [InlineData("http://u:p@host:1234/a/b/", "http://host:1234/a/b/")] + [InlineData("http://u%3Ap:p%25q@host/a%20b?x=%2F", "http://host/a%20b?x=%2F")] + [InlineData("http://user:pass@[::1]:8080/path", "http://[::1]:8080/path")] + public void StripUserInfo_RemovesOnlyTheUserinfo(string input, string expected) + { + Uri host = new(input, UriKind.RelativeOrAbsolute); + + Uri stripped = UserInfoMigration.StripUserInfo(host); + + stripped.OriginalString.ShouldBe(expected); + } + + [Fact] + public void StripUserInfo_NoAtSign_ReturnsHostUnchanged() + { + Uri host = new("http://host/path"); + + Uri stripped = UserInfoMigration.StripUserInfo(host); + + stripped.ShouldBe(host); + } + + #endregion +} diff --git a/code/backend/Cleanuparr.Persistence/Models/Configuration/DownloadClientConfig.cs b/code/backend/Cleanuparr.Persistence/Models/Configuration/DownloadClientConfig.cs index e43b0267..ca986908 100644 --- a/code/backend/Cleanuparr.Persistence/Models/Configuration/DownloadClientConfig.cs +++ b/code/backend/Cleanuparr.Persistence/Models/Configuration/DownloadClientConfig.cs @@ -128,6 +128,12 @@ public sealed record DownloadClientConfig throw new CustomValidationException($"Host cannot be empty"); } + if (Host.IsAbsoluteUri && Host.UserInfo.Length > 0) + { + throw new ValidationException( + "Credentials in the URL are not supported. Use the HTTP authentication fields instead."); + } + if (!string.IsNullOrWhiteSpace(DownloadDirectorySource) != !string.IsNullOrWhiteSpace(DownloadDirectoryTarget)) { throw new CustomValidationException("Both download directory source and target must be set, or both must be empty"); diff --git a/code/backend/Cleanuparr.Persistence/Startup/DownloadClientUserInfoMigrator.cs b/code/backend/Cleanuparr.Persistence/Startup/DownloadClientUserInfoMigrator.cs new file mode 100644 index 00000000..bb97da4e --- /dev/null +++ b/code/backend/Cleanuparr.Persistence/Startup/DownloadClientUserInfoMigrator.cs @@ -0,0 +1,79 @@ +using Cleanuparr.Domain.Enums; +using Cleanuparr.Persistence.Models.Configuration; +using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Storage; +using Microsoft.Extensions.Logging; + +namespace Cleanuparr.Persistence.Startup; + +/// +/// Moves URL userinfo (user:pass@host) out of download client hosts at startup. +/// Safe to rerun: a fixed row has no userinfo left. +/// +public sealed class DownloadClientUserInfoMigrator +{ + private readonly ILogger _logger; + + public DownloadClientUserInfoMigrator(ILogger logger) + { + _logger = logger; + } + + /// + /// Migrates all rows in one transaction. + /// On failure it rolls back and logs, so startup continues. + /// + public async Task MigrateAsync(DataContext context, CancellationToken cancellationToken) + { + await using IDbContextTransaction transaction = await context.Database.BeginTransactionAsync(cancellationToken); + + try + { + List clients = await context.DownloadClients.ToListAsync(cancellationToken); + + foreach (DownloadClientConfig client in clients) + { + if (client.Host is null || !client.Host.IsAbsoluteUri || client.Host.UserInfo.Length == 0) + { + continue; + } + + MigrateClient(client); + } + + await context.SaveChangesAsync(cancellationToken); + await transaction.CommitAsync(cancellationToken); + } + catch (Exception exception) + { + await transaction.RollbackAsync(cancellationToken); + _logger.LogError(exception, "Failed to migrate download client URL credentials; no changes were applied"); + } + } + + private void MigrateClient(DownloadClientConfig client) + { + UserInfoCredential credential = UserInfoMigration.ParseCredential(client.Host!.UserInfo); + + if (client.TypeName.UsesDedicatedHttpAuthFields()) + { + client.HttpAuthUsername = credential.Username; + client.HttpAuthPassword = credential.Password; + } + else if (string.IsNullOrEmpty(client.Username) && string.IsNullOrEmpty(client.Password)) + { + client.Username = credential.Username; + client.Password = credential.Password; + } + else + { + _logger.LogWarning( + "Download client {Name} ({Id}) already has a username or password; kept them and dropped the URL credentials", + client.Name, + client.Id); + } + + client.HttpAuthType = HttpAuthType.Basic; + client.Host = UserInfoMigration.StripUserInfo(client.Host); + } +} diff --git a/code/backend/Cleanuparr.Persistence/Startup/UserInfoCredential.cs b/code/backend/Cleanuparr.Persistence/Startup/UserInfoCredential.cs new file mode 100644 index 00000000..5e96b519 --- /dev/null +++ b/code/backend/Cleanuparr.Persistence/Startup/UserInfoCredential.cs @@ -0,0 +1,6 @@ +namespace Cleanuparr.Persistence.Startup; + +/// +/// A username/password pair decoded out of a URL's userinfo component. +/// +public readonly record struct UserInfoCredential(string? Username, string? Password); diff --git a/code/backend/Cleanuparr.Persistence/Startup/UserInfoMigration.cs b/code/backend/Cleanuparr.Persistence/Startup/UserInfoMigration.cs new file mode 100644 index 00000000..84eeb4fb --- /dev/null +++ b/code/backend/Cleanuparr.Persistence/Startup/UserInfoMigration.cs @@ -0,0 +1,53 @@ +namespace Cleanuparr.Persistence.Startup; + +/// +/// Parses and strips URL userinfo (user:pass@host). +/// +public static class UserInfoMigration +{ + /// + /// Splits raw userinfo on the first colon and percent-decodes both sides. + /// No colon means no password. + /// An empty username becomes null. + /// + public static UserInfoCredential ParseCredential(string userInfo) + { + int separatorIndex = userInfo.IndexOf(':'); + + string usernamePart = separatorIndex < 0 ? userInfo : userInfo[..separatorIndex]; + string? passwordPart = separatorIndex < 0 ? null : userInfo[(separatorIndex + 1)..]; + + string? username = usernamePart.Length == 0 ? null : Uri.UnescapeDataString(usernamePart); + string? password = passwordPart is null ? null : Uri.UnescapeDataString(passwordPart); + + return new UserInfoCredential(username, password); + } + + /// + /// Removes the userinfo and its @, leaving the rest of the string as is. + /// would normalize the scheme and add a default port and trailing slash. + /// + public static Uri StripUserInfo(Uri host) + { + string original = host.OriginalString; + int authorityStart = original.IndexOf("://", StringComparison.Ordinal) + 3; + + int authorityEnd = authorityStart; + while (authorityEnd < original.Length + && original[authorityEnd] != '/' + && original[authorityEnd] != '?' + && original[authorityEnd] != '#') + { + authorityEnd++; + } + + int atIndex = original.IndexOf('@', authorityStart, authorityEnd - authorityStart); + if (atIndex < 0) + { + return host; + } + + string rebuilt = original.Remove(authorityStart, atIndex - authorityStart + 1); + return new Uri(rebuilt, UriKind.RelativeOrAbsolute); + } +}