Skip to content

[Ldap][Security] Remove deprecated eraseCredentials() from (User|Token)Interface #60742

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

Merged
merged 1 commit into from
Jun 16, 2025

Conversation

chalasr
Copy link
Member

@chalasr chalasr commented Jun 8, 2025

Q A
Branch? 8.0
Bug fix? no
New feature? no
Deprecations? no
Issues #59682
License MIT

We didn't deprecate the config option + container parameter from SecurityBundle nor the corresponding AuthenticatorManager constructor param, I propose to keep them no-op and deprecate them in 8.1 (mainly because AuthenticatorManager already deprecates a boolean parameter in 7.3 which makes deprecating the parameter complicates the bc layer and upgrade path significantly).

@carsonbot carsonbot added this to the 8.0 milestone Jun 8, 2025
@carsonbot carsonbot changed the title [Security][Ldap] Remove deprecated eraseCredentials() from (User|Token)Interface [Ldap][Security] Remove deprecated eraseCredentials() from (User|Token)Interface Jun 8, 2025
@chalasr chalasr force-pushed the remove-erasecredentials branch 2 times, most recently from 9127d07 to e1ed824 Compare June 8, 2025 18:12
@@ -209,10 +209,6 @@ private function executeAuthenticator(AuthenticatorInterface $authenticator, Req
// announce the authentication token
$authenticatedToken = $this->eventDispatcher->dispatch(new AuthenticationTokenCreatedEvent($authenticatedToken, $passport))->getAuthenticatedToken();

if ($this->eraseCredentials) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might need a plan to remove the property now

Copy link
Member Author

@chalasr chalasr Jun 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean deprecating on 7.4?

UPGRADE-8.0.md Outdated

```php
#[\Deprecated]
public function eraseCredentials(): void
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this method be removed here?

@nicolas-grekas nicolas-grekas force-pushed the remove-erasecredentials branch from e1ed824 to 7909077 Compare June 16, 2025 06:42
@nicolas-grekas nicolas-grekas force-pushed the remove-erasecredentials branch from 7909077 to 513a272 Compare June 16, 2025 06:44
@nicolas-grekas
Copy link
Member

Thank you @chalasr.

@nicolas-grekas nicolas-grekas merged commit 46aa500 into symfony:8.0 Jun 16, 2025
5 of 8 checks passed
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.

6 participants