email: Code cleanup

- Move render_tabs into plinth.views
- Move admin_tabs and user_tabs into the class
- New interproc module contains inter-process functions
- New helper methods in TabMixin

plinth/modules/email_server/audit/domain.py

@@ -1,7 +1,6 @@
 """Configure email domains"""
 # SPDX-License-Identifier: AGPL-3.0-or-later
 
-import contextlib
 import io
 import json
 import os
@@ -9,7 +8,6 @@ import re
 import select
 import sys
 import time
-import uuid
 
 from types import SimpleNamespace
 
@@ -18,7 +16,7 @@ from plinth.errors import ActionError
 from plinth.actions import superuser_run
 
 from . import models
-from plinth.modules.email_server import postconf
+from plinth.modules.email_server import interproc, postconf
 
 EXIT_VALIDATION = 40
 
@@ -140,7 +138,7 @@ def clean_mydestination(raw):
 
 
 def su_set_mailname(cleaned):
-    with _atomically_rewrite('/etc/mailname', 'x') as fd:
+    with interproc.atomically_rewrite('/etc/mailname') as fd:
         fd.write(cleaned)
         fd.write('\n')
 
@@ -176,28 +174,3 @@ def _stdin_readline():
         return line.decode('utf8')
     except ValueError as e:
         raise ClientError('UTF-8 decode failed') from e
-
-
-@contextlib.contextmanager
-def _atomically_rewrite(filepath, mode):
-    successful = False
-    tmp = '%s.%s.plinth-tmp' % (filepath, uuid.uuid4().hex)
-    fd = open(tmp, mode)
-
-    try:
-        # Let client write to a temporary file
-        yield fd
-        successful = True
-    finally:
-        fd.close()
-
-    try:
-        if successful:
-            # Invoke rename(2) to atomically replace the original
-            os.rename(tmp, filepath)
-    finally:
-        # Delete temp file
-        try:
-            os.unlink(tmp)
-        except FileNotFoundError:
-            pass

plinth/modules/email_server/audit/home.py

@@ -10,6 +10,8 @@ from django.utils.translation import ugettext_lazy as _
 from plinth.actions import superuser_run
 from plinth.errors import ActionError
 
+from plinth.modules.email_server import interproc
+
 logger = logging.getLogger(__name__)
 
 
@@ -65,7 +67,5 @@ def action_mk(arg_type, user_info):
     args.extend(['/bin/sh', '-c', 'mkdir -p ~'])
     completed = subprocess.run(args, capture_output=True)
     if completed.returncode != 0:
-        logger.critical('Subprocess returned %d', completed.returncode)
+        interproc.log_subprocess(completed)
-        logger.critical('Stdout: %r', completed.stdout)
-        logger.critical('Stderr: %r', completed.stderr)
         raise OSError('Could not create home directory')

plinth/modules/email_server/data/etc/dovecot/conf.d/05-freedombox-auth.conf

@@ -15,7 +15,7 @@ passdb {
 userdb {
   # UID number lookup (10001@example.com)
   driver = ldap
-  args = /etc/dovecot/freedombox-ldap-userdb-aliases.conf.ext
+  args = /etc/dovecot/freedombox-ldap-userdb-uid.conf.ext
   result_failure = continue
   result_internalfail = return-fail
   result_success = return-ok

plinth/modules/email_server/data/etc/dovecot/freedombox-ldap-userdb-uid.conf.ext

@@ -11,8 +11,10 @@ user_attrs = \
   =user=%{ldap:uid}, \
   =mail=maildir:~/Maildir:LAYOUT=index
 
+# Support user lookup by UID number
+
 user_filter = \
-  (&(objectClass=posixAccount)(!(uidNumber=0))(uidNumber=%n)(!(uid=%n)))
+  (&(objectClass=posixAccount)(!(uidNumber=0))(uidNumber=%n))
 
 # doveadm -A
 

plinth/modules/email_server/data/etc/dovecot/freedombox-ldap-userdb.conf.ext

@@ -10,6 +10,8 @@ user_attrs = \
   =gid=%{ldap:gidNumber}, \
   =mail=maildir:~/Maildir:LAYOUT=index
 
+# Support user lookup by username
+
 user_filter = (&(objectClass=posixAccount)(uid=%Ln)(!(uidNumber=0)))
 
 # For doveadm

plinth/modules/email_server/data/etc/dovecot/freedombox-sieve-after/README.txt

@@ -0,0 +1,2 @@
+DO NOT PUT PERSONAL ITEMS HERE!
+This folder in its entirety is managed by FreedomBox.

plinth/modules/email_server/interproc.py

@@ -0,0 +1,39 @@
+# SPDX-License-Identifier: AGPL-3.0-or-later
+
+import contextlib
+import logging
+import os
+import uuid
+
+logger = logging.getLogger(__name__)
+
+
+def log_subprocess(result):
+    logger.critical('Subprocess returned %d', result.returncode)
+    logger.critical('Stdout: %r', result.stdout)
+    logger.critical('Stderr: %r', result.stderr)
+
+
+@contextlib.contextmanager
+def atomically_rewrite(filepath):
+    successful = False
+    tmp = '%s.%s.plinth-tmp' % (filepath, uuid.uuid4().hex)
+    fd = open(tmp, 'x')
+
+    try:
+        # Let client write to a temporary file
+        yield fd
+        successful = True
+    finally:
+        fd.close()
+
+    try:
+        if successful:
+            # Invoke rename(2) to atomically replace the original
+            os.rename(tmp, filepath)
+    finally:
+        # Delete temp file
+        try:
+            os.unlink(tmp)
+        except FileNotFoundError:
+            pass

plinth/modules/email_server/lock.py

@@ -8,6 +8,8 @@ import re
 import subprocess
 import threading
 
+from . import interproc
+
 lock_name_pattern = re.compile('^[0-9a-zA-Z_-]+$')
 logger = logging.getLogger(__name__)
 
@@ -89,9 +91,7 @@ class Mutex:
 
         completed = subprocess.run(args, capture_output=True)
         if completed.returncode != 0:
-            logger.critical('Subprocess returned %d', completed.returncode)
+            interproc.log_subprocess(completed)
-            logger.critical('Stdout: %r', completed.stdout)
-            logger.critical('Stderr: %r', completed.stderr)
             raise OSError('Could not create ' + self.lock_path)
 
     def _checked_setresuid(self, ruid, euid, suid):

plinth/modules/email_server/views.py

@@ -9,60 +9,39 @@ from django.core.exceptions import ValidationError
 from django.utils.html import escape
 from django.utils.translation import ugettext_lazy as _
 from django.views.generic.base import TemplateView, View
-from plinth.views import AppView
+from plinth.views import AppView, render_tabs
 
 from . import aliases
 from . import audit
 from . import forms
 
-admin_tabs = [
-    ('', _('Home')),
-    ('my_mail', _('My Mail')),
-    ('my_aliases', _('My Aliases')),
-    ('security', _('Security')),
-    ('domains', _('Domains'))
-]
-
-user_tabs = [
-    ('my_mail', _('Home')),
-    ('my_aliases', _('My Aliases'))
-]
-
 
 class TabMixin(View):
+    admin_tabs = [
+        ('', _('Home')),
+        ('my_mail', _('My Mail')),
+        ('my_aliases', _('My Aliases')),
+        ('security', _('Security')),
+        ('domains', _('Domains'))
+    ]
+
+    user_tabs = [
+        ('my_mail', _('Home')),
+        ('my_aliases', _('My Aliases'))
+    ]
+
     def get_context_data(self, *args, **kwargs):
         # Retrieve context data from the next method in the MRO
         context = super().get_context_data(*args, **kwargs)
         # Populate context with customized data
-        context['tabs'] = self.render_tabs()
+        context['tabs'] = self.render_dynamic_tabs()
         return context
 
-    def render_tabs(self):
+    def render_dynamic_tabs(self):
         if plinth.utils.is_user_admin(self.request):
-            return self.__render_tabs(self.request.path, admin_tabs)
+            return render_tabs(self.request.path, self.admin_tabs)
         else:
-            return self.__render_tabs(self.request.path, user_tabs)
+            return render_tabs(self.request.path, self.user_tabs)
-
-    @staticmethod
-    def __render_tabs(path, tab_data):
-        sb = io.StringIO()
-        sb.write('<ul class="nav nav-tabs">')
-
-        for page_name, link_text in tab_data:
-            cls = 'active' if path.endswith('/' + page_name) else ''
-            href = '#' if cls == 'active' else ('./' + page_name)
-            # -- Begin list
-            sb.write('<li class="nav-item">')
-            # -- Begin link
-            sb.write('<a class="nav-link {}" '.format(cls))
-            sb.write('href="{}">'.format(escape(href)))
-            sb.write('{}</a>'.format(escape(link_text)))
-            # -- End link
-            sb.write('</li>')
-            # -- End list
-
-        sb.write('</ul>')
-        return sb.getvalue()
 
     def render_validation_error(self, validation_error, status=400):
         context = self.get_context_data()
@@ -74,6 +53,14 @@ class TabMixin(View):
         context['error'] = [str(exception)]
         return self.render_to_response(context, status=status)
 
+    def catch_exceptions(self, function, request):
+        try:
+            return function(request)
+        except ValidationError as validation_error:
+            return self.render_validation_error(validation_error)
+        except Exception as error:
+            return self.render_exception(error)
+
     def find_button(self, post):
         key_filter = (k for k in post.keys() if k.startswith('btn_'))
         lst = list(itertools.islice(key_filter, 2))
@@ -109,12 +96,7 @@ class MyMailView(TabMixin, TemplateView):
         return context
 
     def post(self, request):
-        try:
+        return self.catch_exceptions(self._post, request)
-            return self._post(request)
-        except ValidationError as validation_error:
-            return self.render_validation_error(validation_error)
-        except RuntimeError as runtime_error:
-            return self.render_exception(runtime_error)
 
     def _post(self, request):
         if 'btn_mkhome' not in request.POST:
@@ -198,12 +180,7 @@ class AliasView(TabMixin, TemplateView):
         return context
 
     def post(self, request):
-        try:
+        return self.catch_exceptions(self._post, request)
-            return self._post(request)
-        except ValidationError as validation_error:
-            return self.render_validation_error(validation_error)
-        except Exception as exception:
-            return self.render_exception(exception)
 
     def _post(self, request):
         form = self.find_form(request.POST)
@@ -257,12 +234,7 @@ class DomainView(TabMixin, TemplateView):
         return context
 
     def post(self, request):
-        try:
+        return self.catch_exceptions(self._post, request)
-            return self._post(request)
-        except ValidationError as validation_error:
-            return self.render_validation_error(validation_error)
-        except RuntimeError as runtime_error:
-            return self.render_exception(runtime_error)
 
     def _post(self, request):
         changed = {}

plinth/views.py

@@ -3,6 +3,7 @@
 Main FreedomBox views.
 """
 
+import io
 import time
 import urllib.parse
 
@@ -11,6 +12,7 @@ from django.core.exceptions import ImproperlyConfigured
 from django.http import Http404, HttpResponseBadRequest, HttpResponseRedirect
 from django.template.response import TemplateResponse
 from django.urls import reverse
+from django.utils.html import escape
 from django.utils.translation import ugettext as _
 from django.views.generic import TemplateView
 from django.views.generic.edit import FormView
@@ -321,3 +323,30 @@ def notification_dismiss(request, id):
     notes[0].dismiss()
 
     return HttpResponseRedirect(_get_redirect_url_from_param(request))
+
+
+def render_tabs(request_path, tab_data):
+    """Generate a Bootstrap tab group and return the raw HTML
+
+    :param request_path: value of `request.path`
+    :param tab_data: a list of (page_name, link_text) tuples
+    :returns: raw HTML of the tabs
+    """
+    sb = io.StringIO()
+    sb.write('<ul class="nav nav-tabs">')
+
+    for page_name, link_text in tab_data:
+        cls = 'active' if request_path.endswith('/' + page_name) else ''
+        href = '#' if cls == 'active' else ('./' + page_name)
+        # -- Begin list
+        sb.write('<li class="nav-item">')
+        # -- Begin link
+        sb.write('<a class="nav-link {}" '.format(cls))
+        sb.write('href="{}">'.format(escape(href)))
+        sb.write('{}</a>'.format(escape(link_text)))
+        # -- End link
+        sb.write('</li>')
+        # -- End list
+
+    sb.write('</ul>')
+    return sb.getvalue()