From 033bcda99d6532a28fb2eba3a6d1428a9f4a2179 Mon Sep 17 00:00:00 2001 From: Hassan Raza Date: Wed, 19 Mar 2025 12:22:45 +0500 Subject: [PATCH] Hraza/add embargo restricted country (#36398) * feat: add country disabling feature in embargo app * revert: disabled countries list in env * fix: resolved linter issues --------- Co-authored-by: Hassan Raza --- cms/envs/common.py | 7 -- lms/envs/common.py | 9 --- openedx/core/djangoapps/embargo/admin.py | 17 +++- ...003_add_global_restricted_country_model.py | 25 ++++++ openedx/core/djangoapps/embargo/models.py | 75 ++++++++++++++++++ .../core/djangoapps/user_api/accounts/api.py | 11 ++- .../user_api/accounts/tests/test_api.py | 15 ++-- .../user_authn/views/registration_form.py | 34 ++++---- .../user_authn/views/tests/test_register.py | 79 ++++++++++++++----- .../core/djangoapps/user_authn/views/utils.py | 6 +- 10 files changed, 214 insertions(+), 64 deletions(-) create mode 100644 openedx/core/djangoapps/embargo/migrations/0003_add_global_restricted_country_model.py diff --git a/cms/envs/common.py b/cms/envs/common.py index 6f49a86236..7666903041 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -2919,13 +2919,6 @@ MEILISEARCH_PUBLIC_URL = "http://meilisearch.example.com" 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 = [] - # .. setting_name: LIBRARY_ENABLED_BLOCKS # .. setting_default: ['problem', 'video', 'html', 'drag-and-drop-v2'] # .. setting_description: List of block types that are ready/enabled to be created/used diff --git a/lms/envs/common.py b/lms/envs/common.py index 59a6790780..07f489662e 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -5545,15 +5545,6 @@ SURVEY_REPORT_CHECK_THRESHOLD = 6 # .. 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 = [] - - LMS_COMM_DEFAULT_FROM_EMAIL = "no-reply@example.com" diff --git a/openedx/core/djangoapps/embargo/admin.py b/openedx/core/djangoapps/embargo/admin.py index b86409b079..fa96317ac5 100644 --- a/openedx/core/djangoapps/embargo/admin.py +++ b/openedx/core/djangoapps/embargo/admin.py @@ -8,7 +8,7 @@ from config_models.admin import ConfigurationModelAdmin from django.contrib import admin from .forms import IPFilterForm, RestrictedCourseForm -from .models import CountryAccessRule, IPFilter, RestrictedCourse +from .models import CountryAccessRule, GlobalRestrictedCountry, IPFilter, RestrictedCourse class IPFilterAdmin(ConfigurationModelAdmin): @@ -41,5 +41,20 @@ class RestrictedCourseAdmin(admin.ModelAdmin): search_fields = ('course_key',) +class GlobalRestrictedCountryAdmin(admin.ModelAdmin): + """ + Admin configuration for the Global Country Restriction model. + """ + list_display = ("country",) + + def delete_queryset(self, request, queryset): + """ + Override the delete_queryset method to clear the cache when objects are deleted in bulk. + """ + super().delete_queryset(request, queryset) + GlobalRestrictedCountry.update_cache() + + admin.site.register(IPFilter, IPFilterAdmin) admin.site.register(RestrictedCourse, RestrictedCourseAdmin) +admin.site.register(GlobalRestrictedCountry, GlobalRestrictedCountryAdmin) diff --git a/openedx/core/djangoapps/embargo/migrations/0003_add_global_restricted_country_model.py b/openedx/core/djangoapps/embargo/migrations/0003_add_global_restricted_country_model.py new file mode 100644 index 0000000000..d9a422202c --- /dev/null +++ b/openedx/core/djangoapps/embargo/migrations/0003_add_global_restricted_country_model.py @@ -0,0 +1,25 @@ +# Generated by Django 4.2.18 on 2025-01-29 08:19 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('embargo', '0002_data__add_countries'), + ] + + operations = [ + migrations.CreateModel( + name='GlobalRestrictedCountry', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('country', models.ForeignKey(help_text='The country to be restricted.', on_delete=django.db.models.deletion.CASCADE, to='embargo.country', unique=True)), + ], + options={ + 'verbose_name': 'Global Restricted Country', + 'verbose_name_plural': 'Global Restricted Countries', + }, + ), + ] diff --git a/openedx/core/djangoapps/embargo/models.py b/openedx/core/djangoapps/embargo/models.py index 3e72699404..b3a452079c 100644 --- a/openedx/core/djangoapps/embargo/models.py +++ b/openedx/core/djangoapps/embargo/models.py @@ -662,6 +662,81 @@ class CourseAccessRuleHistory(models.Model): get_latest_by = 'timestamp' +class GlobalRestrictedCountry(models.Model): + """ + Model to restrict access to specific countries globally. + """ + country = models.ForeignKey( + "Country", + help_text="The country to be restricted.", + on_delete=models.CASCADE, + unique=True + ) + + CACHE_KEY = "embargo.global.restricted_countries" + + @classmethod + def get_countries(cls): + """ + Retrieve the set of restricted country codes from the cache or refresh it if not available. + + Returns: + set: A set of restricted country codes. + """ + return cache.get_or_set(cls.CACHE_KEY, cls._fetch_restricted_countries) + + @classmethod + def is_country_restricted(cls, country_code): + """ + Check if the given country code is restricted. + + Args: + country_code (str): The country code to check. + + Returns: + bool: True if the country is restricted, False otherwise. + """ + return country_code in cls.get_countries() + + @classmethod + def _fetch_restricted_countries(cls): + """ + Fetch the set of restricted country codes from the database. + + Returns: + set: A set of restricted country codes. + """ + return set(cls.objects.values_list("country__country", flat=True)) + + @classmethod + def update_cache(cls): + """ + Update the cache with the latest restricted country codes. + """ + cache.set(cls.CACHE_KEY, cls._fetch_restricted_countries()) + + def save(self, *args, **kwargs): + """ + Override save method to update cache on insert/update. + """ + super().save(*args, **kwargs) + self.update_cache() + + def delete(self, *args, **kwargs): + """ + Override delete method to update cache on deletion. + """ + super().delete(*args, **kwargs) + self.update_cache() + + def __str__(self): + return f"{self.country.country.name} ({self.country.country})" + + class Meta: + verbose_name = "Global Restricted Country" + verbose_name_plural = "Global Restricted Countries" + + # Connect the signals to the receivers so we record a history # of changes to the course access rules. post_save.connect(CourseAccessRuleHistory.snapshot_post_save_receiver, sender=RestrictedCourse) diff --git a/openedx/core/djangoapps/user_api/accounts/api.py b/openedx/core/djangoapps/user_api/accounts/api.py index 6cc466ba00..6970ea6f85 100644 --- a/openedx/core/djangoapps/user_api/accounts/api.py +++ b/openedx/core/djangoapps/user_api/accounts/api.py @@ -9,10 +9,11 @@ import re from django.conf import settings from django.core.exceptions import ObjectDoesNotExist from django.core.validators import ValidationError, validate_email -from django.utils.translation import override as override_language from django.utils.translation import gettext as _ +from django.utils.translation import override as override_language from eventtracking import tracker from pytz import UTC + from common.djangoapps.student import views as student_views from common.djangoapps.student.models import ( AccountRecovery, @@ -25,7 +26,7 @@ from common.djangoapps.util.model_utils import emit_settings_changed_event from common.djangoapps.util.password_policy_validators import validate_password from lms.djangoapps.certificates.api import get_certificates_for_user from lms.djangoapps.certificates.data import CertificateStatuses - +from openedx.core.djangoapps.embargo.models import GlobalRestrictedCountry from openedx.core.djangoapps.enrollments.api import get_verified_enrollments from openedx.core.djangoapps.user_api import accounts, errors, helpers from openedx.core.djangoapps.user_api.errors import ( @@ -39,6 +40,7 @@ from openedx.core.djangoapps.user_authn.views.registration_form import validate_ from openedx.core.lib.api.view_utils import add_serializer_errors 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 name_affirmation_installed = is_name_affirmation_installed() @@ -151,7 +153,10 @@ 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: + if ( + settings.FEATURES.get('EMBARGO', False) and + GlobalRestrictedCountry.is_country_restricted(update.get('country', '')) + ): field_errors['country'] = { 'developer_message': 'Country is disabled for registration', 'user_message': 'This country cannot be selected for user registration' diff --git a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py index 5123c4cf41..f9071c06a5 100644 --- a/openedx/core/djangoapps/user_api/accounts/tests/test_api.py +++ b/openedx/core/djangoapps/user_api/accounts/tests/test_api.py @@ -7,18 +7,19 @@ import datetime import itertools import unicodedata from unittest.mock import Mock, patch -import pytest + import ddt +import pytest from django.conf import settings from django.contrib.auth.hashers import make_password from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user 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 + from common.djangoapps.student.models import ( AccountRecovery, PendingEmailChange, @@ -28,14 +29,14 @@ from common.djangoapps.student.models import ( from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.student.tests.tests import UserSettingsEventTestMixin from common.djangoapps.student.views.management import activate_secondary_email - from lms.djangoapps.certificates.data import CertificateStatuses from openedx.core.djangoapps.ace_common.tests.mixins import EmailTemplateTagMixin +from openedx.core.djangoapps.embargo.models import Country, GlobalRestrictedCountry from openedx.core.djangoapps.user_api.accounts import PRIVATE_VISIBILITY from openedx.core.djangoapps.user_api.accounts.api import ( get_account_settings, - update_account_settings, - get_name_validation_error + get_name_validation_error, + update_account_settings ) from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import ( # pylint: disable=unused-import RetirementTestCase, @@ -574,12 +575,14 @@ class TestAccountApi(UserSettingsEventTestMixin, EmailTemplateTagMixin, CreateAc 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 + country = Country.objects.create(country="KP") + GlobalRestrictedCountry.objects.create(country=country) + 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 diff --git a/openedx/core/djangoapps/user_authn/views/registration_form.py b/openedx/core/djangoapps/user_authn/views/registration_form.py index 978d303c9a..efee92e700 100644 --- a/openedx/core/djangoapps/user_authn/views/registration_form.py +++ b/openedx/core/djangoapps/user_authn/views/registration_form.py @@ -3,9 +3,8 @@ Objects and utilities used to construct registration forms. """ import copy -from importlib import import_module -from eventtracking import tracker import re +from importlib import import_module from django import forms from django.conf import settings @@ -16,26 +15,25 @@ from django.forms import widgets from django.urls import reverse from django.utils.translation import gettext as _ from django_countries import countries +from eventtracking import tracker from common.djangoapps import third_party_auth from common.djangoapps.edxmako.shortcuts import marketing_link -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.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 ( - CourseEnrollmentAllowed, - UserProfile, - email_exists_or_retired, -) +from common.djangoapps.student.models import CourseEnrollmentAllowed, UserProfile, email_exists_or_retired from common.djangoapps.util.password_policy_validators import ( password_validators_instruction_texts, password_validators_restrictions, - validate_password, + validate_password ) +from openedx.core.djangoapps.embargo.models import GlobalRestrictedCountry +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.utils import check_pwned_password +from openedx.core.djangoapps.user_authn.utils import 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 class TrueCheckbox(widgets.CheckboxInput): @@ -306,7 +304,10 @@ class AccountCreationForm(forms.Form): Check if the user's country is in the embargoed countries list. """ country = self.cleaned_data.get("country") - if country in settings.DISABLED_COUNTRIES: + if ( + settings.FEATURES.get('EMBARGO', False) and + country in GlobalRestrictedCountry.get_countries() + ): raise ValidationError(_("Registration from this country is not allowed due to restrictions.")) return self.cleaned_data.get("country") @@ -981,7 +982,6 @@ class RegistrationFormFactory: 'country', default=default_country.upper() ) - form_desc.add_field( "country", label=country_label, 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 77b5d074fb..5c6aa226f9 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_register.py @@ -2,8 +2,8 @@ import json from datetime import datetime -from unittest import skipIf, skipUnless -from unittest import mock +from unittest import mock, skipIf, skipUnless +from unittest.mock import patch import ddt import httpretty @@ -15,11 +15,26 @@ from django.test import TransactionTestCase from django.test.client import RequestFactory from django.test.utils import override_settings from django.urls import reverse +from openedx_events.tests.utils import OpenEdxEventsTestMixin from pytz import UTC from social_django.models import Partial, UserSocialAuth from testfixtures import LogCapture -from openedx_events.tests.utils import OpenEdxEventsTestMixin +from common.djangoapps.student.helpers import authenticate_new_user +from common.djangoapps.student.tests.factories import AccountRecoveryFactory, UserFactory +from common.djangoapps.third_party_auth.tests.testutil import ThirdPartyAuthTestMixin, simulate_running_pipeline +from common.djangoapps.third_party_auth.tests.utils import ( + ThirdPartyOAuthTestMixin, + ThirdPartyOAuthTestMixinFacebook, + ThirdPartyOAuthTestMixinGoogle +) +from common.djangoapps.util.password_policy_validators import ( + DEFAULT_MAX_PASSWORD_LENGTH, + create_validator_config, + password_validators_instruction_texts, + password_validators_restrictions +) +from openedx.core.djangoapps.embargo.models import Country, GlobalRestrictedCountry from openedx.core.djangoapps.site_configuration.helpers import get_value from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration from openedx.core.djangoapps.user_api.accounts import ( @@ -51,20 +66,6 @@ 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.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 -from common.djangoapps.student.tests.factories import AccountRecoveryFactory, UserFactory -from common.djangoapps.third_party_auth.tests.testutil import ThirdPartyAuthTestMixin, simulate_running_pipeline -from common.djangoapps.third_party_auth.tests.utils import ( - ThirdPartyOAuthTestMixin, - ThirdPartyOAuthTestMixinFacebook, - ThirdPartyOAuthTestMixinGoogle -) -from common.djangoapps.util.password_policy_validators import ( - DEFAULT_MAX_PASSWORD_LENGTH, - create_validator_config, - password_validators_instruction_texts, - password_validators_restrictions -) ENABLE_AUTO_GENERATED_USERNAME = settings.FEATURES.copy() ENABLE_AUTO_GENERATED_USERNAME['ENABLE_AUTO_GENERATED_USERNAME'] = True @@ -2493,11 +2494,13 @@ class RegistrationViewTestV2(RegistrationViewTestV1): }) assert response.status_code == 400 - @override_settings(DISABLED_COUNTRIES=['KP']) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_register_with_disabled_country(self): """ Test case to check user registration is forbidden when registration is disabled for a country """ + country = Country.objects.create(country="KP") + GlobalRestrictedCountry.objects.create(country=country) response = self.client.post(self.url, { "email": self.EMAIL, "name": self.NAME, @@ -2518,6 +2521,26 @@ class RegistrationViewTestV2(RegistrationViewTestV1): ], 'error_code': 'validation-error'} ) + @patch.dict(settings.FEATURES, {'EMBARGO': False}) + def test_registration_allowed_when_embargo_disabled(self): + """ + Ensures that user registration proceeds normally even for restricted countries + when the EMBARGO feature flag is disabled. + """ + country = Country.objects.create(country="KP") + GlobalRestrictedCountry.objects.create(country=country) + + response = self.client.post(self.url, { + "email": self.EMAIL, + "name": self.NAME, + "username": self.USERNAME, + "password": self.PASSWORD, + "honor_code": "true", + "country": "KP", + }) + + self.assertEqual(response.status_code, 200) + @httpretty.activate @ddt.ddt @@ -2625,13 +2648,15 @@ class ThirdPartyRegistrationTestMixin( self._verify_user_existence(user_exists=True, social_link_exists=True, user_is_active=False) - @override_settings(DISABLED_COUNTRIES=['US']) + @patch.dict(settings.FEATURES, {'EMBARGO': True}) def test_with_disabled_country(self): """ - Test case to check user registration is forbidden when registration is disabled for a country + Test case to check user registration is forbidden when registration is restricted for a country """ self._verify_user_existence(user_exists=False, social_link_exists=False) self._setup_provider_response(success=True) + country_obj = Country.objects.create(country="US") + GlobalRestrictedCountry.objects.create(country=country_obj) response = self.client.post(self.url, self.data()) assert response.status_code == 400 assert response.json() == { @@ -2643,6 +2668,20 @@ class ThirdPartyRegistrationTestMixin( } self._verify_user_existence(user_exists=False, social_link_exists=False, user_is_active=False) + @patch.dict(settings.FEATURES, {'EMBARGO': False}) + def test_with_disabled_country_when_embargo_disabled(self): + """ + Ensures that user registration proceeds normally even for restricted countries + when the EMBARGO feature flag is disabled. + """ + self._verify_user_existence(user_exists=False, social_link_exists=False) + self._setup_provider_response(success=True) + country_obj = Country.objects.create(country="US") + GlobalRestrictedCountry.objects.create(country=country_obj) + response = self.client.post(self.url, self.data()) + assert response.status_code == 200 + self._verify_user_existence(user_exists=True, social_link_exists=True, user_is_active=False) + def test_unlinked_active_user(self): user = UserFactory() response = self.client.post(self.url, self.data(user)) diff --git a/openedx/core/djangoapps/user_authn/views/utils.py b/openedx/core/djangoapps/user_authn/views/utils.py index b9fb096621..14eb91d38e 100644 --- a/openedx/core/djangoapps/user_authn/views/utils.py +++ b/openedx/core/djangoapps/user_authn/views/utils.py @@ -14,6 +14,7 @@ from text_unidecode import unidecode from common.djangoapps import third_party_auth from common.djangoapps.third_party_auth import pipeline from common.djangoapps.third_party_auth.models import clean_username +from openedx.core.djangoapps.embargo.models import GlobalRestrictedCountry from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.geoinfo.api import country_code_from_ip import random @@ -191,6 +192,9 @@ def remove_disabled_country_from_list(countries: Dict) -> Dict: Returns: - dict: Dict of countries with disabled countries removed. """ - for country_code in settings.DISABLED_COUNTRIES: + if not settings.FEATURES.get("EMBARGO", False): + return countries + + for country_code in GlobalRestrictedCountry.get_countries(): del countries[country_code] return countries