Re: Patch: add new lmtptarget annotation
On 5/18/2010 12:38 PM, Stephen Grier wrote: All, Just submitting a patch I'm supporting locally for consideration. We use shared mailboxes quite extensively for role-based communication. For quite some time we've had a problem with users deleting or renaming mailboxes into which we deliver mail. We can, and do, use IMAP ACLs to dissallow users from deleting the delivery target mailbox. But when a user creates a child mailbox it inherits the ACLs of the parent, and the user is then not able to delete or rename the sub folder. As a fix, I have written a patch against 2.3.16 to add a new lmtptarget mailbox annotation. When enabled, Cyrus won't allow the mailbox to be deleted or renamed. We can then set whatever ACLs we want inherited by child mailboxes, happy in the knowledge the user won't blat the mailbox and cause mail to bounce. The rationale here is that Cyrus treats user.foo with special significance as a delivery target, but does not do the same for shared mailboxes because there is no way for Cyrus to know which shared mailboxes we intend to deliver mail into. Using a mailbox annotation seems a nice way of flagging this. Patch attached. Comments welcome. Cheers, Stephen Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html I have not tried the patch yet, but if this works OK, I think this would be great to have in the official release. Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
Re: Patch: add new lmtptarget annotation
On 18 May 2010, at 15:11, Nic Bernstein wrote: Where in the documentation should information be added describing this new annotation? Where are available annotations documented currently? As far as I can tell, there this: http://cyrusimap.web.cmu.edu/twiki/bin/view/Cyrus/CyrusAnnotations and the changes.html that ships with cyrus. Seems like a problem, doesn't it? :wes Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
Patch: add new lmtptarget annotation
All,
Just submitting a patch I'm supporting locally for consideration.
We use shared mailboxes quite extensively for role-based communication.
For quite some time we've had a problem with users deleting or renaming
mailboxes into which we deliver mail. We can, and do, use IMAP ACLs to
dissallow users from deleting the delivery target mailbox. But when a
user creates a child mailbox it inherits the ACLs of the parent, and the
user is then not able to delete or rename the sub folder.
As a fix, I have written a patch against 2.3.16 to add a new lmtptarget
mailbox annotation. When enabled, Cyrus won't allow the mailbox to be
deleted or renamed. We can then set whatever ACLs we want inherited by
child mailboxes, happy in the knowledge the user won't blat the mailbox
and cause mail to bounce.
The rationale here is that Cyrus treats user.foo with special
significance as a delivery target, but does not do the same for shared
mailboxes because there is no way for Cyrus to know which shared
mailboxes we intend to deliver mail into. Using a mailbox annotation
seems a nice way of flagging this.
Patch attached. Comments welcome.
Cheers,
Stephen
--
Stephen Grier
Systems Developer
IT Services
Queen Mary, University of London
diff -Naur cyrus-imapd-2.3.16.old/imap/annotate.c cyrus-imapd-2.3.16/imap/annotate.c
--- cyrus-imapd-2.3.16.old/imap/annotate.c 2009-12-21 11:25:22.0 +
+++ cyrus-imapd-2.3.16/imap/annotate.c 2010-05-18 10:54:42.069620739 +0100
@@ -1874,6 +1874,9 @@
{ /vendor/cmu/cyrus-imapd/duplicatedeliver, ATTRIB_TYPE_BOOLEAN, BACKEND_ONLY,
ATTRIB_VALUE_SHARED | ATTRIB_CONTENTTYPE_SHARED,
ACL_ADMIN, annotation_set_mailboxopt, NULL },
+{ /vendor/qmul/cyrus-imapd/lmtptarget, ATTRIB_TYPE_BOOLEAN, BACKEND_ONLY,
+ ATTRIB_VALUE_SHARED | ATTRIB_CONTENTTYPE_SHARED,
+ ACL_ADMIN, annotation_set_todb, NULL },
{ NULL, 0, ANNOTATION_PROXY_T_INVALID, 0, 0, NULL, NULL }
};
diff -Naur cyrus-imapd-2.3.16.old/imap/mboxlist.c cyrus-imapd-2.3.16/imap/mboxlist.c
--- cyrus-imapd-2.3.16.old/imap/mboxlist.c 2009-11-17 03:34:30.0 +
+++ cyrus-imapd-2.3.16/imap/mboxlist.c 2010-05-18 11:18:15.509634066 +0100
@@ -1028,6 +1028,7 @@
int mbtype;
const char *p;
mupdate_handle *mupdate_h = NULL;
+struct annotation_data attrib;
if(!isadmin force) return IMAP_PERMISSION_DENIED;
@@ -1048,6 +1049,14 @@
if (!isadmin) { r = IMAP_PERMISSION_DENIED; goto done; }
}
+/* Does mailbox have the lmtptarget annotation set? */
+if (annotatemore_lookup(name, /vendor/qmul/cyrus-imapd/lmtptarget, , attrib) == 0
+attrib.value !strcasecmp(attrib.value, true)) {
+/* Even admins can't delete a mailbox with the lmtptarget annotation set. */
+r = IMAP_MAILBOX_NOTSUPPORTED;
+goto done;
+}
+
r = mboxlist_mylookup(name, mbtype, path, mpath, NULL, acl, tid, 1);
switch (r) {
case 0:
@@ -1193,6 +1202,7 @@
char *newpartition = NULL;
char *mboxent = NULL;
char *p;
+struct annotation_data attrib;
mupdate_handle *mupdate_h = NULL;
int madenew = 0;
@@ -1299,6 +1309,13 @@
goto done;
}
}
+/* Does mailbox have the lmtptarget annotation set? */
+if (annotatemore_lookup(oldname, /vendor/qmul/cyrus-imapd/lmtptarget, , attrib) == 0
+attrib.value !strcasecmp(attrib.value, true)) {
+/* Even admins can't rename a mailbox with the lmtptarget annotation set. */
+r = IMAP_MAILBOX_NOTSUPPORTED;
+goto done;
+}
r = mboxlist_mycreatemailboxcheck(newname, 0, partition, isadmin,
userid, auth_state, NULL,
newpartition, 1, 0, forceuser, tid);
diff -Naur cyrus-imapd-2.3.16.old/perl/imap/IMAP/Admin.pm cyrus-imapd-2.3.16/perl/imap/IMAP/Admin.pm
--- cyrus-imapd-2.3.16.old/perl/imap/IMAP/Admin.pm 2008-04-04 13:47:11.0 +0100
+++ cyrus-imapd-2.3.16/perl/imap/IMAP/Admin.pm 2010-05-18 11:30:54.437108440 +0100
@@ -789,6 +789,7 @@
expire = /vendor/cmu/cyrus-imapd/expire,
news2mail = /vendor/cmu/cyrus-imapd/news2mail,
sharedseen = /vendor/cmu/cyrus-imapd/sharedseen,
+ lmtptarget = /vendor/qmul/cyrus-imapd/lmtptarget,
sieve = /vendor/cmu/cyrus-imapd/sieve,
squat = /vendor/cmu/cyrus-imapd/squat );
Cyrus Home Page: http://cyrusimap.web.cmu.edu/
Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki
List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
Re: Patch: add new lmtptarget annotation
Seems like a reasonable approach and a good patch. I might suggest some feedback, preferably to the deleting / renaming user but a syslog might also do, when the delete / rename failed. Is there a bugzilla number? :wes On 18 May 2010, at 12:38, Stephen Grier wrote: Just submitting a patch I'm supporting locally for consideration. We use shared mailboxes quite extensively for role-based communication. For quite some time we've had a problem with users deleting or renaming mailboxes into which we deliver mail. We can, and do, use IMAP ACLs to dissallow users from deleting the delivery target mailbox. But when a user creates a child mailbox it inherits the ACLs of the parent, and the user is then not able to delete or rename the sub folder. As a fix, I have written a patch against 2.3.16 to add a new lmtptarget mailbox annotation. When enabled, Cyrus won't allow the mailbox to be deleted or renamed. We can then set whatever ACLs we want inherited by child mailboxes, happy in the knowledge the user won't blat the mailbox and cause mail to bounce. The rationale here is that Cyrus treats user.foo with special significance as a delivery target, but does not do the same for shared mailboxes because there is no way for Cyrus to know which shared mailboxes we intend to deliver mail into. Using a mailbox annotation seems a nice way of flagging this. Patch attached. Comments welcome. Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
Re: Patch: add new lmtptarget annotation
Hello Stephen, We use shared mailboxes quite extensively for role-based communication. For quite some time we've had a problem with users deleting or renaming mailboxes into which we deliver mail. We can, and do, use IMAP ACLs to dissallow users from deleting the delivery target mailbox. But when a user creates a child mailbox it inherits the ACLs of the parent, and the user is then not able to delete or rename the sub folder. This is an issue here as well. Thanks for your solution! Would be a good thing to see it included in an official release. Frank As a fix, I have written a patch against 2.3.16 to add a new lmtptarget mailbox annotation. When enabled, Cyrus won't allow the mailbox to be deleted or renamed. We can then set whatever ACLs we want inherited by child mailboxes, happy in the knowledge the user won't blat the mailbox and cause mail to bounce. The rationale here is that Cyrus treats user.foo with special significance as a delivery target, but does not do the same for shared mailboxes because there is no way for Cyrus to know which shared mailboxes we intend to deliver mail into. Using a mailbox annotation seems a nice way of flagging this. Patch attached. Comments welcome. -- E-Mail: frank.rich...@hrz.tu-chemnitz.de http://www.tu-chemnitz.de/~fri/ Work: Computing Services, Chemnitz University of Technology, Germany Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
Re: Patch: add new lmtptarget annotation
This looks like a great addition. One question: Where in the documentation should information be added describing this new annotation? Where are available annotations documented currently? Thanks for the contribution! -nic On 05/18/2010 12:47 PM, Wesley Craig wrote: Seems like a reasonable approach and a good patch. I might suggest some feedback, preferably to the deleting / renaming user but a syslog might also do, when the delete / rename failed. Is there a bugzilla number? :wes On 18 May 2010, at 12:38, Stephen Grier wrote: Just submitting a patch I'm supporting locally for consideration. We use shared mailboxes quite extensively for role-based communication. For quite some time we've had a problem with users deleting or renaming mailboxes into which we deliver mail. We can, and do, use IMAP ACLs to dissallow users from deleting the delivery target mailbox. But when a user creates a child mailbox it inherits the ACLs of the parent, and the user is then not able to delete or rename the sub folder. As a fix, I have written a patch against 2.3.16 to add a new lmtptarget mailbox annotation. When enabled, Cyrus won't allow the mailbox to be deleted or renamed. We can then set whatever ACLs we want inherited by child mailboxes, happy in the knowledge the user won't blat the mailbox and cause mail to bounce. The rationale here is that Cyrus treats user.foo with special significance as a delivery target, but does not do the same for shared mailboxes because there is no way for Cyrus to know which shared mailboxes we intend to deliver mail into. Using a mailbox annotation seems a nice way of flagging this. Patch attached. Comments welcome. Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html -- Nic Bernstein n...@onlight.com Onlight llc. www.onlight.com 219 N. Milwaukee St., Suite 2av. 414.272.4477 Milwaukee, Wisconsin 53202 Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
Re: Patch: add new lmtptarget annotation
On 18/05/10 18:47, Wesley Craig wrote: Seems like a reasonable approach and a good patch. I might suggest some feedback, preferably to the deleting / renaming user but a syslog might also do, when the delete / rename failed. My patch returns IMAP_MAILBOX_NOTSUPPORTED to the client. I can add a syslog call if people think that would be useful. Is there a bugzilla number? #3220. :wes On 18 May 2010, at 12:38, Stephen Grier wrote: Just submitting a patch I'm supporting locally for consideration. We use shared mailboxes quite extensively for role-based communication. For quite some time we've had a problem with users deleting or renaming mailboxes into which we deliver mail. We can, and do, use IMAP ACLs to dissallow users from deleting the delivery target mailbox. But when a user creates a child mailbox it inherits the ACLs of the parent, and the user is then not able to delete or rename the sub folder. As a fix, I have written a patch against 2.3.16 to add a new lmtptarget mailbox annotation. When enabled, Cyrus won't allow the mailbox to be deleted or renamed. We can then set whatever ACLs we want inherited by child mailboxes, happy in the knowledge the user won't blat the mailbox and cause mail to bounce. The rationale here is that Cyrus treats user.foo with special significance as a delivery target, but does not do the same for shared mailboxes because there is no way for Cyrus to know which shared mailboxes we intend to deliver mail into. Using a mailbox annotation seems a nice way of flagging this. Patch attached. Comments welcome. Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html -- Stephen Grier Systems Developer IT Services Queen Mary, University of London Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
Re: Patch: add new lmtptarget annotation
On Tue, 2010-05-18 at 20:53 +0200, Frank Richter wrote: Hello Stephen, We use shared mailboxes quite extensively for role-based communication. For quite some time we've had a problem with users deleting or renaming mailboxes into which we deliver mail. We can, and do, use IMAP ACLs to dissallow users from deleting the delivery target mailbox. But when a user creates a child mailbox it inherits the ACLs of the parent, and the user is then not able to delete or rename the sub folder. This is an issue here as well. Thanks for your solution! Would be a good thing to see it included in an official release. +1 Cyrus Home Page: http://cyrusimap.web.cmu.edu/ Cyrus Wiki/FAQ: http://cyrusimap.web.cmu.edu/twiki List Archives/Info: http://asg.web.cmu.edu/cyrus/mailing-list.html
