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

[Security] Improved upgrade path for custom remember me services #36832

Merged
merged 1 commit into from May 16, 2020

Conversation

wouterj
Copy link
Member

@wouterj wouterj commented May 16, 2020

Q A
Branch? master
Bug fix? no
New feature? -
Deprecations? -
Tickets #36806 (comment)
License MIT
Doc PR

This improves the upgrade path for custom remember me services now LogoutHandlerInterface has been deprecated.

As suggested in #36806 (comment), the logout() method should be added to the RememberMeServicesInterface in Symfony 6.

This patch allows developers to write a custom class implementing only RememberMeServicesInterface with a logout() method. Requiring them to implement LogoutHandlerInterface will mean they have to maintain 2 version of the class to support both Symfony 5.1+ and 6.0.

@nicolas-grekas nicolas-grekas added this to the 5.1 milestone May 16, 2020
@nicolas-grekas
Copy link
Member

Thank you @wouterj.

@nicolas-grekas nicolas-grekas merged commit c268915 into symfony:master May 16, 2020
@nicolas-grekas
Copy link
Member

I'll let you deal with the doc side of this PR, if there is any.

@wouterj wouterj deleted the pr-36806/add-deprecation branch May 16, 2020 12:07
@wouterj
Copy link
Member Author

wouterj commented May 16, 2020

Thanks for the quick merge!

For future reference: I don't think docs need to be updated. Implementing a custom remember me service is very unlikely (given that the two implementations of Symfony already have a token provider abstraction layer).

@fabpot fabpot mentioned this pull request May 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants