From fae22a3f807ece98e9087848756d7a7784462e5c Mon Sep 17 00:00:00 2001 From: Virtually Nick Date: Thu, 26 Dec 2019 18:05:59 -0500 Subject: [PATCH 1/4] GUACAMOLE-770: Add ability to clear out TOTP data. --- .../guacamole/auth/totp/user/TOTPUser.java | 29 ++++++------ .../auth/totp/user/TOTPUserContext.java | 11 +++++ .../src/main/resources/config/totpConfig.js | 7 +++ .../controllers/totpResetFieldController.js | 46 +++++++++++++++++++ .../src/main/resources/translations/en.json | 9 ++++ 5 files changed, 89 insertions(+), 13 deletions(-) create mode 100644 extensions/guacamole-auth-totp/src/main/resources/controllers/totpResetFieldController.js diff --git a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java index 5a9e4b80f..9fdfae158 100644 --- a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java +++ b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java @@ -19,8 +19,11 @@ package org.apache.guacamole.auth.totp.user; +import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import org.apache.guacamole.form.BooleanField; +import org.apache.guacamole.form.Form; import org.apache.guacamole.net.auth.DelegatingUser; import org.apache.guacamole.net.auth.User; @@ -41,6 +44,17 @@ public class TOTPUser extends DelegatingUser { */ public static final String TOTP_KEY_CONFIRMED_ATTRIBUTE_NAME = "guac-totp-key-confirmed"; + /** + * The form which contains all configurable properties for this user. + */ + public static final Form TOTP_CONFIG_FORM = new Form("totp-config-form", + Arrays.asList( + new BooleanField(TOTP_KEY_SECRET_ATTRIBUTE_NAME, ""), + new BooleanField(TOTP_KEY_CONFIRMED_ATTRIBUTE_NAME, "true") + ) + ); + + /** * Wraps the given User object, hiding and blocking access to the core * attributes used by TOTP. @@ -66,14 +80,8 @@ public class TOTPUser extends DelegatingUser { public Map getAttributes() { // Create independent, mutable copy of attributes - Map attributes = - new HashMap(super.getAttributes()); + Map attributes = new HashMap<>(super.getAttributes()); - // Do not expose any TOTP-related attributes outside this extension - attributes.remove(TOTP_KEY_SECRET_ATTRIBUTE_NAME); - attributes.remove(TOTP_KEY_CONFIRMED_ATTRIBUTE_NAME); - - // Expose only non-TOTP attributes return attributes; } @@ -82,13 +90,8 @@ public class TOTPUser extends DelegatingUser { public void setAttributes(Map attributes) { // Create independent, mutable copy of attributes - attributes = new HashMap(attributes); + attributes = new HashMap<>(attributes); - // Do not expose any TOTP-related attributes outside this extension - attributes.remove(TOTP_KEY_SECRET_ATTRIBUTE_NAME); - attributes.remove(TOTP_KEY_CONFIRMED_ATTRIBUTE_NAME); - - // Set only non-TOTP attributes super.setAttributes(attributes); } diff --git a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUserContext.java b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUserContext.java index 980bbf782..f4785193c 100644 --- a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUserContext.java +++ b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUserContext.java @@ -19,7 +19,11 @@ package org.apache.guacamole.auth.totp.user; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; import org.apache.guacamole.GuacamoleException; +import org.apache.guacamole.form.Form; import org.apache.guacamole.net.auth.DecoratingDirectory; import org.apache.guacamole.net.auth.DelegatingUserContext; import org.apache.guacamole.net.auth.Directory; @@ -60,5 +64,12 @@ public class TOTPUserContext extends DelegatingUserContext { }; } + + @Override + public Collection
getUserAttributes() { + Collection userAttrs = new HashSet<>(super.getUserAttributes()); + userAttrs.add(TOTPUser.TOTP_CONFIG_FORM); + return Collections.unmodifiableCollection(userAttrs); + } } diff --git a/extensions/guacamole-auth-totp/src/main/resources/config/totpConfig.js b/extensions/guacamole-auth-totp/src/main/resources/config/totpConfig.js index 54bb56c08..0de357314 100644 --- a/extensions/guacamole-auth-totp/src/main/resources/config/totpConfig.js +++ b/extensions/guacamole-auth-totp/src/main/resources/config/totpConfig.js @@ -29,5 +29,12 @@ angular.module('guacTOTP').config(['formServiceProvider', controller : 'authenticationCodeFieldController', templateUrl : 'app/ext/totp/templates/authenticationCodeField.html' }); + + // Add field type for resetting TOTP data + formServiceProvider.registerFieldType('GUAC_TOTP_RESET', { + module : 'guacTOTP', + controller : 'totpResetFieldController', + templateUrl : 'app/form/templates/checkboxField.html' + }); }]); diff --git a/extensions/guacamole-auth-totp/src/main/resources/controllers/totpResetFieldController.js b/extensions/guacamole-auth-totp/src/main/resources/controllers/totpResetFieldController.js new file mode 100644 index 000000000..041772305 --- /dev/null +++ b/extensions/guacamole-auth-totp/src/main/resources/controllers/totpResetFieldController.js @@ -0,0 +1,46 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Controller for the "GUAC_TOTP_CODE" field which prompts the user to enter + * the code generated by their authentication device. + */ +angular.module('guacTOTP').controller('totpResetFieldController', ['$scope', '$window', + function totpResetFieldController($scope, $window) { + + var origValue = $scope.model; + + // Update typed value when model is changed + $scope.$watch('model', function modelChanged(model) { + if (!model || model === '') + $scope.typedValue = true; + else + $scope.typedValue = false; + }); + + // Update string value in model when typed value is changed + $scope.$watch('typedValue', function typedValueChanged(typedValue) { + if (typedValue && typedValue !== '') + $scope.model = ''; + + else + $scope.model = origValue; + }); + +}]); \ No newline at end of file diff --git a/extensions/guacamole-auth-totp/src/main/resources/translations/en.json b/extensions/guacamole-auth-totp/src/main/resources/translations/en.json index 6f73aa02d..be1e3b54b 100644 --- a/extensions/guacamole-auth-totp/src/main/resources/translations/en.json +++ b/extensions/guacamole-auth-totp/src/main/resources/translations/en.json @@ -29,6 +29,15 @@ "SECTION_HEADER_DETAILS" : "Details:" + }, + + "USER_ATTRIBUTES" : { + + "FIELD_HEADER_GUAC_TOTP_KEY_SECRET" : "Clear TOTP key:", + "FIELD_HEADER_GUAC_TOTP_KEY_CONFIRMED" : "TOTP key confirmed:", + + "SECTION_HEADER_TOTP_CONFIG_FORM" : "Configure TOTP" + } } From 43e5024676346a75600250e5d879e3eb38073166 Mon Sep 17 00:00:00 2001 From: Virtually Nick Date: Fri, 3 Apr 2020 13:16:53 -0400 Subject: [PATCH 2/4] GUACAMOLE-770: Regenerate code when field is empty. --- .../guacamole/auth/totp/user/UserVerificationService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java index 2414ee872..59e5a7126 100644 --- a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java +++ b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java @@ -108,7 +108,7 @@ public class UserVerificationService { // If no key is defined, attempt to generate a new key String secret = attributes.get(TOTPUser.TOTP_KEY_SECRET_ATTRIBUTE_NAME); - if (secret == null) { + if (secret == null || secret.isEmpty()) { // Generate random key for user TOTPGenerator.Mode mode = confService.getMode(); From 4f4a060d4af0115b73cadf3a912110755a618097 Mon Sep 17 00:00:00 2001 From: Virtually Nick Date: Fri, 3 Apr 2020 14:47:40 -0400 Subject: [PATCH 3/4] GUACAMOLE-770: Switch to Boolean field for clearing data. --- .../guacamole/auth/totp/user/TOTPUser.java | 25 +++++++++- .../src/main/resources/config/totpConfig.js | 7 --- .../controllers/totpResetFieldController.js | 46 ------------------- .../src/main/resources/translations/en.json | 2 +- 4 files changed, 25 insertions(+), 55 deletions(-) delete mode 100644 extensions/guacamole-auth-totp/src/main/resources/controllers/totpResetFieldController.js diff --git a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java index 9fdfae158..e21064c06 100644 --- a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java +++ b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java @@ -43,13 +43,18 @@ public class TOTPUser extends DelegatingUser { * confirmed by the user, and the user is thus fully enrolled. */ public static final String TOTP_KEY_CONFIRMED_ATTRIBUTE_NAME = "guac-totp-key-confirmed"; + + /** + * The name of the field used to trigger a reset of the TOTP data. + */ + public static final String TOTP_KEY_SECRET_RESET_FIELD = "guac-totp-reset"; /** * The form which contains all configurable properties for this user. */ public static final Form TOTP_CONFIG_FORM = new Form("totp-config-form", Arrays.asList( - new BooleanField(TOTP_KEY_SECRET_ATTRIBUTE_NAME, ""), + new BooleanField(TOTP_KEY_SECRET_RESET_FIELD, "true"), new BooleanField(TOTP_KEY_CONFIRMED_ATTRIBUTE_NAME, "true") ) ); @@ -81,6 +86,17 @@ public class TOTPUser extends DelegatingUser { // Create independent, mutable copy of attributes Map attributes = new HashMap<>(super.getAttributes()); + + // Protect the secret value by removing it + String secret = attributes.remove(TOTP_KEY_SECRET_ATTRIBUTE_NAME); + + // If secret is null or empty, mark the reset as true. + if (secret == null || secret.isEmpty()) + attributes.put(TOTP_KEY_SECRET_RESET_FIELD, "true"); + + // If secret has a value, mark the reset as false. + else + attributes.put(TOTP_KEY_SECRET_RESET_FIELD, "false"); return attributes; @@ -91,6 +107,13 @@ public class TOTPUser extends DelegatingUser { // Create independent, mutable copy of attributes attributes = new HashMap<>(attributes); + + // 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); super.setAttributes(attributes); diff --git a/extensions/guacamole-auth-totp/src/main/resources/config/totpConfig.js b/extensions/guacamole-auth-totp/src/main/resources/config/totpConfig.js index 0de357314..54bb56c08 100644 --- a/extensions/guacamole-auth-totp/src/main/resources/config/totpConfig.js +++ b/extensions/guacamole-auth-totp/src/main/resources/config/totpConfig.js @@ -29,12 +29,5 @@ angular.module('guacTOTP').config(['formServiceProvider', controller : 'authenticationCodeFieldController', templateUrl : 'app/ext/totp/templates/authenticationCodeField.html' }); - - // Add field type for resetting TOTP data - formServiceProvider.registerFieldType('GUAC_TOTP_RESET', { - module : 'guacTOTP', - controller : 'totpResetFieldController', - templateUrl : 'app/form/templates/checkboxField.html' - }); }]); diff --git a/extensions/guacamole-auth-totp/src/main/resources/controllers/totpResetFieldController.js b/extensions/guacamole-auth-totp/src/main/resources/controllers/totpResetFieldController.js deleted file mode 100644 index 041772305..000000000 --- a/extensions/guacamole-auth-totp/src/main/resources/controllers/totpResetFieldController.js +++ /dev/null @@ -1,46 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -/** - * Controller for the "GUAC_TOTP_CODE" field which prompts the user to enter - * the code generated by their authentication device. - */ -angular.module('guacTOTP').controller('totpResetFieldController', ['$scope', '$window', - function totpResetFieldController($scope, $window) { - - var origValue = $scope.model; - - // Update typed value when model is changed - $scope.$watch('model', function modelChanged(model) { - if (!model || model === '') - $scope.typedValue = true; - else - $scope.typedValue = false; - }); - - // Update string value in model when typed value is changed - $scope.$watch('typedValue', function typedValueChanged(typedValue) { - if (typedValue && typedValue !== '') - $scope.model = ''; - - else - $scope.model = origValue; - }); - -}]); \ No newline at end of file diff --git a/extensions/guacamole-auth-totp/src/main/resources/translations/en.json b/extensions/guacamole-auth-totp/src/main/resources/translations/en.json index be1e3b54b..55bd69ac4 100644 --- a/extensions/guacamole-auth-totp/src/main/resources/translations/en.json +++ b/extensions/guacamole-auth-totp/src/main/resources/translations/en.json @@ -33,7 +33,7 @@ "USER_ATTRIBUTES" : { - "FIELD_HEADER_GUAC_TOTP_KEY_SECRET" : "Clear TOTP key:", + "FIELD_HEADER_GUAC_TOTP_RESET" : "Clear TOTP secret:", "FIELD_HEADER_GUAC_TOTP_KEY_CONFIRMED" : "TOTP key confirmed:", "SECTION_HEADER_TOTP_CONFIG_FORM" : "Configure TOTP" From c7cb40d8f136a3f169b53bbed9656a317e8c36ce Mon Sep 17 00:00:00 2001 From: Virtually Nick Date: Fri, 3 Apr 2020 23:17:43 -0400 Subject: [PATCH 4/4] GUACAMOLE-770: Avoid letting attribute be manually set and reset confirmation along with secret. --- .../java/org/apache/guacamole/auth/totp/user/TOTPUser.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java index e21064c06..d50e3a68c 100644 --- a/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java +++ b/extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/TOTPUser.java @@ -108,12 +108,17 @@ public class TOTPUser extends DelegatingUser { // Create independent, mutable copy of attributes attributes = new HashMap<>(attributes); + // Do not expose any TOTP secret attribute outside this extension + attributes.remove(TOTP_KEY_SECRET_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")) + if (reset != null && reset.equals("true")) { attributes.put(TOTP_KEY_SECRET_ATTRIBUTE_NAME, null); + attributes.put(TOTP_KEY_CONFIRMED_ATTRIBUTE_NAME, null); + } super.setAttributes(attributes);