From 39af0485f80752fc53348b89ae3f6d594e21f96f Mon Sep 17 00:00:00 2001 From: Spitap Date: Mon, 5 Dec 2022 16:57:38 +0100 Subject: [PATCH 1/8] Imported to main repo --- doc/configuration.rst | 15 ++ doc/upgrade.rst | 35 +++ frontend/src/api/auth.js | 12 + frontend/src/plugins/vee-validate.js | 6 +- frontend/src/router/index.js | 15 ++ frontend/src/views/Login.vue | 5 +- .../views/user/PasswordRecoveryChangeForm.vue | 139 ++++++++++ .../src/views/user/PasswordRecoveryForm.vue | 130 ++++++++++ .../user/PasswordRecoverySmsTotpForm.vue | 127 +++++++++ modoboa/core/api/v2/serializers.py | 242 +++++++++++++++++- modoboa/core/api/v2/tests.py | 45 ++++ modoboa/core/api/v2/urls.py | 10 +- modoboa/core/api/v2/views.py | 87 +++++++ .../core/commands/templates/settings.py.tpl | 4 + modoboa/core/context_processors.py | 8 + modoboa/core/templates/core/dashboard.html | 2 +- modoboa/templates/nlayout.html | 2 +- .../registration/password_reset_email_v2.html | 17 ++ test_project/test_project/settings.py | 4 + 19 files changed, 892 insertions(+), 13 deletions(-) create mode 100644 frontend/src/views/user/PasswordRecoveryChangeForm.vue create mode 100644 frontend/src/views/user/PasswordRecoveryForm.vue create mode 100644 frontend/src/views/user/PasswordRecoverySmsTotpForm.vue create mode 100644 modoboa/templates/registration/password_reset_email_v2.html diff --git a/doc/configuration.rst b/doc/configuration.rst index e41a5caf7..c21709d0e 100644 --- a/doc/configuration.rst +++ b/doc/configuration.rst @@ -183,6 +183,8 @@ To work properly, the system user which runs modoboa (``www-data``, ``apache``, whatever) must have write access to this directory. +.. _customization: + ************* Customization ************* @@ -202,6 +204,19 @@ relative URL of your logo under ``MEDIA_URL``. For example:: Then copy your logo file into the directory indicated by ``MEDIA_ROOT``. +Custom new admin sub-url +======================== + +You can edit the new admin sub-url (by default it is `/new-admin`). + +To do so, open the :file:`settings.py` file and add (or uncomment) +``NEW_ADMIN_URL`` variable. This variable must contain a relative url +without initial and last `'/'`. For example:: + + NEW_ADMIN_URL = 'test_admin' + +You will need to change you web server accordingly and rebuild the frontend. + ****************** Host configuration ****************** diff --git a/doc/upgrade.rst b/doc/upgrade.rst index 403042fca..a667c014b 100644 --- a/doc/upgrade.rst +++ b/doc/upgrade.rst @@ -91,6 +91,41 @@ Rebuild Virtual Environment Specific instructions ********************* +2.0.4 +===== + +* Add ``'modoboa.core.context_processors.new_admin_url',`` at the end of `context_processors` list: + +.. sourcecode:: python + + TEMPLATES = [ + { + 'BACKEND': 'django.template.backends.django.DjangoTemplates', + 'DIRS': [], + 'APP_DIRS': True, + 'OPTIONS': { + 'context_processors': [ + 'django.template.context_processors.debug', + 'django.template.context_processors.request', + 'django.contrib.auth.context_processors.auth', + 'django.template.context_processors.i18n', + 'django.template.context_processors.media', + 'django.template.context_processors.static', + 'django.template.context_processors.tz', + 'django.contrib.messages.context_processors.messages', + 'modoboa.core.context_processors.top_notifications', + 'modoboa.core.context_processors.new_admin_url', + ], + 'debug': False, + }, + }, + ] + + +* You now have the possibility to customize the url of the new-admin interface. +To do so please head up to :ref:`the custom configuration chapter ` (advanced user). + + 2.0.3 ===== diff --git a/frontend/src/api/auth.js b/frontend/src/api/auth.js index 0fec62969..d976c71eb 100644 --- a/frontend/src/api/auth.js +++ b/frontend/src/api/auth.js @@ -3,5 +3,17 @@ import repository from './repository' export default { requestToken (data) { return repository.post('/token/', data) + }, + recoverPassword (data) { + return repository.post('/password_reset/', data) + }, + checkSmsTotp (data) { + return repository.post('/confirm_sms_totp/', data) + }, + changePassword (data) { + return repository.post('/reset_confirm/', data) + }, + resendSmsTotp (data) { + return repository.post('/reset_sms_resend/', data) } } diff --git a/frontend/src/plugins/vee-validate.js b/frontend/src/plugins/vee-validate.js index 0a0a68e61..df8416106 100644 --- a/frontend/src/plugins/vee-validate.js +++ b/frontend/src/plugins/vee-validate.js @@ -1,6 +1,6 @@ import Vue from 'vue' import { ValidationObserver, ValidationProvider, extend } from 'vee-validate' -import { confirmed, numeric, required } from 'vee-validate/dist/rules' +import { confirmed, email, numeric, required } from 'vee-validate/dist/rules' import { translate } from 'vue-gettext' import account from '@/api/account' @@ -19,6 +19,10 @@ extend('required', { ...required, message: () => $gettext('This field is required') }) +extend('email', { + ...email, + message: () => $gettext('Specify an email address') +}) // Add custom rules here extend('validPassword', { diff --git a/frontend/src/router/index.js b/frontend/src/router/index.js index 6b53339c5..2f0c51e2e 100644 --- a/frontend/src/router/index.js +++ b/frontend/src/router/index.js @@ -172,6 +172,21 @@ const routes = [ requiresAuth: true, layout: 'user' } + }, + { + path: '/password_recovery', + name: 'PasswordRecoveryForm', + component: () => import('../views/user/PasswordRecoveryForm.vue') + }, + { + path: '/password_recovery/confirm/:id?/:token?/', + name: 'PasswordRecoveryChangeForm', + component: () => import('../views/user/PasswordRecoveryChangeForm.vue') + }, + { + path: '/password_recovery/sms_confirm', + name: 'PasswordRecoverySms', + component: () => import('../views/user/PasswordRecoverySmsTotpForm.vue') } ] diff --git a/frontend/src/views/Login.vue b/frontend/src/views/Login.vue index 45b5c7ad4..115f4a1e4 100644 --- a/frontend/src/views/Login.vue +++ b/frontend/src/views/Login.vue @@ -34,7 +34,7 @@ outlined /> - Forgot password? + Forgot password? + + + + + + Forgot password? + + +

+ + + + + + + +
+ + Submit + +
+ + + + + + + + diff --git a/frontend/src/views/user/PasswordRecoveryForm.vue b/frontend/src/views/user/PasswordRecoveryForm.vue new file mode 100644 index 000000000..1e79a561a --- /dev/null +++ b/frontend/src/views/user/PasswordRecoveryForm.vue @@ -0,0 +1,130 @@ + + + + + diff --git a/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue b/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue new file mode 100644 index 000000000..5c0211184 --- /dev/null +++ b/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue @@ -0,0 +1,127 @@ + + + + + diff --git a/modoboa/core/api/v2/serializers.py b/modoboa/core/api/v2/serializers.py index 470defbf2..1ce01f2ce 100644 --- a/modoboa/core/api/v2/serializers.py +++ b/modoboa/core/api/v2/serializers.py @@ -1,19 +1,56 @@ """Core API v2 serializers.""" +import django_otp +import oath -from django.utils import formats -from django.utils.translation import ugettext_lazy, ugettext as _ +from django.db.models import Q from django.contrib.auth import password_validation +from django.contrib.sites.shortcuts import get_current_site +from django.contrib.auth.tokens import default_token_generator +from django.core.mail import send_mail +from django.core.validators import validate_email +from django.core.exceptions import ValidationError as djangoValidationError + +from django.template import loader + +from django.utils import formats +from django.utils.http import urlsafe_base64_encode, urlsafe_base64_decode +from django.utils.encoding import force_bytes +from django.utils.translation import ugettext_lazy, ugettext as _ -import django_otp from rest_framework import serializers -from modoboa.lib import fields as lib_fields +from modoboa.core import constants, models, sms_backends, app_settings, context_processors +from modoboa.core.models import User +from modoboa.lib import fields as lib_fields, cryptutils -from ... import constants -from ... import models -from ... import sms_backends -from ... import app_settings + +class NoSMSAvailable(Exception): + """ Raised when no sms totp is availible for password reset (to try email).""" + pass + + +class UserNotFound(Exception): + """ Raised when no valid user has been found (to have a proper 404 http code instead of 400.""" + pass + + +class EmailSendingFailure(Exception): + """ Raised when a reset email has failed to send.""" + pass + + +class PasswordRequirementsFailure(Exception): + """ Raised when django is not happy with password provided.""" + + def __init__(self, message_list, *args: object) -> None: + super().__init__(*args) + self.message_list = message_list + + +class InvalidToken(Exception): + """ Raised during password reset if token is not valid.""" + pass class CoreGlobalParametersSerializer(serializers.Serializer): @@ -242,3 +279,192 @@ class UserAPITokenSerializer(serializers.Serializer): """Serializer used by API access routes.""" token = serializers.CharField() + + +class EmailPasswordRecoveryInitSerializer(serializers.Serializer): + """Serializer used by password recovery route.""" + + email = serializers.EmailField() + + def validate(self, data): + clean_email = data["email"] + self.context["user"] = (User.objects.filter( + email__iexact=clean_email, is_active=True) + .exclude(Q(secondary_email__isnull=True) | Q(secondary_email="")) + ).first() + if self.context["user"] is None: + raise UserNotFound() + return data + + def save(self): + user = self.context["user"] + to_email = user.secondary_email + current_site = get_current_site(self.context["request"]) + site_name = current_site.name + domain = current_site.domain + context = { + 'email': to_email, + 'domain': domain, + 'site_name': site_name, + 'uid': urlsafe_base64_encode(force_bytes(user.pk)), + 'user': user, + 'token': default_token_generator.make_token(user), + 'protocol': 'https', + } + context.update(context_processors.new_admin_url()) + subject = loader.render_to_string( + "registration/password_reset_subject.txt", context) + # Email subject *must not* contain newlines + subject = ''.join(subject.splitlines()) + body = loader.render_to_string( + "registration/password_reset_email_v2.html", context) + try: + if send_mail(subject, body, None, [to_email]) == 0: + raise EmailSendingFailure() + except: + raise EmailSendingFailure() + + +class SMSPasswordRecoveryInitSerializer(serializers.Serializer): + + email = serializers.EmailField() + + def validate(self, data, *args, **kwargs): + request = self.context["request"] + clean_email = data["email"] + + self.context["user"] = (User.objects.filter( + email__iexact=clean_email, is_active=True) + .exclude(Q(phone_number__isnull=True) | Q(phone_number="")) + ).first() + if self.context["user"] is None: + raise NoSMSAvailable() + + if not request.localconfig.parameters.get_value("sms_password_recovery"): + raise NoSMSAvailable() + + user = self.context["user"] + if not user: + raise NoSMSAvailable() + backend = sms_backends.get_active_backend( + request.localconfig.parameters) + secret = cryptutils.random_hex_key(20) + code = oath.totp(secret) + text = _( + "Please use the following code to recover your Modoboa password: {}" + .format(code) + ) + if not backend.send(text, [str(user.phone_number)]): + raise NoSMSAvailable() + request.session["totp_secret"] = secret + request.session["user_pk"] = user.pk + return data + + +class PasswordRecoverySmsSerializer(serializers.Serializer): + + sms_totp = serializers.CharField(min_lenght=6, max_length=6) + + def validate(self, data): + try: + totp_secret = self.context["request"].session["totp_secret"] + self.context["request"].session.pop("totp_secret") + except KeyError: + raise serializers.ValidationError( + "totp secret not set in session", 403) + + if not oath.accept_totp(totp_secret, data["sms_totp"]): + raise serializers.ValidationError("Wrong totp", 403) + + # Attempt to get user, will raise an error if pk is not valid + self.user = User.objects.get( + pk=self.context["user_pk"]) + + return data + + def save(self): + self.context["request"].session.pop("totp_secret") + self.context["request"].session.pop("user") + token = default_token_generator.make_token(self.user) + uid = urlsafe_base64_encode(force_bytes(self.user.pk)) + self.context["response"] = (token, uid) + return True + + +class PasswordRecoveryConfirmSerializer(serializers.Serializer): + + new_password1 = serializers.CharField() + new_password2 = serializers.CharField() + + token = serializers.CharField() + + id = serializers.CharField() + + def get_user(self, user_id): + try: + # urlsafe_base64_decode() decodes to bytestring + uid = urlsafe_base64_decode(user_id).decode() + user = User.objects.get(pk=uid) + except (TypeError, ValueError, OverflowError, User.DoesNotExist): + user = None + return user + + def validate(self, data): + + # Validate password + if data["new_password1"] == "" or data["new_password1"] != data["new_password2"]: + raise serializers.ValidationError( + "Password is empty or does not match") + + user = self.get_user(data["id"]) + + if user is None: + raise UserNotFound + + if not default_token_generator.check_token(user, data["token"]): + raise InvalidToken + + # Check that the password works with set conditions + try: + password_validation.validate_password(data["new_password1"], user) + except djangoValidationError as e: + raise PasswordRequirementsFailure(e.error_list) + + self.context["user"] = user + return super().validate(data) + + def save(self): + user = self.context["user"] + user.set_password(self.validated_data["new_password1"]) + user.save() + return True + + +class PasswordRecoverySmsResendSerializer(serializers.Serializer): + + def validate(self, data): + + try: + self.context["session"].user_pk + self.context["session"].totp_token + return data + except KeyError: + raise serializers.ValidationError( + "No previous reset attempt recorded", 403) + + def save(self): + user = User.objects.get( + pk=self.context["session"].user_pk) + request = self.context["request"] + backend = sms_backends.get_active_backend( + request.localconfig.parameters) + secret = cryptutils.random_hex_key(20) + code = oath.totp(secret) + text = _( + "Please use the following code to recover your Modoboa password: {}" + .format(code) + ) + if not backend.send(text, [str(user.phone_number)]): + return False + request.session["totp_secret"] = secret + request.session["user_pk"] = user.pk diff --git a/modoboa/core/api/v2/tests.py b/modoboa/core/api/v2/tests.py index 84d45be94..5d1b2a770 100644 --- a/modoboa/core/api/v2/tests.py +++ b/modoboa/core/api/v2/tests.py @@ -5,8 +5,10 @@ from unittest import mock from django.urls import reverse +from modoboa.admin import factories from modoboa.core import models from modoboa.lib.tests import ModoAPITestCase +from rest_framework.authtoken.models import Token CORE_SETTINGS = { "authentication_type": "local", @@ -112,6 +114,14 @@ class ParametersAPITestCase(ModoAPITestCase): class AccountViewSetTestCase(ModoAPITestCase): + @classmethod + def setUpTestData(cls): # NOQA:N802 + """Create test data.""" + super().setUpTestData() + factories.populate_database() + cls.da_token = Token.objects.create( + user=models.User.objects.get(username="admin@test.com")) + def test_me(self): url = reverse("v2:account-me") resp = self.client.get(url) @@ -199,6 +209,41 @@ class AccountViewSetTestCase(ModoAPITestCase): resp = self.client.post(url) self.assertEqual(resp.status_code, 201) + @mock.patch("ovh.Client.get") + @mock.patch("ovh.Client.post") + def test_reset_password(self, client_post, client_get): + url = reverse("v2:password_reset") + # No payload provided + response = self.client.post(url) + self.assertEqual(response.status_code, 400) + + # SMS parameter + self.set_global_parameters({ + "sms_password_recovery": True, + "sms_provider": "ovh", + "sms_ovh_application_key": "key", + "sms_ovh_application_secret": "secret", + "sms_ovh_consumer_key": "consumer" + }, app="core") + account = models.User.objects.get(username="user@test.com") + data = {"email": account.email} + # Provide a user w/o phone or secondary + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 404) + # Secondary email for reset + account.secondary_email = "toto@test.com" + account.is_active = True + account.save() + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 204) + # Phone number + account.phone_number = "+33612345678" + account.save() + client_get.return_value = ["service"] + client_post.return_value = {"totalCreditsRemoved": 1} + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 233) + class AuthenticationTestCase(ModoAPITestCase): diff --git a/modoboa/core/api/v2/urls.py b/modoboa/core/api/v2/urls.py index 5d45049a6..7f66e172f 100644 --- a/modoboa/core/api/v2/urls.py +++ b/modoboa/core/api/v2/urls.py @@ -21,5 +21,13 @@ urlpatterns += [ path('token/', views.TokenObtainPairView.as_view(), name="token_obtain_pair"), path('token/refresh/', TokenRefreshView.as_view(), - name='token_refresh'), + name="token_refresh"), + path('password_reset/', views.DefaultPasswordResetView.as_view(), + name="password_reset"), + path('reset_confirm/', views.PasswordResetConfirmView.as_view(), + name="password_reset_confirm_v2"), + path('confirm_sms_totp/', views.PasswordResetConfirmSmsCodeView.as_view(), + name="confirm_sms_totp"), + path('reset_sms_resend', views.PasswordResetResendSmsCodeView.as_view(), + name='reset_sms_resend'), ] diff --git a/modoboa/core/api/v2/views.py b/modoboa/core/api/v2/views.py index e78d71d22..cdc5dbb47 100644 --- a/modoboa/core/api/v2/views.py +++ b/modoboa/core/api/v2/views.py @@ -10,10 +10,13 @@ from django.contrib.auth import login from rest_framework import response, status from rest_framework_simplejwt import views as jwt_views from rest_framework_simplejwt.exceptions import InvalidToken, TokenError +from rest_framework.views import APIView from modoboa.core.password_hashers import get_password_hasher from modoboa.parameters import tools as param_tools +from .serializers import * + logger = logging.getLogger("modoboa.auth") @@ -70,3 +73,87 @@ class TokenObtainPairView(jwt_views.TokenObtainPairView): return response.Response( serializer.validated_data, status=status.HTTP_200_OK) + + +class EmailPasswordResetView(APIView): + """ + An Api View which provides a method to request a password reset token based on an e-mail address. + """ + + def post(self, request, *args, **kwargs): + serializer = EmailPasswordRecoveryInitSerializer( + data=request.data, context={'request': request}) + try: + serializer.is_valid(raise_exception=True) + except UserNotFound: + return response.Response({"no valid user found"}, 404) + try: + serializer.save() + except EmailSendingFailure: + return response.Response({"Email failed to send, possibly a misconfiguration?"}, 502) + # Email response + return response.Response(status=204) + + +class DefaultPasswordResetView(EmailPasswordResetView): + """ + Works with PasswordRecoveryForm.vue. + First checks if SMS recovery is available, else switch to super (Email recovery [with seconday email]). + """ + + def post(self, request, *args, **kwargs): + """Recover password.""" + serializer = SMSPasswordRecoveryInitSerializer( + data=request.data, context={'request': request}) + try: + serializer.is_valid(raise_exception=True) + except NoSMSAvailable: + return super().post(request, *args, **kwargs) + # SMS response + return response.Response(status=233) + + +class PasswordResetConfirmSmsCodeView(APIView): + """ Check SMS Totp code. """ + + def post(self, request, *args, **kwargs): + serializer = PasswordRecoverySmsSerializer(data=request.data) + serializer.is_valid(raise_exception=True) + serializer.save() + + payload = serializer.context["response"] + payload = {"token": payload[0], "id": payload[1]} + + return response.Response(payload, 200) + + +class PasswordResetConfirmView(APIView): + """ Get and set new user password. """ + + def post(self, request, *args, **kwargs): + serializer = PasswordRecoveryConfirmSerializer(data=request.data) + try: + serializer.is_valid(raise_exception=True) + except (PasswordRequirementsFailure, UserNotFound, InvalidToken) as e: + if type(e) == PasswordRequirementsFailure: + errors = [] + for element in e.message_list: + errors.append(element) + return response.Response(errors, 455) + elif type(e) == UserNotFound: + return response.Response(status=404) + elif type(e) == InvalidToken: + return response.Response(status=401) + serializer.save() + return response.Response(status=200) + + +class PasswordResetResendSmsCodeView(APIView): + + def post(self, request, *args, **kwargs): + serializer = PasswordRecoverySmsResendSerializer( + data=request.data, context={"request": request}) + + serializer.is_valid(raise_exception=True) + serializer.save() + return response.Response(status=200) diff --git a/modoboa/core/commands/templates/settings.py.tpl b/modoboa/core/commands/templates/settings.py.tpl index bdbde5683..03f4a7ce2 100644 --- a/modoboa/core/commands/templates/settings.py.tpl +++ b/modoboa/core/commands/templates/settings.py.tpl @@ -141,6 +141,7 @@ TEMPLATES = [ 'django.template.context_processors.tz', 'django.contrib.messages.context_processors.messages', 'modoboa.core.context_processors.top_notifications', + 'modoboa.core.context_processors.new_admin_url', ], 'debug': {{ devmode }}, }, @@ -202,6 +203,9 @@ REST_FRAMEWORK = { } +# Uncomment if you need a custom sub url for new_admin interface +# NEW_ADMIN_URL = 'new_admin' + SPECTACULAR_SETTINGS = { 'SCHEMA_PATH_PREFIX': r'/api/v[0-9]', 'TITLE': 'Modoboa API', diff --git a/modoboa/core/context_processors.py b/modoboa/core/context_processors.py index 3bd8a3c92..2f9126ec4 100644 --- a/modoboa/core/context_processors.py +++ b/modoboa/core/context_processors.py @@ -1,6 +1,7 @@ """Custom context processors.""" from functools import reduce +from django.conf import settings from . import signals @@ -19,3 +20,10 @@ def top_notifications(request): "notifications_check_interval": interval * 1000, "top_notifications": notifications } + + +def new_admin_url(*args): + """A context processor to include new admin url.""" + return { + "new_admin": getattr(settings, "NEW_ADMIN_URL", "new_admin") + } diff --git a/modoboa/core/templates/core/dashboard.html b/modoboa/core/templates/core/dashboard.html index ae96a0ad6..d22e36ea2 100644 --- a/modoboa/core/templates/core/dashboard.html +++ b/modoboa/core/templates/core/dashboard.html @@ -8,7 +8,7 @@ {% if user.is_admin %}
Want to check the new admin interface? - Go to Modoboa v2 + Go to Modoboa v2
{% endif %} diff --git a/modoboa/templates/nlayout.html b/modoboa/templates/nlayout.html index 8a219c595..0d851d75d 100644 --- a/modoboa/templates/nlayout.html +++ b/modoboa/templates/nlayout.html @@ -53,7 +53,7 @@
  • {% if user.is_admin %}
  • - + {% trans "New admin" %}
    • diff --git a/modoboa/templates/registration/password_reset_email_v2.html b/modoboa/templates/registration/password_reset_email_v2.html new file mode 100644 index 000000000..db6213367 --- /dev/null +++ b/modoboa/templates/registration/password_reset_email_v2.html @@ -0,0 +1,17 @@ +{% load i18n %} + +{% autoescape off %} + {% blocktrans with username=user.get_username trimmed %} + To initiate the password reset process for your {{ username }} Modoboa account, click the link below: + {% endblocktrans %} + + {{ protocol }}://{{ domain }}/{{ new_admin }}/password_recovery/confirm/{{ uid }}/{{ token }} + + {% blocktrans trimmed %} + If clicking the link above doesn't work, please copy and paste the URL in a new browser window instead. + {% endblocktrans %} + + {% blocktrans trimmed %} + Sincerely, The Modoboa Team. + {% endblocktrans %} +{% endautoescape %} diff --git a/test_project/test_project/settings.py b/test_project/test_project/settings.py index e701cb6fb..da414290e 100644 --- a/test_project/test_project/settings.py +++ b/test_project/test_project/settings.py @@ -147,6 +147,7 @@ TEMPLATES = [ 'django.template.context_processors.tz', 'django.contrib.messages.context_processors.messages', 'modoboa.core.context_processors.top_notifications', + 'modoboa.core.context_processors.new_admin_url' ], 'debug': DEBUG, }, @@ -194,6 +195,9 @@ REST_FRAMEWORK = { 'DEFAULT_VERSIONING_CLASS': 'rest_framework.versioning.NamespaceVersioning', } +# Uncomment if you need a custom sub url for new_admin interface +# NEW_ADMIN_URL = 'new_admin' + SPECTACULAR_SETTINGS = { 'SCHEMA_PATH_PREFIX': r'/api/v[0-9]', 'TITLE': 'Modoboa API', From 9ad9bf9ce4940deef0199e814766845ebbae8251 Mon Sep 17 00:00:00 2001 From: Spitap Date: Mon, 5 Dec 2022 17:41:41 +0100 Subject: [PATCH 2/8] fix spelling --- modoboa/core/api/v2/serializers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modoboa/core/api/v2/serializers.py b/modoboa/core/api/v2/serializers.py index 1ce01f2ce..14b17a336 100644 --- a/modoboa/core/api/v2/serializers.py +++ b/modoboa/core/api/v2/serializers.py @@ -363,7 +363,7 @@ class SMSPasswordRecoveryInitSerializer(serializers.Serializer): class PasswordRecoverySmsSerializer(serializers.Serializer): - sms_totp = serializers.CharField(min_lenght=6, max_length=6) + sms_totp = serializers.CharField(min_length=6, max_length=6) def validate(self, data): try: From 01461751531e950936e1891156d1ffe0236caee4 Mon Sep 17 00:00:00 2001 From: Spitap Date: Wed, 28 Dec 2022 18:42:58 +0100 Subject: [PATCH 3/8] Refactoring, better tests --- frontend/src/api/auth.js | 5 +- .../views/user/PasswordRecoveryChangeForm.vue | 12 +- .../src/views/user/PasswordRecoveryForm.vue | 49 +++--- .../user/PasswordRecoverySmsTotpForm.vue | 28 ++-- modoboa/core/api/v2/serializers.py | 147 +++++++++--------- modoboa/core/api/v2/tests.py | 125 ++++++++++++--- modoboa/core/api/v2/urls.py | 6 +- modoboa/core/api/v2/views.py | 73 ++++----- modoboa/lib/tests/__init__.py | 10 ++ 9 files changed, 270 insertions(+), 185 deletions(-) diff --git a/frontend/src/api/auth.js b/frontend/src/api/auth.js index d976c71eb..f496844fd 100644 --- a/frontend/src/api/auth.js +++ b/frontend/src/api/auth.js @@ -8,12 +8,9 @@ export default { return repository.post('/password_reset/', data) }, checkSmsTotp (data) { - return repository.post('/confirm_sms_totp/', data) + return repository.post('/sms_totp/', data) }, changePassword (data) { return repository.post('/reset_confirm/', data) - }, - resendSmsTotp (data) { - return repository.post('/reset_sms_resend/', data) } } diff --git a/frontend/src/views/user/PasswordRecoveryChangeForm.vue b/frontend/src/views/user/PasswordRecoveryChangeForm.vue index 7d0f0f1bc..47b557c70 100644 --- a/frontend/src/views/user/PasswordRecoveryChangeForm.vue +++ b/frontend/src/views/user/PasswordRecoveryChangeForm.vue @@ -104,11 +104,13 @@ export default { this.loading = true auth.changePassword(payload).then(resp => { document.getElementById('password_validation').innerHTML = '' - this.loading = false - this.returnLogin() + if (resp.status === 200) { + this.loading = false + this.returnLogin() + } }).catch(err => { this.loading = false - if (err.response.status === 401) { + if (err.response.status === 403) { this.$refs.observer.setErrors({ password_confirmed: this.$gettext('Invalid reset token.') }) @@ -116,9 +118,9 @@ export default { this.$refs.observer.setErrors({ password_confirmed: this.$gettext('User unknown.') }) - } else if (err.response.status === 455) { + } else if (err.response.status === 400 && err.response.data.type === 'password_requirement') { let message = '' - err.response.data.forEach(element => { + err.response.data.errors.forEach(element => { message += this.$gettext(element) + '
    ' }) document.getElementById('password_validation').innerHTML = message diff --git a/frontend/src/views/user/PasswordRecoveryForm.vue b/frontend/src/views/user/PasswordRecoveryForm.vue index 1e79a561a..2aecb0607 100644 --- a/frontend/src/views/user/PasswordRecoveryForm.vue +++ b/frontend/src/views/user/PasswordRecoveryForm.vue @@ -62,30 +62,18 @@ export default { returnLogin () { this.$router.push({ name: 'Login' }) }, - async showDialogEmail () { - const confirm = await this.$refs.dialog_response.open( - this.$gettext('Info'), - this.$gettext( - 'Email sent, please check your inbox' - ), - { - color: 'info', - cancelLabel: this.$gettext('Ok'), - agreeLabel: this.$gettext('Return to login') - }) - if (!confirm) { - return + async showDialog (title, body, error = false) { + let color = 'info' + if (error) { + color = 'error' } - this.returnLogin() - }, - async emailFailedToSend () { const confirm = await this.$refs.dialog_response.open( - this.$gettext('Error'), + this.$gettext(title), this.$gettext( - 'Failed to send email, please check with you administrator' + body ), { - color: 'error', + color: color, cancelLabel: this.$gettext('Ok'), agreeLabel: this.$gettext('Return to login') }) @@ -105,20 +93,23 @@ export default { this.loading = true auth.recoverPassword(payload).then(resp => { this.loading = false - if (resp.status === 233) { - this.$router.push({ name: 'PasswordRecoverySmsTotpForm' }) - } else if (resp.status === 210) { - this.showDialogEmail() + if (resp.status === 200) { + if (resp.data.type === 'sms') { + this.$router.push({ name: 'PasswordRecoverySmsTotpForm' }) + } else if (resp.data.type === 'email') { + this.showDialog('Info', 'Email sent, please check your inbox') + } } }).catch(err => { this.loading = false if (err.response.status === 404) { - // User not found - this.$refs.observer.setErrors({ - email: this.$gettext('Invalid email') - }) - } else if (err.response.status === 502) { - this.emailFailedToSend() + if (err.response.data.type === 'sms' || err.response.data.type === 'email') { + this.$refs.observer.setErrors({ + email: this.$gettext(err.response.data.reason) + }) + } + } else if (err.response.status === 503 && err.response.data.type === 'email') { + this.showDialog('Error', err.response.data.reason, true) } }) } diff --git a/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue b/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue index 5c0211184..23ad9f0c5 100644 --- a/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue +++ b/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue @@ -10,7 +10,7 @@ - Password recovert + Password recovery Submit @@ -69,9 +69,9 @@ export default { }, async userPkError () { const confirm = await this.$refs.confirm.open( - this.$gettext('Warning'), + this.$gettext('Error'), this.$gettext( - 'User seems wrong, return to login or restart reset process?' + 'User seems wrong, return to login or restart reset the process?' ), { color: 'error', @@ -86,13 +86,18 @@ export default { } }, async resendSms () { - auth.resendSmsTotp().then(resp => { + auth.checkSmsTotp({ type: 'resend' }).then(resp => { this.loading = false - // Reset done ! + if (resp.status === 200) { + this.$refs.observer.setErrors({ + password_confirmed: this.$gettext('TOTP resent.') + }) + } }).catch(err => { - console.error(err) - this.loading = false - this.userPkError() + if (err.response.status === 400) { + this.loading = false + this.userPkError() + } }) }, async checkSmsTotp () { @@ -101,12 +106,13 @@ export default { return } const payload = { + type: 'confirm', sms_totp: this.sms_totp } this.loading = true auth.checkSmsTotp(payload).then(resp => { this.loading = false - if (resp.response.status === 200) { + if (resp.status === 200) { // Code is good, redirecting to change password form. this.$router.push({ name: 'PasswordRecoveryChangeForm', data: resp.response.data }) } @@ -114,7 +120,7 @@ export default { this.loading = false if (err.response.status === 500) { this.$refs.observer.setErrors({ - password_confirmed: this.$gettext('Invalid totp, try resend') + password_confirmed: this.$gettext('Invalid totp.') }) } }) diff --git a/modoboa/core/api/v2/serializers.py b/modoboa/core/api/v2/serializers.py index 14b17a336..46e6d792d 100644 --- a/modoboa/core/api/v2/serializers.py +++ b/modoboa/core/api/v2/serializers.py @@ -8,35 +8,39 @@ from django.contrib.auth import password_validation from django.contrib.sites.shortcuts import get_current_site from django.contrib.auth.tokens import default_token_generator from django.core.mail import send_mail -from django.core.validators import validate_email from django.core.exceptions import ValidationError as djangoValidationError from django.template import loader from django.utils import formats +from django.utils.encoding import force_text from django.utils.http import urlsafe_base64_encode, urlsafe_base64_decode from django.utils.encoding import force_bytes from django.utils.translation import ugettext_lazy, ugettext as _ -from rest_framework import serializers +from rest_framework import serializers, status +from rest_framework.exceptions import APIException from modoboa.core import constants, models, sms_backends, app_settings, context_processors from modoboa.core.models import User from modoboa.lib import fields as lib_fields, cryptutils +class CustomValidationError(APIException): + status_code = status.HTTP_400_BAD_REQUEST + default_detail = 'A server error occurred.' + + def __init__(self, detail, status_code): + if status_code is not None: + self.status_code = status_code + if detail is not None: + self.detail = detail + else: + self.detail = {'detail': force_text(self.default_detail)} + + class NoSMSAvailable(Exception): - """ Raised when no sms totp is availible for password reset (to try email).""" - pass - - -class UserNotFound(Exception): - """ Raised when no valid user has been found (to have a proper 404 http code instead of 400.""" - pass - - -class EmailSendingFailure(Exception): - """ Raised when a reset email has failed to send.""" + """ Raised when no sms totp is available for password reset (to try email).""" pass @@ -48,11 +52,6 @@ class PasswordRequirementsFailure(Exception): self.message_list = message_list -class InvalidToken(Exception): - """ Raised during password reset if token is not valid.""" - pass - - class CoreGlobalParametersSerializer(serializers.Serializer): """A serializer for global parameters.""" @@ -281,7 +280,7 @@ class UserAPITokenSerializer(serializers.Serializer): token = serializers.CharField() -class EmailPasswordRecoveryInitSerializer(serializers.Serializer): +class PasswordRecoveryEmailSerializer(serializers.Serializer): """Serializer used by password recovery route.""" email = serializers.EmailField() @@ -293,7 +292,7 @@ class EmailPasswordRecoveryInitSerializer(serializers.Serializer): .exclude(Q(secondary_email__isnull=True) | Q(secondary_email="")) ).first() if self.context["user"] is None: - raise UserNotFound() + raise CustomValidationError({"type": "email", "reason": "No valid user found."}, 404) return data def save(self): @@ -318,14 +317,21 @@ class EmailPasswordRecoveryInitSerializer(serializers.Serializer): subject = ''.join(subject.splitlines()) body = loader.render_to_string( "registration/password_reset_email_v2.html", context) - try: - if send_mail(subject, body, None, [to_email]) == 0: - raise EmailSendingFailure() - except: - raise EmailSendingFailure() + send_mail(subject, body, None, [to_email], fail_silently=False) -class SMSPasswordRecoveryInitSerializer(serializers.Serializer): +def send_sms_code(secret, backend, user): + """ Send a sms containing a TOTP code for password reset. """ + code = oath.totp(secret) + text = _( + "Please use the following code to recover your Modoboa password: {}" + .format(code) + ) + if not backend.send(text, [str(user.phone_number)]): + raise NoSMSAvailable() + + +class PasswordRecoverySmsSerializer(serializers.Serializer): email = serializers.EmailField() @@ -333,6 +339,10 @@ class SMSPasswordRecoveryInitSerializer(serializers.Serializer): request = self.context["request"] clean_email = data["email"] + user = User.objects.filter(email__iexact=clean_email, is_active=True).first() + if user is None: + raise CustomValidationError({"type": "sms", "reason": "No valid user found."}, 404) + self.context["user"] = (User.objects.filter( email__iexact=clean_email, is_active=True) .exclude(Q(phone_number__isnull=True) | Q(phone_number="")) @@ -349,48 +359,63 @@ class SMSPasswordRecoveryInitSerializer(serializers.Serializer): backend = sms_backends.get_active_backend( request.localconfig.parameters) secret = cryptutils.random_hex_key(20) - code = oath.totp(secret) - text = _( - "Please use the following code to recover your Modoboa password: {}" - .format(code) - ) - if not backend.send(text, [str(user.phone_number)]): - raise NoSMSAvailable() + send_sms_code(secret, backend, user) request.session["totp_secret"] = secret request.session["user_pk"] = user.pk return data -class PasswordRecoverySmsSerializer(serializers.Serializer): +class PasswordRecoverySmsConfirmSerializer(serializers.Serializer): sms_totp = serializers.CharField(min_length=6, max_length=6) def validate(self, data): try: - totp_secret = self.context["request"].session["totp_secret"] - self.context["request"].session.pop("totp_secret") + self.context["totp_secret"] = self.context["request"].session["totp_secret"] except KeyError: - raise serializers.ValidationError( - "totp secret not set in session", 403) + raise CustomValidationError( + {"reason": "totp secret not set in session"}, 403) - if not oath.accept_totp(totp_secret, data["sms_totp"]): - raise serializers.ValidationError("Wrong totp", 403) + if not oath.accept_totp(self.context["totp_secret"], data["sms_totp"])[0]: + raise CustomValidationError({"reason": "Wrong totp"}, 403) # Attempt to get user, will raise an error if pk is not valid - self.user = User.objects.get( - pk=self.context["user_pk"]) + self.context["user"] = User.objects.get( + pk=self.context["request"].session["user_pk"]) return data def save(self): self.context["request"].session.pop("totp_secret") - self.context["request"].session.pop("user") - token = default_token_generator.make_token(self.user) - uid = urlsafe_base64_encode(force_bytes(self.user.pk)) + user_pk = self.context["request"].session.pop("user_pk") + token = default_token_generator.make_token(self.context["user"]) + uid = urlsafe_base64_encode(force_bytes(user_pk)) self.context["response"] = (token, uid) return True +class PasswordRecoverySmsResendSerializer(serializers.Serializer): + + def validate(self, data): + try: + self.context["totp_secret"] = self.context["request"].session["totp_secret"] + except KeyError: + raise CustomValidationError( + {"reason": "totp secret not set in session"}, status.HTTP_403_FORBIDDEN) + + # Attempt to get user, will raise an error if pk is not valid + self.context["user"] = User.objects.get( + pk=self.context["request"].session["user_pk"]) + + return data + + def save(self): + backend = sms_backends.get_active_backend( + self.context["request"].localconfig.parameters) + send_sms_code(self.context["totp_secret"], backend, self.context["user"]) + return True + + class PasswordRecoveryConfirmSerializer(serializers.Serializer): new_password1 = serializers.CharField() @@ -419,10 +444,10 @@ class PasswordRecoveryConfirmSerializer(serializers.Serializer): user = self.get_user(data["id"]) if user is None: - raise UserNotFound + raise CustomValidationError({"reason": "User not found."}, 404) if not default_token_generator.check_token(user, data["token"]): - raise InvalidToken + raise CustomValidationError({"reason": "Wrong reset token"}, 403) # Check that the password works with set conditions try: @@ -440,31 +465,3 @@ class PasswordRecoveryConfirmSerializer(serializers.Serializer): return True -class PasswordRecoverySmsResendSerializer(serializers.Serializer): - - def validate(self, data): - - try: - self.context["session"].user_pk - self.context["session"].totp_token - return data - except KeyError: - raise serializers.ValidationError( - "No previous reset attempt recorded", 403) - - def save(self): - user = User.objects.get( - pk=self.context["session"].user_pk) - request = self.context["request"] - backend = sms_backends.get_active_backend( - request.localconfig.parameters) - secret = cryptutils.random_hex_key(20) - code = oath.totp(secret) - text = _( - "Please use the following code to recover your Modoboa password: {}" - .format(code) - ) - if not backend.send(text, [str(user.phone_number)]): - return False - request.session["totp_secret"] = secret - request.session["user_pk"] = user.pk diff --git a/modoboa/core/api/v2/tests.py b/modoboa/core/api/v2/tests.py index 5d1b2a770..138731d31 100644 --- a/modoboa/core/api/v2/tests.py +++ b/modoboa/core/api/v2/tests.py @@ -1,6 +1,7 @@ """Core API related tests.""" import copy +import oath from unittest import mock from django.urls import reverse @@ -114,14 +115,6 @@ class ParametersAPITestCase(ModoAPITestCase): class AccountViewSetTestCase(ModoAPITestCase): - @classmethod - def setUpTestData(cls): # NOQA:N802 - """Create test data.""" - super().setUpTestData() - factories.populate_database() - cls.da_token = Token.objects.create( - user=models.User.objects.get(username="admin@test.com")) - def test_me(self): url = reverse("v2:account-me") resp = self.client.get(url) @@ -209,6 +202,21 @@ class AccountViewSetTestCase(ModoAPITestCase): resp = self.client.post(url) self.assertEqual(resp.status_code, 201) + +class PasswordResetTestCase(ModoAPITestCase): + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.id = 0 + self.sms_token = 0 + + @classmethod + def setUpTestData(cls): # NOQA:N802 + """Create test data.""" + super().setUpTestData() + factories.populate_database() + cls.da_token = Token.objects.create( + user=models.User.objects.get(username="admin@test.com")) + @mock.patch("ovh.Client.get") @mock.patch("ovh.Client.post") def test_reset_password(self, client_post, client_get): @@ -217,7 +225,29 @@ class AccountViewSetTestCase(ModoAPITestCase): response = self.client.post(url) self.assertEqual(response.status_code, 400) - # SMS parameter + # Invalid user + data = {"email": "doesntexist@whoami.com"} + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 404) + self.assertEqual(response.json()["type"], "sms") + + account = models.User.objects.get(username="user@test.com") + + # Email reset test + # No secondary email + data = {"email": account.email} + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 404) + self.assertEqual(response.json()["type"], "email") + # With secondary email + account.secondary_email = "toto@test.com" + account.is_active = True + account.save() + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["type"], "email") + + # SMS reset test self.set_global_parameters({ "sms_password_recovery": True, "sms_provider": "ovh", @@ -225,24 +255,77 @@ class AccountViewSetTestCase(ModoAPITestCase): "sms_ovh_application_secret": "secret", "sms_ovh_consumer_key": "consumer" }, app="core") - account = models.User.objects.get(username="user@test.com") - data = {"email": account.email} - # Provide a user w/o phone or secondary - response = self.client.post(url, data, format="json") - self.assertEqual(response.status_code, 404) - # Secondary email for reset - account.secondary_email = "toto@test.com" - account.is_active = True - account.save() - response = self.client.post(url, data, format="json") - self.assertEqual(response.status_code, 204) + # Phone number account.phone_number = "+33612345678" account.save() client_get.return_value = ["service"] client_post.return_value = {"totalCreditsRemoved": 1} response = self.client.post(url, data, format="json") - self.assertEqual(response.status_code, 233) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["type"], "sms") + + @mock.patch("ovh.Client.get") + @mock.patch("ovh.Client.post") + def test_password_reset_sms_totp(self, client_post, client_get): + """Test reset password by SMS.""" + url_create_sms = reverse("v2:password_reset") + url_sms_totp = reverse("v2:sms_totp") + + # Prepare account + account = models.User.objects.get(username="user@test.com") + account.phone_number = "+33612345678" + account.secondary_email = "toto@test.com" + account.is_active = True + account.save() + + # Send sms + client_get.return_value = ["service"] + client_post.return_value = {"totalCreditsRemoved": 1} + self.set_global_parameters({ + "sms_password_recovery": True, + "sms_provider": "ovh", + "sms_ovh_application_key": "key", + "sms_ovh_application_secret": "secret", + "sms_ovh_consumer_key": "consumer" + }) + self.client.logout() + self.create_session() + data = {"email": account.email} + response = self.client.post(url_create_sms, data, format="json") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["type"], "sms") + self.assertIn("totp_secret", self.client.session) + + # Get the secret + session = self.client.session + secret = session["totp_secret"] + + # Fail to provide type + data = {"sms_totp": "123456"} + response = self.client.post(url_sms_totp, data, format="json") + self.assertEqual(response.status_code, 400) + + # Resend sms + data = {"type": "resend"} + response = self.client.post(url_sms_totp, data, format="json") + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["type"], "resend") + + # False totp code + data = {"sms_totp": "123456", "type": "confirm"} + response = self.client.post(url_sms_totp, data, format="json") + self.assertEqual(response.status_code, 403) + self.assertEqual(response.json()["reason"], "Wrong totp") + + # Good totp code + data = {"sms_totp": oath.totp(secret), "type": "confirm"} + response = self.client.post(url_sms_totp, data, format="json") + self.assertEqual(response.status_code, 200) + self.assertIn("token", response.json()) + self.assertIn("id", response.json()) + self.sms_token = response.json()["token"] + self.id = response.json()["id"] class AuthenticationTestCase(ModoAPITestCase): diff --git a/modoboa/core/api/v2/urls.py b/modoboa/core/api/v2/urls.py index 7f66e172f..3435412ad 100644 --- a/modoboa/core/api/v2/urls.py +++ b/modoboa/core/api/v2/urls.py @@ -26,8 +26,6 @@ urlpatterns += [ name="password_reset"), path('reset_confirm/', views.PasswordResetConfirmView.as_view(), name="password_reset_confirm_v2"), - path('confirm_sms_totp/', views.PasswordResetConfirmSmsCodeView.as_view(), - name="confirm_sms_totp"), - path('reset_sms_resend', views.PasswordResetResendSmsCodeView.as_view(), - name='reset_sms_resend'), + path('sms_totp/', views.PasswordResetSmsTOTP.as_view(), + name="sms_totp"), ] diff --git a/modoboa/core/api/v2/views.py b/modoboa/core/api/v2/views.py index cdc5dbb47..506820552 100644 --- a/modoboa/core/api/v2/views.py +++ b/modoboa/core/api/v2/views.py @@ -4,6 +4,7 @@ import logging from django.utils.html import escape from django.utils.translation import ugettext as _ +from django.utils.datastructures import MultiValueDictKeyError from django.contrib.auth import login @@ -15,6 +16,8 @@ from rest_framework.views import APIView from modoboa.core.password_hashers import get_password_hasher from modoboa.parameters import tools as param_tools +from smtplib import SMTPException + from .serializers import * logger = logging.getLogger("modoboa.auth") @@ -81,51 +84,60 @@ class EmailPasswordResetView(APIView): """ def post(self, request, *args, **kwargs): - serializer = EmailPasswordRecoveryInitSerializer( + serializer = PasswordRecoveryEmailSerializer( data=request.data, context={'request': request}) - try: - serializer.is_valid(raise_exception=True) - except UserNotFound: - return response.Response({"no valid user found"}, 404) + serializer.is_valid(raise_exception=True) try: serializer.save() - except EmailSendingFailure: - return response.Response({"Email failed to send, possibly a misconfiguration?"}, 502) + except SMTPException: + return response.Response({ + "type": "email", + "reason": "Error while sending the email. Please contact an administrator." + }, + 503) # Email response - return response.Response(status=204) + return response.Response({"type": "email"}, 200) class DefaultPasswordResetView(EmailPasswordResetView): - """ + """ Works with PasswordRecoveryForm.vue. - First checks if SMS recovery is available, else switch to super (Email recovery [with seconday email]). + First checks if SMS recovery is available, else switch to super (Email recovery [with secondary email]). """ def post(self, request, *args, **kwargs): """Recover password.""" - serializer = SMSPasswordRecoveryInitSerializer( + serializer = PasswordRecoverySmsSerializer( data=request.data, context={'request': request}) try: serializer.is_valid(raise_exception=True) - except NoSMSAvailable: - return super().post(request, *args, **kwargs) + except NoSMSAvailable as e: + if type(e) == NoSMSAvailable: + return super().post(request, *args, **kwargs) # SMS response - return response.Response(status=233) + return response.Response({"type": "sms"}, 200) -class PasswordResetConfirmSmsCodeView(APIView): +class PasswordResetSmsTOTP(APIView): """ Check SMS Totp code. """ def post(self, request, *args, **kwargs): - serializer = PasswordRecoverySmsSerializer(data=request.data) + try: + if request.data["type"] == "confirm": + serializer = PasswordRecoverySmsConfirmSerializer(data=request.data, context={'request': request}) + elif request.data["type"] == "resend": + serializer = PasswordRecoverySmsResendSerializer(data=request.data, context={'request': request}) + except (MultiValueDictKeyError, KeyError): + return response.Response({"reason": "No type provided."}, 400) serializer.is_valid(raise_exception=True) serializer.save() - - payload = serializer.context["response"] - payload = {"token": payload[0], "id": payload[1]} - + payload = {"type": "resend"} + if request.data["type"] == "confirm": + serializer_response = serializer.context["response"] + payload.update({"token": serializer_response[0], "id": serializer_response[1], "type": "confirm"}) + return response.Response(payload, 200) - + class PasswordResetConfirmView(APIView): """ Get and set new user password. """ @@ -134,26 +146,15 @@ class PasswordResetConfirmView(APIView): serializer = PasswordRecoveryConfirmSerializer(data=request.data) try: serializer.is_valid(raise_exception=True) - except (PasswordRequirementsFailure, UserNotFound, InvalidToken) as e: + except PasswordRequirementsFailure as e: if type(e) == PasswordRequirementsFailure: + data = {"type": "password_requirement"} errors = [] for element in e.message_list: errors.append(element) - return response.Response(errors, 455) - elif type(e) == UserNotFound: - return response.Response(status=404) - elif type(e) == InvalidToken: - return response.Response(status=401) + data.update({"errors": errors}) + return response.Response(data, 400) serializer.save() return response.Response(status=200) -class PasswordResetResendSmsCodeView(APIView): - - def post(self, request, *args, **kwargs): - serializer = PasswordRecoverySmsResendSerializer( - data=request.data, context={"request": request}) - - serializer.is_valid(raise_exception=True) - serializer.save() - return response.Response(status=200) diff --git a/modoboa/lib/tests/__init__.py b/modoboa/lib/tests/__init__.py index 0441301d5..608197a41 100644 --- a/modoboa/lib/tests/__init__.py +++ b/modoboa/lib/tests/__init__.py @@ -2,9 +2,12 @@ import socket +from django.conf import settings from django.core import management from django.test import TestCase +from importlib import import_module + from rest_framework.authtoken.models import Token from rest_framework.test import APITestCase @@ -99,3 +102,10 @@ class ModoAPITestCase(ParametersMixin, APITestCase): """Setup.""" super(ModoAPITestCase, self).setUp() self.client.credentials(HTTP_AUTHORIZATION="Token " + self.token.key) + + def create_session(self): + """Enable session storage across requests.""" + session_engine = import_module(settings.SESSION_ENGINE) + store = session_engine.SessionStore() + store.save() + self.client.cookies[settings.SESSION_COOKIE_NAME] = store.session_key From cc89b23d2e97f4636d8db1f82b222bfd0c9d67ad Mon Sep 17 00:00:00 2001 From: Spitap Date: Thu, 29 Dec 2022 12:38:20 +0100 Subject: [PATCH 4/8] Increase coverage --- modoboa/core/api/v2/tests.py | 80 +++++++++++++++++++++++++++++++++--- 1 file changed, 74 insertions(+), 6 deletions(-) diff --git a/modoboa/core/api/v2/tests.py b/modoboa/core/api/v2/tests.py index 138731d31..258273176 100644 --- a/modoboa/core/api/v2/tests.py +++ b/modoboa/core/api/v2/tests.py @@ -5,6 +5,8 @@ import oath from unittest import mock from django.urls import reverse +from django.utils.encoding import force_bytes +from django.utils.http import urlsafe_base64_encode from modoboa.admin import factories from modoboa.core import models @@ -122,11 +124,11 @@ class AccountViewSetTestCase(ModoAPITestCase): me = resp.json() self.assertEqual(me["username"], "admin") - def test_me_password(self): + def test_me_password(self, password_ko="Toto1234", password_ok="password"): url = reverse("v2:account-check-me-password") - resp = self.client.post(url, {"password": "Toto1234"}, format="json") + resp = self.client.post(url, {"password": password_ko}, format="json") self.assertEqual(resp.status_code, 400) - resp = self.client.post(url, {"password": "password"}, format="json") + resp = self.client.post(url, {"password": password_ok}, format="json") self.assertEqual(resp.status_code, 200) @mock.patch("django_otp.match_token") @@ -203,7 +205,7 @@ class AccountViewSetTestCase(ModoAPITestCase): self.assertEqual(resp.status_code, 201) -class PasswordResetTestCase(ModoAPITestCase): +class PasswordResetTestCase(AccountViewSetTestCase): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) self.id = 0 @@ -324,8 +326,74 @@ class PasswordResetTestCase(ModoAPITestCase): self.assertEqual(response.status_code, 200) self.assertIn("token", response.json()) self.assertIn("id", response.json()) - self.sms_token = response.json()["token"] - self.id = response.json()["id"] + + @mock.patch("ovh.Client.get") + @mock.patch("ovh.Client.post") + def test_password_change(self, client_post, client_get): + url = reverse("v2:password_reset_confirm_v2") + url_create_sms = reverse("v2:password_reset") + url_sms_totp = reverse("v2:sms_totp") + + client_get.return_value = ["service"] + client_post.return_value = {"totalCreditsRemoved": 1} + self.set_global_parameters({ + "sms_password_recovery": True, + "sms_provider": "ovh", + "sms_ovh_application_key": "key", + "sms_ovh_application_secret": "secret", + "sms_ovh_consumer_key": "consumer" + }) + + # Prepare account + account = models.User.objects.get(username="user@test.com") + account.phone_number = "+33612345678" + account.secondary_email = "toto@test.com" + account.is_active = True + account.save() + + # Send SMS + self.create_session() + data = {"email": account.email} + response = self.client.post(url_create_sms, data, format="json") + self.assertEqual(response.status_code, 200) + session = self.client.session + secret = session["totp_secret"] + # Get token and ID + data = {"sms_totp": oath.totp(secret), "type": "confirm"} + response = self.client.post(url_sms_totp, data, format="json") + self.assertEqual(response.status_code, 200) + id_ok = response.json()["id"] + token_ok = response.json()["token"] + + # Password differs + data = {"new_password1": "123456", "new_password2": "123457", "token": token_ok, "id": id_ok} + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 400) + + # Wrong ID + id_ko = urlsafe_base64_encode(force_bytes(-1)) + data = {"new_password1": "123456", "new_password2": "123456", "token": token_ok, "id": id_ko} + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 404) + + # Wrong Token + token_ko = "123456" + data = {"new_password1": "123456", "new_password2": "123456", "token": token_ko, "id": id_ok} + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 403) + + # Failed password requirements + data = {"new_password1": "123456", "new_password2": "123456", "token": token_ok, "id": id_ok} + response = self.client.post(url, data, format="json") + self.assertEqual(response.json()["type"], "password_requirement") + + # All good + data = {"new_password1": "MyHardenedPass1!", "new_password2": "MyHardenedPass1!", "token": token_ok, + "id": id_ok} + response = self.client.post(url, data, format="json") + self.assertEqual(response.status_code, 200) + # TODO: See why user doesn't update it's password --> self.test_me_password(password_ok="MyHardenedPass1!") + class AuthenticationTestCase(ModoAPITestCase): From 391180090dc9d198b76efb88c257358bdde7de76 Mon Sep 17 00:00:00 2001 From: Spitap Date: Thu, 29 Dec 2022 19:03:28 +0100 Subject: [PATCH 5/8] Improved UI --- .../views/user/PasswordRecoveryChangeForm.vue | 14 +++++--------- .../views/user/PasswordRecoverySmsTotpForm.vue | 17 +++++++---------- modoboa/core/api/v2/serializers.py | 6 ++---- modoboa/core/api/v2/tests.py | 2 +- modoboa/core/api/v2/views.py | 13 ++++++------- 5 files changed, 21 insertions(+), 31 deletions(-) diff --git a/frontend/src/views/user/PasswordRecoveryChangeForm.vue b/frontend/src/views/user/PasswordRecoveryChangeForm.vue index 47b557c70..c8e02f40f 100644 --- a/frontend/src/views/user/PasswordRecoveryChangeForm.vue +++ b/frontend/src/views/user/PasswordRecoveryChangeForm.vue @@ -102,29 +102,25 @@ export default { id: this.id } this.loading = true + document.getElementById('password_validation').innerHTML = '' auth.changePassword(payload).then(resp => { - document.getElementById('password_validation').innerHTML = '' if (resp.status === 200) { this.loading = false this.returnLogin() } }).catch(err => { + let message = '' this.loading = false if (err.response.status === 403) { - this.$refs.observer.setErrors({ - password_confirmed: this.$gettext('Invalid reset token.') - }) + message = this.$gettext('Invalid reset token.') } else if (err.response.status === 404) { - this.$refs.observer.setErrors({ - password_confirmed: this.$gettext('User unknown.') - }) + message = this.$gettext('User unknown.') } else if (err.response.status === 400 && err.response.data.type === 'password_requirement') { - let message = '' err.response.data.errors.forEach(element => { message += this.$gettext(element) + '
    ' }) - document.getElementById('password_validation').innerHTML = message } + document.getElementById('password_validation').innerHTML = message }) } }, diff --git a/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue b/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue index 23ad9f0c5..f65ceaf0f 100644 --- a/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue +++ b/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue @@ -57,9 +57,6 @@ export default { return { loading: false, sms_totp: '', - password: '', - password_confirmed: '', - user_pk: -1, errors: {} } }, @@ -67,12 +64,10 @@ export default { returnLogin () { this.$router.push({ name: 'Login' }) }, - async userPkError () { + async showErrorDialog (body) { const confirm = await this.$refs.confirm.open( this.$gettext('Error'), - this.$gettext( - 'User seems wrong, return to login or restart reset the process?' - ), + this.$gettext(body), { color: 'error', cancelLabel: this.$gettext('Return to login'), @@ -96,7 +91,7 @@ export default { }).catch(err => { if (err.response.status === 400) { this.loading = false - this.userPkError() + this.showErrorDialog('User seems wrong, return to login or restart reset the process?') } }) }, @@ -118,9 +113,11 @@ export default { } }).catch(err => { this.loading = false - if (err.response.status === 500) { + if (err.response.status === 403 && 'reason' in err.response.data) { + this.showErrorDialog(err.response.data.reason) + } else if (err.response.status === 400 && 'reason' in err.response.data) { this.$refs.observer.setErrors({ - password_confirmed: this.$gettext('Invalid totp.') + sms_totp: this.$gettext(err.response.data.reason) }) } }) diff --git a/modoboa/core/api/v2/serializers.py b/modoboa/core/api/v2/serializers.py index 46e6d792d..1e847f0c9 100644 --- a/modoboa/core/api/v2/serializers.py +++ b/modoboa/core/api/v2/serializers.py @@ -375,9 +375,8 @@ class PasswordRecoverySmsConfirmSerializer(serializers.Serializer): except KeyError: raise CustomValidationError( {"reason": "totp secret not set in session"}, 403) - if not oath.accept_totp(self.context["totp_secret"], data["sms_totp"])[0]: - raise CustomValidationError({"reason": "Wrong totp"}, 403) + raise CustomValidationError({"reason": "Wrong totp"}, 400) # Attempt to get user, will raise an error if pk is not valid self.context["user"] = User.objects.get( @@ -453,8 +452,7 @@ class PasswordRecoveryConfirmSerializer(serializers.Serializer): try: password_validation.validate_password(data["new_password1"], user) except djangoValidationError as e: - raise PasswordRequirementsFailure(e.error_list) - + raise PasswordRequirementsFailure(e.messages) self.context["user"] = user return super().validate(data) diff --git a/modoboa/core/api/v2/tests.py b/modoboa/core/api/v2/tests.py index 258273176..6c72b102f 100644 --- a/modoboa/core/api/v2/tests.py +++ b/modoboa/core/api/v2/tests.py @@ -317,7 +317,7 @@ class PasswordResetTestCase(AccountViewSetTestCase): # False totp code data = {"sms_totp": "123456", "type": "confirm"} response = self.client.post(url_sms_totp, data, format="json") - self.assertEqual(response.status_code, 403) + self.assertEqual(response.status_code, 400) self.assertEqual(response.json()["reason"], "Wrong totp") # Good totp code diff --git a/modoboa/core/api/v2/views.py b/modoboa/core/api/v2/views.py index 506820552..efe3c9814 100644 --- a/modoboa/core/api/v2/views.py +++ b/modoboa/core/api/v2/views.py @@ -147,13 +147,12 @@ class PasswordResetConfirmView(APIView): try: serializer.is_valid(raise_exception=True) except PasswordRequirementsFailure as e: - if type(e) == PasswordRequirementsFailure: - data = {"type": "password_requirement"} - errors = [] - for element in e.message_list: - errors.append(element) - data.update({"errors": errors}) - return response.Response(data, 400) + data = {"type": "password_requirement"} + errors = [] + for element in e.message_list: + errors.append(element) + data.update({"errors": errors}) + return response.Response(data, 400) serializer.save() return response.Response(status=200) From f91edc84b03b33233a2ddfa391ec7331c68a43d8 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 11 Jan 2023 10:55:06 +0100 Subject: [PATCH 6/8] Few updates --- frontend/src/views/Login.vue | 11 +++------ .../views/user/PasswordRecoveryChangeForm.vue | 14 +++++------ .../user/PasswordRecoverySmsTotpForm.vue | 7 ++---- modoboa/core/api/v2/serializers.py | 23 +++++++++---------- 4 files changed, 23 insertions(+), 32 deletions(-) diff --git a/frontend/src/views/Login.vue b/frontend/src/views/Login.vue index 115f4a1e4..24fb2d5fe 100644 --- a/frontend/src/views/Login.vue +++ b/frontend/src/views/Login.vue @@ -34,7 +34,9 @@ outlined />     - Forgot password? + + Forgot password? + - - diff --git a/frontend/src/views/user/PasswordRecoveryChangeForm.vue b/frontend/src/views/user/PasswordRecoveryChangeForm.vue index c8e02f40f..3055b9d13 100644 --- a/frontend/src/views/user/PasswordRecoveryChangeForm.vue +++ b/frontend/src/views/user/PasswordRecoveryChangeForm.vue @@ -15,7 +15,7 @@ v-slot="{ errors }" rules="required" > -

    +

    {{ password_validation_error }}

    - Return to login? + + Return to login +
    @@ -64,6 +66,7 @@ export default { loading: false, password: '', password_confirmed: '', + password_validation_error: '', errors: {}, token: '', id: '' @@ -87,9 +90,6 @@ export default { } } }, - returnLogin () { - this.$router.push({ name: 'Login' }) - }, async changePassword () { const isValid = await this.$refs.observer.validate() if (!isValid) { @@ -102,7 +102,7 @@ export default { id: this.id } this.loading = true - document.getElementById('password_validation').innerHTML = '' + this.password_validation_error = '' auth.changePassword(payload).then(resp => { if (resp.status === 200) { this.loading = false @@ -120,7 +120,7 @@ export default { message += this.$gettext(element) + '
    ' }) } - document.getElementById('password_validation').innerHTML = message + this.password_validation_error = message }) } }, diff --git a/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue b/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue index f65ceaf0f..2bcc55089 100644 --- a/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue +++ b/frontend/src/views/user/PasswordRecoverySmsTotpForm.vue @@ -39,7 +39,7 @@
    Resend SMS. - Return to login? + Return to login
    @@ -61,9 +61,6 @@ export default { } }, methods: { - returnLogin () { - this.$router.push({ name: 'Login' }) - }, async showErrorDialog (body) { const confirm = await this.$refs.confirm.open( this.$gettext('Error'), @@ -91,7 +88,7 @@ export default { }).catch(err => { if (err.response.status === 400) { this.loading = false - this.showErrorDialog('User seems wrong, return to login or restart reset the process?') + this.showErrorDialog(this.$t('User seems wrong, return to login or restart reset the process?')) } }) }, diff --git a/modoboa/core/api/v2/serializers.py b/modoboa/core/api/v2/serializers.py index 1e847f0c9..635a4b859 100644 --- a/modoboa/core/api/v2/serializers.py +++ b/modoboa/core/api/v2/serializers.py @@ -339,12 +339,9 @@ class PasswordRecoverySmsSerializer(serializers.Serializer): request = self.context["request"] clean_email = data["email"] - user = User.objects.filter(email__iexact=clean_email, is_active=True).first() - if user is None: - raise CustomValidationError({"type": "sms", "reason": "No valid user found."}, 404) - - self.context["user"] = (User.objects.filter( - email__iexact=clean_email, is_active=True) + self.context["user"] = ( + User.objects.filter( + email__iexact=clean_email, is_active=True) .exclude(Q(phone_number__isnull=True) | Q(phone_number="")) ).first() if self.context["user"] is None: @@ -379,8 +376,10 @@ class PasswordRecoverySmsConfirmSerializer(serializers.Serializer): raise CustomValidationError({"reason": "Wrong totp"}, 400) # Attempt to get user, will raise an error if pk is not valid - self.context["user"] = User.objects.get( - pk=self.context["request"].session["user_pk"]) + self.context["user"] = User.objects.filter( + pk=self.context["request"].session["user_pk"]).first() + if self.context["user"] is None: + raise CustomValidationError({"reason": "Invalid user"}, 400) return data @@ -403,8 +402,10 @@ class PasswordRecoverySmsResendSerializer(serializers.Serializer): {"reason": "totp secret not set in session"}, status.HTTP_403_FORBIDDEN) # Attempt to get user, will raise an error if pk is not valid - self.context["user"] = User.objects.get( - pk=self.context["request"].session["user_pk"]) + self.context["user"] = User.objects.filter( + pk=self.context["request"].session["user_pk"]).first() + if self.context["user"] is None: + raise CustomValidationError({"reason": "Invalid user"}, 400) return data @@ -461,5 +462,3 @@ class PasswordRecoveryConfirmSerializer(serializers.Serializer): user.set_password(self.validated_data["new_password1"]) user.save() return True - - From b8bb5f35ff5bbe68343ccb686f58a0a02e9fc7f9 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 11 Jan 2023 11:03:36 +0100 Subject: [PATCH 7/8] Remove useless test --- modoboa/core/api/v2/views.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/modoboa/core/api/v2/views.py b/modoboa/core/api/v2/views.py index efe3c9814..38f242cf7 100644 --- a/modoboa/core/api/v2/views.py +++ b/modoboa/core/api/v2/views.py @@ -112,8 +112,7 @@ class DefaultPasswordResetView(EmailPasswordResetView): try: serializer.is_valid(raise_exception=True) except NoSMSAvailable as e: - if type(e) == NoSMSAvailable: - return super().post(request, *args, **kwargs) + return super().post(request, *args, **kwargs) # SMS response return response.Response({"type": "sms"}, 200) @@ -155,5 +154,3 @@ class PasswordResetConfirmView(APIView): return response.Response(data, 400) serializer.save() return response.Response(status=200) - - From c545489726388e470de4e77a2f9420f4d189b7b7 Mon Sep 17 00:00:00 2001 From: Antoine Nguyen Date: Wed, 11 Jan 2023 11:41:44 +0100 Subject: [PATCH 8/8] Fixed unit test --- modoboa/core/api/v2/serializers.py | 6 ++++++ modoboa/core/api/v2/views.py | 31 ++++++++++++++++-------------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/modoboa/core/api/v2/serializers.py b/modoboa/core/api/v2/serializers.py index 635a4b859..d38123777 100644 --- a/modoboa/core/api/v2/serializers.py +++ b/modoboa/core/api/v2/serializers.py @@ -339,6 +339,12 @@ class PasswordRecoverySmsSerializer(serializers.Serializer): request = self.context["request"] clean_email = data["email"] + user = User.objects.filter(email__iexact=clean_email, is_active=True).first() + if user is None: + raise CustomValidationError({ + "type": "sms", "reason": "No valid user found." + }, 404) + self.context["user"] = ( User.objects.filter( email__iexact=clean_email, is_active=True) diff --git a/modoboa/core/api/v2/views.py b/modoboa/core/api/v2/views.py index 38f242cf7..ef07211af 100644 --- a/modoboa/core/api/v2/views.py +++ b/modoboa/core/api/v2/views.py @@ -18,7 +18,7 @@ from modoboa.parameters import tools as param_tools from smtplib import SMTPException -from .serializers import * +from . import serializers logger = logging.getLogger("modoboa.auth") @@ -84,17 +84,16 @@ class EmailPasswordResetView(APIView): """ def post(self, request, *args, **kwargs): - serializer = PasswordRecoveryEmailSerializer( + serializer = serializers.PasswordRecoveryEmailSerializer( data=request.data, context={'request': request}) serializer.is_valid(raise_exception=True) try: serializer.save() except SMTPException: return response.Response({ - "type": "email", - "reason": "Error while sending the email. Please contact an administrator." - }, - 503) + "type": "email", + "reason": "Error while sending the email. Please contact an administrator." + }, 503) # Email response return response.Response({"type": "email"}, 200) @@ -107,11 +106,11 @@ class DefaultPasswordResetView(EmailPasswordResetView): def post(self, request, *args, **kwargs): """Recover password.""" - serializer = PasswordRecoverySmsSerializer( + serializer = serializers.PasswordRecoverySmsSerializer( data=request.data, context={'request': request}) try: serializer.is_valid(raise_exception=True) - except NoSMSAvailable as e: + except serializers.NoSMSAvailable: return super().post(request, *args, **kwargs) # SMS response return response.Response({"type": "sms"}, 200) @@ -123,9 +122,10 @@ class PasswordResetSmsTOTP(APIView): def post(self, request, *args, **kwargs): try: if request.data["type"] == "confirm": - serializer = PasswordRecoverySmsConfirmSerializer(data=request.data, context={'request': request}) + klass = serializers.PasswordRecoverySmsConfirmSerializer elif request.data["type"] == "resend": - serializer = PasswordRecoverySmsResendSerializer(data=request.data, context={'request': request}) + klass = serializers.PasswordRecoverySmsResendSerializer + serializer = klass(data=request.data, context={'request': request}) except (MultiValueDictKeyError, KeyError): return response.Response({"reason": "No type provided."}, 400) serializer.is_valid(raise_exception=True) @@ -133,8 +133,11 @@ class PasswordResetSmsTOTP(APIView): payload = {"type": "resend"} if request.data["type"] == "confirm": serializer_response = serializer.context["response"] - payload.update({"token": serializer_response[0], "id": serializer_response[1], "type": "confirm"}) - + payload.update({ + "token": serializer_response[0], + "id": serializer_response[1], + "type": "confirm" + }) return response.Response(payload, 200) @@ -142,10 +145,10 @@ class PasswordResetConfirmView(APIView): """ Get and set new user password. """ def post(self, request, *args, **kwargs): - serializer = PasswordRecoveryConfirmSerializer(data=request.data) + serializer = serializers.PasswordRecoveryConfirmSerializer(data=request.data) try: serializer.is_valid(raise_exception=True) - except PasswordRequirementsFailure as e: + except serializers.PasswordRequirementsFailure as e: data = {"type": "password_requirement"} errors = [] for element in e.message_list: