From bc16a70c6379f0fa447c1cf4af5c94aac6672555 Mon Sep 17 00:00:00 2001 From: Jose Antonio Gonzalez Date: Wed, 15 Nov 2017 19:43:48 +0200 Subject: [PATCH 1/4] Add recover password endpoint --- .../student_account/test/test_views.py | 21 ++++ lms/djangoapps/student_account/urls.py | 11 +- lms/djangoapps/student_account/views.py | 107 ++++++++++++++---- .../core/djangoapps/user_api/accounts/api.py | 1 - 4 files changed, 117 insertions(+), 23 deletions(-) diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index bd0b3c9071..4ac49321f1 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -138,6 +138,18 @@ class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin): self._change_password() self.assertRaises(UserAPIInternalError) + @ddt.data( + ({'email': 'walter@graymattertech.com'}, 200), + ({'email': ''}, 400), + ({}, 400), + ) + @ddt.unpack + @skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in LMS') + def test_password_recover_api(self, password_recover_info, expected_status): + self.client.logout() + response = self._recover_password_through_api(**password_recover_info) + self.assertEqual(response.status_code, expected_status) + @ddt.data(True, False) def test_password_change_logged_out(self, send_email): # Log the user out @@ -231,6 +243,15 @@ class StudentAccountUpdateTest(CacheIsolationTestCase, UrlResetMixin): return self.client.post(path=reverse('password_change_request'), data=data) + def _recover_password_through_api(self, email=None): + """Request to change the user's password. """ + data = {} + + if email: + data['email'] = email + + return self.client.post(path=reverse('recover_password_api'), data=data) + def _create_dop_tokens(self, user=None): """Create dop access token for given user if user provided else for default user.""" if not user: diff --git a/lms/djangoapps/student_account/urls.py b/lms/djangoapps/student_account/urls.py index 621f7e45a7..0a1ae42930 100644 --- a/lms/djangoapps/student_account/urls.py +++ b/lms/djangoapps/student_account/urls.py @@ -10,5 +10,14 @@ urlpatterns = [ if settings.FEATURES.get('ENABLE_COMBINED_LOGIN_REGISTRATION'): urlpatterns += [ - url(r'^password$', views.password_change_request_handler, name='password_change_request'), + url( + r'^password$', + views.password_change_request_handler, + name='password_change_request' + ), + url( + r'^recover-password$', + views.RecoverPasswordView.as_view(), + name="recover_password_api" + ), ] diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 3659c60c8d..1964eaa439 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -8,6 +8,7 @@ from datetime import datetime from django.conf import settings from django.contrib import messages from django.contrib.auth import get_user_model +from django.contrib.auth.models import User from django.contrib.auth.decorators import login_required from django.core.urlresolvers import reverse from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden @@ -34,10 +35,12 @@ from openedx.core.djangoapps.user_api.api import ( get_login_session_form, get_password_reset_form ) + from openedx.core.djangoapps.user_api.errors import ( UserNotFound, UserAPIInternalError ) + from openedx.core.lib.edx_api_utils import get_edx_api_data from openedx.core.lib.time_zone_utils import TIME_ZONE_CHOICES from openedx.features.enterprise_support.api import enterprise_customer_for_request @@ -50,6 +53,11 @@ from third_party_auth.decorators import xframe_allow_whitelisted from util.bad_request_rate_limiter import BadRequestRateLimiter from util.date_utils import strftime_localized +from rest_framework.response import Response +from rest_framework import status +from rest_framework import views +from rest_framework.decorators import api_view + AUDIT_LOG = logging.getLogger("audit") log = logging.getLogger(__name__) User = get_user_model() # pylint:disable=invalid-name @@ -173,6 +181,7 @@ def login_and_registration_form(request, initial_mode="login"): @require_http_methods(['POST']) +@api_view(['POST']) def password_change_request_handler(request): """Handle password change requests originating from the account page. @@ -198,32 +207,26 @@ def password_change_request_handler(request): """ - limiter = BadRequestRateLimiter() - if limiter.is_rate_limit_exceeded(request): - AUDIT_LOG.warning("Password reset rate limit exceeded") - return HttpResponseForbidden() - user = request.user # Prefer logged-in user's email - email = user.email if user.is_authenticated() else request.POST.get('email') + email =\ + user.email if user.is_authenticated() else request.POST.get('email') if email: - try: - request_password_change(email, request.is_secure()) - user = user if user.is_authenticated() else User.objects.get(email=email) - destroy_oauth_tokens(user) - except UserNotFound: - AUDIT_LOG.info("Invalid password reset attempt") - # Increment the rate limit counter - limiter.tick_bad_request_counter(request) - except UserAPIInternalError as err: - log.exception('Error occured during password change for user {email}: {error}' - .format(email=email, error=err)) - return HttpResponse(_("Some error occured during password change. Please try again"), status=500) + status_response = change_password(request, email) + if status_response == 500: + return\ + HttpResponse( + _("Some error occured during password change. Please try " + "again"), + status=status_response + ) + elif status_response == 403: + return HttpResponseForbidden() - return HttpResponse(status=200) - else: - return HttpResponseBadRequest(_("No email address provided.")) + return HttpResponse(status=status_response) + + return HttpResponseBadRequest(_("No email address provided.")) def update_context_for_enterprise(request, context): @@ -595,3 +598,65 @@ def account_settings_context(request): } for state in auth_states if state.provider.display_for_login or state.has_account] return context + + +class RecoverPasswordView(views.APIView): + """ + Resets a password of a user by proving the email address + + Example Requests: + POST /account/recover-password + {"email": "staff@example.com"} + + Response Values: + HttpResponse: 200 if the password was reset correctly + HttpResponse: 400 if email wasn't provided + HttpResponse: 403 if the client has been rate limited + HttpResponse: 405 if using an unsupported HTTP method + """ + + http_method_names = ["post"] + + def post(self, request, format=None): + email = request.data.get('email') + if email: + return Response(status=change_password(request, email)) + + return Response(status=status.HTTP_400_BAD_REQUEST) + + +def change_password(request, email): + """ + Changes user's password by providing email. + + Args: + request (HTTPRequest): The request to change user's email + email (String): The user email + + Returns: + status code of Response + Options: + - 200 + - 403 + - 500 + """ + limiter = BadRequestRateLimiter() + if limiter.is_rate_limit_exceeded(request): + AUDIT_LOG.warning("Password reset rate limit exceeded") + return status.HTTP_403_FORBIDDEN + + try: + user = User.objects.get(email=email) + request_password_change(email, request.is_secure()) + destroy_oauth_tokens(user) + except User.DoesNotExist: + AUDIT_LOG.info("Invalid password reset attempt") + # Increment the rate limit counter + limiter.tick_bad_request_counter(request) + except UserAPIInternalError as err: + log.exception( + 'Error occured during password change for user {email}: {error}' + .format(email=email, error=err) + ) + return status.HTTP_500_INTERNAL_SERVER_ERROR + return status.HTTP_200_OK diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 1bda1ee67d..0b4e52a673 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -375,7 +375,6 @@ def request_password_change(email, is_secure): Args: email (str): An email address - orig_host (str): An originating host, extracted from a request with get_host is_secure (bool): Whether the request was made with HTTPS Returns: From 0171e797645e5b1e7d46034212a909c583f7927f Mon Sep 17 00:00:00 2001 From: Jose Antonio Gonzalez Date: Fri, 24 Nov 2017 14:07:23 +0200 Subject: [PATCH 2/4] fix pylint [ci skip] --- lms/djangoapps/student_account/views.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 1964eaa439..23efbed7b4 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -218,7 +218,7 @@ def password_change_request_handler(request): return\ HttpResponse( _("Some error occured during password change. Please try " - "again"), + "again"), status=status_response ) elif status_response == 403: @@ -618,6 +618,9 @@ class RecoverPasswordView(views.APIView): http_method_names = ["post"] def post(self, request, format=None): + """ + Makes the request to change the password + """ email = request.data.get('email') if email: return Response(status=change_password(request, email)) From 5150a960055c31623dacf252a8db13532ee7a20c Mon Sep 17 00:00:00 2001 From: Jose Antonio Gonzalez Date: Fri, 24 Nov 2017 14:59:41 +0200 Subject: [PATCH 3/4] fix pep8 --- lms/djangoapps/student_account/views.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 23efbed7b4..70c9c8c08e 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -212,15 +212,13 @@ def password_change_request_handler(request): email =\ user.email if user.is_authenticated() else request.POST.get('email') + error_message =\ + _("Some error occured during password change. Please try again") + if email: status_response = change_password(request, email) if status_response == 500: - return\ - HttpResponse( - _("Some error occured during password change. Please try " - "again"), - status=status_response - ) + return HttpResponse(error_message, status=status_response) elif status_response == 403: return HttpResponseForbidden() @@ -615,8 +613,6 @@ class RecoverPasswordView(views.APIView): HttpResponse: 405 if using an unsupported HTTP method """ - http_method_names = ["post"] - def post(self, request, format=None): """ Makes the request to change the password From 7a1dd74a0f08af18977eb576fdd04f69bd6ab357 Mon Sep 17 00:00:00 2001 From: Jose Antonio Gonzalez Date: Fri, 24 Nov 2017 15:54:42 +0200 Subject: [PATCH 4/4] dont redefine post method [ci skip] --- lms/djangoapps/student_account/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 70c9c8c08e..8979f7b80e 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -613,7 +613,7 @@ class RecoverPasswordView(views.APIView): HttpResponse: 405 if using an unsupported HTTP method """ - def post(self, request, format=None): + def post(self, request, *args, **kwargs): """ Makes the request to change the password """