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

[Form] Addhash_property_path option toPasswordType#46224

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

Merged

Conversation

@Seb33300
Copy link
Contributor

@Seb33300Seb33300 commentedMay 1, 2022
edited
Loading

QA
Branch?6.2
Bug fix?no
New feature?yes
Deprecations?no
TicketsFix#29066
LicenseMIT
Doc PRsymfony/symfony-docs#15872

Same as#42883 but using a Form Extension and rebased to 6.1 & tests.

This PR adds a newhash_mapping option toPasswordType.
Thehash_mapping option can be set with a property path where we want to set the hashed password.
Thehash_mapping option can only be used on unmapped fields to minimize plain password leak.

@carsonbotcarsonbot added this to the6.1 milestoneMay 1, 2022
@Seb33300Seb33300force-pushed thepassword-hash-mapping-extension branch 5 times, most recently from2724b72 to785c113CompareMay 1, 2022 16:03
@Seb33300Seb33300force-pushed thepassword-hash-mapping-extension branch 5 times, most recently froma8304ce toa8c9dbbCompareMay 2, 2022 09:15
@carsonbot
Copy link

Hey!

I think@michaelKaefer has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@fabpotfabpot modified the milestones:6.1,6.2May 8, 2022
@Seb33300Seb33300force-pushed thepassword-hash-mapping-extension branch 5 times, most recently from88e4fd2 to1f7b15fCompareMay 31, 2022 14:18
@javiereguiluz
Copy link
Member

@wouterj or@chalasr what do you think of this proposal? Thanks!

Copy link
Member

@ycerutoyceruto left a comment

Choose a reason for hiding this comment

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

This approach seems better than the previous one. Thanks for the second try!

@Seb33300Seb33300force-pushed thepassword-hash-mapping-extension branch fromef8b110 toffe9830CompareOctober 17, 2022 12:01
@Seb33300Seb33300force-pushed thepassword-hash-mapping-extension branch from5afae70 to533142bCompareOctober 18, 2022 14:14
@nicolas-grekasnicolas-grekas added the ❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze" labelOct 19, 2022
yceruto
yceruto previously approved these changesOct 20, 2022
Copy link
Member

@ycerutoyceruto left a comment

Choose a reason for hiding this comment

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

LGTM.

@yceruto
Copy link
Member

Some tests failures seem to be relatedhttps://github.com/symfony/symfony/actions/runs/3274200868/jobs/5387417225#step:7:2208

can you check?

Seb33300 reacted with eyes emoji

@ycerutoyceruto dismissed theirstale reviewOctober 20, 2022 11:37

Tests failing

@Seb33300
Copy link
ContributorAuthor

Fixed ine765a9b

All green now 🚀
Thanks for your review 🙏🏻

@chalasrchalasrforce-pushed thepassword-hash-mapping-extension branch from88f6c8b to9c06ab2CompareOctober 20, 2022 18:52
@Seb33300Seb33300force-pushed thepassword-hash-mapping-extension branch from9c06ab2 to56c1282CompareOctober 21, 2022 02:43
@Seb33300Seb33300force-pushed thepassword-hash-mapping-extension branch from56c1282 to7065dfeCompareOctober 21, 2022 03:46
@Seb33300
Copy link
ContributorAuthor

@chalasr I force pushed to change!$parentForm || !($user = $parentForm->getData()) into!($user = $parentForm?->getData()) and fix static analysis related to undefined$user

@fabpot
Copy link
Member

Thank you@Seb33300.

@fabpotfabpot merged commitcf073c4 intosymfony:6.2Oct 22, 2022
wouterj added a commit to symfony/symfony-docs that referenced this pull requestOct 23, 2022
…300)This PR was squashed before being merged into the 6.2 branch.Discussion----------[Form] Document the `hash_property_path` optionDocumentation forsymfony/symfony#46224Commits-------228d73e [Form] Document the `hash_property_path` option
@Seb33300Seb33300 deleted the password-hash-mapping-extension branchOctober 24, 2022 05:46
@fabpotfabpot mentioned this pull requestOct 24, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@wouterjwouterjwouterj requested changes

@fabpotfabpotfabpot approved these changes

@ycerutoycerutoyceruto approved these changes

@chalasrchalasrchalasr approved these changes

@xabbuhxabbuhAwaiting requested review from xabbuhxabbuh is a code owner

Assignees

No one assigned

Labels

FeatureForm❄️ Feature FreezeImportant Pull Requests to finish before the next Symfony "feature freeze"Status: Reviewed

Projects

None yet

Milestone

6.2

Development

Successfully merging this pull request may close these issues.

Auto Password Encoder

8 participants

@Seb33300@carsonbot@javiereguiluz@chalasr@yceruto@fabpot@wouterj@nicolas-grekas

[8]ページ先頭

©2009-2025 Movatter.jp