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

[Security] Simplifying the DEV firewall's pattern#20794

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

Open
ThomasLandauer wants to merge3 commits intosymfony:6.4
base:6.4
Choose a base branch
Loading
fromThomasLandauer:patch-17

Conversation

ThomasLandauer
Copy link
Contributor

@ThomasLandauerThomasLandauer commentedMar 21, 2025
edited
Loading

Page:https://symfony.com/doc/6.4/security.html#the-firewall

Reasons:

Question:
Shouldn't thisdev firewall be loaded in DEV environment only? (i.e. under something likewhen@dev)

Page:https://symfony.com/doc/6.4/security.html#the-firewallReasons:* The inner parentheses `_(profiler|wdt)` are overly complicated* AssetMapper recommends to have all assets under `/asset/`:https://symfony.com/doc/6.4/frontend/asset_mapper.html
@carsonbotcarsonbot added this to the6.4 milestoneMar 21, 2025
@carsonbotcarsonbot changed the title[Security]: Simplifying the DEV firewall's pattern[Security] : Simplifying the DEV firewall's patternMar 21, 2025
security.rst Outdated
@@ -497,7 +497,7 @@ will be able to authenticate (e.g. login form, API token, etc).
# the order in which firewalls are defined is very important, as the
# request will be handled by the first firewall whose pattern matches
dev:
pattern: ^/(_(profiler|wdt)|css|images|js)/
pattern: ^/(_profiler|_wdt|assets)/
Copy link
Member

Choose a reason for hiding this comment

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

OskarStark and wouterj reacted with thumbs up emoji
Copy link
ContributorAuthor

@ThomasLandauerThomasLandauerMar 22, 2025
edited by javiereguiluz
Loading

Choose a reason for hiding this comment

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

Well, then let's change it there too :-)symfony/recipes#1395

@xabbuh
Copy link
Member

Question:
Shouldn't thisdev firewall be loaded in DEV environment only? (i.e. under something likewhen@dev)

The security config is not merged between environments. So you would have to repeat everything for thedev environment.

@OskarStarkOskarStark changed the title[Security] : Simplifying the DEV firewall's pattern[Security] Simplifying the DEV firewall's patternMar 22, 2025
ThomasLandauer added a commit to ThomasLandauer/recipes that referenced this pull requestMar 22, 2025
@ThomasLandauer
Copy link
ContributorAuthor

Is this true forall parts of the config?
Cause athttps://symfony.com/doc/current/security/passwords.html#configuring-a-password-hasher (green box) it's recommended to reconfigure the password hasher inconfig/packages/test/security.php, and I did this inconfig/packages/security.php like this:

if ('test' ===$containerConfigurator->env()) {// ...}

@wouterj
Copy link
Member

wouterj commentedMar 22, 2025
edited
Loading

Is this true for all parts of the config?

Not to all parts, and some parts behave differently. We don't merge configuration fromsecurity.role_hierarchy andsecurity.password_hashers, and we don't allow new items insecurity.firewalls (i.e. you may change options of the firewalls, but you can't add new firewalls).


About this PR, I think it makes sense, but let's wait for the recipe to be accepted as the documentation have to be in sync with the generated recipes.

@wouterjwouterj modified the milestones:6.4,7.3Mar 22, 2025
@wouterjwouterj added the Waiting Code MergeDocs for features pending to be merged labelMar 22, 2025
@carsonbotcarsonbot modified the milestones:7.3,nextMar 22, 2025
@@ -497,7 +497,7 @@ will be able to authenticate (e.g. login form, API token, etc).
# the order in which firewalls are defined is very important, as the
# request will be handled by the first firewall whose pattern matches
dev:
pattern: ^/(_(profiler|wdt)|css|images|js)/
pattern: ^/_profiler|_wdt|assets|build/ # `assets` is for AssetMapper; `build` is for Webpack Encore
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
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

@@ -529,8 +529,8 @@ will be able to authenticate (e.g. login form, API token, etc).
<!-- the order in which firewalls are defined is very important, as the
request will be handled by the first firewall whose pattern matches -->
<firewall name="dev"
pattern="^/(_(profiler|wdt)|css|images|js)/"
security="false"/>
pattern="^/_profiler|_wdt|assets|build/"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pattern="^/_profiler|_wdt|assets|build/"
pattern="^/(_profiler|_wdt|assets|build)/"

@@ -555,7 +555,7 @@ will be able to authenticate (e.g. login form, API token, etc).
// the order in which firewalls are defined is very important, as the
// request will be handled by the first firewall whose pattern matches
$security->firewall('dev')
->pattern('^/(_(profiler|wdt)|css|images|js)/')
->pattern('^/_profiler|_wdt|assets|build/') // `assets` is for AssetMapper; `build` is for Webpack Encore
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
->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

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@xabbuhxabbuhxabbuh left review comments

Assignees
No one assigned
Labels
SecurityStatus: Needs ReviewWaiting Code MergeDocs for features pending to be merged
Projects
None yet
Milestone
next
Development

Successfully merging this pull request may close these issues.

4 participants
@ThomasLandauer@xabbuh@wouterj@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp