[GitHub] [nifi] thenatog commented on a change in pull request #5131: NIFI-8651: Refactor Sensitive Properties Providers for extension

[GitBox <gi...@apache.org>]

thenatog commented on a change in pull request #5131:
URL: https://github.com/apache/nifi/pull/5131#discussion_r650008806



##########
File path: nifi-commons/nifi-sensitive-property-provider/src/main/java/org/apache/nifi/properties/ApplicationPropertiesProtector.java
##########
@@ -0,0 +1,348 @@
+/*
+ * 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.
+ */
+package org.apache.nifi.properties;
+
+import org.apache.commons.lang3.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+import static java.util.Arrays.asList;
+
+/**
+ * Class performing unprotection activities before returning a clean
+ * implementation of {@link ApplicationProperties}.
+ * This encapsulates the sensitive property access logic from external consumers
+ * of {@code ApplicationProperties}.
+ *
+ * @param <T> The type of protected application properties
+ * @param <U> The type of standard application properties that backs the protected application properties
+ */
+public class ApplicationPropertiesProtector<T extends ProtectedProperties<U>, U extends ApplicationProperties>
+        implements SensitivePropertyProtector<T, U> {
+    public static final String PROTECTED_KEY_SUFFIX = ".protected";
+
+    private static final Logger logger = LoggerFactory.getLogger(ApplicationPropertiesProtector.class);
+
+    private T protectedProperties;
+
+    private Map<String, SensitivePropertyProvider> localProviderCache = new HashMap<>();
+
+    /**
+     * Creates an instance containing the provided {@link ProtectedProperties}.
+     *
+     * @param protectedProperties the ProtectedProperties to contain
+     */
+    public ApplicationPropertiesProtector(final T protectedProperties) {
+        this.protectedProperties = protectedProperties;
+        logger.debug("Loaded {} properties (including {} protection schemes) into {}", getPropertyKeysIncludingProtectionSchemes().size(),
+                getProtectedPropertyKeys().size(), this.getClass().getName());
+    }
+
+    /**
+     * Returns the sibling property key which specifies the protection scheme for this key.
+     * <p>
+     * Example:
+     * <p>
+     * nifi.sensitive.key=ABCXYZ
+     * nifi.sensitive.key.protected=aes/gcm/256
+     * <p>
+     * nifi.sensitive.key -> nifi.sensitive.key.protected
+     *
+     * @param key the key identifying the sensitive property
+     * @return the key identifying the protection scheme for the sensitive property
+     */
+    public static String getProtectionKey(final String key) {
+        if (key == null || key.isEmpty()) {
+            throw new IllegalArgumentException("Cannot find protection key for null key");
+        }
+
+        return key + PROTECTED_KEY_SUFFIX;
+    }
+
+    /**
+     * Retrieves all known property keys.
+     *
+     * @return all known property keys
+     */
+    @Override
+    public Set<String> getPropertyKeys() {
+        Set<String> filteredKeys = getPropertyKeysIncludingProtectionSchemes();
+        filteredKeys.removeIf(p -> p.endsWith(PROTECTED_KEY_SUFFIX));
+        return filteredKeys;
+    }
+
+    @Override
+    public int size() {
+        return getPropertyKeys().size();
+    }
+
+    @Override
+    public Set<String> getPropertyKeysIncludingProtectionSchemes() {
+        return protectedProperties.getApplicationProperties().getPropertyKeys();
+    }
+
+    /**
+     * Splits a single string containing multiple property keys into a List. Delimited by ',' or ';' and ignores leading and trailing whitespace around delimiter.
+     *
+     * @param multipleProperties a single String containing multiple properties, i.e. "nifi.property.1; nifi.property.2, nifi.property.3"
+     * @return a List containing the split and trimmed properties
+     */
+    private static List<String> splitMultipleProperties(final String multipleProperties) {
+        if (multipleProperties == null || multipleProperties.trim().isEmpty()) {
+            return new ArrayList<>(0);
+        } else {
+            List<String> properties = new ArrayList<>(asList(multipleProperties.split("\\s*[,;]\\s*")));
+            for (int i = 0; i < properties.size(); i++) {
+                properties.set(i, properties.get(i).trim());
+            }
+            return properties;
+        }
+    }
+
+    private String getProperty(final String key) {
+        return protectedProperties.getApplicationProperties().getProperty(key);
+    }
+
+    private String getAdditionalSensitivePropertiesKeys() {
+        return getProperty(protectedProperties.getAdditionalSensitivePropertiesKeysName());
+    }
+
+    private String getAdditionalSensitivePropertiesKeysName() {
+        return protectedProperties.getAdditionalSensitivePropertiesKeysName();
+    }
+
+    @Override
+    public List<String> getSensitivePropertyKeys() {
+        final String additionalPropertiesString = getAdditionalSensitivePropertiesKeys();
+        final String additionalPropertiesKeyName = protectedProperties.getAdditionalSensitivePropertiesKeysName();
+        if (additionalPropertiesString == null || additionalPropertiesString.trim().isEmpty()) {
+            return protectedProperties.getDefaultSensitiveProperties();
+        } else {
+            List<String> additionalProperties = splitMultipleProperties(additionalPropertiesString);
+            /* Remove this key if it was accidentally provided as a sensitive key
+             * because we cannot protect it and read from it
+            */
+            if (additionalProperties.contains(additionalPropertiesKeyName)) {
+                logger.warn("The key '{}' contains itself. This is poor practice and should be removed", additionalPropertiesKeyName);
+                additionalProperties.remove(additionalPropertiesKeyName);
+            }
+            additionalProperties.addAll(protectedProperties.getDefaultSensitiveProperties());
+            return additionalProperties;
+        }
+    }
+
+    @Override
+    public List<String> getPopulatedSensitivePropertyKeys() {
+        List<String> allSensitiveKeys = getSensitivePropertyKeys();
+        return allSensitiveKeys.stream().filter(k -> StringUtils.isNotBlank(getProperty(k))).collect(Collectors.toList());
+    }
+
+    @Override
+    public boolean hasProtectedKeys() {
+        final List<String> sensitiveKeys = getSensitivePropertyKeys();
+        for (String k : sensitiveKeys) {
+            if (isPropertyProtected(k)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
+    @Override
+    public Map<String, String> getProtectedPropertyKeys() {
+        final List<String> sensitiveKeys = getSensitivePropertyKeys();
+
+        // This is the Java 8 way, but can likely be optimized (and not sure of correctness)

Review comment:
       Are these examples here for a reason?




-- 
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:
users@infra.apache.org