[
https://issues.apache.org/jira/browse/KNOX-970?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16185894#comment-16185894
]
Sandeep More commented on KNOX-970:
-----------------------------------
Great, thanks for the patch Jeff, this is some great work !
Given the scope of the patch I would also like [~lmccay] to review it.
When I try to apply the patch it fails with the following error
{code}
knox git:(master) git apply
/Users/smore/dev/review-patches/KNOX-970-PR-9-full.patch
error:
gateway-service-nifi/src/main/java/org/apache/hadoop/gateway/dispatch/NiFIRequestModifier.java:
No such file or directory
/Users/smore/dev/review-patches/KNOX-970-PR-9-full.patch:1471: new blank line
at EOF.
+
error:
gateway-service-nifi/src/main/java/org/apache/hadoop/gateway/dispatch/NiFiRequestUtil.java:
No such file or directory
{code}
Looks like the patch did not pickup addition of new files or initial commits.
Following are my comments based on the the patch.
1. We should add UnitTests for this feature.
2. I am not sure whether we need to keep the commented out configuration
section in sandbox.xml, it definately needs to go in Knox Docs but I think we
should move it from here to keep sandbox.xml simple.
3. Just a suggestion, in class ServiceDefinitionDeploymentContributor.java ,
the variable 'twoWaySslAlias' can be changed to 'useTwoWaySsl' given it is a
value and not an alias.
4. In NiFiRequestUtil class at this line
'effectivePrincipalName.equalsIgnoreCase("anonymous")' you assign it as blank,
why ? this could affect some parts for e.g. logging anonymous users in audit.log
5. In NiFiRequestUtil class, ssoCookieName is hard coded, I think users have
the ability to change this, this could be an issue, may be [~lmccay] can keep
me honest here.
Overall looks terrific !
> Add support for proxying NiFi
> -----------------------------
>
> Key: KNOX-970
> URL: https://issues.apache.org/jira/browse/KNOX-970
> Project: Apache Knox
> Issue Type: New Feature
> Components: Server
> Reporter: Jeff Storck
> Fix For: 0.14.0
>
> Attachments: KNOX-970-PR-9-full.patch
>
>
> Apache NiFi hosts several known UIs/APIs at various context paths (/nifi,
> /nifi-api, /nifi-docs, etc) and several dynamically discovered UIs/APIs
> depending on individual installations/configurations of NiFi through multiple
> component versions and custom NARs.
> Knox needs to be able to proxy to all of the available context paths in NiFi
> without being configured for each one individually.
> The X-Forwarded-Context header set by Knox when proxying needs to include the
> context path at which Knox is hosted (for example, /gateway/sandbox) and the
> path at which the NiFi services are proxied (for example, nifi-web). Using
> this header with the extra context path information (from the given examples,
> /gateway/sandbox/nifi-web), Knox needs to be able to rewrite URLs of incoming
> requests to the root context of the web server hosted by NiFi.
> When proxying to a secured NiFi instance/cluster set up with multi-tenancy,
> Knox also needs to set an additional header required by NiFi,
> X-ProxiedEntitiesChain, which will contain the identity of the user making
> the request to Knox. If the header is present in an incoming request to
> Knox, it must be able to take the DN from the SSL cert of the requesting
> client (two-way SSL) and add it to the value received in the header. The
> requests made from Knox to NiFi must also be made with two-way SSL so that
> NiFi can obtain the Knox server DN from its certificate. The values present
> in the X-ProxiedEntitiesChain will be used to authorize each identity
> specified in the header of the proxied request before the operation will be
> performed by NiFi.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
