jdeppe-pivotal commented on a change in pull request #7094:
URL: https://github.com/apache/geode/pull/7094#discussion_r745976715
##########
File path:
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/AuthIntegrationTest.java
##########
@@ -227,4 +244,85 @@ public void
givenSecurityWithWritePermission_getCommandFails() throws Exception
assertThatThrownBy(() -> jedis.get("foo"))
.hasMessageContaining(RedisConstants.ERROR_NOT_AUTHORIZED);
}
+
+ @Test
+ public void givenSecurity_authCommandPasswordIsNotLoggedAtDebugLevel()
throws IOException {
+ File logFile = temporaryFolder.newFile();
+
+ createRadishServerWithLogFileAndSecurityManager(logFile, "fine",
NoOpSecurityManager.class);
+
+ jedis.auth(LOGGED_PASSWORD);
+
+ checkLogFileForPassword(logFile, LOGGED_PASSWORD);
+ }
+
+ @Test
+ public void givenSecurity_authCommandPasswordIsNotLoggedOnException() throws
IOException {
+ File logFile = temporaryFolder.newFile();
+
+ createRadishServerWithLogFileAndSecurityManager(logFile, "info",
+ ThrowsOnAuthorizeSecurityManager.class);
+
+ // The first AUTH command will authenticate the user, and the second will
cause the authorize()
+ // method on the ThrowsOnAuthorizeSecurityManager to be invoked, throwing
an exception
+ jedis.auth(LOGGED_PASSWORD);
+ assertThatThrownBy(() -> jedis.auth(LOGGED_PASSWORD))
+ .hasMessageContaining(SERVER_ERROR_MESSAGE);
+
+ checkLogFileForPassword(logFile, LOGGED_PASSWORD);
+ }
+
+ private void createRadishServerWithLogFileAndSecurityManager(File logFile,
String logLevel,
+ Class<?> securityManager) {
+ System.setProperty("io.netty.eventLoopThreads", "1");
+ port = AvailablePortHelper.getRandomAvailableTCPPort();
Review comment:
I think it would be safer to move this down one line. I think there's a
possibility that the cache creation would end up using the port just deemed
free here.
##########
File path:
geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/AuthIntegrationTest.java
##########
@@ -227,4 +244,85 @@ public void
givenSecurityWithWritePermission_getCommandFails() throws Exception
assertThatThrownBy(() -> jedis.get("foo"))
.hasMessageContaining(RedisConstants.ERROR_NOT_AUTHORIZED);
}
+
+ @Test
+ public void givenSecurity_authCommandPasswordIsNotLoggedAtDebugLevel()
throws IOException {
+ File logFile = temporaryFolder.newFile();
+
+ createRadishServerWithLogFileAndSecurityManager(logFile, "fine",
NoOpSecurityManager.class);
+
+ jedis.auth(LOGGED_PASSWORD);
+
+ checkLogFileForPassword(logFile, LOGGED_PASSWORD);
+ }
+
+ @Test
+ public void givenSecurity_authCommandPasswordIsNotLoggedOnException() throws
IOException {
+ File logFile = temporaryFolder.newFile();
+
+ createRadishServerWithLogFileAndSecurityManager(logFile, "info",
+ ThrowsOnAuthorizeSecurityManager.class);
+
+ // The first AUTH command will authenticate the user, and the second will
cause the authorize()
+ // method on the ThrowsOnAuthorizeSecurityManager to be invoked, throwing
an exception
+ jedis.auth(LOGGED_PASSWORD);
+ assertThatThrownBy(() -> jedis.auth(LOGGED_PASSWORD))
+ .hasMessageContaining(SERVER_ERROR_MESSAGE);
+
+ checkLogFileForPassword(logFile, LOGGED_PASSWORD);
+ }
+
+ private void createRadishServerWithLogFileAndSecurityManager(File logFile,
String logLevel,
+ Class<?> securityManager) {
+ System.setProperty("io.netty.eventLoopThreads", "1");
Review comment:
This isn't necessary.
--
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]
