From ba41be8f8555d71de5d3b7afb7d3a45c55551cb0 Mon Sep 17 00:00:00 2001 From: Giuseppe Villani Date: Wed, 15 Feb 2023 14:56:14 +0100 Subject: [PATCH] [wvzcpHQH] Add SSFR check for apoc.spatial procedures (#305) * [wvzcpHQH] Add SSFR check for apoc.spatial procedures * [wvzcpHQH] Fix test in the CI * [wvzcpHQH] added descriptive method and speed up tests --- .../test/java/apoc/spatial/GeocodeTest.java | 111 +++++++++++++++++- 1 file changed, 109 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/apoc/spatial/GeocodeTest.java b/core/src/test/java/apoc/spatial/GeocodeTest.java index 123bcffc1..0cea9b5e0 100755 --- a/core/src/test/java/apoc/spatial/GeocodeTest.java +++ b/core/src/test/java/apoc/spatial/GeocodeTest.java @@ -2,7 +2,10 @@ import apoc.util.JsonUtil; import apoc.util.TestUtil; +import inet.ipaddr.IPAddressString; import org.junit.*; +import org.neo4j.configuration.GraphDatabaseInternalSettings; +import org.neo4j.graphdb.QueryExecutionException; import org.neo4j.test.rule.DbmsRule; import org.neo4j.test.rule.ImpermanentDbmsRule; @@ -10,6 +13,7 @@ import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.stream.Stream; import static apoc.ApocConfig.apocConfig; import static apoc.util.MapUtil.map; @@ -19,8 +23,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 internal.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"; + @ClassRule - public static DbmsRule db = new ImpermanentDbmsRule(); + public static DbmsRule db = new ImpermanentDbmsRule() + .withSetting( GraphDatabaseInternalSettings.cypher_ip_blocklist, List.of(new IPAddressString(BLOCKED_ADDRESS)) ); @BeforeClass public static void initDb() { @@ -39,8 +51,103 @@ public void testWrongUrlButViaOtherProvider() throws Exception { testGeocodeWithThrottling("osm", false, map("url", "https://api.opencagedata.com/geocode/v1/json?q=PLACE&key=KEY111")); } + + @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); + }); + } - // -- with apoc config + @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);