From f8c28cdbea2f617d5e7e0e583cfe707b3edbd330 Mon Sep 17 00:00:00 2001 From: blackrock-engineering Date: Thu, 26 Jul 2018 10:17:04 +0100 Subject: [PATCH 1/4] 1. Set a flexible timeout whilst waiting for getAllUsers() operation to complete. 2. Set 'active' flag on SymUser when calling getAllUsersWithDetails(). --- .../clients/impl/UsersClientImpl.java | 32 +++++++++++-------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java b/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java index dbfd6dbf..c0a1bc25 100644 --- a/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java +++ b/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java @@ -58,6 +58,7 @@ public class UsersClientImpl implements org.symphonyoss.symphony.clients.UsersClient { private final SymAuth symAuth; private final ApiClient apiClient; + private final long getAllUsersTimeout; private final Logger logger = LoggerFactory.getLogger(UsersClientImpl.class); @@ -68,10 +69,7 @@ public class UsersClientImpl implements org.symphonyoss.symphony.clients.UsersCl * @param config Symphony Client config */ public UsersClientImpl(SymAuth symAuth, SymphonyClientConfig config) { - this(symAuth, config, null); - - } /** @@ -93,7 +91,7 @@ public UsersClientImpl(SymAuth symAuth, SymphonyClientConfig config, Client http apiClient.setBasePath(config.get(SymphonyClientConfigID.POD_URL)); - + getAllUsersTimeout = Long.valueOf(System.getProperty("SYMPHONY_GET_ALL_USERS_TIMEOUT", "5")).longValue(); } @@ -294,11 +292,12 @@ public Set getAllUsers() throws UsersClientException { executor.shutdown(); - executor.awaitTermination(5, TimeUnit.SECONDS); - - - logger.debug("Finished all threads. Total time retrieving users: {} sec", (System.currentTimeMillis() - startTime) / 1000); - + final boolean processCompleted = executor.awaitTermination(getAllUsersTimeout, TimeUnit.SECONDS); + if (processCompleted) { + logger.debug("Finished all threads. Total time retrieving users: {} sec", (System.currentTimeMillis() - startTime) / 1000); + } else { + logger.warn("Process timed-out waiting to getAllUsers(). Total time retrieving users: {} sec", (System.currentTimeMillis() - startTime) / 1000); + } } catch (ApiException e) { throw new UsersClientException("API Error communicating with POD, while retrieving all user details", @@ -334,11 +333,16 @@ public Set getAllUsersWithDetails() throws UsersClientException { symUser.setFeatures(featureList); userDetail = userApi.v1AdminUserUidGet(sessionToken, uid); symUser.setRoles(new HashSet<>(userDetail.getRoles())); - if (userDetail.getUserSystemInfo().getLastLoginDate() != null) - symUser.setLastLoginDate(new Date(userDetail.getUserSystemInfo().getLastLoginDate())); - - if (userDetail.getUserSystemInfo().getCreatedDate() != null) - symUser.setCreatedDate(new Date(userDetail.getUserSystemInfo().getCreatedDate())); + + if (userDetail.getUserSystemInfo()) { + symUser.setActive(UserSystemInfo.StatusEnum.ENABLED == userDetail.getUserSystemInfo().getStatus()); + if (userDetail.getUserSystemInfo().getLastLoginDate() != null) { + symUser.setLastLoginDate(new Date(userDetail.getUserSystemInfo().getLastLoginDate())); + } + if (userDetail.getUserSystemInfo().getCreatedDate() != null) { + symUser.setCreatedDate(new Date(userDetail.getUserSystemInfo().getCreatedDate())); + } + } } } catch (ApiException e) { From 99edc108cbdd3094ab15f2796722187079fdca45 Mon Sep 17 00:00:00 2001 From: blackrock-engineering Date: Thu, 26 Jul 2018 10:26:51 +0100 Subject: [PATCH 2/4] Add null check. --- .../org/symphonyoss/symphony/clients/impl/UsersClientImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java b/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java index c0a1bc25..0d06156c 100644 --- a/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java +++ b/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java @@ -334,7 +334,7 @@ public Set getAllUsersWithDetails() throws UsersClientException { userDetail = userApi.v1AdminUserUidGet(sessionToken, uid); symUser.setRoles(new HashSet<>(userDetail.getRoles())); - if (userDetail.getUserSystemInfo()) { + if (userDetail.getUserSystemInfo() != null) { symUser.setActive(UserSystemInfo.StatusEnum.ENABLED == userDetail.getUserSystemInfo().getStatus()); if (userDetail.getUserSystemInfo().getLastLoginDate() != null) { symUser.setLastLoginDate(new Date(userDetail.getUserSystemInfo().getLastLoginDate())); From 8923accca3ad3a2030ba99eec8a8dfe8743e105b Mon Sep 17 00:00:00 2001 From: blackrock-engineering Date: Thu, 26 Jul 2018 10:29:15 +0100 Subject: [PATCH 3/4] Fix indentation. --- .../symphony/clients/impl/UsersClientImpl.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java b/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java index 0d06156c..1adce2a1 100644 --- a/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java +++ b/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java @@ -333,15 +333,15 @@ public Set getAllUsersWithDetails() throws UsersClientException { symUser.setFeatures(featureList); userDetail = userApi.v1AdminUserUidGet(sessionToken, uid); symUser.setRoles(new HashSet<>(userDetail.getRoles())); - + if (userDetail.getUserSystemInfo() != null) { - symUser.setActive(UserSystemInfo.StatusEnum.ENABLED == userDetail.getUserSystemInfo().getStatus()); - if (userDetail.getUserSystemInfo().getLastLoginDate() != null) { - symUser.setLastLoginDate(new Date(userDetail.getUserSystemInfo().getLastLoginDate())); - } - if (userDetail.getUserSystemInfo().getCreatedDate() != null) { - symUser.setCreatedDate(new Date(userDetail.getUserSystemInfo().getCreatedDate())); - } + symUser.setActive(UserSystemInfo.StatusEnum.ENABLED == userDetail.getUserSystemInfo().getStatus()); + if (userDetail.getUserSystemInfo().getLastLoginDate() != null) { + symUser.setLastLoginDate(new Date(userDetail.getUserSystemInfo().getLastLoginDate())); + } + if (userDetail.getUserSystemInfo().getCreatedDate() != null) { + symUser.setCreatedDate(new Date(userDetail.getUserSystemInfo().getCreatedDate())); + } } } From 5b2cd53bbec5eea98a03c2be9529a69f1733bce3 Mon Sep 17 00:00:00 2001 From: blackrock-engineering Date: Wed, 22 Aug 2018 18:30:05 +0100 Subject: [PATCH 4/4] Modify configuration for 'get all users timeout' to be managed via SymphonyClientConfig (in response to pull request comments). --- .../client/SymphonyClientConfigBuilder.java | 30 ++++++++++++------- .../client/SymphonyClientConfigID.java | 1 + .../clients/impl/UsersClientImpl.java | 4 ++- 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/symphony-client/src/main/java/org/symphonyoss/client/SymphonyClientConfigBuilder.java b/symphony-client/src/main/java/org/symphonyoss/client/SymphonyClientConfigBuilder.java index a1275399..4b0ac96e 100644 --- a/symphony-client/src/main/java/org/symphonyoss/client/SymphonyClientConfigBuilder.java +++ b/symphony-client/src/main/java/org/symphonyoss/client/SymphonyClientConfigBuilder.java @@ -81,6 +81,7 @@ public static interface UserCredsStep public static interface BuildStep { BuildStep withReceiverEmail(String receiverEmail); + BuildStep withGetAllUsersTimeout(String getAllUsersTimeout); BuildStep withServices(boolean enabled); BuildStep withJMXHealthcheck(boolean enabled); SymphonyClientConfig build(); @@ -105,6 +106,7 @@ private static class Steps private String userCertFilename = null; private char[] userCertPassword = null; private String receiverEmail = null; + private String getAllUsersTimeout = null; private boolean servicesEnabled = true; private boolean jmxHealthCheckEnabled = true; @@ -160,6 +162,13 @@ public BuildStep withReceiverEmail(String receiverEmail) return (this); } + @Override + public BuildStep withGetAllUsersTimeout(String getAllUsersTimeout) + { + this.getAllUsersTimeout = getAllUsersTimeout; + return (this); + } + @Override public BuildStep withServices(boolean enabled) { @@ -179,16 +188,17 @@ public SymphonyClientConfig build() { SymphonyClientConfig result = new SymphonyClientConfig(); - if (this.sessionAuthUrl != null) result.set(SymphonyClientConfigID.SESSIONAUTH_URL, this.sessionAuthUrl); - if (this.keyAuthUrl != null) result.set(SymphonyClientConfigID.KEYAUTH_URL, this.keyAuthUrl); - if (this.podUrl != null) result.set(SymphonyClientConfigID.POD_URL, this.podUrl); - if (this.agentUrl != null) result.set(SymphonyClientConfigID.AGENT_URL, this.agentUrl); - if (this.trustStoreFilename != null) result.set(SymphonyClientConfigID.TRUSTSTORE_FILE, this.trustStoreFilename); - if (this.trustStorePassword != null) result.set(SymphonyClientConfigID.TRUSTSTORE_PASSWORD, new String(this.trustStorePassword)); // SECURITY HOLE DUE TO STRING INTERNING - if (this.userEmail != null) result.set(SymphonyClientConfigID.USER_EMAIL, this.userEmail); - if (this.userCertFilename != null) result.set(SymphonyClientConfigID.USER_CERT_FILE, this.userCertFilename); - if (this.userCertPassword != null) result.set(SymphonyClientConfigID.USER_CERT_PASSWORD, new String(this.userCertPassword)); // SECURITY HOLE DUE TO STRING INTERNING - if (this.receiverEmail != null) result.set(SymphonyClientConfigID.RECEIVER_EMAIL, this.receiverEmail); + if (this.sessionAuthUrl != null) result.set(SymphonyClientConfigID.SESSIONAUTH_URL, this.sessionAuthUrl); + if (this.keyAuthUrl != null) result.set(SymphonyClientConfigID.KEYAUTH_URL, this.keyAuthUrl); + if (this.podUrl != null) result.set(SymphonyClientConfigID.POD_URL, this.podUrl); + if (this.agentUrl != null) result.set(SymphonyClientConfigID.AGENT_URL, this.agentUrl); + if (this.trustStoreFilename != null) result.set(SymphonyClientConfigID.TRUSTSTORE_FILE, this.trustStoreFilename); + if (this.trustStorePassword != null) result.set(SymphonyClientConfigID.TRUSTSTORE_PASSWORD, new String(this.trustStorePassword)); // SECURITY HOLE DUE TO STRING INTERNING + if (this.userEmail != null) result.set(SymphonyClientConfigID.USER_EMAIL, this.userEmail); + if (this.userCertFilename != null) result.set(SymphonyClientConfigID.USER_CERT_FILE, this.userCertFilename); + if (this.userCertPassword != null) result.set(SymphonyClientConfigID.USER_CERT_PASSWORD, new String(this.userCertPassword)); // SECURITY HOLE DUE TO STRING INTERNING + if (this.receiverEmail != null) result.set(SymphonyClientConfigID.RECEIVER_EMAIL, this.receiverEmail); + if (this.getAllUsersTimeout != null) result.set(SymphonyClientConfigID.GET_ALL_USERS_TIMEOUT, this.getAllUsersTimeout); result.set(SymphonyClientConfigID.DISABLE_SERVICES, String.valueOf(!this.servicesEnabled)); result.set(SymphonyClientConfigID.HEALTHCHECK_JMX_ENABLED, String.valueOf(this.jmxHealthCheckEnabled)); diff --git a/symphony-client/src/main/java/org/symphonyoss/client/SymphonyClientConfigID.java b/symphony-client/src/main/java/org/symphonyoss/client/SymphonyClientConfigID.java index 66de9add..c52b7b9c 100644 --- a/symphony-client/src/main/java/org/symphonyoss/client/SymphonyClientConfigID.java +++ b/symphony-client/src/main/java/org/symphonyoss/client/SymphonyClientConfigID.java @@ -28,6 +28,7 @@ public enum SymphonyClientConfigID { USER_CERT_PASSWORD("javax.net.ssl.keyStorePassword"), USER_EMAIL, RECEIVER_EMAIL(false), + GET_ALL_USERS_TIMEOUT(false), DISABLE_SERVICES(false), HEALTHCHECK_JMX_ENABLED(false); diff --git a/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java b/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java index 1adce2a1..22eec6ca 100644 --- a/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java +++ b/symphony-client/src/main/java/org/symphonyoss/symphony/clients/impl/UsersClientImpl.java @@ -23,6 +23,8 @@ package org.symphonyoss.symphony.clients.impl; import com.google.common.base.Strings; + +import org.apache.commons.lang3.math.NumberUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.symphonyoss.client.SymphonyClientConfig; @@ -91,7 +93,7 @@ public UsersClientImpl(SymAuth symAuth, SymphonyClientConfig config, Client http apiClient.setBasePath(config.get(SymphonyClientConfigID.POD_URL)); - getAllUsersTimeout = Long.valueOf(System.getProperty("SYMPHONY_GET_ALL_USERS_TIMEOUT", "5")).longValue(); + getAllUsersTimeout = NumberUtils.toLong(config.get(SymphonyClientConfigID.GET_ALL_USERS_TIMEOUT), 5); }