Skip to content

Commit f4e65dd

Browse files
vga91vga91
authored
[NOID] Fixex #4256: Obfuscate JDBC Password in query.log (#4313)
* [NOID] Fixex #4256: Obfuscate JDBC Password in query.log * [NOID] format changes * [NOID] removed duplicated test and format changes
1 parent 7e0d2a5 commit f4e65dd

File tree

6 files changed

+83
-61
lines changed

6 files changed

+83
-61
lines changed

‎core/src/main/java/apoc/load/util/JdbcUtil.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,4 +90,8 @@ public static String getSqlOrKey(String sqlOrKey) {
9090
? sqlOrKey
9191
: Util.getLoadUrlByConfigFile(LOAD_TYPE, sqlOrKey, "sql").orElse("SELECT * FROM " + sqlOrKey);
9292
}
93+
94+
public static String obfuscateJdbcUrl(String query) {
95+
return query.replaceAll("(jdbc:[^:]+://)([^\\s\\\"']+)", "$1*******");
96+
}
9397
}

‎full/src/main/java/apoc/load/Jdbc.java‎

Lines changed: 20 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -131,17 +131,7 @@ private Stream<RowResult> executeQuery(
131131
throw sqle;
132132
}
133133
} catch (Exception e) {
134-
log.error(String.format("Cannot execute SQL statement `%s`.%nError:%n%s", query, e.getMessage()), e);
135-
String errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s";
136-
if (e.getMessage().contains("No suitable driver"))
137-
errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s%n%s";
138-
throw new RuntimeException(
139-
String.format(
140-
errorMessage,
141-
query,
142-
e.getMessage(),
143-
"Please download and copy the JDBC driver into $NEO4J_HOME/plugins,more details at https://neo4j-contrib.github.io/neo4j-apoc-procedures/#_load_jdbc_resources"),
144-
e);
134+
throw logsErrorAndThrowsException(e, query, log);
145135
}
146136
}
147137

@@ -182,20 +172,28 @@ private Stream<RowResult> executeUpdate(
182172
throw sqle;
183173
}
184174
} catch (Exception e) {
185-
log.error(String.format("Cannot execute SQL statement `%s`.%nError:%n%s", query, e.getMessage()), e);
186-
String errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s";
187-
if (e.getMessage().contains("No suitable driver"))
188-
errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s%n%s";
189-
throw new RuntimeException(
190-
String.format(
191-
errorMessage,
192-
query,
193-
e.getMessage(),
194-
"Please download and copy the JDBC driver into $NEO4J_HOME/plugins,more details at https://neo4j-contrib.github.io/neo4j-apoc-procedures/#_load_jdbc_resources"),
195-
e);
175+
throw logsErrorAndThrowsException(e, query, log);
196176
}
197177
}
198178

