[SecurityBundle] Allow for custom logout request matcher #22572
Conversation
| * | ||
| * @param RequestMatcherInterface $requestMatcher | ||
| */ | ||
| public function setRequestMatcher(RequestMatcherInterface $requestMatcher) |
There was a problem hiding this comment.
Cannot it be passed as a new optional constructor argument rather than by using a method call?
I'm not sure someone is supposed to replace the abstract service definition, so it shouldn't be an issue.
This PR was squashed before being merged into the 2.7 branch (closes #22574). Discussion ---------- [Security] Fix phpdoc logout listener | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> Separated from #22572 Commits ------- e843924 [Security] Fix phpdoc logout listener
|
Looking at the firewall configuration, which allows either Status: needs work |
|
Then again.. the logout path is needed for url generation and enables using routes (also for matching). So tend to keep it as is :) Status: needs review |
… path (ro0NL) This PR was merged into the 2.7 branch. Discussion ---------- [Security] Avoid unnecessary route lookup for empty logout path | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no-ish | Deprecations? | no | Tests pass? | yes/no | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> i first included this with #22572 where having `logout: { path: ~ }` makes more sense for disabling logout path matching/generation. But currently it's already allowed and causes an unneeded route lookup and url generation. Commits ------- 2967807 [Security] Avoid unnecessary route lookup for empty logout path
|
rebase needed |
| return $this->httpUtils->checkRequestPath($request, $this->options['logout_path']); | ||
| if (!isset($this->options['logout_path']) && null === $this->requestMatcher) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Should this condition be allowed at all? This seems invalid: you should pass one or the other. We could catch this with validation in MainConfiguration (and an exception here). We should probably also not allow both to be set.
|
Moving to 4.1. Rebase on master might be needed, where PHP 7.1 features can be used btw. |
So you can do something like
and bypass path-matching, or combine it with a custom check afterwards.
Should go after #22574 and #22584