[GitHub] [nifi] tpalfy commented on a diff in pull request #6777: NIFI-10969: Created extension point for Signer Override in AWS S3 pro…

[GitBox]

tpalfy commented on code in PR #6777:
URL: https://github.com/apache/nifi/pull/6777#discussion_r1049665128


##########
nifi-nar-bundles/nifi-aws-bundle/nifi-aws-processors/src/test/java/org/apache/nifi/processors/aws/credentials/provider/factory/TestCredentialsProviderFactory.java:
##########
@@ -296,8 +285,42 @@ public void testAssumeRoleMissingProxyPort() throws 
Throwable {
     public void testAssumeRoleInvalidProxyPort() throws Throwable {
         final TestRunner runner = 
TestRunners.newTestRunner(MockAWSProcessor.class);
         runner.setProperty(CredentialPropertyDescriptors.CREDENTIALS_FILE, 
"src/test/resources/mock-aws-credentials.properties");
+        runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_ARN, 
"BogusArn");
+        runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_NAME, 
"BogusSession");
         
runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_PROXY_HOST, 
"proxy.company.com");
         
runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_PROXY_PORT, 
"notIntPort");
         runner.assertNotValid();
     }
+
+    @Test
+    public void testAssumeRoleCredentialsWithCustomSigner() throws Exception {
+        final TestRunner runner = 
TestRunners.newTestRunner(MockAWSProcessor.class);
+        runner.setProperty(CredentialPropertyDescriptors.CREDENTIALS_FILE, 
"src/test/resources/mock-aws-credentials.properties");
+        runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_ARN, 
"BogusArn");
+        runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_NAME, 
"BogusSession");
+        
runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_STS_SIGNER_OVERRIDE,
 AwsSignerType.CUSTOM_SIGNER.getValue());
+        
runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_STS_CUSTOM_SIGNER_CLASS_NAME,
 CustomSTSSigner.class.getName());
+        runner.assertValid();
+
+        final Map<PropertyDescriptor, String> properties = 
runner.getProcessContext().getProperties();
+        final CredentialsProviderFactory factory = new 
CredentialsProviderFactory();
+
+        final AWSCredentialsProvider credentialsProvider = 
factory.getCredentialsProvider(properties);
+
+        final Field stsClientField = 
credentialsProvider.getClass().getDeclaredField("securityTokenService");
+        stsClientField.setAccessible(true);
+        AWSSecurityTokenServiceClient stsClient = 
(AWSSecurityTokenServiceClient) stsClientField.get(credentialsProvider);
+
+        ClientConfiguration stsClientConfig = 
stsClient.getClientConfiguration();
+
+        final String signerName = stsClientConfig.getSignerOverride();
+        assertNotNull(signerName);
+        final Signer signer = SignerFactory.createSigner(signerName, new 
SignerParams("sts", "us-west-2"));
+        assertNotNull(signer);
+        assertSame(CustomSTSSigner.class, signer.getClass());
+    }
+
+    public static class CustomSTSSigner extends AWS4Signer {
+
+    }

Review Comment:
   The `.setAccessible(true);` call should be avoided as it may even break in 
newer Java versions (16 and 17 onwards).
   
   We can use a more traditional approach:
   
   ```suggestion
       @Test
       public void testAssumeRoleCredentialsWithCustomSigner() throws Exception 
{
           final TestRunner runner = 
TestRunners.newTestRunner(MockAWSProcessor.class);
           runner.setProperty(CredentialPropertyDescriptors.CREDENTIALS_FILE, 
"src/test/resources/mock-aws-credentials.properties");
           runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_ARN, 
"BogusArn");
           runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_NAME, 
"BogusSession");
           
runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_STS_SIGNER_OVERRIDE,
 AwsSignerType.CUSTOM_SIGNER.getValue());
           
runner.setProperty(CredentialPropertyDescriptors.ASSUME_ROLE_STS_CUSTOM_SIGNER_CLASS_NAME,
 CustomSTSSigner.class.getName());
           runner.assertValid();
   
           final Map<PropertyDescriptor, String> properties = 
runner.getProcessContext().getProperties();
           final CredentialsProviderFactory factory = new 
CredentialsProviderFactory();
   
           Signer signerChecker = Mockito.mock(Signer.class);
           CustomSTSSigner.setSignerChecker(signerChecker);
   
           final AWSCredentialsProvider credentialsProvider = 
factory.getCredentialsProvider(properties);
   
           try {
               credentialsProvider.getCredentials();
           } catch (Exception e) {
               // Expected to fail, we are only intersted in the Signer
           }
   
           verify(signerChecker).sign(any(), any());
       }
   
       public static class CustomSTSSigner extends AWS4Signer {
           private static final ThreadLocal<Signer> SIGNER_CHECKER = new 
ThreadLocal<>();
   
           public static void setSignerChecker(Signer signerChecker) {
               SIGNER_CHECKER.set(signerChecker);
           }
   
           @Override
           public void sign(SignableRequest<?> request, AWSCredentials 
credentials) {
               SIGNER_CHECKER.get().sign(request, credentials);
           }
       }
   ```



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

To unsubscribe, e-mail: issues-unsubscr...@nifi.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org