Re: [Dovecot] patch: list shared namespace
All of this is now implemented. I think shared mailboxes/ACLs are now fully working. The only thing left is to avoid calling acl_lookup_dict_rebuild() after each ACL change, but rather just update the dict directly with the changes. Hmm. Wonder how quota behaves with shared mailboxes.. That's probably broken. On Sat, 2008-11-01 at 23:43 +0200, Timo Sirainen wrote: > On Fri, 2008-10-31 at 17:51 +0200, Timo Sirainen wrote: > > LIST % -> List "foo" as non-existing > > LIST foo -> List "foo" as non-existing > > LIST * -> List "foo/bar" only > > There are also some truly horrible cases. For example: > > 1 list "" foo* > * LIST (\HasNoChildren) "." "foo.foo.foo" > * LIST (\HasNoChildren) "." "foo.bar.baz" > 1 ok > > 2 list "" f*o.% > * LIST (\HasNoChildren) "." "foo.foo.foo" > * LIST (\Noselect \HasChildren) "." "foo.bar" > 2 OK List completed. > > 3 list "" f*r > * LIST (\Noselect \HasChildren) "." "foo.bar" > 3 OK List completed. > > As you can see, the non-existing "foo.foo" isn't returned because its > child "foo.foo.foo" also matches the pattern and is returned. But the > non-existing "foo.bar" is returned because its children don't match the > pattern. It took me forever to get all this stuff working right with > Maildir++. :) > > I think it would be possible to implement the same somewhat easily in > ACL code: > > 1. When ACL code sees that a non-existing mailbox is to be returned, > find out if there are any patterns that match the mailbox and that ends > with "*" character. If yes, don't return the mailbox (because its > children will be returned anyway). If not: > > 2. Start a new mailbox listing that lists children of the non-existing > mailbox (mailbox/*). If you find: > > a) A visible mailbox that matches the original patterns -> don't return > the original non-existing mailbox (since its child will be returned > later) > > b) No visible mailboxes -> don't return the original non-existing > mailbox > > c) Fallback to returning the non-existing mailbox > > The same logic should also be used when determining if shared namespace > prefixes should be returned (I think ACL code can do that too?) > > Also that code should work properly when mailbox names contain "*" or > "%" characters. Basically it means that when generating the mailbox/* > pattern replace all "*" chars with "%" chars in the mailbox name and > then later when going through the results skip over everything that > doesn't begin with the real mailbox name. signature.asc Description: This is a digitally signed message part
Re: [Dovecot] patch: list shared namespace
On Mon, 2008-11-03 at 13:03 +0100, Bernhard Herzog wrote: > > As you can see, the non-existing "foo.foo" isn't returned because its > > child "foo.foo.foo" also matches the pattern and is returned. But the > > non-existing "foo.bar" is returned because its children don't match the > > pattern. It took me forever to get all this stuff working right with > > Maildir++. :) > > I can imagine :). The reason it should work with ACLs more or less > automatically is that when the mailbox list is populated by > acl_mailbox_try_list_fast, it only adds the mailboxes that the user can see > using mailbox_list_iter_update. mailbox_list_iter_update takes care of > filling in the nonexisting parent mailboxes if necessary. That's not correct actually. acl_mailbox_try_list_fast adds all mailboxes that exist in dovecot-acl-list file, i.e. all mailboxes that have 'l' right set to someone (not necessarily to you). So if you have: foo: owner foo/bar: user=xyz l Then "foo" should be visible as non-existing mailbox for user xyz, but no-one else. With your change it will be visible to everyone. > Of course, assuming there's a reason acl_mailbox_try_list_fast has a "try" in > its name and that it actually can fail, foo, foo.foo and foo.bar could > perhaps end up in the mailbox list even if they do not have children that are > visible to the user. The name implies that it could fail. But .. hmm. I'm not sure yet, have to look at the code some more. :) signature.asc Description: This is a digitally signed message part
Re: [Dovecot] patch: list shared namespace
On Fri, 2008-10-31 at 17:33 +0200, Timo Sirainen wrote: > What I'm worrying here is if all users have shared something. For > example how does this work with global ACLs? IIRC if there's a global > ACL for e.g. "Spam", Dovecot will create dovecot-acl-list with "Spam" in > it for all users. Even without global ACLs the number of users sharing > something might be a bit too large. > > So the performance would be a lot better if the dict stored something > like source_user -> dest, where dest would be all the user names and > group names that are included in the user's ACLs (for any mailbox). Then > you'd look only at those users' mailboxes where you or your groups are > mentioned. Negative user/group rules perhaps shouldn't be in the dict. Maybe something like: Dict contains /acl/$dest/$source_user -> 1 where $dest is either the ACL user or ACL group (with u= and g= prefixes so they don't get mixed) and $source_user is the user whose mailbox ACL is being changed. This allows getting a list of all possible users whose shared mailboxes we have visible. Just do a dict iteration for /acl/$user and /acl/$group for each group we belong to. And I guess also check /acl/anyone and /acl/authenticated. As for updating the dict, it should normally be done by acl_backend_vfile_object_update(). It should also immediately update the dovecot-acl-list file. This should take care of all the ACL changes that are done via IMAP. The dict changes are single entry updates or deletions, no iterations, so the IMAP ACL commands are relatively cheap to use. Changing dovecot-acl files by hand then requires dropping removed users/groups from the dict. Since we don't know anymore what was removed, we have to go through the entire dict and find all the entries with /acl/*/$source_user. Then add the missing entries and delete the unnecessary entries. Triggering this rescan could be done by acl_backend_vfile_acllist_verify() when it's also rebuilding the dovecot-acl-list file. Then there's a final problem of how to rebuild the dict in case it gets broken, desynced or whatever. I think this would be solved most easily with an increasing acl-validity value (UNIX timestamp). In dict store it to /acl/validity and in each dovecot-acl-list store it to its header (which doesn't exist yet, need to add it). When reading or updating dovecot-acl-list file check if its acl-validity value is different from dict's /acl/validity value. If it is, trigger a dictionary rescan. I think it should rescan only the one user whose dovecot-acl-list is being read (so the code would be identical to handling changed dovecot-acl files), because it might take too long to rescan all users' ACL files. This means anyway that deleting everything from dictionary would cause shared mailboxes to be lost from LIST until either the dovecot-acl-list is being read by either a) the owning user logging in and issuing a LIST command, b) the destination users having those mailboxes subscribed and issuing a LSUB command. Any hope of getting you to implement at least parts of this? :) Also the IMAP ACL code needs to be moved to a separate imap-acl plugin. After those two I could start trying to get all of it merged. signature.asc Description: This is a digitally signed message part
Re: [Dovecot] patch: list shared namespace
On Nov 4, 2008, at 8:38 PM, Bernhard Herzog wrote: On 31.10.2008, Timo Sirainen wrote: On Tue, 2008-10-28 at 21:07 +0100, Bernhard Herzog wrote: I'm not sure the new hook is really needed. The patch could perhaps just as well extend the acl_next_hook_mail_storage_created and acl_next_hook_mailbox_list_created functions to do the namespace creation when they're called for a shared storage or mailbox list. Perhaps hook_mail_namespaces_created? Or perhaps hook_mailbox_list_created? It would be nice to make sure that if a user makes a mailbox available to another user, that it shows up as soon as the other user issues the next LIST command. How would hook_mailbox_list_created help with that? What's the best way to determine whether the mailbox list or namespace that was created is actually a shared namespace (or list for a shared namespace)? Should I just check whether ns->storage->name equals "shared"? ns->type == NAMESPACE_SHARED? PGP.sig Description: This is a digitally signed message part
Re: [Dovecot] patch: list shared namespace
On 31.10.2008, Timo Sirainen wrote: > On Tue, 2008-10-28 at 21:07 +0100, Bernhard Herzog wrote: > > I'm not sure the new hook is really needed. The patch could perhaps > > just as well extend the acl_next_hook_mail_storage_created and > > acl_next_hook_mailbox_list_created functions to do the namespace > > creation when they're called for a shared storage or mailbox list. > > Perhaps hook_mail_namespaces_created? Or perhaps hook_mailbox_list_created? It would be nice to make sure that if a user makes a mailbox available to another user, that it shows up as soon as the other user issues the next LIST command. What's the best way to determine whether the mailbox list or namespace that was created is actually a shared namespace (or list for a shared namespace)? Should I just check whether ns->storage->name equals "shared"? Bernhard -- Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner signature.asc Description: This is a digitally signed message part.
Re: [Dovecot] patch: list shared namespace
On 31.10.2008, Timo Sirainen wrote:
> v1.2 code supports multiple users per process. That means you can't
> really use getenv("USER") and you can't store per-user objects as static
> variables. Rather you should hook into hook_mail_user_created and add
> the per-user variables to the mail_user structure. See for example
> lazy-expunge plugin (struct lazy_expunge_mail_user) or quota plugin how
> that works.
I have done that for the metadata plugin now, which actually does use per-user
dicts if private metadata is enabled. However, the only reason my ACL plugin
changes use getenv("USER") is that dict_init requires a username. That
username is not actually needed for what the code does with the dict, though.
All entries are public entries and it doesn't matter which user reads or
modifies the dict. Maybe the dict API should have the concept of a purely
public dict which doesn't require a username.
> i_info("no acl_shared_dict specified; shared namespaces will not be
> listed") could be written only if getenv("DEBUG") != NULL.
I did that now.
> Is acl_shared_debug stuff only a temporary developer debugging thing, or
> will it be useful also for sysadmins?
I put that in because it was useful for me while developing. I'm not sure
it's useful for admins later on. So maybe I should remove them.
Bernhard
--
Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/
Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998
Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
signature.asc
Description: This is a digitally signed message part.
Re: [Dovecot] patch: list shared namespace
On 01.11.2008, Timo Sirainen wrote: > On Fri, 2008-10-31 at 17:51 +0200, Timo Sirainen wrote: > > LIST % -> List "foo" as non-existing > > LIST foo -> List "foo" as non-existing > > LIST * -> List "foo/bar" only > > There are also some truly horrible cases. I tested this with my acl_mailbox_list_info_is_visible modification in a vanilla dovecot 1.2 (rev. c6482b5cdea1). User [EMAIL PROTECTED] has these mailboxes: * LIST (\HasChildren) "/" "INBOX/foo" * LIST (\HasChildren) "/" "INBOX/foo/foo" * LIST (\HasNoChildren) "/" "INBOX/foo/foo/foo" * LIST (\HasChildren) "/" "INBOX/foo/bar" * LIST (\HasNoChildren) "/" "INBOX/foo/bar/baz" INBOX/foo/foo/foo and INBOX/foo/bar/baz have ACLs which give [EMAIL PROTECTED] the l-permission. The other mailboxes involved have no ACLs or only ACL settings for the owner. The results for listtest1 are as follows: > 1 list "" foo* > * LIST (\HasNoChildren) "." "foo.foo.foo" > * LIST (\HasNoChildren) "." "foo.bar.baz" > 1 ok 1 list "" "users/[EMAIL PROTECTED]/foo*" * LIST (\HasNoChildren) "/" "users/[EMAIL PROTECTED]/foo/foo/foo" * LIST (\HasNoChildren) "/" "users/[EMAIL PROTECTED]/foo/bar/baz" 1 OK List completed. > 2 list "" f*o.% > * LIST (\HasNoChildren) "." "foo.foo.foo" > * LIST (\Noselect \HasChildren) "." "foo.bar" > 2 OK List completed. 2 list "" "users/[EMAIL PROTECTED]/f*o.%" 2 OK List completed. The equivalent list command for the owner of the mailboxes, listtest2, doesn't return anything either: 2 list "" "INBOX/f*o.%" 2 OK List completed. > 3 list "" f*r > * LIST (\Noselect \HasChildren) "." "foo.bar" > 3 OK List completed. 3 list "" "users/[EMAIL PROTECTED]/f*r" * LIST (\Noselect \HasChildren) "/" "users/[EMAIL PROTECTED]/foo/bar" 3 OK List completed. > As you can see, the non-existing "foo.foo" isn't returned because its > child "foo.foo.foo" also matches the pattern and is returned. But the > non-existing "foo.bar" is returned because its children don't match the > pattern. It took me forever to get all this stuff working right with > Maildir++. :) I can imagine :). The reason it should work with ACLs more or less automatically is that when the mailbox list is populated by acl_mailbox_try_list_fast, it only adds the mailboxes that the user can see using mailbox_list_iter_update. mailbox_list_iter_update takes care of filling in the nonexisting parent mailboxes if necessary. In your example, that means only foo.foo.foo and foo.bar.baz are added, regardless of whether foo, foo.foo or foo.bar actually exist. foo, foo.foo and foo.bar are added to the list as nonexisting mailboxes automatically, though. So AFAICT from the other user's point of view it really is as if only foo.foo.foo and foo.bar.baz actually existed. Of course, assuming there's a reason acl_mailbox_try_list_fast has a "try" in its name and that it actually can fail, foo, foo.foo and foo.bar could perhaps end up in the mailbox list even if they do not have children that are visible to the user. Bernhard -- Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner signature.asc Description: This is a digitally signed message part.
Re: [Dovecot] patch: list shared namespace
On Fri, 2008-10-31 at 17:51 +0200, Timo Sirainen wrote: > LIST % -> List "foo" as non-existing > LIST foo -> List "foo" as non-existing > LIST * -> List "foo/bar" only There are also some truly horrible cases. For example: 1 list "" foo* * LIST (\HasNoChildren) "." "foo.foo.foo" * LIST (\HasNoChildren) "." "foo.bar.baz" 1 ok 2 list "" f*o.% * LIST (\HasNoChildren) "." "foo.foo.foo" * LIST (\Noselect \HasChildren) "." "foo.bar" 2 OK List completed. 3 list "" f*r * LIST (\Noselect \HasChildren) "." "foo.bar" 3 OK List completed. As you can see, the non-existing "foo.foo" isn't returned because its child "foo.foo.foo" also matches the pattern and is returned. But the non-existing "foo.bar" is returned because its children don't match the pattern. It took me forever to get all this stuff working right with Maildir++. :) I think it would be possible to implement the same somewhat easily in ACL code: 1. When ACL code sees that a non-existing mailbox is to be returned, find out if there are any patterns that match the mailbox and that ends with "*" character. If yes, don't return the mailbox (because its children will be returned anyway). If not: 2. Start a new mailbox listing that lists children of the non-existing mailbox (mailbox/*). If you find: a) A visible mailbox that matches the original patterns -> don't return the original non-existing mailbox (since its child will be returned later) b) No visible mailboxes -> don't return the original non-existing mailbox c) Fallback to returning the non-existing mailbox The same logic should also be used when determining if shared namespace prefixes should be returned (I think ACL code can do that too?) Also that code should work properly when mailbox names contain "*" or "%" characters. Basically it means that when generating the mailbox/* pattern replace all "*" chars with "%" chars in the mailbox name and then later when going through the results skip over everything that doesn't begin with the real mailbox name. signature.asc Description: This is a digitally signed message part
Re: [Dovecot] patch: list shared namespace
On Oct 31, 2008, at 10:03 PM, Bernhard Herzog wrote: On 31.10.2008, Timo Sirainen wrote: Right, it could (would) cause mailboxes to be listed that aren't supposed to be listed. I think you'll also have a problem if e.g. "foo" exists but doesn't have 'l' right and "foo/bar" exists and has 'l' right. I think % will currently not list "foo". If it behaved correctly it should list it as non-existing mailbox. That case seems to work correctly with my patch. Oh. Wonder why.. In my tests so far, it basically behaves exactly like you explain: LIST % -> List "foo" as non-existing LIST foo -> List "foo" as non-existing LIST * -> List "foo/bar" only Maybe there are circumstances that I didn't encounter yet, where it does indeed fail. Well, the main failure is that if the child mailboxes aren't listable, the parent mailbox shouldn't be listed either. Like if I share a secret/dovecot corp/contracts/google/october mailbox to someone then other people really shouldn't be seeing secret/dovecot corp/contracts/ google :) PGP.sig Description: This is a digitally signed message part
Re: [Dovecot] patch: list shared namespace
On 31.10.2008, Timo Sirainen wrote: > Right, it could (would) cause mailboxes to be listed that aren't > supposed to be listed. I think you'll also have a problem if e.g. "foo" > exists but doesn't have 'l' right and "foo/bar" exists and has 'l' > right. I think % will currently not list "foo". If it behaved correctly > it should list it as non-existing mailbox. That case seems to work correctly with my patch. In my tests so far, it basically behaves exactly like you explain: > LIST % -> List "foo" as non-existing > LIST foo -> List "foo" as non-existing > LIST * -> List "foo/bar" only Maybe there are circumstances that I didn't encounter yet, where it does indeed fail. Bernhard -- Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner signature.asc Description: This is a digitally signed message part.
Re: [Dovecot] patch: list shared namespace
On Oct 31, 2008, at 9:42 PM, Bernhard Herzog wrote: The behavior of the _verify funtion is the problem. It only rebuilds if any of the already referenced acl files has changed but not if new acl files have been created. I'm not sure yet what the best way is to determine whether new mailbox whose acllist is being verified has a new acl file. Doesn't it notice the new ACL file when that mailbox is being listed (or selected)? That's at least how I planned that it should work. Also your IMAP ACL plugin probably should somehow trigger the dovecot- acl-list rebuild whenever an ACL is added to a new mailbox. PGP.sig Description: This is a digitally signed message part
Re: [Dovecot] patch: list shared namespace
On 28.10.2008, Bernhard Herzog wrote: > - The dovecot-acl-list is not always rebuilt, even when it should have >been, AFAICT. In particular, if the file exists but is empty, it's >never updated, even when ACL later change. Maybe this is a bug in >the Kolab branch. It happens with vanilla dovecot 1.2 too. Here's my analysis so far: The dovecot-acl-list is rebuilt by acl_backend_vfile_acllist_rebuild which called in two places, acl_backend_vfile_acllist_refresh and acl_backend_vfile_acllist_verify. The _refresh-funtion rereads the file if it has changed and rebuilds it if it cannot be read. The _verify-function checks whether any of the acl files referenced by the dovecot-acl-list file has been changed and rebuilds the dovecot-acl-list file if that's the case. The behavior of the _verify funtion is the problem. It only rebuilds if any of the already referenced acl files has changed but not if new acl files have been created. I'm not sure yet what the best way is to determine whether new mailbox whose acllist is being verified has a new acl file. Cheers, Bernhard -- Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner signature.asc Description: This is a digitally signed message part.
Re: [Dovecot] patch: list shared namespace
On Fri, 2008-10-31 at 12:52 +0100, Bernhard Herzog wrote: > On 28.10.2008, Bernhard Herzog wrote: > > - List with "%" doesn't list all intermediate mailboxes. > > > >On the one hand arthur sees this: > > > > x LIST "" "*" > > ... > > * LIST (\Noselect \HasChildren) "/" "users/ford" > > * LIST (\HasNoChildren) "/" "users/ford/INBOX/hhgttg" > > x OK List completed. > > > >OTOH, with "%" only this: > > > > x LIST "" "users/ford/%" > > x OK List completed. > > I've looked into this. The problem is that acl_mailbox_list_info_is_visible > in src/plugins/acl/acl-mailbox-list.c considers nonexistent mailboxes as not > visible. The attached patch fixes that for me. I'm not sure it really is > the right fix, though. Maybe it would cause some mailboxes to be listed even > though they shouldn't. Right, it could (would) cause mailboxes to be listed that aren't supposed to be listed. I think you'll also have a problem if e.g. "foo" exists but doesn't have 'l' right and "foo/bar" exists and has 'l' right. I think % will currently not list "foo". If it behaved correctly it should list it as non-existing mailbox. The real solution would be to find out if there are any visible child mailboxes. That's kind of annoying to implement though. You'll only need to do that if the mailbox list pattern expects that mailbox to be returned. For example with the above foo and foo/bar mailboxes: LIST % -> List "foo" as non-existing LIST foo -> List "foo" as non-existing LIST * -> List "foo/bar" only I'm not exactly sure what's the right way to implement this. That's why it's still in my TODO list instead of actually implemented. :) > There's one thing about acl_mailbox_list_info_is_visible and struct > acl_mailbox_list_iterate_context that I don't understand. What's the member > struct mailbox_info info; used for? Looks like it's a bug. Perhaps I moved some of the code to acl_mailbox_list_info_is_visible() which broke it. The idea was anyway to modify info.flags and store them to ctx->info and then return ctx->info from the function. But that's clearly not happening. Fixed: http://hg.dovecot.org/dovecot-1.2/rev/692aac54ae1c ..and I immediately noticed that's buggy too, so..: http://hg.dovecot.org/dovecot-1.2/rev/ca4e277a6615 signature.asc Description: This is a digitally signed message part
Re: [Dovecot] patch: list shared namespace
On Tue, 2008-10-28 at 21:07 +0100, Bernhard Herzog wrote:
> One of the main problems the patch addresses is getting a list of all
> users that have mailboxes the logged in user can see. The patch uses a
> dict to cache information about which users have at least one mailbox
> that is visible to other users. The dict doesn't cache which other
> users, though. The cache entry for a given user is updated whenever the
> dovecot-acl-list file in the maildir root directory is updated. This
> ties the implementation to a specific acl backend to an extent, but that
> shouldn't be a problem at the moment.
What I'm worrying here is if all users have shared something. For
example how does this work with global ACLs? IIRC if there's a global
ACL for e.g. "Spam", Dovecot will create dovecot-acl-list with "Spam" in
it for all users. Even without global ACLs the number of users sharing
something might be a bit too large.
So the performance would be a lot better if the dict stored something
like source_user -> dest, where dest would be all the user names and
group names that are included in the user's ACLs (for any mailbox). Then
you'd look only at those users' mailboxes where you or your groups are
mentioned. Negative user/group rules perhaps shouldn't be in the dict.
> I'm not sure the new hook is really needed. The patch could perhaps
> just as well extend the acl_next_hook_mail_storage_created and
> acl_next_hook_mailbox_list_created functions to do the namespace
> creation when they're called for a shared storage or mailbox list.
Perhaps hook_mail_namespaces_created?
Some other things:
v1.2 code supports multiple users per process. That means you can't
really use getenv("USER") and you can't store per-user objects as static
variables. Rather you should hook into hook_mail_user_created and add
the per-user variables to the mail_user structure. See for example
lazy-expunge plugin (struct lazy_expunge_mail_user) or quota plugin how
that works.
i_info("no acl_shared_dict specified; shared namespaces will not be
listed") could be written only if getenv("DEBUG") != NULL.
Is acl_shared_debug stuff only a temporary developer debugging thing, or
will it be useful also for sysadmins?
> All of my tests so far involved a shared namespace of the form
>
> namespace shared {
> separator = /
> prefix = users/%%u/
> location = maildir:.../var/mail/%%u:...
> subscriptions = no
> list = yes
> hidden = no
> }
>
> Also, let's assume two users, ford and arthur with ford's "INBOX/hhgttg"
> available to arthur as "users/ford/INBOX/hhgttg". Arthur may not list
> ford's INBOX, though. In the following the current user is always
> arthur.
>
> I found the following problems:
>
> - LIST response includes namespaces the user doesn't really have access
>to. E.g. if there's another user, zaphod who's made some mailbox
>available to somebody else, but not arthur, arthur still sees
>
>* LIST (\Noselect \HasChildren) "/" "users/zaphod"
>
>Not sure it's worth fixing this, though.
It'll expose other users' names, which isn't good. It needs to be fixed
before I can make a release. Probably not too difficult though. I think
the ACL plugin's mailbox listing code could detect when a shared
namespace prefix is listed and not show it if it doesn't have visible
mailboxes under it. And that's related to the next problem:
> - List with "%" doesn't list all intermediate mailboxes.
Yes, this is actually in my TODO list already. I'll read and aswer to
your next mail about this..
> - The dovecot-acl-list is not always rebuilt, even when it should have
>been, AFAICT. In particular, if the file exists but is empty, it's
>never updated, even when ACL later change. Maybe this is a bug in
>the Kolab branch.
I haven't actually tested dovecot-acl-list all that much, so it's
possible that there are bugs in it.
signature.asc
Description: This is a digitally signed message part
Re: [Dovecot] patch: list shared namespace
On 28.10.2008, Bernhard Herzog wrote: > - List with "%" doesn't list all intermediate mailboxes. > >On the one hand arthur sees this: > > x LIST "" "*" > ... > * LIST (\Noselect \HasChildren) "/" "users/ford" > * LIST (\HasNoChildren) "/" "users/ford/INBOX/hhgttg" > x OK List completed. > >OTOH, with "%" only this: > > x LIST "" "users/ford/%" > x OK List completed. I've looked into this. The problem is that acl_mailbox_list_info_is_visible in src/plugins/acl/acl-mailbox-list.c considers nonexistent mailboxes as not visible. The attached patch fixes that for me. I'm not sure it really is the right fix, though. Maybe it would cause some mailboxes to be listed even though they shouldn't. There's one thing about acl_mailbox_list_info_is_visible and struct acl_mailbox_list_iterate_context that I don't understand. What's the member struct mailbox_info info; used for? Regards, Bernhard -- Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner diff -r 7c615ac48711 src/plugins/acl/acl-mailbox-list.c --- a/src/plugins/acl/acl-mailbox-list.c Thu Oct 30 17:41:02 2008 +0200 +++ b/src/plugins/acl/acl-mailbox-list.c Fri Oct 31 12:43:38 2008 +0100 @@ -187,6 +187,9 @@ acl_mailbox_list_info_is_visible(struct /* skip ACL checks. */ return 1; } + + if ((info->flags & MAILBOX_NONEXISTENT) != 0) + return 1; acl_name = acl_mailbox_list_iter_get_name(&ctx->ctx, info->name); ret = acl_mailbox_list_have_right(ctx->ctx.list, acl_name, signature.asc Description: This is a digitally signed message part.
Re: [Dovecot] patch: list shared namespace
On 28.10.2008, Bernhard Herzog wrote: > I've been working on a patch for dovecot 1.2 from the Kolab branch > (http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/) that > implements listing of shared namespaces. I've got something that works > in some basic way but is still missing some pieces. See the attached > patch, which also contains some installation and configuration notes. The patch is now available in the kolab branch hg repository: http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/rev/c2396923cd2f Bernhard -- Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/ Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998 Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner signature.asc Description: This is a digitally signed message part.
[Dovecot] patch: list shared namespace
Hi,
I've been working on a patch for dovecot 1.2 from the Kolab branch
(http://hg.intevation.org/kolab/dovecot-1.2_kolab-branch/) that
implements listing of shared namespaces. I've got something that works
in some basic way but is still missing some pieces. See the attached
patch, which also contains some installation and configuration notes.
Implementation notes:
One of the main problems the patch addresses is getting a list of all
users that have mailboxes the logged in user can see. The patch uses a
dict to cache information about which users have at least one mailbox
that is visible to other users. The dict doesn't cache which other
users, though. The cache entry for a given user is updated whenever the
dovecot-acl-list file in the maildir root directory is updated. This
ties the implementation to a specific acl backend to an extent, but that
shouldn't be a problem at the moment.
Another problem is that namespaces for all those users have to be
created. The patch does that in shared-storage.c when the shared
storage is created. At this stage of development of the patch that
works well enough, I think, but it might be better to update the
namespaces whenever a list iterator is created.
To avoid unnecessary coupling between the shared namespace code and the
ACL plugin, the shared namespace code has a hook that it calls when it
needs a list of all the users who may have mailboxes visible to the
current user. The ACL plugin sets that hook and uses the dict to
produce that list. This way, the ACL plugin depends on the shared
namespace code but not the other way round and all the dict handling is
in the ACL plugin.
I'm not sure the new hook is really needed. The patch could perhaps
just as well extend the acl_next_hook_mail_storage_created and
acl_next_hook_mailbox_list_created functions to do the namespace
creation when they're called for a shared storage or mailbox list.
Problems:
All of my tests so far involved a shared namespace of the form
namespace shared {
separator = /
prefix = users/%%u/
location = maildir:.../var/mail/%%u:...
subscriptions = no
list = yes
hidden = no
}
Also, let's assume two users, ford and arthur with ford's "INBOX/hhgttg"
available to arthur as "users/ford/INBOX/hhgttg". Arthur may not list
ford's INBOX, though. In the following the current user is always
arthur.
I found the following problems:
- LIST response includes namespaces the user doesn't really have access
to. E.g. if there's another user, zaphod who's made some mailbox
available to somebody else, but not arthur, arthur still sees
* LIST (\Noselect \HasChildren) "/" "users/zaphod"
Not sure it's worth fixing this, though.
- List with "%" doesn't list all intermediate mailboxes.
On the one hand arthur sees this:
x LIST "" "*"
...
* LIST (\Noselect \HasChildren) "/" "users/ford"
* LIST (\HasNoChildren) "/" "users/ford/INBOX/hhgttg"
x OK List completed.
OTOH, with "%" only this:
x LIST "" "users/ford/%"
x OK List completed.
cyrus shows
x LIST "" "users/ford/%"
* LIST (\Noselect \HasChildren) "/" "users/ford/INBOX"
x OK List completed.
At least Kontact resp. KMail rely on this.
- The dovecot-acl-list is not always rebuilt, even when it should have
been, AFAICT. In particular, if the file exists but is empty, it's
never updated, even when ACL later change. Maybe this is a bug in
the Kolab branch.
Cheers,
Bernhard
--
Bernhard Herzog | ++49-541-335 08 30 | http://www.intevation.de/
Intevation GmbH, Neuer Graben 17, 49074 Osnabrück | AG Osnabrück, HR B 18998
Geschäftsführer: Frank Koormann, Bernhard Reiter, Dr. Jan-Oliver Wagner
Patch to add listing of shared namespaces to dovecot 1.2.
The patch was produced for dovecot revision 97ed7b408525.
To install, apply the patch, and regenerate the Makefile.in files with
autogen.sh and rerun configure and make.
To configure, create an entry for dict section of dovecot.conf like this:
acl_shared_dict = sqlite:$PREFIX/etc/acl-shared-dict.conf
with the acl-shared-dict.conf containing this:
connect = $PREFIX/var/lib/dovecot/acl-shared-ns.sqlite
map {
table = acl_shared_ns
pattern = shared/acl_shared_ns/$owner
value_field = has_visible_folders
fields {
owner = $owner
}
}
The corresponding table in the sqlite database can be created with
CREATE TABLE acl_shared_ns (
owner,
has_visible_folders,
PRIMARY KEY (owner) ON CONFLICT REPLACE
);
In the imap section of dovecot.conf, add
acl_shared_dict = proxy::acl_shared_dict
diff -r 97ed7b408525 src/lib-storage/index/shared/shared-storage.c
--- a/src/lib-storage/index/shared/shared-storage.c Tue Oct 28 10:08:33 2008 +0100
+++ b/src/lib-storage/index/shared/shared-storage.c Tue Oct 28 16:44:46 2008 +0100
@@ -15,6 +15,10 @@ static MODULE_CONTEXT_DEFINE_INIT(shared
static MODULE_CONTEXT_DEFINE_INIT(shared_mailbox_l
