Merge branch '1-suspicious-request' into 'master'

Resolve "Suspicious request detector (many invalid logins from one IP, many attempts on one username, etc)"

Closes #1

See merge request BeryJu.org/passbook!3

passbook/audit/models.py

@@ -1,5 +1,4 @@
 """passbook audit models"""
-from datetime import timedelta
 from logging import getLogger
 
 from django.conf import settings
@@ -7,11 +6,10 @@ from django.contrib.auth.models import AnonymousUser
 from django.contrib.postgres.fields import JSONField
 from django.core.exceptions import ValidationError
 from django.db import models
-from django.utils import timezone
 from django.utils.translation import gettext as _
 from ipware import get_client_ip
 
-from passbook.lib.models import CreatedUpdatedModel, UUIDModel
+from passbook.lib.models import UUIDModel
 
 LOGGER = getLogger(__name__)
 
@@ -75,43 +73,3 @@ class AuditEntry(UUIDModel):
 
         verbose_name = _('Audit Entry')
         verbose_name_plural = _('Audit Entries')
-
-
-class LoginAttempt(CreatedUpdatedModel):
-    """Track failed login-attempts"""
-
-    target_uid = models.CharField(max_length=254)
-    request_ip = models.GenericIPAddressField()
-    attempts = models.IntegerField(default=1)
-
-    @staticmethod
-    def attempt(target_uid, request):
-        """Helper function to create attempt or count up existing one"""
-        if not target_uid:
-            return
-        client_ip, _ = get_client_ip(request)
-        # Since we can only use 254 chars for target_uid, truncate target_uid.
-        target_uid = target_uid[:254]
-        time_threshold = timezone.now() - timedelta(minutes=10)
-        existing_attempts = LoginAttempt.objects.filter(
-            target_uid=target_uid,
-            request_ip=client_ip,
-            last_updated__gt=time_threshold).order_by('created')
-        if existing_attempts.exists():
-            attempt = existing_attempts.first()
-            attempt.attempts += 1
-            attempt.save()
-            LOGGER.debug("Increased attempts on %s", attempt)
-        else:
-            attempt = LoginAttempt.objects.create(
-                target_uid=target_uid,
-                request_ip=client_ip)
-            LOGGER.debug("Created new attempt %s", attempt)
-
-    def __str__(self):
-        return "LoginAttempt to %s from %s (x%d)" % (self.target_uid,
-                                                     self.request_ip, self.attempts)
-
-    class Meta:
-
-        unique_together = (('target_uid', 'request_ip', 'created'),)

passbook/audit/requirements.txt

@@ -1 +0,0 @@
-django-ipware

passbook/audit/signals.py

@@ -1,9 +1,8 @@
 """passbook audit signal listener"""
-from django.contrib.auth.signals import (user_logged_in, user_logged_out,
-                                         user_login_failed)
+from django.contrib.auth.signals import user_logged_in, user_logged_out
 from django.dispatch import receiver
 
-from passbook.audit.models import AuditEntry, LoginAttempt
+from passbook.audit.models import AuditEntry
 from passbook.core.signals import (invitation_created, invitation_used,
                                    user_signed_up)
 
@@ -34,8 +33,3 @@ def on_invitation_used(sender, request, invitation, **kwargs):
     """Log Invitation usage"""
     AuditEntry.create(AuditEntry.ACTION_INVITE_USED, request,
                       invitation_uuid=invitation.uuid.hex)
-
-@receiver(user_login_failed)
-def on_user_login_failed(sender, request, credentials, **kwargs):
-    """Log failed login attempt"""
-    LoginAttempt.attempt(target_uid=credentials.get('username'), request=request)

passbook/core/auth/view.py

@@ -65,7 +65,7 @@ class AuthenticationView(UserPassesTestMixin, View):
             self.pending_factors = []
             for factor in _all_factors:
                 policy_engine = PolicyEngine(factor.policies.all())
-                policy_engine.for_user(self.pending_user)
+                policy_engine.for_user(self.pending_user).with_request(request).build()
                 if policy_engine.result[0]:
                     self.pending_factors.append((factor.uuid.hex, factor.type))
         # Read and instantiate factor from session

