ecphp / cas-bundle

CAS Bundle, a standard Symfony bundle for authentication using CAS protocol.
https://ecphp-cas-bundle.readthedocs.io
BSD 3-Clause "New" or "Revised" License
43 stars 9 forks source link

Symfony 6.0 compatibility #58

Closed damienfa closed 1 year ago

damienfa commented 2 years ago

Description

The Guards have been removed in Symfony 6.0. The CasGuardAuthenticator should be migrated to something like "CasCustomAuthenticator" using the new "custom authenticator".

Expected Result

drupol commented 2 years ago

Hello Damien,

Upgrading the packages for Symfony >= 5.4 has been discussed yesterday and it will come very soon.

Regarding the final keyword, I'm afraid that this won't change. If you're willing to "extend" a class, I would suggest you to use composition en inheritance.

We also did a presentation here : https://github.com/ecphp/session--composition-and-inheritance/

Feel free to let me know if you have any other questions.

damienfa commented 2 years ago

Ok, great about the update. Can't wait to use this library back. 👌🏼

About the final, I can perfectly use a composition (and I will).

But, you make me wonder… I feel like my use-case is particularly adapted to inheritance and not composition : once the CAS has validated the user, I would like that my InheritedCasAuthenticator validates that the user has a valid registration on the product. For that, I just "add" protected/private methods (= to check the product registration) + overwrite one of the existing methods to call them and calling a parent:: inside it. The semantic and mechanical aspect of the inheritance are respected. Isn't it? My subclass is still a CasAutenticator, but upgraded with a new feature. My inheritage is not done to "cherry pick" methods in order to reuse code, it's done to reuse the whole class and its logic within an "upgraded CasAuthenticator" (via inheritage). In my case, why should you force to "composition pattern" the parent class with a final ?

drupol commented 2 years ago

Thanks :)

However, I'm using Composition over inheritance since years and I never found a use-case where inheritance was a better pattern.

If you want to customize the validate method of the CasAuthenticator, you have to create your own Authenticator and decorate the original one.

Some code worth ten thousands lines of explanation:


// This is the original class
final class CasGuardAuthenticator extends AbstractGuardAuthenticator {
  // I omitted the body of it for brevity.
}

// My own custom decorator.
final class MyCustomCasGuardAuthenticator extends AbstractGuardAuthenticator {
  private AuthenticatorInterface $authenticator; // This is the decorated service.

  public function __construct(AuthenticatorInterface $authenticator) {
    $this->authenticator = $authenticator;
  }

    public function getCredentials(Request $request): ?ResponseInterface {
        // Call the original method here:
        $response = $this->authenticator->getCredentials($request);

       // Then do extra control here:
       return $this->checkIfUserHasValidRegistration($response);
    }

    private function checkIfUserHasValidRegistration(ResponseInterface $response): bool
    {
        return true; // Implements your custom business logic here.
    }

    // Implements all the rest of the methods and call the decorated authenticator
    // when you do not need to customize anything.
}

And don't forget to add this in services.yaml so the container will do it's job properly, So that when you call the cas.guardauthenticator, your custom service will be served.

services:
    FQDN\Of\My\Custom\GuardAuthenticator:
        decorates: 'cas.guardauthenticator'
        arguments: ['@.inner']
damienfa commented 2 years ago

Okay 😄 I get it. Thanks! I don't usually think about the Symfony Service's "decorator" but it's a convenient way to do that. You did well to specify the content of "services.yaml", I would not have understood at first reading how you injected it otherwise.

The only problem I see (if I understand well) is that for ALL the other methods in the class, I will have to do :

public function anOtherMethods(…) {
        return $authenticator->anOtherMethods(…);
}

It's a bit tedious :sweat_smile:

thanks !

drupol commented 2 years ago

Yes, that's the price to pay and the only downside of using the decorator pattern. I actually could provide an abstract class which does most of the job, that's an idea I keep in my head for sure for the next iteration release.

drupol commented 2 years ago

@damienfa

I created a branch that should fix the issue. I'm going to wait for some feedback of yours before merging anything.

The PR is sketchy, so it is possible that it might be broken, don't be afraid to report the least thing you see.

Tests are currently broken, I will fix them and the documentation in the meantime.

drupol commented 2 years ago

@damienfa Do you have any feedback to share so far?

Lubna93 commented 2 years ago

Hello, many thanks for your great efforts! Really appreciate your time and your helpful bundle :) I am wondering if I can use this CAS bundle for Symfony 6 for now?

Thank you again!

drupol commented 2 years ago

Hi @Lubna93 !

I have it working with Symfony 6 on my laptop, but I still need to make some changes before committing everything.

I'll update this thread when I'll commit my stuff so you'll get the notification.

drupol commented 1 year ago

Dear @Lubna93 and @damienfa ,

ecphp/cas-bundle is fully compatible with Symfony 6 and up now.

Can I close the issue ?

Lubna93 commented 1 year ago

@drupol Thanks a lot for your great efforts! Your hard work to help others is so much appreciated! Yes, you can close it with many thanks!

drupol commented 1 year ago

You're very welcome :)