[Security] Add an option to allow path instead of service for firewalls entry points (#39520) #41339
Conversation
johnkrovitch
force-pushed
the
feature/add-entry-point-path
branch
from
50afbac
to
7e8885a
Compare
|
Hey! I think @edefimov has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
johnkrovitch
force-pushed
the
feature/add-entry-point-path
branch
from
7e8885a
to
a19c0bd
Compare
|
Tests fixed |
|
Could we make |
|
Hi, following the previous comment of @chalasr, we can not decide on what to do : do we keep the current option The first one allows to not add a new option, which is nice, but can be a bit complicated to read and debug. The second is clearer, but it add a new option and there is already a lot of option in the security. Do you have an opinion on the topic ? Thank you very much ! |
|
@chalasr see #39520 (comment) . I think the |
johnkrovitch
force-pushed
the
feature/add-entry-point-path
branch
from
a19c0bd
to
6dfcb28
Compare
johnkrovitch
force-pushed
the
feature/add-entry-point-path
branch
from
6dfcb28
to
2f60a6a
Compare
src/Symfony/Component/Security/Http/EntryPoint/RouteAuthenticationEntryPoint.php
Outdated
Show resolved
Hide resolved
| @@ -440,6 +440,15 @@ private function createFirewall(ContainerBuilder $container, string $id, array $ | |||
| // Determine default entry point | |||
| $configuredEntryPoint = $firewall['entry_point'] ?? null; | |||
|
|
|||
| // When a entry point path is configured, it overrides the default entry point | |||
There was a problem hiding this comment.
What if one set both entry_point and entry_point_path at the firewall level? Should we forbid such mix for clarity? A test would be nice for that case btw
There was a problem hiding this comment.
Currently, if both options are set, the entry point path overrides the entry point.
It could be a good thing to throw an exception if both are set to avoid confusion and ease debug.
I try to add a test if I had the time.
johnkrovitch
force-pushed
the
feature/add-entry-point-path
branch
from
2f60a6a
to
9664ebb
Compare
johnkrovitch
force-pushed
the
feature/add-entry-point-path
branch
from
9664ebb
to
b5ba905
Compare
Hi, this PR aims to fix #39520. It creates a new option in the firewall configuration section "entry_point_path", allowing to have a common entry point using a route instead of a service id.
Thanks !