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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
Correctly handle denied access in the firewall #6805
Conversation
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.
Sure needs some manual testing which I currently don't have the time for but from a logical standpoint, the code looks good to me.
9bb19bd
to
272f0ad
Compare
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.
The code works fine, however, on the 401 page, I can log in as a different user now. Shouldn鈥榯 we remove the username
field in this case?
You are right. Username field removed in 9dc8d9e I also manually tested the two-factor authentication, which was bumpy even for the regular login 馃檲 |
Co-authored-by: Leo Feyer <1192057+leofeyer@users.noreply.github.com>
Thank you @aschempp. |
This now fixes and improves a lot of things 馃檲
ContaoLoginAuthenticator
from rendering the wrong page for login (if the user is not fully authenticated).Fixes the remember me authentication system(see Drop the custom "remember me" implementation聽#6815)AccessDeniedHandler
to render the 403 page via the firewall exception handlerThere are two "behaviour changes" I can think of
TODO:
Needs a migration for existing RememberMe tokens (the migration tool cannot migrate binary data to string value because the binary data is null-padded and therefore longer than the new string field).