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/04/30 06:47:06 UTC

[GitHub] [guacamole-client] sporeno opened a new pull request #504: GUACAMOLE-1056: Add support for tracking the time drift between guacamole and TOTP-tokens

sporeno opened a new pull request #504:
URL: https://github.com/apache/guacamole-client/pull/504


   We added a new attribute for storing the value of the time drift next to the TOTP-Key. The module will accept 3 codes, one previous, one on time and one in the future. Depending on the accepted code the value will be decremented (previous) or incremented (future) or unchanged (on time). The new value will be used to compensate the timedrift on the next login.


----------------------------------------------------------------
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



[GitHub] [guacamole-client] necouchman commented on a change in pull request #504: GUACAMOLE-1056: Add support for tracking the time drift between guacamole and TOTP-tokens

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



[GitHub] [guacamole-client] necouchman commented on pull request #504: GUACAMOLE-1056: Add support for tracking the time drift between guacamole and TOTP-tokens

Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #504:
URL: https://github.com/apache/guacamole-client/pull/504#issuecomment-730347058


   You have an extra commit (8239c16), now, that needs to be removed.


----------------------------------------------------------------
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



[GitHub] [guacamole-client] sporeno edited a comment on pull request #504: GUACAMOLE-1056: Add support for tracking the time drift between guacamole and TOTP-tokens

Posted by GitBox <gi...@apache.org>.
sporeno edited a comment on pull request #504:
URL: https://github.com/apache/guacamole-client/pull/504#issuecomment-732164066


   I seeams that I did it correct now.
   @necouchman Thank you for your help.


----------------------------------------------------------------
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



[GitHub] [guacamole-client] amelchio commented on pull request #504: GUACAMOLE-1056: Add support for tracking the time drift between guacamole and TOTP-tokens

Posted by GitBox <gi...@apache.org>.
amelchio commented on pull request #504:
URL: https://github.com/apache/guacamole-client/pull/504#issuecomment-904964104


   I do not have time drift problems but I would nevertheless love to see this integrated to avoid issues around the time when a code expires and a new one must be used.


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

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



[GitHub] [guacamole-client] necouchman commented on pull request #504: GUACAMOLE-1056: Add support for tracking the time drift between guacamole and TOTP-tokens

Posted by GitBox <gi...@apache.org>.
necouchman commented on pull request #504:
URL: https://github.com/apache/guacamole-client/pull/504#issuecomment-731360150


   > What have I to do to resolve it?
   
   Make sure that your forked copy of the `master` branch of the repo is up-to-date, and then rebase your `TOTPTimeDrift` branch on top of `master` via the command `git rebase master`. You'll likely run into the conflicts there and you'll need to resolve them.


----------------------------------------------------------------
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



[GitHub] [guacamole-client] sporeno edited a comment on pull request #504: GUACAMOLE-1056: Add support for tracking the time drift between guacamole and TOTP-tokens

Posted by GitBox <gi...@apache.org>.
sporeno edited a comment on pull request #504:
URL: https://github.com/apache/guacamole-client/pull/504#issuecomment-730358168


   > You have an extra commit ([8239c16](https://github.com/apache/guacamole-client/commit/8239c16ddda778bd8ba4c140ae07cc7ab2d5cf9e)), now, that needs to be removed.
   
   Commit removed, I was created by resolving the conflict. Now it's present again. 
   What have I to do to resolve 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



[GitHub] [guacamole-client] sporeno commented on pull request #504: GUACAMOLE-1056: Add support for tracking the time drift between guacamole and TOTP-tokens

Posted by GitBox <gi...@apache.org>.
sporeno commented on pull request #504:
URL: https://github.com/apache/guacamole-client/pull/504#issuecomment-732164066


   I seeam that I did it correct now.
   @necouchman Thank you for your help.


----------------------------------------------------------------
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



[GitHub] [guacamole-client] amelchio commented on a change in pull request #504: GUACAMOLE-1056: Add support for tracking the time drift between guacamole and TOTP-tokens

Posted by GitBox <gi...@apache.org>.
amelchio commented on a change in pull request #504:
URL: https://github.com/apache/guacamole-client/pull/504#discussion_r695186417



##########
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:
       A period is the time interval where a particular code is valid, usually 30 seconds.
   
   So an offset of 1 will accept the current code, the previous code and the following code.




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@guacamole.apache.org

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



[GitHub] [guacamole-client] sporeno commented on pull request #504: GUACAMOLE-1056: Add support for tracking the time drift between guacamole and TOTP-tokens

Posted by GitBox <gi...@apache.org>.
sporeno commented on pull request #504:
URL: https://github.com/apache/guacamole-client/pull/504#issuecomment-730358168


   > You have an extra commit ([8239c16](https://github.com/apache/guacamole-client/commit/8239c16ddda778bd8ba4c140ae07cc7ab2d5cf9e)), now, that needs to be removed.
   
   Commit removed, I was created by resolving the conflict. Now it's present again. 
   What have I to do to resolf 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