Skip to content

Adding caution box about no other form allowed #17426

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

Closed
wants to merge 1 commit into from

Conversation

ThomasLandauer
Copy link
Contributor

I'm not sure about the blockquote formatting...

I'm not sure about the blockquote formatting...
@carsonbot carsonbot added this to the 5.4 milestone Nov 11, 2022
@xabbuh
Copy link
Member

xabbuh commented Nov 11, 2022

I am not sure about this change. You usually do not have a controller listening to the route configured with the check path. That’s why I do not see why there should be any form at all.

@ThomasLandauer
Copy link
Contributor Author

ThomasLandauer commented Nov 11, 2022

But if the username is wrong, the check_patth needs to display the login form, doesn't it? At least, that's what the page says at https://symfony.com/doc/current/security.html#form-login

Don't let this controller confuse you. Its job is only to render the form: the form_login authenticator will handle the form submission automatically. If the user submits an invalid email or password, that authenticator will store the error and redirect back to this controller, where we read the error (using AuthenticationUtils) so that it can be displayed back to the user.

The use case I'm looking at is a login form being part of the site's menu (i.e. present on every page). In this case it's not so obvious what to use as check_path.

BTW: "skittles" ≈ "bowling" ;-)

@xabbuh
Copy link
Member

xabbuh commented Nov 11, 2022

It redirects back to the route that is used to render the initial login form.

@javiereguiluz
Copy link
Member

Folks, what should we do here? Merge, Close, Reword? Thanks.

@OskarStark
Copy link
Contributor

Not merge from my side

@javiereguiluz
Copy link
Member

Feedback agrees on closing without merging, so let's close it. Sorry!

@ThomasLandauer ThomasLandauer deleted the patch-12 branch January 23, 2024 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants