Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

SEC-1793: DefaultSpringSecurityContextSource convenience constructor for multiple servers #2018

spring-issuemaster opened this Issue Aug 2, 2011 · 5 comments


None yet
1 participant

Steffen Ryll (Migrated from SEC-1793) said:

The constructor of DefaultSpringSecurityContextSource requires you to pass a specially formatted string if you want it to use more than one LDAP servers (fail-over scenario). This string format is hard to ensure/ construct from the XML configuration. Instead, it would be easier to pass a List of LDAP URLs and a base DN string.

I've already creates a patch and posted a Gitorious Merge request here: http://git.springsource.org/spring-security/spring-security/merge_requests/3
The summary is this:

I've added a convenience constructor to DefaultSpringSecurityContextSource which takes a List of server URLs and the base DN to construct a provider provider URL that the underlying Spring LDAP understands.

This saves users the hassle of finding out how the provider URL should look like in server fail-over setups. The new constructor takes care of putting everything together in the right order.
Test cases have been included.

I'd be very happy to see this included in trunk.

Luke Taylor said:

Thanks for the patch. However, it seems to contain quite a lot of unnecessary reformatting of classes and so on, making it difficult to review. Please could you resubmit it, preferably based against the current master branch and with the minimum diff necessary. Note that unlike SFW, Spring Security historically uses spaces rather than tabs for formatting.

Luke Taylor said:

Note that you'll also need to sign up to the committer agreement at https://support.springsource.com/spring_committer_signup if you haven't already done so, in order to have patches merged. It's just a simple online form.

Steffen Ryll said:

I've push --force'd an updated version of my changes, so that it no longer includes commits from the master which I had merged... git newbie. I hope the history threads now makes sense again.
I've also corrected the indentation to use spaces.

I will sign the committer agreement shortly and hope that you'll get notified as soon as I submit the form.

Steffen Ryll said:

I've submitted the Committer Agreement some time ago - did you get notified? Is there anything else I need to get sorted out before this Merge Request can be merged?

Luke Taylor said:

Finally getting round to squeezing this in for 3.1.

Thanks for the patch!

@spring-issuemaster spring-issuemaster added this to the 3.1.0 milestone Feb 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment