cyrusimap / cyrus-imapd

Cyrus IMAP is an email, contacts and calendar server
http://cyrusimap.org
Other
548 stars 150 forks source link

imap/annotate.c:annotate_state_writemask: put shared attributes on shared mailboxes #2492

Open dilyanpalauzov opened 6 years ago

dilyanpalauzov commented 6 years ago

The mailbox #addressbook/shared does not belong to anybody, but a user with write-privileges to it shall be able to set global/shared properties over WebDAV like CARD:addressbook-description or DAV:displayname, that are valid for all users. So if a mailbox does not belong to a user, then the set properties are always shared.

diff --git a/imap/annotate.c b/imap/annotate.c
index 9e8badb00..6e81f82eb 100644
--- a/imap/annotate.c
+++ b/imap/annotate.c
@@ -2643,7 +2643,7 @@ EXPORTED int annotate_state_writemask(annotate_state_t *state,
                                       const struct buf *value)
 {
     /* if the user is the owner, then write to the shared namespace */
-    if (mboxname_userownsmailbox(userid, state->mailbox->name))
+    if (mboxname_userownsmailbox(userid, state->mailbox->name) || !mboxname_isusermailbox(state->mailbox->name, 1))
         return annotate_state_write(state, entry, "", value);
     else
         return annotate_state_write(state, entry, userid, value);
karagian commented 6 years ago

Not sure if this change has any meaning... ACLs apply properly to shared collections. So if a user has the proper access to the collection (write permission), he should be able to make changes. If the user has only read access, he shouldn't be able to make any changes. That's how it is supposed to work, even for shared collections. The code you changed is actually the exception to the ACLs for the user's own mailbox, where the owner always has write access to his own mailbox. I don't think this should apply to shared mailboxes/collections.

dilyanpalauzov commented 6 years ago

With appropriate ACLs you can set the DAV:displayname on a collection that is not owned by anybody, but others will not see your change.

karagian commented 6 years ago

hm now I see what you mean... maybe it would be better to check whether the user is an admin account then, instead of allowing all users change that shared property? Generally only admin accounts have those special access rights to create/modify shared objects. In that case, users that have write access could still modify that property, but only they would see their change? Not sure how these properties work...