Skip to content

Commit

Permalink
[qxkr2RlZ] apoc.load.ldap cant find config value due to missing (#3727)…
Browse files Browse the repository at this point in the history 
… (#3756)

* [qxkr2RlZ] apoc.load.ldap cant find config value due to missing

* [qxkr2RlZ] added fallback with config key without dot
  • Loading branch information
@vga91
vga91 authored Aug 31, 2023
1 parent 4cb6264 commit 9172ba5
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 15 deletions.
36 changes: 32 additions & 4 deletions full/src/main/java/apoc/load/LoadLdap.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@

import apoc.Extended;
import com.novell.ldap.*;
import org.neo4j.logging.Log;
import org.neo4j.procedure.Context;
import org.neo4j.procedure.Description;
import org.neo4j.procedure.Mode;
import org.neo4j.procedure.Name;
Expand All @@ -35,21 +37,47 @@
@Extended
public class LoadLdap {

@Context
public Log log;

@Procedure(name = "apoc.load.ldap", mode = Mode.READ)
@Description("apoc.load.ldap(\"key\" or {connectionMap},{searchMap}) Load entries from an ldap source (yield entry)")
public Stream<LDAPResult> ldapQuery(@Name("connection") final Object conn, @Name("search") final Map<String,Object> search) {

LDAPManager mgr = new LDAPManager(getConnectionMap(conn));
LDAPManager mgr = new LDAPManager(getConnectionMap(conn, log));

return mgr.executeSearch(search);
}

public static Map<String, Object> getConnectionMap(Object conn) {
public static Map<String, Object> getConnectionMap(Object conn, Log log) {
if (conn instanceof String) {
//String value = "ldap.forumsys.com cn=read-only-admin,dc=example,dc=com password";
String value = apocConfig().getString("apoc.loadldap" + conn.toString() + ".config");
String key = String.format("apoc.loadldap.%s.config", conn);
String value = apocConfig().getString(key);
// format <ldaphost:port> <logindn> <loginpw>
if (value == null) throw new RuntimeException("No apoc.loadldap."+conn+".config ldap access configuration specified");
if (value == null) {
// fallback: if `apoc.loadldap.<LDAP_KEY>.config` is not set
// we check for a config with key `apoc.loadldap<LDAP_KEY>.config`
String keyOld = String.format("apoc.loadldap%s.config", conn);
value = apocConfig().getString(keyOld);

// if the value is set and log == null (that is, not from the test LoadLdapTest.testLoadLDAPConfig),
// we print a log warn, since the correct way should be with a dot before <LDAP_KEY>
if (value != null && log != null) {
String msgWarn = "Not to cause breaking-change, the current config `%s` is valid,\n" +
"but in future releases it will be removed in favor of `%s` (with dot before `%s`),\n" +
"as documented here: https://neo4j.com/labs/apoc/5/database-integration/load-ldap/#_credentials.\n";
String msgWarnFormatted = String.format(msgWarn,
keyOld, key, conn);
log.warn(msgWarnFormatted);
}
}

// if neither `apoc.loadldap.<LDAP_KEY>.config` nor `apoc.loadldap<LDAP_KEY>.config` is set.
// we throw an error
if (value == null) {
throw new RuntimeException("No " + key + " ldap access configuration specified");
}
Map<String, Object> config = new HashMap<>();
String[] sConf = value.split(" ");
config.put("ldapHost", sConf[0]);
Expand Down
102 changes: 91 additions & 11 deletions full/src/test/java/apoc/load/LoadLdapTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
package apoc.load;


import apoc.util.FileUtils;
import apoc.util.TestUtil;
import com.novell.ldap.LDAPEntry;
import com.novell.ldap.LDAPSearchResults;
Expand All @@ -27,16 +28,26 @@
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Test;
import org.neo4j.test.rule.DbmsRule;
import org.neo4j.test.rule.ImpermanentDbmsRule;
import org.junit.rules.TemporaryFolder;
import org.neo4j.configuration.GraphDatabaseSettings;
import org.neo4j.dbms.api.DatabaseManagementService;
import org.neo4j.graphdb.GraphDatabaseService;
import org.neo4j.test.TestDatabaseManagementServiceBuilder;
import org.zapodot.junit.ldap.EmbeddedLdapRule;
import org.zapodot.junit.ldap.EmbeddedLdapRuleBuilder;

import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.List;
import java.util.Map;

import static apoc.ApocConfig.apocConfig;
import static apoc.util.TestUtil.testCall;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;

public class LoadLdapTest {
public static final String BIND_DSN = "uid=admin,cn=users,cn=accounts,dc=demo1,dc=freeipa";
Expand All @@ -46,8 +57,11 @@ public class LoadLdapTest {
public static Map<String, Object> searchParams;

@ClassRule
public static DbmsRule db = new ImpermanentDbmsRule();

public static TemporaryFolder tempFolder = new TemporaryFolder();

private static GraphDatabaseService db;
private static DatabaseManagementService dbms;

@ClassRule
public static EmbeddedLdapRule embeddedLdapRule = EmbeddedLdapRuleBuilder
.newInstance()
Expand All @@ -58,6 +72,8 @@ public class LoadLdapTest {

@BeforeClass
public static void beforeClass() throws Exception {
dbms = new TestDatabaseManagementServiceBuilder(tempFolder.getRoot().toPath()).build();
db = dbms.database(GraphDatabaseSettings.DEFAULT_DATABASE_NAME);
TestUtil.registerProcedure(db, LoadLdap.class);

ldapConnection = embeddedLdapRule.unsharedLdapConnection();
Expand All @@ -75,22 +91,86 @@ public static void beforeClass() throws Exception {
@AfterClass
public static void afterClass() {
ldapConnection.close();
db.shutdown();
dbms.shutdown();
}

@Test
public void testLoadLDAPWithApocConfig() {
String key = "apoc.loadldap.myldap.config";
testWithStringConfigCommon(key);

// the config with dot after loadldap shouldn't print a log warn
String logWarn = "Not to cause breaking-change, the current config `apoc.loadldap.myldap.config` is valid";
assertFalse(getLogFileContent().contains(logWarn));
}

@Test
public void testLoadLDAPWithApocConfigWithoutDotBeforeLdapKey() {
// analogous to `testLoadLDAPWithApocConfig`, but without dot between `loadldap` and `myldap`
// it still works not to cause a breaking change
String key = "apoc.loadldapmyldap.config";
testWithStringConfigCommon(key);

// the config without dot after loadldap should print a log warn
String logWarn = "Not to cause breaking-change, the current config `apoc.loadldapmyldap.config` is valid";
assertTrue(getLogFileContent().contains(logWarn));
}

private static String getLogFileContent() {
try {
File logFile = new File(FileUtils.getLogDirectory(), "debug.log");
return Files.readString(logFile.toPath());
} catch (IOException e) {
throw new RuntimeException(e);
}
}

private void testWithStringConfigCommon(String key) {
// set a config `key=localhost:port dns pwd`
String ldapValue = String.format("%s %s %s",
"localhost:" + ldapConnection.getConnectedPort(),
BIND_DSN,
BIND_PWD);
apocConfig().setProperty(key, ldapValue);

testCall(db, "call apoc.load.ldap($conn, $search)",
Map.of("conn", "myldap", "search", searchParams),
this::testLoadAssertionCommon);

// remove current config to prevent multiple confs in other tests
apocConfig().getConfig().clearProperty(key);
}

@Test
public void testLoadLDAPWithWrongApocConfig() {
apocConfig().setProperty("apoc.loadldap.mykey.config", "host logindn pwd");

String expected = "No apoc.loadldap.wrongKey.config ldap access configuration specified";
try {
testCall(db, "call apoc.load.ldap('wrongKey', {})",
r -> fail("Should fail due to: " + expected));
} catch (RuntimeException e) {
String actual = e.getMessage();
assertTrue("Current err. message is: " + actual, actual.contains(expected));
}
}

@Test
public void testLoadLDAP() {
testCall(db, "call apoc.load.ldap($conn, $search)",
Map.of("conn", connParams, "search", searchParams), r -> {
final Map<String, String> expected = Map.of("uid", "training",
"dn", "uid=training,dc=example,dc=com");
assertEquals(expected, r.get("entry"));
});
Map.of("conn", connParams, "search", searchParams),
this::testLoadAssertionCommon);
}

private void testLoadAssertionCommon(Map<String, Object> r) {
final Map<String, String> expected = Map.of("uid", "training",
"dn", "uid=training,dc=example,dc=com");
assertEquals(expected, r.get("entry"));
}

@Test
public void testLoadLDAPConfig() throws Exception {
LoadLdap.LDAPManager mgr = new LoadLdap.LDAPManager(LoadLdap.getConnectionMap(connParams));
LoadLdap.LDAPManager mgr = new LoadLdap.LDAPManager(LoadLdap.getConnectionMap(connParams, null));

LDAPSearchResults results = mgr.doSearch(searchParams);
LDAPEntry le = results.next();
Expand Down

0 comments on commit 9172ba5

Please sign in to comment.