Re: [Dovecot] patch: list shared namespace

2008-11-19 Thread Timo Sirainen
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 no rights
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

2008-11-19 Thread Timo Sirainen
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

2008-11-07 Thread Timo Sirainen
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

2008-11-04 Thread Bernhard Herzog
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

2008-11-04 Thread Bernhard Herzog
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

2008-11-04 Thread Timo Sirainen

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

2008-11-03 Thread Bernhard Herzog
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

2008-11-01 Thread Timo Sirainen
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

2008-10-31 Thread Bernhard Herzog
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

2008-10-31 Thread Timo Sirainen
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

2008-10-31 Thread Timo Sirainen
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

2008-10-31 Thread Bernhard Herzog
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

2008-10-31 Thread Timo Sirainen

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

2008-10-31 Thread Timo Sirainen

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

2008-10-29 Thread Bernhard Herzog
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

2008-10-28 Thread Bernhard Herzog
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_list_module,