Allow optional context for getRoles

[zionsg]

This allows the identity to return different roles for different contexts. A scenario would be a user having different roles for different projects and the project name is passed in as the context. Implements Solution 1 in #258 (comment)

[danizord]

Keep optional arguments out of the contract.

[bakura10]

Why not? It at least enforces that getRoles can receive an optional context.

[bakura10]

I like the idea of being able to pass context, however don't we have a BC here?

[zionsg]

Yes, it's a BC especially since projects would implement IdentityInterface directly, given that there is no AbstractIdentity to extend from. I did try out Solution 2 in #258 (comment) but it did not work out. The defining of permissions per role for the role provider uses simple permissions without the project name being prepended - there seemed no way to prepend the project name in the whole process. Either I missed out something or it was getting too contrived.

I am using this fork in a project right now and it was easier to setup. The User entity implements the IdentityInterface and is populated with the team roles when fetched by the mapper during authentication. Roles are divided into site roles and team/project roles.

    public function getRoles($context = null)
    {
        if (!$context) {
            return $this->getSiteRoleName();
        }

        return isset($this->teamRoles[$context]) ? $this->teamRoles[$context] : array();
    }

[bakura10]

Ok, as I told you I like the idea but I'd prefer not doing a BC here. There is high chance that Rbac package (the one that ZfcRbac uses) will be merged to ZF, so I want ZfcRbac 3 to rely on ZF package. I'd prefer not to BC now if I need to release another major version in 2 months.

Regarding the issue itself this is actually a limitation of the rbac model itself. For such cases we would need to use more advanced authorization model like CARBAC but it's definitely harder so none of us had the time to do it.

But your fix provide an easy way to make rbac a bit more flexible.

Envoyé de mon iPhone

Le 28 juil. 2015 à 05:43, Ng Kitt Horng, Zion [email protected] a écrit :

Yes, it's a BC especially since projects would implement IdentityInterface directly, given that there is no AbstractIdentity to extend from. I did try out Solution 2 in #258 (comment) but it did not work out. The defining of permissions per role for the role provider uses simple permissions without the project name being prepended - there seemed no way to prepend the project name in the whole process. Either I missed out something or it was getting too contrived.

I am using this fork in a project right now and it was easier to setup. The User entity implements the IdentityInterface and is populated with the team roles when fetched by the mapper during authentication. Roles are divided into site roles and project roles.

public function getRoles($context = null)
{
    if (!$context) {
        return $this->getSiteRoleName();
    }

    return isset($this->teamRoles[$context]) ? $this->teamRoles[$context] : array();
}

—
Reply to this email directly or view it on GitHub.

[zionsg]

Ya, understood the BC implications. Nevertheless it has been a good learning exercise for me which got me a temporary solution for my current project. Learnt more from #241 after you mentioned CARBAC. Thanks for your feedback and advice (and danizord's too)!

[danizord]

Well, maybe I'm missing something, but I don't see any BC breaks there. You are only adding an optional argument to the interface, current implementations will keep working AFAIK.

Anyway I think this logic should happen in "role provider", not in the identity object. But it's currently impossible because ZfcRbac calls getRoles() directly in the identity object. The good news is that I'm planning to propose a change on this behavior for 3.0, so that we can write our custom role providers with specific domain logic.

@bakura10 I'd say it is mergeable for 2.x, but the bad thing is that we are adding a new behavior that will likely change in 3.x. Leaving that decision for you :P

[bakura10]

Oh that would be awesome @danizord to be able to write custom role providers.

I'd prefer to not merge that for now (but I'll keep that open for reference), and we'll see the evolution of both Rbac and ZfcRbac component.

Btw any news @danizord for the Rbac stuff on ZF repo? You still need to convince Matthew and make PR to the branch for removing the non-generator stuff ^^.

[danizord]

@bakura10 I was procrastinating this one tbh, haha :P but tonight I will start writing.

[imonteiro]

I'm very interested in this topic. In my application the roles depend of which project (aka context) is selected, but I still haven't discovered how I should solve.

@zionsg Can you share a complete example?

Thanks in advance.

[zionsg]

@imonteiro I posted 2 possible solutions earlier on, of which I preferred the first. You can read it at #258 (comment)