Browse Source

#923: moved auth api (minus registration) from forms to serializers

Rafał Pitoń 7 years ago
parent
commit
379b3a2634

+ 77 - 104
misago/users/api/auth.py

@@ -2,17 +2,18 @@ from rest_framework.decorators import api_view, permission_classes
 from rest_framework.response import Response
 
 from django.contrib import auth
-from django.contrib.auth.password_validation import validate_password
 from django.core.exceptions import ValidationError
 from django.utils.translation import ugettext as _
 from django.views.decorators.csrf import csrf_protect
+from django.shortcuts import get_object_or_404
 
 from misago.conf import settings
-from misago.core.decorators import require_dict_data
 from misago.core.mail import mail_user
 from misago.users.bans import get_user_ban
 from misago.users.forms.auth import AuthenticationForm, ResendActivationForm, ResetPasswordForm
-from misago.users.serializers import AnonymousUserSerializer, AuthenticatedUserSerializer
+from misago.users.serializers import (
+    AnonymousUserSerializer, AuthenticatedUserSerializer, LoginSerializer,
+    ResendActivationSerializer, SendPasswordFormSerializer, ChangePasswordSerializer)
 from misago.users.tokens import (
     is_password_change_token_valid, make_activation_token, make_password_change_token)
 
@@ -32,20 +33,20 @@ def gateway(request):
 @api_view(['POST'])
 @permission_classes((UnbannedAnonOnly, ))
 @csrf_protect
-@require_dict_data
 def login(request):
     """
     POST /auth/ with CSRF, username and password
     will attempt to authenticate new user
     """
-    form = AuthenticationForm(request, data=request.data)
-    if form.is_valid():
-        auth.login(request, form.user_cache)
-        return Response(
-            AuthenticatedUserSerializer(form.user_cache).data,
-        )
-    else:
-        return Response(form.get_errors_dict(), status=400)
+    serializer = LoginSerializer(request, data=request.data)
+    serializer.is_valid(raise_exception=True)
+
+    user = serializer.validated_data['user']
+    auth.login(request, user)
+
+    return Response(
+        AuthenticatedUserSerializer(user).data,
+    )
 
 
 @api_view()
@@ -60,9 +61,9 @@ def session_user(request):
 
 
 @api_view(['GET'])
