-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Clarify comment about "Retrieving the User Object" #8542
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
Conversation
References #8535 |
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.
If I remember this correctly, @weaverryan was against getting the user via class injection and he prefers to keep promoting the $this->getUser()
shortcut or long alternative based on the token, etc. So, let' wait for more comments. Thanks!
That’s correct! My vote is to only show the ->getUser() way, with a note (maybe inline comment in code?) that says you can also type-hint a UserInterface arg. |
Promote the preferred way of retrieving the object of the authenticated user
Changed that. Any particular reason not to use type-hinting in this case? Or is it just because of the better style? |
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.
Thanks @pathmissing for the fast changes! While reviewing, I noticed we need a few more - I'm glad you opened a PR around this section :).
To your question about why I prefer $this->getUser()
over type-hinting UserInterface
, it's mostly that the type-hinting option doesn't provide auto-completion on my custom User methods. Actually, neither does $this->getUser()
, but you can (and I do) add your own base-class where getUser() has the proper @return
for your User class to get auto-completion.
Thanks!
security.rst
Outdated
@@ -999,13 +999,14 @@ look like:: | |||
|
|||
use Symfony\Component\Security\Core\User\UserInterface; | |||
|
|||
public function indexAction(UserInterface $user) | |||
public function indexAction() | |||
{ | |||
if (!$this->get('security.authorization_checker')->isGranted('IS_AUTHENTICATED_FULLY')) { | |||
throw $this->createAccessDeniedException(); | |||
} |
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.
Bah, now that I look a bit more, this is a bit of a mess :/ still. I'd like to propose a few more changes:
-
In the above code, use the simpler
$this->denyAccessUnlessGranted()
. -
Below, user the simpler
$user = $this->getUser()
. -
Move the suggestion about UserInterface to below, and I'll suggest a wording tweak too :)
$user = $this->getUser();
// or you can also type-hint a method argument with UserInterface: e.g. "UserInterface $user"
- Below (you need to expand the code-blocks to see it), there is a
versionadded:: 3.2
. I think we should shorten that quite a bit:
.. versionadded:: 3.2
The ability to get the user by type-hinting an argument with UserInterface
was introduced in Symfony 3.2.
@weaverryan Thank you for proposing these changes, I implemented them in my latest commit. Creating a base class which returns your custom User for autocompletion is actually a really nice idea, which I am going to implement myself. Thanks for that :) |
Thank you Marcus! |
No description provided.