[keycloak-dev] Groups design

[Stian Thorgersen]

----- 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:35:50 PM
> Subject: Re: [keycloak-dev] Groups design
> 
> 
> 
> 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.

IMO it's the same thing. Having two options increases the amount of concepts a user has to understand, increases documentation, etc..

More importantly if we remove composite roles and replace fully with groups, then default roles are much less usable.

My vote is to remove default roles and just have default groups, then focus on making it easy to do the required setup around it.

> 
> >>
> >>> 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.

Doesn't have to be, we can make it easy to do through the admin console

> 
> >>
> >>
> >>>> 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.

Sure, but a user could be member of many groups, where each groups in turn could be members of other groups. IMO this will just end up messy even with a relatively simple group hierarchy.

> 
> >>
> >>>>
> >>>>> * 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.

I think I've misunderstood what you've suggested. Is it correct that:

* A realm has one or more "group mappers"
* A client belongs to one or more "group mappers"

If so, it sounds good. I think a flat structure is good enough though (no a group mapper can have one or more group mappers etc..).

Group mappers is probably a horrible name, so client template sounds much better.

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

I'm not following

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