[GitHub] [guacamole-manual] necouchman commented on a change in pull request #130: GUACAMOLE-1005: Docker, configure RemoteIPValve

[GitBox <gi...@apache.org>]

necouchman commented on a change in pull request #130:
URL: https://github.com/apache/guacamole-manual/pull/130#discussion_r650584408



##########
File path: src/guacamole-docker.md
##########
@@ -646,6 +646,41 @@ valid Docker variables for enabling and configuring header authentication:
   header that will be used used to authenticate the user to Guacamole. If this
   is not specified the default value of REMOTE_USER will be used.
 
+(guacamole-docker-tomcat-remote-ip-valve)=
+
+### Execution behind a proxy
+
+To run Guacamole behind a reverse proxy, the Tomcat's [`RemoteIpValve`](https://tomcat.apache.org/tomcat-8.5-doc/config/valve.html#Remote_IP_Valve) as in [](tomcat-remote-ip).

Review comment:
       I'm not entirely sure what you're trying to say, here, so I think this could use some clarification. You don't _need_ to set the `RemoteIpValve` in order for Guacamole to work correctly behind a reverse proxy. You need to set it if Guacamole is running behind a reverse proxy and you wish to have the client IP address accurately recorded by Guacamole (subject to several caveats mentioned in the proxy section).

##########
File path: src/guacamole-docker.md
##########
@@ -646,6 +646,41 @@ valid Docker variables for enabling and configuring header authentication:
   header that will be used used to authenticate the user to Guacamole. If this
   is not specified the default value of REMOTE_USER will be used.
 
+(guacamole-docker-tomcat-remote-ip-valve)=
+
+### Execution behind a proxy
+
+To run Guacamole behind a reverse proxy, the Tomcat's [`RemoteIpValve`](https://tomcat.apache.org/tomcat-8.5-doc/config/valve.html#Remote_IP_Valve) as in [](tomcat-remote-ip).
+
+(guacamole-docker-tomcat-remote-ip-valve-required-vars)=
+
+#### Required environment variables
+
+The following environment variables have to be set in order to configure the [`RemoteIpValve`](https://tomcat.apache.org/tomcat-8.5-doc/config/valve.html#Remote_IP_Valve):
+
+`PROXY_ALLOWED_IPS_REGEX`
+: The regex indicating the hosts allowed to set the remote IP via headers.
+  Specify `.*` to allow any address.
+  This maps with Tomcat's `internalProxies` directive.

Review comment:
       I feel like this section needs to be tuned one way or the other. In particular:
   * Just telling people to "specify `.*` to allow any address", without any warning or additional information about why they'd want to do that (or why they may not want to do that) seems to me like there's an implied recommendation to go that route. If we're going to provide examples, that's fine, but let's provide more than just, "Here's how to open it up to allow anything."
   * Saying that "This maps with Tomcat's `internalProxies` directive" has very little meaning, here. I would recommend that we either just point people to the Proxy section of the manual that contains the detail and leave out details, here, or that we build out the detail here a little more to match the Proxy section so that this makes complete sense for someone configuring Guacamole in Docker.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org