Skip to content

Commit

Permalink
feat: added country disabling feature (#35451)
Browse files Browse the repository at this point in the history
* feat: added country disabling feature
  • Loading branch information
AhtishamShahid authored Sep 25, 2024
1 parent 75d111a commit b50c423
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 12 deletions.
7 changes: 7 additions & 0 deletions cms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -2936,3 +2936,10 @@ def _should_send_learning_badge_events(settings):
# See https://www.meilisearch.com/docs/learn/security/tenant_tokens
MEILISEARCH_INDEX_PREFIX = ""
MEILISEARCH_API_KEY = "devkey"

# .. setting_name: DISABLED_COUNTRIES
# .. setting_default: []
# .. setting_description: List of country codes that should be disabled
# .. for now it wil impact country listing in auth flow and user profile.
# .. eg ['US', 'CA']
DISABLED_COUNTRIES = []
7 changes: 7 additions & 0 deletions cms/envs/production.py
Original file line number Diff line number Diff line change
Expand Up @@ -689,3 +689,10 @@ def get_env_setting(setting):
}

BEAMER_PRODUCT_ID = ENV_TOKENS.get('BEAMER_PRODUCT_ID', BEAMER_PRODUCT_ID)

# .. setting_name: DISABLED_COUNTRIES
# .. setting_default: []
# .. setting_description: List of country codes that should be disabled
# .. for now it wil impact country listing in auth flow and user profile.
# .. eg ['US', 'CA']
DISABLED_COUNTRIES = ENV_TOKENS.get('DISABLED_COUNTRIES', [])
8 changes: 8 additions & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -5544,3 +5544,11 @@ def _should_send_learning_badge_events(settings):
# .. setting_default: empty dictionary
# .. setting_description: Dictionary with additional information that you want to share in the report.
SURVEY_REPORT_EXTRA_DATA = {}


# .. setting_name: DISABLED_COUNTRIES
# .. setting_default: []
# .. setting_description: List of country codes that should be disabled
# .. for now it wil impact country listing in auth flow and user profile.
# .. eg ['US', 'CA']
DISABLED_COUNTRIES = []
7 changes: 7 additions & 0 deletions lms/envs/production.py
Original file line number Diff line number Diff line change
Expand Up @@ -1124,3 +1124,10 @@ def get_env_setting(setting):
EVENT_BUS_PRODUCER_CONFIG = merge_producer_configs(EVENT_BUS_PRODUCER_CONFIG,
ENV_TOKENS.get('EVENT_BUS_PRODUCER_CONFIG', {}))
BEAMER_PRODUCT_ID = ENV_TOKENS.get('BEAMER_PRODUCT_ID', BEAMER_PRODUCT_ID)

# .. setting_name: DISABLED_COUNTRIES
# .. setting_default: []
# .. setting_description: List of country codes that should be disabled
# .. for now it wil impact country listing in auth flow and user profile.
# .. eg ['US', 'CA']
DISABLED_COUNTRIES = ENV_TOKENS.get('DISABLED_COUNTRIES', [])
7 changes: 6 additions & 1 deletion openedx/core/djangoapps/user_api/accounts/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
Programmatic integration point for User API Accounts sub-application
"""


import datetime
import re

Expand Down Expand Up @@ -152,6 +151,12 @@ def update_account_settings(requesting_user, update, username=None):

_validate_email_change(user, update, field_errors)
_validate_secondary_email(user, update, field_errors)
if update.get('country', '') in settings.DISABLED_COUNTRIES:
field_errors['country'] = {
'developer_message': 'Country is disabled for registration',
'user_message': 'This country cannot be selected for user registration'
}

old_name = _validate_name_change(user_profile, update, field_errors)
old_language_proficiencies = _get_old_language_proficiencies_if_updating(user_profile, update)

Expand Down
31 changes: 24 additions & 7 deletions openedx/core/djangoapps/user_api/accounts/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
Most of the functionality is covered in test_views.py.
"""


import datetime
import itertools
import unicodedata
Expand All @@ -16,6 +15,7 @@
from django.http import HttpResponse
from django.test import TestCase
from django.test.client import RequestFactory
from django.test.utils import override_settings
from django.urls import reverse
from pytz import UTC
from social_django.models import UserSocialAuth
Expand Down Expand Up @@ -82,7 +82,8 @@ def create_account(self, username, password, email):

