[GitHub] [nifi] jfrazee commented on a change in pull request #4843: NIFI-8258: Add support for Service Principal authentication in ADLS p…

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

jfrazee commented on a change in pull request #4843:
URL: https://github.com/apache/nifi/pull/4843#discussion_r583023120



##########
File path: nifi-nar-bundles/nifi-azure-bundle/nifi-azure-processors/src/main/java/org/apache/nifi/services/azure/storage/ADLSCredentialsControllerService.java
##########
@@ -74,12 +75,56 @@
         .addValidator(StandardValidators.BOOLEAN_VALIDATOR)
         .build();
 
+    public static final PropertyDescriptor SERVICE_PRINCIPAL_TENANT_ID = new PropertyDescriptor.Builder()
+            .name("service-principal-tenant-id")
+            .displayName("Service Principal Tenant ID")
+            .description("Tenant ID of the Azure Active Directory hosting the Service Principal. The property is required when Service Principal authentication is used.")
+            .sensitive(true)
+            .required(false)
+            .addValidator(StandardValidators.NON_BLANK_VALIDATOR)
+            .expressionLanguageSupported(ExpressionLanguageScope.FLOWFILE_ATTRIBUTES)

Review comment:
       That is definitely an issue, but there is an important use case for it -- when you need to shard the data across a lot of storage accounts for performance reasons or you're using storage accounts that may only be known at runtime. When I've seen this, though, the volatile provenance repository was being used so the surface area was small.
   
   It is called out in the property documentation but maybe it could be louder ("certain risks" doesn't sound very scary) or include a more detailed explanation in additional details. What's there now: 
   
   > 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). In addition, the provenance repositories may be put on encrypted disk partitions.
   
   
   
   




----------------------------------------------------------------
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