diff --git a/src/main/java/io/supertokens/storage/mysql/output/CustomLayout.java b/src/main/java/io/supertokens/storage/mysql/output/CustomLayout.java index b2c0c9c..74dca68 100644 --- a/src/main/java/io/supertokens/storage/mysql/output/CustomLayout.java +++ b/src/main/java/io/supertokens/storage/mysql/output/CustomLayout.java @@ -20,13 +20,11 @@ import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.CoreConstants; import ch.qos.logback.core.LayoutBase; -import io.supertokens.storage.mysql.Start; +import io.supertokens.storage.mysql.utils.Utils; import java.text.DateFormat; import java.text.SimpleDateFormat; import java.util.Date; -import java.util.regex.Matcher; -import java.util.regex.Pattern; class CustomLayout extends LayoutBase { @@ -37,21 +35,6 @@ class CustomLayout extends LayoutBase { this.processID = processID; } - public static String maskDBPassword(String log) { - String regex = "(\\|db_pass\\|)(.*?)(\\|db_pass\\|)"; - - Matcher matcher = Pattern.compile(regex).matcher(log); - StringBuffer maskedLog = new StringBuffer(); - - while (matcher.find()) { - String maskedPassword = "*".repeat(8); - matcher.appendReplacement(maskedLog, "|" + maskedPassword + "|"); - } - - matcher.appendTail(maskedLog); - return maskedLog.toString(); - } - @Override public String doLayout(ILoggingEvent event) { StringBuilder sbuf = new StringBuilder(); @@ -75,7 +58,7 @@ public String doLayout(ILoggingEvent event) { sbuf.append(event.getCallerData()[1]); sbuf.append(" | "); - sbuf.append(maskDBPassword(event.getFormattedMessage())); + sbuf.append(Utils.maskDBPassword(event.getFormattedMessage())); sbuf.append(CoreConstants.LINE_SEPARATOR); sbuf.append(CoreConstants.LINE_SEPARATOR); diff --git a/src/main/java/io/supertokens/storage/mysql/output/Logging.java b/src/main/java/io/supertokens/storage/mysql/output/Logging.java index 531d76e..084f1a6 100644 --- a/src/main/java/io/supertokens/storage/mysql/output/Logging.java +++ b/src/main/java/io/supertokens/storage/mysql/output/Logging.java @@ -37,10 +37,10 @@ public class Logging extends ResourceDistributor.SingletonResource { private Logging(Start start, String infoLogPath, String errorLogPath) { this.infoLogger = infoLogPath.equals("null") - ? createLoggerForConsole(start, "io.supertokens.storage.mysql.Info") + ? createLoggerForConsole(start, "io.supertokens.storage.mysql.Info", LOG_LEVEL.INFO) : createLoggerForFile(start, infoLogPath, "io.supertokens.storage.mysql.Info"); this.errorLogger = errorLogPath.equals("null") - ? createLoggerForConsole(start, "io.supertokens.storage.mysql.Error") + ? createLoggerForConsole(start, "io.supertokens.storage.mysql.Error", LOG_LEVEL.ERROR) : createLoggerForFile(start, errorLogPath, "io.supertokens.storage.mysql.Error"); } @@ -154,12 +154,12 @@ public static void error(Start start, String message, boolean toConsoleAsWell, E private static void systemOut(String msg) { if (!Start.silent) { - System.out.println(msg); + System.out.println(Utils.maskDBPassword(msg)); } } private static void systemErr(String err) { - System.err.println(err); + System.err.println(Utils.maskDBPassword(err)); } public static void stopLogging(Start start) { @@ -198,7 +198,7 @@ private Logger createLoggerForFile(Start start, String file, String name) { return logger; } - private Logger createLoggerForConsole(Start start, String name) { + private Logger createLoggerForConsole(Start start, String name, LOG_LEVEL logLevel) { Logger logger = (Logger) LoggerFactory.getLogger(name); // We don't need to add appender if it is already added @@ -211,6 +211,7 @@ private Logger createLoggerForConsole(Start start, String name) { ple.setContext(lc); ple.start(); ConsoleAppender logConsoleAppender = new ConsoleAppender<>(); + logConsoleAppender.setTarget(logLevel == LOG_LEVEL.ERROR ? "System.err" : "System.out"); logConsoleAppender.setEncoder(ple); logConsoleAppender.setContext(lc); logConsoleAppender.start(); diff --git a/src/main/java/io/supertokens/storage/mysql/utils/Utils.java b/src/main/java/io/supertokens/storage/mysql/utils/Utils.java index 9dcb1f9..e68d6dd 100644 --- a/src/main/java/io/supertokens/storage/mysql/utils/Utils.java +++ b/src/main/java/io/supertokens/storage/mysql/utils/Utils.java @@ -19,6 +19,8 @@ import java.io.ByteArrayOutputStream; import java.io.PrintStream; +import java.util.regex.Matcher; +import java.util.regex.Pattern; public class Utils { public static String exceptionStacktraceToString(Exception e) { @@ -39,4 +41,19 @@ public static String generateCommaSeperatedQuestionMarks(int size) { } return builder.toString(); } + + public static String maskDBPassword(String log) { + String regex = "(\\|db_pass\\|)(.*?)(\\|db_pass\\|)"; + + Matcher matcher = Pattern.compile(regex).matcher(log); + StringBuffer maskedLog = new StringBuffer(); + + while (matcher.find()) { + String maskedPassword = "*".repeat(8); + matcher.appendReplacement(maskedLog, "|" + maskedPassword + "|"); + } + + matcher.appendTail(maskedLog); + return maskedLog.toString(); + } } diff --git a/src/test/java/io/supertokens/storage/mysql/test/LoggingTest.java b/src/test/java/io/supertokens/storage/mysql/test/LoggingTest.java index c15840f..52ab2c0 100644 --- a/src/test/java/io/supertokens/storage/mysql/test/LoggingTest.java +++ b/src/test/java/io/supertokens/storage/mysql/test/LoggingTest.java @@ -30,6 +30,7 @@ import io.supertokens.multitenancy.Multitenancy; import io.supertokens.pluginInterface.multitenancy.*; import io.supertokens.storage.mysql.Start; +import io.supertokens.storage.mysql.config.MySQLConfig; import io.supertokens.storage.mysql.output.Logging; import io.supertokens.storageLayer.StorageLayer; import org.apache.tomcat.util.http.fileupload.FileUtils; @@ -312,7 +313,6 @@ public void confirmHikariLoggerClosedOnlyWhenProcessEnds() throws Exception { @Test public void testDBPasswordMaskingOnDBConnectionFailUsingConnectionUri() throws Exception { - StorageLayer.close(); String[] args = { "../" }; String dbUser = "db_user"; @@ -323,177 +323,266 @@ public void testDBPasswordMaskingOnDBConnectionFailUsingConnectionUri() throws E Utils.setValueInConfig("mysql_connection_uri", dbConnectionUri); Utils.commentConfigValue("mysql_user"); Utils.commentConfigValue("mysql_password"); + Utils.setValueInConfig("error_log_path", "null"); - TestingProcessManager.TestingProcess process = TestingProcessManager.start(args); - - process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.INIT_FAILURE); + ByteArrayOutputStream errorOutput = new ByteArrayOutputStream(); + System.setErr(new PrintStream(errorOutput)); - File errorLog = new File(Config.getConfig(process.getProcess()).getErrorLogPath(process.getProcess())); + TestingProcessManager.TestingProcess process = TestingProcessManager.start(args, false); + try { + process.startProcess(); + process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.INIT_FAILURE); - boolean dbPasswordMaskedInErrorLog = false; + assertTrue(fileContainsString(errorOutput, dbUser)); + assertTrue(fileContainsString(errorOutput, dbName)); + assertTrue(fileContainsString(errorOutput, "********")); + assertFalse(fileContainsString(errorOutput, dbPassword)); - try (Scanner errorScanner = new Scanner(errorLog, StandardCharsets.UTF_8)) { - while (errorScanner.hasNextLine()) { - String line = errorScanner.nextLine(); - if(line.contains(dbName) && line.contains(dbUser) && !line.contains(dbPassword) && line.contains("********")){ - dbPasswordMaskedInErrorLog = true; - break; - } - } + } finally { + process.kill(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + System.setErr(new PrintStream(new FileOutputStream(FileDescriptor.err))); } - - assertTrue(dbPasswordMaskedInErrorLog); - process.kill(); - - assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); } @Test public void testDBPasswordMaskingOnDBConnectionFailUsingCredentials() throws Exception { - StorageLayer.close(); String[] args = { "../" }; String dbUser = "db_user"; String dbPassword = "db_password"; String dbName = "db_does_not_exist"; + Utils.commentConfigValue("mysql_connection_uri"); Utils.setValueInConfig("mysql_user", dbUser); Utils.setValueInConfig("mysql_password", dbPassword); Utils.setValueInConfig("mysql_database_name", dbName); + Utils.setValueInConfig("error_log_path", "null"); - TestingProcessManager.TestingProcess process = TestingProcessManager.start(args); - - process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.INIT_FAILURE); + ByteArrayOutputStream errorOutput = new ByteArrayOutputStream(); + System.setErr(new PrintStream(errorOutput)); - File errorLog = new File(Config.getConfig(process.getProcess()).getErrorLogPath(process.getProcess())); + TestingProcessManager.TestingProcess process = TestingProcessManager.start(args, false); + try { + process.startProcess(); + process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.INIT_FAILURE); - boolean dbPasswordMaskedInErrorLog = false; + assertTrue(fileContainsString(errorOutput, dbUser)); + assertTrue(fileContainsString(errorOutput, dbName)); + assertTrue(fileContainsString(errorOutput, "********")); + assertFalse(fileContainsString(errorOutput, dbPassword)); - try (Scanner errorScanner = new Scanner(errorLog, StandardCharsets.UTF_8)) { - while (errorScanner.hasNextLine()) { - String line = errorScanner.nextLine(); - if(line.contains(dbName) && line.contains(dbUser) && !line.contains(dbPassword) && line.contains("********")){ - dbPasswordMaskedInErrorLog = true; - break; - } - } + } finally { + process.kill(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + System.setErr(new PrintStream(new FileOutputStream(FileDescriptor.err))); } - - assertTrue(dbPasswordMaskedInErrorLog); - process.kill(); - - assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); } @Test - public void testDBPasswordMaskingOnDBConnectionFailWhenCreatingTenant() throws Exception { - StorageLayer.close(); + public void testDBPasswordMasking() throws Exception { String[] args = { "../" }; - String dbUser = "db_user"; - String dbPassword = "db_password"; - String dbName = "db_does_not_exist"; - String dbConnectionUri = "mysql://" + dbUser + ":" + dbPassword + "@localhost:3306/" + dbName; + ByteArrayOutputStream stdOutput = new ByteArrayOutputStream(); + ByteArrayOutputStream errorOutput = new ByteArrayOutputStream(); + + Utils.setValueInConfig("info_log_path", "null"); + Utils.setValueInConfig("error_log_path", "null"); + + System.setOut(new PrintStream(stdOutput)); + System.setErr(new PrintStream(errorOutput)); TestingProcessManager.TestingProcess process = TestingProcessManager.start(args, false); - Main main = process.getProcess(); - FeatureFlagTestContent.getInstance(main) - .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[]{ - EE_FEATURES.ACCOUNT_LINKING, EE_FEATURES.MULTI_TENANCY}); + try { + process.startProcess(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); - process.startProcess(); + Logging.info((Start) StorageLayer.getStorage(process.getProcess()), "INFO LOG: |db_pass|password|db_pass|", + false); + Logging.error((Start) StorageLayer.getStorage(process.getProcess()), + "ERROR LOG: |db_pass|password|db_pass|", false); - assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + assertTrue(fileContainsString(stdOutput, "INFO LOG: |********|")); + assertTrue(fileContainsString(errorOutput, "ERROR LOG: |********|")); - JsonObject config = new JsonObject(); - TenantIdentifier tenantIdentifier = new TenantIdentifier(null, "a1", null); + } finally { + process.kill(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + System.setOut(new PrintStream(new FileOutputStream(FileDescriptor.out))); + System.setErr(new PrintStream(new FileOutputStream(FileDescriptor.err))); + } + } + + @Test + public void testDBPasswordIsNotLoggedWhenProcessStartsEnds() throws Exception { + String[] args = { "../" }; + + Utils.setValueInConfig("error_log_path", "null"); + Utils.setValueInConfig("info_log_path", "null"); - config.addProperty("mysql_connection_uri", dbConnectionUri); - StorageLayer.getStorage(new TenantIdentifier(null, null, null), main) - .modifyConfigToAddANewUserPoolForTesting(config, 1); + ByteArrayOutputStream stdOutput = new ByteArrayOutputStream(); + ByteArrayOutputStream errorOutput = new ByteArrayOutputStream(); + + System.setOut(new PrintStream(stdOutput)); + System.setErr(new PrintStream(errorOutput)); try { - Multitenancy.addNewOrUpdateAppOrTenant( - main, - new TenantIdentifier(null, null, null), - new TenantConfig( - tenantIdentifier, - new EmailPasswordConfig(true), - new ThirdPartyConfig(true, null), - new PasswordlessConfig(true), - config - ) - ); - - } catch (Exception e) { - - } + // Case 1: DB Password shouldn't be logged after starting/stopping the process with correct credentials + { + TestingProcessManager.TestingProcess process = TestingProcessManager.start(args); - File errorLog = new File(Config.getConfig(main).getErrorLogPath(main)); + process.startProcess(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); - boolean dbPasswordMaskedInErrorLog = false; + Start start = (Start) StorageLayer.getStorage(process.getProcess()); + MySQLConfig userConfig = io.supertokens.storage.mysql.config.Config.getConfig(start); + String dbPasswordFromConfig = userConfig.getPassword(); - try (Scanner errorScanner = new Scanner(errorLog, StandardCharsets.UTF_8)) { - while (errorScanner.hasNextLine()) { - String line = errorScanner.nextLine(); - System.out.println("line: " + line); - if(line.contains(dbName) && line.contains(dbUser) && !line.contains(dbPassword) && line.contains("********")){ - dbPasswordMaskedInErrorLog = true; - break; - } + process.kill(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + + assertFalse(fileContainsString(stdOutput, dbPasswordFromConfig)); + assertFalse(fileContainsString(errorOutput, dbPasswordFromConfig)); } - } - assertTrue(dbPasswordMaskedInErrorLog); - process.kill(); + // Case 2: DB Password shouldn't be logged after starting/stopping the process with incorrect credentials + { + String dbUser = "db_user"; + String dbPassword = "db_password"; + String dbName = "db_does_not_exist"; + String dbConnectionUri = "mysql://" + dbUser + ":" + dbPassword + "@localhost:3306/" + dbName; - assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + Utils.setValueInConfig("mysql_connection_uri", dbConnectionUri); + + TestingProcessManager.TestingProcess process = TestingProcessManager.start(args); + + process.startProcess(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.INIT_FAILURE)); + + Start start = (Start) StorageLayer.getStorage(process.getProcess()); + MySQLConfig userConfig = io.supertokens.storage.mysql.config.Config.getConfig(start); + String dbPasswordFromConfig = userConfig.getPassword(); + + process.kill(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + + assertFalse(fileContainsString(stdOutput, dbPasswordFromConfig)); + assertFalse(fileContainsString(errorOutput, dbPasswordFromConfig)); + } + + } finally { + System.setOut(new PrintStream(new FileOutputStream(FileDescriptor.out))); + System.setErr(new PrintStream(new FileOutputStream(FileDescriptor.err))); + } } - + @Test - public void testDBPasswordMasking() throws Exception { - StorageLayer.close(); + public void testDBPasswordIsNotLoggedWhenTenantIsCreated() throws Exception { String[] args = { "../" }; - TestingProcessManager.TestingProcess process = TestingProcessManager.start(args); - - assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); - File infoLog = new File(Config.getConfig(process.getProcess()).getInfoLogPath(process.getProcess())); - File errorLog = new File(Config.getConfig(process.getProcess()).getErrorLogPath(process.getProcess())); + Utils.setValueInConfig("error_log_path", "null"); + Utils.setValueInConfig("info_log_path", "null"); - Logging.error((Start) StorageLayer.getStorage(process.getProcess()), "ERROR LOG: |db_pass|password|db_pass|", false); - Logging.info((Start) StorageLayer.getStorage(process.getProcess()), "INFO LOG: |db_pass|password|db_pass|", true); + ByteArrayOutputStream stdOutput = new ByteArrayOutputStream(); + ByteArrayOutputStream errorOutput = new ByteArrayOutputStream(); - boolean dbPasswordMaskedInInfoLog = false; - boolean dbPasswordMaskedInErrorLog = false; + System.setOut(new PrintStream(stdOutput)); + System.setErr(new PrintStream(errorOutput)); - try (Scanner scanner = new Scanner(infoLog, StandardCharsets.UTF_8)) { - while (scanner.hasNextLine()) { - String line = scanner.nextLine(); - if(line.contains("INFO LOG") && line.contains("INFO LOG: |********|")) { - dbPasswordMaskedInInfoLog = true; - break; - } + try { + // Case 1: DB Password shouldn't be logged when tenant is created with valid credentials + { + TestingProcessManager.TestingProcess process = TestingProcessManager.start(args); + + process.startProcess(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + + Start start = (Start) StorageLayer.getStorage(process.getProcess()); + MySQLConfig userConfig = io.supertokens.storage.mysql.config.Config.getConfig(start);; + String dbPasswordFromConfig = userConfig.getPassword(); + + Main main = process.getProcess(); + + FeatureFlagTestContent.getInstance(main) + .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[] { + EE_FEATURES.ACCOUNT_LINKING, EE_FEATURES.MULTI_TENANCY }); + + JsonObject config = new JsonObject(); + TenantIdentifier tenantIdentifier = new TenantIdentifier(null, "a1", null); + StorageLayer.getBaseStorage(process.getProcess()).modifyConfigToAddANewUserPoolForTesting(config, 1); + Multitenancy.addNewOrUpdateAppOrTenant( + main, + new TenantIdentifier(null, null, null), + new TenantConfig( + tenantIdentifier, + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + config + )); + + process.kill(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + + assertFalse(fileContainsString(stdOutput, dbPasswordFromConfig)); + assertFalse(fileContainsString(errorOutput, dbPasswordFromConfig)); } - } - try (Scanner errorScanner = new Scanner(errorLog, StandardCharsets.UTF_8)) { - while (errorScanner.hasNextLine()) { - String line = errorScanner.nextLine(); - if(line.contains("ERROR LOG") && line.contains("ERROR LOG: |********|")) { - dbPasswordMaskedInErrorLog = true; - break; + // Case 2: DB Password shouldn't be logged when tenant is created with invalid credentials + { + String dbUser = "db_user"; + String dbPassword = "db_password"; + String dbName = "db_does_not_exist"; + String dbConnectionUri = "mysql://" + dbUser + ":" + dbPassword + "@localhost:3306/" + dbName; + + TestingProcessManager.TestingProcess process = TestingProcessManager.start(args); + + process.startProcess(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + + Start start = (Start) StorageLayer.getStorage(process.getProcess()); + MySQLConfig userConfig = io.supertokens.storage.mysql.config.Config.getConfig(start); + String dbPasswordFromConfig = userConfig.getPassword(); + + Main main = process.getProcess(); + + FeatureFlagTestContent.getInstance(main) + .setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES, new EE_FEATURES[] { + EE_FEATURES.ACCOUNT_LINKING, EE_FEATURES.MULTI_TENANCY }); + + TenantIdentifier tenantIdentifier = new TenantIdentifier(null, "a1", null); + JsonObject config = new JsonObject(); + config.addProperty("mysql_connection_uri", dbConnectionUri); + StorageLayer.getBaseStorage(process.getProcess()).modifyConfigToAddANewUserPoolForTesting(config, 1); + try { + Multitenancy.addNewOrUpdateAppOrTenant( + main, + new TenantIdentifier(null, null, null), + new TenantConfig( + tenantIdentifier, + new EmailPasswordConfig(true), + new ThirdPartyConfig(true, null), + new PasswordlessConfig(true), + new JsonObject())); + + } catch (Exception e) { + } - } - } - assertTrue(dbPasswordMaskedInInfoLog && dbPasswordMaskedInErrorLog); - process.kill(); + process.kill(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); - assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + assertFalse(fileContainsString(stdOutput, dbPasswordFromConfig)); + assertFalse(fileContainsString(errorOutput, dbPasswordFromConfig)); + } + + } finally { + System.setOut(new PrintStream(new FileOutputStream(FileDescriptor.out))); + System.setErr(new PrintStream(new FileOutputStream(FileDescriptor.err))); + } } - + private static int countAppenders(ch.qos.logback.classic.Logger logger) { int count = 0; Iterator> appenderIter = logger.iteratorForAppenders();