email_server: Don't use user IDs when performing lookups

- Typical mail systems are configured to work on usernames or virtual usernames.
UIDs/GIDs are only needed at the final moment when delivering mails to user
inboxes that need to have proper UID/GID set.

- This makes it easy for dovecot to simply use PAM authentication instead of
having to use LDAP.

- Trying to hide UID from email headers is no longer necessary. Received: header
is important for debugging mail delivery across the chain. Don't miss out.

Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: James Valleroy <jvalleroy@mailbox.org>

plinth/modules/email_server/aliases.py

@@ -11,7 +11,7 @@ from plinth import actions
 
 @dataclass
 class Alias:
-    value: int
+    value: str
     name: str
     enabled: bool = field(init=False)
     status: InitVar[int]
@@ -34,11 +34,11 @@ def _get_cursor():
         connection.close()
 
 
-def get(uid):
+def get(username):
     """Get all aliases of a user."""
     query = 'SELECT name, value, status FROM alias WHERE value=?'
     with _get_cursor() as cursor:
-        rows = cursor.execute(query, (uid, ))
+        rows = cursor.execute(query, (username, ))
         return [Alias(**row) for row in rows]
 
 
@@ -56,40 +56,40 @@ def exists(name):
         return cursor.fetchone()[0] != 0
 
 
-def put(uid, name):
+def put(username, name):
     """Insert if not exists a new alias."""
     query = 'INSERT INTO alias (name, value, status) VALUES (?, ?, ?)'
     with _get_cursor() as cursor:
         try:
-            cursor.execute(query, (name, uid, 1))
+            cursor.execute(query, (name, username, 1))
         except sqlite3.IntegrityError:
             pass  # Alias exists, rare since we are already checking
 
 
-def delete(uid, aliases):
+def delete(username, aliases):
     """Delete a set of aliases."""
     query = 'DELETE FROM alias WHERE value=? AND name=?'
-    parameter_seq = ((uid, name) for name in aliases)
+    parameter_seq = ((username, name) for name in aliases)
     with _get_cursor() as cursor:
         cursor.execute('BEGIN')
         cursor.executemany(query, parameter_seq)
         cursor.execute('COMMIT')
 
 
-def enable(uid, aliases):
+def enable(username, aliases):
     """Enable a list of aliases."""
-    return _set_status(uid, aliases, 1)
+    return _set_status(username, aliases, 1)
 
 
-def disable(uid, aliases):
+def disable(username, aliases):
     """Disable a list of aliases."""
-    return _set_status(uid, aliases, 0)
+    return _set_status(username, aliases, 0)
 
 
-def _set_status(uid, aliases, status):
+def _set_status(username, aliases, status):
     """Set the status value of a list of aliases."""
     query = 'UPDATE alias SET status=? WHERE value=? AND name=?'
-    parameter_seq = ((status, uid, name) for name in aliases)
+    parameter_seq = ((status, username, name) for name in aliases)
     with _get_cursor() as cursor:
         cursor.execute('BEGIN')
         cursor.executemany(query, parameter_seq)
@@ -106,7 +106,7 @@ PRAGMA journal_mode=WAL;
 BEGIN;
 CREATE TABLE IF NOT EXISTS alias (
     name TEXT NOT NULL,
-    value INTEGER NOT NULL,
+    value TEXT NOT NULL,
     status INTEGER NOT NULL,
     PRIMARY KEY (name)
 );

plinth/modules/email_server/audit/ldap.py

@@ -54,9 +54,7 @@ default_smtps_options = {
 }
 
 MAILSRV_DIR = '/var/lib/plinth/mailsrv'
-ETC_ALIASES = 'hash:/etc/aliases'
-BEFORE_ALIASES = 'ldap:/etc/postfix/freedombox-username-to-uid-number.cf'
-AFTER_ALIASES = 'sqlite:/etc/postfix/freedombox-aliases.cf'
+SQLITE_ALIASES = 'sqlite:/etc/postfix/freedombox-aliases.cf'
 
 logger = logging.getLogger(__name__)
 
@@ -123,45 +121,26 @@ def check_alias_maps(title=''):
     """Check the ability to mail to usernames and user aliases"""
     diagnosis = models.MainCfDiagnosis(title)
 
-    analysis = models.AliasMapsAnalysis()
-    analysis.parsed = postconf.parse_maps_by_key_unsafe('alias_maps')
-    analysis.isystem = list_find(analysis.parsed, ETC_ALIASES)
-    analysis.ibefore = list_find(analysis.parsed, BEFORE_ALIASES)
-    analysis.iafter = list_find(analysis.parsed, AFTER_ALIASES)
-
-    if analysis.ibefore == -1 or analysis.iafter == -1:
-        diagnosis.flag_once('alias_maps', user=analysis)
+    alias_maps = postconf.get_unsafe('alias_maps').replace(',', ' ').split(' ')
+    if SQLITE_ALIASES not in alias_maps:
+        diagnosis.flag_once('alias_maps', user=alias_maps)
         diagnosis.critical('Required maps not in list')
-    if analysis.ibefore > analysis.iafter:
-        diagnosis.flag_once('alias_maps', user=analysis)
-        diagnosis.critical('Insecure map order')
 
     return diagnosis
 
 
 def fix_alias_maps(diagnosis):
-    diagnosis.repair('alias_maps', rearrange_alias_maps)
+
+    def fix_value(alias_maps):
+        if SQLITE_ALIASES not in alias_maps:
+            alias_maps.append(SQLITE_ALIASES)
+
+        return ' '.join(alias_maps)
+
+    diagnosis.repair('alias_maps', fix_value)
     diagnosis.apply_changes(postconf.set_many_unsafe)
 
 
-def rearrange_alias_maps(analysis):
-    # Delete *all* references to BEFORE_ALIASES and AFTER_ALIASES
-    for i in range(len(analysis.parsed)):
-        if analysis.parsed[i] in (BEFORE_ALIASES, AFTER_ALIASES):
-            analysis.parsed[i] = ''
-    # Does hash:/etc/aliases exist in list?
-    if analysis.isystem >= 0:
-        # Put the maps around hash:/etc/aliases
-        val = '%s %s %s' % (BEFORE_ALIASES, ETC_ALIASES, AFTER_ALIASES)
-        analysis.parsed[analysis.isystem] = val
-    else:
-        # To the end
-        analysis.parsed.append(BEFORE_ALIASES)
-        analysis.parsed.append(AFTER_ALIASES)
-    # List -> string
-    return ' '.join(filter(None, analysis.parsed))
-
-
 def check_local_recipient_maps(title=''):
     diagnosis = models.MainCfDiagnosis(title)
     lrcpt_maps = postconf.parse_maps_by_key_unsafe('local_recipient_maps')

plinth/modules/email_server/audit/models.py

@@ -1,9 +1,7 @@
 # SPDX-License-Identifier: AGPL-3.0-or-later
 """Models of the audit module"""
 
-import dataclasses
 import logging
-import typing
 
 logger = logging.getLogger(__name__)
 
@@ -207,14 +205,3 @@ class MainCfDiagnosis(Diagnosis):
         contains an unresolved issue (i.e. an uncorrected key)"""
         if None in self.advice.values():
             raise UnresolvedIssueError('Assertion failed')
-
-
-@dataclasses.dataclass(init=False)
-class AliasMapsAnalysis:
-    parsed = typing.List[str]
-    ibefore = int
-    isystem = int
-    iafter = int
-
-    def __init__(self):
-        pass

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

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

plinth/modules/email_server/data/etc/dovecot/conf.d/90-freedombox-lmtp.conf

@@ -1,10 +1,6 @@
 # Direct edits to this file will be lost!
 # Manage your settings on Plinth <https://localhost/plinth/apps/email_server>
 
-# Privacy: remove the recipient's UID number from email headers
-lmtp_add_received_header = no
-lmtp_hdr_delivery_address = original
-
 protocol lmtp {
   mail_plugins = $mail_plugins sieve
 }

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

@@ -1,22 +0,0 @@
-# Direct edits to this file will be lost!
-# Manage your settings on Plinth https://localhost/plinth/apps/email_server
-
-uris = ldap://127.0.0.1
-base = dc=thisbox
-
-user_attrs = \
-  =home=%{ldap:homeDirectory}, \
-  =uid=%{ldap:uidNumber}, \
-  =gid=%{ldap:gidNumber}, \
-  =user=%{ldap:uid}, \
-  =mail=maildir:~/Maildir:LAYOUT=index
-
-# Support user lookup by UID number
-
-user_filter = \
-  (&(objectClass=posixAccount)(!(uidNumber=0))(uidNumber=%n))
-
-# doveadm -A
-
-iterate_attrs = =user=%{ldap:uid}
-iterate_filter = (objectClass=posixAccount)

plinth/modules/email_server/data/etc/postfix/freedombox-username-to-uid-number.cf

@@ -1,10 +0,0 @@
-# This file is managed by FreedomBox
-
-# Map user name to UID number
-
-bind = no
-server_host = ldap://127.0.0.1
-search_base = dc=thisbox
-query_filter = (&(objectClass=posixAccount)(uid=%s))
-result_attribute = uidNumber
-result_format = %s@localhost

plinth/modules/email_server/views.py

@@ -2,7 +2,6 @@
 """
 Views for the email app.
 """
-import pwd
 
 from django.contrib import messages
 from django.core.exceptions import ValidationError
@@ -147,13 +146,9 @@ class AliasView(FormView):
 
         return kwargs
 
-    def _get_uid(self):
-        """Return the UID of the user that made the request."""
-        return pwd.getpwnam(self.request.user.username).pw_uid
-
     def _get_current_aliases(self):
         """Return current list of aliases."""
-        return aliases_module.get(self._get_uid())
+        return aliases_module.get(self.request.user.username)
 
     def get_context_data(self, *args, **kwargs):
         context = super().get_context_data(*args, **kwargs)
@@ -185,17 +180,18 @@ class AliasView(FormView):
         """Handle a valid alias list form operation."""
         aliases = form.cleaned_data['aliases']
         action = form.cleaned_data['action']
-        uid = self._get_uid()
+        username = self.request.user.username
         if action == 'delete':
-            aliases_module.delete(uid, aliases)
+            aliases_module.delete(username, aliases)
         elif action == 'disable':
-            aliases_module.disable(uid, aliases)
+            aliases_module.disable(username, aliases)
         elif action == 'enable':
-            aliases_module.enable(uid, aliases)
+            aliases_module.enable(username, aliases)
 
     def _create_form_valid(self, form):
         """Handle a valid create alias form operation."""
-        aliases_module.put(self._get_uid(), form.cleaned_data['alias'])
+        username = self.request.user.username
+        aliases_module.put(username, form.cleaned_data['alias'])
 
 
 class DomainsView(FormView):