Support authenticated forward HTTP/HTTPS proxies across the client stack#7331
Open
pditommaso wants to merge 10 commits into
Open
Support authenticated forward HTTP/HTTPS proxies across the client stack#7331pditommaso wants to merge 10 commits into
pditommaso wants to merge 10 commits into
Conversation
Behind an authenticating corporate proxy, HxClient-based paths (plugin downloads, registry calls, Tower, Wave) failed with 407 because proxy routing and credentials were never propagated to them. Move ProxyConfig to nf-commons and add static register/configure helpers plus an HxProxyConfig view (proxyConfig()). Launcher.setProxy now registers the parsed proxy and NO_PROXY hosts so every HxClient construction site and WaveClient's token-refresh/main clients honour the same proxy and credentials. Bump lib-httpx to 2.4.0 for the proxy passthroughs. Fixes #7305 Replaces #7306 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
✅ Deploy Preview for nextflow-docs canceled.
|
…nfigure Supplying a client verbatim via HxClient.Builder.httpClient(...) makes withProxyConfig a no-op and skips proxy propagation to the internal token-refresh clients. Configure version/redirects/timeout/executor directly on HxClient.Builder instead and apply the proxy with withProxyConfig, so the main and token-refresh clients both route through the proxy. Removes the now-unused ProxyConfig.configure(HttpClient.Builder) helper and stops the local libseqera includeBuild in favour of the published lib-httpx 2.4.0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
pditommaso
force-pushed
the
update-lib-httpx-2.4.0
branch
from
766eab4 to
821cb35
Compare
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> # Conflicts: # plugins/nf-seqera/build.gradle
Convert its plain JDK HttpClient to HxClient (no bearer token) and apply the resolved ProxyConfig, so signed-URL fetches honour HTTP_PROXY/HTTPS_PROXY. Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
TowerXAuth keeps its plain JDK HttpClient (cookie-based refresh flow); apply the resolved proxy selector and, when present, the proxy authenticator so the token-refresh call honours HTTP_PROXY/HTTPS_PROXY. Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
- setProxy() falls back to ALL_PROXY/all_proxy when no scheme-specific proxy variable is set (scheme-specific still takes precedence). - ProxyConfig.parse() percent-decodes the userinfo username/password so credentials with special characters work; a literal '+' is preserved. Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
The JDK default (Basic) strips proxy credentials from the HTTPS CONNECT request, so HTTPS targets behind an authenticating proxy fail with 407 even when the authenticator is wired. Clear the property when the proxy carries credentials, without overriding a value the user set via NXF_OPTS. Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
pditommaso
changed the title
…ing caveat Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
3 tasks
LauncherTest exercises Launcher.setProxy, which populates the static ProxyConfig registry. @RestoreSystemProperties restores system properties but not that registry, so a leaked proxy caused later tests (AssetManagerTest cloning via RepositoryProvider, now proxy-aware) to route through a dead proxy and hang until the CI job timeout. Clear the registry in cleanup(). Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
Member
|
Does this PR also resolve #5634 ? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Behind an authenticating corporate proxy, Nextflow's HTTP paths (plugin downloads, registry calls, SCM, Tower, Wave, data-links) failed with an immediate
407: proxy routing and credentials fromHTTP_PROXY/HTTPS_PROXY/ALL_PROXY/NO_PROXYwere never propagated to thejava.net.http.HttpClientclients Nextflow builds.This PR threads the parsed proxy configuration into every HTTP client construction site, adds the missing environment-variable handling, and fixes the JDK's HTTPS-tunnelling credential quirk.
Changes
Proxy plumbing (
HxClientstack)ProxyConfigfromnextflowtonf-commonsso plugins can reach it.HxProxyConfigview (ProxyConfig.proxyConfig()) resolved from the launch environment.Launcher.setProxyregisters the parsed proxy plusNO_PROXYhosts, soHttpPluginRepository,RegistryClientFactory,RepositoryProvider,SraExplorer,TowerClient,AuthCommandImplandWaveClient(both the main and token-refresh clients) honour the same routing and credentials.HxClient.Builderdirectly (not via.httpClient(...)), sowithProxyConfig(...)isn't a silent no-op and the internal token-refresh clients inherit the proxy too. The now-unusedProxyConfig.configure(HttpClient.Builder)helper is removed.lib-httpxto2.4.0(carrieswithProxyConfig/HxProxyConfig).Remaining raw-
HttpClientsites in nf-towerDataLinksResourceHandler: converted from a plain JDKHttpClienttoHxClient(no bearer token, so the pre-signed-URL fetch still does not carry the SeqeraAuthorizationheader) withwithProxyConfig.TowerXAuth: kept on a plain JDKHttpClient— its token-refresh flow owns aCookieManagerand reads theJWT/JWT_REFRESH_TOKENcookies back out of it, andHxClient.Builderhas nocookieHandler()hook. The proxy selector + proxy authenticator are wired explicitly instead (rationale documented inline).Environment-variable handling (
Launcher/ProxyConfig)ALL_PROXY/all_proxyis used as a fallback when no scheme-specific proxy variable is set (scheme-specific still takes precedence).ProxyConfig.parsepercent-decodes the userinfouser:passso credentials with special characters (@,:, …) work; a literal+is preserved (userinfo is not form-encoded).HTTPS CONNECT tunnelling (the core #7305 fix)
jdk.http.auth.tunneling.disabledSchemes=Basicstrips proxy credentials from the HTTPSCONNECTrequest, so HTTPS targets behind an authenticating proxy fail with407even when the authenticator is wired.Launchernow clears the property when the proxy carries credentials, without ever overriding a value the user set (e.g. viaNXF_OPTS).Testing
Each change lands as a separate commit with unit tests (RED→GREEN): proxy resolution +
withProxyConfigwiring, a WireMock integration test proving a request routes through an authenticating proxy,ALL_PROXYfallback, credential URL-decoding,disabledSchemesclearing (including the no-credentials and user-set guardrails), and per-site tests forDataLinksResourceHandlerandTowerXAuth.Not included (follow-ups)
ALL_PROXYand theNXF_OPTS='-Djdk.http.auth.tunneling.disabledSchemes='fallback inenv-vars.mdx.CONNECT-tunnelling integration test — the current WireMock test deliberately covers only plain-HTTP, so thedisabledSchemesfix is unit-covered but not exercised end-to-end.Related
🤖 Generated with Claude Code