From 56f48ed6b854f21b5a9cc532ea4795d359803cf1 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Sat, 25 Oct 2014 17:14:33 +0200 Subject: [PATCH] Better settings to control manual learning. see #622 --- doc/extending/events.rst | 7 +-- modoboa/extensions/amavis/app_settings.py | 39 ++++++++++----- .../extensions/amavis/general_callbacks.py | 47 +++++++++++++++-- modoboa/extensions/amavis/lib.py | 50 +++++++++++++++---- .../templates/amavis/main_action_bar.html | 4 +- .../amavis/templatetags/amavis_tags.py | 48 +++++++++++------- modoboa/extensions/amavis/views.py | 16 +++--- 7 files changed, 152 insertions(+), 59 deletions(-) diff --git a/doc/extending/events.rst b/doc/extending/events.rst index 2741bd421..6d51a7ea3 100644 --- a/doc/extending/events.rst +++ b/doc/extending/events.rst @@ -712,10 +712,11 @@ Raised when an existing mailbox is modified. *Callback prototype*:: - def callback(newmailbox, oldmailbox): pass + def callback(mailbox): pass -* ``newmailbox`` is a ``Mailbox`` instance containing the new values -* ``oldmailbox`` is a ``Mailbox`` instance containing the old values +* ``mailbox`` is the ``Mailbox`` modified instance. It contains a + ``old_full_address`` extra field to check if the address was + modified. PasswordChange ============== diff --git a/modoboa/extensions/amavis/app_settings.py b/modoboa/extensions/amavis/app_settings.py index de5c43b77..bb0874caa 100644 --- a/modoboa/extensions/amavis/app_settings.py +++ b/modoboa/extensions/amavis/app_settings.py @@ -74,12 +74,22 @@ class ParametersForm(AdminParametersForm): help_text=_("The e-mail address used to send notitications") ) - lsep = SeparatorField(label=_("Learning from messages")) + lsep = SeparatorField(label=_("Manual learning")) - amavis_is_local = YesNoField( - label=_("Is amavis local?"), + manual_learning = YesNoField( + label=_("Enable manual learning"), initial="yes", - help_text=_("Tell if amavis is running on the same server than modoboa") + help_text=_( + "Allow users to manually train Spamassassin about spam and non-spam" + ) + ) + + sa_is_local = YesNoField( + label=_("Is Spamassassin local?"), + initial="yes", + help_text=_( + "Tell if spamassassin is running on the same server than modoboa" + ) ) default_user = forms.CharField( @@ -90,12 +100,6 @@ class ParametersForm(AdminParametersForm): ) ) - per_user_bayesian_filter = YesNoField( - label=_("Enable per-user bayesian filters?"), - initial="no", - help_text=_("Allow users to personally train spamassassin") - ) - spamd_address = forms.CharField( label=_("Spamd address"), initial="127.0.0.1", @@ -108,12 +112,23 @@ class ParametersForm(AdminParametersForm): help_text=_("The TCP port spamd is listening on") ) + user_manual_learning = YesNoField( + label=_("Enable per-user manual learning"), + initial="no", + help_text=_("Allow users to personally tell Spamassassin about " + "spam and non-spam") + ) + visibility_rules = { "am_pdp_host": "am_pdp_mode=inet", "am_pdp_port": "am_pdp_mode=inet", "am_pdp_socket": "am_pdp_mode=unix", - "spamd_address": "amavis_is_local=no", - "spamd_port": "amavis_is_local=no", + + "sa_is_local": "manual_learning=yes", + "default_user": "manual_learning=yes", + "spamd_address": "sa_is_local=no", + "spamd_port": "sa_is_local=no", + "user_manual_learning": "manual_learning=yes" } diff --git a/modoboa/extensions/amavis/general_callbacks.py b/modoboa/extensions/amavis/general_callbacks.py index 1ef740e64..3223ca551 100644 --- a/modoboa/extensions/amavis/general_callbacks.py +++ b/modoboa/extensions/amavis/general_callbacks.py @@ -8,6 +8,8 @@ from modoboa.extensions.amavis.lib import ( create_user_and_policy, update_user_and_policy, delete_user_and_policy, create_user_and_use_policy, delete_user ) +from .lib import manual_learning_enabled +from .models import Users @events.observe("UserMenuDisplay") @@ -54,18 +56,53 @@ def on_domain_alias_deleted(domainaliases): delete_user("@{0}".format(domainalias.name)) -@events.observe("MailboxCreated") -def on_mailbox_created(user, mailbox): - """Create amavis records for the new mailbox.""" - create_user_and_policy(mailbox.full_address) +@events.observe("MailboxModified") +def on_mailbox_modified(mailbox): + """Update amavis records if address has changed.""" + if parameters.get_admin("MANUAL_LEARNING") == "no" or \ + mailbox.full_address == mailbox.old_full_address: + return + user = Users.objects.select_related.get(email=mailbox.old_full_address) + full_address = mailbox.full_address + user.email = full_address + user.policy.policy_name = full_address[:32] + user.policy.sa_username = full_address + user.policy.save() + user.save() @events.observe("MailboxDeleted") def on_mailbox_deleted(mailbox): - """Delete amavis records related to this mailbox.""" + """Clean amavis database when a mailbox is removed.""" + if parameters.get_admin("MANUAL_LEARNING") == "no": + return delete_user_and_policy(mailbox.full_address) +@events.observe("MailboxAliasCreated") +def on_mailboxalias_created(user, alias): + """Create amavis record for the new alias. + + FIXME: how to deal with distibution lists ? + """ + if not manual_learning_enabled(user) or alias.type != "alias": + return + policy = Policy.objects.get( + policy_name=alias.mboxes.all()[0].full_address + ) + email = alias.full_address + Users.objects.create(email=email, policy=policy, fullname=email) + + +@events.observe("MailboxAliasDeleted") +def on_mailboxalias_deleted(alias): + """Clean amavis database when an alias is removed.""" + if parameters.get_admin("MANUAL_LEARNING") == "no": + return + if Users.objects.exists(email=alias.fullname): + Users.objects.delete(email=alias.fullname) + + @events.observe("GetStaticContent") def extra_static_content(caller, user): if user.group == "SimpleUsers": diff --git a/modoboa/extensions/amavis/lib.py b/modoboa/extensions/amavis/lib.py index 118687f4f..163682de3 100644 --- a/modoboa/extensions/amavis/lib.py +++ b/modoboa/extensions/amavis/lib.py @@ -88,20 +88,20 @@ class SpamassassinClient(object): """A stupid spamassassin client.""" - def __init__(self, username=None): + def __init__(self, user): """Constructor.""" - self._amavis_is_local = parameters.get_admin("AMAVIS_IS_LOCAL") - if username is None: - self._username = parameters.get_admin("DEFAULT_USER") + self._sa_is_local = parameters.get_admin("SA_IS_LOCAL") + if user.group == "SimpleUsers": + user_manual_learning = parameters.get_admin("USER_MANUAL_LEARNING") + if user_manual_learning == "yes": + self._username = user.email else: - self._username = username + self._username = parameters.get_admin("DEFAULT_USER") + setup_manual_learning(user) self.error = None - if self._amavis_is_local == "yes": + if self._sa_is_local == "yes": self._learn_cmd = "sa-learn --{0} --no-sync -u {1}" self._learn_cmd_kwargs = {} - # self._learn_cmd_kwargs = { - # "sudo_user": parameters.get_admin("AMAVIS_SYSTEM_USER") - # } self._expected_exit_codes = [0] else: self._learn_cmd = "spamc -d {0} -p {1}".format( @@ -131,7 +131,7 @@ class SpamassassinClient(object): def done(self): """Call this method at the end of the processing.""" - if self._amavis_is_local: + if self._sa_is_local: exec_cmd("sa-learn -u {0} --sync".format(self._username), **self._learn_cmd_kwargs) @@ -177,11 +177,13 @@ def create_user_and_policy(name, priority=7): an identifier. :param str name: name + :return: the new ``Policy`` object """ policy = Policy.objects.create(policy_name=name[:32]) Users.objects.create( email=name, fullname=name, priority=priority, policy=policy ) + return policy def create_user_and_use_policy(name, policy_name, priority=7): @@ -234,3 +236,31 @@ def delete_user(name): Users.objects.get(email=name).delete() except Users.DoesNotExist: pass + + +def manual_learning_enabled(user): + """Check if manual learning is enabled or not. + + Also check for :kw:`user` if necessary. + + :return: True if learning is enabled, False otherwise. + """ + manual_learning = parameters.get_admin("MANUAL_LEARNING") == "yes" + if manual_learning and user.group == "SimpleUsers": + manual_learning = parameters.get_admin("USER_MANUAL_LEARNING") == "yes" + return manual_learning + + +def setup_manual_learning(user): + """Setup manual learning if necessary. + + :return: True if learning has been setup, False otherwise + """ + result = False + if user.group == "SimpleUsers": + if not Policy.objects.exists(email=user.email): + policy = create_user_and_policy(user.email) + for alias in user.mailbox_set.all()[0].alias_addresses: + create_user_and_use_policy(alias.full_address, policy) + result = True + return result diff --git a/modoboa/extensions/amavis/templates/amavis/main_action_bar.html b/modoboa/extensions/amavis/templates/amavis/main_action_bar.html index 3b3684f03..36b607005 100644 --- a/modoboa/extensions/amavis/templates/amavis/main_action_bar.html +++ b/modoboa/extensions/amavis/templates/amavis/main_action_bar.html @@ -23,7 +23,8 @@ {% trans "Delete" %} -
+ {% if manual_learning %} +
@@ -32,6 +33,7 @@
  • {% trans "Mark as non-spam" %}
    • + {% endif %} {% include "common/email_searchbar.html" with extraopts=extraopts %}
    diff --git a/modoboa/extensions/amavis/templatetags/amavis_tags.py b/modoboa/extensions/amavis/templatetags/amavis_tags.py index 187cbe997..7f644ebed 100644 --- a/modoboa/extensions/amavis/templatetags/amavis_tags.py +++ b/modoboa/extensions/amavis/templatetags/amavis_tags.py @@ -1,13 +1,20 @@ +""" +Amavis frontend template tags. +""" + from django import template from django.template.loader import render_to_string from django.utils.translation import ugettext as _ from django.core.urlresolvers import reverse +from ..lib import manual_learning_enabled + register = template.Library() @register.simple_tag -def viewm_menu(mail_id, rcpt): +def viewm_menu(user, mail_id, rcpt): + """Menu displayed within the viewmail action.""" entries = [ {"name": "release", "img": "icon-white icon-ok", @@ -21,20 +28,6 @@ def viewm_menu(mail_id, rcpt): "url": reverse('modoboa.extensions.amavis.views.delete', args=[mail_id]) + ("?rcpt=%s" % rcpt if rcpt else ""), "label": _("Delete")}, - {"name": "process", - "img": "icon-cog", - "menu": [ - {"name": "mark-as-spam", - "label": _("Mark as spam"), - "url": reverse( - "modoboa.extensions.amavis.views.mark_as_spam", args=[mail_id] - ) + ("?rcpt=%s" % rcpt if rcpt else "")}, - {"name": "mark-as-ham", - "label": _("Mark as non-spam"), - "url": reverse( - "modoboa.extensions.amavis.views.mark_as_ham", args=[mail_id] - ) + ("?rcpt=%s" % rcpt if rcpt else "")} - ]}, {"name": "headers", "url": reverse( 'modoboa.extensions.amavis.views.viewheaders', args=[mail_id] @@ -42,6 +35,24 @@ def viewm_menu(mail_id, rcpt): "label": _("View full headers")}, ] + if manual_learning_enabled(user): + entries.insert(1, { + "name": "process", + "img": "icon-cog", + "menu": [ + {"name": "mark-as-spam", + "label": _("Mark as spam"), + "url": reverse( + "modoboa.extensions.amavis.views.mark_as_spam", args=[mail_id] + ) + ("?rcpt=%s" % rcpt if rcpt else "")}, + {"name": "mark-as-ham", + "label": _("Mark as non-spam"), + "url": reverse( + "modoboa.extensions.amavis.views.mark_as_ham", args=[mail_id] + ) + ("?rcpt=%s" % rcpt if rcpt else "")} + ] + }) + menu = render_to_string('common/buttons_list.html', {"entries": entries, "extraclasses": "pull-left"}) @@ -80,13 +91,14 @@ def viewm_menu_simple(user, mail_id, rcpt, secret_id=""): @register.simple_tag -def quar_menu(): +def quar_menu(user): """Render the quarantine listing menu. :rtype: str :return: resulting HTML """ extraopts = [{"name": "to", "label": _("To")}] - return render_to_string('amavis/main_action_bar.html', { - 'extraopts': extraopts + return render_to_string("amavis/main_action_bar.html", { + "extraopts": extraopts, + "manual_learning": manual_learning_enabled(user) }) diff --git a/modoboa/extensions/amavis/views.py b/modoboa/extensions/amavis/views.py index 5cd084222..9fbdfff37 100644 --- a/modoboa/extensions/amavis/views.py +++ b/modoboa/extensions/amavis/views.py @@ -19,7 +19,7 @@ from modoboa.extensions.amavis.templatetags.amavis_tags import ( ) from .lib import ( selfservice, AMrelease, QuarantineNavigationParameters, - SpamassassinClient + SpamassassinClient, manual_learning_enabled ) from .models import Msgrcpt from .sql_connector import get_connector @@ -57,7 +57,7 @@ def _listing(request): "ok", listing=content, paginbar=pagination_bar(page), page=page.number ) if request.session.get('location', 'listing') != 'listing': - ctx['menu'] = quar_menu() + ctx['menu'] = quar_menu(request.user) request.session['location'] = 'listing' return render_to_json_response(ctx) @@ -115,7 +115,7 @@ def viewmail(request, mail_id): content = Template(""" """).render(Context({"url": reverse(getmailcontent, args=[mail_id])})) - menu = viewm_menu(mail_id, rcpt) + menu = viewm_menu(request.user, mail_id, rcpt) ctx = getctx("ok", menu=menu, listing=content) request.session['location'] = 'viewmail' return render_to_json_response(ctx) @@ -263,15 +263,11 @@ def mark_messages(request, selection, mtype): :param str selection: message unique identifier :param str mtype: type of marking (spam or ham) """ + if not manual_learning_enabled(request.user): + return render_to_json_response({"status": "ok"}) selection = check_mail_id(request, selection) connector = get_connector() - per_user_bayesian_filter = parameters.get_admin("PER_USER_BAYESIAN_FILTER") - if per_user_bayesian_filter == "yes" \ - and request.user.group == "SimpleUsers": - username = request.user.email - else: - username = None - saclient = SpamassassinClient(username) + saclient = SpamassassinClient(request.user) for item in selection: rcpt, mail_id = item.split() content = "".join(