Skip to content

Block login page for logged in users #951

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 2 commits into from
Closed

Block login page for logged in users #951

wants to merge 2 commits into from

Conversation

Carrancio
Copy link
Contributor

@Carrancio Carrancio commented Feb 26, 2019

This PR blocks the login page for logged in users.

@javiereguiluz
Copy link
Member

@Carrancio thanks for this contribution.

As you know, this app is "the reference Symfony app" and lots of people (not only newcomers) look into it to check how things are done. So it's very important that we only do what Symfony really promotes as a best practice.

Although your proposal is technical correct ... I wonder if this is something we should do and promote. I've never seen this in a Symfony app ... and we don't recommend this in Symfony Docs. But that doesn't mean that your proposal is wrong. I just want to wait and read more opinions about this. Thanks.

@mbabker
Copy link

mbabker commented Mar 13, 2019

I'd say this is a good practice to follow, even if not promoted in documentation. As an authenticated user, if I visit the login page and see a login form I would be mildly confused as to why I'm seeing it. So this is a check I implement in my apps.

FWIW Laravel ships a middleware in their skeleton application, enabled by default, that allows you to define routes and/or controllers that should be accessed by guests only (and this is applied to the default LoginController). Another practical example would be trying to reach https://github.com/login while logged in here on GitHub.

@javiereguiluz
Copy link
Member

@Carrancio this is now merged! Thanks ... and congrats on your first Symfony Demo contribution.

Note: while merging, we refactored the code to use the Symfony\Component\Security\Core\Security class, which is the recommended in the Symfony Docs.

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.

4 participants