You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@guacamole.apache.org by GitBox <gi...@apache.org> on 2020/02/06 09:05:18 UTC
[GitHub] [guacamole-client] benrubson opened a new pull request #471:
GUACAMOLE-753: Add TOTP auth method to Docker image
benrubson opened a new pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471
Hi,
This PR adds TOTP authentication method to the Docker image.
It's really helpful, making TOTP easy to use with Docker.
Let's merge this low-impact PR with #469 in a 1.1.1 Docker image ?
Many thanks 👍
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r383098401
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -707,6 +722,11 @@ END
exit 1;
fi
+# Use TOTP if specified.
+if [ -n "$TOTP_ENABLED" ]; then
Review comment:
My preference would be:
* Continue using `TOTP_ENABLED`.
* Actually check the value rather than simply whether it's set. The current implementation would enable TOTP if the user sets `TOTP_ENABLED` to "1", to "false", to "salmon", etc., and that's not exactly rigorous.
I'm not against additionally accepting `TOTP_*` in lieu of `TOTP_ENABLED` for convenience, but if so:
* Only actual, known variables should be accepted (for similar reasons to the above... setting `TOTP_FISH` to "salmon" shouldn't result in TOTP being enabled).
* `TOTP_ENABLED` should still probably be accepted (and checked), as users shouldn't be required to set a TOTP-related variable to its default value just to enable use of TOTP.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on issue #471:
GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on issue #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#issuecomment-603244827
Many thanks @jolentes for your positive feedback 👍
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r377386500
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -707,6 +722,11 @@ END
exit 1;
fi
+# Use TOTP if specified.
+if [ -n "$TOTP_ENABLED" ]; then
Review comment:
I'm not a huge fan of having to add an environment variable that doesn't map to a property, as it deviates from established patterns, but since the TOTP auth doesn't have any required properties I'm not sure we have any choice.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on issue #471:
GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on issue #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#issuecomment-603314906
Perhaps I should have rather pushed to the `staging/1.2.0` branch ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r377475089
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -707,6 +722,11 @@ END
exit 1;
fi
+# Use TOTP if specified.
+if [ -n "$TOTP_ENABLED" ]; then
Review comment:
The only "generic" solution I see is :
`if env | grep -q "^TOTP_"; then`
So that enforcing at least one of the `TOTP` properties would enable it.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman merged pull request #471:
GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
necouchman merged pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r377469712
##########
File path: guacamole-docker/bin/build-guacamole.sh
##########
@@ -135,6 +136,15 @@ if [ -f extensions/guacamole-auth-openid/target/guacamole-auth-openid*.jar ]; th
cp extensions/guacamole-auth-openid/target/guacamole-auth-openid*.jar "$DESTINATION/openid"
fi
+#
+# Copy TOTP auth extension if it was built
Review comment:
I took exactly what is done above and below for other extensions :)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r377397840
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -707,6 +722,11 @@ END
exit 1;
fi
+# Use TOTP if specified.
+if [ -n "$TOTP_ENABLED" ]; then
Review comment:
FWIW, I'm not tied to this - if we want to change it to something else or some other way of enabling it, I'm open to the options. It just seemed a little weird to me to trigger it by adding a parameter that isn't required. But, I'm willing to go with whatever.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on issue #471:
GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
necouchman commented on issue #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#issuecomment-603396569
Sorry, it isn't slated for 1.2.0, it's slated for 1.3.0, so it was merged correctly.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r375940141
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -707,6 +722,11 @@ END
exit 1;
fi
+# Use TOTP if specified.
+if [ -n "$TOTP_ISSUER" ]; then
Review comment:
Is this the best way to do this? The `totp-issuer` has a default value, per the manual, so should setting the issuer be what triggers activation of this module? Or should we go with something like `TOTP_ENABLED`?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r377387025
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -536,6 +536,21 @@ END
}
+##
+## Adds properties to guacamole.properties which configure the TOTP two-factor
+## authentication service.
Review comment:
Unlike Duo, TOTP isn't an authentication service. There may be a more correct way to document this.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r383117648
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -707,6 +722,11 @@ END
exit 1;
fi
+# Use TOTP if specified.
+if [ -n "$TOTP_ENABLED" ]; then
Review comment:
Thank you for your feedbacks.
I then updated to :
`if [ "$TOTP_ENABLED" = "true" ]`
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r397108205
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -536,6 +536,21 @@ END
}
+##
+## Adds properties to guacamole.properties which configure the TOTP two-factor
+## authentication mechanism.
+##
+associate_totp() {
+ # Update config file
+ set_property "totp-issuer" "$TOTP_ISSUER"
+ set_property "totp-digits" "$TOTP_DIGITS"
+ set_property "totp-period" "$TOTP_PERIOD"
+ set_property "totp-mode" "$TOTP_MODE"
Review comment:
I think we want `set_optional_property`, right, since all of these already have default values?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r377476292
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -536,6 +536,21 @@ END
}
+##
+## Adds properties to guacamole.properties which configure the TOTP two-factor
+## authentication service.
Review comment:
Done, thank you for your review 👍
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r375944302
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -707,6 +722,11 @@ END
exit 1;
fi
+# Use TOTP if specified.
+if [ -n "$TOTP_ISSUER" ]; then
Review comment:
Done, as it really seems proper 👍
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r375941444
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -707,6 +722,11 @@ END
exit 1;
fi
+# Use TOTP if specified.
+if [ -n "$TOTP_ISSUER" ]; then
Review comment:
I did not want to add another specific var to keep it as simple as possible, but I can for sure :+1:
Your new var / varname sounds good to me.
Should I ?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] mike-jumper commented on a change in pull
request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
mike-jumper commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r377385326
##########
File path: guacamole-docker/bin/build-guacamole.sh
##########
@@ -135,6 +136,15 @@ if [ -f extensions/guacamole-auth-openid/target/guacamole-auth-openid*.jar ]; th
cp extensions/guacamole-auth-openid/target/guacamole-auth-openid*.jar "$DESTINATION/openid"
fi
+#
+# Copy TOTP auth extension if it was built
Review comment:
Why would it not have been built? From what I recall, the only extension that's conditional within the build is RADIUS support.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on issue #471:
GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on issue #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#issuecomment-603246613
Many thanks @necouchman 👍
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r397108205
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -536,6 +536,21 @@ END
}
+##
+## Adds properties to guacamole.properties which configure the TOTP two-factor
+## authentication mechanism.
+##
+associate_totp() {
+ # Update config file
+ set_property "totp-issuer" "$TOTP_ISSUER"
+ set_property "totp-digits" "$TOTP_DIGITS"
+ set_property "totp-period" "$TOTP_PERIOD"
+ set_property "totp-mode" "$TOTP_MODE"
Review comment:
I think we want `set_option_property`, right, since all of these already have default values?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r387911213
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -536,6 +536,21 @@ END
}
+##
+## Adds properties to guacamole.properties which configure the TOTP two-factor
+## authentication mechanism.
+##
+associate_totp() {
+ # Update config file
+ [ -n "$TOTP_ISSUER" ] && set_property "totp-issuer" "$TOTP_ISSUER"
+ [ -n "$TOTP_DIGITS" ] && set_property "totp-digits" "$TOTP_DIGITS"
+ [ -n "$TOTP_PERIOD" ] && set_property "totp-period" "$TOTP_PERIOD"
+ [ -n "$TOTP_MODE" ] && set_property "totp-mode" "$TOTP_MODE"
Review comment:
Looking at the other sections (mostly above this), it looks like instead of doing
```
[ -n "$TOTP_ISSUER" ] && set_property "totp-issuer" "$TOTP_ISSUER"
```
you should be able to do:
```
set_optional_property "totp-issuer" "$TOTP_ISSUER"
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r392621985
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -536,6 +536,21 @@ END
}
+##
+## Adds properties to guacamole.properties which configure the TOTP two-factor
+## authentication mechanism.
+##
+associate_totp() {
+ # Update config file
+ [ -n "$TOTP_ISSUER" ] && set_property "totp-issuer" "$TOTP_ISSUER"
+ [ -n "$TOTP_DIGITS" ] && set_property "totp-digits" "$TOTP_DIGITS"
+ [ -n "$TOTP_PERIOD" ] && set_property "totp-period" "$TOTP_PERIOD"
+ [ -n "$TOTP_MODE" ] && set_property "totp-mode" "$TOTP_MODE"
Review comment:
This is the code for `set_optional_property()`:
```
set_optional_property() {
NAME="$1"
VALUE="$2"
# Set the property only if a value is provided
if [ -n "$VALUE" ]; then
set_property "$NAME" "$VALUE"
fi
}
```
So, it's essentially duplicating what you're doing, no?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r392670520
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -536,6 +536,21 @@ END
}
+##
+## Adds properties to guacamole.properties which configure the TOTP two-factor
+## authentication mechanism.
+##
+associate_totp() {
+ # Update config file
+ [ -n "$TOTP_ISSUER" ] && set_property "totp-issuer" "$TOTP_ISSUER"
+ [ -n "$TOTP_DIGITS" ] && set_property "totp-digits" "$TOTP_DIGITS"
+ [ -n "$TOTP_PERIOD" ] && set_property "totp-period" "$TOTP_PERIOD"
+ [ -n "$TOTP_MODE" ] && set_property "totp-mode" "$TOTP_MODE"
Review comment:
Tired certainly...
Thank you @necouchman for pointing this, just fixed 👍
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r387917186
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -536,6 +536,21 @@ END
}
+##
+## Adds properties to guacamole.properties which configure the TOTP two-factor
+## authentication mechanism.
+##
+associate_totp() {
+ # Update config file
+ [ -n "$TOTP_ISSUER" ] && set_property "totp-issuer" "$TOTP_ISSUER"
+ [ -n "$TOTP_DIGITS" ] && set_property "totp-digits" "$TOTP_DIGITS"
+ [ -n "$TOTP_PERIOD" ] && set_property "totp-period" "$TOTP_PERIOD"
+ [ -n "$TOTP_MODE" ] && set_property "totp-mode" "$TOTP_MODE"
Review comment:
I'm not sure because we would then set empty options in the configuration file.
In the other sections, options are tested to always have a value.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r397157210
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -536,6 +536,21 @@ END
}
+##
+## Adds properties to guacamole.properties which configure the TOTP two-factor
+## authentication mechanism.
+##
+associate_totp() {
+ # Update config file
+ set_property "totp-issuer" "$TOTP_ISSUER"
+ set_property "totp-digits" "$TOTP_DIGITS"
+ set_property "totp-period" "$TOTP_PERIOD"
+ set_property "totp-mode" "$TOTP_MODE"
Review comment:
For sure, I just then pushed the required fix.
Thank you @necouchman 👍
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] necouchman commented on a change in pull request
#471: GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
necouchman commented on a change in pull request #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#discussion_r382937150
##########
File path: guacamole-docker/bin/start.sh
##########
@@ -707,6 +722,11 @@ END
exit 1;
fi
+# Use TOTP if specified.
+if [ -n "$TOTP_ENABLED" ]; then
Review comment:
@mike-jumper Thoughts, here? I'm not tied to my suggestion, so if you have a vote...?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [guacamole-client] benrubson commented on issue #471:
GUACAMOLE-753: Add TOTP auth method to Docker image
Posted by GitBox <gi...@apache.org>.
benrubson commented on issue #471: GUACAMOLE-753: Add TOTP auth method to Docker image
URL: https://github.com/apache/guacamole-client/pull/471#issuecomment-594026214
Let's merge this @mike-jumper @necouchman ?
Many thanks 👍
----------------------------------------------------------------
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
With regards,
Apache Git Services