necouchman commented on a change in pull request #504:
URL: https://github.com/apache/guacamole-client/pull/504#discussion_r632120223



##########
File path: 
extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/conf/ConfigurationService.java
##########
@@ -144,6 +156,22 @@ public int getPeriod() throws GuacamoleException {
         return environment.getProperty(TOTP_PERIOD, 30);
     }
 
+    /**
+     * Returns the maximun allowed offset between gucamole  and  TOTP token, in
+     * periodes. If not specified, 0 will be used by default.

Review comment:
       Is periodes the same as seconds?

##########
File path: 
extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java
##########
@@ -110,14 +116,16 @@ public void setAttributes(Map<String, String> attributes) 
{
         
         // Do not expose any TOTP secret attribute outside this extension
         attributes.remove(TOTP_KEY_SECRET_ATTRIBUTE_NAME);
-        
+        attributes.remove(TOTP_TIMEDRIFT_OFFSET_ATTRIBUTE_NAME);
+
         // Pull off the boolean reset field
         String reset = attributes.remove(TOTP_KEY_SECRET_RESET_FIELD);
         
         // If reset has been set to true, clear the secret.
         if (reset != null && reset.equals("true")) {
             attributes.put(TOTP_KEY_SECRET_ATTRIBUTE_NAME, null);
             attributes.put(TOTP_KEY_CONFIRMED_ATTRIBUTE_NAME, null);
+            attributes.put(TOTP_TIMEDRIFT_OFFSET_ATTRIBUTE_NAME, "0");

Review comment:
       I think in a reset case, this should be set to `null` and not `0`, as we 
want the rows to actually come out of the DB (which is what setting to `null` 
does).




-- 
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:
[email protected]


Reply via email to