[keycloak-dev] Groups design

[Marek Posolda]

On 13.8.2015 16:35, Bill Burke wrote:
>
> 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.
-1 from me for this as well. What if user is member of "groupA", which 
has 10 protocol mappers for adding 10 attributes to the token. Then I 
have clientA where I want these attributes, but another clientB where I 
don't want them. Do I have a possibility to disable having those 10 
attributes in the token for clientB?
>
>>>>>> * 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...
+1 for ClientTemplate .

Marek
>
> 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:
>
>
>