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

[Validator] Make API endpoint for NotCompromisedPasswordValidator configurable#31060

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
fabpot merged 1 commit intosymfony:masterfromxelan:feature/configurable-not-compromised-password-validator-api-endpoint
May 6, 2019
Merged

[Validator] Make API endpoint for NotCompromisedPasswordValidator configurable#31060

fabpot merged 1 commit intosymfony:masterfromxelan:feature/configurable-not-compromised-password-validator-api-endpoint
May 6, 2019

Conversation

@xelan
Copy link
Contributor

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?yes, but acceptable [1]
Deprecations?no [1]
Tests pass?yes
Fixed tickets#30871,#31054
LicenseMIT
Doc PRsymfony/symfony-docs#... (TODO)

Makes the API endpoint for theNotCompromisedPasswordValidator configurable. The endpoint includes the placeholder which will be replaced with the first digits of the password hash for k-anonymity.

The endpoint can either be set via constructor injection of the validator if the component is used standalone, or via the framework configuration of symfony/framework-bundle.

[1] As discussed in#31054, the validator is not in a stable release yet, therefore the BC break is considered acceptable. No deprecation / BC layer is necessary.

dunglas reacted with thumbs up emoji
@xelan
Copy link
ContributorAuthor

I'll create the Doc PR if the rest is ok.

@xelan
Copy link
ContributorAuthor

Thank you very much for your fast review and constructive feedback,@stof. I've integrated the changes you requested.

Copy link
Member

@nicolas-grekasnicolas-grekas 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-grekasnicolas-grekas added this to thenext milestoneApr 11, 2019
@nicolas-grekas
Copy link
Member

What do you all think ofGenuinePasswordValidator instead ofNotCompromisedPasswordValidator?

@xelan
Copy link
ContributorAuthor

Would also be possible, however I think NotCompromisedPasswordValidator is better concerning the responsiblity of the validator. It uses a blacklist of hashes for known compromised passwords.

chalasr and javiereguiluz reacted with thumbs up emoji

@xelan
Copy link
ContributorAuthor

xelan commentedApr 12, 2019•
edited
Loading

An initial prototype of my HIBP-compatible password compromisation check server, based on a simple program to chunk the source data in buckets and some Apache 2.4 rewrite rules, is available here:https://github.com/xelan/sphynx

Feedback is welcome 😄

@xelan
Copy link
ContributorAuthor

The requested changes concerning constant naming and documentation are incorporated. I'm still not sure about the section configurationcanBeEnabled/canBeDisabled. How would you prefer to have it,@nicolas-grekas and@ro0NL? The advantage ofcanBeDisabled is that the section is enabled by default and theenabled key is automatically added but without a description (akainfo()), right?

@stof
Copy link
Member

Well, I think in this case, having the info on the field is important to explain what will be the effect of disabling the validator (it won't prevent you to use the constraint, but will disable the validation)

@xelan
Copy link
ContributorAuthor

Adapted, thanks for the feedback 😄

@nicolas-grekasnicolas-grekas modified the milestones:next,4.3May 5, 2019
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

(with one minor comment)

@fabpot
Copy link
Member

Thank you@xelan.

@fabpotfabpot merged commitf6a80c2 intosymfony:masterMay 6, 2019
fabpot added a commit that referenced this pull requestMay 6, 2019
…rdValidator configurable (xelan)This PR was squashed before being merged into the 4.3-dev branch (closes#31060).Discussion----------[Validator] Make API endpoint for NotCompromisedPasswordValidator configurable| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | yes, but acceptable [1]| Deprecations? | no [1]| Tests pass?   | yes| Fixed tickets |#30871,#31054| License       | MIT| Doc PR        | symfony/symfony-docs#... (TODO)Makes the API endpoint for the `NotCompromisedPasswordValidator` configurable. The endpoint includes the placeholder which will be replaced with the first digits of the password hash for k-anonymity.The endpoint can either be set via constructor injection of the validator if the component is used standalone, or via the framework configuration of symfony/framework-bundle.[1] As discussed in#31054, the validator is not in a stable release yet, therefore the BC break is considered acceptable. No deprecation / BC layer is necessary.Commits-------f6a80c2 [Validator] Make API endpoint for NotCompromisedPasswordValidator configurable
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestMay 7, 2019
…ssword constraint (javiereguiluz)This PR was squashed before being merged into the master branch (closes#11527).Discussion----------Updated the configuration reference for NotCompromisedPassword constraintDocuments the changes made insymfony/symfony#31060Commits-------f5b081e Updated the configuration reference for NotCompromisedPassword constraint
@fabpotfabpot mentioned this pull requestMay 9, 2019
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull requestMay 31, 2019
… (ogizanagi)This PR was merged into the 4.3 branch.Discussion----------Fix `validation.not_compromised_password` option defaultReflectingsymfony/symfony#31060 changes in the docs.Commits-------35925da Fix `validation.not_compromised_password` option default
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@javiereguiluzjaviereguiluzjaviereguiluz left review comments

@stofstofstof left review comments

@xabbuhxabbuhxabbuh left review comments

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

+2 more reviewers

@ro0NLro0NLro0NL left review comments

@maxheliasmaxheliasmaxhelias approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

9 participants

@xelan@nicolas-grekas@stof@fabpot@javiereguiluz@ro0NL@xabbuh@maxhelias@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp