-
-
Notifications
You must be signed in to change notification settings - Fork 491
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
Update security.yaml: Simplifying regex (#2) #1395
base: main
Are you sure you want to change the base?
Conversation
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 authenticate 4.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 authenticate 5.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.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 6.4 vs 7.3diff --git a/symfony/security-bundle/6.4/config/packages/security.yaml b/symfony/security-bundle/7.3/config/packages/security.yaml
index 367af25..6726a5c 100644
--- a/symfony/security-bundle/6.4/config/packages/security.yaml
+++ b/symfony/security-bundle/7.3/config/packages/security.yaml
@@ -7,7 +7,7 @@ security:
users_in_memory: { memory: null }
firewalls:
dev:
- pattern: ^/(_(profiler|wdt)|css|images|js)/
+ pattern: ^/_profiler|_wdt|assets|build/ # `assets` is for AssetMapper; `build` is for Webpack Encore
security: false
main:
lazy: true |
Head branch was pushed to by a user without write access
users_in_memory: { memory: null } | ||
firewalls: | ||
dev: | ||
pattern: ^/_profiler|_wdt|assets|build/ # `assets` is for AssetMapper; `build` is for Webpack Encore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pattern: ^/_profiler|_wdt|assets|build/ # `assets` is for AssetMapper; `build` is for Webpack Encore | |
pattern: ^/(_profiler|_wdt|assets|build)/ # `assets` is for AssetMapper; `build` is for Webpack Encore |
Shouldn't this be with parentheses instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not needed if in the union is the full pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but here we have ^/
and /
that are not part of the union right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @mtarld is right; https://www.php.net/manual/en/regexp.reference.alternation.php doesn't say anything about the alternative branches stopping at special characters. So currently, the 4 alternatives are:
^/_profiler
_wdt
assets
build/
But it "works" nevertheless, since _wdt
matches anything like /_wdt/whatever...
;-)
So the way to go IMO is to bring back the parentheses.
However, one thing is irritating me: While testing this, I commented out that line in my security.php
, and the web debug toolbar is still showing (and all assets loading) in DEV environment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You commented out that single line or the whole dev firewall?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh correct, parenthesis are required because it's not the full pattern. The /.../
confused me into thinking that they were pattern delimiters 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ThomasLandauer try having an access_control rule requiring some authentication for all pages except the login page. When accessing the login page, the toolbar would not load as it would suddenly require authentication. That's why we have this dev
firewall disabling authentication for dev URLs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks, so this line is only needed if there's an access_control
? And you're assuming that most people have one?
So what about adding some comment here, like:
Exception for the web debug toolbar and assets (only needed if you have a broad
access_control
):
About the (missing) delimiter: This irritated me too. The best solution would probably be to include the delimiter here in the string. But that would be a breaking change, which is probably not worth it. But what about at least adding some comment like:
The delimiter
xxx
is added automatically
This might be helpful if the delimiter happens to appear in somebody's path (cause they'd have to escape it then).
=> So which delimiter is it?
As requested at #1392 (review)