implement password policy checking on signup and password change closes #8

passbook/core/exceptions.py

@@ -0,0 +1,10 @@
+"""passbook core exceptions"""
+
+class PasswordPolicyInvalid(Exception):
+    """Exception raised when a Password Policy fails"""
+
+    messages = []
+
+    def __init__(self, *messages):
+        super().__init__()
+        self.messages = messages

passbook/core/models.py

@@ -4,6 +4,7 @@ from datetime import timedelta
 from logging import getLogger
 from random import SystemRandom
 from time import sleep
+from typing import Tuple, Union
 from uuid import uuid4
 
 from django.contrib.auth.models import AbstractUser
@@ -49,7 +50,8 @@ class User(AbstractUser):
     password_change_date = models.DateTimeField(auto_now_add=True)
 
     def set_password(self, password):
-        password_changed.send(sender=self, user=self, password=password)
+        if self.pk:
+            password_changed.send(sender=self, user=self, password=password)
         self.password_change_date = now()
         return super().set_password(password)
 
@@ -69,8 +71,9 @@ class PolicyModel(UUIDModel, CreatedUpdatedModel):
 
     policies = models.ManyToManyField('Policy', blank=True)
 
-    def passes(self, user: User) -> bool:
+    def passes(self, user: User) -> Union[bool, Tuple[bool, str]]:
-        """Return true if user passes, otherwise False or raise Exception"""
+        """Return False, str if a user fails where str is a
+        reasons shown to the user. Return True if user succeeds."""
         for policy in self.policies.all():
             if not policy.passes(user):
                 return False
@@ -222,7 +225,7 @@ class Policy(UUIDModel, CreatedUpdatedModel):
             return self.name
         return "%s action %s" % (self.name, self.action)
 
-    def passes(self, user: User) -> bool:
+    def passes(self, user: User) -> Union[bool, Tuple[bool, str]]:
         """Check if user instance passes this policy"""
         raise NotImplementedError()
 
@@ -267,7 +270,7 @@ class FieldMatcherPolicy(Policy):
             description = "%s: %s" % (self.name, description)
         return description
 
-    def passes(self, user: User) -> bool:
+    def passes(self, user: User) -> Union[bool, Tuple[bool, str]]:
         """Check if user instance passes this role"""
         if not hasattr(user, self.user_field):
             raise ValueError("Field does not exist")
@@ -302,10 +305,11 @@ class PasswordPolicy(Policy):
     amount_symbols = models.IntegerField(default=0)
     length_min = models.IntegerField(default=0)
     symbol_charset = models.TextField(default=r"!\"#$%&'()*+,-./:;<=>?@[\]^_`{|}~ ")
+    error_message = models.TextField()
 
     form = 'passbook.core.forms.policies.PasswordPolicyForm'
 
-    def passes(self, user: User) -> bool:
+    def passes(self, user: User) -> Union[bool, Tuple[bool, str]]:
         # Only check if password is being set
         if not hasattr(user, '__password__'):
             return True
@@ -320,6 +324,8 @@ class PasswordPolicy(Policy):
             filter_regex += r'[%s]{%d,}' % (self.symbol_charset, self.amount_symbols)
         result = bool(re.compile(filter_regex).match(password))
         LOGGER.debug("User got %r", result)
+        if not result:
+            return result, self.error_message
         return result
 
     class Meta:
@@ -378,7 +384,7 @@ class DebugPolicy(Policy):
         wait = SystemRandom().randrange(self.wait_min, self.wait_max)
         LOGGER.debug("Policy '%s' waiting for %ds", self.name, wait)
         sleep(wait)
-        return self.result
+        return self.result, 'Debugging'
 
     class Meta:
 

passbook/core/policies.py

@@ -42,7 +42,11 @@ class PolicyEngine:
     @property
     def result(self):
         """Get policy-checking result"""
+        messages = []
         for policy_result in self._group.get():
+            if isinstance(policy_result, (tuple, list)):
+                policy_result, policy_message = policy_result
+                messages.append(policy_message)
             if policy_result is False:
-                return False
+                return False, messages
-        return True
+        return True, messages

passbook/core/signals.py

@@ -1,12 +1,27 @@
 """passbook core signals"""
 
 from django.core.signals import Signal
+from django.dispatch import receiver
 
-# from django.db.models.signals import post_save, pre_delete
+from passbook.core.exceptions import PasswordPolicyInvalid
-# from django.dispatch import receiver
-# from passbook.core.models import Invitation, User
 
 user_signed_up = Signal(providing_args=['request', 'user'])
 invitation_created = Signal(providing_args=['request', 'invitation'])
 invitation_used = Signal(providing_args=['request', 'invitation', 'user'])
 password_changed = Signal(providing_args=['user', 'password'])
