Skip to content

Update recommended password encoder to "auto" #11767

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
Sep 24, 2019

Conversation

phansys
Copy link
Contributor

@phansys phansys commented Jun 18, 2019

Q A
Branch 4.3
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT

See https://github.com/symfony/symfony/blob/v4.3.0/UPGRADE-4.3.md#securitybundle.

@phansys phansys force-pushed the security_encoder branch from da4d91f to 1bdd1d8 Compare July 2, 2019 21:56
@OskarStark OskarStark added this to the 4.3 milestone Aug 15, 2019
@OskarStark
Copy link
Contributor

@phansys can you please rebase your PR and @chalasr can you please give us a review here? Thanks 👍

@OskarStark
Copy link
Contributor

@wouterj and @chalasr I would appreciate a review from your side here 👍🏻

Copy link
Member

@wouterj wouterj left a comment

Choose a reason for hiding this comment

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

Good job of searching through the whole documentation! I've left two comments to slightly modify some of the changes (while we recommend auto, we should not completely remove bcrypt imo).

@@ -280,12 +280,12 @@ password) so you don't have to deal with it.

.. _reference-security-bcrypt:

Using the BCrypt Password Encoder
Using the "auto" Password Encoder
Copy link
Member

Choose a reason for hiding this comment

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

Imho, we should keep this section, it is just talking about the BCrypt algorithm, and instead add a new section for the auto password encoder.

Copy link
Member

@chalasr chalasr Aug 18, 2019

Choose a reason for hiding this comment

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

The bcrypt encoder has been deprecated though. Also technically speaking auto is not an encoder, it's only a possible value for the algorithm config option which saves from having to choose between native and sodium. So I'm not fond of documenting it this way neither.

javiereguiluz added a commit that referenced this pull request Sep 24, 2019
This PR was squashed before being merged into the 4.3 branch (closes #11767).

Discussion
----------

Update recommended password encoder to "auto"

|Q            |A  |
|---          |---|
|Branch       |4.3|
|Bug fix?     |no |
|New feature? |no |
|BC breaks?   |no |
|Deprecations?|no |
|Tests pass?  |yes|
|Fixed tickets|n/a|
|License      |MIT|

See https://github.com/symfony/symfony/blob/v4.3.0/UPGRADE-4.3.md#securitybundle.

Commits
-------

fb21211 Update recommended password encoder to \"auto\"
@javiereguiluz javiereguiluz merged commit fb21211 into symfony:4.3 Sep 24, 2019
@javiereguiluz
Copy link
Member

Javier, thanks for this contribution! It's merged now.

@phansys phansys deleted the security_encoder branch September 24, 2019 16:31
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.

7 participants