-def get_criteria(request):
-    """GET /auth/criteria/ will return password and username criteria for accounts"""
-    criteria = {
+def get_requirements(request):
+    """GET /auth/requirements/ will return password and username requirements"""
+    requirements = {
         'username': {
             'min_length': settings.username_length_min,
             'max_length': settings.username_length_max,
@@ -72,89 +73,82 @@ def get_criteria(request):
 
     for validator in settings.AUTH_PASSWORD_VALIDATORS:
         validator_dict = {'name': validator['NAME'].split('.')[-1]}
-
         validator_dict.update(validator.get('OPTIONS', {}))
+        requirements['password'].append(validator_dict)
 
-        criteria['password'].append(validator_dict)
-
-    return Response(criteria)
+    return Response(requirements)
 
 
 @api_view(['POST'])
 @permission_classes((UnbannedAnonOnly, ))
 @csrf_protect
-@require_dict_data
 def send_activation(request):
     """
     POST /auth/send-activation/ with CSRF token and email
     will mail account activation link to requester
     """
-    form = ResendActivationForm(request.data)
-    if form.is_valid():
-        requesting_user = form.user_cache
-
-        mail_subject = _("Activate %(user)s account on %(forum_name)s forums") % {
-            'user': requesting_user.username,
-            'forum_name': settings.forum_name,
-        }
-
-        mail_user(
-            request,
-            requesting_user,
-            mail_subject,
-            'misago/emails/activation/by_user',
-            {
-                'activation_token': make_activation_token(requesting_user),
-            },
-        )
-
-        return Response({
-            'username': form.user_cache.username,
-            'email': form.user_cache.email,
-        })
-    else:
-        return Response(
-            form.get_errors_dict(),
-            status=400,
-        )
+    serializer = ResendActivationSerializer(data=request.data)
+    serializer.is_valid(raise_exception=True)
+    serializer.raise_if_banned()
+
+    user = serializer.validated_data['user']
+
+    mail_subject = _("Activate %(user)s account on %(forum_name)s forums") % {
+        'user': user.username,
+        'forum_name': settings.forum_name,
+    }
+
+    mail_user(
+        request,
+        user,
+        mail_subject,
+        'misago/emails/activation/by_user',
+        {
+            'activation_token': make_activation_token(user),
+        },
+    )
+
+    return Response({
+        'username': user.username,
+        'email': user.email,
+    })
 
 
 @api_view(['POST'])
 @permission_classes((UnbannedOnly, ))
 @csrf_protect
-@require_dict_data
 def send_password_form(request):
     """
     POST /auth/send-password-form/ with CSRF token and email
     will mail change password form link to requester
     """
-    form = ResetPasswordForm(request.data)
-    if form.is_valid():
-        requesting_user = form.user_cache
-
-        mail_subject = _("Change %(user)s password on %(forum_name)s forums") % {
-            'user': requesting_user.username,
-            'forum_name': settings.forum_name,
-        }
-
-        confirmation_token = make_password_change_token(requesting_user)
-
-        mail_user(
-            request,
-            requesting_user,
-            mail_subject,
-            'misago/emails/change_password_form_link',
-            {
-                'confirmation_token': confirmation_token,
-            },
-        )
-
-        return Response({
-            'username': form.user_cache.username,
-            'email': form.user_cache.email,
-        })
-    else:
-        return Response(form.get_errors_dict(), status=400)
+    serializer = SendPasswordFormSerializer(data=request.data)
+    serializer.is_valid(raise_exception=True)
+    serializer.raise_if_banned()
+
+    user = serializer.validated_data['user']
+
+    mail_subject = _("Change %(user)s password on %(forum_name)s forums") % {
+        'user': user.username,
+        'forum_name': settings.forum_name,
+    }
+
+    confirmation_token = make_password_change_token(user)
+
+    mail_user(
+        request,
+        user,
+        mail_subject,
+        'misago/emails/change_password_form_link',
+        {
+            'confirmation_token': confirmation_token,
+        },
+    )
+
+    return Response({
+        'username': user.username,
+        'email': user.email,
+    })
 
 
 class PasswordChangeFailed(Exception):
@@ -164,36 +158,15 @@ class PasswordChangeFailed(Exception):
 @api_view(['POST'])
 @permission_classes((UnbannedOnly, ))
 @csrf_protect
-@require_dict_data
-def change_forgotten_password(request, pk, token):
+def change_forgotten_password(request, pk):
     """
-    POST /auth/change-password/user/token/ with CSRF and new password
+    POST /auth/change-password/user/ with CSRF and new password
     will change forgotten password
     """
-    invalid_message = _("Form link is invalid. Please try again.")
-    expired_message = _("Your link has expired. Please request new one.")
-
-    try:
-        try:
-            user = UserModel.objects.get(pk=pk, is_active=True)
-        except UserModel.DoesNotExist:
-            raise PasswordChangeFailed(invalid_message)
-
-        if request.user.is_authenticated and request.user.id != user.id:
-            raise PasswordChangeFailed(invalid_message)
-        if not is_password_change_token_valid(user, token):
-            raise PasswordChangeFailed(invalid_message)
-
-        if user.requires_activation:
-            raise PasswordChangeFailed(expired_message)
-        if get_user_ban(user):
-            raise PasswordChangeFailed(expired_message)
-    except PasswordChangeFailed as e:
-        return Response({'detail': e.args}, status=400)
-
-    new_password = request.data.get('password', '')
-    validate_password(new_password, user=user)
-    user.set_password(new_password)
-    user.save()
+    user = get_object_or_404(UserModel, pk=pk, is_active=True)
+    serializer = ChangePasswordSerializer(user, data=request.data)
+    serializer.is_valid(raise_exception=True)
+
+    serializer.save()
 
     return Response({'username': user.username})

+ 73 - 0
misago/users/authmixin.py

@@ -0,0 +1,73 @@
+from django.contrib.auth import authenticate, get_user_model
+from django.core.exceptions import ValidationError
+from django.utils.translation import ugettext_lazy, ugettext as _
+
+from misago.core.exceptions import Banned
+from misago.users.bans import get_user_ban
+
+
+UserModel = get_user_model()
+
+
+class AuthMixin(object):
+    """
+    Mixin with utils for Auth forms and serializers
+    """
+    auth_messages = {
+        'empty_data': ugettext_lazy("Fill out both fields."),
+        'invalid_login': ugettext_lazy("Login or password is incorrect."),
+        'inactive_user': ugettext_lazy("You have to activate your account before you will be able to sign in."),
+        'inactive_admin': ugettext_lazy(
+            "Your account has to be activated by Administrator before you will be able to sign in."
+        ),
+    }
+
+    def authenticate(self, username, password):
+        if username and password:
+            user = authenticate(username=username, password=password)
+
+            if user is None or not user.is_active:
+                self.raise_for_code('invalid_login')
+        else:
+            self.raise_for_code('empty_data')
+
+        return user
+
+    def get_user_by_email(self, email):
+        if not email:
+            return None
+
+        try:
+            user = UserModel.objects.get_by_email(email)
+            if not user.is_active:
+                raise UserModel.DoesNotExist()
+            return user
+        except UserModel.DoesNotExist:
+            raise ValidationError(_("No user with this e-mail exists."))
+
+    def confirm_login_allowed(self, user):
+        self.confirm_user_active(user)
+        self.confirm_user_not_banned(user)
+
+    def confirm_user_active(self, user):
+        if user.requires_activation_by_admin:
+            self.raise_for_code('inactive_admin')
+        if user.requires_activation_by_user:
+            self.raise_for_code('inactive_user')
+
+    def confirm_user_not_banned(self, user):
+        ban = self.get_user_ban(user)
+        if ban:
+            raise Banned(ban=ban)
+
+    def get_user_ban(self, user):
+        if user.is_staff:
+            return None
+        return get_user_ban(user)
+
+    def raise_if_banned(self):
+        user = self.validated_data.get('user')
+        self.confirm_user_not_banned(user)
+
+    def raise_for_code(self, code):
+        raise ValidationError(self.auth_messages[code], code=code)

+ 104 - 0
misago/users/serializers/auth.py

@@ -1,9 +1,14 @@
 from rest_framework import serializers
 
 from django.contrib.auth import get_user_model
+from django.contrib.auth.password_validation import validate_password
+from django.core.exceptions import ValidationError
 from django.urls import reverse
+from django.utils.translation import ugettext_lazy, ugettext as _
 
 from misago.acl import serialize_acl
+from misago.users.authmixin import AuthMixin
+from misago.users.tokens import is_password_change_token_valid
 
 from .user import UserSerializer
 
@@ -13,6 +18,10 @@ UserModel = get_user_model()
 __all__ = [
     'AuthenticatedUserSerializer',
     'AnonymousUserSerializer',
+    'LoginSerializer',
+    'ResendActivationSerializer',
+    'SendPasswordFormSerializer',
+    'ChangePasswordSerializer',
 ]
 
 
@@ -81,3 +90,98 @@ class AnonymousUserSerializer(serializers.Serializer, AuthFlags):
             return serialize_acl(obj)
         else:
             return {}
+
+
+class LoginSerializer(serializers.Serializer, AuthMixin):
+    username = serializers.CharField(max_length=254)
+    password = serializers.CharField(max_length=255, trim_whitespace=False)
+
+    def validate(self, data):
+        user = self.authenticate(data.get('username'), data.get('password'))
+        self.confirm_login_allowed(user)
+        return {'user': user}
+
+
+class GetUserSerializer(serializers.Serializer, AuthMixin):
+    email = serializers.EmailField(max_length=255)
+
+    def validate(self, data):
+        user = self.get_user_by_email(data.get('email'))
+        self.confirm_allowed(user)
+        return {'user': user}
+
+    def confirm_allowed(self, user):
+        """override this method to include additional checks"""
+        pass
+
+
+class ResendActivationSerializer(GetUserSerializer):
+    def confirm_allowed(self, user):
+        username_format = {'user': user.username}
+        if not user.requires_activation:
+            message = _("%(user)s, your account is already active.")
+            raise ValidationError(message % username_format)
+        if user.requires_activation_by_admin:
+            message = _("%(user)s, only administrator may activate your account.")
+            raise ValidationError(message % username_format)
+
+
+class SendPasswordFormSerializer(GetUserSerializer):
+    auth_messages = {
+        'inactive_user': ugettext_lazy(
+            "You have to activate your account before "
+            "you will be able to request new password."
+        ),
+        'inactive_admin': ugettext_lazy(
+            "Administrator has to activate your account before "
+            "you will be able to request new password."
+        ),
+    }
+
+    def confirm_allowed(self, user):
+        self.confirm_user_active(user)
+
+
+class ChangePasswordSerializer(serializers.Serializer, AuthMixin):
+    password = serializers.CharField(
+        max_length=255,
+        trim_whitespace=False,
+    )
+    token = serializers.CharField(max_length=255)
+
+    auth_messages = {
+        'inactive_user': ugettext_lazy(
+            "You have to activate your account before "
+            "you will be able to change your password."
+        ),
+        'inactive_admin': ugettext_lazy(
+            "Administrator has to activate your account before "
+            "you will be able to change your password."
+        ),
+    }
+
+    def confirm_allowed(self):
+        self.confirm_user_active(self.instance)
+        self.confirm_user_not_banned(self.instance)
+
+    def validate_password(self, value):
+        validate_password(value, user=self.instance)
+        return value
+
+    def validate_token(self, value):
+        if not is_password_change_token_valid(self.instance, value):
+            raise ValidationError(_("Form link is invalid or expired. Please try again."))
+        return value
+
+    def validate(self, data):
+        self.confirm_allowed()
+        return data
+
+    def save(self):
+        self.instance.set_password(self.validated_data['password'])
+        self.instance.save()
+
+
+class CreateUserSerializer(serializers.ModelSerializer):
+    class Meta:
+        model = UserModel

+ 127 - 65
misago/users/tests/test_auth_api.py

@@ -79,10 +79,28 @@ class GatewayTests(TestCase):
         self.assertEqual(user_json['id'], user.id)
         self.assertEqual(user_json['username'], user.username)
 
-    def test_submit_empty(self):
+    def test_submit_no_data(self):
         """login api errors for no body"""
         response = self.client.post('/api/auth/')
-        self.assertContains(response, 'empty_data', status_code=400)
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {
+            'username': ['This field is required.'],
+            'password': ['This field is required.'],
+        })
+
+    def test_submit_empty(self):
+        """login api errors for empty fields"""
+        response = self.client.post('/api/auth/', data={
+            'username': '',
+            'password': '',
+        })
+
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {
+            'username': ['This field may not be blank.'],
+            'password': ['This field may not be blank.'],
+        })
 
     def test_submit_invalid(self):
         """login api errors for invalid data"""
@@ -91,6 +109,7 @@ class GatewayTests(TestCase):
             'false',
             content_type="application/json",
         )
+
         self.assertContains(response, "Invalid data.", status_code=400)
 
     def test_login_banned(self):
@@ -110,15 +129,14 @@ class GatewayTests(TestCase):
                 'password': 'Pass.123',
             },
         )
-        self.assertEqual(response.status_code, 400)
-
-        response_json = response.json()
-
-        self.assertEqual(response_json['detail']['plain'], ban.user_message)
-        self.assertEqual(
-            response_json['detail']['html'], '<p>%s</p>' % ban.user_message
-        )
-        self.assertIn('expires_on', response_json)
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.json(), {
+            'detail': {
+                'html': '<p>%s</p>' % ban.user_message,
+                'plain': ban.user_message,
+            },
+            'expires_on': None,
+        })
 
         response = self.client.get('/api/auth/')
         self.assertEqual(response.status_code, 200)
@@ -157,7 +175,7 @@ class GatewayTests(TestCase):
 
     def test_login_inactive_admin(self):
         """login api fails to sign admin-activated user in"""
-        UserModel.objects.create_user('Bob', 'bob@test.com', 'Pass.123', requires_activation=1)
+        UserModel.objects.create_user('Bob', 'bob@test.com', 'Pass.123', requires_activation=2)
 
         response = self.client.post(
             '/api/auth/',
@@ -167,9 +185,11 @@ class GatewayTests(TestCase):
             },
         )
         self.assertEqual(response.status_code, 400)
-
-        response_json = response.json()
-        self.assertEqual(response_json['code'], 'inactive_user')
+        self.assertEqual(response.json(), {
+            'non_field_errors': [
+                "Your account has to be activated by Administrator before you will be able to sign in.",
+            ],
+        })
 
         response = self.client.get('/api/auth/')
         self.assertEqual(response.status_code, 200)
@@ -179,7 +199,7 @@ class GatewayTests(TestCase):
 
     def test_login_inactive_user(self):
         """login api fails to sign user-activated user in"""
-        UserModel.objects.create_user('Bob', 'bob@test.com', 'Pass.123', requires_activation=2)
+        UserModel.objects.create_user('Bob', 'bob@test.com', 'Pass.123', requires_activation=1)
 
         response = self.client.post(
             '/api/auth/',
@@ -189,9 +209,11 @@ class GatewayTests(TestCase):
             },
         )
         self.assertEqual(response.status_code, 400)
-
-        response_json = response.json()
-        self.assertEqual(response_json['code'], 'inactive_admin')
+        self.assertEqual(response.json(), {
+            'non_field_errors': [
+                "You have to activate your account before you will be able to sign in.",
+            ],
+        })
 
         response = self.client.get('/api/auth/')
         self.assertEqual(response.status_code, 200)
@@ -222,10 +244,10 @@ class GatewayTests(TestCase):
         self.assertIsNone(user_json['id'])
 
 