+
+@receiver(password_changed)
+# pylint: disable=unused-argument
+def password_policy_checker(sender, password, **kwargs):
+    """Run password through all password policies which are applied to the user"""
+    from passbook.core.models import PasswordFactor
+    from passbook.core.policies import PolicyEngine
+    setattr(sender, '__password__', password)
+    _all_factors = PasswordFactor.objects.filter(enabled=True).order_by('order')
+    for factor in _all_factors:
+        if factor.passes(sender):
+            policy_engine = PolicyEngine(factor.password_policies.all().select_subclasses())
+            policy_engine.for_user(sender)
+            passing, messages = policy_engine.result
+            if not passing:
+                raise PasswordPolicyInvalid(*messages)

passbook/core/templates/partials/form_login.html

@@ -3,45 +3,48 @@
 
 {% csrf_token %}
 {% for field in form %}
-<div class="form-group login-pf-settings">
+<div class="form-group login-pf-settings {% if field.errors %} has-error {% endif %}">
-  {% if field.field.widget|fieldtype == 'RadioSelect' %}
+    {% if field.field.widget|fieldtype == 'RadioSelect' %}
-    <label class="col-sm-2 control-label" {% if field.field.required %}class="required"{% endif %} for="{{ field.name }}-{{ forloop.counter0 }}">
+    <label class="col-sm-2 control-label" {% if field.field.required %}class="required" {% endif %}
-      {{ field.label }}
+        for="{{ field.name }}-{{ forloop.counter0 }}">
+        {{ field.label }}
     </label>
     {% for c in field %}
     <div class="radio col-sm-10">
-      <input type="radio" id="{{ field.name }}-{{ forloop.counter0 }}" name="{% if wizard %}{{ wizard.steps.current }}-{% endif %}{{ field.name }}" value="{{ c.data.value }}" {% if c.data.selected %} checked {% endif %}>
+        <input type="radio" id="{{ field.name }}-{{ forloop.counter0 }}"
-      <label class="col-sm-2 control-label" for="{{ field.name }}-{{ forloop.counter0 }}">{{ c.choice_label }}</label>
+            name="{% if wizard %}{{ wizard.steps.current }}-{% endif %}{{ field.name }}" value="{{ c.data.value }}"
+            {% if c.data.selected %} checked {% endif %}>
+        <label class="col-sm-2 control-label" for="{{ field.name }}-{{ forloop.counter0 }}">{{ c.choice_label }}</label>
     </div>
     {% endfor %}
-  {% elif field.field.widget|fieldtype == 'Select' %}
+    {% elif field.field.widget|fieldtype == 'Select' %}
-    <label class="col-sm-2 control-label" {% if field.field.required %}class="required"{% endif %} for="{{ field.name }}-{{ forloop.counter0 }}">
+    <label class="col-sm-2 control-label" {% if field.field.required %}class="required" {% endif %}
-      {{ field.label }}
+        for="{{ field.name }}-{{ forloop.counter0 }}">
+        {{ field.label }}
     </label>
     <div class="select col-sm-10">
-      {{ field }}
+        {{ field }}
     </div>
-  {% elif field.field.widget|fieldtype == 'CheckboxInput' %}
+    {% elif field.field.widget|fieldtype == 'CheckboxInput' %}
     <label class="checkbox-label">
-      {{ field }} {{ field.label }}
+        {{ field }} {{ field.label }}
     </label>
-  {% else %}
+    {% else %}
-  <label class="col-sm-2 sr-only" {% if field.field.required %}class="required"{% endif %} for="{{ field.name }}-{{ forloop.counter0 }}">
+    <label class="col-sm-2 sr-only" {% if field.field.required %}class="required" {% endif %}
-    {{ field.label }}
+        for="{{ field.name }}-{{ forloop.counter0 }}">
-  </label>
+        {{ field.label }}
-  {{ field|css_class:'form-control input-lg' }}
+    </label>
-  {% if field.help_text %}
+    {{ field|css_class:'form-control input-lg' }}
-  <span>
+    {% if field.help_text %}
-    {{ field.help_text }}
+    <span>
-  </span>
+        {{ field.help_text }}
-  {% endif %}
+    </span>
-  {% endif %}
+    {% endif %}
-  {% for error in field.errors %}
+    {% endif %}
-  <hr>
+    {% for error in field.errors %}
-  <div class="alert alert-danger alert-block">
+    <span class="help-block">
-    <span class="pficon pficon-error-circle-o"></span>
+        {{ error }}
-    <strong>{{ error }}</strong>
+    </span>
-  </div>
+    {% endfor %}
-  {% endfor %}
 </div>
 {% endfor %}

passbook/core/views/authentication.py

@@ -1,10 +1,11 @@
-"""Core views"""
+"""passbook core authentication views"""
 from logging import getLogger
 from typing import Dict
 
 from django.contrib import messages
 from django.contrib.auth import login, logout
 from django.contrib.auth.mixins import LoginRequiredMixin, UserPassesTestMixin
+from django.forms.utils import ErrorList
 from django.http import HttpRequest, HttpResponse
 from django.shortcuts import get_object_or_404, redirect, reverse
 from django.utils.translation import ugettext as _
