diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index 4ac49321f1..bd0b3c9071 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -138,18 +138,6 @@ 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 @@ -243,15 +231,6 @@ 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 0a1ae42930..621f7e45a7 100644 --- a/lms/djangoapps/student_account/urls.py +++ b/lms/djangoapps/student_account/urls.py @@ -10,14 +10,5 @@ urlpatterns = [ if settings.FEATURES.get('ENABLE_COMBINED_LOGIN_REGISTRATION'): urlpatterns += [ - url( - r'^password$', - views.password_change_request_handler, - name='password_change_request' - ), - url( - r'^recover-password$', - views.RecoverPasswordView.as_view(), - name="recover_password_api" - ), + url(r'^password$', views.password_change_request_handler, name='password_change_request'), ] diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 8979f7b80e..3659c60c8d 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -8,7 +8,6 @@ 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 @@ -35,12 +34,10 @@ 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 @@ -53,11 +50,6 @@ 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 @@ -181,7 +173,6 @@ 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. @@ -207,24 +198,32 @@ 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') - - error_message =\ - _("Some error occured during password change. Please try again") + email = user.email if user.is_authenticated() else request.POST.get('email') if email: - status_response = change_password(request, email) - if status_response == 500: - return HttpResponse(error_message, status=status_response) - elif status_response == 403: - return HttpResponseForbidden() + 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) - return HttpResponse(status=status_response) - - return HttpResponseBadRequest(_("No email address provided.")) + return HttpResponse(status=200) + else: + return HttpResponseBadRequest(_("No email address provided.")) def update_context_for_enterprise(request, context): @@ -596,66 +595,3 @@ 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 - """ - - def post(self, request, *args, **kwargs): - """ - Makes the request to change the password - """ - 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 0b4e52a673..1bda1ee67d 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -375,6 +375,7 @@ 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: