[GitHub] mike-jumper commented on a change in pull request #355: GUACAMOLE-271: Duo in docker image

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

mike-jumper commented on a change in pull request #355: GUACAMOLE-271: Duo in docker image
URL: https://github.com/apache/guacamole-client/pull/355#discussion_r247746775
 
 

 ##########
 File path: guacamole-docker/bin/start.sh
 ##########
 @@ -591,6 +591,18 @@ END
     exit 1;
 fi
 
+# Use Duo if specified
+if [ -n "$DUO_API_HOSTNAME" ] &&        \
+    [ -n "$DUO_INTEGRATION_KEY" ] &&    \
+    [ -n "$DUO_SECRET_KEY" ] &&         \
+    [ -n "$DUO_APPLICATION_KEY" ] ; then
+    set_optional_property "duo-api-hostname"        "$DUO_API_HOSTNAME"
+    set_optional_property "duo-integration-key"     "$DUO_INTEGRATION_KEY"
+    set_optional_property "duo-secret-key"          "$DUO_SECRET_KEY"
 
 Review comment:
   These properties are actually all required, not optional:
   
   http://guacamole.apache.org/doc/gug/duo-auth.html#guac-duo-config
   
   If any are absent, the startup script should abort and warn the user, as with the other extensions supported by the image. For example:
   
   https://github.com/apache/guacamole-client/blob/9dcee8bdac2d40ae98ebabb53bdba2e9237ca93c/guacamole-docker/bin/start.sh#L128-L146
   
   This should also be set up in a similar manner to the others. We currently have extension-specific setup, parsing, etc. is organized within separate functions, keeping the logic which follows later simple and unobstructed:
   
   https://github.com/apache/guacamole-client/blob/9dcee8bdac2d40ae98ebabb53bdba2e9237ca93c/guacamole-docker/bin/start.sh#L551-L555
   
   You're correct to not update the `INSTALLED_AUTH` environment variable, as things will not work unless some other auth mechanism is set up, but things will be easier to read and more maintainable if the existing pattern is followed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


With regards,
Apache Git Services