Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Security] Deprecate UserInterface & TokenInterface'seraseCredentials()#59682

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

nicolas-grekas
Copy link
Member

@nicolas-grekasnicolas-grekas commentedFeb 3, 2025
edited
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?yes
IssuesFix#57842
LicenseMIT

As promised, this PR adds a commit on top of#59106 to improve the BC layer. This approach didn't fit in a review comment :) /cc@chalasr

This PR leverages the new#[\Deprecated] attribute to decide if someeraseCredentials() method is to be called or not.

My target DX here is to save us all (the community) from having to adderase_credentials: false configuration in all our apps.

So, instead of having to opt-out from the deprecation using this config option, the opt-out is done by adding the attribute on the method:

Before:

publicfunctioneraseCredentials():void{}

After:

#[\Deprecated]publicfunctioneraseCredentials():void{}// If your eraseCredentials() method was used to empty a "password" property:publicfunction__serialize():array{$data = (array)$this;    unset($data["\0".self::class."\0password"]);return$data;}

This should provide a smoother upgrade path (and maker-bundle could be updated right-away).

@nicolas-grekasnicolas-grekasforce-pushed thedeprecate-erase-credentials branch 3 times, most recently from012a13f to2069f53CompareFebruary 3, 2025 21:33
Copy link
Member

@chalasrchalasr left a comment

Choose a reason for hiding this comment

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

Thanks!

@nicolas-grekas
Copy link
MemberAuthor

Comments addressed thanks.

@nicolas-grekasnicolas-grekasforce-pushed thedeprecate-erase-credentials branch 2 times, most recently fromb7d3929 to5df85e4CompareFebruary 4, 2025 07:25
Copy link
Member

@chalasrchalasr left a comment

Choose a reason for hiding this comment

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

This kind of BC layer feels new to me, refreshing :) 👍 with some more minor catches

@nicolas-grekasnicolas-grekasforce-pushed thedeprecate-erase-credentials branch from5df85e4 toe5c94e6CompareFebruary 4, 2025 08:23
@nicolas-grekas
Copy link
MemberAuthor

Thank you@chalasr.

chalasr reacted with heart emoji

@nicolas-grekasnicolas-grekas merged commit5a09d52 intosymfony:7.3Feb 4, 2025
8 of 12 checks passed
@nicolas-grekasnicolas-grekas deleted the deprecate-erase-credentials branchFebruary 4, 2025 14:30
@fabpotfabpot mentioned this pull requestMay 2, 2025
nicolas-grekas added a commit that referenced this pull requestJun 2, 2025
This PR was merged into the 7.3 branch.Discussion----------[Ldap] Fix `LdapUser::isEqualTo`| Q             | A| ------------- | ---| Branch?       | 7.3| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        | Fix#60454| License       | MITSince#59682 `LdapUser`s’ password no longer is serialized in the session, which means that `isEqualTo` will crash when trying to access it.This PR makes `getPassword` returns `null` by default to fix this, and update `isEqualTo` to apply#59539’s logic.Commits-------ad74742 [Ldap] Fix `LdapUser::isEqualTo`
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@welcoMatticwelcoMatticwelcoMattic approved these changes

@chalasrchalasrchalasr approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

Move or retire UserInterface::eraseCredentials
4 participants
@nicolas-grekas@welcoMattic@chalasr@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp