From 0dd55069bcb13c6cd6529b200f77c626bc1b753c Mon Sep 17 00:00:00 2001
From: Antoine Nguyen <tonio@ngyn.org>
Date: Thu, 7 Sep 2017 18:18:43 +0200
Subject: [PATCH] Added random password generator. (#1225)

* Added random password generator.

see #1219

* Fixed unit test.

see #1219

* Fixed typo.

see #1219

* Increased coverage.

see #1219
---
 modoboa/admin/forms/account.py                | 62 ++++++++++++-------
 modoboa/admin/forms/domain.py                 | 26 ++++++--
 modoboa/admin/lib.py                          | 12 ++++
 modoboa/admin/serializers.py                  | 34 ++++++----
 modoboa/admin/static/admin/js/admin.js        | 19 ++++++
 .../templates/admin/account_general_form.html |  1 +
 .../templates/admin/domain_options_form.html  |  1 +
 modoboa/admin/tests/test_account.py           | 51 +++++++++++++++
 modoboa/admin/tests/test_api.py               | 14 +++++
 modoboa/admin/tests/test_domain.py            | 12 ++++
 modoboa/core/app_settings.py                  |  8 +++
 modoboa/core/tests/test_views.py              |  1 +
 12 files changed, 200 insertions(+), 41 deletions(-)

diff --git a/modoboa/admin/forms/account.py b/modoboa/admin/forms/account.py
index 50b2dda21..c50d1c83c 100644
--- a/modoboa/admin/forms/account.py
+++ b/modoboa/admin/forms/account.py
@@ -25,6 +25,7 @@ from modoboa.lib.validators import validate_utf8_email
 from modoboa.lib.web_utils import render_to_json_response
 from modoboa.parameters import tools as param_tools
 
+from .. import lib
 from .. import models
 from .. import signals
 
@@ -45,8 +46,17 @@ class AccountFormGeneral(forms.ModelForm):
         choices=[("", ugettext_lazy("Choose"))],
         help_text=ugettext_lazy("What level of permission this user will have")
     )
+    random_password = forms.BooleanField(
+        label=ugettext_lazy("Random password"),
+        help_text=ugettext_lazy(
+            "Generate a random password. If you're updating this account and "
+            "check this box, a new password will be generated."
+        ),
+        required=False
+    )
     password1 = forms.CharField(
-        label=ugettext_lazy("Password"), widget=forms.widgets.PasswordInput
+        label=ugettext_lazy("Password"), widget=forms.widgets.PasswordInput,
+        required=False
     )
 
     password2 = forms.CharField(
@@ -54,7 +64,8 @@ class AccountFormGeneral(forms.ModelForm):
         widget=forms.widgets.PasswordInput,
         help_text=ugettext_lazy(
             "Enter the same password as above, for verification."
-        )
+        ),
+        required=False
     )
 
     class Meta:
@@ -71,8 +82,9 @@ class AccountFormGeneral(forms.ModelForm):
         super(AccountFormGeneral, self).__init__(*args, **kwargs)
         self.fields = OrderedDict(
             (key, self.fields[key]) for key in
-            ["role", "username", "first_name", "last_name", "password1",
-             "password2", "master_user", "is_active"]
+            ["role", "username", "first_name", "last_name",
+             "random_password", "password1", "password2",
+             "master_user", "is_active"]
         )
         self.user = user
         condition = (
@@ -103,21 +115,15 @@ class AccountFormGeneral(forms.ModelForm):
         )
         if domain_disabled:
             self.fields["is_active"].widget.attrs["disabled"] = "disabled"
-        if args:
-            empty_password = (
-                args[0].get("password1", "") == "" and
-                args[0].get("password2", "") == "")
-            if empty_password:
-                self.fields["password1"].required = False
-                self.fields["password2"].required = False
-            if domain_disabled:
-                del self.fields["is_active"]
+        if args and domain_disabled:
+            del self.fields["is_active"]
         self.fields["role"].initial = self.instance.role
         condition = (
             not self.instance.is_local and
             param_tools.get_global_parameter(
                 "ldap_auth_method", app="core") == "directbind")
         if condition:
+            del self.fields["random_password"]
             del self.fields["password1"]
             del self.fields["password2"]
 
@@ -151,16 +157,6 @@ class AccountFormGeneral(forms.ModelForm):
         validate_utf8_email(uname)
         return uname
 
-    def clean_password2(self):
-        password1 = self.cleaned_data.get("password1", "")
-        password2 = self.cleaned_data["password2"]
-        if password1 != password2:
-            raise forms.ValidationError(
-                _("The two password fields didn't match."))
-        if password2 != "":
-            password_validation.validate_password(password2, self.instance)
-        return password2
-
     def clean(self):
         """Check master user mode."""
         super(AccountFormGeneral, self).clean()
@@ -175,6 +171,26 @@ class AccountFormGeneral(forms.ModelForm):
                 "master_user",
                 _("Only super administrators are allowed for this mode")
             )
+        random_password = self.cleaned_data.get("random_password")
+        if random_password:
+            self.cleaned_data["password2"] = lib.make_password()
+        elif "random_password" in self.fields and not random_password:
+            password1 = self.cleaned_data.get("password1", "")
+            password2 = self.cleaned_data.get("password2", "")
+            empty_password = password1 == "" and password2 == ""
+            if not self.instance.pk or not empty_password:
+                if not password1:
+                    self.add_error("password1", _("This field is required."))
+                if not password2:
+                    self.add_error("password2", _("This field is required."))
+                if not self.errors:
+                    if password1 != password2:
+                        self.add_error(
+                            "password2",
+                            _("The two password fields didn't match."))
+                    else:
+                        password_validation.validate_password(
+                            password2, self.instance)
         return self.cleaned_data
 
     def save(self, commit=True):
diff --git a/modoboa/admin/forms/domain.py b/modoboa/admin/forms/domain.py
index 3789e25fc..c93b7439a 100644
--- a/modoboa/admin/forms/domain.py
+++ b/modoboa/admin/forms/domain.py
@@ -20,8 +20,9 @@ from modoboa.lib.form_utils import (
 from modoboa.lib.web_utils import render_to_json_response
 from modoboa.parameters import tools as param_tools
 
+from .. import lib
 from .. import signals
-from ..lib import check_if_domain_exists, domain_has_authorized_mx
+
 from ..models import (
     Domain, DomainAlias, Mailbox, Alias
 )
@@ -85,7 +86,7 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm):
     def clean_name(self):
         """Check unicity and more."""
         name = self.cleaned_data["name"]
-        label = check_if_domain_exists(
+        label = lib.check_if_domain_exists(
             name, [(DomainAlias, _('domain alias'))])
         if label is not None:
             raise forms.ValidationError(
@@ -95,7 +96,7 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm):
             param_tools.get_global_parameter("domains_must_have_authorized_mx")
         )
         if domains_must_have_authorized_mx and not self.user.is_superuser:
-            if not domain_has_authorized_mx(name):
+            if not lib.domain_has_authorized_mx(name):
                 raise forms.ValidationError(
                     _("No authorized MX record found for this domain"))
 
@@ -145,7 +146,7 @@ class DomainFormGeneral(forms.ModelForm, DynamicForm):
                 )
                 del cleaned_data[k]
                 continue
