security
security copied to clipboard
Create interface for User
- bug report? no
- feature request? yes
- version: 3.0
Few times I wanted to use own User
implementation, bt there is no simple way how to force Nette to use it. I'd like to have IUser
same way as IUserStorage
to be able to use my own implementation in framework.
User is registered in DIC as a service with the name therefore it should be possible to replace it with your own implementation similarly like in this thread.
Problem is, there is User class used directly in Nette code. If not in code, definitelly in comments which isn't good for IDE.
My tip: don't use the User
class at all, operate directly on IUserStorage
. It has almost the same API.
@JanTvrdik Thanks for the tip, but this doesn't solve issue in framework. User is somehow hardcoded and it would be great if it wasn't. I'm happy to make PR (or at least try it), but I want to know others opinions first.
@foxycode Actually the User class is problematic in quite a few use-cases so in advanced systems it's better to not use it at all. Rather than a PR it's better to just make your own library like I did.
@enumag Again, why not solve it directly in framework? Yes, I can make my own library. I actually already did.
I believe, if we put our heads together, we can solve it directly in framework. I train new programmers in Nette and it's confusing for them. I can't simply tell them to write own library :)
@foxycode It would become a new package separate from nette/security anyway (because of BC). In which case it doesn't matter if it is in nette namespace or not. Can't speak for @dg but I don't think he wants to add something completely new into nette.
@enumag I think we can afford BC breaks in 3.0 and this is only about adding interface. Possibly this wouldn't need BC at all. I'd like to know what @dg thinks too. Can you tell me which problems do you see with User?
this is only about adding interface
Yes, but it is an interface of a class with bad name and bad design. The goal should be to remove (or somehow fix?) the class instead of making even more stuff dependent on in.
@JanTvrdik Ok, I agree with it :) I'd like to try, but need to know what others don't like about User class. Can you tell your opinion?
Few random things that are bad about the User class.
- The name – User is usually an entity in application. The responsibility of this class is not to represent a user but to serve as some kind of manager of active user session.
- It has way to many responsibilities (that's why it cannot have a good name). It is better to use directly IAuthenticator, IUserStorage and some alternative of IAuthorizator.
- It relies on bad interface IAuthorizator (see this RFC on forum)
God, I just wanted to post the same as @foxycode.
I am upgrading a larger codebase from nette v 2.4 and it's just not possible, since we are using an extended version of User
class, bud suddenly some methods are final
😖. What is even worse, Nette\Application\UI\Presenter
class won't accept any other than Nette\Security\User
object and the getUser
method is (yet again) final
.
So... how do we extend the functionality of User
object?? Please, this is a framework. Why do we constantly need to hit private
and final
stuff? We want to extend and bend a framework. This is where Nette gets in way. I do not wish to copypaste 1400 lines of Presenter
code just to be able to use my own User
implementation.
In particular, in my case I have some custom logic in User::getIdentity
method, why it is final, I have no idea.
Hi, any updates on this topic? It's quite old, but still it is a issue when upgrading large codebase from v2
I've just came accross this. I want NOT to use Nette\Security\User
, but I can't because if it's baked into Presenter
, UserPanel
etc.
While I agree that User
class has bad design, the thing is, that it's still in Nette and it will be probably for some time. So I think one of these solutions should be applied until the whole class is rethought:
- make
Nette\Application\UI\Presenter#getUser()
not final – that's the easiest one. You can not replaceUser
, but at least you can ensure that no one is going to use it in presenters. But e.g. User panel in Tracy still gets information fromUser
class. - make
Nette\Security\User
an interface –User
keeps working, but making it an interface, there is place for custom implementations. I think it could be actually done without BC breaks.
@dg Could you please write some thoughts to this old issue? At least for me, a big blocker NOT to make PRs is the fact, that I'll never know if the whole stuff wouldn't be rejected. And I don't want to/can't waste my time. If you say "well, yes, let's do it this way, would someone do it?", I believe more people would contribute.
Edit: just realized that non final getUser()
solves it for presenters only, which is not enough in my opinion. So I would add myself to the call for interface. 🙂
In case the interface is too much to ask or simply not desired, I suggest also removing the return type hint, so that the overriding methods can return anything, not descendants of the User
object only.
// Presenter.php
/**
* @return Nette\Security\User
*/
public function getUser() //: Nette\Security\User
{
if (!$this->user) {
throw new Nette\InvalidStateException('Service User has not been set.');
}
return $this->user;
}
This is a backwards-compatible <10 minute fix.
Simple, stop using it at all. Only hard which is to how to remove it from DI, so nobody can wire it. I have approached it using this fake factory overriding original registration from nette/security di-extension.
services:
security.user:
factory: CampApp\App\Infrastructure\NoNetteUserFactory::throwErrorWhenAccessed()
autowired: false
final class NoNetteUserFactory
{
public static function throwErrorWhenAccessed(): User
{
throw new UsageException('Hey! Nette\User is disabled in this project (as interface does not match use-cases needed in this project). Use <YourClass> instead.');
}
}
Wouldn't it then be better to completely remove it from Presenter
, extension etc.? It gets in the way.
Why not just enable overriding, which is simpler and backwards-compatible?