ZF-Commons / zfc-rbac

Role-based access control module to provide additional features on top of Zend\Permissions\Rbac
BSD 3-Clause "New" or "Revised" License
181 stars 111 forks source link

AssertionAggregate? #217

Open aeneasr opened 10 years ago

aeneasr commented 10 years ago

https://github.com/zendframework/zf2/pull/5628 should we do this for zfc-rbac as well?

bakura10 commented 10 years ago

That's interesting. Could you try a PR ? :)

aeneasr commented 10 years ago

Yeah I'll try to have a close look into that next week. I also wanted to create a new PR for the AuthorizationResult so maybe those features combined could trigger a new version for zfc-rbac

bakura10 commented 10 years ago

Any news about this? I'm pretty curious about how this thing could work: ).

aeneasr commented 10 years ago

I'm so sorry I have zero time, I'll try to look into it over the weekend

aeneasr commented 10 years ago

Sorry, I won't be able to implement this over the weekend. Our release is 2 weeks away and there is to much to be done yet. Feel free to propose your own idea :)

bakura10 commented 10 years ago

I'll wait for you to do it :). It's not in a hurry anyway.

bakura10 commented 10 years ago

Hi,

I'm giving some more thought. Here is how I think about it.

  1. Instead of registering individual assertions, you register an aggregate:
'assertion_map' => [
    'Application\Assertion\UserAggregate'
]

(maybe we could create a new 'assertion_aggregate', but I'd like avoid the multiplication of options. Assertion aggregate interface is made of one method: registerAssertions, that receive the AuthorizationService:

class UserAggregate implements AssertionAggregateInterface
{
    public function registerAssertions(AuthorizationService $authorizationService)
    {
        $authorizationService->setAssertion('user.read', [$this, 'canReadUser']);
        $authorizationService->setAssertion('user.delete', [$this, 'canDeleteUser']);
    }

    public function canReadUser(AuthorizationService $authService, $context = null)
    {
    }

    public function canDeleteUser(AuthorizationService $authService, $context = null)
    {
    }
}

The advantages of this approach: you avoid the multiplication of files and simplify your config files. Disadvantages: the authorization service needs to instantiate each aggregate and cannot benefit anymore from the lazy load.

@arekkas @danizord what do you think ? :)

aeneasr commented 10 years ago

My last post was written late and therefore, well, bs ;) I've deleted it in case you wonder.

@bakura10 : What I don't get, you're bypassing the AssertionPluginManager completely? Or did I miss something?

edit:// Ah nevermind, the UserAggregate get's instantiated by the APM as well, right? I think both options should be available, but combined in one array:

'assertion_map' => [
    'Application\Assertion\UserAggregate',
    'user.admin' => 'Application\Assertion\AdminAssertion'
]
aeneasr commented 10 years ago

However, this kind of multitype parameter thingy is not cool. ;)

bakura10 commented 10 years ago

I agree. Let's delay that for now. I don't have a good idea about how to do that cleanly.

aeneasr commented 10 years ago

I agree. I'll try to come up with a solution for the AuthorizationResultAware Assertions. I think that has a higher priority.

demichl68 commented 8 years ago

Any updates here? Or is there a suggested way of tying multiple assertions to a single permission?