contributte / menu-control

🍔 Menu and breadcrumb components for Nette framework (@nette)
MIT License
28 stars 20 forks source link

Roles as implementation of Nette\Security\IRole not string #11

Closed davidkastanek closed 9 years ago

davidkastanek commented 9 years ago

Now Roles are checked agains User roles by method Nette\Security\User::isInRole(). This method searches for given role in Nette\Security\Identity::roles. This array is meant to contain set of Nette\Security\IRole implementations, but this module work with roles as they were strings, not Nette\Security\IRole.

Could you please convert every string role into implementation of Nette\Security\IRole before giving it to method DK\Menu\Item::setAllowedForRoles?

davidkudera commented 9 years ago

Hello, please, can you tell me why it should be this way? Because wherever I look, I see only strings. Only if search just IRole, I can find something.... But not much or almost nothing..

So if people using strings, I don't thing that it should be automatically translated into IRole.

I'm also looking at Security\User code and I think that it will not be possible at all... The isInRole method uses in_array function, but if I will create IRole object, it'll be different than your IRole object so it would return false always? Not?

Please, if I'm missing something, tell me.

davidkudera commented 9 years ago

Closed by mistake ;-)

davidkastanek commented 9 years ago

First of all thank you very much for your incredible extension :) And second of all let me explain my proposition a little better. In my app an authenticated user is represented by Nette\Security\Identity object. This object has $roles property of type array, that is true. To this property one can save array of strings where each string represents given role for this user.

But I believe that this array ($roles) is meant to store an array of Nette\Security\IRole objects (AKA implementations because its just an interface).

I know most people don't implement IRole and they just use strings for this purpose but I wanted to do things right. So I created class Role which implements IRole and now my $roles property is an array of Role objects. The problem with this is that it doesn't work with your module.

Do you think you could at least accept both variant, array of strings and array of IRole implementations in your module?

Some links: https://github.com/nette/security/blob/master/src/Security/Identity.php https://github.com/nette/security/blob/master/src/Security/IRole.php

Thanks very much.

davidkudera commented 9 years ago

Thanks :)

Yep, I looked at these classes and I understand that it feels right to use IRole in setRoles method. This is without any question.

But there is that problem with not same instances. You know, you will create eg. App\Model\Entities\Role, but I'll created DK\Menu\Security\Role class. So the in_array will always return false.

davidkastanek commented 9 years ago

Wow, you are right. I didn't think of that, sorry. I wish there was \Nette\Security\Role class we could both use :-(.

Ok, I will fall back to string. I'm very sorry to bother you, thank you very much for your time and have a nice day.

davidkudera commented 9 years ago

You didn't bother me at all :)

I tried to find some solution but unfortenatelly nothing came to my mind..

davidkudera commented 9 years ago

Hi again, I was thinking about it and probably I found a solution ;-)

There could be some DK\Menu\Security\IRoleProvider interface with method eg. getRole($name). There would be also base "string implementation". Something like DK\Menu\Security\StringRoleProvider which will just return $name, nothing else.

So then you will be able to create custom IRoleProvider. Eg. App\Menu\EntityRoleProvider which will load role from somewhere and return it. But.... It would have to be exactly same instance, not just same class type.

davidkastanek commented 9 years ago

Hi, thank you for your input, but I'm not sure I follow. If I understand this correctly, instances of App\Menu\EntityRoleProvider would have to match those entities I would have in \Nette\Security\Identity::$roles? And I would hand these App\Menu\EntityRoleProvider instances over to your extension and you would check them agains contents of \Nette\Security\Identity::$roles?

At this point I'm really not sure if proper working with roles of other type that string can be achieved without Role class in Nette. But again I'm not that good in OOP design :)