179+
private static RuntimeException logsErrorAndThrowsException(Exception e, String query, Log log) {
180+
String errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s";
181+
String exceptionMsg = e.getMessage();
182+
if (e.getMessage().contains("No suitable driver")) {
183+
errorMessage = "Cannot execute SQL statement `%s`.%nError:%n%s%n%s";
184+
exceptionMsg = obfuscateJdbcUrl(e.getMessage());
185+
}
186+
Exception ex = new Exception(exceptionMsg);
187+
log.error(String.format("Cannot execute SQL statement `%s`.%nError:%n%s", query, exceptionMsg), ex);
188+
return new RuntimeException(
189+
String.format(
190+
errorMessage,
191+
query,
192+
exceptionMsg,
193+
"Please download and copy the JDBC driver into $NEO4J_HOME/plugins, more details at https://neo4j-contrib.github.io/neo4j-apoc-procedures/#_load_jdbc_resources"),
194+
ex);
195+
}
196+
199197
static void closeIt(Log log, AutoCloseable... closeables) {
200198
for (AutoCloseable c : closeables) {
201199
try {

‎full/src/test/java/apoc/load/JdbcTest.java‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,13 @@
1919
package apoc.load;
2020

2121
import static apoc.ApocConfig.apocConfig;
22+
import static apoc.util.ExtendedTestUtil.assertFails;
23+
import static apoc.util.ExtendedTestUtil.getLogFileContent;
2224
import static apoc.util.MapUtil.map;
2325
import static apoc.util.TestUtil.testCall;
2426
import static apoc.util.TestUtil.testResult;
2527
import static org.junit.Assert.assertEquals;
28+
import static org.junit.Assert.assertTrue;
2629

2730
import apoc.periodic.Periodic;
2831
import apoc.util.TestUtil;
@@ -39,17 +42,23 @@
3942
import java.util.Map;
4043
import org.junit.*;
4144
import org.junit.rules.ExpectedException;
45+
import org.junit.rules.TemporaryFolder;
4246
import org.junit.rules.TestName;
47+
import org.neo4j.configuration.GraphDatabaseSettings;
48+
import org.neo4j.dbms.api.DatabaseManagementService;
49+
import org.neo4j.graphdb.GraphDatabaseService;
4350
import org.neo4j.graphdb.QueryExecutionException;
4451
import org.neo4j.internal.helpers.collection.Iterators;
4552
import org.neo4j.internal.helpers.collection.MapUtil;
46-
import org.neo4j.test.rule.DbmsRule;
47-
import org.neo4j.test.rule.ImpermanentDbmsRule;
53+
import org.neo4j.test.TestDatabaseManagementServiceBuilder;
4854

4955
public class JdbcTest extends AbstractJdbcTest {
5056

5157
@Rule
52-
public DbmsRule db = new ImpermanentDbmsRule();
58+
public TemporaryFolder STORE_DIR = new TemporaryFolder();
59+
60+
private GraphDatabaseService db;
61+
private DatabaseManagementService dbms;
5362

5463
private Connection conn;
5564

@@ -63,6 +72,9 @@ public class JdbcTest extends AbstractJdbcTest {
6372

6473
@Before
6574
public void setUp() throws Exception {
75+
dbms = new TestDatabaseManagementServiceBuilder(STORE_DIR.getRoot().toPath()).build();
76+
db = dbms.database(GraphDatabaseSettings.DEFAULT_DATABASE_NAME);
77+
6678
apocConfig().setProperty("apoc.jdbc.derby.url", "jdbc:derby:derbyDB");
6779
apocConfig().setProperty("apoc.jdbc.test.sql", "SELECT * FROM PERSON");
6880
apocConfig().setProperty("apoc.jdbc.testparams.sql", "SELECT * FROM PERSON WHERE NAME = ?");
@@ -92,7 +104,7 @@ public void tearDown() throws SQLException {
92104
System.clearProperty("derby.connection.requireAuthentication");
93105
System.clearProperty("derby.user.apoc");
94106

95-
db.shutdown();
107+
dbms.shutdown();
96108
}
97109

98110
@Test
@@ -133,6 +145,23 @@ public void testLoadJdbcParams() throws Exception {
133145
(row) -> assertResult(row));
134146
}
135147

148+
@Test
149+
public void testExceptionAndLogWithObfuscatedUrl() {
150+
String url = "jdbc:ajeje://localhost:3306/data_mart?user=root&password=root";
151+
String errorMsgWithObfuscatedUrl = "No suitable driver found for jdbc:ajeje://*******";
152+
153+
// obfuscated exception
154+
assertFails(
155+
db,
156+
"CALL apoc.load.jdbc($url,'SELECT * FROM PERSON WHERE NAME = ?',['John'])",
157+
Map.of("url", url),
158+
errorMsgWithObfuscatedUrl);
159+
160+
// obfuscated log in `debug.log`
161+
String fileContent = getLogFileContent();
162+
assertTrue("Actual log content is: " + fileContent, fileContent.contains(errorMsgWithObfuscatedUrl));
163+
}
164+
136165
@Test
137166
public void testLoadJdbcParamsWithConfigLocalDateTime() throws Exception {
138167
testCall(

‎full/src/test/java/apoc/load/LoadLdapTest.java‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -19,20 +19,17 @@
1919
package apoc.load;
2020

2121
import static apoc.ApocConfig.apocConfig;
22+
import static apoc.util.ExtendedTestUtil.getLogFileContent;
2223
import static apoc.util.TestUtil.testCall;
2324
import static org.junit.Assert.assertEquals;
2425
import static org.junit.Assert.assertFalse;
2526
import static org.junit.Assert.assertTrue;
2627
import static org.junit.Assert.fail;
2728

28-
import apoc.util.FileUtils;
2929
import apoc.util.TestUtil;
3030
import com.novell.ldap.LDAPEntry;
3131
import com.novell.ldap.LDAPSearchResults;
3232
import com.unboundid.ldap.sdk.LDAPConnection;
33-
import java.io.File;
34-
import java.io.IOException;
35-
import java.nio.file.Files;
3633
import java.util.List;
3734
import java.util.Map;
3835
import org.junit.AfterClass;
@@ -117,15 +114,6 @@ public void testLoadLDAPWithApocConfigWithoutDotBeforeLdapKey() {
117114
assertTrue(getLogFileContent().contains(logWarn));
118115
}
119116

120-
private static String getLogFileContent() {
121-
try {
122-
File logFile = new File(FileUtils.getLogDirectory(), "debug.log");
123-
return Files.readString(logFile.toPath());
124-
} catch (IOException e) {
125-
throw new RuntimeException(e);
126-
}
127-
}
128-
129117
private void testWithStringConfigCommon(String key) {
130118
// set a config `key=localhost:port dns pwd`
131119
String ldapValue =

‎full/src/test/java/apoc/periodic/PeriodicExtendedTest.java‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import apoc.nlp.gcp.GCPProcedures;
3333
import apoc.nodes.NodesExtended;
3434
import apoc.util.TestUtil;
35-
import java.sql.SQLException;
3635
import java.util.Collections;
3736
import java.util.Map;
3837
import java.util.concurrent.TimeUnit;
@@ -222,27 +221,6 @@ public void testIterateErrors() {
222221
});
223222
}
224223

225-
@Test
226-
public void testIterateJDBC() {
227-
TestUtil.ignoreException(
228-
() -> {
229-
testResult(
230-
db,
231-
"CALL apoc.periodic.iterate('call apoc.load.jdbc(\"jdbc:mysql://localhost:3306/northwind?user=root\",\"customers\")', 'create (c:Customer) SET c += $row', {batchSize:10,parallel:true})",
232-
result -> {
233-
Map<String, Object> row = Iterators.single(result);
234-
assertEquals(3L, row.get("batches"));
235-
assertEquals(29L, row.get("total"));
236-
});
237-
238-
testCall(
239-
db,
240-
"MATCH (p:Customer) return count(p) as count",
241-
row -> assertEquals(29L, row.get("count")));
242-
},
243-
SQLException.class);
244-
}
245-
246224
@Test
247225
public void testRock_n_roll_while() {
248226
// setup

‎full/src/test/java/apoc/util/ExtendedTestUtil.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
package apoc.util;
22

3+
import static apoc.util.TestUtil.testCall;
34
import static apoc.util.TestUtil.testCallAssertions;
5+
import static org.junit.Assert.assertTrue;
6+
import static org.junit.Assert.fail;
47
import static org.neo4j.test.assertion.Assert.assertEventually;
58

9+
import java.io.File;
10+
import java.io.IOException;
11+
import java.nio.file.Files;
612
import java.util.Collections;
713
import java.util.Map;
814
import java.util.concurrent.TimeUnit;
@@ -67,4 +73,23 @@ public static void testResultEventually(
6773
timeout,
6874
TimeUnit.SECONDS);
6975
}
76+
77+
public static void assertFails(
78+
GraphDatabaseService db, String query, Map<String, Object> params, String expectedErrMsg) {
79+
try {
80+
testCall(db, query, params, r -> fail("Should fail due to " + expectedErrMsg));
81+
} catch (Exception e) {
82+
String actualErrMsg = e.getMessage();
83+
assertTrue("Actual err. message is: " + actualErrMsg, actualErrMsg.contains(expectedErrMsg));
84+
}
85+
}
86+
87+
public static String getLogFileContent() {
88+
try {
89+
File logFile = new File(FileUtils.getLogDirectory(), "debug.log");
90+
return Files.readString(logFile.toPath());
91+
} catch (IOException e) {
92+
throw new RuntimeException(e);
93+
}
94+
}
7095
}

0 commit comments

Comments
 (0)