-            label = check_if_domain_exists(
+            label = lib.check_if_domain_exists(
                 cleaned_data[k], [(Domain, _("domain"))])
             if label is not None:
                 self.add_error(
@@ -212,6 +213,15 @@ class DomainFormOptions(forms.Form):
         required=False
     )
 
+    random_password = YesNoField(
+        label=ugettext_lazy("Random password"),
+        initial=False,
+        help_text=ugettext_lazy(
+            "Generate a random password for the administrator."
+        ),
+        required=False
+    )
+
     with_mailbox = YesNoField(
         label=ugettext_lazy("With a mailbox"),
         initial=True,
@@ -274,8 +284,12 @@ class DomainFormOptions(forms.Form):
         core_signals.can_create_object.send(
             self.__class__, context=user, klass=Mailbox)
         da = User(username=username, email=username, is_active=True)
-        da.set_password(
-            param_tools.get_global_parameter("default_password", app="core"))
+        if self.cleaned_data["random_password"]:
+            password = lib.make_password()
+        else:
+            password = param_tools.get_global_parameter(
+                "default_password", app="core")
+        da.set_password(password)
         da.save()
         da.role = "DomainAdmins"
         da.post_create(user)
diff --git a/modoboa/admin/lib.py b/modoboa/admin/lib.py
index c5f0780a5..3955ec25b 100644
--- a/modoboa/admin/lib.py
+++ b/modoboa/admin/lib.py
@@ -7,7 +7,9 @@ from __future__ import unicode_literals
 from functools import wraps
 import ipaddress
 from itertools import chain
+import random
 import socket
+import string
 
 import dns.resolver
 
@@ -206,3 +208,13 @@ def domain_has_authorized_mx(name):
             if mx_ip_addr in subnet:
                 return True
     return False
+
+
+def make_password():
+    """Create a random password."""
+    length = int(
+        param_tools.get_global_parameter("random_password_length", app="core")
+    )
+    return "".join(
+        random.SystemRandom().choice(
+            string.ascii_letters + string.digits) for _ in range(length))
diff --git a/modoboa/admin/serializers.py b/modoboa/admin/serializers.py
index 1199f04f3..e73892155 100644
--- a/modoboa/admin/serializers.py
+++ b/modoboa/admin/serializers.py
@@ -189,11 +189,13 @@ class AccountPasswordSerializer(serializers.ModelSerializer):
 class WritableAccountSerializer(AccountSerializer):
     """Serializer to create account."""
 
+    random_password = serializers.BooleanField(default=False)
     role = serializers.ChoiceField(choices=core_constants.ROLES)
+    password = serializers.CharField(required=False)
 
     class Meta(AccountSerializer.Meta):
         fields = AccountSerializer.Meta.fields + (
-            "password", )
+            "password", "random_password")
 
     def __init__(self, *args, **kwargs):
         """Adapt fields to current user."""
@@ -206,16 +208,6 @@ class WritableAccountSerializer(AccountSerializer):
             choices=permissions.get_account_roles(user))
         self.fields["domains"] = serializers.ListField(
             child=serializers.CharField(), allow_empty=False, required=False)
-        if request.method == "PUT":
-            self.fields["password"].required = False
-
-    def validate_password(self, value):
-        """Check password constraints."""
-        try:
-            password_validation.validate_password(value, self.instance)
-        except ValidationError as exc:
-            raise serializers.ValidationError(exc.messages[0])
-        return value
 
     def validate(self, data):
         """Check constraints."""
@@ -235,6 +227,19 @@ class WritableAccountSerializer(AccountSerializer):
                 raise serializers.ValidationError({
                     "username": _("Must be equal to mailbox full_address")
                 })
+        if not data["random_password"]:
+            password = data.get("password")
+            if password:
+                try:
+                    password_validation.validate_password(
+                        data["password"], self.instance)
+                except ValidationError as exc:
+                    raise serializers.ValidationError({
+                        "password": exc.messages[0]})
+            elif not self.instance:
+                raise serializers.ValidationError({
+                    "password": _("This field is required.")
+                })
         domain_names = data.get("domains")
         if not domain_names:
             return data
@@ -296,8 +301,13 @@ class WritableAccountSerializer(AccountSerializer):
         mailbox_data = validated_data.pop("mailbox", None)
         role = validated_data.pop("role")
         domains = validated_data.pop("domains", [])
+        random_password = validated_data.pop("random_password", False)
         user = core_models.User(**validated_data)
-        user.set_password(validated_data["password"])
+        if random_password:
+            password = lib.make_password()
+        else:
+            password = validated_data.pop("password")
+        user.set_password(password)
         user.save(creator=creator)
         if mailbox_data:
             self._create_mailbox(creator, user, mailbox_data)
diff --git a/modoboa/admin/static/admin/js/admin.js b/modoboa/admin/static/admin/js/admin.js
index 7df3c579e..5c33d4f1c 100644
--- a/modoboa/admin/static/admin/js/admin.js
+++ b/modoboa/admin/static/admin/js/admin.js
@@ -468,6 +468,21 @@ Identities.prototype = {
         }
     },
 
+    /**
+     * Hide or show password inputs depending on value.
+     *
+     * @param {bool} value - visibility flag
+     */
+    togglePasswordInputs: function(value) {
+        if (value) {
+            $('#id_password1').parents(".form-group").show();
+            $('#id_password2').parents(".form-group").show();
+        } else {
+            $('#id_password1').parents(".form-group").hide();
+            $('#id_password2').parents(".form-group").hide();
+        }
+    },
+
     generalform_init: function(notrigger) {
         $("#id_role").change($.proxy(function(e) {
             var $this = $(e.target);
@@ -479,6 +494,10 @@ Identities.prototype = {
                 this.normal_mode(value);
             }
         }, this));
+        $('#id_random_password').click($.proxy(function (e) {
+            var $this = $(e.target);
+            this.togglePasswordInputs(!$this.prop('checked'));
+        }, this));
         this.toggle_master_user($("#id_role").val());
         if (notrigger !== undefined && notrigger) {
             return;
diff --git a/modoboa/admin/templates/admin/account_general_form.html b/modoboa/admin/templates/admin/account_general_form.html
index d71149c46..0ea29ae20 100644
--- a/modoboa/admin/templates/admin/account_general_form.html
+++ b/modoboa/admin/templates/admin/account_general_form.html
@@ -4,6 +4,7 @@
 {% render_field form.username %}
 {% render_field form.first_name %}
 {% render_field form.last_name %}
+{% render_field form.random_password %}
 {% render_field form.password1 %}
 {% render_field form.password2 %}
 {% if form.master_user %}{% render_field form.master_user %}{% endif %}
diff --git a/modoboa/admin/templates/admin/domain_options_form.html b/modoboa/admin/templates/admin/domain_options_form.html
index aba4e73e6..b45d04a81 100644
--- a/modoboa/admin/templates/admin/domain_options_form.html
+++ b/modoboa/admin/templates/admin/domain_options_form.html
@@ -3,6 +3,7 @@
 {% if form.fields %}
   {% render_field form.create_dom_admin %}
   {% render_field form.dom_admin_username appended_text="@" %}
+  {% render_field form.random_password %}
   {% render_field form.with_mailbox %}
   {% render_field form.create_aliases %}
 {% else %}
diff --git a/modoboa/admin/tests/test_account.py b/modoboa/admin/tests/test_account.py
index 016c740b2..2a6444767 100644
--- a/modoboa/admin/tests/test_account.py
+++ b/modoboa/admin/tests/test_account.py
@@ -107,6 +107,57 @@ class AccountTestCase(ModoTestCase):
                 alias__internal=True).exists()
         )
 
+    def test_password_constraints(self):
+        """Check password constraints."""
+        values = {
+            "username": "tester@test.com",
+            "first_name": "Tester", "last_name": "Toto",
+            "password1": "", "password2": "",
+            "role": "SimpleUsers",
+            "quota_act": True, "is_active": True, "email": "tester@test.com",
+            "stepid": "step2"
+        }
+        resp = self.ajax_post(reverse("admin:account_add"), values, 400)
+        self.assertEqual(
+            resp["form_errors"]["password1"][0],
+            "This field is required.")
+        values["password1"] = "Toto1234"
+        values["password2"] = "Toto12345"
+        resp = self.ajax_post(reverse("admin:account_add"), values, 400)
+        self.assertEqual(
+            resp["form_errors"]["password2"][0],
+            "The two password fields didn't match.")
+
+    def test_random_password(self):
+        """Try to create an account with a random password."""
+        values = {
+            "username": "tester@test.com",
+            "first_name": "Tester", "last_name": "Toto",
+            "random_password": True, "role": "SimpleUsers",
+            "quota_act": True, "is_active": True, "email": "tester@test.com",
+            "stepid": "step2"
+        }
+        self.ajax_post(reverse("admin:account_add"), values)
+
+        account = User.objects.get(username=values["username"])
+        password = account.password
+        values["language"] = "en"
+        # Since 'random_password' is still True, a new password should
+        # be generated
+        self.ajax_post(
+            reverse("admin:account_change", args=[account.pk]), values
+        )
+        account.refresh_from_db()
+        self.assertNotEqual(password, account.password)
+        password = account.password
+
+        values["random_password"] = False
+        self.ajax_post(
+            reverse("admin:account_change", args=[account.pk]), values
+        )
+        account.refresh_from_db()
+        self.assertEqual(password, account.password)
+
     def test_delete_default_superadmin(self):
         """Delete default superadmin."""
         sadmin2 = core_factories.UserFactory(
diff --git a/modoboa/admin/tests/test_api.py b/modoboa/admin/tests/test_api.py
index e3292d8bb..3e897ce18 100644
--- a/modoboa/admin/tests/test_api.py
+++ b/modoboa/admin/tests/test_api.py
@@ -285,7 +285,21 @@ class AccountAPITestCase(ModoAPITestCase):
         data = copy.deepcopy(self.ACCOUNT_DATA)
         data["username"] = "fromapi_ééé@test.com"
         data["mailbox"]["full_address"] = data["username"]
+
+        del data["password"]
+        response = self.client.post(url, data, format="json")
+        self.assertEqual(response.status_code, 400)
+
+        data["password"] = "Toto1234"
+        response = self.client.post(url, data, format="json")
+        self.assertEqual(response.status_code, 201)
+
+    def test_create_account_with_random_password(self):
+        """Try to create a new account with random password."""
         url = reverse("api:account-list")
+        data = dict(self.ACCOUNT_DATA)
+        data["random_password"] = True
+        del data["password"]
         response = self.client.post(url, data, format="json")
         self.assertEqual(response.status_code, 201)
 
diff --git a/modoboa/admin/tests/test_domain.py b/modoboa/admin/tests/test_domain.py
index 257e8eb2a..19e7893d0 100644
--- a/modoboa/admin/tests/test_domain.py
+++ b/modoboa/admin/tests/test_domain.py
@@ -87,6 +87,18 @@ class DomainTestCase(ModoTestCase):
             Alias.objects.filter(
                 address="postmaster@pouet2.com", internal=False).exists())
 
+    def test_create_with_template_and_random_password(self):
+        """Test domain creation with administrator and random password."""
+        values = {
+            "name": "pouet.com", "quota": 1000, "default_mailbox_quota": 100,
+            "create_dom_admin": True, "dom_admin_username": "toto",
+            "with_mailbox": True, "create_aliases": True, "type": "domain",
+            "stepid": "step3", "random_password": True
+        }
+        self.ajax_post(reverse("admin:domain_add"), values)
+        self.assertFalse(
+            self.client.login(username="admin@pouet.com", password="password"))
+
     def test_create_with_template_and_no_mailbox(self):
         """Test domain creation with administrator but no mailbox."""
         values = {
diff --git a/modoboa/core/app_settings.py b/modoboa/core/app_settings.py
index e407ed4a6..398874d3a 100644
--- a/modoboa/core/app_settings.py
+++ b/modoboa/core/app_settings.py
@@ -86,6 +86,14 @@ class GeneralParametersForm(param_forms.AdminParametersForm):
             "Default password for automatically created accounts.")
     )
 
+    random_password_length = forms.IntegerField(
+        label=ugettext_lazy("Random password length"),
+        min_value=8,
+        initial=8,
+        help_text=ugettext_lazy(
+            "Length of randomly generated passwords.")
+    )
+
     # LDAP specific settings
     ldap_sep = SeparatorField(label=ugettext_lazy("LDAP settings"))
 
diff --git a/modoboa/core/tests/test_views.py b/modoboa/core/tests/test_views.py
index 87e331bc4..9d0410b4d 100644
--- a/modoboa/core/tests/test_views.py
+++ b/modoboa/core/tests/test_views.py
@@ -158,6 +158,7 @@ class SettingsTestCase(ModoTestCase):
         "core-hide_features_widget": "False",
         "core-inactive_account_threshold": "30",
         "admin-domains_must_have_authorized_mx": False,
+        "core-random_password_length": "8"
     }
 
     def test_get_settings(self):