mike-jumper commented on a change in pull request #584:
URL: https://github.com/apache/guacamole-client/pull/584#discussion_r637502751
##########
File path:
extensions/guacamole-auth-totp/src/main/java/org/apache/guacamole/auth/totp/user/UserVerificationService.java
##########
@@ -166,7 +166,7 @@ private boolean setKey(UserContext context, UserTOTPKey key)
// Get mutable set of attributes
User self = context.self();
- Map<String, String> attributes = new HashMap<String, String>();
+ Map<String, String> attributes = new HashMap<>(self.getAttributes());
Review comment:
I think the issue boils down to the current implementation of
`setUnrestrictedAttributes()` and `setRestrictedAttributes()`. For example,
consider `setUnrestrictedAttributes()`:
https://github.com/apache/guacamole-client/blob/754e9649f1fa0ba225ee42b56ded64bc283d17df/extensions/guacamole-auth-jdbc/modules/guacamole-auth-jdbc-base/src/main/java/org/apache/guacamole/auth/jdbc/user/ModeledUser.java#L474-L488
When this is called from within `setAttributes()`, it will clear out any of
the modeled attributes that are absent from the `attributes` map, which is
incorrect behavior.
In both `setUnrestrictedAttributes()` and `setRestrictedAttributes()`, I
think code like:
```java
// Translate full name attribute
getModel().setFullName(TextField.parse(attributes.get(User.Attribute.FULL_NAME)));
```
needs to be updated to:
```java
// Translate full name attribute
if (attributes.containsKey(User.Attribute.FULL_NAME))
getModel().setFullName(TextField.parse(attributes.get(User.Attribute.FULL_NAME)));
```
--
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]