@@ -12,6 +13,7 @@ from django.views import View
 from django.views.generic import FormView
 
 from passbook.core.auth.view import AuthenticationView
+from passbook.core.exceptions import PasswordPolicyInvalid
 from passbook.core.forms.authentication import LoginForm, SignUpForm
 from passbook.core.models import Invitation, Nonce, Source, User
 from passbook.core.signals import invitation_used, user_signed_up
@@ -141,7 +143,15 @@ class SignUpView(UserPassesTestMixin, FormView):
 
     def form_valid(self, form: SignUpForm) -> HttpResponse:
         """Create user"""
-        self._user = SignUpView.create_user(form.cleaned_data, self.request)
+        try:
+            self._user = SignUpView.create_user(form.cleaned_data, self.request)
+        except PasswordPolicyInvalid as exc:
+            # Manually inject error into form
+            # pylint: disable=protected-access
+            errors = form._errors.setdefault("password", ErrorList())
+            for error in exc.messages:
+                errors.append(error)
+            return self.form_invalid(form)
         needs_confirmation = True
         if self._invitation and not self._invitation.needs_confirmation:
             needs_confirmation = False
@@ -187,25 +197,30 @@ class SignUpView(UserPassesTestMixin, FormView):
             The user created
 
         Raises:
-            SignalException: if any signals raise an exception. This also deletes the created user.
+            PasswordPolicyInvalid: if any policy are not fulfilled.
+                                   This also deletes the created user.
         """
         # Create user
-        new_user = User.objects.create_user(
+        new_user = User.objects.create(
             username=data.get('username'),
             email=data.get('email'),
             first_name=data.get('first_name'),
             last_name=data.get('last_name'),
         )
         new_user.is_active = True
-        new_user.set_password(data.get('password'))
+        try:
-        new_user.save()
+            new_user.set_password(data.get('password'))
-        request.user = new_user
+            new_user.save()
-        # Send signal for other auth sources
+            request.user = new_user
-        user_signed_up.send(
+            # Send signal for other auth sources
-            sender=SignUpView,
+            user_signed_up.send(
-            user=new_user,
+                sender=SignUpView,
-            request=request)
+                user=new_user,
-        return new_user
+                request=request)
+            return new_user
+        except PasswordPolicyInvalid as exc:
+            new_user.delete()
+            raise exc
 
 
 class SignUpConfirmView(View):

passbook/core/views/user.py

@@ -1,10 +1,12 @@
 """passbook core user views"""
 from django.contrib import messages
 from django.contrib.auth import logout, update_session_auth_hash
+from django.forms.utils import ErrorList
 from django.shortcuts import redirect, reverse
 from django.utils.translation import gettext as _
 from django.views.generic import DeleteView, FormView, UpdateView
 
+from passbook.core.exceptions import PasswordPolicyInvalid
 from passbook.core.forms.users import PasswordChangeForm, UserDetailForm
 from passbook.lib.config import CONFIG
 
@@ -38,10 +40,20 @@ class UserChangePasswordView(FormView):
     template_name = 'login/form_with_user.html'
 
     def form_valid(self, form: PasswordChangeForm):
-        self.request.user.set_password(form.cleaned_data.get('password'))
+        try:
-        self.request.user.save()
+            self.request.user.set_password(form.cleaned_data.get('password'))
-        update_session_auth_hash(self.request, self.request.user)
+            self.request.user.save()
-        messages.success(self.request, _('Successfully changed password'))
+            update_session_auth_hash(self.request, self.request.user)
+            messages.success(self.request, _('Successfully changed password'))
+        except PasswordPolicyInvalid as exc:
+            # Manually inject error into form
+            # pylint: disable=protected-access
+            errors = form._errors.setdefault("password_repeat", ErrorList(''))
+            # pylint: disable=protected-access
+            errors = form._errors.setdefault("password", ErrorList())
+            for error in exc.messages:
+                errors.append(error)
+            return self.form_invalid(form)
         return redirect('passbook_core:overview')
 
     def get_context_data(self, **kwargs):

passbook/hibp_policy/models.py

@@ -1,6 +1,6 @@
 """passbook HIBP Models"""
-
 from hashlib import sha1
+from logging import getLogger
 
 from django.db import models
 from django.utils.translation import gettext as _
@@ -8,6 +8,7 @@ from requests import get
 
 from passbook.core.models import Policy, User
 
+LOGGER = getLogger(__name__)
 
 class HaveIBeenPwendPolicy(Policy):
     """Check if password is on HaveIBeenPwned's list by upload the first
@@ -33,8 +34,9 @@ class HaveIBeenPwendPolicy(Policy):
             full_hash, count = line.split(':')
             if pw_hash[5:] == full_hash.lower():
                 final_count = int(count)
+        LOGGER.debug("Got count %d for hash %s", final_count, pw_hash[:5])
         if final_count > self.allowed_count:
-            return False
+            return False, _("Password exists on %(count)d online lists." % {'count': final_count})
         return True
 
     class Meta: