Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Symfony 6.0 compatibility #58

Open
damienfa opened this issue Feb 3, 2022 · 9 comments
Open

Symfony 6.0 compatibility #58

damienfa opened this issue Feb 3, 2022 · 9 comments

Comments

@damienfa
Copy link

damienfa commented Feb 3, 2022

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

  • Be compatible with Symfony 6.0 (and 5.4)
  • The new Authenticator should not be "final", it will be convenient to be able to "extends" in the code if needed.
@drupol
Copy link
Member

drupol commented Feb 3, 2022

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
Copy link
Author

damienfa commented Feb 3, 2022

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
Copy link
Member

drupol commented Feb 3, 2022

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
Copy link
Author

damienfa commented Feb 3, 2022

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 😅

thanks !

@drupol
Copy link
Member

drupol commented Feb 3, 2022

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 drupol mentioned this issue Feb 9, 2022
3 tasks
@drupol
Copy link
Member

drupol commented Feb 9, 2022

@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
Copy link
Member

drupol commented Feb 18, 2022

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

@Lubna93
Copy link

Lubna93 commented Mar 21, 2022

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
Copy link
Member

drupol commented Mar 21, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants