fix: remove deprecated eraseCredentials() — breaks logout on Symfony 8 - #111
fix: remove deprecated eraseCredentials() — breaks logout on Symfony 8#111smbpunt wants to merge 4 commits into
Conversation
`eraseCredentials()` was deprecated in Symfony 7.3 and removed in 8.0. - Remove empty `eraseCredentials()` from `CasUser` - Remove `$security->getToken()?->eraseCredentials()` call from `Logout` controller - Remove unused `Security` dependency from `Logout` controller Ref: https://symfony.com/blog/new-in-symfony-7-3-security-improvements#deprecate-erasecredentials-method
|
Heads up — compatibility concern with this PR as-is. Removing How would you like to handle this @drupol ? Should I adjust the PR, or do you prefer a different approach? |
- Remove empty eraseCredentials() from CasUser (removed in SF8) - Remove eraseCredentials() call from Logout controller - Remove now-unused Security dependency from Logout controller Co-authored-by: smbpunt <smbpunt@users.noreply.github.com> See: ecphp#111
|
What's the best way to handle this ? |
UserInterface still requires eraseCredentials() on Symfony 6.4 and 7 (deprecated in 7.3, removed in 8.0). The empty method is harmless on Symfony 8 since it's no longer part of the interface.
|
I reworked the PR. Here's what I propose:
My recommendation: merge this as-is and tag it as 3.1.1. It's a bug fix on top of an already-tagged release, nothing more, so a patch version fits. Separately, worth discussing: the bundle supports three major Symfony versions at once (6.4, 7, and 8). Looking at Might be worth planning a proper 6.4 drop soon, instead of leaving that inconsistency in place — it's exactly the kind of half-state that caused this bug. And with Symfony 8.2 landing in November, closing the gap with 8.x sooner rather than later would help avoid supporting three majors for much longer. One more thought: this PR sat open for a while, so I get that everyone's busy. Maybe the next major version could just jump straight to Symfony 8 only. That would get rid of this whole eraseCredentials() mess for good, and give the bundle a clean start. |
The Security collaborator was removed from Logout::__invoke() but the spec still passed it, sending a Security double where a TokenStorageInterface was expected (TypeError on the 3 examples).
Login::__invoke() has not taken a Security argument for a while, but the spec still declared and passed one. PHP silently ignores the extra trailing argument, so this was dead weight rather than a failure.
|
I pushed two more commits. Both remove a leftover |
|
LGTM ! I'll push this further by Friday 21. Thank you! |
Summary
eraseCredentials()method fromCasUser$security->getToken()?->eraseCredentials()call fromLogoutcontrollerSecuritydependency fromLogoutcontrollerContext
eraseCredentials()was deprecated in Symfony 7.3 and removed in 8.0. Sincecas-bundlenow supports Symfony 8 ("symfony/framework-bundle": "^7 || ^8"), the call in theLogoutcontroller causes a fatal error, breaking logout entirely.Ref: https://symfony.com/blog/new-in-symfony-7-3-security-improvements#deprecate-erasecredentials-method