test-support-unboundid EmbeddedLdapServer has hard-coded credentials

[Interessierter]

Hello,

this has cost me some hours to debug :(
See here: https://github.com/spring-projects/spring-ldap/blob/master/test-support-unboundid/src/main/java/org/springframework/ldap/test/unboundid/EmbeddedLdapServer.java#L43

The unboundid part of the spring-ldap-test has hard-coded credentials, making it impossible to use the org.springframework.ldap.test.unboundid.LdapTestUtils or other classes utlizing this (like the very useful org.springframework.ldap.test.unboundid.EmbeddedLdapServerFactoryBean) with different credentials, even if the API and the documentation clearly say so. The existing test does not catch this as it tests the code with exactly the hardcoded credentials.

IMHO the API of LdapTestUtils should be changed to require the user/password always.

[jzheaux]

Thanks for the report, @Interessierter. Can you clarify why you need to supply a different set of credentials in a test?

[Interessierter]

Hi Josh, thanks for the reply! I agree that specifying a different set of credentials isnt usually neccessary (although it could be in more complex test setups when you're re-using existing configuration) but what tripped me was that the API of org.springframework.ldap.test.unboundid.TestContextSourceFactoryBean (which is really useful!) which is featured on the documentation for using an embedded server for testing (https://docs.spring.io/spring-ldap/docs/2.3.3.RELEASE/reference/#spring-ldap-testing-unboundid ) really makes it appear you could use a different set of credentials and if you do it isn't working and you spend some time wondering why and only looking in the sourcecode of spring-ldap finally explains it.

So in IMHO either the principaland password fields should be removed from org.springframework.ldap.test.unboundid.TestContextSourceFactoryBean (and the documentation be updated) or this class and the org.springframework.ldap.test.unboundid.LdapTestUtils it is using should be updated to support a different set of credentials, the current API is very misleading to outside developers.

[rwinch]

The configuration also specifies the ldif that has the custom credentials used to connect. Does that help to clarify?

[Interessierter]

@rwinch was that addressed at me? Then no, I dont understand what you're trying to say. As you can see I have opened a pull request (with tests) which fixes the issue.

Are you trying to say if you have the account you want to use for binding is included in the LDIF it should work? It doesnt, I have tested it (and btw. the hardcoded account uid=admin,ou=system in the current impl isnt included there as well)

[rwinch]

Ok I see the reason this does not work. The TestContextSourceFactory uses the ContextSource to perform the ldif import, but the credentials for the ContextSource are not present within the unboundid InMemoryDirectoryServer yet. Likely the import should be done using loadLdif(InMemoryDirectoryServer directoryServer, Resource ldifFile) which does not require access to a ContextSource, but there is no reference to the InMemoryDirectoryServer due to starting the server with the utility class.

I'm open to making the change. I'll close this in favor of continuing the discussion on the PR gh-578