phpCAS Client
  1. phpCAS Client
  2. PHPCAS-69

Refactor the ST / PT detection decision making in the client

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.0.0, 1.0.1, 1.1.0, 1.1.1
    • Fix Version/s: 1.3.0
    • Labels:
      None

      Description

      The whole ST / PT semantic should be reworked. There is a lot of confusion where a PT or ST is stored in client or proxy mode on the different protocols.
      The ST storage is used for the CAS 1.0 protocol and PT storage is used for the CAS 2.0 protocol.
      The proxy() allows for phpCAS to proxy other apps but has no effect on the being proxied which would be controlled by the validation calls.

      The decision should be based on:

      CAS_VERSION_1_0

      • client only (will only accept STs validate against (validate)
        CAS_VERSION_2_0
      • client() (will only accept STs and validate them against serviceValdiate)
      • proxy() (will only accept ST and validate them against serviceValdiate but is able to proxy other applications)
      • some boolean allowToBeProxied (switches all serviceValidate against proxyValidate)
        SAML_VERSION_1_1
      • client only ...

        Activity

        Hide
        Joachim Fritschi added a comment - - edited

        Nice

        @Adam: Do you think we can move this stuff into /trunk? I'm quite happy with the current status and would like to start stabilizing /trunk and push for a new release in a few weeks.

        @all: Any opinions whether we can still call this one 1.2.3 after tearing through the guts of the client like that and changing at least one client visible behavior. (default allow -> default deny) On the other hand for anyone affected by this change it's a oneliner and with our testframework i am pretty certain we broke nothing.

        This is why i'm leaning a bit more torwards naming it 1.2.3.

        Show
        Joachim Fritschi added a comment - - edited Nice @Adam: Do you think we can move this stuff into /trunk? I'm quite happy with the current status and would like to start stabilizing /trunk and push for a new release in a few weeks. @all: Any opinions whether we can still call this one 1.2.3 after tearing through the guts of the client like that and changing at least one client visible behavior. (default allow -> default deny) On the other hand for anyone affected by this change it's a oneliner and with our testframework i am pretty certain we broke nothing. This is why i'm leaning a bit more torwards naming it 1.2.3.
        Hide
        Adam Franco added a comment -

        I'm happy merging this into /trunk. I can't imagine changing direction at this point and we can always make a small bugfix if needed.

        As for version numbering, I'm inclined to bump to 1.3.0 due to the client-visible API changes that can break existing configurations. My personal preference is that tertiary increments are always non-breaking drop-in replacements for the previous version.

        I agree with you on the gutting though, without the API changes even somewhat large internal refactoring doesn't need to be more than a tertiary bump.

        Show
        Adam Franco added a comment - I'm happy merging this into /trunk. I can't imagine changing direction at this point and we can always make a small bugfix if needed. As for version numbering, I'm inclined to bump to 1.3.0 due to the client-visible API changes that can break existing configurations. My personal preference is that tertiary increments are always non-breaking drop-in replacements for the previous version. I agree with you on the gutting though, without the API changes even somewhat large internal refactoring doesn't need to be more than a tertiary bump.
        Hide
        Joachim Fritschi added a comment -

        I have merged everything into /trunk. Version will be bumped to 1.3.0 and i have already changed the roadmap. I however think that we have to stress in the release notes that the changes a not as big as with the 1.1->1.2 change and for most people it's a drop in replacement unless the are using proxied apps. I just don't want this to be a reason that pevents people from adopting this release quickly.

        @All: Thanks for your input/contribution

        Show
        Joachim Fritschi added a comment - I have merged everything into /trunk. Version will be bumped to 1.3.0 and i have already changed the roadmap. I however think that we have to stress in the release notes that the changes a not as big as with the 1.1->1.2 change and for most people it's a drop in replacement unless the are using proxied apps. I just don't want this to be a reason that pevents people from adopting this release quickly. @All: Thanks for your input/contribution
        Hide
        Joachim Fritschi added a comment -

        For completeness:
        Since this issue was also security relevant it has CVE-2012-1104 assigned.

        Show
        Joachim Fritschi added a comment - For completeness: Since this issue was also security relevant it has CVE-2012-1104 assigned.
        Hide
        Joachim Fritschi added a comment -

        Closing all old issues.

        Show
        Joachim Fritschi added a comment - Closing all old issues.

          People

          • Assignee:
            Joachim Fritschi
            Reporter:
            Joachim Fritschi
          • Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: