[PATCH 2/2] lib-sieve: actions - Create fileinto mailbox without the autocreate flag

Stop setting MAILBOX_FLAG_AUTO_CREATE/SUBSCRIBE in act_store. Instead, on
mailbox_open() returning MAIL_ERROR_NOTFOUND, create the mailbox explicitly
with sieve_act_store_create_mailbox() when autocreation is enabled (the
:create side effect already creates it explicitly).

This makes the creation go through the normal ACL CREATE gate, so a fileinto
(with lda_mailbox_autocreate=yes) or fileinto :create can no longer create a
mailbox in a shared or public namespace that the user lacks the CREATE right
for.

A permission failure to create the mailbox is classified by
sieve_act_store_create_error_status(): for a user's personal script it is a
permanent failure (implicit keep), while for a global (administrator-defined)
script it is deferred (temporary failure), so the administrator notices the
script is behaving in a way they did not intend.

Gbp-Pq: Name 0002-lib-sieve-actions-Create-fileinto-mailbox-without-th.patch

diff --git a/pigeonhole/src/lib-sieve/sieve-actions.c b/pigeonhole/src/lib-sieve/sieve-actions.c
index 342c68a223888243f43b051ac2b50bb5f06569bd..df8851df1a943298c94105c16e5c6dcb9e501530 100644 (file)
--- a/pigeonhole/src/lib-sieve/sieve-actions.c
+++ b/pigeonhole/src/lib-sieve/sieve-actions.c
@@ -372,6 +372,31 @@ void sieve_act_store_get_storage_error(const struct sieve_action_exec_env *aenv,
                                                &trans->error_code));
 }
 
+int sieve_act_store_create_error_status(
+       const struct sieve_action_exec_env *aenv, enum mail_error error_code)
+{
+       const struct sieve_execute_env *eenv = aenv->exec_env;
+
+       switch (error_code) {
+       case MAIL_ERROR_NONE:
+               return SIEVE_EXEC_OK;
+       case MAIL_ERROR_TEMP:
+               return SIEVE_EXEC_TEMP_FAILURE;
+       case MAIL_ERROR_PERM:
+               /* The mailbox cannot be created because of insufficient
+                  permissions. This is a permanent condition. For a user's
+                  personal script, fall through to implicit keep rather than
+                  deferring the message. For a global (administrator-defined)
+                  script, defer instead, so that the administrator notices the
+                  script is behaving in a way they did not intend. */
+               if ((eenv->flags & SIEVE_EXECUTE_FLAG_NOGLOBAL) != 0)
+                       return SIEVE_EXEC_FAILURE;
+               return SIEVE_EXEC_TEMP_FAILURE;
+       default:
+               return SIEVE_EXEC_FAILURE;
+       }
+}
+
 int sieve_act_store_create_mailbox(const struct sieve_action_exec_env *aenv,
                                   struct act_store_transaction *trans)
 {
@@ -384,13 +409,13 @@ int sieve_act_store_create_mailbox(const struct sieve_action_exec_env *aenv,
        /* Create mailbox */
        if (mailbox_create(box, NULL, FALSE) < 0) {
                sieve_act_store_get_storage_error(aenv, trans);
-               if (trans->error_code == MAIL_ERROR_EXISTS) {
-                       trans->error = NULL;
-                       trans->error_code = MAIL_ERROR_NONE;
-               } else {
-                       return (trans->error_code == MAIL_ERROR_TEMP ?
-                               SIEVE_EXEC_TEMP_FAILURE : SIEVE_EXEC_FAILURE);
+               if (trans->error_code != MAIL_ERROR_EXISTS) {
+                       return sieve_act_store_create_error_status(
+                               aenv, trans->error_code);
                }
+               /* Lost a race with another session; the mailbox exists now. */
+               trans->error = NULL;
+               trans->error_code = MAIL_ERROR_NONE;
        }
 
        /* Subscribe to it if necessary */
@@ -402,10 +427,9 @@ int sieve_act_store_create_mailbox(const struct sieve_action_exec_env *aenv,
 
        /* Try opening again */
        if (mailbox_open(box) < 0) {
-               /* Failed definitively */
                sieve_act_store_get_storage_error(aenv, trans);
-               return (trans->error_code == MAIL_ERROR_TEMP ?
-                       SIEVE_EXEC_TEMP_FAILURE : SIEVE_EXEC_FAILURE);
+               return sieve_act_store_create_error_status(
+                       aenv, trans->error_code);
        }
        return SIEVE_EXEC_OK;
 }
@@ -434,10 +458,6 @@ act_store_mailbox_alloc(const struct sieve_action_exec_env *aenv,
                return FALSE;
        }
 
-       if (eenv->scriptenv->mailbox_autocreate)
-               flags |= MAILBOX_FLAG_AUTO_CREATE;
-       if (eenv->scriptenv->mailbox_autosubscribe)
-               flags |= MAILBOX_FLAG_AUTO_SUBSCRIBE;
        *box_r = box = mailbox_alloc_for_user(eenv->scriptenv->user, mailbox,
                                              flags);
        *storage = mailbox_get_storage(box);
@@ -640,6 +660,16 @@ act_store_execute(const struct sieve_action_exec_env *aenv, void *tr_context,
                }
        }
 
+       /* Create the mailbox if it does not exist yet and autocreation is
+          enabled (e.g. lda_mailbox_autocreate=yes). The :create side effect
+          creates it earlier; this handles a plain fileinto. */
+       if (trans->error_code == MAIL_ERROR_NOTFOUND &&
+           eenv->scriptenv->mailbox_autocreate) {
+               int cstatus = sieve_act_store_create_mailbox(aenv, trans);
+               if (cstatus != SIEVE_EXEC_OK)
+                       return cstatus;
+       }
+
        /* Exit early if transaction already failed */
        switch (trans->error_code) {
        case MAIL_ERROR_NONE:

diff --git a/pigeonhole/src/lib-sieve/sieve-actions.h b/pigeonhole/src/lib-sieve/sieve-actions.h
index 10f5daa306e0d30852ab81455b1aa02900c85559..0a365f3e0b7e1570ea8793a2fcd19af408559fc3 100644 (file)
--- a/pigeonhole/src/lib-sieve/sieve-actions.h
+++ b/pigeonhole/src/lib-sieve/sieve-actions.h
@@ -246,8 +246,17 @@ void sieve_act_store_add_flags(const struct sieve_action_exec_env *aenv,
 void sieve_act_store_get_storage_error(const struct sieve_action_exec_env *aenv,
                                       struct act_store_transaction *trans);
 
+/* Map a failure to create the target mailbox to an execution status. A
+   permanent failure (e.g. the user lacks permission to create the mailbox)
+   makes a user's personal script fall through to implicit keep, but is
+   deferred for a global (administrator-defined) script, so that the
+   unintended behaviour is surfaced to the administrator. */
+int sieve_act_store_create_error_status(
+       const struct sieve_action_exec_env *aenv, enum mail_error error_code);
+
 /* Create the target mailbox of the store transaction (and subscribe to it
-   when configured). */
+   when configured). Returns SIEVE_EXEC_OK on success, or the status from
+   sieve_act_store_create_error_status() on failure. */
 int sieve_act_store_create_mailbox(const struct sieve_action_exec_env *aenv,
                                   struct act_store_transaction *trans);