{"action":"created","issue":{"url":"https://api.github.com/repos/symfony/symfony/issues/19452","repository_url":"https://api.github.com/repos/symfony/symfony","labels_url":"https://api.github.com/repos/symfony/symfony/issues/19452/labels{/name}","comments_url":"https://api.github.com/repos/symfony/symfony/issues/19452/comments","events_url":"https://api.github.com/repos/symfony/symfony/issues/19452/events","html_url":"https://github.com/symfony/symfony/pull/19452","id":167959979,"number":19452,"title":"Remove the new SecurityUserValueResolver","user":{"login":"weaverryan","id":121003,"avatar_url":"https://avatars.githubusercontent.com/u/121003?v=3","gravatar_id":"","url":"https://api.github.com/users/weaverryan","html_url":"https://github.com/weaverryan","followers_url":"https://api.github.com/users/weaverryan/followers","following_url":"https://api.github.com/users/weaverryan/following{/other_user}","gists_url":"https://api.github.com/users/weaverryan/gists{/gist_id}","starred_url":"https://api.github.com/users/weaverryan/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/weaverryan/subscriptions","organizations_url":"https://api.github.com/users/weaverryan/orgs","repos_url":"https://api.github.com/users/weaverryan/repos","events_url":"https://api.github.com/users/weaverryan/events{/privacy}","received_events_url":"https://api.github.com/users/weaverryan/received_events","type":"User","site_admin":false},"labels":[{"url":"https://api.github.com/repos/symfony/symfony/labels/Feature","name":"Feature","color":"02e10c"},{"url":"https://api.github.com/repos/symfony/symfony/labels/Status:%20Needs%20Review","name":"Status: Needs Review","color":"ededed"}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":1,"created_at":"2016-07-27T21:23:04Z","updated_at":"2016-07-27T21:55:52Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/symfony/symfony/pulls/19452","html_url":"https://github.com/symfony/symfony/pull/19452","diff_url":"https://github.com/symfony/symfony/pull/19452.diff","patch_url":"https://github.com/symfony/symfony/pull/19452.patch"},"body":"| Q             | A\r\n| ------------- | ---\r\n| Branch?       | master\r\n| Bug fix?      | no\r\n| New feature?  | yes\r\n| BC breaks?    | no (the feature hasn't been released yet)\r\n| Deprecations? | no\r\n| Tests pass?   | yes\r\n| Fixed tickets | n/a\r\n| License       | MIT\r\n| Doc PR        | n/a\r\n\r\nHi guys!\r\n\r\nThis is a revert for #18510 (ping @iltar), which is a nice idea, but will have some big practical implications:\r\n\r\n1) You are only allowed to type-hint the argument with `UserInterface` exactly. For the 90% of Symfony project's that user a User entity for their User, this will be weird: I'll receive a `UserInterface`, that immediately call methods on it that aren't in the interface (and also, your IDE won't give you auto-completion). And as #18510 mentions, we can't allow people to type-hint their concrete `User` class, because this will conflict with SensioFWExtraBundle ParamConverter if there is a user id in the URL\r\n\r\n2) Deprecating and removing `$this->getUser()` in a controller is a step back. Where we can, let's make controllers and services act *more* like each other. You can't call `$this->getUser()` in a service, but at least if you look at this method in `Controller`, you can see that it uses `security.token_storage` - which is how you will get the User object if you need it from within services.\r\n\r\nSorry for being late on this original issue! It looked good to me at first :).\r\n\r\nCheers!"},"comment":{"url":"https://api.github.com/repos/symfony/symfony/issues/comments/235733884","html_url":"https://github.com/symfony/symfony/pull/19452#issuecomment-235733884","issue_url":"https://api.github.com/repos/symfony/symfony/issues/19452","id":235733884,"user":{"login":"iltar","id":1754678,"avatar_url":"https://avatars.githubusercontent.com/u/1754678?v=3","gravatar_id":"","url":"https://api.github.com/users/iltar","html_url":"https://github.com/iltar","followers_url":"https://api.github.com/users/iltar/followers","following_url":"https://api.github.com/users/iltar/following{/other_user}","gists_url":"https://api.github.com/users/iltar/gists{/gist_id}","starred_url":"https://api.github.com/users/iltar/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/iltar/subscriptions","organizations_url":"https://api.github.com/users/iltar/orgs","repos_url":"https://api.github.com/users/iltar/repos","events_url":"https://api.github.com/users/iltar/events{/privacy}","received_events_url":"https://api.github.com/users/iltar/received_events","type":"User","site_admin":false},"created_at":"2016-07-27T21:55:52Z","updated_at":"2016-07-27T21:55:52Z","body":"While I do not disagree with your points, I would like to additionally add some comments:\r\n\r\n> For the 90% of Symfony project's that user a User entity for their User\r\n\r\nThis is pretty much a practice because it's a recommended path via the FOSUserBundle, which is more of a pain than a useful bundle lately if you ask me (A lot of questions on `#symfony` are related to this). It makes the security look more complex than it actually is.\r\n\r\nI know that @wouterj was trying to phase out the security user completely because of its redundancy in the Token. So maybe phasing out the user completely would definitely be a good option which could indeed revert this feature.\r\n\r\n>  this will be weird: I'll receive a UserInterface, that immediately call methods on it that aren't in the interface (and also, your IDE won't give you auto-completion).\r\n\r\n100% of the same behavior as `$this->getUser()` so that won't really make a difference imo.\r\n\r\n> we can't allow people to type-hint their concrete User class, because this will conflict with SensioFWExtraBundle ParamConverter if there is a user id in the URL\r\n\r\nYet another issue of having your [Entity as Security User][blog post]. \r\n\r\n> Where we can, let's make controllers and services act more like each other.\r\n\r\nThe `UserInterface` would give you the ability for a consistent method to retrieve the user regardless of Controller as a Service or simply extending the base controller.\r\n\r\n> you can see that it uses security.token_storage - which is how you will get the User object if you need it from within services.\r\n\r\nI think this is indeed a fair point (besides of some people not using an IDE and asking this in `#symfony` anyway). While I agree that looking in the code is a good thing to understand what's going on, the `Request` has the same challenge. As long as the documentation explains everything, it should be fine. Symfony has (imo) top quality documentation so it shouldn't be hard for people to find the solution. Even if people manage to find out it's the `security.token_storage`, there's a ton of other challenges they face which may lead to not even getting a token when you expect to have one.\r\n\r\n----\r\n\r\nSo with this comments added, I have no hard feelings if it gets removed. I know that a bunch of people would like the feature, but I don't want it to become more complex for the majority. \r\n\r\n[blog post]: http://stovepipe.systems/post/decoupling-your-security-user"}}
{"actor":{"display_login":"iltar"}}