[symfony/security-bundle] set default algorithm to plaintext #1379
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the PR 😍 How to test these changes in your application
Diff between recipe versionsIn order to help with the review stage, I'm in charge of computing the diff between the various versions of patched recipes. symfony/security-bundle3.3 vs 4.4diff --git a/symfony/security-bundle/3.3/config/packages/security.yaml b/symfony/security-bundle/4.4/config/packages/security.yaml
index f7ae4b7..811681e 100644
--- a/symfony/security-bundle/3.3/config/packages/security.yaml
+++ b/symfony/security-bundle/4.4/config/packages/security.yaml
@@ -7,7 +7,7 @@ security:
pattern: ^/(_(profiler|wdt)|css|images|js)/
security: false
main:
- anonymous: true
+ anonymous: lazy
provider: users_in_memory
# activate different ways to authenticate4.4 vs 5.1diff --git a/symfony/security-bundle/4.4/config/packages/security.yaml b/symfony/security-bundle/5.1/config/packages/security.yaml
index 811681e..0e4cf3d 100644
--- a/symfony/security-bundle/4.4/config/packages/security.yaml
+++ b/symfony/security-bundle/5.1/config/packages/security.yaml
@@ -7,7 +7,8 @@ security:
pattern: ^/(_(profiler|wdt)|css|images|js)/
security: false
main:
- anonymous: lazy
+ anonymous: true
+ lazy: true
provider: users_in_memory
# activate different ways to authenticate5.1 vs 5.3diff --git a/symfony/security-bundle/5.1/config/packages/security.yaml b/symfony/security-bundle/5.3/config/packages/security.yaml
index 0e4cf3d..789a9ac 100644
--- a/symfony/security-bundle/5.1/config/packages/security.yaml
+++ b/symfony/security-bundle/5.3/config/packages/security.yaml
@@ -1,5 +1,9 @@
security:
- # https://symfony.com/doc/current/security.html#where-do-users-come-from-user-providers
+ enable_authenticator_manager: true
+ # https://symfony.com/doc/current/security.html#registering-the-user-hashing-passwords
+ password_hashers:
+ Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface: 'auto'
+ # https://symfony.com/doc/current/security.html#loading-the-user-the-user-provider
providers:
users_in_memory: { memory: null }
firewalls:
@@ -7,12 +11,11 @@ security:
pattern: ^/(_(profiler|wdt)|css|images|js)/
security: false
main:
- anonymous: true
lazy: true
provider: users_in_memory
# activate different ways to authenticate
- # https://symfony.com/doc/current/security.html#firewalls-authentication
+ # https://symfony.com/doc/current/security.html#the-firewall
# https://symfony.com/doc/current/security/impersonating_user.html
# switch_user: true
@@ -22,3 +25,16 @@ security:
access_control:
# - { path: ^/admin, roles: ROLE_ADMIN }
# - { path: ^/profile, roles: ROLE_USER }
+
+when@test:
+ security:
+ password_hashers:
+ # By default, password hashers are resource intensive and take time. This is
+ # important to generate secure password hashes. In tests however, secure hashes
+ # are not important, waste resources and increase test times. The following
+ # reduces the work factor to the lowest possible values.
+ Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface:
+ algorithm: auto
+ cost: 4 # Lowest possible value for bcrypt
+ time_cost: 3 # Lowest possible value for argon
+ memory_cost: 10 # Lowest possible value for argon
diff --git a/symfony/security-bundle/5.1/manifest.json b/symfony/security-bundle/5.3/manifest.json
index 5d8527e..4a48e0c 100644
--- a/symfony/security-bundle/5.1/manifest.json
+++ b/symfony/security-bundle/5.3/manifest.json
@@ -5,5 +5,8 @@
"copy-from-recipe": {
"config/": "%CONFIG_DIR%/"
},
- "aliases": ["security"]
+ "aliases": ["security"],
+ "conflict": {
+ "symfony/framework-bundle": "<5.3"
+ }
}5.3 vs 6.0diff --git a/symfony/security-bundle/5.3/config/packages/security.yaml b/symfony/security-bundle/6.0/config/packages/security.yaml
index 789a9ac..367af25 100644
--- a/symfony/security-bundle/5.3/config/packages/security.yaml
+++ b/symfony/security-bundle/6.0/config/packages/security.yaml
@@ -1,5 +1,4 @@
security:
- enable_authenticator_manager: true
# https://symfony.com/doc/current/security.html#registering-the-user-hashing-passwords
password_hashers:
Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface: 'auto'6.0 vs 6.4diff --git a/symfony/security-bundle/6.0/config/packages/security.yaml b/symfony/security-bundle/6.4/config/packages/security.yaml
index 367af25..b048fec 100644
--- a/symfony/security-bundle/6.0/config/packages/security.yaml
+++ b/symfony/security-bundle/6.4/config/packages/security.yaml
@@ -30,10 +30,11 @@ when@test:
password_hashers:
# By default, password hashers are resource intensive and take time. This is
# important to generate secure password hashes. In tests however, secure hashes
- # are not important, waste resources and increase test times. The following
- # reduces the work factor to the lowest possible values.
+ # are not important, waste resources and increase test times.
Symfony\Component\Security\Core\User\PasswordAuthenticatedUserInterface:
- algorithm: auto
- cost: 4 # Lowest possible value for bcrypt
- time_cost: 3 # Lowest possible value for argon
- memory_cost: 10 # Lowest possible value for argon
+ algorithm: plaintext # disable hashing all together
+ # You can also use the following configuration to use the lowest possible values for bcrypt and argon:
+ # algorithm: auto
+ # cost: 4 # Lowest possible value for bcrypt
+ # time_cost: 3 # Lowest possible value for argon
+ # memory_cost: 10 # Lowest possible value for argon
diff --git a/symfony/security-bundle/6.4/config/routes/security.yaml b/symfony/security-bundle/6.4/config/routes/security.yaml
new file mode 100644
index 0000000..f853be1
--- /dev/null
+++ b/symfony/security-bundle/6.4/config/routes/security.yaml
@@ -0,0 +1,3 @@
+_security_logout:
+ resource: security.route_loader.logout
+ type: service |
|
I'd need a better rationale to be convinced. plaintext is bad practice to me, even in this case, because some ppl will copy/paste and 💥 |
|
The doc state this as a « tip » |
|
Yeah interesting thing to think about. I agree with the copy paste and can be damaging, although we have explicitly set it in test env and have the default case set as well. In our case we are working with Symfony for 8 years and only recently discover this feature and made our pipelines faster. We could find middle ground and comment out plaintext and show the other options like we had before? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I think its nice to have default hashing set to
plaintextand show the option to have lowest configuration the docs also start with mention ofplaintextsee: https://symfony.com/doc/6.4/security/passwords.html#:~:text=algorithm%3A%20plaintext%20%23%20disable%20hashing%20(only%20do%20this%20in%20tests!)