[DISCUSS] - Dynamic group memberships

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[DISCUSS] - Dynamic group memberships

Colm O hEigeartaigh
Hi all,

In UserTO, we maintain a list of static group memberships via the
MembershipTO class. However, for dynamic group membership, we just maintain
a list of Strings, which correspond to the Id of the group.

So for example, if we do a GET on a given user we could see something like:

"memberships":[{"type":"Membership","rightType":"GROUP","rightKey":"06fba4e9-a013-4ffc-bba4-e9a0133ffc79","groupName":"tempgroup"
"dynGroups":["06fba4e9-a013-4ffc-bba4-e9a0133ffc79"]

One drawback to this approach is that if we want to find the names of the
groups a user is a member of, including dynamic membership, then we need to
make a separate request with a fiql search query to return the groups, and
then parse the response appropriately.

Ideally we would be able to see the group names for the dynamic case as
well when retrieving the users - this means only one call would be needed.
Would it be possible to update UserTO with a list of MembershipTOs that
correspond to the dynamic case?

Colm.

--
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [DISCUSS] - Dynamic group memberships

ilgrosso
Administrator
On 20/07/2017 13:40, Colm O hEigeartaigh wrote:

> Hi all,
>
> In UserTO, we maintain a list of static group memberships via the
> MembershipTO class. However, for dynamic group membership, we just maintain
> a list of Strings, which correspond to the Id of the group.
>
> So for example, if we do a GET on a given user we could see something like:
>
> "memberships":[{"type":"Membership","rightType":"GROUP","rightKey":"06fba4e9-a013-4ffc-bba4-e9a0133ffc79","groupName":"tempgroup"
> "dynGroups":["06fba4e9-a013-4ffc-bba4-e9a0133ffc79"]
>
> One drawback to this approach is that if we want to find the names of the
> groups a user is a member of, including dynamic membership, then we need to
> make a separate request with a fiql search query to return the groups, and
> then parse the response appropriately.
>
> Ideally we would be able to see the group names for the dynamic case as
> well when retrieving the users - this means only one call would be needed.
> Would it be possible to update UserTO with a list of MembershipTOs that
> correspond to the dynamic case?

Hi Colm,
it seems there are no much people around interested in dynamic
memberships :-)

Anyway, your proposed change looks fine, but I would not mix in the same
field both static and dynamic memberships; hence, I propose to:

* keep List<MembershipTO> memberships
* change List<String> dynGroups into List<MembershipTO> dynMemberships,
where each MembershipTO has naturally empty attributes lists

WDYT?
Regards.

--
Francesco Chicchiriccò

Tirasa - Open Source Excellence
http://www.tirasa.net/

Member at The Apache Software Foundation
Syncope, Cocoon, Olingo, CXF, OpenJPA, PonyMail
http://home.apache.org/~ilgrosso/

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [DISCUSS] - Dynamic group memberships

Colm O hEigeartaigh
Hi Francesco,

On Wed, Jul 26, 2017 at 9:35 AM, Francesco Chicchiriccò <[hidden email]
> wrote:

>
>
> Anyway, your proposed change looks fine, but I would not mix in the same
> field both static and dynamic memberships; hence, I propose to:
>
> * keep List<MembershipTO> memberships
> * change List<String> dynGroups into List<MembershipTO> dynMemberships,
> where each MembershipTO has naturally empty attributes lists
>

Yes, that's the change I was thinking of, let me take care of it! I have
two somewhat related questions:

a) When creating a user with a membership via the REST API, no exception is
thrown if the membership key is not valid - the user is just created
without the associated membership. I think it would probably be better here
to throw an exception...WDYT?

b) When creating a membership via e.g.:

MembershipTO.Builder().group("9e955aa9-edf1-491c-955a-a9edf1591c7b", "boss")

the second parameter appears to be ignored by Syncope - you can specify any
value so long as the key is correct. Again, I think we should consider
throwing an exception here, as otherwise there is no point to specifying
the group name at all. Maybe we should go further and allow just the group
name to be specified?

Colm.



>
> WDYT?
> Regards.
>
> --
> Francesco Chicchiriccò
>
> Tirasa - Open Source Excellence
> http://www.tirasa.net/
>
> Member at The Apache Software Foundation
> Syncope, Cocoon, Olingo, CXF, OpenJPA, PonyMail
> http://home.apache.org/~ilgrosso/
>
>


--
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [DISCUSS] - Dynamic group memberships

ilgrosso
Administrator
On 2017-07-26 13:49 Colm O hEigeartaigh wrote:

> Hi Francesco,
>
> On Wed, Jul 26, 2017 at 9:35 AM, Francesco Chicchiriccò
> <[hidden email]> wrote:
>>
>> Anyway, your proposed change looks fine, but I would not mix in the
>> same
>> field both static and dynamic memberships; hence, I propose to:
>>
>> * keep List<MembershipTO> memberships
>> * change List<String> dynGroups into List<MembershipTO>
>> dynMemberships,
>> where each MembershipTO has naturally empty attributes lists
>>
>
> Yes, that's the change I was thinking of, let me take care of it! I
> have
> two somewhat related questions:
>
> a) When creating a user with a membership via the REST API, no
> exception is
> thrown if the membership key is not valid - the user is just created
> without the associated membership. I think it would probably be better
> here
> to throw an exception...WDYT?