-class UserCredentialsTests(TestCase):
+class UserRequirementsTests(TestCase):
     def test_edge_returns_response(self):
         """api edge has no showstoppers"""
-        response = self.client.get('/api/auth/criteria/')
+        response = self.client.get('/api/auth/requirements/')
         self.assertEqual(response.status_code, 200)
 
 
@@ -250,8 +272,8 @@ class SendActivationAPITests(TestCase):
         self.assertIn('Activate Bob', mail.outbox[0].subject)
 
     def test_submit_banned(self):
-        """request activation link api passes for banned users"""
-        Ban.objects.create(
+        """request activation link api errors for banned users"""
+        ban = Ban.objects.create(
             check_type=Ban.USERNAME,
             banned_value=self.user.username,
             user_message='Nope!',
@@ -263,9 +285,16 @@ class SendActivationAPITests(TestCase):
                 'email': self.user.email,
             },
         )
-        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.json(), {
+            'detail': {
+                'html': '<p>%s</p>' % ban.user_message,
+                'plain': ban.user_message,
+            },
+            'expires_on': None,
+        })
 
-        self.assertIn('Activate Bob', mail.outbox[0].subject)
+        self.assertTrue(not mail.outbox)
 
     def test_submit_disabled(self):
         """request activation link api fails disabled users"""
@@ -278,14 +307,15 @@ class SendActivationAPITests(TestCase):
                 'email': self.user.email,
             },
         )
-        self.assertContains(response, 'not_found', status_code=400)
+        self.assertContains(response, "No user with this e-mail exists.", status_code=400)
 
         self.assertTrue(not mail.outbox)
 
     def test_submit_empty(self):
         """request activation link api errors for no body"""
         response = self.client.post(self.link)
-        self.assertContains(response, 'empty_email', status_code=400)
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {'email': ["This field is required."]})
 
         self.assertTrue(not mail.outbox)
 
@@ -306,7 +336,7 @@ class SendActivationAPITests(TestCase):
                 'email': 'fake@mail.com',
             },
         )
-        self.assertContains(response, 'not_found', status_code=400)
+        self.assertContains(response, "No user with this e-mail exists.", status_code=400)
 
         self.assertTrue(not mail.outbox)
 
@@ -321,7 +351,7 @@ class SendActivationAPITests(TestCase):
                 'email': self.user.email,
             },
         )
-        self.assertContains(response, 'Bob, your account is already active.', status_code=400)
+        self.assertContains(response, "Bob, your account is already active.", status_code=400)
 
     def test_submit_inactive_user(self):
         """request activation link api errors for admin-activated users"""
@@ -334,7 +364,7 @@ class SendActivationAPITests(TestCase):
                 'email': self.user.email,
             },
         )
-        self.assertContains(response, 'inactive_admin', status_code=400)
+        self.assertContains(response, "only administrator may activate your account", status_code=400)
 
         self.assertTrue(not mail.outbox)
 
@@ -347,7 +377,10 @@ class SendActivationAPITests(TestCase):
                 'email': self.user.email,
             }
         )
-        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.json(), {
+            'username': self.user.username,
+            'email': self.user.email,
+        })
 
         self.assertTrue(mail.outbox)
 
@@ -371,8 +404,8 @@ class SendPasswordFormAPITests(TestCase):
         self.assertIn('Change Bob password', mail.outbox[0].subject)
 
     def test_submit_banned(self):
