smolnar82 commented on a change in pull request #218: KNOX-2134 - Checking if
password is available via local alias service before going to fetch it from
remote ZK server
URL: https://github.com/apache/knox/pull/218#discussion_r358968189
##########
File path:
gateway-server/src/main/java/org/apache/knox/gateway/services/security/impl/ZookeeperRemoteAliasService.java
##########
@@ -44,461 +44,403 @@
* An {@link AliasService} implementation based on zookeeper remote service
registry.
*/
public class ZookeeperRemoteAliasService implements AliasService {
- public static final String TYPE = "zookeeper";
-
- public static final String PATH_KNOX = "/knox";
- public static final String PATH_KNOX_SECURITY = PATH_KNOX + "/security";
- public static final String PATH_KNOX_ALIAS_STORE_TOPOLOGY =
- PATH_KNOX_SECURITY + "/topology";
- public static final String PATH_SEPARATOR = "/";
-
- private static final GatewayMessages LOG =
MessagesFactory.get(GatewayMessages.class);
- // N.B. This is ZooKeeper-specific, and should be abstracted when another
registry is supported
- private static final RemoteConfigurationRegistryClient.EntryACL
AUTHENTICATED_USERS_ALL =
- new RemoteConfigurationRegistryClient.EntryACL() {
+ public static final String TYPE = "zookeeper";
+ public static final String PATH_KNOX = "/knox";
+ public static final String PATH_KNOX_SECURITY = PATH_KNOX + "/security";
+ public static final String PATH_KNOX_ALIAS_STORE_TOPOLOGY =
PATH_KNOX_SECURITY + "/topology";
+ public static final String PATH_SEPARATOR = "/";
+
+ private static final GatewayMessages LOG =
MessagesFactory.get(GatewayMessages.class);
+ // N.B. This is ZooKeeper-specific, and should be abstracted when another
registry is supported
+ private static final RemoteConfigurationRegistryClient.EntryACL
AUTHENTICATED_USERS_ALL = new RemoteConfigurationRegistryClient.EntryACL() {
@Override
public String getId() {
- return "";
+ return "";
}
@Override
public String getType() {
- return "auth";
+ return "auth";
}
@Override
public Object getPermissions() {
- return ZooDefs.Perms.ALL;
+ return ZooDefs.Perms.ALL;
}
@Override
public boolean canRead() {
- return true;
+ return true;
}
@Override
public boolean canWrite() {
- return true;
+ return true;
}
- };
-
- private final AliasService localAliasService;
- private final MasterService ms;
- private final RemoteConfigurationRegistryClientService
remoteConfigurationRegistryClientService;
-
- private RemoteConfigurationRegistryClient remoteClient;
- private ConfigurableEncryptor encryptor;
- private GatewayConfig config;
-
- ZookeeperRemoteAliasService(AliasService localAliasService, MasterService ms,
- RemoteConfigurationRegistryClientService
remoteConfigurationRegistryClientService) {
- this.localAliasService = localAliasService;
- this.ms = ms;
- this.remoteConfigurationRegistryClientService =
remoteConfigurationRegistryClientService;
- }
-
- /**
- * Build an entry path for the given cluster and alias
- */
- private static String buildAliasEntryName(final String clusterName,
- final String alias) {
- // Convert all alias names to lower case (JDK-4891485)
- return buildClusterEntryName(clusterName) + PATH_SEPARATOR +
alias.toLowerCase(Locale.ROOT);
- }
-
- /**
- * Build an entry path for the given cluster
- */
- private static String buildClusterEntryName(final String clusterName) {
- return PATH_KNOX_ALIAS_STORE_TOPOLOGY + PATH_SEPARATOR + clusterName;
- }
-
- /**
- * Ensure that the given entry path exists.
- */
- private static void ensureEntry(final String path,
- final RemoteConfigurationRegistryClient remoteClient) {
- if (!remoteClient.entryExists(path)) {
- remoteClient.createEntry(path);
- } else {
- // Validate the ACL
- List<RemoteConfigurationRegistryClient.EntryACL> entryACLs = remoteClient
- .getACL(path);
- for (RemoteConfigurationRegistryClient.EntryACL entryACL : entryACLs) {
- // N.B. This is ZooKeeper-specific, and should be abstracted when
another registry is supported
- // For now, check for world:anyone with ANY permissions (even
read-only)
- if (entryACL.getType().equals("world") && entryACL.getId()
- .equals("anyone")) {
- LOG.suspectWritableRemoteConfigurationEntry(path);
-
- // If the client is authenticated, but "anyone" can write the
content, then the content may not
- // be trustworthy.
- if (remoteClient.isAuthenticationConfigured()) {
- LOG.correctingSuspectWritableRemoteConfigurationEntry(path);
-
- // Replace the existing ACL with one that permits only
authenticated users
- remoteClient.setACL(path,
- Collections.singletonList(AUTHENTICATED_USERS_ALL));
- }
+ };
+
+ private final AliasService localAliasService;
+ private final MasterService ms;
+ private final RemoteConfigurationRegistryClientService
remoteConfigurationRegistryClientService;
+
+ private RemoteConfigurationRegistryClient remoteClient;
+ private ConfigurableEncryptor encryptor;
+ private GatewayConfig config;
+
+ ZookeeperRemoteAliasService(AliasService localAliasService, MasterService
ms, RemoteConfigurationRegistryClientService
remoteConfigurationRegistryClientService) {
+ this.localAliasService = localAliasService;
+ this.ms = ms;
+ this.remoteConfigurationRegistryClientService =
remoteConfigurationRegistryClientService;
+ }
+
+ /**
+ * Build an entry path for the given cluster and alias
+ */
+ private static String buildAliasEntryName(final String clusterName, final
String alias) {
+ // Convert all alias names to lower case (JDK-4891485)
+ return buildClusterEntryName(clusterName) + PATH_SEPARATOR +
alias.toLowerCase(Locale.ROOT);
+ }
+
+ /**
+ * Build an entry path for the given cluster
+ */
+ private static String buildClusterEntryName(final String clusterName) {
+ return PATH_KNOX_ALIAS_STORE_TOPOLOGY + PATH_SEPARATOR + clusterName;
+ }
+
+ /**
+ * Ensure that the given entry path exists.
+ */
+ private static void ensureEntry(final String path, final
RemoteConfigurationRegistryClient remoteClient) {
+ if (!remoteClient.entryExists(path)) {
+ remoteClient.createEntry(path);
+ } else {
+ // Validate the ACL
+ List<RemoteConfigurationRegistryClient.EntryACL> entryACLs =
remoteClient.getACL(path);
+ for (RemoteConfigurationRegistryClient.EntryACL entryACL :
entryACLs) {
+ // N.B. This is ZooKeeper-specific, and should be abstracted
when another registry is supported
+ // For now, check for world:anyone with ANY permissions (even
read-only)
+ if ("world".equals(entryACL.getType()) &&
"anyone".equals(entryACL.getId())) {
+ LOG.suspectWritableRemoteConfigurationEntry(path);
+
+ // If the client is authenticated, but "anyone" can write
the content, then the
+ // content may not be trustworthy.
+ if (remoteClient.isAuthenticationConfigured()) {
+
LOG.correctingSuspectWritableRemoteConfigurationEntry(path);
+
+ // Replace the existing ACL with one that permits only
authenticated users
+ remoteClient.setACL(path,
Collections.singletonList(AUTHENTICATED_USERS_ALL));
+ }
+ }
+ }
}
- }
}
- }
-
- /**
- * Check to make sure all the required entries are properly set up
- */
- private static void checkPathsExist(
- final RemoteConfigurationRegistryClient remoteClient) {
- ensureEntry(PATH_KNOX, remoteClient);
- ensureEntry(PATH_KNOX_SECURITY, remoteClient);
- ensureEntry(PATH_KNOX_ALIAS_STORE_TOPOLOGY, remoteClient);
- ensureEntry(
- PATH_KNOX_ALIAS_STORE_TOPOLOGY + PATH_SEPARATOR + NO_CLUSTER_NAME,
- remoteClient);
- }
-
- /**
- * Get a list of all aliases for a given cluster.
- * Remote aliases are preferred over local.
- *
- * @param clusterName cluster name
- * @return List of all the aliases
- */
- @Override
- public List<String> getAliasesForCluster(final String clusterName) throws
AliasServiceException {
- List<String> remoteAliases = new ArrayList<>();
-
- /* If we have remote registry configured, query it */
- if (remoteClient != null) {
- remoteAliases = remoteClient
- .listChildEntries(buildClusterEntryName(clusterName));
+
+ /**
+ * Check to make sure all the required entries are properly set up
+ */
+ private static void checkPathsExist(final
RemoteConfigurationRegistryClient remoteClient) {
+ ensureEntry(PATH_KNOX, remoteClient);
+ ensureEntry(PATH_KNOX_SECURITY, remoteClient);
+ ensureEntry(PATH_KNOX_ALIAS_STORE_TOPOLOGY, remoteClient);
+ ensureEntry(PATH_KNOX_ALIAS_STORE_TOPOLOGY + PATH_SEPARATOR +
NO_CLUSTER_NAME, remoteClient);
}
- return remoteAliases;
- }
-
- @Override
- public void addAliasForCluster(final String clusterName,
- final String alias, final String value)
- throws AliasServiceException {
- if (remoteClient != null) {
- final String aliasEntryPath = buildAliasEntryName(clusterName, alias);
-
- /* Ensure the entries are properly set up */
- checkPathsExist(remoteClient);
- ensureEntry(buildClusterEntryName(clusterName), remoteClient);
- try {
- remoteClient.createEntry(aliasEntryPath, encrypt(value));
- } catch (Exception e) {
- throw new AliasServiceException(e);
- }
-
- if (remoteClient.getEntryData(aliasEntryPath) == null) {
- throw new IllegalStateException(String.format(Locale.ROOT,
- "Failed to store alias %s for cluster %s in remote registry",
alias,
- clusterName));
- }
+ /**
+ * Get a list of all aliases for a given cluster. Remote aliases are
preferred over local.
+ *
+ * @param clusterName
+ * cluster name
+ * @return List of all the aliases
+ */
+ @Override
+ public List<String> getAliasesForCluster(final String clusterName) throws
AliasServiceException {
+ return remoteClient == null ? new ArrayList<>() :
remoteClient.listChildEntries(buildClusterEntryName(clusterName));
Review comment:
This is correct; I was only focusing on retrieving passwords as quickly as
possible. Let me fix this and submit a new patch soon.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
