From 34a4cbf715867b439885fe60f0b517d37479904d Mon Sep 17 00:00:00 2001 From: Shafqat Farhan Date: Tue, 6 Sep 2022 05:49:07 +0500 Subject: [PATCH] feat: VAN-1075 - Added country field validation on registration endpoint --- .../tests/test_configuration_overrides.py | 2 +- .../djangoapps/user_authn/views/register.py | 26 ++++ .../user_authn/views/tests/test_register.py | 113 +++++++++++++++++- 3 files changed, 138 insertions(+), 3 deletions(-) diff --git a/common/djangoapps/student/tests/test_configuration_overrides.py b/common/djangoapps/student/tests/test_configuration_overrides.py index be4aae913d..477f8255f6 100644 --- a/common/djangoapps/student/tests/test_configuration_overrides.py +++ b/common/djangoapps/student/tests/test_configuration_overrides.py @@ -68,7 +68,7 @@ class TestSite(TestCase): "address1": "foo", "city": "foo", "state": "foo", - "country": "foo", + "country": "US", "company": "foo", "title": "foo" }.items())) diff --git a/openedx/core/djangoapps/user_authn/views/register.py b/openedx/core/djangoapps/user_authn/views/register.py index 79cdbd39f6..7c39a751b0 100644 --- a/openedx/core/djangoapps/user_authn/views/register.py +++ b/openedx/core/djangoapps/user_authn/views/register.py @@ -21,6 +21,7 @@ from django.utils.translation import get_language from django.utils.translation import gettext as _ from django.views.decorators.csrf import csrf_exempt, ensure_csrf_cookie from django.views.decorators.debug import sensitive_post_parameters +from django_countries import countries from edx_django_utils.monitoring import set_custom_attribute from edx_toggles.toggles import WaffleFlag from openedx_events.learning.data import UserData, UserPersonalData @@ -588,6 +589,10 @@ class RegistrationView(APIView): if response: return response + response = self._handle_country_code_validation(request, data) + if response: + return response + response, user = self._create_account(request, data) if response: return response @@ -607,6 +612,27 @@ class RegistrationView(APIView): mark_user_change_as_expected(user.id) return response + def _handle_country_code_validation(self, request, data): + # pylint: disable=no-member + country = data.get('country') + is_valid_country_code = country in dict(countries).keys() + + errors = {} + error_code = 'invalid-country' + error_message = accounts_settings.REQUIRED_FIELD_COUNTRY_MSG + extra_fields = configuration_helpers.get_value( + 'REGISTRATION_EXTRA_FIELDS', + getattr(settings, 'REGISTRATION_EXTRA_FIELDS', {}) + ) + + if extra_fields.get('country', 'hidden') == 'required' and not is_valid_country_code: + errors['country'] = [{'user_message': error_message}] + elif country and not is_valid_country_code: + errors['country'] = [{'user_message': error_message}] + + if errors: + return self._create_response(request, errors, status_code=400, error_code=error_code) + def _handle_duplicate_email_username(self, request, data): # pylint: disable=no-member # TODO Verify whether this check is needed here - it may be duplicated in user_api. diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_register.py b/openedx/core/djangoapps/user_authn/views/tests/test_register.py index 82121a4cef..cd5000ed59 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -32,6 +32,7 @@ from openedx.core.djangoapps.user_api.accounts import ( EMAIL_MIN_LENGTH, NAME_MAX_LENGTH, REQUIRED_FIELD_CONFIRM_EMAIL_MSG, + REQUIRED_FIELD_COUNTRY_MSG, USERNAME_BAD_LENGTH_MSG, USERNAME_INVALID_CHARS_ASCII, USERNAME_INVALID_CHARS_UNICODE, @@ -89,7 +90,7 @@ class RegistrationViewValidationErrorTest( YEAR_OF_BIRTH = "1998" ADDRESS = "123 Fake Street" CITY = "Springfield" - COUNTRY = "us" + COUNTRY = "US" GOALS = "Learn all the things!" @classmethod @@ -399,6 +400,66 @@ class RegistrationViewValidationErrorTest( } ) + def test_duplicate_email_username_error(self): + # Register the first user + response = self.client.post(self.url, { + "email": self.EMAIL, + "name": self.NAME, + "username": self.USERNAME, + "password": self.PASSWORD, + "honor_code": "true", + }) + self.assertHttpOK(response) + + # Try to create a second user with the same username and email + response = self.client.post(self.url, { + "email": self.EMAIL, + "name": "Someone Else", + "username": self.USERNAME, + "password": self.PASSWORD, + "country": self.COUNTRY, + "honor_code": "true", + }) + + response_json = json.loads(response.content.decode('utf-8')) + assert response.status_code == 409 + username_suggestions = response_json.pop('username_suggestions') + assert len(username_suggestions) == 3 + self.assertDictEqual( + response_json, + { + "username": [{ + "user_message": AUTHN_USERNAME_CONFLICT_MSG, + }], + "email": [{ + "user_message": AUTHN_EMAIL_CONFLICT_MSG, + }], + "error_code": "duplicate-email-username" + } + ) + + def test_invalid_country_code_error(self): + response = self.client.post(self.url, { + "email": self.EMAIL, + "name": self.NAME, + "username": self.USERNAME, + "password": self.PASSWORD, + "country": "Invalid country code", + "honor_code": "true", + }) + + response_json = json.loads(response.content.decode('utf-8')) + self.assertHttpBadRequest(response) + self.assertDictEqual( + response_json, + { + "country": [{ + "user_message": REQUIRED_FIELD_COUNTRY_MSG, + }], + "error_code": "invalid-country" + } + ) + @ddt.ddt @skip_unless_lms @@ -1528,7 +1589,6 @@ class RegistrationViewTestV1( response = self.client.post(self.url, data) self.assertHttpBadRequest(response) - @override_settings(REGISTRATION_EXTRA_FIELDS={"country": "required"}) @ddt.data("email", "name", "username", "password", "country") def test_register_missing_required_field(self, missing_field): data = { @@ -1545,6 +1605,55 @@ class RegistrationViewTestV1( response = self.client.post(self.url, data) self.assertHttpBadRequest(response) + @override_settings(REGISTRATION_EXTRA_FIELDS={"country": "required"}) + def test_register_missing_country_required_field(self): + data = { + "email": self.EMAIL, + "name": self.NAME, + "username": self.USERNAME, + "password": self.PASSWORD, + "country": self.COUNTRY, + } + del data['country'] + + response = self.client.post(self.url, data) + response_json = json.loads(response.content.decode('utf-8')) + + self.assertHttpBadRequest(response) + self.assertDictEqual( + response_json, + { + "country": [{ + "user_message": REQUIRED_FIELD_COUNTRY_MSG, + }], + "error_code": "invalid-country" + } + ) + + @override_settings(REGISTRATION_EXTRA_FIELDS={"country": "required"}) + def test_register_invalid_country_required_field(self): + data = { + "email": self.EMAIL, + "name": self.NAME, + "username": self.USERNAME, + "password": self.PASSWORD, + "country": "Invalid country code", + } + + response = self.client.post(self.url, data) + response_json = json.loads(response.content.decode('utf-8')) + + self.assertHttpBadRequest(response) + self.assertDictEqual( + response_json, + { + "country": [{ + "user_message": REQUIRED_FIELD_COUNTRY_MSG, + }], + "error_code": "invalid-country" + } + ) + def test_register_duplicate_email(self): # Register the first user response = self.client.post(self.url, {