From 6d365ca1da62fbcf2ac442c258e3aca718214bce Mon Sep 17 00:00:00 2001 From: Talia Date: Fri, 24 Jul 2020 14:45:22 -0400 Subject: [PATCH] fixes for front end saml work and to align with data requirements. --- .../migrations/0002_auto_20200721_1650.py | 33 ++++++++++++++++ common/djangoapps/third_party_auth/models.py | 30 ++++++++++++++ .../tests/test_samlproviderconfig.py | 16 +++++++- .../samlproviderconfig/views.py | 4 +- .../tests/test_samlproviderdata.py | 39 ++++++++++++++++--- .../samlproviderdata/views.py | 8 +++- .../tests/specs/test_testshib.py | 2 + .../views/tests/test_logistration.py | 1 + 8 files changed, 122 insertions(+), 11 deletions(-) create mode 100644 common/djangoapps/third_party_auth/migrations/0002_auto_20200721_1650.py diff --git a/common/djangoapps/third_party_auth/migrations/0002_auto_20200721_1650.py b/common/djangoapps/third_party_auth/migrations/0002_auto_20200721_1650.py new file mode 100644 index 0000000000..b79cf2c814 --- /dev/null +++ b/common/djangoapps/third_party_auth/migrations/0002_auto_20200721_1650.py @@ -0,0 +1,33 @@ +# Generated by Django 2.2.14 on 2020-07-21 16:50 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('third_party_auth', '0002_samlproviderconfig_country'), + ] + + operations = [ + migrations.AlterField( + model_name='samlproviderconfig', + name='send_to_registration_first', + field=models.BooleanField(default=True, help_text='If this option is selected, users will be directed to the registration page immediately after authenticating with the third party instead of the login page.'), + ), + migrations.AlterField( + model_name='samlproviderconfig', + name='skip_email_verification', + field=models.BooleanField(default=True, help_text='If this option is selected, users will not be required to confirm their email, and their account will be activated immediately upon registration.'), + ), + migrations.AlterField( + model_name='samlproviderconfig', + name='skip_hinted_login_dialog', + field=models.BooleanField(default=True, help_text='If this option is enabled, users that visit a "TPA hinted" URL for this provider (e.g. a URL ending with `?tpa_hint=[provider_name]`) will be forwarded directly to the login URL of the provider instead of being first prompted with a login dialog.'), + ), + migrations.AlterField( + model_name='samlproviderconfig', + name='skip_registration_form', + field=models.BooleanField(default=True, help_text='If this option is enabled, users will not be asked to confirm their details (name, email, etc.) during the registration process. Only select this option for trusted providers that are known to provide accurate user information.'), + ), + ] diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index 793337c4db..07c1165cbc 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -625,6 +625,36 @@ class SAMLProviderConfig(ProviderConfig): ), blank=True, ) + skip_hinted_login_dialog = models.BooleanField( + default=True, + help_text=_( + "If this option is enabled, users that visit a \"TPA hinted\" URL for this provider " + "(e.g. a URL ending with `?tpa_hint=[provider_name]`) will be forwarded directly to " + "the login URL of the provider instead of being first prompted with a login dialog." + ), + ) + skip_registration_form = models.BooleanField( + default=True, + help_text=_( + "If this option is enabled, users will not be asked to confirm their details " + "(name, email, etc.) during the registration process. Only select this option " + "for trusted providers that are known to provide accurate user information." + ), + ) + skip_email_verification = models.BooleanField( + default=True, + help_text=_( + "If this option is selected, users will not be required to confirm their " + "email, and their account will be activated immediately upon registration." + ), + ) + send_to_registration_first = models.BooleanField( + default=True, + help_text=_( + "If this option is selected, users will be directed to the registration page " + "immediately after authenticating with the third party instead of the login page." + ), + ) 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 c11296feb9..3d14636d5d 100644 --- a/common/djangoapps/third_party_auth/samlproviderconfig/tests/test_samlproviderconfig.py +++ b/common/djangoapps/third_party_auth/samlproviderconfig/tests/test_samlproviderconfig.py @@ -13,7 +13,7 @@ from rest_framework import status from rest_framework.test import APITestCase from enterprise.models import EnterpriseCustomerIdentityProvider, EnterpriseCustomer -from enterprise.constants import ENTERPRISE_ADMIN_ROLE +from enterprise.constants import ENTERPRISE_ADMIN_ROLE, ENTERPRISE_LEARNER_ROLE from third_party_auth.tests.samlutils import set_jwt_cookie from third_party_auth.models import SAMLProviderConfig from third_party_auth.tests import testutil @@ -218,3 +218,17 @@ class SAMLProviderConfigTests(APITestCase): self.assertEqual(response.status_code, status.HTTP_201_CREATED) provider_config = SAMLProviderConfig.objects.get(slug='test-slug-empty') self.assertEqual(provider_config.country, '') + + def test_unauthenticated_request_is_forbidden(self): + self.client.logout() + urlbase = reverse('saml_provider_config-list') + query_kwargs = {'enterprise_customer_uuid': ENTERPRISE_ID} + url = '{}?{}'.format(urlbase, urlencode(query_kwargs)) + set_jwt_cookie(self.client, self.user, [(ENTERPRISE_LEARNER_ROLE, ENTERPRISE_ID)]) + response = self.client.get(url, format='json') + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + self.client.logout() + set_jwt_cookie(self.client, self.user, [(ENTERPRISE_ADMIN_ROLE, ENTERPRISE_ID_NON_EXISTENT)]) + response = self.client.get(url, format='json') + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) diff --git a/common/djangoapps/third_party_auth/samlproviderconfig/views.py b/common/djangoapps/third_party_auth/samlproviderconfig/views.py index fa41e23174..26de21607e 100644 --- a/common/djangoapps/third_party_auth/samlproviderconfig/views.py +++ b/common/djangoapps/third_party_auth/samlproviderconfig/views.py @@ -57,14 +57,14 @@ class SAMLProviderConfigViewSet(PermissionRequiredMixin, SAMLProviderMixin, view EnterpriseCustomerIdentityProvider, enterprise_customer__uuid=self.requested_enterprise_uuid ) - return SAMLProviderConfig.objects.filter(slug=enterprise_customer_idp.provider_id) + return SAMLProviderConfig.objects.current_set().filter(slug=enterprise_customer_idp.provider_id) @property def requested_enterprise_uuid(self): """ The enterprise customer uuid from request params or post body """ - if self.request.method == "POST": + if self.request.method in ('POST', 'PUT'): uuid_str = self.request.POST.get('enterprise_customer_uuid') if uuid_str is None: raise ParseError('Required enterprise_customer_uuid is missing') diff --git a/common/djangoapps/third_party_auth/samlproviderdata/tests/test_samlproviderdata.py b/common/djangoapps/third_party_auth/samlproviderdata/tests/test_samlproviderdata.py index 254888e741..7ba16ab72c 100644 --- a/common/djangoapps/third_party_auth/samlproviderdata/tests/test_samlproviderdata.py +++ b/common/djangoapps/third_party_auth/samlproviderdata/tests/test_samlproviderdata.py @@ -11,7 +11,7 @@ from rest_framework import status from rest_framework.test import APITestCase from enterprise.models import EnterpriseCustomer, EnterpriseCustomerIdentityProvider -from enterprise.constants import ENTERPRISE_ADMIN_ROLE +from enterprise.constants import ENTERPRISE_ADMIN_ROLE, ENTERPRISE_LEARNER_ROLE from third_party_auth.tests import testutil from third_party_auth.models import SAMLProviderData, SAMLProviderConfig @@ -39,6 +39,7 @@ SINGLE_PROVIDER_DATA_2['entity_id'] = 'http://entity-id-2' SINGLE_PROVIDER_DATA_2['sso_url'] = 'http://test2.url' ENTERPRISE_ID = str(uuid4()) +BAD_ENTERPRISE_ID = str(uuid4()) @unittest.skipUnless(testutil.AUTH_FEATURE_ENABLED, testutil.AUTH_FEATURES_KEY + ' not enabled') @@ -67,7 +68,7 @@ class SAMLProviderDataTests(APITestCase): fetched_at=SINGLE_PROVIDER_DATA['fetched_at'] ) cls.enterprise_customer_idp, _ = EnterpriseCustomerIdentityProvider.objects.get_or_create( - provider_id=cls.saml_provider_config.id, + provider_id=cls.saml_provider_config.slug, enterprise_customer_id=ENTERPRISE_ID ) @@ -141,12 +142,12 @@ class SAMLProviderDataTests(APITestCase): def test_delete_one_provider_data(self): # DELETE auth/saml/v0/providerdata/ -d data - url = reverse('saml_provider_data-detail', kwargs={'pk': self.saml_provider_data.id}) - data = {} - data['enterprise_customer_uuid'] = ENTERPRISE_ID + url_base = reverse('saml_provider_data-detail', kwargs={'pk': self.saml_provider_data.id}) + query_kwargs = {'enterprise_customer_uuid': ENTERPRISE_ID} + url = '{}?{}'.format(url_base, urlencode(query_kwargs)) orig_count = SAMLProviderData.objects.count() - response = self.client.delete(url, data) + response = self.client.delete(url) self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) self.assertEqual(SAMLProviderData.objects.count(), orig_count - 1) @@ -154,3 +155,29 @@ class SAMLProviderDataTests(APITestCase): # ensure only the sso_url was updated query_set_count = SAMLProviderData.objects.filter(pk=self.saml_provider_data.id).count() self.assertEqual(query_set_count, 0) + + def test_get_one_provider_data_failure(self): + set_jwt_cookie(self.client, self.user, [(ENTERPRISE_ADMIN_ROLE, BAD_ENTERPRISE_ID)]) + self.client.force_authenticate(user=self.user) + url_base = reverse('saml_provider_data-list') + query_kwargs = {'enterprise_customer_uuid': BAD_ENTERPRISE_ID} + url = '{}?{}'.format(url_base, urlencode(query_kwargs)) + + response = self.client.get(url, format='json') + + self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND) + + def test_unauthenticated_request_is_forbidden(self): + self.client.logout() + urlbase = reverse('saml_provider_data-list') + query_kwargs = {'enterprise_customer_uuid': ENTERPRISE_ID} + url = '{}?{}'.format(urlbase, urlencode(query_kwargs)) + set_jwt_cookie(self.client, self.user, [(ENTERPRISE_LEARNER_ROLE, ENTERPRISE_ID)]) + response = self.client.get(url, format='json') + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + # manually running second case as DDT is having issues. + self.client.logout() + set_jwt_cookie(self.client, self.user, [(ENTERPRISE_ADMIN_ROLE, BAD_ENTERPRISE_ID)]) + response = self.client.get(url, format='json') + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) diff --git a/common/djangoapps/third_party_auth/samlproviderdata/views.py b/common/djangoapps/third_party_auth/samlproviderdata/views.py index 33b9151601..e569cfe4a1 100644 --- a/common/djangoapps/third_party_auth/samlproviderdata/views.py +++ b/common/djangoapps/third_party_auth/samlproviderdata/views.py @@ -3,6 +3,7 @@ """ from django.shortcuts import get_object_or_404 +from django.http import Http404 from edx_rbac.mixins import PermissionRequiredMixin from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from rest_framework import permissions, viewsets @@ -52,7 +53,10 @@ class SAMLProviderDataViewSet(PermissionRequiredMixin, SAMLProviderDataMixin, vi EnterpriseCustomerIdentityProvider, enterprise_customer__uuid=self.requested_enterprise_uuid ) - saml_provider = SAMLProviderConfig.objects.get(pk=enterprise_customer_idp.provider_id) + try: + saml_provider = SAMLProviderConfig.objects.current_set().get(slug=enterprise_customer_idp.provider_id) + except SAMLProviderConfig.DoesNotExist: + raise Http404('No matching SAML provider found.') return SAMLProviderData.objects.filter(entity_id=saml_provider.entity_id) @property @@ -60,7 +64,7 @@ class SAMLProviderDataViewSet(PermissionRequiredMixin, SAMLProviderDataMixin, vi """ The enterprise customer uuid from request params or post body """ - if self.request.method in ('POST', 'PATCH', 'DELETE'): + if self.request.method in ('POST', 'PATCH'): uuid_str = self.request.POST.get('enterprise_customer_uuid') if uuid_str is None: raise ParseError('Required enterprise_customer_uuid is missing') diff --git a/common/djangoapps/third_party_auth/tests/specs/test_testshib.py b/common/djangoapps/third_party_auth/tests/specs/test_testshib.py index a4a154f822..342f313ccb 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_testshib.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_testshib.py @@ -109,6 +109,8 @@ class SamlIntegrationTestUtilities(object): kwargs.setdefault('icon_class', 'fa-university') kwargs.setdefault('attr_email', 'urn:oid:1.3.6.1.4.1.5923.1.1.1.6') # eduPersonPrincipalName kwargs.setdefault('max_session_length', None) + kwargs.setdefault('send_to_registration_first', False) + kwargs.setdefault('skip_email_verification', False) saml_provider = self.configure_saml_provider(**kwargs) # pylint: disable=no-member if fetch_metadata: diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py b/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py index e99e6afe51..fbdab0bf1a 100644 --- a/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py +++ b/openedx/core/djangoapps/user_authn/views/tests/test_logistration.py @@ -274,6 +274,7 @@ class LoginAndRegistrationTest(ThirdPartyAuthTestMixin, UrlResetMixin, ModuleSto kwargs.setdefault('icon_class', 'fa-university') kwargs.setdefault('attr_email', 'dummy-email-attr') kwargs.setdefault('max_session_length', None) + kwargs.setdefault('skip_registration_form', False) self.configure_saml_provider(**kwargs) @mock.patch('django.conf.settings.MESSAGE_STORAGE', 'django.contrib.messages.storage.cookie.CookieStorage')