9uapaw commented on a change in pull request #3206:
URL: https://github.com/apache/hadoop/pull/3206#discussion_r673708836
##########
File path:
hadoop-common-project/hadoop-auth/src/main/java/org/apache/hadoop/security/authentication/server/AuthenticationFilter.java
##########
@@ -237,6 +237,8 @@ public static SignerSecretProvider constructSecretProvider(
provider.init(config, ctx, validity);
} catch (Exception e) {
if (!disallowFallbackToRandomSecretProvider) {
+ LOG.error("Unable to initialize FileSignerSecretProvider, reason: "
+ + e.getMessage());
LOG.info("Unable to initialize FileSignerSecretProvider, " +
Review comment:
I think this should be removed as well.
##########
File path:
hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/server/TestAuthenticationFilter.java
##########
@@ -305,6 +305,33 @@ public void init(Properties config, ServletContext
servletContext,
filter.destroy();
}
}
+
+ @Test
+ public void testEmptySecretFileFallbacksToRandomSecret() throws Exception {
+ AuthenticationFilter filter = new AuthenticationFilter();
+ try {
+ FilterConfig config = Mockito.mock(FilterConfig.class);
+ Mockito.when(config.getInitParameter(
+ AuthenticationFilter.AUTH_TYPE)).thenReturn("simple");
+ File secretFile = File.createTempFile("test_empty_secret", ".txt");
Review comment:
This tempFile is missing a deleteOnExit call.
##########
File path:
hadoop-common-project/hadoop-auth/src/test/java/org/apache/hadoop/security/authentication/util/TestFileSignerSecretProvider.java
##########
@@ -48,4 +51,22 @@ public void testGetSecrets() throws Exception {
Assert.assertEquals(1, allSecrets.length);
Assert.assertArrayEquals(secretValue.getBytes(), allSecrets[0]);
}
+
+ @Test
+ public void testEmptySecretFileThrows() throws Exception {
+ File secretFile = File.createTempFile("test_empty_secret", ".txt");
+ assertTrue(secretFile.exists());
+
+ FileSignerSecretProvider secretProvider
+ = new FileSignerSecretProvider();
+ Properties secretProviderProps = new Properties();
+ secretProviderProps.setProperty(
+ AuthenticationFilter.SIGNATURE_SECRET_FILE,
+ secretFile.getAbsolutePath());
+
+ Exception exception = assertThrows(RuntimeException.class, () ->
Review comment:
As this patch could be a potential candidate for a branch 2.10 backport
(if anyone needs it of course), using lambda expressions is generally
discouraged. I personally find it useful here and there, but it is really
divising among the community. My advice would be to use it whenever you have a
huge gain by its usage, in order to reduce friction between community members.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
