openeuropa / pcas Goto Github PK

The PCasUser object is adhering to Symfony's UserInterface specification. This makes it less flexible to use in other PHP frameworks, especially since this interface defines a number of methods which doesn't make sense in the scope of CAS authentication, such as ::getPassword() and ::getSalt().

I think it would be a good idea to create a Symfony-specific component which has this library as a dependency and wraps the PCas-classes inside versions that comply with Symfony. The classes PCasGuardAuthenticator and PCasUserProvider also seem to have little use outside of Symfony.

Replace namespace drupol\ with OpenEuropa\.

In order to clean and make the use of the library easier, I would like to remove the duplicate dependency to the HttpClient from the Protocol object and use the one provided in the PCas constructor.

Both the demo-server and demo-client applications are using PHP7-specific functionality so they are not compatible with the PHP5.6 target which is specified in the Composer manifest.

More specifically, they are using the null coalescing operator ?? in the bin/console launchers. This is a feature that was added in PHP 7.0.

Ref. Null coalescing operator

If base_url in PCas properties has a path (e.g. /cas), it is ignored and overwritten with /login (whereas it should be /cas/login).

There are a number of classes declared as services on the DI container which are not designed to be used as such:

• Symfony\Component\Serializer\Serializer: this class requires an array of $normalizers and $decoders to be passed in and these are stored locally on the object. This implies that for every set of normalizers and decoders a new instance of the class is expected to be instantiated. This is not compatible with the singleton model of services. If you need to retrieve an instance of this class through the container, you can use a factory service that will pass you a fresh instance.
• Symfony\Component\Serializer\Encoder\XmlEncoder: this has a whole bunch of local properties which are not intended to be modified by external code. This is not intended to be used as a singleton. Exposing this on the container can have unintended side effects, for example if another class calls ::setRootNodeName() then it will overwrite the $this->rootNodeName local property and any subsequent calls to ::encode() by the PCas library will not give the expected results.
• Symfony\Component\Serializer\Encoder\JsonEncoder: upon instantiation this accepts an encoder and a decoder, so this is not suitable to be used as a singleton, if it is really necessary to retrieve this class through the container it should be handled through a factory class.
• Symfony\Component\Serializer\Normalizer\ObjectNormalizer: this object has a bunch of local properties which should not be accessible by other users of the container, for example a call to ::setSerializer() by external code will make this service unusable for the PCas library.

Instead of exposing these on the container they should be stored in local protected properties by the consumers.