Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added country disabling feature #35451

Merged
merged 5 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
AhtishamShahid marked this conversation as resolved.
Show resolved Hide resolved
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
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
Loading