[dev] [commits] Horde branch H4-Group updated. 840392473461fdde7040004a26bceac2a3cc3dcf

[Jan Schneider]

Zitat von Michael Rubinsky <mrubinsk at horde.org>:

>
> Quoting Jan Schneider <jan at horde.org>:
>
>> Zitat von Michael Rubinsky <mrubinsk at horde.org>:
>>
>>>
>>> Quoting Jan Schneider <jan at horde.org>:
>>>
>>> <snip>
>>>
>>>> commit a45e4bb5980601ebeea3c3231e0f1afac9fcb944
>>>> Author: Jan Schneider <jan at horde.org>
>>>> Date:   Sat Mar 5 18:04:12 2011 +0100
>>>>
>>>>  Tweaks and fixes to the contact group API.
>>>>
>>>>  Michael, can we drop listUserGroupObjects(), because this seems  
>>>> to provide the
>>>>  same functionality like getGroupMemberships()?
>>>
>>>
>>> getGroupMemberships() returns a list group id => group names that  
>>> the current user is a *member* of (useful for checking if a user  
>>> has permissions on a resource to which the group has permissions  
>>> to), while listUserGroupObjects() returns an array of turba lists  
>>> that the current user has Horde_Perms::SHOW on, to be used e.g, in  
>>> listing the groups the user has available for assigning  
>>> permissions to his/her resources.  In other words, User "A" can be  
>>> a member of User B's contact list, but not have permissions to  
>>> User B's list (so should not be able to use User B's list to  
>>> assign permissions to one of User A's resources).
>>
>> Hm, that doesn't align at all with the general group API (and how  
>> we use it in application, if I didn't miss anything), because there  
>> is only the destinction between listing *all* groups and only the  
>> groups the user is a member of.
>>
>> With that approach we need to consider different group lists for  
>> permission checking and permission assignment. How did that ever  
>> work in the past? :) Ah, I guess because we had  
>> userIsMemberOfGroup() (or some such), which has been dropped in the  
>> rewrite.
>
> I'll have to take a closer look at the new code when I can, but,  
> with the contactList driver, it doesn't make much sense to me to  
> return groups based on contact lists that the current user doesn't  
> even have access to. That is what the listUserGroupObjects() method  
> call does - it only returns lists the user can actually see. When  
> User A is assigning group permissions to User A's resources, how is  
> he supposed to know what users belong in some group that he doesn't  
> have permissions enough to see? Not to mention, for an installation  
> like SAPO's, that would lead to some seriously large group lists.
>
> With the cursory glance I took at the new API, I don't see why the  
> this won't work. The contactList driver could use the results of  
> listUserGroupObjects() in Horde_Group::listAll(). This result will  
> only contain lists the user can see. When *checking* perms we don't  
> care about all existing groups, we only care about groups that the  
> user is a member of (Horde_Group::listGroups()), so this check  
> should also work as expected.

Not quite, because there are switches in application code whether to  
use listAll() or listGroups() to display the available groups. This  
switch is configured with the $conf['share']['any_group'] setting.
This could be solved by having the admin to always enable this setting  
when usig the Contactlists driver. But I feel uncomfortable with using  
such side-effects. Not to mention that this behavior is still  
completely different from the other drivers. This could be solved  
after the alpha release though.

> As we discussed when developing this driver, it was never designed  
> to provide access to ALL contact lists to ALL users. It was designed  
> to allow one user to assign permissions to his resources based on  
> that user's groups, not every user's groups.

Yes, and I'm still fine with that.

Jan.

-- 
Do you need professional PHP or Horde consulting?
http://horde.org/consulting/