@skip_unless_lms
@ddt.ddt
@patch('common.djangoapps.student.views.management.render_to_response', Mock(side_effect=mock_render_to_response, autospec=True)) # lint-amnesty, pylint: disable=line-too-long
@patch('common.djangoapps.student.views.management.render_to_response',
Mock(side_effect=mock_render_to_response, autospec=True)) # lint-amnesty, pylint: disable=line-too-long
class TestAccountApi(UserSettingsEventTestMixin, EmailTemplateTagMixin, CreateAccountMixin, RetirementTestCase):
"""
These tests specifically cover the parts of the API methods that are not covered by test_views.py.
Expand Down Expand Up @@ -205,7 +206,7 @@ def test_add_social_links(self):

account_settings = get_account_settings(self.default_request)[0]
assert account_settings['social_links'] == \
sorted((original_social_links + extra_social_links), key=(lambda s: s['platform']))
sorted((original_social_links + extra_social_links), key=(lambda s: s['platform']))

def test_replace_social_links(self):
original_facebook_link = dict(platform="facebook", social_link="https://www.facebook.com/myself")
Expand Down Expand Up @@ -306,7 +307,7 @@ def test_update_validation_error_for_enterprise(
with pytest.raises(AccountValidationError) as validation_error:
update_account_settings(self.user, update_data)
field_errors = validation_error.value.field_errors
assert 'This field is not editable via this API' ==\
assert 'This field is not editable via this API' == \
field_errors[field_name_value[0]]['developer_message']
else:
update_account_settings(self.user, update_data)
Expand Down Expand Up @@ -424,8 +425,8 @@ def test_name_update_does_not_require_idv(self, has_passable_cert, enrolled_in_v
"""
Test that the user can change their name if change does not require IDV.
"""
with patch('openedx.core.djangoapps.user_api.accounts.api.get_certificates_for_user') as mock_get_certs,\
patch('openedx.core.djangoapps.user_api.accounts.api.get_verified_enrollments') as \
with patch('openedx.core.djangoapps.user_api.accounts.api.get_certificates_for_user') as mock_get_certs, \
patch('openedx.core.djangoapps.user_api.accounts.api.get_verified_enrollments') as \
mock_get_verified_enrollments:
mock_get_certs.return_value = (
[{'status': CertificateStatuses.downloadable}] if
Expand All @@ -439,7 +440,8 @@ def test_name_update_does_not_require_idv(self, has_passable_cert, enrolled_in_v
assert account_settings['name'] == 'New Name'

@patch('django.core.mail.EmailMultiAlternatives.send')
@patch('common.djangoapps.student.views.management.render_to_string', Mock(side_effect=mock_render_to_string, autospec=True)) # lint-amnesty, pylint: disable=line-too-long
@patch('common.djangoapps.student.views.management.render_to_string',
Mock(side_effect=mock_render_to_string, autospec=True))
def test_update_sending_email_fails(self, send_mail):
"""Test what happens if all validation checks pass, but sending the email for email change fails."""
send_mail.side_effect = [Exception, None]
Expand Down Expand Up @@ -514,6 +516,7 @@ def test_language_proficiency_eventing(self):
"""
Test that eventing of language proficiencies, which happens update_account_settings method, behaves correctly.
"""

def verify_event_emitted(new_value, old_value):
"""
Confirm that the user setting event was properly emitted
Expand Down Expand Up @@ -571,6 +574,20 @@ def test_change_country_removes_state(self):
assert account_settings['country'] is None
assert account_settings['state'] is None

@override_settings(DISABLED_COUNTRIES=['KP'])
def test_change_to_disabled_country(self):
"""
Test that changing the country to a disabled country is not allowed
"""
# First set the country and state
update_account_settings(self.user, {"country": UserProfile.COUNTRY_WITH_STATES, "state": "MA"})
account_settings = get_account_settings(self.default_request)[0]
assert account_settings['country'] == UserProfile.COUNTRY_WITH_STATES
assert account_settings['state'] == 'MA'

with self.assertRaises(AccountValidationError):
update_account_settings(self.user, {"country": "KP"})

def test_get_name_validation_error_too_long(self):
"""
Test validation error when the name is too long.
Expand Down
1 change: 0 additions & 1 deletion openedx/core/djangoapps/user_authn/config/waffle.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
Waffle flags and switches for user authn.
"""


from edx_toggles.toggles import WaffleSwitch

_WAFFLE_NAMESPACE = 'user_authn'
Expand Down
14 changes: 12 additions & 2 deletions openedx/core/djangoapps/user_authn/views/registration_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from openedx.core.djangoapps.user_api import accounts
from openedx.core.djangoapps.user_api.helpers import FormDescription
from openedx.core.djangoapps.user_authn.utils import check_pwned_password, is_registration_api_v1 as is_api_v1
from openedx.core.djangoapps.user_authn.views.utils import remove_disabled_country_from_list
from openedx.core.djangolib.markup import HTML, Text
from openedx.features.enterprise_support.api import enterprise_customer_for_request
from common.djangoapps.student.models import (
Expand Down Expand Up @@ -297,6 +298,15 @@ def cleaned_extended_profile(self):
if key in self.extended_profile_fields and value is not None
}

def clean_country(self):
"""
Check if the user's country is in the embargoed countries list.
"""
country = self.cleaned_data.get("country")
if country in settings.DISABLED_COUNTRIES:
raise ValidationError(_("Registration from this country is not allowed due to restrictions."))
return self.cleaned_data.get("country")


def get_registration_extension_form(*args, **kwargs):
"""
Expand Down Expand Up @@ -686,7 +696,7 @@ def _add_marketing_emails_opt_in_field(self, form_desc, required=False):
"""
opt_in_label = _(
'I agree that {platform_name} may send me marketing messages.').format(
platform_name=configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
platform_name=configuration_helpers.get_value('PLATFORM_NAME', settings.PLATFORM_NAME),
)

form_desc.add_field(
Expand Down Expand Up @@ -974,7 +984,7 @@ def _add_country_field(self, form_desc, required=True):
label=country_label,
instructions=country_instructions,
field_type="select",
options=list(countries),
options=list(remove_disabled_country_from_list(dict(countries)).items()),
include_default_option=True,
required=required,
error_messages={
Expand Down
46 changes: 45 additions & 1 deletion openedx/core/djangoapps/user_authn/views/tests/test_register.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
password_validators_instruction_texts,
password_validators_restrictions
)

ENABLE_AUTO_GENERATED_USERNAME = settings.FEATURES.copy()
ENABLE_AUTO_GENERATED_USERNAME['ENABLE_AUTO_GENERATED_USERNAME'] = True

Expand Down Expand Up @@ -1556,7 +1557,7 @@ def test_activation_email(self):
assert len(mail.outbox) == 1
sent_email = mail.outbox[0]
assert sent_email.to == [self.EMAIL]
assert sent_email.subject ==\
assert sent_email.subject == \
f'Action Required: Activate your {settings.PLATFORM_NAME} account'
assert f'high-quality {settings.PLATFORM_NAME} courses' in sent_email.body

Expand Down Expand Up @@ -2468,6 +2469,31 @@ def test_register_error_with_pwned_password(self, emit):
})
assert response.status_code == 400

@override_settings(DISABLED_COUNTRIES=['KP'])
def test_register_with_disabled_country(self):
"""
Test case to check user registration is forbidden when registration is disabled for a country
"""
response = self.client.post(self.url, {
"email": self.EMAIL,
"name": self.NAME,
"username": self.USERNAME,
"password": self.PASSWORD,
"honor_code": "true",
"country": "KP",
})
assert response.status_code == 400
response_json = json.loads(response.content.decode('utf-8'))
self.assertDictEqual(
response_json,
{'country':
[
{
'user_message': 'Registration from this country is not allowed due to restrictions.'
}
], 'error_code': 'validation-error'}
)


@httpretty.activate
@ddt.ddt
Expand Down Expand Up @@ -2575,6 +2601,24 @@ def test_success(self):

self._verify_user_existence(user_exists=True, social_link_exists=True, user_is_active=False)

@override_settings(DISABLED_COUNTRIES=['US'])
def test_with_disabled_country(self):
"""
Test case to check user registration is forbidden when registration is disabled for a country
"""
self._verify_user_existence(user_exists=False, social_link_exists=False)
self._setup_provider_response(success=True)
response = self.client.post(self.url, self.data())
assert response.status_code == 400
assert response.json() == {
'country': [
{
'user_message': 'Registration from this country is not allowed due to restrictions.'
}
], 'error_code': 'validation-error'
}
self._verify_user_existence(user_exists=False, social_link_exists=False, user_is_active=False)

def test_unlinked_active_user(self):
user = UserFactory()
response = self.client.post(self.url, self.data(user))
Expand Down
17 changes: 17 additions & 0 deletions openedx/core/djangoapps/user_authn/views/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
"""
import logging
import re
from typing import Dict

from django.conf import settings
from django.contrib import messages
from django.utils.translation import gettext as _
Expand Down Expand Up @@ -177,3 +179,18 @@ def get_auto_generated_username(data):
# We generate the username regardless of whether the name is empty or invalid. We do this
# because the name validations occur later, ensuring that users cannot create an account without a valid name.
return f"{username_prefix}_{username_suffix}" if username_prefix else username_suffix


def remove_disabled_country_from_list(countries: Dict) -> Dict:
"""
Remove disabled countries from the list of countries.
Args:
- countries (dict): List of countries.
Returns:
- dict: Dict of countries with disabled countries removed.
"""
for country_code in settings.DISABLED_COUNTRIES:
del countries[country_code]
return countries

0 comments on commit b50c423

Please sign in to comment.