[dev] [commits] Horde branch master updated. a61da56f8c575c7da9c2c018a350119e4301a472

[Michael J Rubinsky]

Quoting Jan Schneider <jan at horde.org>:

> Zitat von Michael J Rubinsky <mrubinsk at horde.org>:
>
>> Quoting Jan Schneider <jan at horde.org>:
>>
>>> Zitat von Michael J Rubinsky <mrubinsk at horde.org>:
>>>
>>>> Quoting Jan Schneider <jan at horde.org>:
>>>>
>>>>>> commit a61da56f8c575c7da9c2c018a350119e4301a472
>>>>>> Author: Michael J Rubinsky <mrubinsk at horde.org>
>>>>>> Date:   Fri Apr 24 10:32:55 2015 -0400
>>>>>>
>>>>>> Ensure admins see all system shares.
>>>>>>
>>>>>> This is why my system shares were not included in listShares.
>>>>>> If they are created, but don't explicitly have permissions set for
>>>>>> at least PERMS_VIEW, they will not be returned.
>>>>>>
>>>>>> kronolith/lib/Shares.php |   10 +++++++++-
>>>>>> 1 files changed, 9 insertions(+), 1 deletions(-)
>>>>>>
>>>>>> http://github.com/horde/horde/commit/a61da56f8c575c7da9c2c018a350119e4301a472
>>>>>
>>>>> This is intended. System share should for admins only show up in  
>>>>> the system shares section, not in the regular section. For  
>>>>> non-admin users, the system shares should show up in the shared  
>>>>> calendars section, if they have sufficient perms.
>>>>
>>>> This didn't have anything to do with what section they show up  
>>>> in. If you create a system share, but do not explicitly set any  
>>>> permissions on it, it will not be viewable _at_all_, to anyone,  
>>>> ever. This commit fixes them not showing up in the system shares  
>>>> section for admins only.
>>>
>>> But since listSystemShares() doesn't do permission checking, they  
>>> should still show up in the system calendars section, no matter if  
>>> permissions have been set or not.
>>
>> That's correct, but you removed the use of listSystemShares for  
>> listing the shares in the sidebar in a previous commit stating that  
>> listInternalShares() already returns systemShares - which it does,  
>> but only if permissions have been explicitly set.
>>
>> See  
>> https://github.com/horde/horde/commit/22516d5b4b07fcc49d370e699b4a339cb21c4d7a
>
> Yes, and that's exactly how it should work.
>
> Rewind. The problem seems to be that you cannot set permissions of  
> system shares if you created them without any permissions, because  
> they don't show up for admins?

Correct, because listInternalShares only returns shares with explicit  
permissions set - because the share backend doesn't know about  
administrators and there is never any owner for system shares other  
than administrators.

> First let's leave listShares() or listInternalCalendars() alone  
> because those should (and I guess do?) return the correct shares for  
> regular users.
> To allow admins to set permissions on system shares, they need to  
> show up in the system calendars section (don't they? they do for me).

With this commit they do for me. Without this commit only system  
shares that the admin user would have permissions to if he/she were  
not an admin show up. Try this: Create a new system share, but do NOT  
assign any permissions to it. I.e., in advanced sharing there should  
be NOTHING checked. If you revert the commit in question the share  
will NOT show in the system shares section, or anywhere else for that  
matter, for anyone, ever.


> This section should be populated by listSystemShares() which doesn't  
> check permissions (and should thus only be used for admins).

This is the part that you are missing. listSystemShares is NOT used to  
build the system shares section in the dynamic view. There is no  
separate polling from kronolith.js for "system" shares, only  
"internal" shares. In updateCalendarList() the calendar type is  
detected and placed in the correct sidebar section depending on the  
admin status of the user. Well, to be more accurate, based on your  
commit b593763c2079cc5d8ca25528661389520eb2621a, depending on if the  
$('kronolithSystemCalendars') node exists or not, and THIS is  
determined by the admin status of the user.

> Which point am I missing or which of this point isn't working for you?
>
> -- 
> Jan Schneider
> The Horde Project
> http://www.horde.org/
> https://www.facebook.com/hordeproject
>
> -- 
> dev mailing list
> Frequently Asked Questions: http://wiki.horde.org/FAQ
> To unsubscribe, mail: dev-unsubscribe at lists.horde.org


-- 
mike
The Horde Project
http://www.horde.org
https://www.facebook.com/hordeproject
https://www.twitter.com/hordeproject
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 5869 bytes
Desc: S/MIME Signature
URL: <http://lists.horde.org/archives/dev/attachments/20150427/99db46ad/attachment.bin>