Spring Security
  1. Spring Security
  2. SEC-1793

DefaultSpringSecurityContextSource convenience constructor for multiple servers

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: 3.0.5
    • Fix Version/s: 3.1.0
    • Component/s: LDAP
    • Labels:
      None

      Description

      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<String> 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.

        Activity

        Hide
        Luke Taylor added a comment -

        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.

        Show
        Luke Taylor added a comment - 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.
        Hide
        Luke Taylor added a comment -

        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.

        Show
        Luke Taylor added a comment - 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.
        Hide
        Steffen Ryll added a comment -

        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.

        Show
        Steffen Ryll added a comment - 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.
        Hide
        Steffen Ryll added a comment -

        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?

        Show
        Steffen Ryll added a comment - 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?
        Hide
        Luke Taylor added a comment -

        Finally getting round to squeezing this in for 3.1.

        Thanks for the patch!

        Show
        Luke Taylor added a comment - Finally getting round to squeezing this in for 3.1. Thanks for the patch!

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Steffen Ryll
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: