Spring Security
  1. Spring Security
  2. SEC-1341

CasProcessingFilterEntryPoint more extensible

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.4, 3.0.0
    • Fix Version/s: 3.0.1
    • Component/s: CAS
    • Labels:
      None

      Description

      I need to extend CasAuthenticationEntryPoint in order to add additional parameter to the CAS server.

      Because all the code reside in one method I need to duplicate the code just to change the redirected URL.
      Moreover the encodeServiceUrlWithSessionId property is not accessible therefore I need to do that:
      public class CustomCasProcessingFilterEntryPoint extends CasProcessingFilterEntryPoint {
      private boolean encodeServiceUrlWithSessionId;

      ...

      public void setEncodeServiceUrlWithSessionId(final boolean encodeServiceUrlWithSessionId)

      { super.setEncodeServiceUrlWithSessionId(encodeServiceUrlWithSessionId); this.encodeServiceUrlWithSessionId = encodeServiceUrlWithSessionId; }

      }

      Patch against trunk attached to provide more extensibility.

        Activity

        Hide
        Luke Taylor added a comment -

        I don't think you should need encodeServiceUrlWithSessionId if you are using an up to date CAS server. There are only 3 lines in the commence method so writing a custom end point should be trivial.

        Show
        Luke Taylor added a comment - I don't think you should need encodeServiceUrlWithSessionId if you are using an up to date CAS server. There are only 3 lines in the commence method so writing a custom end point should be trivial.
        Hide
        Sébastien Launay added a comment -

        Yes encodeServiceUrlWithSessionId is not necessary anymore but in order to properly extends the entry point I wanted to keep the existing features.
        I agree also that there are only 3 lines but here also I wanted to keep the same code for an easy maintenance in case of a (unlikely) modification or maybe a refactoring.

        My custom entry point is working it's just that my implementation can be less intrusive .

        Show
        Sébastien Launay added a comment - Yes encodeServiceUrlWithSessionId is not necessary anymore but in order to properly extends the entry point I wanted to keep the existing features. I agree also that there are only 3 lines but here also I wanted to keep the same code for an easy maintenance in case of a (unlikely) modification or maybe a refactoring. My custom entry point is working it's just that my implementation can be less intrusive .
        Hide
        Luke Taylor added a comment -

        Extending a class is really more fragile in terms of the impact of potential modifications to the parent class and that's one of the main reasons we prefer to encourage people to implement interfaces directly. It gives us more freedom to make changes to the existing implementations without worrying about breaking existing code.

        In fact, this raises the possibility that we should maybe deprecate this property for 3.0, since most people are probably already on a CAS version that doesn't require it, certainly in the case of new developments.

        Scott will be able to comment more on this. What do you think Scott? And when is CAS 4.0 due .

        Show
        Luke Taylor added a comment - Extending a class is really more fragile in terms of the impact of potential modifications to the parent class and that's one of the main reasons we prefer to encourage people to implement interfaces directly. It gives us more freedom to make changes to the existing implementations without worrying about breaking existing code. In fact, this raises the possibility that we should maybe deprecate this property for 3.0, since most people are probably already on a CAS version that doesn't require it, certainly in the case of new developments. Scott will be able to comment more on this. What do you think Scott? And when is CAS 4.0 due .
        Hide
        Scott Battaglia added a comment -

        Yes, we should be able to deprecate that property. I'll take a look at it now actually, and see if I can work any magic to make the class more extensible too (no promises)

        Show
        Scott Battaglia added a comment - Yes, we should be able to deprecate that property. I'll take a look at it now actually, and see if I can work any magic to make the class more extensible too (no promises)
        Hide
        Scott Battaglia added a comment -

        all, please check out what I just committed for this issue. Luke, not sure what version we'd target this for so I didn't mark it as resolved.

        Show
        Scott Battaglia added a comment - all, please check out what I just committed for this issue. Luke, not sure what version we'd target this for so I didn't mark it as resolved.

          People

          • Assignee:
            Scott Battaglia
            Reporter:
            Sébastien Launay
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: