Skip to content

Clarify target path functionality #11192

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

Clarify target path functionality #11192

wants to merge 2 commits into from

Conversation

sergiu-popa
Copy link

Explain that target_path is set from Symfony only when the user visits a secured route which will redirect him to the login. This example is for when the user visits some public routes, then the login page. After it logs in successfully, it will be redirected to that public route.

Explain that target_path is set from Symfony only when the user visits a secured route which will redirect him to the login. This example is for when the user visits some public routes, then the login page. After it logs in successfully, it will be redirected to that public route.
}
}

This listener will save the target path for the *main* firewall for the `$includedRoutes`. If a user visits `route-1` (public route), then successfully logs in, it will be redirected to that route.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, is the idea that route-1 is a protected page, but it doesn’t live under the main firewall? Or is it a public page, and there’s a login form somewhere on it?

Copy link
Author

@sergiu-popa sergiu-popa Mar 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a public page from where you can login, and you will be redirected back to that pages. In your "language": I visit symfonycasts, then I visit /courses, and when I login, I'm back at /courses; actually SFCasts does this right now.

I had the impression that Symfony does this by default... but at least following the Login Guard Authenticator documentation, it does not.

@javiereguiluz
Copy link
Member

Can't we solve this calling saveTargetPath() from the login action instead of creating an event subscriber? We do that in the Symfony Demo app:

https://github.com/symfony/demo/blob/31ce9eabcb2f4ddd408905a4a673aa61fce41851/src/Controller/SecurityController.php#L41

@sergiu-popa
Copy link
Author

Can't we solve this calling saveTargetPath() from the login action instead of creating an event subscriber? We do that in the Symfony Demo app:

https://github.com/symfony/demo/blob/31ce9eabcb2f4ddd408905a4a673aa61fce41851/src/Controller/SecurityController.php#L41

I think it's different. In that example the route is hardcoded to the administration area. In this case, the route be whatever route on the site previously visited before login.

@weaverryan, how it is done on SymfonyCasts?

return;
}

if ($request->isXmlHttpRequest()) {
Copy link
Contributor

@dbrumann dbrumann Apr 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to combine both if conditions to simplify/shorten this example a bit?

if (!$event->isMasterRequest() || $request->isXmlHttpRequest()) {
    return;
}

@@ -373,4 +373,67 @@ deal with this low level session variable. However, the
:class:`Symfony\\Component\\Security\\Http\\Util\\TargetPathTrait` utility
can be used to read (like in the example above) or set this value manually.

When the user tries to access a restricted page, it is redirected to the login page.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the user tries to access a restricted page, they are being redirected to the
login page. At that point target path will be set. After a successful login,
the user will be redirected to this previously set target path.

At that point target path is set and after a successful login, the user will
be redirected to the target path set before.

To set it on certain public routes, you can create an Event Subscriber:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think since this is a new paragraph it might be a bit difficult to track what it refers to. Maybe we should repeat target path here.

Suggested change
To set it on certain public routes, you can create an Event Subscriber:
To set the target path on public routes, you can create an Event Subscriber:

@javiereguiluz
Copy link
Member

Closing in favor of #12380.

javiereguiluz added a commit that referenced this pull request Sep 26, 2019
This PR was merged into the 4.3 branch.

Discussion
----------

Clarify target path functionality

I tried to merge #11192 but I can't because the original fork/branch was removed. This PR reproduces the original PR with the reviewers suggestions applied, so all credit goes to them. Thanks.

Commits
-------

45f0ddb Clarify target path functionality
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.

6 participants