[keycloak-dev] Groups design

[Bill Burke]

On 8/13/2015 10:17 AM, Stian Thorgersen wrote:
>
>
> ----- Original Message -----
>> From: "Bill Burke" <bburke at redhat.com>
>> To: "Stian Thorgersen" <stian at redhat.com>
>> Cc: keycloak-dev at lists.jboss.org
>> Sent: Thursday, 13 August, 2015 4:03:18 PM
>> Subject: Re: [keycloak-dev] Groups design
>>
>>
>>
>> On 8/13/2015 9:32 AM, Stian Thorgersen wrote:
>>
>>>>>> * Groups can be members of one or more groups
>>>>>> * Users can be members of one or more groups
>>>>>> * Users inherit attributes of the groups they belong to.
>>>>>> * UserModel now has a getGroups(), hasGroup(), grantGroup(),
>>>>>> deleteGroup()
>>>>>> * Similar to default roles, we also have default groups.
>>>>>
>>>>> If we add default groups we don't need default roles and should remove
>>>>> them.
>>>>>
>>>>
>>>> Not sure I agree.  Users may not want roles.
>>>
>>> You mean may not want groups? I just don't like maintaining multiple ways
>>> to achieve the same thing - extra work for us and complicates
>>> documentation, etc..
>>>
>>
>> Yeah, sorry, users just might not create any groups, plus its backward
>> compatible to keep default roles.  The extra work to add default groups
>> is simple.  Its just duplication of the equivalent role code.
>
> It's still extra stuff that needs documenting/explaining and for users to understand. Having multiple things that does the "same thing" is a good way of making software complicated IMO.
>

Its not the same thing.  Its a convenience thing as with your proposal 
you have the extra step of creating the default group.

>>
>>> Why not just have a default group? Then users can add whatever roles and
>>> other groups they want to it?
>>>
>>
>> I don't like a "default group" for the same reason I don't like these
>> pseudo "built in" clients.
>
> I agree
>
> But, if someone wants to add some default roles to a user, why would they care if it's done through adding a default group and adding roles to that, rather than adding default roles directly? If we remove composite roles, it more or less makes the default roles feature a bit useless in either case, so would be better to use groups for it.
>

Why would they care?  Its an extra step.

>>
>>
>>>> Not really our problem.  If somebody does an overcomplicated design,
>>>> that's their problem.
>>>
>>> I think it is. It would be very hard for a user to interpret what would end
>>> up in the token even with relatively simple groups.
>>>
>>
>> Still, I dont think its our problem.  They can choose not to use the
>> feature.
>
> I disagree - KC is supposed to be easy to use, that should include the advanced features as well. We shouldn't create something that by design would potential be hard to use.
>

I don't think a Group mapper would conflict with other Group mappers as 
they would generally be concerned with mapping attribute and roles that 
are defined within the Group.

>>
>>>>
>>>>> * Shouldn't token format be defined by a client, not by groups? A client
>>>>> will expect a token is a certain format, but if it's dependent on what
>>>>> group a user belongs to all bets is off.
>>>>>
>>>>
>>>> Probably depends on the app.  But that's a good point.  Just seem cool
>>>> to be able to assign behavior to a group or even a role, rather than
>>>> assigning behavior to the client.
>>>
>>> I'd rather think that groups are the source of the information. So the
>>> available claims are:
>>>
>>> * Users roles and attributes
>>> * Groups roles and attributes
>>>
>>> But, then you still have a set of protocol mappers (either specified on a
>>> per-client as now, or ability to share these) that takes these claims and
>>> adds it to the token.
>>>
>>
>> Another way of looking at it is that the group defines of how attributes
>> and roles look inside the claim/assertion, rather than the client
>> defining how an attribute looks.
>>
>> I'm not entirely convinced is a great thing to have, but it does seem
>> like it would be useful.
>
> I'm not at all convinced ;)
>
> It'll be more important to be able to share a set of protocol mappers between clients. So a client should be able to define it's own protocol mappers, and it should be possible to define protocol mappers groups (or whatever it's called) at the realm level and re-use those. One issue with a shared protocol mappers group is that would define the "token format", but you then loose the ability to define what attributes/claims each client should retrieve.
>

I think defining a MapperGroup that could be shared between clients is a 
good thing.  Maybe a better name would be a ClientTemplate or something 
in which you could define not only mappers, but also protocol settings 
and scope.  I think this is orthogonal to Group mappers though...

Again, I don't think a Group mapper would conflict with other Group 
mappers as they would generally be concerned with mapping attribute and 
roles that are defined within the Group.

Group mappers are just a different type of association.  Again, allowing 
you to trigger behavior via a Group than by a Client.


>>
>>
>>>>
>>>>>>
>>>>>> Questions:
>>>>>> * Do we want to expand the concept of a Group so that clients and
>>>>>> identity brokers can belong to a Group? Or just create a separate
>>>>>> composite structure for this?
>>>>>
>>>>> Not sure we need that at all. Can't identity brokers and clients just use
>>>>> mappings to achieve the required effect? Or am I confusing what effect
>>>>> this would have.
>>>>>
>>>>
>>>> The concept of associating mappers to a Group isn't required, its just a
>>>> different way of attacking the problem.
>>>
>>> I don't like alternatives ;)
>>>
>>
>> I don't think it is an alternative.  It is just allowing the Group
>> decide how to map things.
>
> Can you elaborate on it a bit more then? As I see it:
>
> * Identity brokers should be able to add a mapper that defines what groups a user belongs to
> * Clients should be able to use a "group" mapper to add a group to the token. The group mapper would add the attributes and roles associated with the group to the token.
>

You would have to define a mapper like

Has Group:
Attribute name:
Maps to claim name:

Instead of just creating a mapper on the Group of

Attribute name:
Mapps to Claim Name:



-- 
Bill Burke
JBoss, a division of Red Hat
http://bill.burkecentral.com