From a7587637e2b9c096ba81ff21a1204ff1c58a6d3c Mon Sep 17 00:00:00 2001 From: Giuseppe Villani Date: Tue, 21 Feb 2023 09:16:36 +0100 Subject: [PATCH] [wvzcpHQH] Add SSFR check for apoc.spatial procedures (neo4j/apoc#305) (#3451) * [wvzcpHQH] Add SSFR check for apoc.spatial procedures (neo4j/apoc#305) * [wvzcpHQH] Add SSFR check for apoc.spatial procedures * [wvzcpHQH] Fix test in the CI * [wvzcpHQH] added descriptive method and speed up tests * [wvzcpHQH] changed err. message in 4.4 --- .../test/java/apoc/spatial/GeocodeTest.java | 109 +++++++++++++++++- 1 file changed, 108 insertions(+), 1 deletion(-) diff --git a/core/src/test/java/apoc/spatial/GeocodeTest.java b/core/src/test/java/apoc/spatial/GeocodeTest.java index 7db7ed4dcb..105378619c 100755 --- a/core/src/test/java/apoc/spatial/GeocodeTest.java +++ b/core/src/test/java/apoc/spatial/GeocodeTest.java @@ -2,8 +2,10 @@ import apoc.util.JsonUtil; import apoc.util.TestUtil; +import inet.ipaddr.IPAddressString; import org.junit.*; import org.neo4j.graphdb.QueryExecutionException; +import org.neo4j.configuration.GraphDatabaseInternalSettings; import org.neo4j.test.rule.DbmsRule; import org.neo4j.test.rule.ImpermanentDbmsRule; @@ -20,8 +22,16 @@ public class GeocodeTest { + private static final String BLOCKED_ADDRESS = "127.168.0.0"; + private static final String NON_BLOCKED_ADDRESS = "localhost"; + private static final String BLOCKED_ERROR = "access to /" + BLOCKED_ADDRESS + " is blocked via the configuration property unsupported.dbms.cypher_ip_blocklist"; + private static final String JAVA_NET_EXCEPTION = "Caused by: java.net"; + private static final String URL_FORMAT = "%s://%s/geocode/v1/json?q=PLACE&key=KEY"; + private static final String REVERSE_URL_FORMAT = "%s://%s/geocode/v1/json?q=LAT+LNG&key=KEY"; + @Rule - public DbmsRule db = new ImpermanentDbmsRule(); + public DbmsRule db = new ImpermanentDbmsRule() + .withSetting( GraphDatabaseInternalSettings.cypher_ip_blocklist, List.of(new IPAddressString(BLOCKED_ADDRESS)) ); @Before public void initDb() throws Exception { @@ -44,6 +54,103 @@ public void testWrongUrlWithOpenCage() throws Exception { } // -- with apoc config + + @Test + public void testGeocodeWithBlockedAddressWithApocConf() { + final String geocodeBaseConfig = Geocode.PREFIX + ".opencage"; + apocConfig().setProperty(geocodeBaseConfig + ".key", "myKey"); + + Stream.of("https", "http", "ftp").forEach(protocol -> { + final String nonBlockedUrl = String.format(URL_FORMAT, protocol, NON_BLOCKED_ADDRESS); + final String nonBlockedReverseUrl = String.format(REVERSE_URL_FORMAT, protocol, NON_BLOCKED_ADDRESS); + + final String geocodeConfigUrl = geocodeBaseConfig + ".url"; + final String geocodeConfigReverseUrl = geocodeBaseConfig + ".reverse.url"; + + apocConfig().setProperty(geocodeConfigUrl, nonBlockedUrl); + apocConfig().setProperty(geocodeConfigReverseUrl, String.format(REVERSE_URL_FORMAT, protocol, BLOCKED_ADDRESS)); + + assertGeocodeBlockedUrl(true); + + apocConfig().setProperty(geocodeConfigUrl, String.format(URL_FORMAT, protocol, BLOCKED_ADDRESS)); + apocConfig().setProperty(geocodeConfigReverseUrl, nonBlockedReverseUrl); + + assertGeocodeBlockedUrl(false); + + + apocConfig().setProperty(geocodeConfigUrl, nonBlockedUrl); + apocConfig().setProperty(geocodeConfigReverseUrl, nonBlockedReverseUrl); + + assertGeocodeAllowedUrl(false); + assertGeocodeAllowedUrl(true); + }); + } + + @Test + public void testGeocodeWithBlockedAddressWithConfigMap() { + Stream.of("https", "http", "ftp").forEach(protocol -> { + + final String nonBlockedUrl = String.format(URL_FORMAT, protocol, NON_BLOCKED_ADDRESS); + final String nonBlockedReverseUrl = String.format(REVERSE_URL_FORMAT, protocol, NON_BLOCKED_ADDRESS); + + assertGeocodeBlockedUrl(true, + nonBlockedUrl, + String.format(REVERSE_URL_FORMAT, protocol, BLOCKED_ADDRESS) + ); + + assertGeocodeBlockedUrl(false, + String.format(URL_FORMAT, protocol, BLOCKED_ADDRESS), + nonBlockedReverseUrl + ); + + assertGeocodeAllowedUrl(false, + nonBlockedUrl, nonBlockedReverseUrl); + + assertGeocodeAllowedUrl(true, + nonBlockedUrl, nonBlockedReverseUrl); + }); + } + + private void assertGeocodeBlockedUrl(boolean reverseGeocode) { + assertGeocodeBlockedUrl(reverseGeocode, null, null); + } + + private void assertGeocodeBlockedUrl(boolean reverseGeocode, String url, String reverseUrl) { + // check that if either url or reverse address are blocked + // respectively the apoc.spatial.geocode and the apoc.spatial.reverseGeocode procedure fails + assertGeocodeFails(reverseGeocode, BLOCKED_ERROR, url, reverseUrl); + } + + private void assertGeocodeAllowedUrl(boolean reverseGeocode) { + assertGeocodeAllowedUrl(reverseGeocode, null, null); + } + + private void assertGeocodeAllowedUrl(boolean reverseGeocode, String url, String reverseUrl) { + // check that if neither url nor reverse url are blocked + // the procedures continue the execution (in this case by throwing a `401` Exception) + assertGeocodeFails(reverseGeocode, JAVA_NET_EXCEPTION, url, reverseUrl); + } + + private void assertGeocodeFails(boolean reverseGeocode, String expectedMsgError, String url, String reverseUrl) { + // url == null means that it is defined via apoc.conf + Map conf = url == null + ? Collections.emptyMap() + : Map.of("key", "myOwnKey", + "url", url, + "reverseUrl", reverseUrl); + + assertGeocodeFails(reverseGeocode, expectedMsgError, conf); + } + + private void assertGeocodeFails(boolean reverseGeocode, String expectedMsgError, Map conf) { + QueryExecutionException e = assertThrows(QueryExecutionException.class, + () -> testGeocode( "opencage", 100, reverseGeocode, conf ) + ); + + final String actualMsgErr = e.getMessage(); + assertTrue("Actual err. message is " + actualMsgErr, actualMsgErr.contains(expectedMsgError)); + } + @Test public void testGeocodeOSM() throws Exception { testGeocodeWithThrottling("osm", false);