passbook/core/models.py

@@ -153,7 +153,7 @@ class Application(PolicyModel):
     def user_is_authorized(self, user: User) -> bool:
         """Check if user is authorized to use this application"""
         from passbook.core.policies import PolicyEngine
-        return PolicyEngine(self.policies.all()).for_user(user).result
+        return PolicyEngine(self.policies.all()).for_user(user).build().result
 
     def get_provider(self):
         """Get casted provider instance"""

passbook/core/policies.py

@@ -2,6 +2,7 @@
 from logging import getLogger
 
 from celery import group
+from ipware import get_client_ip
 
 from passbook.core.celery import CELERY_APP
 from passbook.core.models import Policy, User
@@ -33,18 +34,36 @@ class PolicyEngine:
 
     policies = None
     _group = None
+    _request = None
+    _user = None
 
     def __init__(self, policies):
         self.policies = policies
+        self._request = None
+        self._user = None
 
     def for_user(self, user):
         """Check policies for user"""
+        self._user = user
+        return self
+
+    def with_request(self, request):
+        """Set request"""
+        self._request = request
+        return self
+
+    def build(self):
+        """Build task group"""
         signatures = []
         kwargs = {
-            '__password__': getattr(user, '__password__', None)
+            '__password__': getattr(self._user, '__password__', None),
         }
+        if self._request:
+            kwargs['remote_ip'], _ = get_client_ip(self._request)
+            if not kwargs['remote_ip']:
+                kwargs['remote_ip'] = '255.255.255.255'
         for policy in self.policies:
-            signatures.append(_policy_engine_task.s(user.pk, policy.pk.hex, **kwargs))
+            signatures.append(_policy_engine_task.s(self._user.pk, policy.pk.hex, **kwargs))
         self._group = group(signatures)()
         return self
 

passbook/core/requirements.txt

@@ -1,5 +1,6 @@
 django>=2.0
 django-model-utils
+django-ipware
 djangorestframework
 PyYAML
 raven

passbook/core/settings.py

@@ -77,6 +77,7 @@ INSTALLED_APPS = [
     'passbook.hibp_policy.apps.PassbookHIBPConfig',
     'passbook.pretend.apps.PassbookPretendConfig',
     'passbook.password_expiry_policy.apps.PassbookPasswordExpiryPolicyConfig',
+    'passbook.suspicious_policy.apps.PassbookSuspiciousPolicyConfig',
 ]
 
 # Message Tag fix for bootstrap CSS Classes

passbook/core/signals.py

@@ -20,7 +20,7 @@ def password_policy_checker(sender, password, **kwargs):
     _all_factors = PasswordFactor.objects.filter(enabled=True).order_by('order')
     for factor in _all_factors:
         policy_engine = PolicyEngine(factor.password_policies.all().select_subclasses())
-        policy_engine.for_user(sender)
+        policy_engine.for_user(sender).build()
         passing, messages = policy_engine.result
         if not passing:
             raise PasswordPolicyInvalid(*messages)

passbook/core/templatetags/passbook_user_settings.py

@@ -16,7 +16,7 @@ def user_factors(context):
     for factor in _all_factors:
         _link = factor.has_user_settings()
         policy_engine = PolicyEngine(factor.policies.all())
-        policy_engine.for_user(user)
+        policy_engine.for_user(user).with_request(context.get('request')).build()
         if policy_engine.result[0] and _link:
             matching_factors.append(_link)
     return matching_factors

passbook/password_expiry_policy/models.py

@@ -24,7 +24,7 @@ class PasswordExpiryPolicy(Policy):
         """If password change date is more than x days in the past, call set_unusable_password
         and show a notice"""
         actual_days = (now() - user.password_change_date).days
-        days_since_expiry = now() - (user.password_change_date + timedelta(days=self.days)).days
+        days_since_expiry = (now() - (user.password_change_date + timedelta(days=self.days))).days
         if actual_days >= self.days:
             if not self.deny_only:
                 user.set_unusable_password()

passbook/saml_idp/views.py

@@ -104,7 +104,7 @@ class LoginProcessView(ProviderMixin, LoginRequiredMixin, View):
     def _has_access(self):
         """Check if user has access to application"""
         policy_engine = PolicyEngine(self.provider.application.policies.all())
-        policy_engine.for_user(self.request.user)
+        policy_engine.for_user(self.request.user).with_request(self.request).build()
         return policy_engine.result
 
     def get(self, request, application):

passbook/suspicious_policy/__init__.py

@@ -0,0 +1,2 @@
+"""passbook suspicious_policy"""
+__version__ = '0.1.1-beta'

passbook/suspicious_policy/admin.py

@@ -0,0 +1,5 @@
+"""Passbook suspicious_policy Admin"""
+
+from passbook.lib.admin import admin_autoregister
+
+admin_autoregister('passbook_suspicious_policy')

passbook/suspicious_policy/apps.py

@@ -0,0 +1,15 @@
+"""Passbook suspicious_policy app config"""
+from importlib import import_module
+
+from django.apps import AppConfig
+
+
+class PassbookSuspiciousPolicyConfig(AppConfig):
+    """Passbook suspicious_policy app config"""
+
+    name = 'passbook.suspicious_policy'
+    label = 'passbook_suspicious_policy'
+    verbose_name = 'passbook Suspicious Request Detector'
+
+    def ready(self):
+        import_module('passbook.suspicious_policy.signals')

passbook/suspicious_policy/forms.py

@@ -0,0 +1,18 @@
+"""passbook suspicious request forms"""
+from django import forms
+
+from passbook.core.forms.policies import GENERAL_FIELDS
+from passbook.suspicious_policy.models import SuspiciousRequestPolicy
+
+
+class SuspiciousRequestPolicyForm(forms.ModelForm):
+    """Form to edit SuspiciousRequestPolicy"""
+
+    class Meta:
+
+        model = SuspiciousRequestPolicy
+        fields = GENERAL_FIELDS + ['check_ip', 'check_username', 'threshold']
+        widgets = {
+            'name': forms.TextInput(),
+            'value': forms.TextInput(),
+        }

passbook/suspicious_policy/migrations/0001_initial.py

@@ -0,0 +1,49 @@
+# Generated by Django 2.1.7 on 2019-03-03 18:17
+
+import django.db.models.deletion
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    initial = True
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('passbook_core', '0016_auto_20190227_1355'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='IPScore',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('ip', models.GenericIPAddressField()),
+                ('score', models.IntegerField(default=0)),
+                ('updated', models.DateTimeField(auto_now=True)),
+            ],
+        ),
+        migrations.CreateModel(
+            name='SuspiciousRequestPolicy',
+            fields=[
+                ('policy_ptr', models.OneToOneField(auto_created=True, on_delete=django.db.models.deletion.CASCADE, parent_link=True, primary_key=True, serialize=False, to='passbook_core.Policy')),
+                ('check_ip', models.BooleanField(default=True)),
+                ('check_username', models.BooleanField(default=True)),
+                ('threshold', models.IntegerField(default=-5)),
+            ],
+            options={
+                'abstract': False,
+            },
+            bases=('passbook_core.policy',),
+        ),
+        migrations.CreateModel(
+            name='UserScore',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('score', models.IntegerField(default=0)),
+                ('updated', models.DateTimeField(auto_now=True)),
+                ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
+            ],
+        ),
+    ]

passbook/suspicious_policy/migrations/0002_auto_20190303_1820.py

@@ -0,0 +1,17 @@
+# Generated by Django 2.1.7 on 2019-03-03 18:20
+
+from django.db import migrations
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('passbook_suspicious_policy', '0001_initial'),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name='suspiciousrequestpolicy',
+            options={'verbose_name': 'Suspicious Request Policy', 'verbose_name_plural': 'Suspicious Request Policies'},
+        ),
+    ]

passbook/suspicious_policy/migrations/0003_auto_20190303_1833.py

@@ -0,0 +1,25 @@
+# Generated by Django 2.1.7 on 2019-03-03 18:33
+
+import django.db.models.deletion
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('passbook_suspicious_policy', '0002_auto_20190303_1820'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='ipscore',
+            name='ip',
+            field=models.GenericIPAddressField(unique=True),
+        ),
+        migrations.AlterField(
+            model_name='userscore',
+            name='user',
+            field=models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL),
+        ),
+    ]

passbook/suspicious_policy/models.py

@@ -0,0 +1,51 @@
+"""passbook suspicious request policy"""
+from django.db import models
+from django.utils.translation import gettext as _
+
+from passbook.core.models import Policy, User
+
+
+class SuspiciousRequestPolicy(Policy):
+    """Return true if request IP/target username's score is below a certain threshold"""
+
+    check_ip = models.BooleanField(default=True)
+    check_username = models.BooleanField(default=True)
+    threshold = models.IntegerField(default=-5)
+
+    form = 'passbook.suspicious_policy.forms.SuspiciousRequestPolicyForm'
+
+    def passes(self, user: User):
+        remote_ip = user.remote_ip
+        passing = True
+        if self.check_ip:
+            ip_scores = IPScore.objects.filter(ip=remote_ip, score__lte=self.threshold)
+            passing = passing and ip_scores.exists()
+        if self.check_username:
+            user_scores = UserScore.objects.filter(user=user, score__lte=self.threshold)
+            passing = passing and user_scores.exists()
+        return passing
+
+    class Meta:
+
+        verbose_name = _('Suspicious Request Policy')
+        verbose_name_plural = _('Suspicious Request Policies')
+
+class IPScore(models.Model):
+    """Store score coming from the same IP"""
+
+    ip = models.GenericIPAddressField(unique=True)
+    score = models.IntegerField(default=0)
+    updated = models.DateTimeField(auto_now=True)
+
+    def __str__(self):
+        return "IPScore for %s @ %d" % (self.ip, self.score)
+
+class UserScore(models.Model):
+    """Store score attempting to log in as the same username"""
+
+    user = models.OneToOneField(User, on_delete=models.CASCADE)
+    score = models.IntegerField(default=0)
+    updated = models.DateTimeField(auto_now=True)
+
+    def __str__(self):
+        return "UserScore for %s @ %d" % (self.user, self.score)

passbook/suspicious_policy/signals.py

@@ -0,0 +1,39 @@
+"""passbook suspicious request signals"""
+from logging import getLogger
+
+from django.contrib.auth.signals import user_logged_in, user_login_failed
+from django.dispatch import receiver
+from ipware import get_client_ip
+
+from passbook.core.models import User
+from passbook.suspicious_policy.models import IPScore, UserScore
+
+LOGGER = getLogger(__name__)
+
+
+def update_score(request, username, amount):
+    """Update score for IP and User"""
+    remote_ip, _ = get_client_ip(request)
+    if not remote_ip:
+        remote_ip = '255.255.255.255'
+    ip_score, _ = IPScore.objects.update_or_create(ip=remote_ip)
+    ip_score.score += amount
+    ip_score.save()
+    LOGGER.debug("Added %s to score of IP %s", amount, remote_ip)
+    user = User.objects.filter(username=username)
+    if not user.exists():
+        return
+    user_score, _ = UserScore.objects.update_or_create(user=user.first())
+    user_score.score += amount
+    user_score.save()
+    LOGGER.debug("Added %s to score of User %s", amount, username)
+
+@receiver(user_login_failed)
+def handle_failed_login(sender, request, credentials, **kwargs):
+    """Lower Score for failed loging attempts"""
+    update_score(request, credentials.get('username'), -1)
+
+@receiver(user_logged_in)
+def handle_successful_login(sender, request, user, **kwargs):
+    """Raise score for successful attempts"""
+    update_score(request, user.username, 1)

requirements.txt

@@ -4,7 +4,6 @@
 -r passbook/saml_idp/requirements.txt
 -r passbook/otp/requirements.txt
 -r passbook/oauth_provider/requirements.txt
--r passbook/audit/requirements.txt
 -r passbook/captcha_factor/requirements.txt
 -r passbook/admin/requirements.txt
 -r passbook/api/requirements.txt