From 2e2493f99d95f7f37160b627cd4593f9e192870b Mon Sep 17 00:00:00 2001 From: Alexander Sheehan Date: Tue, 21 Jul 2020 15:07:12 -0400 Subject: [PATCH 1/2] ENH adding country to SAML mapping config --- .../0002_samlproviderconfig_country.py | 18 ++++++++++++++++++ common/djangoapps/third_party_auth/models.py | 7 +++++++ .../tests/test_samlproviderconfig.py | 8 ++++++-- 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 common/djangoapps/third_party_auth/migrations/0002_samlproviderconfig_country.py diff --git a/common/djangoapps/third_party_auth/migrations/0002_samlproviderconfig_country.py b/common/djangoapps/third_party_auth/migrations/0002_samlproviderconfig_country.py new file mode 100644 index 0000000000..d0bf0b7c64 --- /dev/null +++ b/common/djangoapps/third_party_auth/migrations/0002_samlproviderconfig_country.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.14 on 2020-07-21 19:08 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('third_party_auth', '0001_squashed_0026_auto_20200401_1932'), + ] + + operations = [ + migrations.AddField( + model_name='samlproviderconfig', + name='country', + field=models.CharField(blank=True, help_text=('String representation of the mapping between user`s IDP and edx`s country field.',), max_length=255), + ), + ] diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index 8edc0d9d2d..2f19ac6b4f 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -618,6 +618,13 @@ class SAMLProviderConfig(ProviderConfig): "This is helpful for testing/setup but should always be disabled before users start using this provider." ), ) + country = models.CharField( + max_length=255, + help_text=( + u'String representation of the mapping between user`s IDP and edx`s country field.', + ), + blank=True, + ) other_settings = models.TextField( verbose_name=u"Advanced settings", blank=True, help_text=( diff --git a/common/djangoapps/third_party_auth/samlproviderconfig/tests/test_samlproviderconfig.py b/common/djangoapps/third_party_auth/samlproviderconfig/tests/test_samlproviderconfig.py index 22b4cedfe5..a9bb6e6ab7 100644 --- a/common/djangoapps/third_party_auth/samlproviderconfig/tests/test_samlproviderconfig.py +++ b/common/djangoapps/third_party_auth/samlproviderconfig/tests/test_samlproviderconfig.py @@ -23,7 +23,8 @@ SINGLE_PROVIDER_CONFIG = { 'metadata_source': 'http://test.url', 'name': 'name-of-config', 'enabled': 'true', - 'slug': 'test-slug' + 'slug': 'test-slug', + 'country': 'https:://example.customer.com/countrycode' } SINGLE_PROVIDER_CONFIG_2 = copy.copy(SINGLE_PROVIDER_CONFIG) @@ -54,7 +55,8 @@ class SAMLProviderConfigTests(APITestCase): cls.samlproviderconfig, _ = SAMLProviderConfig.objects.get_or_create( entity_id=SINGLE_PROVIDER_CONFIG['entity_id'], metadata_source=SINGLE_PROVIDER_CONFIG['metadata_source'], - slug=SINGLE_PROVIDER_CONFIG['slug'] + slug=SINGLE_PROVIDER_CONFIG['slug'], + country=SINGLE_PROVIDER_CONFIG['country'], ) def setUp(self): @@ -82,6 +84,7 @@ class SAMLProviderConfigTests(APITestCase): self.assertEqual(len(results), 1) self.assertEqual(results[0]['entity_id'], SINGLE_PROVIDER_CONFIG['entity_id']) self.assertEqual(results[0]['metadata_source'], SINGLE_PROVIDER_CONFIG['metadata_source']) + self.assertEqual(response.data['results'][0]['country'], SINGLE_PROVIDER_CONFIG['country']) self.assertEqual(SAMLProviderConfig.objects.count(), 1) def test_get_one_config_by_enterprise_uuid_invalid_uuid(self): @@ -131,6 +134,7 @@ class SAMLProviderConfigTests(APITestCase): self.assertEqual(SAMLProviderConfig.objects.count(), orig_count + 1) provider_config = SAMLProviderConfig.objects.get(slug=SINGLE_PROVIDER_CONFIG_2['slug']) self.assertEqual(provider_config.name, 'name-of-config-2') + self.assertEqual(provider_config.country, SINGLE_PROVIDER_CONFIG_2['country']) # check association has also been created self.assertTrue( From 0e5b70a800f85d1295ed3973d66c691df2d8407d Mon Sep 17 00:00:00 2001 From: Alexander Sheehan Date: Wed, 22 Jul 2020 18:47:27 -0400 Subject: [PATCH 2/2] Adding country to list of attrs, updating help text and max length --- .../0002_samlproviderconfig_country.py | 4 +- common/djangoapps/third_party_auth/models.py | 6 +-- .../tests/test_samlproviderconfig.py | 42 ++++++++++++++++++- 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/common/djangoapps/third_party_auth/migrations/0002_samlproviderconfig_country.py b/common/djangoapps/third_party_auth/migrations/0002_samlproviderconfig_country.py index d0bf0b7c64..2037dcad85 100644 --- a/common/djangoapps/third_party_auth/migrations/0002_samlproviderconfig_country.py +++ b/common/djangoapps/third_party_auth/migrations/0002_samlproviderconfig_country.py @@ -1,4 +1,4 @@ -# Generated by Django 2.2.14 on 2020-07-21 19:08 +# Generated by Django 2.2.14 on 2020-07-22 22:45 from django.db import migrations, models @@ -13,6 +13,6 @@ class Migration(migrations.Migration): migrations.AddField( model_name='samlproviderconfig', name='country', - field=models.CharField(blank=True, help_text=('String representation of the mapping between user`s IDP and edx`s country field.',), max_length=255), + field=models.CharField(blank=True, help_text=('URN of SAML attribute containing the user`s country.',), max_length=128), ), ] diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index 2f19ac6b4f..793337c4db 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -619,9 +619,9 @@ class SAMLProviderConfig(ProviderConfig): ), ) country = models.CharField( - max_length=255, + max_length=128, help_text=( - u'String representation of the mapping between user`s IDP and edx`s country field.', + u'URN of SAML attribute containing the user`s country.', ), blank=True, ) @@ -699,7 +699,7 @@ class SAMLProviderConfig(ProviderConfig): conf = {} attrs = ( 'attr_user_permanent_id', 'attr_full_name', 'attr_first_name', - 'attr_last_name', 'attr_username', 'attr_email', 'entity_id') + 'attr_last_name', 'attr_username', 'attr_email', 'entity_id', 'country') attr_defaults = { 'attr_full_name': 'default_full_name', 'attr_first_name': 'default_first_name', diff --git a/common/djangoapps/third_party_auth/samlproviderconfig/tests/test_samlproviderconfig.py b/common/djangoapps/third_party_auth/samlproviderconfig/tests/test_samlproviderconfig.py index a9bb6e6ab7..c11296feb9 100644 --- a/common/djangoapps/third_party_auth/samlproviderconfig/tests/test_samlproviderconfig.py +++ b/common/djangoapps/third_party_auth/samlproviderconfig/tests/test_samlproviderconfig.py @@ -18,13 +18,14 @@ from third_party_auth.tests.samlutils import set_jwt_cookie from third_party_auth.models import SAMLProviderConfig from third_party_auth.tests import testutil +# country here refers to the URN provided by a user's IDP SINGLE_PROVIDER_CONFIG = { 'entity_id': 'id', 'metadata_source': 'http://test.url', 'name': 'name-of-config', 'enabled': 'true', 'slug': 'test-slug', - 'country': 'https:://example.customer.com/countrycode' + 'country': 'https://example.customer.com/countrycode' } SINGLE_PROVIDER_CONFIG_2 = copy.copy(SINGLE_PROVIDER_CONFIG) @@ -178,3 +179,42 @@ class SAMLProviderConfigTests(APITestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(SAMLProviderConfig.objects.count(), orig_count) + + def test_create_one_config_with_no_country_urn(self): + """ + POST auth/saml/v0/provider_config/ -d data + """ + url = reverse('saml_provider_config-list') + provider_config_no_country = { + 'entity_id': 'id', + 'metadata_source': 'http://test.url', + 'name': 'name-of-config-no-country', + 'enabled': 'true', + 'slug': 'test-slug-none', + 'enterprise_customer_uuid': ENTERPRISE_ID, + } + + response = self.client.post(url, provider_config_no_country) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + provider_config = SAMLProviderConfig.objects.get(slug='test-slug-none') + self.assertEqual(provider_config.country, '') + + def test_create_one_config_with_empty_country_urn(self): + """ + POST auth/saml/v0/provider_config/ -d data + """ + url = reverse('saml_provider_config-list') + provider_config_blank_country = { + 'entity_id': 'id', + 'metadata_source': 'http://test.url', + 'name': 'name-of-config-blank-country', + 'enabled': 'true', + 'slug': 'test-slug-empty', + 'enterprise_customer_uuid': ENTERPRISE_ID, + 'country': '', + } + + response = self.client.post(url, provider_config_blank_country) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + provider_config = SAMLProviderConfig.objects.get(slug='test-slug-empty') + self.assertEqual(provider_config.country, '')