From e0a9364dde890bff48481ad9e95affa38a77abb4 Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Wed, 17 Aug 2022 18:05:41 +0000 Subject: [PATCH] GUACAMOLE-1661: Simplify and clarify KSM domain search code. --- .../guacamole/vault/ksm/secret/KsmClient.java | 16 +++--- .../vault/ksm/secret/KsmSecretService.java | 4 +- .../{UserDomain.java => UserLogin.java} | 50 +++++++------------ 3 files changed, 30 insertions(+), 40 deletions(-) rename extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/{UserDomain.java => UserLogin.java} (63%) diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java index 34f350234..a573259b5 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmClient.java @@ -177,7 +177,7 @@ public class KsmClient { * this Map, {@link #cachedAmbiguousUsers} must first be checked to * verify that there is indeed only one record associated with that user. */ - private final Map cachedRecordsByUser = new HashMap<>(); + private final Map cachedRecordsByUser = new HashMap<>(); /** * The set of all username/domain combos that are associated with multiple @@ -187,7 +187,7 @@ public class KsmClient { * acquired appropriately. This Set must be checked before using a value * retrieved from {@link #cachedRecordsByUser}. */ - private final Set cachedAmbiguousUsers = new HashSet<>(); + private final Set cachedAmbiguousUsers = new HashSet<>(); /** * All records retrieved from Keeper Secrets Manager, where each key is the @@ -307,10 +307,12 @@ public class KsmClient { WindowsUsername usernameAndDomain = ( WindowsUsername.splitWindowsUsernameFromDomain(username)); - // Use the username-split domain if not already set explicitly - if (usernameAndDomain.hasDomain()) + // Use the username-split domain if available + if (usernameAndDomain.hasDomain()) { domain = usernameAndDomain.getDomain(); + username = usernameAndDomain.getUsername(); addRecordForDomain(record, domain); + } } @@ -407,7 +409,7 @@ public class KsmClient { if (username == null) return; - UserDomain userDomain = new UserDomain(username, domain); + UserLogin userDomain = new UserLogin(username, domain); KeeperRecord existing = cachedRecordsByUser.putIfAbsent( userDomain, record); if (existing != null && record != existing) @@ -504,7 +506,7 @@ public class KsmClient { * The username of the record to return. * * @param domain - * The domain of the record to return. + * The domain of the record to return, or null if no domain exists. * * @return * The record associated with the given username and domain, or null @@ -519,7 +521,7 @@ public class KsmClient { validateCache(); cacheLock.readLock().lock(); - UserDomain userDomain = new UserDomain(username, domain); + UserLogin userDomain = new UserLogin(username, domain); try { diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java index 3b8a6dc0d..cb5868cfa 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/KsmSecretService.java @@ -373,7 +373,9 @@ public class KsmSecretService implements VaultSecretService { filter.filter(gatewayUsername), filteredGatewayDomain)); - } else { + } + + else { // Retrieve and define user-specific tokens, if any // NOTE that non-RDP connections do not have a domain diff --git a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/UserDomain.java b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/UserLogin.java similarity index 63% rename from extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/UserDomain.java rename to extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/UserLogin.java index bb2c2309d..5cd50ee6c 100644 --- a/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/UserDomain.java +++ b/extensions/guacamole-vault/modules/guacamole-vault-ksm/src/main/java/org/apache/guacamole/vault/ksm/secret/UserLogin.java @@ -19,6 +19,8 @@ package org.apache.guacamole.vault.ksm.secret; +import java.util.Objects; + import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -29,7 +31,7 @@ import javax.annotation.Nullable; * actually be identified by both the user and domain, if the appropriate * settings are enabled. */ -class UserDomain { +class UserLogin { /** * The username associated with the user record. @@ -44,17 +46,17 @@ class UserDomain { private final String domain; /** - * Create a new UserDomain instance with the provided username and + * Create a new UserLogin instance with the provided username and * domain. The domain may be null, but the username should never be. * * @param username - * The username to create the UserDomain instance with. This should + * The username to create the UserLogin instance with. This should * never be null. * * @param domain - * The domain to create the UserDomain instance with. This can be null. + * The domain to create the UserLogin instance with. This can be null. */ - UserDomain(@Nonnull String username, @Nullable String domain) { + UserLogin(@Nonnull String username, @Nullable String domain) { this.username = username; this.domain = domain; } @@ -62,13 +64,7 @@ class UserDomain { @Override public int hashCode() { - final int prime = 31; - - int result = 1; - result = prime * result + ((domain == null) ? 0 : domain.hashCode()); - result = prime * result + ((username == null) ? 0 : username.hashCode()); - - return result; + return Objects.hash(domain, username); } @@ -83,33 +79,23 @@ class UserDomain { if (obj == null) return false; - // Check if the other object is also a UserDomain + // Check if the other object is also a UserLogin if (getClass() != obj.getClass()) return false; - // If it is a UserDomain, it must have the same username... - UserDomain other = (UserDomain) obj; - if (username == null) { - if (other.username != null) - return false; - } else if (!username.equals(other.username)) - return false; + // If the other object is also a UserLogin, it must + // have the same username and domain + UserLogin other = (UserLogin) obj; + return Objects.equals(username, other.username) + && Objects.equals(domain, other.domain); - // .. and the same domain - if (domain == null) { - if (other.domain != null) - return false; - } else if (!domain.equals(other.domain)) - return false; - - return true; } /** - * Get the username associated with this UserDomain. + * Get the username associated with this UserLogin. * * @return - * The username associated with this UserDomain. + * The username associated with this UserLogin. */ public String getUsername() { return username; @@ -117,10 +103,10 @@ class UserDomain { /** - * Get the domain associated with this UserDomain. + * Get the domain associated with this UserLogin. * * @return - * The domain associated with this UserDomain. + * The domain associated with this UserLogin. */ public String getDomain() { return domain;