NIFI-8656 Support EL for SAS Token in the ADLS Gen2 processors

This closes #5119

Signed-off-by: Joey Frazee <jfrazee@apache.org>

nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/processors/azure/storage/utils/AzureStorageUtils.java

@@ -56,37 +56,46 @@ public final class AzureStorageUtils {
     public static final String STORAGE_SAS_TOKEN_PROPERTY_DESCRIPTOR_NAME = "storage-sas-token";
     public static final String STORAGE_ENDPOINT_SUFFIX_PROPERTY_DESCRIPTOR_NAME = "storage-endpoint-suffix";
 
+    public static final String ACCOUNT_KEY_BASE_DESCRIPTION =
+            "The storage account key. This is an admin-like password providing access to every container in this account. It is recommended " +
+            "one uses Shared Access Signature (SAS) token instead for fine-grained control with policies.";
+
+    public static final String ACCOUNT_KEY_SECURITY_DESCRIPTION =
+            " There are certain risks in allowing the account key to be stored as a flowfile " +
+            "attribute. While it does provide for a more flexible flow by allowing the account key to " +
+            "be fetched dynamically from a flowfile attribute, care must be taken to restrict access to " +
+            "the event provenance data (e.g., by strictly controlling the policies governing provenance for this processor). " +
+            "In addition, the provenance repositories may be put on encrypted disk partitions.";
+
     public static final PropertyDescriptor ACCOUNT_KEY = new PropertyDescriptor.Builder()
             .name(STORAGE_ACCOUNT_KEY_PROPERTY_DESCRIPTOR_NAME)
             .displayName("Storage Account Key")
-            .description("The storage account key. This is an admin-like password providing access to every container in this account. It is recommended " +
+            .description(ACCOUNT_KEY_BASE_DESCRIPTION + ACCOUNT_KEY_SECURITY_DESCRIPTION)
-                    "one uses Shared Access Signature (SAS) token instead for fine-grained control with policies. " +
-                    "There are certain risks in allowing the account key to be stored as a flowfile " +
-                    "attribute. While it does provide for a more flexible flow by allowing the account key to " +
-                    "be fetched dynamically from a flow file attribute, care must be taken to restrict access to " +
-                    "the event provenance data (e.g. by strictly controlling the policies governing provenance for this Processor). " +
-                    "In addition, the provenance repositories may be put on encrypted disk partitions.")
             .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
             .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
             .required(false)
             .sensitive(true)
             .build();
 
-    public static final String ACCOUNT_NAME_BASE_DESCRIPTION =
+    public static final String ACCOUNT_NAME_BASE_DESCRIPTION = "The storage account name.";
-            "The storage account name.  There are certain risks in allowing the account name to be stored as a flowfile " +
+
+    public static final String ACCOUNT_NAME_SECURITY_DESCRIPTION =
+            " There are certain risks in allowing the account name to be stored as a flowfile " +
             "attribute. While it does provide for a more flexible flow by allowing the account name to " +
             "be fetched dynamically from a flowfile attribute, care must be taken to restrict access to " +
-            "the event provenance data (e.g. by strictly controlling the policies governing provenance for this Processor). " +
+            "the event provenance data (e.g., by strictly controlling the policies governing provenance for this processor). " +
             "In addition, the provenance repositories may be put on encrypted disk partitions.";
 
+    public static final String ACCOUNT_NAME_CREDENTIAL_SERVICE_DESCRIPTION =
+            " Instead of defining the Storage Account Name, Storage Account Key and SAS Token properties directly on the processor, " +
+            "the preferred way is to configure them through a controller service specified in the Storage Credentials property. " +
+            "The controller service can provide a common/shared configuration for multiple/all Azure processors. Furthermore, the credentials " +
+            "can also be looked up dynamically with the 'Lookup' version of the service.";
+
     public static final PropertyDescriptor ACCOUNT_NAME = new PropertyDescriptor.Builder()
             .name(STORAGE_ACCOUNT_NAME_PROPERTY_DESCRIPTOR_NAME)
             .displayName("Storage Account Name")
-            .description(ACCOUNT_NAME_BASE_DESCRIPTION +
+            .description(ACCOUNT_NAME_BASE_DESCRIPTION + ACCOUNT_NAME_SECURITY_DESCRIPTION + ACCOUNT_NAME_CREDENTIAL_SERVICE_DESCRIPTION)
-                    " Instead of defining the Storage Account Name, Storage Account Key and SAS Token properties directly on the processor, " +
-                    "the preferred way is to configure them through a controller service specified in the Storage Credentials property. " +
-                    "The controller service can provide a common/shared configuration for multiple/all Azure processors. Furthermore, the credentials " +
-                    "can also be looked up dynamically with the 'Lookup' version of the service.")
             .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
             .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
             .required(false)
@@ -117,15 +126,19 @@ public final class AzureStorageUtils {
             .required(true)
             .build();
 
+    public static final String SAS_TOKEN_BASE_DESCRIPTION = "Shared Access Signature token, including the leading '?'. Specify either SAS token (recommended) or Account Key.";
+
+    public static final String SAS_TOKEN_SECURITY_DESCRIPTION =
+            " There are certain risks in allowing the SAS token to be stored as a flowfile " +
+            "attribute. While it does provide for a more flexible flow by allowing the SAS token to " +
+            "be fetched dynamically from a flowfile attribute, care must be taken to restrict access to " +
+            "the event provenance data (e.g., by strictly controlling the policies governing provenance for this processor). " +
+            "In addition, the provenance repositories may be put on encrypted disk partitions.";
+
     public static final PropertyDescriptor PROP_SAS_TOKEN = new PropertyDescriptor.Builder()
             .name(STORAGE_SAS_TOKEN_PROPERTY_DESCRIPTOR_NAME)
             .displayName("SAS Token")
-            .description("Shared Access Signature token, including the leading '?'. Specify either SAS Token (recommended) or Account Key. " +
+            .description(SAS_TOKEN_BASE_DESCRIPTION + SAS_TOKEN_SECURITY_DESCRIPTION)
-                    "There are certain risks in allowing the SAS token to be stored as a flowfile " +
-                    "attribute. While it does provide for a more flexible flow by allowing the account name to " +
-                    "be fetched dynamically from a flowfile attribute, care must be taken to restrict access to " +
-                    "the event provenance data (e.g. by strictly controlling the policies governing provenance for this Processor). " +
-                    "In addition, the provenance repositories may be put on encrypted disk partitions.")
             .required(false)
             .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
             .sensitive(true)

nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/ADLSCredentialsControllerService.java

@@ -67,12 +67,13 @@ public class ADLSCredentialsControllerService extends AbstractControllerService
 
     public static final PropertyDescriptor ACCOUNT_KEY = new PropertyDescriptor.Builder()
             .fromPropertyDescriptor(AzureStorageUtils.ACCOUNT_KEY)
+            .description(AzureStorageUtils.ACCOUNT_KEY_BASE_DESCRIPTION)
             .expressionLanguageSupported(ExpressionLanguageScope.NONE)
             .build();
 
     public static final PropertyDescriptor SAS_TOKEN = new PropertyDescriptor.Builder()
             .fromPropertyDescriptor(AzureStorageUtils.PROP_SAS_TOKEN)
-            .expressionLanguageSupported(ExpressionLanguageScope.NONE)
+            .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
             .build();
 
     public static final PropertyDescriptor USE_MANAGED_IDENTITY = new PropertyDescriptor.Builder()

nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/AzureStorageCredentialsControllerService.java

@@ -49,7 +49,7 @@ public class AzureStorageCredentialsControllerService extends AbstractController
     public static final PropertyDescriptor ACCOUNT_NAME = new PropertyDescriptor.Builder()
             .name(AzureStorageUtils.ACCOUNT_NAME.getName())
             .displayName(AzureStorageUtils.ACCOUNT_NAME.getDisplayName())
-            .description(AzureStorageUtils.ACCOUNT_NAME_BASE_DESCRIPTION)
+            .description(AzureStorageUtils.ACCOUNT_NAME_BASE_DESCRIPTION + AzureStorageUtils.ACCOUNT_NAME_SECURITY_DESCRIPTION)
             .addValidator(StandardValidators.NON_EMPTY_VALIDATOR)
             .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)
             .required(true)

nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/resources/docs/org.apache.nifi.services.azure.storage.ADLSCredentialsControllerService/additionalDetails.html

@@ -0,0 +1,44 @@
+<!DOCTYPE html>
+<html lang="en">
+<!--
+  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.
+-->
+<head>
+    <meta charset="utf-8" />
+    <title>ADLSCredentialsControllerService</title>
+    <link rel="stylesheet" href="/nifi-docs/css/component-usage.css" type="text/css" />
+</head>
+
+<body>
+
+<h3>Security considerations of using Expression Language for sensitive properties</h3>
+
+<p>
+    Allowing Expression Language for a property has the advantage of configuring the property dynamically via FlowFile attributes
+    or Variable Registry entries. In case of sensitive properties, it also has a drawback of exposing sensitive information like
+    passwords, security keys or tokens. When the value of a sensitive property comes from a FlowFile attribute, it travels by the
+    FlowFile in clear text form and is also saved in the provenance repository. Variable Registry does not support the encryption of
+    sensitive information either. Due to these, the sensitive credential data can be exposed to unauthorized parties.
+<p>
+    Best practices for using Expression Language for sensitive properties:
+    <ul>
+        <li>use it only if necessary</li>
+        <li>control access to the flow and to provenance repository</li>
+        <li>encrypt disks storing FlowFiles and provenance data</li>
+        <li>if the sensitive data is a temporary token (like the SAS token), use a shorter lifetime and refresh the token periodically</li>
+    </ul>
+
+</body>
+</html>
+

nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/test/java/org/apache/nifi/services/azure/storage/TestADLSCredentialsControllerService.java

@@ -327,6 +327,24 @@ public class TestADLSCredentialsControllerService {
         assertNull(actual.getServicePrincipalClientSecret());
     }
 
+    @Test
+    public void testGetCredentialsDetailsWithSasTokenUsingEL() throws Exception {
+        configureAccountName();
+        configureSasTokenUsingEL();
+
+        runner.enableControllerService(credentialsService);
+
+        ADLSCredentialsDetails actual = credentialsService.getCredentialsDetails(new HashMap<>());
+        assertEquals(ACCOUNT_NAME_VALUE, actual.getAccountName());
+        assertEquals(SAS_TOKEN_VALUE, actual.getSasToken());
+        assertNull(actual.getAccountKey());
+        assertFalse(actual.getUseManagedIdentity());
+        assertNotNull(actual.getEndpointSuffix());
+        assertNull(actual.getServicePrincipalTenantId());
+        assertNull(actual.getServicePrincipalClientId());
+        assertNull(actual.getServicePrincipalClientSecret());
+    }
+
     @Test
     public void testGetCredentialsDetailsWithUseManagedIdentity() throws Exception {
         // GIVEN
@@ -417,6 +435,11 @@ public class TestADLSCredentialsControllerService {
         runner.setProperty(credentialsService, ADLSCredentialsControllerService.SAS_TOKEN, SAS_TOKEN_VALUE);
     }
 
+    private void configureSasTokenUsingEL() {
+        String variableName = "sas.token";
+        configurePropertyUsingEL(ADLSCredentialsControllerService.SAS_TOKEN, variableName, SAS_TOKEN_VALUE);
+    }
+
     private void configureUseManagedIdentity() {
         runner.setProperty(credentialsService, ADLSCredentialsControllerService.USE_MANAGED_IDENTITY, "true");
     }