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 -

        I think i will push this back to the 2.0 release. No reason to fix with a lot of work and rewrite it later anyway.

        Show
        Joachim Fritschi added a comment - I think i will push this back to the 2.0 release. No reason to fix with a lot of work and rewrite it later anyway.
        Hide
        Joachim Fritschi added a comment -

        I have started a branch to implement this for the current version /branches/PHPCAS-69

        Show
        Joachim Fritschi added a comment - I have started a branch to implement this for the current version /branches/ PHPCAS-69
        Hide
        Joachim Fritschi added a comment -

        Ok i think i'm pretty much done with a first shot of a possible implementation. The user API is extended with one additional function allowToBeProxied().

        I have written some basic unit test and extended the examples. It works for me and i think i covered everything.

        Would be nice if anyone could verify/test the stuff.

        Show
        Joachim Fritschi added a comment - Ok i think i'm pretty much done with a first shot of a possible implementation. The user API is extended with one additional function allowToBeProxied(). I have written some basic unit test and extended the examples. It works for me and i think i covered everything. Would be nice if anyone could verify/test the stuff.
        Hide
        Andrew Petro added a comment -

        I inspected but did not install or exercise this code.

        In general this new test case seems to be testing the right things and this branch seems to make the key improvement of rejecting proxy tickets by default.

        https://source.jasig.org/cas-clients/phpcas/branches/PHPCAS-69/test/tests/ProxyTicketValidationTest.php

        I suggest that the examples stop including regexes that would match any URL and that the documentation should more clearly advise care in authorizing proxies.

        For example:

        > phpCAS::allowToBeProxied(true,array('/^https:\/\/myservice\.com\/.$/','http://myservice.com','/^http.$/'));

        That last http:*$ is a little scary, as is

        [
        // Allowing to be proxied should only be enabled when you want any other service
        // to proxy this service. By default it is disabled. You can allow any services
        // to proxy this service:
        //phpCAS::allowToBeProxied(true);
        ]

        In most cases, adopters shouldn't allow any service to proxy their service.

        Also, it might be a little confusing to include an http: example of an authorized proxy chain, in that normally pgtUrls will be https:// URLs, their point being to authenticate the service by means of an SSLed https:// callback.

        Still, that's nitpicking around the edges. Making tickets with proxy chains rejected by default is a huge improvement, a very important step forward for phpCAS. The changes in this branch look like that – laudable, much-appreciated improvement – though I have not personally exercised these changes.

        Show
        Andrew Petro added a comment - I inspected but did not install or exercise this code. In general this new test case seems to be testing the right things and this branch seems to make the key improvement of rejecting proxy tickets by default. https://source.jasig.org/cas-clients/phpcas/branches/PHPCAS-69/test/tests/ProxyTicketValidationTest.php I suggest that the examples stop including regexes that would match any URL and that the documentation should more clearly advise care in authorizing proxies. For example: > phpCAS::allowToBeProxied(true,array('/^https:\/\/myservice\.com\/. $/','http://myservice.com','/^http. $/')); That last http:*$ is a little scary, as is [ // Allowing to be proxied should only be enabled when you want any other service // to proxy this service. By default it is disabled. You can allow any services // to proxy this service: //phpCAS::allowToBeProxied(true); ] In most cases, adopters shouldn't allow any service to proxy their service. Also, it might be a little confusing to include an http: example of an authorized proxy chain, in that normally pgtUrls will be https:// URLs, their point being to authenticate the service by means of an SSLed https:// callback. Still, that's nitpicking around the edges. Making tickets with proxy chains rejected by default is a huge improvement, a very important step forward for phpCAS. The changes in this branch look like that – laudable, much-appreciated improvement – though I have not personally exercised these changes.
        Hide
        Joachim Fritschi added a comment -

        Thanks for your review Andrew.

        The http* rule is there because the idea of our examples is that with minimal settings you have a fully functioning setup of over 10 use cases by simply configuring your cas server in minimal central config. This however requires some security checks to be disabled to run on any webserver out of the box (Just like the default service settings in the cas server webapp ) I will however try to get get some autodetection code in there to configure a proper autodetected url that people don't copy&paste stuff without thinking/understanding.

        You are totally right about the the "any proxy" needing a proper warning sign around it just like the cas server certifcate check has. For my personal experience many people need these "drop all security" settings to get their application running for the first time without tripping over all the security layers but it should indeed have a proper warning. I think i will also write a big warning in the logs while i'm at it.

        The https is a good point. I didn't think this far

        The /branch seems to be on a good track so far and at least all the tests suggest that nothing broke.

        Show
        Joachim Fritschi added a comment - Thanks for your review Andrew. The http* rule is there because the idea of our examples is that with minimal settings you have a fully functioning setup of over 10 use cases by simply configuring your cas server in minimal central config. This however requires some security checks to be disabled to run on any webserver out of the box (Just like the default service settings in the cas server webapp ) I will however try to get get some autodetection code in there to configure a proper autodetected url that people don't copy&paste stuff without thinking/understanding. You are totally right about the the "any proxy" needing a proper warning sign around it just like the cas server certifcate check has. For my personal experience many people need these "drop all security" settings to get their application running for the first time without tripping over all the security layers but it should indeed have a proper warning. I think i will also write a big warning in the logs while i'm at it. The https is a good point. I didn't think this far The /branch seems to be on a good track so far and at least all the tests suggest that nothing broke.
        Hide
        Joachim Fritschi added a comment -

        All suggested changes added to the /branch

        Show
        Joachim Fritschi added a comment - All suggested changes added to the /branch
        Hide
        Joachim Fritschi added a comment -

        I have worked a bit more on the code and restructured everything in a new Object (ProxyChains) outside of the core client.
        This rips out more logic in this new object and allows more flexibility for later. It also made it possible to have multiple chains with proper checking of the order of proxies.

        Show
        Joachim Fritschi added a comment - I have worked a bit more on the code and restructured everything in a new Object (ProxyChains) outside of the core client. This rips out more logic in this new object and allows more flexibility for later. It also made it possible to have multiple chains with proper checking of the order of proxies.
        Hide
        Adam Franco added a comment -

        Nice work Joachim. I'm still going through the code in this branch, but a first minor thought is to rename ProxyChains to CAS_ProxyChains to match the naming convention of the rest of the classes in phpcas/source/CAS/. If the classname will be part of the public API, then it makes sense to get this lined up now.

        One other question: How do you envision the proxy-chain restrictions in phpCAS interacting with the "Service Allowed to Proxy" setting in the CAS service-management tool? Would more centralized chain-management (prior to a PGT even being granted) be preferred but just unavailable?

        I'm thinking of a situation I have where application A is proxied by B. If I want application C to proxy B, then I need to update the configuration on both B and A rather than just B. Getting back to the current feature, would there be a way to specify that A trusts B to validate any additional steps in the chain?

        Show
        Adam Franco added a comment - Nice work Joachim. I'm still going through the code in this branch, but a first minor thought is to rename ProxyChains to CAS_ProxyChains to match the naming convention of the rest of the classes in phpcas/source/CAS/ . If the classname will be part of the public API, then it makes sense to get this lined up now. One other question: How do you envision the proxy-chain restrictions in phpCAS interacting with the "Service Allowed to Proxy" setting in the CAS service-management tool? Would more centralized chain-management (prior to a PGT even being granted) be preferred but just unavailable? I'm thinking of a situation I have where application A is proxied by B. If I want application C to proxy B, then I need to update the configuration on both B and A rather than just B. Getting back to the current feature, would there be a way to specify that A trusts B to validate any additional steps in the chain?
        Hide
        Adam Franco added a comment -

        Looking at the ProxyChains class some more, I think we'd get quite a bit more flexibility if each allowed chain was an object rather than an array. This would allow for a variety of implementations to work in tandem. This means that the ProxyChains class would just be a looping mechanism or could be folded back into the client.

        Here is an example:

        interface CAS_ProxyChain_Interface {
        	/**
        	 * Match a list of proxies.
        	 * 
        	 * @param array $list The list of proxies in front of this service.
        	 * @return boolean
        	 */
        	public function matches(array $list);
        }
        
        /**
         * A proxy-chain that will validate the full chain of proxies in front of this service.
         */
        class CAS_ProxyChain implements CAS_ProxyChain_Interface {
        	/**
        	 * @param array $chain An array of regular expressions or strings to match in sequence.
        	 */
        	public function __construct(array $chain) {
        		$this->_chain = $chain;
        	}
        	
        	/**
        	 * Match a list of proxies.
        	 * 
        	 * @param array $list The list of proxies in front of this service.
        	 * @return boolean
        	 */
        	public function matches(array $list) {
        		....
        	}
        }
        
        /**
         * A proxy-chain that will validate the first service in a proxy chain and trust
         * it to validate the rest of the chain.
         */
        class CAS_ProxyChain_Trusted implements CAS_ProxyChain_Interface {
        	/**
        	 * @param string $trustedProxy A regular expression or plain string that will match a single proxy
        	 */
        	public function __construct($trustedProxy) {
        		$this->_trustedProxy = $trustedProxy;
        	}
        	
        	/**
        	 * Match a list of proxies.
        	 * 
        	 * @param array $list The list of proxies in front of this service.
        	 * @return boolean
        	 */
        	public function matches(array $list) {
        		.....
        	}
        }
        
        // Usage
        $proxies = new ProxyChains();
        $proxies->addChain(new CAS_ProxyChain(array('/^https:\/\/myservice\.com\/.*$/','https://other_service.com')));
        $proxies->addChain(new CAS_ProxyChain(array('/^https:\/\/other\.com\/.*$/', 'https://yet_another_service.com')));
        $proxies->addChain(new CAS_ProxyChain_Trusted('/^https:\/\/trusted.example.com\/.*$/'));
        phpCAS::allowToBeProxied(true,$proxies);
        

        The naming of these classes and methods might not be quite ideal, but maybe something like the following might make semantic sense:

        phpCAS::allowToBeProxiedBy(new CAS_ProxyChain(array('/^https:\/\/myservice\.com\/.*$/', 'https://other_service.com')));
        phpCAS::allowToBeProxiedBy(new CAS_ProxyChain(array('/^https:\/\/other\.com\/.*$/', 'https://yet_another_service.com')));
        phpCAS::allowToBeProxiedBy(new CAS_ProxyChain_Trusted('/^https:\/\/trusted.example.com\/.*$/'));
        

        This could also provide for the case of allowing any proxy:

        phpCAS::allowToBeProxiedBy(new CAS_ProxyChain_Any());
        

        If you feel this is a good track to take I can put together a full patch.

        By the way, the cleanup of the ST/PT code is fabulous!

        Show
        Adam Franco added a comment - Looking at the ProxyChains class some more, I think we'd get quite a bit more flexibility if each allowed chain was an object rather than an array. This would allow for a variety of implementations to work in tandem. This means that the ProxyChains class would just be a looping mechanism or could be folded back into the client. Here is an example: interface CAS_ProxyChain_Interface { /** * Match a list of proxies. * * @param array $list The list of proxies in front of this service. * @ return boolean */ public function matches(array $list); } /** * A proxy-chain that will validate the full chain of proxies in front of this service. */ class CAS_ProxyChain implements CAS_ProxyChain_Interface { /** * @param array $chain An array of regular expressions or strings to match in sequence. */ public function __construct(array $chain) { $ this ->_chain = $chain; } /** * Match a list of proxies. * * @param array $list The list of proxies in front of this service. * @ return boolean */ public function matches(array $list) { .... } } /** * A proxy-chain that will validate the first service in a proxy chain and trust * it to validate the rest of the chain. */ class CAS_ProxyChain_Trusted implements CAS_ProxyChain_Interface { /** * @param string $trustedProxy A regular expression or plain string that will match a single proxy */ public function __construct($trustedProxy) { $ this ->_trustedProxy = $trustedProxy; } /** * Match a list of proxies. * * @param array $list The list of proxies in front of this service. * @ return boolean */ public function matches(array $list) { ..... } } // Usage $proxies = new ProxyChains(); $proxies->addChain( new CAS_ProxyChain(array('/^https:\/\/myservice\.com\/.*$/','https: //other_service.com'))); $proxies->addChain( new CAS_ProxyChain(array('/^https:\/\/other\.com\/.*$/', 'https: //yet_another_service.com'))); $proxies->addChain( new CAS_ProxyChain_Trusted('/^https:\/\/trusted.example.com\/.*$/')); phpCAS::allowToBeProxied( true ,$proxies); The naming of these classes and methods might not be quite ideal, but maybe something like the following might make semantic sense: phpCAS::allowToBeProxiedBy( new CAS_ProxyChain(array('/^https:\/\/myservice\.com\/.*$/', 'https: //other_service.com'))); phpCAS::allowToBeProxiedBy( new CAS_ProxyChain(array('/^https:\/\/other\.com\/.*$/', 'https: //yet_another_service.com'))); phpCAS::allowToBeProxiedBy( new CAS_ProxyChain_Trusted('/^https:\/\/trusted.example.com\/.*$/')); This could also provide for the case of allowing any proxy: phpCAS::allowToBeProxiedBy( new CAS_ProxyChain_Any()); If you feel this is a good track to take I can put together a full patch. By the way, the cleanup of the ST/PT code is fabulous!
        Hide
        Andrew Petro added a comment -

        I'm not sure I understand your last question, Adam. Each service gets to evaluate the proxy chain only as it receives it.

        So, user logs in via CAS to, say, Drupal, functioning in this story as a "portal". Drupal acquires a Proxy Granting Ticket and from that obtains a Proxy Ticket to authenticate to an IMAP-to-XML bridge that happens to be implemented in Java. That bridge validates the proxy ticket, inspects the proxy chain, and decides that it trusts the drupal instance identified by its proxy callback URL. This requires configuration in the bridge CAS client library to accept that particular one-link-long proxy chain.

        Now, that bridge itself obtains a proxy granting ticket and proxies to a CASified IMAP server. This requires no additional configuration of the phpCAS client in Drupal. The bridge obtains a proxy ticket and presents it to the IMAP server. When the IMAP server validates the ticket, it sees a two-link-long proxy chain: Drupal, followed by the bridge. It needs configuration to decide that that chain is okay.

        This address the question?

        Show
        Andrew Petro added a comment - I'm not sure I understand your last question, Adam. Each service gets to evaluate the proxy chain only as it receives it. So, user logs in via CAS to, say, Drupal, functioning in this story as a "portal". Drupal acquires a Proxy Granting Ticket and from that obtains a Proxy Ticket to authenticate to an IMAP-to-XML bridge that happens to be implemented in Java. That bridge validates the proxy ticket, inspects the proxy chain, and decides that it trusts the drupal instance identified by its proxy callback URL. This requires configuration in the bridge CAS client library to accept that particular one-link-long proxy chain. Now, that bridge itself obtains a proxy granting ticket and proxies to a CASified IMAP server. This requires no additional configuration of the phpCAS client in Drupal. The bridge obtains a proxy ticket and presents it to the IMAP server. When the IMAP server validates the ticket, it sees a two-link-long proxy chain: Drupal, followed by the bridge. It needs configuration to decide that that chain is okay. This address the question?
        Hide
        Adam Franco added a comment -

        Hi Andrew,

        I currently have a Drupal site making proxy-authenticated requests to a Directory Service:

        Drupal site: authentication via phpCAS


        Directory Service: a PHP service authenticated using phpCAS that returns XML info about people.

        My Drupal site in turn exposes a web service that allows users to query information that Drupal combines with responses from the Directory Service. The directory service is configured to allow itself to be proxied by the Drupal site.

        If I add a client that proxy-authenticates requests to the Drupal site, this adds another entry in the proxy chain:

        Mobile dashboard system: A PHP application authenticating via phpCAS.


        Drupal site: authentication via phpCAS


        Directory Service: a PHP service authenticated using phpCAS that returns XML info about people.

        When I add the Mobile system, I will need to configure the Drupal site to allow itself to be proxied by the Mobile system, but I would also need to configure the Directory Service to allow itself to be Drupal plus the Mobile system. If another client to the Drupal site is added, the Directory Service would need to be touched yet again.

        In my question above, I'm hoping that I can configure my underlying service to trust the next layer up so that each layer only has to manage the clients directly proxying it rather than the full chain.

        Best,
        Adam

        Show
        Adam Franco added a comment - Hi Andrew, I currently have a Drupal site making proxy-authenticated requests to a Directory Service: Drupal site: authentication via phpCAS Directory Service: a PHP service authenticated using phpCAS that returns XML info about people. My Drupal site in turn exposes a web service that allows users to query information that Drupal combines with responses from the Directory Service. The directory service is configured to allow itself to be proxied by the Drupal site. If I add a client that proxy-authenticates requests to the Drupal site, this adds another entry in the proxy chain: Mobile dashboard system: A PHP application authenticating via phpCAS. Drupal site: authentication via phpCAS Directory Service: a PHP service authenticated using phpCAS that returns XML info about people. When I add the Mobile system, I will need to configure the Drupal site to allow itself to be proxied by the Mobile system, but I would also need to configure the Directory Service to allow itself to be Drupal plus the Mobile system. If another client to the Drupal site is added, the Directory Service would need to be touched yet again. In my question above, I'm hoping that I can configure my underlying service to trust the next layer up so that each layer only has to manage the clients directly proxying it rather than the full chain. Best, Adam
        Hide
        Joachim Fritschi added a comment -

        Ups, the naming convention. Fix commited

        The service management tool only allows to configure whether a service can get an PT for any kind of service. There is currently no way to manage the possible targets (services being proxied) by a service. The cas authorization model has always been decentralized. The current service registry kind of breaks with this a bit (proxy yes/no), opt-out and attributes. But still it's highly decentralized and some people don't even use the service registry at all. (default https*) The security is pretty much delegated to the admin who can decide whether to trust another service or a chain of services in front.

        @Adam: I really like your suggestion. It takes my ideas a bit further in the right direction I especially like the CAS_ProxyChain_Any() and dropping the whole boolean stuff. I think this could remove quite a bit of unecessary code and get down to a fully object oriented code. I think the new CAS_ProxyChain_Trusted() is also a nice idea for the usecases you suggested (When you control/trust the proxy immediately in from of you then you can just delegate the validation of the rest of the chain). If you want just jump right in an work on the branch.

        Thanks for the compliment. The testframework made the changes easy and controllable and somehow it came really easy chipping away over 200 lines of code. I was really suprised how easy it was to actually get rid of several hacks and produce pretty clean code once i made my mind up My last try at this was before the test framework and the client just broke horribly and i gave up.

        Show
        Joachim Fritschi added a comment - Ups, the naming convention. Fix commited The service management tool only allows to configure whether a service can get an PT for any kind of service. There is currently no way to manage the possible targets (services being proxied) by a service. The cas authorization model has always been decentralized. The current service registry kind of breaks with this a bit (proxy yes/no), opt-out and attributes. But still it's highly decentralized and some people don't even use the service registry at all. (default https*) The security is pretty much delegated to the admin who can decide whether to trust another service or a chain of services in front. @Adam: I really like your suggestion. It takes my ideas a bit further in the right direction I especially like the CAS_ProxyChain_Any() and dropping the whole boolean stuff. I think this could remove quite a bit of unecessary code and get down to a fully object oriented code. I think the new CAS_ProxyChain_Trusted() is also a nice idea for the usecases you suggested (When you control/trust the proxy immediately in from of you then you can just delegate the validation of the rest of the chain). If you want just jump right in an work on the branch. Thanks for the compliment. The testframework made the changes easy and controllable and somehow it came really easy chipping away over 200 lines of code. I was really suprised how easy it was to actually get rid of several hacks and produce pretty clean code once i made my mind up My last try at this was before the test framework and the client just broke horribly and i gave up.
        Hide
        Adam Franco added a comment -

        Thanks for the clarification on the service registry. I've always found it useful, but lacking in some ways .

        I'll jump in on the branch and refactor tomorrow (Tuesday). A couple of questions before I get started:

        1. Shall we move the looping into the CAS_Client or leave it in an additional ProxyChains class?

        2. Any suggestions for a better name than phpCAS::allowToBeProxiedBy(CAS_ProxyChain $chain)?

        Show
        Adam Franco added a comment - Thanks for the clarification on the service registry. I've always found it useful, but lacking in some ways . I'll jump in on the branch and refactor tomorrow (Tuesday). A couple of questions before I get started: 1. Shall we move the looping into the CAS_Client or leave it in an additional ProxyChains class? 2. Any suggestions for a better name than phpCAS::allowToBeProxiedBy(CAS_ProxyChain $chain) ?
        Hide
        Joachim Fritschi added a comment -

        1. I would really suggest moving as much as possible code out of the CAS_Client. This is a good opportunity continue to slowly move away from this current monolithic client to a more modular code which is better testable and also will be much better to reuse once we start pushing for phpCAS 2.0.

        2. No nothing comes to mind.

        Thanks for jumping in. I think this really helps to get the "quick fix" i implemented on friday and improved over weekend to progress to a real solid and future proof solution.

        Show
        Joachim Fritschi added a comment - 1. I would really suggest moving as much as possible code out of the CAS_Client. This is a good opportunity continue to slowly move away from this current monolithic client to a more modular code which is better testable and also will be much better to reuse once we start pushing for phpCAS 2.0. 2. No nothing comes to mind. Thanks for jumping in. I think this really helps to get the "quick fix" i implemented on friday and improved over weekend to progress to a real solid and future proof solution.
        Hide
        Adam Franco added a comment -

        I have now refactored the proxy-chain system and committed it to the branch.

        1. I kept the CAS_ProxyChains object, but hid it from the public interface to keep the syntax more clean. This allowed me to remove a few more methods from the CAS_Client which now just serves as a registry for and user of the CAS_ProxyChains.

        2. I ended up going with phpCAS::allowProxyingBy(new CAS_ProxyChain(...)) as it sounded ok to my ear. If anyone has better suggestions we can rename this method.

        I also added some tests dedicated to the ProxyChains at test/tests/ProxyChainsTest.php and updated the CAS_ProxyChain to support PCRE pattern modifiers.

        Let me know if there is anything you would like me to change or improve on this or otherwise need help on for this issue.

        Show
        Adam Franco added a comment - I have now refactored the proxy-chain system and committed it to the branch. 1. I kept the CAS_ProxyChains object, but hid it from the public interface to keep the syntax more clean. This allowed me to remove a few more methods from the CAS_Client which now just serves as a registry for and user of the CAS_ProxyChains . 2. I ended up going with phpCAS::allowProxyingBy(new CAS_ProxyChain(...)) as it sounded ok to my ear. If anyone has better suggestions we can rename this method. I also added some tests dedicated to the ProxyChains at test/tests/ProxyChainsTest.php and updated the CAS_ProxyChain to support PCRE pattern modifiers. Let me know if there is anything you would like me to change or improve on this or otherwise need help on for this issue.
        Hide
        Joachim Fritschi added a comment -

        I think the ProxyChain class should be moved to the ProxyChain folder. Maybe even ProxyChains? Just to tidy things up

        The java client uses allowedProxyChain? Maybe we should simply align? But i really have no preference here.

        There rest seems perfect.

        Show
        Joachim Fritschi added a comment - I think the ProxyChain class should be moved to the ProxyChain folder. Maybe even ProxyChains? Just to tidy things up The java client uses allowedProxyChain? Maybe we should simply align? But i really have no preference here. There rest seems perfect.
        Hide
        Adam Franco added a comment -

        I decided to rename allowProxyingBy() to allowProxyChain(). This makes it more in line with the Java client, but keeps the method name a verb (rather than an adjective in the Java client case).

        Also, I moved the CAS_ProxyChains class into the ProxyChain directory and renamed it to CAS_ProxyChain_AllowedList to make it clear that it is different than the individual chain definitions.

        I have not moved the CAS_ProxyChain class file to keep with the convention of the directory and filename mapping to the classname (replacing '/' with '_' and dropping the extension). One of these days I hope to set up autoload for phpCAS using spl_autoload_register(). We could keep the convention and rename it to something like CAS_ProxyChain_Full, but that would make the standard use case a bit more verbose.

        Show
        Adam Franco added a comment - I decided to rename allowProxyingBy() to allowProxyChain() . This makes it more in line with the Java client, but keeps the method name a verb (rather than an adjective in the Java client case). Also, I moved the CAS_ProxyChains class into the ProxyChain directory and renamed it to CAS_ProxyChain_AllowedList to make it clear that it is different than the individual chain definitions. I have not moved the CAS_ProxyChain class file to keep with the convention of the directory and filename mapping to the classname (replacing '/' with '_' and dropping the extension). One of these days I hope to set up autoload for phpCAS using spl_autoload_register() . We could keep the convention and rename it to something like CAS_ProxyChain_Full , but that would make the standard use case a bit more verbose.
        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: