A couple of goodies

[Bron Gondwana]

On Tue, 24 Jul 2007 23:40:18 +1000, "Bron Gondwana" <brong at fastmail.fm> said:
> On Tue, Jul 24, 2007 at 09:45:18AM +0100, David Carter wrote:
> > On Tue, 24 Jul 2007, Bron Gondwana wrote:
> >
> >> Oh, one more bug too (I haven't fixed this yet - probably tomorrow) is 
> >> that the admin user can no longer delete user mailboxes.  Not sure why 
> >> yet, I've just read the cron messages this very second and I'm out all 
> >> evening.
> >
> > Seems to work for me. The admin user needs to add a delete ACL for itself, 
> > but I think that this has always been the case.
> >
> 
> Ok, I'll have another look - I just know that since I upgraded cyrus
> earlier today all delete attempts by our "expired user" cleanup failed
> and spat errors out - but it could be because they're doing something
> else odd.
> 
> We certainly do set the delete ACL for the admin user.  Maybe only the
> old-style one though.  Hmm.

. list "fastmailtest.com\!user.testdelete" "*"
* LIST (\HasChildren) "." "user.testdelete at fastmailtest.com"
* LIST (\HasNoChildren) "." "user.testdelete.Drafts at fastmailtest.com"
* LIST (\HasNoChildren) "." "user.testdelete.Sent Items at fastmailtest.com"
* LIST (\HasNoChildren) "." "user.testdelete.Trash at fastmailtest.com"
. OK Completed (0.000 secs 5 calls)
. getacl "fastmailtest.com\!user.testdelete"
* ACL fastmailtest.com!user.testdelete testdelete at fastmailtest.com lrswipkxtecd admin lrswipkxtecda anyone p
. OK Completed
. delete "fastmailtest.com\!user.testdelete"
. NO Operation is not supported on mailbox
. delete "user.testdelete at fastmailtest.com"
. NO Operation is not supported on mailbox



OK, here's the problem:

  1217      } else if ((p = mboxname_isusermailbox(oldname, 1))) {
  1218          if (!strncmp(p, userid, config_virtdomains ? strcspn(userid, "@") :
  1219                       strlen(userid))) {
  1220              /* Special case of renaming inbox */
  1221              access = cyrus_acl_myrights(auth_state, oldacl);
  1222              if (!(access & ACL_DELETEMBOX)) {
  1223                r = IMAP_PERMISSION_DENIED;
  1224                goto done;
  1225              }
  1226              isusermbox = 1;
  1227          } else if (config_getswitch(IMAPOPT_ALLOWUSERMOVES) &&
  1228                     mboxname_isusermailbox(newname, 1)) {
  1229              /* Special case of renaming a user */
  1230              access = cyrus_acl_myrights(auth_state, oldacl);
  1231              if (!(access & ACL_DELETEMBOX) && !isadmin) {
  1232                  r = (isadmin || (access & ACL_LOOKUP)) ?
  1233                      IMAP_PERMISSION_DENIED : IMAP_MAILBOX_NONEXISTENT;
  1234                  goto done;
  1235              }
  1236          } else {
  1237              /* Only admins can rename users (INBOX to INBOX) */
  1238              r = IMAP_MAILBOX_NOTSUPPORTED;
  1239              goto done;
  1240          }

Line 1238 in particular (of mboxlist.c).  This might not match up exactly
for you because you don't run all the same patches, but the concept is there.
If you're renaming from a user mailbox to not a user mailbox, it fails during
the rename part.

This probably works for you because you're using an alternative hierarchy
separator and mboxname_isusermailbox is probably bogus/broken in that case.
Man I hate Cyrus's multiple namespace methods and lack of clarity in name
conversion sometimes.  If it was a web app it would have XSS bugs.


So yeah, you don't get this behaviour because it's masked by a different bug.
*sigh*.  I'll come back and patch this when I get back from the gym.

Bron.

-- 
  Bron Gondwana
  brong at fastmail.fm