fix(nzbget): preserve Basic auth across same-host non-downgrade redirects

[kevinheneveld]

Summary

Adding NZBGet as a download client could fail with NZBGet XML-RPC error: Unauthorized even when the same credentials worked from a manual curl. The trigger is a reverse proxy in front of NZBGet (Caddy/Nginx/Traefik HTTPS upgrade, trailing-slash normalization, etc.) returning a 30x — .NET's default HttpClientHandler follows the redirect but strips Authorization to prevent credential leakage to unintended hosts, so NZBGet sees the follow-up without auth and 401s.

Approach

Revised after @T4g1's review — the original commit on this branch (URL-embedded user:pass@host) was abandoned because it would also forward credentials across cross-host or HTTPS→HTTP redirects, defeating the very security control it was working around.

The new approach treats this as a redirect-handling concern rather than an auth-encoding concern:

• The named "nzbget" HttpClient is now configured with AllowAutoRedirect = false and a custom NzbgetSafeRedirectHandler in its delegating-handler chain.
• On 301/302/303/307/308, the handler validates that the next URL is (a) the same host:port as the previous request and (b) not an HTTPS→HTTP downgrade. Only then does it re-apply the Authorization header on the next request.
• 307/308 preserve method and (buffered) body per RFC 7231; 301/302/303 change to GET, matching HttpClientHandler's defaults.
• Hop cap of 5 to defend against redirect loops.
• On any rule violation (cross-host, scheme downgrade, missing Location, redirect loop), the handler throws a typed NzbgetSafeRedirectException with a descriptive message. TestConnectionAsync surfaces that message verbatim so users see the actual misconfiguration instead of a generic "Authentication failed" / "network error".

The handler is wired into both Program.cs and the equivalent ServiceRegistrationExtensions.AddListenarrHttpClients registration, so every code path that uses the named "nzbget" HttpClient is covered — including FetchDownloadsAsync which calls /jsonrpc (the JSON-RPC gap T4g1 flagged in the same review). AddViaJsonRpcAsync delegates through CallXmlRpcAsync and is covered transitively.

The Authorization header is now the sole auth path. NZBGet's own WebServer.cpp accepts Authorization: Basic natively (see daemon/remote/WebServer.cpp ParseHeaders()), so URL-embedded credentials were never required.

Why not let HttpClientHandler handle redirects with its built-in safety?

Considered, but HttpClientHandler.AllowAutoRedirect = true is exactly what causes the original bug: it strips Authorization on every hop, unconditionally, so any 30x kills auth. The framework has no per-host re-apply hook. A DelegatingHandler with AllowAutoRedirect = false is the established pattern in this codebase (used in TransmissionAdapter, TorrentFileDownloader, ImageCacheService, MyAnonamouseHelper, all with similar comments about preserving auth/cookies across redirects).

Test plan

• dotnet test --filter "FullyQualifiedName~NzbgetAdapterTests" passes locally (13 tests including new redirect-flow coverage)
• Full suite green: dotnet test → 641 passed, 0 failed
• Manual: configure NZBGet as a download client in Listenarr against an instance behind a reverse proxy that issues a same-host HTTPS upgrade; "Test" returns connected
• Manual: point at an attacker.example URL and confirm the cross-host error message surfaces correctly

[T4g1]

I see in the adapter that it mention using JSON-RPC too but I can't find reference to call made using that instead of XML-RPC, at first glance I would suggest to make sure JSON-RPC is not used or that this change does not applies to it too

Also, i believe the auto redirect that strips the authorization does that to prevent credential leakage through malevolent redirects, this change breaks that security right ? Wouldn't it be better to define a custom redirection handler that validates the domain is the same and it's not downgrading (HTTPS -> HTTP) then add back the relevant headers instead ?

[kevinheneveld]

Thanks @T4g1 — both points were on target and I've pivoted the PR accordingly. Quick summary of what changed:

On the JSON-RPC code path: you were right that it exists and was missed. NzbgetAdapter.FetchDownloadsAsync calls /jsonrpc directly via BuildUri(client, "/jsonrpc"), and AddViaJsonRpcAsync delegates through CallXmlRpcAsync so it's covered transitively. The pivoted approach below is registered at the HttpClient level (the named "nzbget" client), so both endpoints get the same treatment without per-call wiring.

On the security concern about URL-embedded creds: also right — user:pass@host survives redirects to any host (cross-domain, scheme-downgraded, anything), which defeats the protection the framework's redirect-strip is implementing rather than implementing it properly. I checked NZBGet's own WebServer.cpp (ParseHeaders()) and it accepts the Authorization: Basic header natively, so URL-embedded creds were never required — the original commit's justification was wrong. I also checked whether /xmlrpc actually 30x's on its own: it doesn't (only /nzbget redirects to /nzbget/), so the realistic 3xx case is a reverse proxy in front of NZBGet doing HTTPS upgrade or path normalization.

Implementation: new NzbgetSafeRedirectHandler : DelegatingHandler wired into the named "nzbget" HttpClient with AllowAutoRedirect = false. On 30x it validates same-host + no-HTTPS-downgrade before re-applying the Authorization header. On rule violation (cross-host, scheme downgrade, missing Location, or redirect loop) it throws a typed NzbgetSafeRedirectException with a descriptive message that TestConnectionAsync surfaces to the user, so misconfigurations are obvious instead of showing up as mystery Unauthorized errors.

PR body and tests rewritten — [Theory] over 301/302/303/307/308 for the re-apply path, plus negative tests for cross-host, downgrade, and loop. Force-pushed to fix/nzbget-xmlrpc-auth. Take another look when you have a moment.