This is more like a general approach, also taken for other cases where,
for example, it is requested to set values for attributes not allowed:
log a warning and discard.

We might want to change this behavior, but then it will affect more than
just memberships.

> b) When creating a membership via e.g.:
>
> MembershipTO.Builder().group("9e955aa9-edf1-491c-955a-a9edf1591c7b",
> "boss")
>
> the second parameter appears to be ignored by Syncope - you can specify
> any
> value so long as the key is correct. Again, I think we should consider
> throwing an exception here, as otherwise there is no point to
> specifying
> the group name at all. Maybe we should go further and allow just the
> group
> name to be specified?

Specifying the group name is more meant as a commodity when it is
Syncope to output MembershipTO instances: if you think this could be
misleading, we might remove the possibility to set the name via
MembershipTO.Builder.

Not sure about requesting membership to be set via group name rather
than via group key: the former is unique but mutable.

Regards.
--
Francesco Chicchiriccò

Tirasa - Open Source Excellence
http://www.tirasa.net/

Member at The Apache Software Foundation
Syncope, Cocoon, Olingo, CXF, OpenJPA, PonyMail
http://home.apache.org/~ilgrosso/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [DISCUSS] - Dynamic group memberships

Colm O hEigeartaigh
On Wed, Jul 26, 2017 at 4:28 PM, Francesco Chicchiriccò <[hidden email]
> wrote:


> This is more like a general approach, also taken for other cases where,
> for example, it is requested to set values for attributes not allowed: log
> a warning and discard.
>
> We might want to change this behavior, but then it will affect more than
> just memberships.
>

OK - no let's just leave it as it is, for now anyway.


>
> Specifying the group name is more meant as a commodity when it is Syncope
> to output MembershipTO instances: if you think this could be misleading, we
> might remove the possibility to set the name via MembershipTO.Builder.
>

It is a bit misleading, but as the functionality is used when outputing
MembershipTO instances as you say, let's just leave this as it is as well.

Thanks,

Colm.



>
>
> Regards.
> --
> Francesco Chicchiriccò
>
> Tirasa - Open Source Excellence
> http://www.tirasa.net/
>
> Member at The Apache Software Foundation
> Syncope, Cocoon, Olingo, CXF, OpenJPA, PonyMail
> http://home.apache.org/~ilgrosso/
>



--
Colm O hEigeartaigh

Talend Community Coder
http://coders.talend.com
Loading...