Skip to content

fix type hint for salt in PasswordEncoderInterface #31813

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 3, 2019

Conversation

garak
Copy link
Contributor

@garak garak commented Jun 3, 2019

See issue #31812

Q A
Branch? 3.4
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #31812
License MIT
Doc PR none

Pretty self-explanatory

@xabbuh
Copy link
Member

xabbuh commented Jun 3, 2019

The mergePasswordAndSalt() method in the BasePasswordEncoder class could be updated as well.

Copy link
Contributor

@Tobion Tobion left a comment

Choose a reason for hiding this comment

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

Null is also used

But in general I think we should deprecate the salt argument completely as it's not best practice anymore. E.g. php also deprecated it in https://www.php.net/manual/en/function.password-hash.php

@nicolas-grekas
Copy link
Member

we should deprecate the salt argument completely as it's not best practice anymore

100% the only issue is figuring out a smooth way to upgrade. I think we'll be able to do it in 5.1 once we provide a way to migrate passwords to new hash algos. Would that make sense?

@nicolas-grekas nicolas-grekas force-pushed the fix-encoder-typehint branch from 1f0ea28 to 0e741f9 Compare June 3, 2019 19:38
@nicolas-grekas
Copy link
Member

Thank you @garak.

@nicolas-grekas nicolas-grekas merged commit 0e741f9 into symfony:3.4 Jun 3, 2019
nicolas-grekas added a commit that referenced this pull request Jun 3, 2019
This PR was squashed before being merged into the 3.4 branch (closes #31813).

Discussion
----------

fix type hint for salt in PasswordEncoderInterface

See issue #31812

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #31812
| License       | MIT
| Doc PR        | none

Pretty self-explanatory

Commits
-------

0e741f9 fix type hint for salt in PasswordEncoderInterface
@garak garak deleted the fix-encoder-typehint branch June 5, 2019 19:04
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