From 4462ed37b243f56c481e80c21988ecc7337a9786 Mon Sep 17 00:00:00 2001 From: Julia Eskew Date: Mon, 26 Apr 2021 10:07:36 -0400 Subject: [PATCH] Revert "update login api" (#27416) This PR might be causing e2e tests to fail. Reverting and merging without waiting on tests. --- cms/djangoapps/contentstore/tests/tests.py | 2 +- .../third_party_auth/tests/specs/base.py | 2 +- lms/djangoapps/courseware/tests/helpers.py | 2 +- .../lang_pref/tests/test_middleware.py | 2 +- .../core/djangoapps/user_authn/urls_common.py | 2 +- .../core/djangoapps/user_authn/views/login.py | 48 ++++--------------- .../djangoapps/user_authn/views/login_form.py | 2 +- .../user_authn/views/tests/test_login.py | 14 +----- .../core/djangoapps/user_authn/views/utils.py | 2 +- 9 files changed, 19 insertions(+), 57 deletions(-) diff --git a/cms/djangoapps/contentstore/tests/tests.py b/cms/djangoapps/contentstore/tests/tests.py index 1bca9a9d12..8424e144b4 100644 --- a/cms/djangoapps/contentstore/tests/tests.py +++ b/cms/djangoapps/contentstore/tests/tests.py @@ -29,7 +29,7 @@ class ContentStoreTestCase(ModuleStoreTestCase): returned json """ resp = self.client.post( - reverse('user_api_login_session', kwargs={'api_version': 'v1'}), + reverse('user_api_login_session'), {'email': email, 'password': password} ) return resp diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 60d84ce4ad..fd9bf3fa96 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -448,7 +448,7 @@ class IntegrationTestMixin(testutil.TestCase, test.TestCase, HelperMixin): # Now the user enters their username and password. # The AJAX on the page will log them in: ajax_login_response = self.client.post( - reverse('user_api_login_session', kwargs={'api_version': 'v1'}), + reverse('user_api_login_session'), {'email': self.user.email, 'password': 'test'} ) assert ajax_login_response.status_code == 200 diff --git a/lms/djangoapps/courseware/tests/helpers.py b/lms/djangoapps/courseware/tests/helpers.py index d500bf5bbf..7bb65476f7 100644 --- a/lms/djangoapps/courseware/tests/helpers.py +++ b/lms/djangoapps/courseware/tests/helpers.py @@ -199,7 +199,7 @@ class LoginEnrollmentTestCase(TestCase): """ Login, check that the corresponding view's response has a 200 status code. """ - resp = self.client.post(reverse('user_api_login_session', kwargs={'api_version': 'v1'}), + resp = self.client.post(reverse('user_api_login_session'), {'email': email, 'password': password}) assert resp.status_code == 200 diff --git a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py index 3d746b2962..b53dda1983 100644 --- a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py +++ b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py @@ -186,7 +186,7 @@ class TestUserPreferenceMiddleware(CacheIsolationTestCase): # Use an actual call to the login endpoint, to validate that the middleware # stack does the right thing response = self.client.post( - reverse('user_api_login_session', kwargs={'api_version': 'v1'}), + reverse('user_api_login_session'), data={ 'email': self.user.email, 'password': UserFactory._DEFAULT_PASSWORD, # pylint: disable=protected-access diff --git a/openedx/core/djangoapps/user_authn/urls_common.py b/openedx/core/djangoapps/user_authn/urls_common.py index fdc3a286fd..966420b972 100644 --- a/openedx/core/djangoapps/user_authn/urls_common.py +++ b/openedx/core/djangoapps/user_authn/urls_common.py @@ -54,7 +54,7 @@ urlpatterns = [ # Moved from user_api/legacy_urls.py url( - r'^api/user/(?Pv(1|2))/account/login_session/$', + r'^api/user/v1/account/login_session/$', login.LoginSessionView.as_view(), name="user_api_login_session" ), diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index eeaeca8ebb..9d47e2bc7e 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -13,11 +13,9 @@ import urllib from django.conf import settings from django.contrib.auth import authenticate from django.contrib.auth import login as django_login -from django.contrib.auth import get_user_model from django.contrib.auth.decorators import login_required from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user from django.contrib import admin -from django.db.models import Q from django.http import HttpRequest, HttpResponse, HttpResponseForbidden from django.shortcuts import redirect from django.urls import reverse @@ -39,9 +37,7 @@ from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError from openedx.core.djangoapps.user_authn.toggles import should_redirect_to_authn_microfrontend from openedx.core.djangoapps.util.user_messages import PageLevelMessages from openedx.core.djangoapps.user_authn.views.password_reset import send_password_reset_email_for_user -from openedx.core.djangoapps.user_authn.views.utils import ( - ENTERPRISE_ENROLLMENT_URL_REGEX, UUID4_REGEX, API_V1 -) +from openedx.core.djangoapps.user_authn.views.utils import ENTERPRISE_ENROLLMENT_URL_REGEX, UUID4_REGEX from openedx.core.djangoapps.user_authn.toggles import is_require_third_party_auth_enabled from openedx.core.djangoapps.user_authn.config.waffle import ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY from openedx.core.djangolib.markup import HTML, Text @@ -58,7 +54,6 @@ from common.djangoapps.util.password_policy_validators import normalize_password log = logging.getLogger("edx.student") AUDIT_LOG = logging.getLogger("audit") -USER_MODEL = get_user_model() def _do_third_party_auth(request): @@ -109,34 +104,12 @@ def _get_user_by_email(request): email = request.POST['email'] try: - return USER_MODEL.objects.get(email=email) - except USER_MODEL.DoesNotExist: + return User.objects.get(email=email) + except User.DoesNotExist: digest = hashlib.shake_128(email.encode('utf-8')).hexdigest(16) # pylint: disable=too-many-function-args AUDIT_LOG.warning(f"Login failed - Unknown user email {digest}") -def _get_user_by_email_or_username(request): - """ - Finds a user object in the database based on the given request, ignores all fields except for email and username. - """ - if not ( - 'email' in request.POST or 'username' in request.POST - ) or 'password' not in request.POST: - raise AuthFailedError(_('There was an error receiving your login information. Please email us.')) - - email = request.POST.get('email', None) - username = request.POST.get('username', None) - - try: - return USER_MODEL.objects.get( - Q(username=username) | Q(email=email) - ) - except USER_MODEL.DoesNotExist: - username_or_email = email or username - digest = hashlib.shake_128(username_or_email.encode('utf-8')).hexdigest(16) # pylint: disable=too-many-function-args - AUDIT_LOG.warning(f"Login failed - Unknown user username/email {digest}") - - def _check_excessive_login_attempts(user): """ See if account has been locked out due to excessive login failures @@ -455,7 +428,7 @@ def enterprise_selection_page(request, user, next_url): rate=settings.LOGISTRATION_RATELIMIT_RATE, method='POST', ) # lint-amnesty, pylint: disable=too-many-statements -def login_user(request, api_version='v1'): +def login_user(request): """ AJAX request to log in the user. @@ -521,10 +494,8 @@ def login_user(request, api_version='v1'): response_content = e.get_response() return JsonResponse(response_content, status=403) else: - if api_version == API_V1: - user = _get_user_by_email(request) - else: - user = _get_user_by_email_or_username(request) + user = _get_user_by_email(request) + _check_excessive_login_attempts(user) possibly_authenticated_user = user @@ -621,11 +592,12 @@ class LoginSessionView(APIView): authentication_classes = [] @method_decorator(ensure_csrf_cookie) - def get(self, request, *args, **kwargs): + def get(self, request): return HttpResponse(get_login_session_form(request).to_json(), content_type="application/json") # lint-amnesty, pylint: disable=http-response-with-content-type-json + @method_decorator(require_post_params(["email", "password"])) @method_decorator(csrf_protect) - def post(self, request, api_version): + def post(self, request): """Log in a user. See `login_user` for details. @@ -638,7 +610,7 @@ class LoginSessionView(APIView): 200 {'success': true} """ - return login_user(request, api_version) + return login_user(request) @method_decorator(sensitive_post_parameters("password")) def dispatch(self, request, *args, **kwargs): diff --git a/openedx/core/djangoapps/user_authn/views/login_form.py b/openedx/core/djangoapps/user_authn/views/login_form.py index 7506fd99a8..4f43efb9c5 100644 --- a/openedx/core/djangoapps/user_authn/views/login_form.py +++ b/openedx/core/djangoapps/user_authn/views/login_form.py @@ -89,7 +89,7 @@ def get_login_session_form(request): HttpResponse """ - form_desc = FormDescription("post", reverse("user_api_login_session", kwargs={'api_version': 'v1'})) + form_desc = FormDescription("post", reverse("user_api_login_session")) _apply_third_party_auth_overrides(request, form_desc) # Translators: This label appears above a field on the login form diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login.py b/openedx/core/djangoapps/user_authn/views/tests/test_login.py index 9d75acd5c1..171c6d1b86 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_login.py @@ -957,7 +957,7 @@ class LoginSessionViewTest(ApiTestCase): def setUp(self): super().setUp() - self.url = reverse("user_api_login_session", kwargs={'api_version': 'v1'}) + self.url = reverse("user_api_login_session") @ddt.data("get", "post") def test_auth_disabled(self, method): @@ -986,7 +986,7 @@ class LoginSessionViewTest(ApiTestCase): # Verify that the form description matches what we expect form_desc = json.loads(response.content.decode('utf-8')) assert form_desc['method'] == 'post' - assert form_desc['submit_url'] == reverse('user_api_login_session', kwargs={'api_version': 'v1'}) + assert form_desc['submit_url'] == reverse('user_api_login_session') assert form_desc['fields'] == [{'name': 'email', 'defaultValue': '', 'type': 'email', 'required': True, 'label': 'Email', 'placeholder': '', 'instructions': 'The email address you used to register with {platform_name}' @@ -1050,16 +1050,6 @@ class LoginSessionViewTest(ApiTestCase): {'category': 'conversion', 'provider': None, 'label': track_label} ) - def test_login_with_username(self): - UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD) - data = { - "username": self.USERNAME, - "password": self.PASSWORD, - } - self.url = reverse("user_api_login_session", kwargs={'api_version': 'v2'}) - response = self.client.post(self.url, data) - self.assertHttpOK(response) - def test_session_cookie_expiry(self): # Create a test user UserFactory.create(username=self.USERNAME, email=self.EMAIL, password=self.PASSWORD) diff --git a/openedx/core/djangoapps/user_authn/views/utils.py b/openedx/core/djangoapps/user_authn/views/utils.py index 4d1645987b..12432b3a1f 100644 --- a/openedx/core/djangoapps/user_authn/views/utils.py +++ b/openedx/core/djangoapps/user_authn/views/utils.py @@ -11,7 +11,7 @@ from common.djangoapps.third_party_auth import pipeline from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.geoinfo.api import country_code_from_ip -API_V1 = 'v1' + UUID4_REGEX = '[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}' ENTERPRISE_ENROLLMENT_URL_REGEX = fr'/enterprise/{UUID4_REGEX}/course/{settings.COURSE_KEY_REGEX}/enroll'