Merge branch '883-stronger-passwords' into 'develop'

Fix #883: Prevent usage of too weak passwords

Closes #883

See merge request funkwhale/funkwhale!901

api/config/settings/common.py

@@ -583,6 +583,17 @@ JWT_AUTH = {
     "JWT_GET_USER_SECRET_KEY": get_user_secret_key,
 }
 OLD_PASSWORD_FIELD_ENABLED = True
+AUTH_PASSWORD_VALIDATORS = [
+    {
+        "NAME": "django.contrib.auth.password_validation.UserAttributeSimilarityValidator"
+    },
+    {
+        "NAME": "django.contrib.auth.password_validation.MinimumLengthValidator",
+        "OPTIONS": {"min_length": env.int("PASSWORD_MIN_LENGTH", default=8)},
+    },
+    {"NAME": "django.contrib.auth.password_validation.CommonPasswordValidator"},
+    {"NAME": "django.contrib.auth.password_validation.NumericPasswordValidator"},
+]
 ACCOUNT_ADAPTER = "funkwhale_api.users.adapters.FunkwhaleAccountAdapter"
 CORS_ORIGIN_ALLOW_ALL = True
 # CORS_ORIGIN_WHITELIST = (

api/funkwhale_api/users/serializers.py

@@ -5,7 +5,7 @@ from django.utils.deconstruct import deconstructible
 from django.utils.translation import gettext_lazy as _
 
 from rest_auth.serializers import PasswordResetSerializer as PRS
-from rest_auth.registration.serializers import RegisterSerializer as RS
+from rest_auth.registration.serializers import RegisterSerializer as RS, get_adapter
 from rest_framework import serializers
 from versatileimagefield.serializers import VersatileImageFieldSerializer
 
@@ -42,6 +42,15 @@ class RegisterSerializer(RS):
         except models.Invitation.DoesNotExist:
             raise serializers.ValidationError("Invalid invitation code")
 
+    def validate(self, validated_data):
+        data = super().validate(validated_data)
+        # we create a fake user obj with validated data so we can validate
+        # password properly (we have a password validator that requires
+        # a user object)
+        user = models.User(username=data["username"], email=data["email"])
+        get_adapter().clean_password(data["password1"], user)
+        return data
+
     def save(self, request):
         user = super().save(request)
         if self.validated_data.get("invitation"):

api/tests/users/test_admin.py

@@ -4,7 +4,11 @@ from funkwhale_api.users.admin import MyUserCreationForm
 def test_clean_username_success(db):
     # Instantiate the form with a new username
     form = MyUserCreationForm(
-        {"username": "alamode", "password1": "123456", "password2": "123456"}
+        {
+            "username": "alamode",
+            "password1": "thisismypassword",
+            "password2": "thisismypassword",
+        }
     )
     # Run is_valid() to trigger the validation
     valid = form.is_valid()
@@ -19,7 +23,11 @@ def test_clean_username_false(factories):
     user = factories["users.User"]()
     # Instantiate the form with the same username as self.user
     form = MyUserCreationForm(
-        {"username": user.username, "password1": "123456", "password2": "123456"}
+        {
+            "username": user.username,
+            "password1": "thisismypassword",
+            "password2": "thisismypassword",
+        }
     )
     # Run is_valid() to trigger the validation, which is going to fail
     # because the username is already taken

api/tests/users/test_serializers.py

+import pytest
+
+from funkwhale_api.users import serializers
+
+
+@pytest.mark.parametrize(
+    "data, expected_error",
+    [
+        (
+            {
+                "username": "myusername",
+                "email": "test@hello.com",
+                "password1": "myusername",
+            },
+            r".*password is too similar.*",
+        ),
+        (
+            {"username": "myusername", "email": "test@hello.com", "password1": "test"},
+            r".*must contain at least 8 characters.*",
+        ),
+        (
+            {
+                "username": "myusername",
+                "email": "test@hello.com",
+                "password1": "superman",
+            },
+            r".*password is too common.*",
+        ),
+        (
+            {
+                "username": "myusername",
+                "email": "test@hello.com",
+                "password1": "123457809878",
+            },
+            r".*password is entirely numeric.*",
+        ),
+    ],
+)
+def test_registration_serializer_validates_password_properly(data, expected_error, db):
+    data["password2"] = data["password1"]
+    serializer = serializers.RegisterSerializer(data=data)
+
+    with pytest.raises(serializers.serializers.ValidationError, match=expected_error):
+        serializer.is_valid(raise_exception=True)

api/tests/users/test_views.py

@@ -9,8 +9,8 @@ def test_can_create_user_via_api(preferences, api_client, db):
     data = {
         "username": "test1",
         "email": "test1@test.com",
-        "password1": "testtest",
-        "password2": "testtest",
+        "password1": "thisismypassword",
+        "password2": "thisismypassword",
     }
     preferences["users__registration_enabled"] = True
     response = api_client.post(url, data)
@@ -72,8 +72,8 @@ def test_can_signup_with_invitation(preferences, factories, api_client):
     data = {
         "username": "test1",
         "email": "test1@test.com",
-        "password1": "testtest",
-        "password2": "testtest",
+        "password1": "thisismypassword",
+        "password2": "thisismypassword",
         "invitation": "hello",
     }
     preferences["users__registration_enabled"] = False
@@ -157,11 +157,16 @@ def test_changing_password_updates_secret_key(logged_in_api_client):
     user = logged_in_api_client.user
     password = user.password
     secret_key = user.secret_key
-    payload = {"old_password": "test", "new_password1": "new", "new_password2": "new"}
+    payload = {
+        "old_password": "test",
+        "new_password1": "thisismypassword",
+        "new_password2": "thisismypassword",
+    }
     url = reverse("change_password")
 
-    logged_in_api_client.post(url, payload)
+    response = logged_in_api_client.post(url, payload)
 
+    assert response.status_code == 200
     user.refresh_from_db()
 
     assert user.secret_key != secret_key
@@ -295,8 +300,8 @@ def test_creating_user_creates_actor_as_well(
     data = {
         "username": "test1",
         "email": "test1@test.com",
-        "password1": "testtest",
-        "password2": "testtest",
+        "password1": "thisismypassword",
+        "password2": "thisismypassword",
     }
     preferences["users__registration_enabled"] = True
     mocker.patch("funkwhale_api.users.models.create_actor", return_value=actor)
@@ -316,8 +321,8 @@ def test_creating_user_sends_confirmation_email(
     data = {
         "username": "test1",
         "email": "test1@test.com",
-        "password1": "testtest",
-        "password2": "testtest",
+        "password1": "thisismypassword",
+        "password2": "thisismypassword",
     }
     preferences["users__registration_enabled"] = True
     preferences["instance__name"] = "Hello world"

changes/changelog.d/883.enhancement

+Prevent usage of too weak passwords (#883)