-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[BC Break] Change of logout path access control behavior #28058
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
Comments
hmm, #24805 made the logout listener the last one. But the access listener is also special (it is not about authenticating, but about authorizing), so I think this one should have been kept after the logout one. |
ping @MatTheCat maybe |
Was it intentional to not take access control into account on logout? Seems like a bug to me. |
I'm divided, we must specify an access rule for any login path so the original behavior for logout sounds inconsistent, but I'm not sure it deserves such a behavior change. |
The current behaviour is a bug to me. What would be the point in not allowing the user to log out (and that's the only affect you could achieve by defining access control rules for this path, right?)? We cannot prevent the user from logging out in most cases anyway (they can delete cookies for example). |
I'm on it. |
@xabbuh the point would be to do what is actually expected. Everyone must ensure their login form is under a public firewall but it makes no sense authenticating someone on a login form. That's just the way it works and I don't see why the access control on logout would be an exception. |
See #28100 |
This PR was merged into the 2.8 branch. Discussion ---------- [Security] Call AccessListener after LogoutListener | Q | A | ------------- | --- | Branch? | 2.8 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #28058 | License | MIT | Doc PR | n/a Commits ------- 44dbea6 [Security] Call AccessListener after LogoutListener
Symfony version(s) affected: Reproduced with 2.7.47 (LTS) and 3.4.10 (LTS). Probably other versions where PR #24805 was merged are affected as well.
Description
Apparently, access control behavior for the logout path was changed in bugfix releases of Symfony, I suppose this is caused by #24805.
In Symfony 2.7.46, the logout path was not considered by the
access_control
rules defined insecurity.yml
. Starting from 2.7.47 it is, so an "Access Denied" exception is thrown if the logout path is not allowed.Also see hslavich/OneloginSamlBundle#53
How to reproduce
This example reproduces this issue with a minimal Symfony 2.7 application created with the Symfony installer.
user
, passwordpass
=> you see an example page with a logout linkIf you change composer.json to
"symfony/symfony": "2.7.46"
, runcomposer update
and test again, the last step (logout) is successful.Possible Solution
IMHO, two approaches would be possible:
access_control
configuration before), but clearly document the change in the upgrade guides for all supported versions.Additional context
app/config/security.yml
:The text was updated successfully, but these errors were encountered: