Skip to content

Commit

Permalink
feat: added country disabling feature
Browse files Browse the repository at this point in the history
fix: resolved linter errors

fix: resolved linter errors

feat: added unit test for social registration

fix: removed linter error
  • Loading branch information
AhtishamShahid committed Sep 12, 2024
1 parent 082350e commit eb7f912
Show file tree
Hide file tree
Showing 8 changed files with 147 additions and 12 deletions.
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', ['RU'])
11 changes: 10 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 @@ -41,6 +40,7 @@
from openedx.features.enterprise_support.utils import get_enterprise_readonly_account_fields
from openedx.features.name_affirmation_api.utils import is_name_affirmation_installed
from .serializers import AccountLegacyProfileSerializer, AccountUserSerializer, UserReadOnlySerializer, _visible_fields
from ...user_authn.config.waffle import ENABLE_COUNTRY_DISABLING

name_affirmation_installed = is_name_affirmation_installed()
if name_affirmation_installed:
Expand Down Expand Up @@ -152,6 +152,15 @@ def update_account_settings(requesting_user, update, username=None):

_validate_email_change(user, update, field_errors)
_validate_secondary_email(user, update, field_errors)
if (
ENABLE_COUNTRY_DISABLING.is_enabled()
and update['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
34 changes: 27 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,7 +15,9 @@
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 edx_toggles.toggles.testutils import override_waffle_switch
from pytz import UTC
from social_django.models import UserSocialAuth
from common.djangoapps.student.models import (
Expand Down Expand Up @@ -48,6 +49,7 @@
UserNotAuthorized,
UserNotFound
)
from openedx.core.djangoapps.user_authn.config.waffle import ENABLE_COUNTRY_DISABLING
from openedx.core.djangolib.testing.utils import skip_unless_lms
from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerUserFactory

Expand Down Expand Up @@ -82,7 +84,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 +208,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 +309,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 +427,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 +442,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 +518,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 +576,21 @@ def test_change_country_removes_state(self):
assert account_settings['country'] is None
assert account_settings['state'] is None

@override_waffle_switch(ENABLE_COUNTRY_DISABLING, active=True)
@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
13 changes: 12 additions & 1 deletion openedx/core/djangoapps/user_authn/config/waffle.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
Waffle flags and switches for user authn.
"""


from edx_toggles.toggles import WaffleSwitch


_WAFFLE_NAMESPACE = 'user_authn'

# .. toggle_name: user_authn.enable_login_using_thirdparty_auth_only
Expand Down Expand Up @@ -32,3 +32,14 @@
ENABLE_PWNED_PASSWORD_API = WaffleSwitch(
f'{_WAFFLE_NAMESPACE}.enable_pwned_password_api', __name__
)

# .. toggle_name: user_authn.enable_country_disabling
# .. toggle_implementation: WaffleSwitch
# .. toggle_default: False
# .. toggle_description: Waffle flag to enable the feature of registration login from specific countries
# .. toggle_use_cases: temporary, open_edx
# .. toggle_creation_date: 2021-09-22
# .. toggle_target_removal_date: 2021-12-31
# .. toggle_warning: When the flag is ON, the feature of disabling registration from specific countries is enabled.
# .. toggle_tickets: VAN-664
ENABLE_COUNTRY_DISABLING = WaffleSwitch(f'{_WAFFLE_NAMESPACE}.enable_country_disabling', __name__)
16 changes: 14 additions & 2 deletions openedx/core/djangoapps/user_authn/views/registration_form.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.user_api import accounts
from openedx.core.djangoapps.user_api.helpers import FormDescription
from openedx.core.djangoapps.user_authn.config.waffle import ENABLE_COUNTRY_DISABLING
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 +299,16 @@ 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.
"""
if ENABLE_COUNTRY_DISABLING.is_enabled():
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 +698,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 +986,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
50 changes: 49 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 @@ -15,6 +15,7 @@
from django.test.client import RequestFactory
from django.test.utils import override_settings
from django.urls import reverse
from edx_toggles.toggles.testutils import override_waffle_switch
from pytz import UTC
from social_django.models import Partial, UserSocialAuth
from testfixtures import LogCapture
Expand Down Expand Up @@ -49,6 +50,7 @@
from openedx.core.djangoapps.user_api.tests.test_constants import SORTED_COUNTRIES
from openedx.core.djangoapps.user_api.tests.test_helpers import TestCaseForm
from openedx.core.djangoapps.user_api.tests.test_views import UserAPITestCase
from openedx.core.djangoapps.user_authn.config.waffle import ENABLE_COUNTRY_DISABLING
from openedx.core.djangolib.testing.utils import CacheIsolationTestCase, skip_unless_lms
from openedx.core.lib.api import test_utils
from common.djangoapps.student.helpers import authenticate_new_user
Expand All @@ -65,6 +67,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 +1559,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 +2471,32 @@ def test_register_error_with_pwned_password(self, emit):
})
assert response.status_code == 400

@override_waffle_switch(ENABLE_COUNTRY_DISABLING, True)
@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 +2604,25 @@ def test_success(self):

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

@override_waffle_switch(ENABLE_COUNTRY_DISABLING, True)
@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
20 changes: 20 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 All @@ -18,6 +20,8 @@
import string
from datetime import datetime

from openedx.core.djangoapps.user_authn.config.waffle import ENABLE_COUNTRY_DISABLING

log = logging.getLogger(__name__)
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}'
Expand Down Expand Up @@ -177,3 +181,19 @@ 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.
"""
if ENABLE_COUNTRY_DISABLING.is_enabled():
for country_code in settings.DISABLED_COUNTRIES:
del countries[country_code]
return countries

0 comments on commit eb7f912

Please sign in to comment.