[
https://issues.apache.org/jira/browse/NIFI-5748?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16675362#comment-16675362
]
ASF GitHub Bot commented on NIFI-5748:
--------------------------------------
GitHub user jtstorck opened a pull request:
https://github.com/apache/nifi/pull/3129
[WIP] NIFI-5748 Fixed proxy header support to use X-Forwarded-Host instead …
…of X-ForwardedServer
Added support for the context path header used by Traefik when proxying a
service (X-Forwarded-Prefix)
Added tests to ApplicationResourceTest for X-Forwarded-Context and
X-Forwarded-Prefix
Updated administration doc to include X-Forwarded-Prefix
Thank you for submitting a contribution to Apache NiFi.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced
in the commit message?
- [ ] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number
you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target
branch (typically master)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] Have you ensured that the full suite of tests is executed via mvn
-Pcontrib-check clean install at the root nifi folder?
- [ ] Have you written or updated unit tests to verify your changes?
- [ ] If adding new dependencies to the code, are these dependencies
licensed in a way that is compatible for inclusion under [ASF
2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file, including the main
LICENSE file under nifi-assembly?
- [ ] If applicable, have you updated the NOTICE file, including the main
NOTICE file found under nifi-assembly?
- [ ] If adding new Properties, have you added .displayName in addition to
.name (programmatic access) for each of the new properties?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in
which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build
issues and submit an update to your PR as soon as possible.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jtstorck/nifi NIFI-5748
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/3129.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #3129
----
commit 49aecd1127132e2c4cb12639cb9d66b14dee60d0
Author: Jeff Storck <jtswork@...>
Date: 2018-10-29T17:29:28Z
NIFI-5748 Fixed proxy header support to use X-Forwarded-Host instead of
X-ForwardedServer
Added support for the context path header used by Traefik when proxying a
service (X-Forwarded-Prefix)
Added tests to ApplicationResourceTest for X-Forwarded-Context and
X-Forwarded-Prefix
Updated administration doc to include X-Forwarded-Prefix
----
> Improve handling of X-Forwarded-* headers in URI Rewriting
> ----------------------------------------------------------
>
> Key: NIFI-5748
> URL: https://issues.apache.org/jira/browse/NIFI-5748
> Project: Apache NiFi
> Issue Type: Improvement
> Reporter: Kevin Doran
> Assignee: Jeff Storck
> Priority: Major
>
> This ticket is to improve the handling of headers used by popular proxies
> when rewriting URIs in NiFI. Currently, NiFi checks the following headers
> when determining how to re-write URLs it returns clients:
> From
> [ApplicationResource|https://github.com/apache/nifi/blob/2201f7746fd16874aefbd12d546565f5d105ab04/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ApplicationResource.java#L110]:
> {code:java}
> public static final String PROXY_SCHEME_HTTP_HEADER = "X-ProxyScheme";
> public static final String PROXY_HOST_HTTP_HEADER = "X-ProxyHost";
> public static final String PROXY_PORT_HTTP_HEADER = "X-ProxyPort";
> public static final String PROXY_CONTEXT_PATH_HTTP_HEADER =
> "X-ProxyContextPath";
> public static final String FORWARDED_PROTO_HTTP_HEADER = "X-Forwarded-Proto";
> public static final String FORWARDED_HOST_HTTP_HEADER = "X-Forwarded-Server";
> public static final String FORWARDED_PORT_HTTP_HEADER = "X-Forwarded-Port";
> public static final String FORWARDED_CONTEXT_HTTP_HEADER =
> "X-Forwarded-Context";
> // ...
> final String scheme = getFirstHeaderValue(PROXY_SCHEME_HTTP_HEADER,
> FORWARDED_PROTO_HTTP_HEADER);
> final String host = getFirstHeaderValue(PROXY_HOST_HTTP_HEADER,
> FORWARDED_HOST_HTTP_HEADER);
> final String port = getFirstHeaderValue(PROXY_PORT_HTTP_HEADER,
> FORWARDED_PORT_HTTP_HEADER);
> {code}
> Based on this, it looks like if both {{X-Forwarded-Server}} and
> {{X-Forwarded-Host}} are set, that {{-Host}} will contain the hostname the
> user/client requested, and {{-Server}} will contain the hostname of the proxy
> server (ie, what the proxy server is able to determine from inspecting the
> hostname of the instance it is on). See this for more details:
> https://stackoverflow.com/questions/43689625/x-forwarded-host-vs-x-forwarded-server
> Here is a demo based on docker containers and a reverse-proxy called Traefik
> that shows where the current NiFi logic can break:
> https://gist.github.com/kevdoran/2892004ccbfbb856115c8a756d9d4538
> To use this gist, you can run the following:
> {noformat}
> wget -qO-
> https://gist.githubusercontent.com/kevdoran/2892004ccbfbb856115c8a756d9d4538/raw/fb72151900d4d8fdcf4919fe5c8a94805fbb8401/docker-compose.yml
> | docker-compose -f - up
> {noformat}
> Once the environment is up. Go to http://nifi.docker.localhost/nifi in Chrome
> and try adding/configuring/moving a processor. This will reproduce the issue.
> For this task, the following improvement is recommended:
> Change the Header (string literal) for FORWARDED_HOST_HTTP_HEADER from
> "X-Forwarded-Server" to "X-Forwarded-Host"
> Additionally, some proxies use a different header for the context path
> prefix. Traefik uses {{X-Forwarded-Prefix}}. There does not appear to be a
> universal standard. In the future, we could make this header configurable,
> but for now, perhaps we should add {{X-Forwarded-Prefix}} to the headers
> checked by NiFi so that Traefik is supported out-of-the-box.
> *Important:* After making this change, verify that proxying NiFi via Knox
> still works, i.e., Knox should be sending the X-Forwarded-Host header that
> matches what the user requested in the browser.
> This change applies to NiFi Registry as well.
> /cc [~jtstorck]
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