-        """request change password form link api sends reset link mail"""
-        Ban.objects.create(
+        """request change password form link api errors for banned users"""
+        ban = Ban.objects.create(
             check_type=Ban.USERNAME,
             banned_value=self.user.username,
             user_message='Nope!',
@@ -384,9 +417,16 @@ class SendPasswordFormAPITests(TestCase):
                 'email': self.user.email,
             },
         )
-        self.assertEqual(response.status_code, 200)
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.json(), {
+            'detail': {
+                'html': '<p>%s</p>' % ban.user_message,
+                'plain': ban.user_message,
+            },
+            'expires_on': None,
+        })
 
-        self.assertIn('Change Bob password', mail.outbox[0].subject)
+        self.assertTrue(not mail.outbox)
 
     def test_submit_disabled(self):
         """request change password form api fails disabled users"""
@@ -399,14 +439,15 @@ class SendPasswordFormAPITests(TestCase):
                 'email': self.user.email,
             },
         )
-        self.assertContains(response, 'not_found', status_code=400)
+        self.assertContains(response, "No user with this e-mail exists.", status_code=400)
 
         self.assertTrue(not mail.outbox)
 
     def test_submit_empty(self):
         """request change password form link api errors for no body"""
         response = self.client.post(self.link)
-        self.assertContains(response, 'empty_email', status_code=400)
+        self.assertEqual(response.status_code, 400)
+        self.assertEqual(response.json(), {'email': ["This field is required."]})
 
         self.assertTrue(not mail.outbox)
 
@@ -418,7 +459,7 @@ class SendPasswordFormAPITests(TestCase):
                 'email': 'fake@mail.com',
             },
         )
-        self.assertContains(response, 'not_found', status_code=400)
+        self.assertContains(response, "No user with this e-mail exists.", status_code=400)
 
         self.assertTrue(not mail.outbox)
 
@@ -442,7 +483,8 @@ class SendPasswordFormAPITests(TestCase):
                 'email': self.user.email,
             },
         )
-        self.assertContains(response, 'inactive_user', status_code=400)
+        self.assertContains(response, "You have to activate your account", status_code=400)
+        self.assertTrue(not mail.outbox)
 
         self.user.requires_activation = 2
         self.user.save()
@@ -453,23 +495,22 @@ class SendPasswordFormAPITests(TestCase):
                 'email': self.user.email,
             },
         )
-        self.assertContains(response, 'inactive_admin', status_code=400)
-
+        self.assertContains(response, "Administrator has to activate your account", status_code=400)
         self.assertTrue(not mail.outbox)
 
 
 class ChangePasswordAPITests(TestCase):
     def setUp(self):
         self.user = UserModel.objects.create_user('Bob', 'bob@test.com', 'Pass.123')
-
-        self.link = '/api/auth/change-password/%s/%s/'
+        self.link = '/api/auth/change-password/%s/'
 
     def test_submit_valid(self):
         """submit change password form api changes password"""
         response = self.client.post(
-            self.link % (self.user.pk, make_password_change_token(self.user)),
+            self.link % self.user.pk,
             data={
                 'password': 'n3wp4ss!',
+                'token': make_password_change_token(self.user),
             },
         )
         self.assertEqual(response.status_code, 200)
@@ -480,9 +521,10 @@ class ChangePasswordAPITests(TestCase):
     def test_submit_with_whitespaces(self):
         """submit change password form api changes password with whitespaces"""
         response = self.client.post(
-            self.link % (self.user.pk, make_password_change_token(self.user)),
+            self.link % self.user.pk,
             data={
                 'password': ' n3wp4ss! ',
+                'token': make_password_change_token(self.user),
             },
         )
         self.assertEqual(response.status_code, 200)
@@ -493,30 +535,46 @@ class ChangePasswordAPITests(TestCase):
     def test_submit_invalid_data(self):
         """login api errors for invalid data"""
         response = self.client.post(
-            self.link % (self.user.pk, make_password_change_token(self.user)),
+            self.link % self.user.pk,
             'false',
             content_type="application/json",
         )
         self.assertContains(response, "Invalid data.", status_code=400)
 
-    def test_invalid_token_link(self):
+    def test_invalid_token(self):
         """api errors on invalid user id link"""
-        response = self.client.post(self.link % (self.user.pk, 'asda7ad89sa7d9s789as'))
-
-        self.assertContains(response, "Form link is invalid.", status_code=400)
+        response = self.client.post(
+            self.link % self.user.pk,
+            data={
+                'password': 'n3wp4ss!',
+                'token': 'invalid!',
+            },
+        )
+        self.assertContains(response, "Form link is invalid or expired.", status_code=400)
 
     def test_banned_user_link(self):
         """request errors because user is banned"""
-        Ban.objects.create(
+        ban = Ban.objects.create(
             check_type=Ban.USERNAME,
             banned_value=self.user.username,
             user_message='Nope!',
         )
 
         response = self.client.post(
-            self.link % (self.user.pk, make_password_change_token(self.user))
+            self.link % self.user.pk,
+            data={
+                'password': 'n3wp4ss!',
+                'token': make_password_change_token(self.user),
+            },
         )
-        self.assertContains(response, "Your link has expired.", status_code=400)
+        self.assertEqual(response.status_code, 403)
+        self.assertEqual(response.json(), {
+            'detail': {
+                'html': '<p>%s</p>' % ban.user_message,
+                'plain': ban.user_message,
+            },
+            'expires_on': None,
+        })
 
     def test_inactive_user(self):
         """change password api errors for inactive users"""
@@ -524,31 +582,35 @@ class ChangePasswordAPITests(TestCase):
         self.user.save()
 
         response = self.client.post(
-            self.link % (self.user.pk, make_password_change_token(self.user))
+            self.link % self.user.pk,
+            data={
+                'password': 'n3wp4ss!',
+                'token': make_password_change_token(self.user),
+            },
         )
-        self.assertContains(response, "Your link has expired.", status_code=400)
+        self.assertContains(response, "You have to activate your account", status_code=400)
 
         self.user.requires_activation = 2
         self.user.save()
 
         response = self.client.post(
-            self.link % (self.user.pk, make_password_change_token(self.user))
+            self.link % self.user.pk,
+            data={
+                'password': 'n3wp4ss!',
+                'token': make_password_change_token(self.user),
+            },
         )
-        self.assertContains(response, "Your link has expired.", status_code=400)
+        self.assertContains(response, "Administrator has to activate your account", status_code=400)
 
     def test_disabled_user(self):
         """change password api errors for disabled users"""
         self.user.is_active = False
         self.user.save()
 
-        response = self.client.post(
-            self.link % (self.user.pk, make_password_change_token(self.user))
-        )
-        self.assertContains(response, "Form link is invalid.", status_code=400)
+        response = self.client.post(self.link % self.user.pk)
+        self.assertEqual(response.status_code, 404)
 
     def test_submit_empty(self):
         """change password api errors for empty body"""
-        response = self.client.post(
-            self.link % (self.user.pk, make_password_change_token(self.user))
-        )
-        self.assertContains(response, "This password is too shor", status_code=400)
+        response = self.client.post(self.link % self.user.pk)
+        self.assertContains(response, "This field is required.", status_code=400)

+ 2 - 6
misago/users/urls/api.py

@@ -9,14 +9,10 @@ from misago.users.api.users import UserViewSet
 
 urlpatterns = [
     url(r'^auth/$', auth.gateway, name='auth'),
-    url(r'^auth/criteria/$', auth.get_criteria, name='auth-criteria'),
+    url(r'^auth/requirements/$', auth.get_requirements, name='auth-requirements'),
     url(r'^auth/send-activation/$', auth.send_activation, name='send-activation'),
     url(r'^auth/send-password-form/$', auth.send_password_form, name='send-password-form'),
-    url(
-        r'^auth/change-password/(?P<pk>\d+)/(?P<token>[a-zA-Z0-9]+)/$',
-        auth.change_forgotten_password,
-        name='change-forgotten-password'
-    ),
+    url(r'^auth/change-password/(?P<pk>\d+)/', auth.change_forgotten_password, name='change-forgotten-password'),
     url(r'^captcha-question/$', captcha.question, name='captcha-question'),
     url(r'^mention/$', mention.mention_suggestions, name='mention-suggestions'),
 ]

+ 1 - 1
misago/users/views/forgottenpassword.py

@@ -28,7 +28,7 @@ class ResetError(Exception):
 
 @reset_view
 def reset_password_form(request, pk, token):
-    requesting_user = get_object_or_404(get_user_model(), pk=pk)
+    requesting_user = get_object_or_404(get_user_model(), pk=pk, is_active=True)
 
     try:
         if (request.user.is_authenticated and request.user.id != requesting_user.id):