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

AuthenticationSuccessHandler should not be tagged as final class #1140

Open
mboron83 opened this issue May 9, 2023 · 2 comments
Open

AuthenticationSuccessHandler should not be tagged as final class #1140

mboron83 opened this issue May 9, 2023 · 2 comments

Comments

@mboron83
Copy link

mboron83 commented May 9, 2023

Hi, my phpstan discovered that AuthenticationSuccessHandler is final class and I'm not able to extend it. I checked source and class is not final but has @final annotation. In my opinion it's wrong because on the other side you allow to configure custom success handler via security.yaml:

    api_login:
        provider: app_user_provider
        pattern: ^/api/login
        stateless: true
        json_login:
            check_path: /api/login_check
            success_handler: App\Service\Authentication\ApiLoginSuccessHandler
            failure_handler: App\Service\Authentication\ApiLoginFailureHandler
@Spomky
Copy link
Contributor

Spomky commented May 9, 2023

This looks good to me. The @final is used to avoid incompatibilities with old PHP versions. But at a time, it will become a final statement.

You should not extend it, but create you own class with the application logic you expect. Then, configure it as you mentioned.

@mboron83
Copy link
Author

mboron83 commented May 9, 2023

This looks good to me. The @final is used to avoid incompatibilities with old PHP versions. But at a time, it will become a final statement.

You should not extend it, but create you own class with the application logic you expect. Then, configure it as you mentioned.

Thank you for reply. Now I understand your point of view. I guess. in that case AuthenticationFailureHandler should also be tagged as final because now there is small inconsistency.

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

2 participants