diff --git a/cms/envs/common.py b/cms/envs/common.py index d1c3b78af1..a96310a774 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -315,7 +315,6 @@ FEATURES = { 'ENABLE_GRADE_DOWNLOADS': True, 'ENABLE_MKTG_SITE': False, 'ENABLE_DISCUSSION_HOME_PANEL': True, - 'ENABLE_COMBINED_LOGIN_REGISTRATION': True, 'ENABLE_CORS_HEADERS': False, 'ENABLE_CROSS_DOMAIN_CSRF_COOKIE': False, 'ENABLE_COUNTRY_ACCESS': False, diff --git a/cms/envs/test.py b/cms/envs/test.py index bc43fb0510..35bc42324c 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -222,8 +222,6 @@ FEATURES['ENABLE_SERVICE_STATUS'] = True # Toggles embargo on for testing FEATURES['EMBARGO'] = True -FEATURES['ENABLE_COMBINED_LOGIN_REGISTRATION'] = True - TEST_THEME = COMMON_ROOT / "test" / "test-theme" # For consistency in user-experience, keep the value of this setting in sync with diff --git a/common/djangoapps/student/forms.py b/common/djangoapps/student/forms.py index 80c49c6c42..c980357e98 100644 --- a/common/djangoapps/student/forms.py +++ b/common/djangoapps/student/forms.py @@ -395,8 +395,6 @@ def get_registration_extension_form(*args, **kwargs): An example form app for this can be found at http://github.com/open-craft/custom-form-app """ - if not settings.FEATURES.get("ENABLE_COMBINED_LOGIN_REGISTRATION"): - return None if not getattr(settings, 'REGISTRATION_EXTENSION_FORM', None): return None module, klass = settings.REGISTRATION_EXTENSION_FORM.rsplit('.', 1) diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index f123691393..09cb91c379 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -220,40 +220,6 @@ def check_verify_status_by_course(user, course_enrollments): return status_by_course -def auth_pipeline_urls(auth_entry, redirect_url=None): - """Retrieve URLs for each enabled third-party auth provider. - - These URLs are used on the "sign up" and "sign in" buttons - on the login/registration forms to allow users to begin - authentication with a third-party provider. - - Optionally, we can redirect the user to an arbitrary - url after auth completes successfully. We use this - to redirect the user to a page that required login, - or to send users to the payment flow when enrolling - in a course. - - Args: - auth_entry (string): Either `pipeline.AUTH_ENTRY_LOGIN` or `pipeline.AUTH_ENTRY_REGISTER` - - Keyword Args: - redirect_url (unicode): If provided, send users to this URL - after they successfully authenticate. - - Returns: - dict mapping provider IDs to URLs - - """ - if not third_party_auth.is_enabled(): - return {} - - return { - provider.provider_id: third_party_auth.pipeline.get_login_url( - provider.provider_id, auth_entry, redirect_url=redirect_url - ) for provider in third_party_auth.provider.Registry.displayed_for_login() - } - - # Query string parameters that can be passed to the "finish_auth" view to manage # things like auto-enrollment. POST_AUTH_PARAMS = ('course_id', 'enrollment_action', 'course_mode', 'email_opt_in', 'purchase_workflow') diff --git a/common/djangoapps/student/tests/test_long_username_email.py b/common/djangoapps/student/tests/test_long_username_email.py index 351a186eaa..aba7ffc68b 100644 --- a/common/djangoapps/student/tests/test_long_username_email.py +++ b/common/djangoapps/student/tests/test_long_username_email.py @@ -37,13 +37,13 @@ class TestLongUsernameEmail(TestCase): obj = json.loads(response.content.decode('utf-8')) self.assertEqual( - obj['value'], + obj['username'][0]['user_message'], USERNAME_BAD_LENGTH_MSG, ) def test_spoffed_name(self): """ - Test name cannot contains html. + Test name cannot contain html. """ self.url_params['name'] = '


Name
Content spoof' response = self.client.post(self.url, self.url_params) @@ -65,6 +65,6 @@ class TestLongUsernameEmail(TestCase): obj = json.loads(response.content.decode('utf-8')) self.assertEqual( - obj['value'], + obj['email'][0]['user_message'], "Email cannot be more than 254 characters long", ) diff --git a/common/djangoapps/student/tests/test_password_policy.py b/common/djangoapps/student/tests/test_password_policy.py index 022707a2ab..6432cd79a7 100644 --- a/common/djangoapps/student/tests/test_password_policy.py +++ b/common/djangoapps/student/tests/test_password_policy.py @@ -14,7 +14,6 @@ from django.urls import reverse from mock import patch from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory -from openedx.core.djangoapps.user_authn.views.deprecated import create_account from util.password_policy_validators import create_validator_config @@ -43,7 +42,7 @@ class TestPasswordPolicy(TestCase): self.assertEqual(response.status_code, 400) obj = json.loads(response.content.decode('utf-8')) self.assertEqual( - obj['value'], + obj['password'][0]['user_message'], "This password is too short. It must contain at least 6 characters.", ) @@ -66,7 +65,7 @@ class TestPasswordPolicy(TestCase): self.assertEqual(response.status_code, 400) obj = json.loads(response.content.decode('utf-8')) self.assertEqual( - obj['value'], + obj['password'][0]['user_message'], "This password is too long. It must contain no more than 12 characters.", ) @@ -79,7 +78,7 @@ class TestPasswordPolicy(TestCase): self.assertEqual(response.status_code, 400) obj = json.loads(response.content.decode('utf-8')) self.assertEqual( - obj['value'], + obj['password'][0]['user_message'], "This password must contain at least 3 uppercase letters.", ) @@ -102,7 +101,7 @@ class TestPasswordPolicy(TestCase): self.assertEqual(response.status_code, 400) obj = json.loads(response.content.decode('utf-8')) self.assertEqual( - obj['value'], + obj['password'][0]['user_message'], "This password must contain at least 3 lowercase letters.", ) @@ -125,7 +124,7 @@ class TestPasswordPolicy(TestCase): self.assertEqual(response.status_code, 400) obj = json.loads(response.content.decode('utf-8')) self.assertEqual( - obj['value'], + obj['password'][0]['user_message'], "This password must contain at least 3 punctuation marks.", ) @@ -149,7 +148,7 @@ class TestPasswordPolicy(TestCase): self.assertEqual(response.status_code, 400) obj = json.loads(response.content.decode('utf-8')) self.assertEqual( - obj['value'], + obj['password'][0]['user_message'], "This password must contain at least 3 numbers.", ) @@ -173,7 +172,7 @@ class TestPasswordPolicy(TestCase): self.assertEqual(response.status_code, 400) obj = json.loads(response.content.decode('utf-8')) self.assertEqual( - obj['value'], + obj['password'][0]['user_message'], "This password must contain at least 3 letters.", ) @@ -198,12 +197,13 @@ class TestPasswordPolicy(TestCase): response = self.client.post(self.url, self.url_params) self.assertEqual(response.status_code, 400) obj = json.loads(response.content.decode('utf-8')) - errstring = ( - "This password must contain at least 3 uppercase letters. " - "This password must contain at least 3 numbers. " - "This password must contain at least 3 punctuation marks." - ) - self.assertEqual(obj['value'], errstring) + error_strings = [ + "This password must contain at least 3 uppercase letters.", + "This password must contain at least 3 numbers.", + "This password must contain at least 3 punctuation marks.", + ] + for i in range(3): + self.assertEqual(obj['password'][i]['user_message'], error_strings[i]) @override_settings(AUTH_PASSWORD_VALIDATORS=[ create_validator_config('util.password_policy_validators.MinimumLengthValidator', {'min_length': 3}), @@ -228,7 +228,7 @@ class TestPasswordPolicy(TestCase): self.assertEqual(response.status_code, 400) obj = json.loads(response.content.decode('utf-8')) self.assertEqual( - obj['value'], + obj['password'][0]['user_message'], "This password is too common.", ) @@ -280,7 +280,7 @@ class TestUsernamePasswordNonmatch(TestCase): self.assertEquals(response.status_code, 400) obj = json.loads(response.content.decode('utf-8')) self.assertEqual( - obj['value'], + obj['password'][0]['user_message'], "The password is too similar to the username.", ) diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index fecd47629d..6f2608a85e 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -275,7 +275,7 @@ class ProviderConfig(ConfigurationModel): def get_register_form_data(cls, pipeline_kwargs): """Gets dict of data to display on the register form. - openedx.core.djangoapps.user_authn.views.deprecated.register_user uses this to populate + register_user uses this to populate the new account creation form with values supplied by the user's chosen provider, preventing duplicate data entry. diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index a1a54a52bf..6e29ff62f8 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -21,8 +21,9 @@ from social_django import utils as social_utils from social_django import views as social_views from lms.djangoapps.commerce.tests import TEST_API_URL -from openedx.core.djangoapps.user_authn.views.deprecated import signin_user, create_account, register_user +from openedx.core.djangoapps.user_api.views import RegistrationView from openedx.core.djangoapps.user_authn.views.login import login_user +from openedx.core.djangoapps.user_authn.views.login_form import login_and_registration_form from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory from openedx.core.djangoapps.user_api.accounts.settings_views import account_settings_context from student import models as student_models @@ -32,6 +33,10 @@ from third_party_auth import middleware, pipeline from third_party_auth.tests import testutil +def create_account(request): + return RegistrationView().post(request) + + class HelperMixin(object): """ Contains helper methods for IntegrationTestMixin and IntegrationTest classes below. @@ -65,14 +70,18 @@ class HelperMixin(object): # Check that the correct provider was selected. self.assertContains( response, - u'successfully signed in with %s' % self.provider.name, + u'"errorMessage": null' + ) + self.assertContains( + response, + u'"currentProvider": "{}"'.format(self.provider.name), ) # Expect that each truthy value we've prepopulated the register form # with is actually present. form_field_data = self.provider.get_register_form_data(pipeline_kwargs) for prepopulated_form_data in form_field_data: if prepopulated_form_data in required_fields: - self.assertIn(form_field_data[prepopulated_form_data], response.content.decode('utf-8')) + self.assertContains(response, form_field_data[prepopulated_form_data]) # pylint: disable=invalid-name def assert_account_settings_context_looks_correct(self, context, duplicate=False, linked=None): @@ -129,17 +138,18 @@ class HelperMixin(object): def assert_json_failure_response_is_username_collision(self, response): """Asserts the json response indicates a username collision.""" - self.assertEqual(400, response.status_code) + self.assertEqual(409, response.status_code) payload = json.loads(response.content.decode('utf-8')) self.assertFalse(payload.get('success')) - self.assertIn('belongs to an existing account', payload.get('value')) + self.assertIn('belongs to an existing account', payload['username'][0]['user_message']) - def assert_json_success_response_looks_correct(self, response): + def assert_json_success_response_looks_correct(self, response, verify_redirect_url): """Asserts the json response indicates success and redirection.""" self.assertEqual(200, response.status_code) payload = json.loads(response.content.decode('utf-8')) self.assertTrue(payload.get('success')) - self.assertEqual(pipeline.get_complete_url(self.provider.backend_name), payload.get('redirect_url')) + if verify_redirect_url: + self.assertEqual(pipeline.get_complete_url(self.provider.backend_name), payload.get('redirect_url')) def assert_login_response_before_pipeline_looks_correct(self, response): """Asserts a GET of /login not in the pipeline looks correct.""" @@ -285,7 +295,6 @@ class HelperMixin(object): """Creates user, profile, registration, and (usually) social auth. This synthesizes what happens during /register. - See student.views.register and student.helpers.do_create_account. """ response_data = self.get_response_data() uid = strategy.request.backend.get_user_id(response_data, response_data) @@ -541,7 +550,6 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): actions.do_complete(request.backend, social_views._do_login, # pylint: disable=protected-access request=request) - signin_user(strategy.request) login_user(strategy.request) actions.do_complete(request.backend, social_views._do_login, # pylint: disable=protected-access request=request) @@ -598,7 +606,6 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): request=request) with self._patch_edxmako_current_request(strategy.request): - signin_user(strategy.request) login_user(strategy.request) actions.do_complete(request.backend, social_views._do_login, user=user, # pylint: disable=protected-access request=request) @@ -665,7 +672,6 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): request=request) with self._patch_edxmako_current_request(strategy.request): - signin_user(strategy.request) login_user(strategy.request) actions.do_complete(request.backend, social_views._do_login, # pylint: disable=protected-access user=user, request=request) @@ -710,12 +716,12 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): # At this point we know the pipeline has resumed correctly. Next we # fire off the view that displays the login form and posts it via JS. with self._patch_edxmako_current_request(strategy.request): - self.assert_login_response_in_pipeline_looks_correct(signin_user(strategy.request)) + self.assert_login_response_in_pipeline_looks_correct(login_user(strategy.request)) # Next, we invoke the view that handles the POST, and expect it # redirects to /auth/complete. In the browser ajax handlers will # redirect the user to the dashboard; we invoke it manually here. - self.assert_json_success_response_looks_correct(login_user(strategy.request)) + self.assert_json_success_response_looks_correct(login_user(strategy.request), verify_redirect_url=True) # We should be redirected back to the complete page, setting # the "logged in" cookie for the marketing site. @@ -806,7 +812,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): # fire off the view that displays the registration form. with self._patch_edxmako_current_request(request): self.assert_register_response_in_pipeline_looks_correct( - register_user(strategy.request), + login_and_registration_form(strategy.request, initial_mode='register'), pipeline.get(request)['kwargs'], ['name', 'username', 'email'] ) @@ -828,7 +834,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): # ...but when we invoke create_account the existing edX view will make # it, but not social auths. The pipeline creates those later. with self._patch_edxmako_current_request(strategy.request): - self.assert_json_success_response_looks_correct(create_account(strategy.request)) + self.assert_json_success_response_looks_correct(create_account(strategy.request), verify_redirect_url=False) # We've overridden the user's password, so authenticate() with the old # value won't work: created_user = self.get_user_by_email(strategy, email) @@ -881,7 +887,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase, HelperMixin): with self._patch_edxmako_current_request(request): self.assert_register_response_in_pipeline_looks_correct( - register_user(strategy.request), + login_and_registration_form(strategy.request, initial_mode='register'), pipeline.get(request)['kwargs'], ['name', 'username', 'email'] ) 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 df4a195017..33e5fe5409 100644 --- a/common/djangoapps/third_party_auth/tests/specs/test_testshib.py +++ b/common/djangoapps/third_party_auth/tests/specs/test_testshib.py @@ -22,7 +22,6 @@ from social_django.models import UserSocialAuth from testfixtures import LogCapture from enterprise.models import EnterpriseCustomerIdentityProvider, EnterpriseCustomerUser -from openedx.core.djangoapps.user_authn.views.deprecated import signin_user from openedx.core.djangoapps.user_authn.views.login import login_user from openedx.core.djangoapps.user_api.accounts.settings_views import account_settings_context from openedx.features.enterprise_support.tests.factories import EnterpriseCustomerFactory @@ -210,7 +209,6 @@ class TestShibIntegrationTest(SamlIntegrationTestUtilities, IntegrationTestMixin request=request) with self._patch_edxmako_current_request(strategy.request): - signin_user(strategy.request) login_user(strategy.request) actions.do_complete(request.backend, social_views._do_login, user=user, # pylint: disable=protected-access request=request) diff --git a/common/test/acceptance/pages/lms/login_and_register.py b/common/test/acceptance/pages/lms/login_and_register.py index f0da365b93..8cb855994f 100644 --- a/common/test/acceptance/pages/lms/login_and_register.py +++ b/common/test/acceptance/pages/lms/login_and_register.py @@ -110,10 +110,6 @@ class ResetPasswordPage(PageObject): class CombinedLoginAndRegisterPage(PageObject): """Interact with combined login and registration page. - This page is currently hidden behind the feature flag - `ENABLE_COMBINED_LOGIN_REGISTRATION`, which is enabled - in the bok choy settings. - When enabled, the new page is available from either `/login` or `/register`; the new page is also served at `/account/login/` or `/account/register/`, where it was diff --git a/lms/envs/bok_choy.yml b/lms/envs/bok_choy.yml index f41199d4af..c90d3c4221 100644 --- a/lms/envs/bok_choy.yml +++ b/lms/envs/bok_choy.yml @@ -79,7 +79,7 @@ EVENT_TRACKING_BACKENDS: OPTIONS: {collection: events, database: test} FEATURES: {ALLOW_AUTOMATED_SIGNUPS: true, AUTH_USE_OPENID_PROVIDER: true, AUTOMATIC_AUTH_FOR_TESTING: true, AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING: true, CERTIFICATES_HTML_VIEW: true, - CERTIFICATES_INSTRUCTOR_GENERATION: true, CUSTOM_COURSES_EDX: true, ENABLE_COMBINED_LOGIN_REGISTRATION: true, + CERTIFICATES_INSTRUCTOR_GENERATION: true, CUSTOM_COURSES_EDX: true, ENABLE_COURSE_DISCOVERY: true, ENABLE_DISCUSSION_SERVICE: true, ENABLE_GRADE_DOWNLOADS: true, ENABLE_PAYMENT_FAKE: true, ENABLE_SPECIAL_EXAMS: true, ENABLE_THIRD_PARTY_AUTH: true, ENABLE_VERIFIED_CERTIFICATES: true, EXPOSE_CACHE_PROGRAMS_ENDPOINT: true, MODE_CREATION_FOR_TESTING: true, diff --git a/lms/envs/bok_choy_docker.yml b/lms/envs/bok_choy_docker.yml index 4f136cef69..f1765060b1 100644 --- a/lms/envs/bok_choy_docker.yml +++ b/lms/envs/bok_choy_docker.yml @@ -83,7 +83,7 @@ EVENT_TRACKING_BACKENDS: port: 27017 FEATURES: {ALLOW_AUTOMATED_SIGNUPS: true, AUTH_USE_OPENID_PROVIDER: true, AUTOMATIC_AUTH_FOR_TESTING: true, AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING: true, CERTIFICATES_HTML_VIEW: true, - CERTIFICATES_INSTRUCTOR_GENERATION: true, CUSTOM_COURSES_EDX: true, ENABLE_COMBINED_LOGIN_REGISTRATION: true, + CERTIFICATES_INSTRUCTOR_GENERATION: true, CUSTOM_COURSES_EDX: true, ENABLE_COURSE_DISCOVERY: true, ENABLE_DISCUSSION_SERVICE: true, ENABLE_GRADE_DOWNLOADS: true, ENABLE_PAYMENT_FAKE: true, ENABLE_SPECIAL_EXAMS: true, ENABLE_THIRD_PARTY_AUTH: true, ENABLE_VERIFIED_CERTIFICATES: true, EXPOSE_CACHE_PROGRAMS_ENDPOINT: true, MODE_CREATION_FOR_TESTING: true, diff --git a/lms/envs/common.py b/lms/envs/common.py index d609330876..6c806b9199 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -262,8 +262,6 @@ FEATURES = { # ENABLE_OAUTH2_PROVIDER to True 'ENABLE_MOBILE_REST_API': False, - # Enable the combined login/registration form - 'ENABLE_COMBINED_LOGIN_REGISTRATION': False, 'ENABLE_COMBINED_LOGIN_REGISTRATION_FOOTER': False, # Enable organizational email opt-in diff --git a/lms/envs/devstack_docker.py b/lms/envs/devstack_docker.py index 8e76e74fc4..d88a7cb384 100644 --- a/lms/envs/devstack_docker.py +++ b/lms/envs/devstack_docker.py @@ -39,7 +39,6 @@ FEATURES.update({ 'ENABLE_DISCUSSION_SERVICE': True, 'SHOW_HEADER_LANGUAGE_SELECTOR': True, 'ENABLE_ENTERPRISE_INTEGRATION': False, - 'ENABLE_COMBINED_LOGIN_REGISTRATION': True, }) ENABLE_MKTG_SITE = os.environ.get('ENABLE_MARKETING_SITE', False) diff --git a/lms/envs/test.py b/lms/envs/test.py index 32a5307d28..a2fed948e6 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -82,8 +82,6 @@ FEATURES['ENABLE_VERIFIED_CERTIFICATES'] = True # Toggles embargo on for testing FEATURES['EMBARGO'] = True -FEATURES['ENABLE_COMBINED_LOGIN_REGISTRATION'] = True - # Enable the milestones app in tests to be consistent with it being enabled in production FEATURES['MILESTONES_APP'] = True diff --git a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py index e633a07a4d..00ff48005e 100644 --- a/openedx/core/djangoapps/lang_pref/tests/test_middleware.py +++ b/openedx/core/djangoapps/lang_pref/tests/test_middleware.py @@ -188,24 +188,14 @@ class TestUserPreferenceMiddleware(CacheIsolationTestCase): # Use an actual call to the login endpoint, to validate that the middleware # stack does the right thing - if settings.FEATURES.get('ENABLE_COMBINED_LOGIN_REGISTRATION'): - response = self.client.post( - reverse('user_api_login_session'), - data={ - 'email': self.user.email, - 'password': UserFactory._DEFAULT_PASSWORD, - 'remember': True, - } - ) - else: - response = self.client.post( - reverse('login_post'), - data={ - 'email': self.user.email, - 'password': UserFactory._DEFAULT_PASSWORD, - 'honor_code': True, - } - ) + response = self.client.post( + reverse('user_api_login_session'), + data={ + 'email': self.user.email, + 'password': UserFactory._DEFAULT_PASSWORD, # pylint: disable=protected-access + 'remember': True, + } + ) self.assertEqual(response.status_code, 200) diff --git a/openedx/core/djangoapps/user_api/legacy_urls.py b/openedx/core/djangoapps/user_api/legacy_urls.py index 77ddd36ed8..e1d12bd515 100644 --- a/openedx/core/djangoapps/user_api/legacy_urls.py +++ b/openedx/core/djangoapps/user_api/legacy_urls.py @@ -37,12 +37,11 @@ urlpatterns = [ ), ] -if settings.FEATURES.get('ENABLE_COMBINED_LOGIN_REGISTRATION'): - urlpatterns += [ - url(r'^v1/account/login_session/$', user_api_views.LoginSessionView.as_view(), - name="user_api_login_session"), - url(r'^v1/account/registration/$', user_api_views.RegistrationView.as_view(), - name="user_api_registration"), - url(r'^v1/account/password_reset/$', user_api_views.PasswordResetView.as_view(), - name="user_api_password_reset"), - ] +urlpatterns += [ + url(r'^v1/account/login_session/$', user_api_views.LoginSessionView.as_view(), + name="user_api_login_session"), + url(r'^v1/account/registration/$', user_api_views.RegistrationView.as_view(), + name="user_api_registration"), + url(r'^v1/account/password_reset/$', user_api_views.PasswordResetView.as_view(), + name="user_api_password_reset"), +] diff --git a/openedx/core/djangoapps/user_api/tests/test_views.py b/openedx/core/djangoapps/user_api/tests/test_views.py index 5fd642343b..b1d6666f20 100644 --- a/openedx/core/djangoapps/user_api/tests/test_views.py +++ b/openedx/core/djangoapps/user_api/tests/test_views.py @@ -825,9 +825,10 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa }) self.assertEqual(response.status_code, 400) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, { + "success": False, "email": [{ "user_message": ( u"It looks like {} belongs to an existing account. " @@ -867,9 +868,10 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa }) self.assertEqual(response.status_code, 409) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, { + "success": False, "email": [{ "user_message": ( u"It looks like {} belongs to an existing account. " @@ -909,9 +911,10 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa }) self.assertEqual(response.status_code, 409) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, { + "success": False, "username": [{ "user_message": ( u"It looks like {} belongs to an existing account. " @@ -946,9 +949,10 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa }) self.assertEqual(response.status_code, 400) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, { + "success": False, "email": [{ "user_message": ( u"It looks like {} belongs to an existing account. " @@ -983,9 +987,10 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa }) self.assertEqual(response.status_code, 409) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, { + u"success": False, u"username": [{ u"user_message": ( u"An account with the Public Username '{}' already exists." @@ -1019,9 +1024,10 @@ class RegistrationViewValidationErrorTest(ThirdPartyAuthTestMixin, UserAPITestCa }) self.assertEqual(response.status_code, 400) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, { + "success": False, "email": [{ "user_message": ( u"It looks like {} belongs to an existing account. " @@ -2182,9 +2188,10 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): }) self.assertEqual(response.status_code, 409) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, { + "success": False, "email": [{ "user_message": ( u"It looks like {} belongs to an existing account. " @@ -2217,9 +2224,10 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): }) self.assertEqual(response.status_code, 409) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, { + "success": False, "username": [{ "user_message": ( u"It looks like {} belongs to an existing account. " @@ -2252,9 +2260,10 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): }) self.assertEqual(response.status_code, 409) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, { + "success": False, "username": [{ "user_message": ( u"It looks like {} belongs to an existing account. " @@ -2295,9 +2304,10 @@ class RegistrationViewTest(ThirdPartyAuthTestMixin, UserAPITestCase): ) self.assertEqual(response.status_code, 400) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, { + u"success": False, u"username": [{u"user_message": USERNAME_BAD_LENGTH_MSG}], u"password": [{u"user_message": u"This field is required."}], } @@ -2454,18 +2464,24 @@ class ThirdPartyRegistrationTestMixin(ThirdPartyOAuthTestMixin, CacheIsolationTe """Assert that the given response was an error for the access_token field with the given error message.""" self.assertEqual(response.status_code, 400) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, - {"access_token": [{"user_message": expected_error_message}]} + { + "success": False, + "access_token": [{"user_message": expected_error_message}], + } ) def _assert_third_party_session_expired_error(self, response, expected_error_message): """Assert that given response is an error due to third party session expiry""" self.assertEqual(response.status_code, 400) response_json = json.loads(response.content.decode('utf-8')) - self.assertEqual( + self.assertDictEqual( response_json, - {"session_expired": [{"user_message": expected_error_message}]} + { + "success": False, + "session_expired": [{"user_message": expected_error_message}], + } ) def _verify_user_existence(self, user_exists, social_link_exists, user_is_active=None, username=None): diff --git a/openedx/core/djangoapps/user_api/views.py b/openedx/core/djangoapps/user_api/views.py index b947e0fcf4..d4f674ed67 100644 --- a/openedx/core/djangoapps/user_api/views.py +++ b/openedx/core/djangoapps/user_api/views.py @@ -98,12 +98,18 @@ class LoginSessionView(APIView): class RegistrationView(APIView): + # pylint: disable=missing-docstring """HTTP end-points for creating a new user. """ # This end-point is available to anonymous users, # so do not require authentication. authentication_classes = [] + @method_decorator(transaction.non_atomic_requests) + @method_decorator(sensitive_post_parameters("password")) + def dispatch(self, request, *args, **kwargs): + return super(RegistrationView, self).dispatch(request, *args, **kwargs) + @method_decorator(ensure_csrf_cookie) def get(self, request): return HttpResponse(RegistrationFormFactory().get_registration_form(request).to_json(), @@ -129,11 +135,25 @@ class RegistrationView(APIView): HttpResponse: 403 operation not allowed """ data = request.POST.copy() + self._handle_terms_of_service(data) + response = self._handle_duplicate_email_username(data) + if response: + return response + + response, user = self._create_account(request, data) + if response: + return response + + response = self._create_response({}, status_code=200) + set_logged_in_cookies(request, response, user) + return response + + def _handle_duplicate_email_username(self, data): + # TODO Verify whether this check is needed here - it may be duplicated in user_api. email = data.get('email') username = data.get('username') - # Handle duplicate email/username conflicts = check_account_exists(email=email, username=username) if conflicts: conflict_messages = { @@ -144,8 +164,9 @@ class RegistrationView(APIView): field: [{"user_message": conflict_messages[field]}] for field in conflicts } - return JsonResponse(errors, status=409) + return self._create_response(errors, status_code=409) + def _handle_terms_of_service(self, data): # Backwards compatibility: the student view expects both # terms of service and honor code values. Since we're combining # these into a single checkbox, the only value we may get @@ -156,33 +177,32 @@ class RegistrationView(APIView): if data.get("honor_code") and "terms_of_service" not in data: data["terms_of_service"] = data["honor_code"] + def _create_account(self, request, data): + response, user = None, None try: user = create_account_with_params(request, data) except AccountValidationError as err: errors = { err.field: [{"user_message": text_type(err)}] } - return JsonResponse(errors, status=409) + response = self._create_response(errors, status_code=409) except ValidationError as err: - # Should only get non-field errors from this function + # Should only get field errors from this exception assert NON_FIELD_ERRORS not in err.message_dict # Only return first error for each field errors = { field: [{"user_message": error} for error in error_list] for field, error_list in err.message_dict.items() } - return JsonResponse(errors, status=400) + response = self._create_response(errors, status_code=400) except PermissionDenied: - return HttpResponseForbidden(_("Account creation not allowed.")) + response = HttpResponseForbidden(_("Account creation not allowed.")) - response = JsonResponse({"success": True}) - set_logged_in_cookies(request, response, user) - return response + return response, user - @method_decorator(transaction.non_atomic_requests) - @method_decorator(sensitive_post_parameters("password")) - def dispatch(self, request, *args, **kwargs): - return super(RegistrationView, self).dispatch(request, *args, **kwargs) + def _create_response(self, response_dict, status_code): + response_dict['success'] = (status_code == 200) + return JsonResponse(response_dict, status=status_code) class PasswordResetView(APIView): diff --git a/openedx/core/djangoapps/user_authn/urls.py b/openedx/core/djangoapps/user_authn/urls.py index a278f3355e..5d7a4aece6 100644 --- a/openedx/core/djangoapps/user_authn/urls.py +++ b/openedx/core/djangoapps/user_authn/urls.py @@ -6,7 +6,7 @@ from django.conf.urls import include, url from openedx.core.djangoapps.user_api.accounts import settings_views -from .views import deprecated, login, login_form +from .views import login, login_form urlpatterns = [ # TODO this should really be declared in the user_api app @@ -18,17 +18,10 @@ urlpatterns = [ ] -if settings.FEATURES.get('ENABLE_COMBINED_LOGIN_REGISTRATION'): - # Backwards compatibility with old URL structure, but serve the new views - urlpatterns += [ - url(r'^login$', login_form.login_and_registration_form, - {'initial_mode': 'login'}, name='signin_user'), - url(r'^register$', login_form.login_and_registration_form, - {'initial_mode': 'register'}, name='register_user'), - ] -else: - # Serve the old views - urlpatterns += [ - url(r'^login$', deprecated.signin_user, name='signin_user'), - url(r'^register$', deprecated.register_user, name='register_user'), - ] +# Backwards compatibility with old URL structure, but serve the new views +urlpatterns += [ + url(r'^login$', login_form.login_and_registration_form, + {'initial_mode': 'login'}, name='signin_user'), + url(r'^register$', login_form.login_and_registration_form, + {'initial_mode': 'register'}, name='register_user'), +] diff --git a/openedx/core/djangoapps/user_authn/urls_common.py b/openedx/core/djangoapps/user_authn/urls_common.py index b20698e8b0..ab3fa911c8 100644 --- a/openedx/core/djangoapps/user_authn/urls_common.py +++ b/openedx/core/djangoapps/user_authn/urls_common.py @@ -11,10 +11,12 @@ from __future__ import absolute_import from django.conf import settings from django.conf.urls import url -from .views import auto_auth, deprecated, login, logout +from openedx.core.djangoapps.user_api.views import RegistrationView +from .views import auto_auth, login, logout + urlpatterns = [ - url(r'^create_account$', deprecated.create_account, name='create_account'), + url(r'^create_account$', RegistrationView.as_view(), name='create_account'), url(r'^login_post$', login.login_user, name='login_post'), url(r'^login_ajax$', login.login_user, name="login"), url(r'^login_ajax/(?P[^/]*)$', login.login_user), diff --git a/openedx/core/djangoapps/user_authn/views/deprecated.py b/openedx/core/djangoapps/user_authn/views/deprecated.py deleted file mode 100644 index af4f0160a1..0000000000 --- a/openedx/core/djangoapps/user_authn/views/deprecated.py +++ /dev/null @@ -1,150 +0,0 @@ -""" User Authn code for deprecated views. """ -from __future__ import absolute_import - -import warnings - -from django.conf import settings -from django.contrib import messages -from django.core.validators import ValidationError -from django.db import transaction -from django.http import HttpResponseForbidden -from django.shortcuts import redirect -from django.utils.translation import ugettext as _ -from django.views.decorators.csrf import csrf_exempt, ensure_csrf_cookie -from six import iteritems, text_type - -import third_party_auth -from edxmako.shortcuts import render_to_response -from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, SYSTEM_MAINTENANCE_MSG, waffle -from openedx.core.djangoapps.user_authn.cookies import set_logged_in_cookies -from openedx.core.djangoapps.user_authn.views.register import create_account_with_params -from student.helpers import AccountValidationError, auth_pipeline_urls, get_next_url_for_login_page -from third_party_auth import pipeline, provider -from util.json_request import JsonResponse - - -@ensure_csrf_cookie -def signin_user(request): - """Deprecated. To be replaced by :class:`user_authn.views.login_form.login_and_registration_form`.""" - # Determine the URL to redirect to following login: - redirect_to = get_next_url_for_login_page(request) - if request.user.is_authenticated: - return redirect(redirect_to) - - third_party_auth_error = None - for msg in messages.get_messages(request): - if msg.extra_tags.split()[0] == "social-auth": - # msg may or may not be translated. Try translating [again] in case we are able to: - third_party_auth_error = _(text_type(msg)) - break - - context = { - 'login_redirect_url': redirect_to, # This gets added to the query string of the "Sign In" button in the header - # Bool injected into JS to submit form if we're inside a running third- - # party auth pipeline; distinct from the actual instance of the running - # pipeline, if any. - 'pipeline_running': 'true' if pipeline.running(request) else 'false', - 'pipeline_url': auth_pipeline_urls(pipeline.AUTH_ENTRY_LOGIN, redirect_url=redirect_to), - 'platform_name': configuration_helpers.get_value( - 'platform_name', - settings.PLATFORM_NAME - ), - 'third_party_auth_error': third_party_auth_error - } - - return render_to_response('login.html', context) - - -@ensure_csrf_cookie -def register_user(request, extra_context=None): - """ - Deprecated. To be replaced by :class:`user_authn.views.login_form.login_and_registration_form`. - """ - # Determine the URL to redirect to following login: - redirect_to = get_next_url_for_login_page(request) - if request.user.is_authenticated: - return redirect(redirect_to) - - context = { - 'login_redirect_url': redirect_to, # This gets added to the query string of the "Sign In" button in the header - 'email': '', - 'name': '', - 'running_pipeline': None, - 'pipeline_urls': auth_pipeline_urls(pipeline.AUTH_ENTRY_REGISTER, redirect_url=redirect_to), - 'platform_name': configuration_helpers.get_value( - 'platform_name', - settings.PLATFORM_NAME - ), - 'selected_provider': '', - 'username': '', - } - - if extra_context is not None: - context.update(extra_context) - - if context.get("extauth_domain", '').startswith(settings.SHIBBOLETH_DOMAIN_PREFIX): - return render_to_response('register-shib.html', context) - - # If third-party auth is enabled, prepopulate the form with data from the - # selected provider. - if third_party_auth.is_enabled() and pipeline.running(request): - running_pipeline = pipeline.get(request) - current_provider = provider.Registry.get_from_pipeline(running_pipeline) - if current_provider is not None: - overrides = current_provider.get_register_form_data(running_pipeline.get('kwargs')) - overrides['running_pipeline'] = running_pipeline - overrides['selected_provider'] = current_provider.name - context.update(overrides) - - return render_to_response('register.html', context) - - -@csrf_exempt -@transaction.non_atomic_requests -def create_account(request, post_override=None): - """ - Deprecated. Use RegistrationView instead. - JSON call to create new edX account. - Used by form in signup_modal.html, which is included into header.html - """ - # Check if ALLOW_PUBLIC_ACCOUNT_CREATION flag turned off to restrict user account creation - if not configuration_helpers.get_value( - 'ALLOW_PUBLIC_ACCOUNT_CREATION', - settings.FEATURES.get('ALLOW_PUBLIC_ACCOUNT_CREATION', True) - ): - return HttpResponseForbidden(_("Account creation not allowed.")) - - if waffle().is_enabled(PREVENT_AUTH_USER_WRITES): - return HttpResponseForbidden(SYSTEM_MAINTENANCE_MSG) - - warnings.warn("Please use RegistrationView instead.", DeprecationWarning) - - try: - user = create_account_with_params(request, post_override or request.POST) - except AccountValidationError as exc: - return JsonResponse({'success': False, 'value': text_type(exc), 'field': exc.field}, status=400) - except ValidationError as exc: - field, error_list = next(iteritems(exc.message_dict)) - return JsonResponse( - { - "success": False, - "field": field, - "value": ' '.join(error_list), - }, - status=400 - ) - - redirect_url = None # The AJAX method calling should know the default destination upon success - - # Resume the third-party-auth pipeline if necessary. - if third_party_auth.is_enabled() and pipeline.running(request): - running_pipeline = pipeline.get(request) - redirect_url = pipeline.get_complete_url(running_pipeline['backend']) - - response = JsonResponse({ - 'success': True, - 'redirect_url': redirect_url, - }) - set_logged_in_cookies(request, response, user) - return response diff --git a/openedx/core/djangoapps/user_authn/views/login_form.py b/openedx/core/djangoapps/user_authn/views/login_form.py index b6187c6874..0ca063ad67 100644 --- a/openedx/core/djangoapps/user_authn/views/login_form.py +++ b/openedx/core/djangoapps/user_authn/views/login_form.py @@ -17,7 +17,6 @@ from django.views.decorators.http import require_http_methods import third_party_auth from edxmako.shortcuts import render_to_response from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -from openedx.core.djangoapps.theming.helpers import is_request_in_themed_site from openedx.core.djangoapps.user_api.accounts.utils import is_secondary_email_feature_enabled from openedx.core.djangoapps.user_api.api import ( RegistrationFormFactory, @@ -25,8 +24,6 @@ from openedx.core.djangoapps.user_api.api import ( get_password_reset_form ) from openedx.core.djangoapps.user_authn.cookies import are_logged_in_cookies_set -from openedx.core.djangoapps.user_authn.views.deprecated import register_user as old_register_view -from openedx.core.djangoapps.user_authn.views.deprecated import signin_user as old_login_view from openedx.features.enterprise_support.api import enterprise_customer_for_request from openedx.features.enterprise_support.utils import ( handle_enterprise_cookies_for_logistration, @@ -71,31 +68,25 @@ def login_and_registration_form(request, initial_mode="login"): if '?' in redirect_to: try: next_args = six.moves.urllib.parse.parse_qs(six.moves.urllib.parse.urlparse(redirect_to).query) - provider_id = next_args['tpa_hint'][0] - tpa_hint_provider = third_party_auth.provider.Registry.get(provider_id=provider_id) - if tpa_hint_provider: - if tpa_hint_provider.skip_hinted_login_dialog: - # Forward the user directly to the provider's login URL when the provider is configured - # to skip the dialog. - if initial_mode == "register": - auth_entry = pipeline.AUTH_ENTRY_REGISTER - else: - auth_entry = pipeline.AUTH_ENTRY_LOGIN - return redirect( - pipeline.get_login_url(provider_id, auth_entry, redirect_url=redirect_to) - ) - third_party_auth_hint = provider_id - initial_mode = "hinted_login" + if 'tpa_hint' in next_args: + provider_id = next_args['tpa_hint'][0] + tpa_hint_provider = third_party_auth.provider.Registry.get(provider_id=provider_id) + if tpa_hint_provider: + if tpa_hint_provider.skip_hinted_login_dialog: + # Forward the user directly to the provider's login URL when the provider is configured + # to skip the dialog. + if initial_mode == "register": + auth_entry = pipeline.AUTH_ENTRY_REGISTER + else: + auth_entry = pipeline.AUTH_ENTRY_LOGIN + return redirect( + pipeline.get_login_url(provider_id, auth_entry, redirect_url=redirect_to) + ) + third_party_auth_hint = provider_id + initial_mode = "hinted_login" except (KeyError, ValueError, IndexError) as ex: log.exception(u"Unknown tpa_hint provider: %s", ex) - # We are defaulting to true because all themes should now be using the newer page. - if is_request_in_themed_site() and not configuration_helpers.get_value('ENABLE_COMBINED_LOGIN_REGISTRATION', True): - if initial_mode == "login": - return old_login_view(request) - elif initial_mode == "register": - return old_register_view(request) - # Account activation message account_activation_messages = [ { diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_login_registration_forms.py b/openedx/core/djangoapps/user_authn/views/tests/test_login_registration_forms.py deleted file mode 100644 index 3b8e0661b7..0000000000 --- a/openedx/core/djangoapps/user_authn/views/tests/test_login_registration_forms.py +++ /dev/null @@ -1,237 +0,0 @@ -"""Tests for the login and registration form rendering. """ -from __future__ import absolute_import - -import unittest - -import ddt -import six -import six.moves.urllib.error # pylint: disable=import-error -import six.moves.urllib.parse # pylint: disable=import-error -import six.moves.urllib.request # pylint: disable=import-error -from django.conf import settings -from django.urls import reverse -from mock import patch - -from openedx.core.djangolib.js_utils import js_escaped_string -from third_party_auth.tests.testutil import ThirdPartyAuthTestMixin -from util.testing import UrlResetMixin -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory - -# This relies on third party auth being enabled in the test -# settings with the feature flag `ENABLE_THIRD_PARTY_AUTH` -THIRD_PARTY_AUTH_BACKENDS = ["google-oauth2", "facebook"] -THIRD_PARTY_AUTH_PROVIDERS = ["Google", "Facebook"] - - -def _third_party_login_url(backend_name, auth_entry, redirect_url=None): - """Construct the login URL to start third party authentication. """ - params = [("auth_entry", auth_entry)] - if redirect_url: - params.append(("next", redirect_url)) - - return u"{url}?{params}".format( - url=reverse("social:begin", kwargs={"backend": backend_name}), - params=six.moves.urllib.parse.urlencode(params) - ) - - -def _finish_auth_url(params): - """ Construct the URL that follows login/registration if we are doing auto-enrollment """ - return u"{}?{}".format(reverse('finish_auth'), six.moves.urllib.parse.urlencode(params)) - - -@ddt.ddt -@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class LoginFormTest(ThirdPartyAuthTestMixin, UrlResetMixin, SharedModuleStoreTestCase): - """Test rendering of the login form. """ - - URLCONF_MODULES = [ - 'openedx.core.djangoapps.user_authn.urls', - 'openedx.core.djangoapps.user_api.legacy_urls', - ] - - @classmethod - def setUpClass(cls): - super(LoginFormTest, cls).setUpClass() - cls.course = CourseFactory.create() - - @patch.dict(settings.FEATURES, {"ENABLE_COMBINED_LOGIN_REGISTRATION": False}) - def setUp(self): # pylint: disable=arguments-differ - super(LoginFormTest, self).setUp() - - self.url = reverse("signin_user") - self.course_id = six.text_type(self.course.id) - self.courseware_url = reverse("courseware", args=[self.course_id]) - self.configure_google_provider(enabled=True, visible=True) - self.configure_facebook_provider(enabled=True, visible=True) - - @patch.dict(settings.FEATURES, {"ENABLE_THIRD_PARTY_AUTH": False}) - @ddt.data(THIRD_PARTY_AUTH_PROVIDERS) - def test_third_party_auth_disabled(self, provider_name): - response = self.client.get(self.url) - self.assertNotContains(response, provider_name) - - @ddt.data(*THIRD_PARTY_AUTH_BACKENDS) - def test_third_party_auth_no_course_id(self, backend_name): - response = self.client.get(self.url) - expected_url = _third_party_login_url(backend_name, "login") - self.assertContains(response, expected_url) - - @ddt.data(*THIRD_PARTY_AUTH_BACKENDS) - def test_third_party_auth_with_course_id(self, backend_name): - # Provide a course ID to the login page, simulating what happens - # when a user tries to enroll in a course without being logged in - params = [('course_id', self.course_id)] - response = self.client.get(self.url, params) - - # Expect that the course ID is added to the third party auth entry - # point, so that the pipeline will enroll the student and - # redirect the student to the track selection page. - expected_url = _third_party_login_url( - backend_name, - "login", - redirect_url=_finish_auth_url(params), - ) - self.assertContains(response, expected_url) - - @ddt.data(*THIRD_PARTY_AUTH_BACKENDS) - def test_courseware_redirect(self, backend_name): - # Try to access courseware while logged out, expecting to be - # redirected to the login page. - response = self.client.get(self.courseware_url, follow=True, HTTP_ACCEPT="text/html") - self.assertRedirects( - response, - u"{url}?next={redirect_url}".format( - url=reverse("signin_user"), - redirect_url=self.courseware_url - ) - ) - - # Verify that the third party auth URLs include the redirect URL - # The third party auth pipeline will redirect to this page - # once the user successfully authenticates. - expected_url = _third_party_login_url( - backend_name, - "login", - redirect_url=self.courseware_url - ) - self.assertContains(response, expected_url) - - @ddt.data(*THIRD_PARTY_AUTH_BACKENDS) - def test_third_party_auth_with_params(self, backend_name): - params = [ - ('course_id', self.course_id), - ('enrollment_action', 'enroll'), - ('course_mode', 'honor'), - ('email_opt_in', 'true'), - ('next', '/custom/final/destination'), - ] - response = self.client.get(self.url, params, HTTP_ACCEPT="text/html") - expected_url = _third_party_login_url( - backend_name, - "login", - redirect_url=_finish_auth_url(params), - ) - self.assertContains(response, expected_url) - - @ddt.data(None, "true", "false") - def test_params(self, opt_in_value): - params = [ - ('course_id', self.course_id), - ('enrollment_action', 'enroll'), - ('course_mode', 'honor'), - ('email_opt_in', opt_in_value), - ('next', '/custom/final/destination'), - ] - - # Get the login page - response = self.client.get(self.url, params, HTTP_ACCEPT="text/html") - - # Verify that the parameters are sent on to the next page correctly - post_login_handler = _finish_auth_url(params) - js_success_var = u'var nextUrl = "{}";'.format(js_escaped_string(post_login_handler)) - self.assertContains(response, js_success_var) - - # Verify that the login link preserves the querystring params - login_link = u"{url}?{params}".format( - url=reverse('signin_user'), - params=six.moves.urllib.parse.urlencode([('next', post_login_handler)]) - ) - self.assertContains(response, login_link) - - -@ddt.ddt -@unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') -class RegisterFormTest(ThirdPartyAuthTestMixin, UrlResetMixin, SharedModuleStoreTestCase): - """Test rendering of the registration form. """ - - URLCONF_MODULES = ['openedx.core.djangoapps.user_authn.urls'] - - @classmethod - def setUpClass(cls): - super(RegisterFormTest, cls).setUpClass() - cls.course = CourseFactory.create() - - @patch.dict(settings.FEATURES, {"ENABLE_COMBINED_LOGIN_REGISTRATION": False}) - def setUp(self): # pylint: disable=arguments-differ - super(RegisterFormTest, self).setUp() - - self.url = reverse("register_user") - self.course_id = six.text_type(self.course.id) - self.configure_google_provider(enabled=True, visible=True) - self.configure_facebook_provider(enabled=True, visible=True) - - @patch.dict(settings.FEATURES, {"ENABLE_THIRD_PARTY_AUTH": False}) - @ddt.data(*THIRD_PARTY_AUTH_PROVIDERS) - def test_third_party_auth_disabled(self, provider_name): - response = self.client.get(self.url) - self.assertNotContains(response, provider_name) - - @ddt.data(*THIRD_PARTY_AUTH_BACKENDS) - def test_register_third_party_auth_no_course_id(self, backend_name): - response = self.client.get(self.url) - expected_url = _third_party_login_url(backend_name, "register") - self.assertContains(response, expected_url) - - @ddt.data(*THIRD_PARTY_AUTH_BACKENDS) - def test_register_third_party_auth_with_params(self, backend_name): - params = [ - ('course_id', self.course_id), - ('enrollment_action', 'enroll'), - ('course_mode', 'honor'), - ('email_opt_in', 'true'), - ('next', '/custom/final/destination'), - ] - response = self.client.get(self.url, params, HTTP_ACCEPT="text/html") - expected_url = _third_party_login_url( - backend_name, - "register", - redirect_url=_finish_auth_url(params), - ) - self.assertContains(response, expected_url) - - @ddt.data(None, "true", "false") - def test_params(self, opt_in_value): - params = [ - ('course_id', self.course_id), - ('enrollment_action', 'enroll'), - ('course_mode', 'honor'), - ('email_opt_in', opt_in_value), - ('next', '/custom/final/destination'), - ] - - # Get the login page - response = self.client.get(self.url, params, HTTP_ACCEPT="text/html") - - # Verify that the parameters are sent on to the next page correctly - post_login_handler = _finish_auth_url(params) - js_success_var = u'var nextUrl = "{}";'.format(js_escaped_string(post_login_handler)) - self.assertContains(response, js_success_var) - - # Verify that the login link preserves the querystring params - login_link = u"{url}?{params}".format( - url=reverse('signin_user'), - params=six.moves.urllib.parse.urlencode([('next', post_login_handler)]) - ) - self.assertContains(response, login_link) diff --git a/openedx/core/djangoapps/user_authn/views/tests/test_register.py b/openedx/core/djangoapps/user_authn/views/tests/test_register.py deleted file mode 100644 index edf92b6898..0000000000 --- a/openedx/core/djangoapps/user_authn/views/tests/test_register.py +++ /dev/null @@ -1,866 +0,0 @@ -# -*- coding: utf-8 -*- -"""Tests for account creation""" -from __future__ import absolute_import - -import json -import unicodedata -import unittest -from datetime import datetime - -import ddt -import mock -import pytz -from django.conf import settings -from django.contrib.auth.hashers import make_password -from django.contrib.auth.models import AnonymousUser, User -from django.test import TestCase, TransactionTestCase -from django.test.client import RequestFactory -from django.test.utils import override_settings -from django.urls import reverse - -from lms.djangoapps.discussion.notification_prefs import NOTIFICATION_PREF_KEY -from openedx.core.djangoapps.django_comment_common.models import ForumsConfig -from openedx.core.djangoapps.lang_pref import LANGUAGE_KEY -from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin -from openedx.core.djangoapps.site_configuration.tests.test_util import with_site_configuration -from openedx.core.djangoapps.user_api.accounts import ( - USERNAME_BAD_LENGTH_MSG, - USERNAME_INVALID_CHARS_ASCII, - USERNAME_INVALID_CHARS_UNICODE -) -from openedx.core.djangoapps.user_api.config.waffle import PREVENT_AUTH_USER_WRITES, waffle -from openedx.core.djangoapps.user_api.preferences.api import get_user_preference -from openedx.core.djangoapps.user_authn.views.register import ( - REGISTRATION_AFFILIATE_ID, - REGISTRATION_UTM_CREATED_AT, - REGISTRATION_UTM_PARAMETERS, - _skip_activation_email -) -from student.models import UserAttribute -from student.tests.factories import UserFactory -from third_party_auth.tests import factories as third_party_auth_factory - -TEST_CS_URL = 'https://comments.service.test:123/' - -TEST_USERNAME = 'test_user' -TEST_EMAIL = 'test@test.com' - - -def get_mock_pipeline_data(username=TEST_USERNAME, email=TEST_EMAIL): - """ - Return mock pipeline data. - """ - return { - 'backend': 'tpa-saml', - 'kwargs': { - 'username': username, - 'auth_entry': 'register', - 'request': { - 'SAMLResponse': [], - 'RelayState': [ - 'testshib-openedx' - ] - }, - 'is_new': True, - 'new_association': True, - 'user': None, - 'social': None, - 'details': { - 'username': username, - 'fullname': 'Test Test', - 'last_name': 'Test', - 'first_name': 'Test', - 'email': email, - }, - 'response': {}, - 'uid': 'testshib-openedx:{}'.format(username) - } - } - - -@ddt.ddt -@with_site_configuration( - configuration={"extended_profile_fields": ["extra1", "extra2"]} -) -@override_settings( - REGISTRATION_EXTRA_FIELDS={ - key: "optional" - for key in [ - "level_of_education", "gender", "mailing_address", "city", "country", "goals", - "year_of_birth" - ] - } -) -class TestCreateAccount(SiteMixin, TestCase): - """Tests for account creation""" - - def setUp(self): - super(TestCreateAccount, self).setUp() - self.username = "test_user" - self.url = reverse("create_account") - self.request_factory = RequestFactory() - self.params = { - "username": self.username, - "email": "test@example.org", - "password": u"testpass", - "name": "Test User", - "honor_code": "true", - "terms_of_service": "true", - } - - @ddt.data("en", "eo") - def test_default_lang_pref_saved(self, lang): - with mock.patch("django.conf.settings.LANGUAGE_CODE", lang): - response = self.client.post(self.url, self.params) - self.assertEqual(response.status_code, 200) - user = User.objects.get(username=self.username) - self.assertEqual(get_user_preference(user, LANGUAGE_KEY), lang) - - @ddt.data("en", "eo") - def test_header_lang_pref_saved(self, lang): - response = self.client.post(self.url, self.params, HTTP_ACCEPT_LANGUAGE=lang) - user = User.objects.get(username=self.username) - self.assertEqual(response.status_code, 200) - self.assertEqual(get_user_preference(user, LANGUAGE_KEY), lang) - - def create_account_and_fetch_profile(self, host='test.localhost'): - """ - Create an account with self.params, assert that the response indicates - success, and return the UserProfile object for the newly created user - """ - response = self.client.post(self.url, self.params, HTTP_HOST=host) - self.assertEqual(response.status_code, 200) - user = User.objects.get(username=self.username) - return user.profile - - def test_create_account_and_normalize_password(self): - """ - Test that unicode normalization on passwords is happening when a user registers. - """ - # Set user password to NFKD format so that we can test that it is normalized to - # NFKC format upon account creation. - self.params['password'] = unicodedata.normalize('NFKD', u'Ṗŕệṿïệẅ Ṯệẍt') - response = self.client.post(self.url, self.params) - self.assertEqual(response.status_code, 200) - - user = User.objects.get(username=self.username) - salt_val = user.password.split('$')[1] - - expected_user_password = make_password(unicodedata.normalize('NFKC', u'Ṗŕệṿïệẅ Ṯệẍt'), salt_val) - self.assertEqual(expected_user_password, user.password) - - def test_marketing_cookie(self): - response = self.client.post(self.url, self.params) - self.assertEqual(response.status_code, 200) - self.assertIn(settings.EDXMKTG_LOGGED_IN_COOKIE_NAME, self.client.cookies) - self.assertIn(settings.EDXMKTG_USER_INFO_COOKIE_NAME, self.client.cookies) - - def test_profile_saved_no_optional_fields(self): - profile = self.create_account_and_fetch_profile() - self.assertEqual(profile.name, self.params["name"]) - self.assertEqual(profile.level_of_education, "") - self.assertEqual(profile.gender, "") - self.assertEqual(profile.mailing_address, "") - self.assertEqual(profile.city, "") - self.assertEqual(profile.country, "") - self.assertEqual(profile.goals, "") - self.assertEqual( - profile.get_meta(), - { - "extra1": "", - "extra2": "", - } - ) - self.assertIsNone(profile.year_of_birth) - - @override_settings(LMS_SEGMENT_KEY="testkey") - @mock.patch('openedx.core.djangoapps.user_authn.views.register.segment.track') - @mock.patch('openedx.core.djangoapps.user_authn.views.register.segment.identify') - def test_segment_tracking(self, mock_segment_identify, _): - year = datetime.now().year - year_of_birth = year - 14 - self.params.update({ - "level_of_education": "a", - "gender": "o", - "mailing_address": "123 Example Rd", - "city": "Exampleton", - "country": "US", - "goals": "To test this feature", - "year_of_birth": str(year_of_birth), - "extra1": "extra_value1", - "extra2": "extra_value2", - }) - - expected_payload = { - 'email': self.params['email'], - 'username': self.params['username'], - 'name': self.params['name'], - 'age': 13, - 'yearOfBirth': year_of_birth, - 'education': 'Associate degree', - 'address': self.params['mailing_address'], - 'gender': 'Other/Prefer Not to Say', - 'country': self.params['country'], - } - - profile = self.create_account_and_fetch_profile() - - mock_segment_identify.assert_called_with(profile.user.id, expected_payload) - - def test_profile_saved_all_optional_fields(self): - self.params.update({ - "level_of_education": "a", - "gender": "o", - "mailing_address": "123 Example Rd", - "city": "Exampleton", - "country": "US", - "goals": "To test this feature", - "year_of_birth": "2015", - "extra1": "extra_value1", - "extra2": "extra_value2", - }) - profile = self.create_account_and_fetch_profile() - self.assertEqual(profile.level_of_education, "a") - self.assertEqual(profile.gender, "o") - self.assertEqual(profile.mailing_address, "123 Example Rd") - self.assertEqual(profile.city, "Exampleton") - self.assertEqual(profile.country, "US") - self.assertEqual(profile.goals, "To test this feature") - self.assertEqual( - profile.get_meta(), - { - "extra1": "extra_value1", - "extra2": "extra_value2", - } - ) - self.assertEqual(profile.year_of_birth, 2015) - - def test_profile_saved_empty_optional_fields(self): - self.params.update({ - "level_of_education": "", - "gender": "", - "mailing_address": "", - "city": "", - "country": "", - "goals": "", - "year_of_birth": "", - "extra1": "", - "extra2": "", - }) - profile = self.create_account_and_fetch_profile() - self.assertEqual(profile.level_of_education, "") - self.assertEqual(profile.gender, "") - self.assertEqual(profile.mailing_address, "") - self.assertEqual(profile.city, "") - self.assertEqual(profile.country, "") - self.assertEqual(profile.goals, "") - self.assertEqual( - profile.get_meta(), - {"extra1": "", "extra2": ""} - ) - self.assertEqual(profile.year_of_birth, None) - - def test_profile_year_of_birth_non_integer(self): - self.params["year_of_birth"] = "not_an_integer" - profile = self.create_account_and_fetch_profile() - self.assertIsNone(profile.year_of_birth) - - @ddt.data(True, False) - def test_discussions_email_digest_pref(self, digest_enabled): - with mock.patch.dict("student.models.settings.FEATURES", {"ENABLE_DISCUSSION_EMAIL_DIGEST": digest_enabled}): - response = self.client.post(self.url, self.params) - self.assertEqual(response.status_code, 200) - user = User.objects.get(username=self.username) - preference = get_user_preference(user, NOTIFICATION_PREF_KEY) - if digest_enabled: - self.assertIsNotNone(preference) - else: - self.assertIsNone(preference) - - @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') - def test_affiliate_referral_attribution(self): - """ - Verify that a referral attribution is recorded if an affiliate - cookie is present upon a new user's registration. - """ - affiliate_id = 'test-partner' - self.client.cookies[settings.AFFILIATE_COOKIE_NAME] = affiliate_id - user = self.create_account_and_fetch_profile().user - self.assertEqual(UserAttribute.get_user_attribute(user, REGISTRATION_AFFILIATE_ID), affiliate_id) - - @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') - def test_utm_referral_attribution(self): - """ - Verify that a referral attribution is recorded if an affiliate - cookie is present upon a new user's registration. - """ - utm_cookie_name = 'edx.test.utm' - with mock.patch('student.models.RegistrationCookieConfiguration.current') as config: - instance = config.return_value - instance.utm_cookie_name = utm_cookie_name - - timestamp = 1475521816879 - utm_cookie = { - 'utm_source': 'test-source', - 'utm_medium': 'test-medium', - 'utm_campaign': 'test-campaign', - 'utm_term': 'test-term', - 'utm_content': 'test-content', - 'created_at': timestamp - } - - created_at = datetime.fromtimestamp(timestamp / float(1000), tz=pytz.UTC) - - self.client.cookies[utm_cookie_name] = json.dumps(utm_cookie) - user = self.create_account_and_fetch_profile().user - self.assertEqual( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_source')), - utm_cookie.get('utm_source') - ) - self.assertEqual( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_medium')), - utm_cookie.get('utm_medium') - ) - self.assertEqual( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_campaign')), - utm_cookie.get('utm_campaign') - ) - self.assertEqual( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_term')), - utm_cookie.get('utm_term') - ) - self.assertEqual( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_content')), - utm_cookie.get('utm_content') - ) - self.assertEqual( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_CREATED_AT), - str(created_at) - ) - - @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') - def test_no_referral(self): - """Verify that no referral is recorded when a cookie is not present.""" - utm_cookie_name = 'edx.test.utm' - with mock.patch('student.models.RegistrationCookieConfiguration.current') as config: - instance = config.return_value - instance.utm_cookie_name = utm_cookie_name - - self.assertIsNone(self.client.cookies.get(settings.AFFILIATE_COOKIE_NAME)) - self.assertIsNone(self.client.cookies.get(utm_cookie_name)) - user = self.create_account_and_fetch_profile().user - self.assertIsNone(UserAttribute.get_user_attribute(user, REGISTRATION_AFFILIATE_ID)) - self.assertIsNone(UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_source'))) - self.assertIsNone(UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_medium'))) - self.assertIsNone(UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_campaign'))) - self.assertIsNone(UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_term'))) - self.assertIsNone(UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_content'))) - self.assertIsNone(UserAttribute.get_user_attribute(user, REGISTRATION_UTM_CREATED_AT)) - - @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') - def test_incomplete_utm_referral(self): - """Verify that no referral is recorded when a cookie is not present.""" - utm_cookie_name = 'edx.test.utm' - with mock.patch('student.models.RegistrationCookieConfiguration.current') as config: - instance = config.return_value - instance.utm_cookie_name = utm_cookie_name - - utm_cookie = { - 'utm_source': 'test-source', - 'utm_medium': 'test-medium', - # No campaign - 'utm_term': 'test-term', - 'utm_content': 'test-content', - # No created at - } - - self.client.cookies[utm_cookie_name] = json.dumps(utm_cookie) - user = self.create_account_and_fetch_profile().user - - self.assertEqual( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_source')), - utm_cookie.get('utm_source') - ) - self.assertEqual( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_medium')), - utm_cookie.get('utm_medium') - ) - self.assertEqual( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_term')), - utm_cookie.get('utm_term') - ) - self.assertEqual( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_content')), - utm_cookie.get('utm_content') - ) - self.assertIsNone( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_PARAMETERS.get('utm_campaign')) - ) - self.assertIsNone( - UserAttribute.get_user_attribute(user, REGISTRATION_UTM_CREATED_AT) - ) - - @mock.patch("openedx.core.djangoapps.site_configuration.helpers.get_value", mock.Mock(return_value=False)) - def test_create_account_not_allowed(self): - """ - Test case to check user creation is forbidden when ALLOW_PUBLIC_ACCOUNT_CREATION feature flag is turned off - """ - response = self.client.get(self.url) - self.assertEqual(response.status_code, 403) - - def test_create_account_prevent_auth_user_writes(self): - with waffle().override(PREVENT_AUTH_USER_WRITES, True): - response = self.client.get(self.url) - assert response.status_code == 403 - - def test_created_on_site_user_attribute_set(self): - profile = self.create_account_and_fetch_profile(host=self.site.domain) - self.assertEqual(UserAttribute.get_user_attribute(profile.user, 'created_on_site'), self.site.domain) - - @ddt.data( - ( - False, get_mock_pipeline_data(), - { - 'SKIP_EMAIL_VALIDATION': False, 'AUTOMATIC_AUTH_FOR_TESTING': False, - }, - False # Do not skip activation email for normal scenario. - ), - ( - False, get_mock_pipeline_data(), - { - 'SKIP_EMAIL_VALIDATION': True, 'AUTOMATIC_AUTH_FOR_TESTING': False, - }, - True # Skip activation email when `SKIP_EMAIL_VALIDATION` FEATURE flag is active. - ), - ( - False, get_mock_pipeline_data(), - { - 'SKIP_EMAIL_VALIDATION': False, 'AUTOMATIC_AUTH_FOR_TESTING': True, - }, - True # Skip activation email when `AUTOMATIC_AUTH_FOR_TESTING` FEATURE flag is active. - ), - ( - True, get_mock_pipeline_data(), - { - 'SKIP_EMAIL_VALIDATION': False, 'AUTOMATIC_AUTH_FOR_TESTING': False, - }, - True # Skip activation email if `skip_email_verification` is set for third party authentication. - ), - ( - False, get_mock_pipeline_data(email='invalid@yopmail.com'), - { - 'SKIP_EMAIL_VALIDATION': False, 'AUTOMATIC_AUTH_FOR_TESTING': False, - }, - False # Send activation email when `skip_email_verification` is not set. - ) - ) - @ddt.unpack - @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') - def test_should_skip_activation_email( - self, skip_email_verification, running_pipeline, feature_overrides, expected, - ): - """ - Test `skip_activation_email` works as expected. - """ - third_party_provider = third_party_auth_factory.SAMLProviderConfigFactory( - skip_email_verification=skip_email_verification, - ) - user = UserFactory(username=TEST_USERNAME, email=TEST_EMAIL) - - with override_settings(FEATURES=dict(settings.FEATURES, **feature_overrides)): - result = _skip_activation_email( - user=user, - running_pipeline=running_pipeline, - third_party_provider=third_party_provider - ) - - assert result == expected - - -@ddt.ddt -class TestCreateAccountValidation(TestCase): - """ - Test validation of various parameters in the create_account view - """ - def setUp(self): - super(TestCreateAccountValidation, self).setUp() - self.url = reverse("create_account") - self.minimal_params = { - "username": "test_username", - "email": "test_email@example.com", - "password": "test_password", - "name": "Test Name", - "honor_code": "true", - "terms_of_service": "true", - } - - def assert_success(self, params): - """ - Request account creation with the given params and assert that the - response properly indicates success - """ - response = self.client.post(self.url, params) - self.assertEqual(response.status_code, 200) - response_data = json.loads(response.content.decode('utf-8')) - self.assertTrue(response_data["success"]) - - def assert_error(self, params, expected_field, expected_value): - """ - Request account creation with the given params and assert that the - response properly indicates an error with the given field and value - """ - response = self.client.post(self.url, params) - self.assertEqual(response.status_code, 400) - response_data = json.loads(response.content.decode('utf-8')) - self.assertFalse(response_data["success"]) - self.assertEqual(response_data["field"], expected_field) - self.assertEqual(response_data["value"], expected_value) - - def test_minimal_success(self): - self.assert_success(self.minimal_params) - - def test_username(self): - params = dict(self.minimal_params) - - def assert_username_error(expected_error): - """ - Assert that requesting account creation results in the expected - error - """ - self.assert_error(params, "username", expected_error) - - # Missing - del params["username"] - assert_username_error(USERNAME_BAD_LENGTH_MSG) - - # Empty, too short - for username in ["", "a"]: - params["username"] = username - assert_username_error(USERNAME_BAD_LENGTH_MSG) - - # Too long - params["username"] = "this_username_has_31_characters" - assert_username_error(USERNAME_BAD_LENGTH_MSG) - - # Invalid - params["username"] = "invalid username" - assert_username_error(str(USERNAME_INVALID_CHARS_ASCII)) - - def test_email(self): - params = dict(self.minimal_params) - - def assert_email_error(expected_error): - """ - Assert that requesting account creation results in the expected - error - """ - self.assert_error(params, "email", expected_error) - - # Missing - del params["email"] - assert_email_error("A properly formatted e-mail is required") - - # Empty - params["email"] = "" - assert_email_error("A properly formatted e-mail is required") - - #too short - params["email"] = "a" - assert_email_error("A properly formatted e-mail is required " - "Ensure this value has at least 3 characters (it has 1).") - - # Too long - params["email"] = '{email}@example.com'.format( - email='this_email_address_has_254_characters_in_it_so_it_is_unacceptable' * 4 - ) - - # Assert that we get error when email has more than 254 characters. - self.assertGreater(len(params['email']), 254) - assert_email_error("Email cannot be more than 254 characters long") - - # Valid Email - params["email"] = "student@edx.com" - # Assert success on valid email - self.assertLess(len(params["email"]), 254) - self.assert_success(params) - - # Invalid - params["email"] = "not_an_email_address" - assert_email_error("A properly formatted e-mail is required") - - @override_settings( - REGISTRATION_EMAIL_PATTERNS_ALLOWED=[ - r'.*@edx.org', # Naive regex omitting '^', '$' and '\.' should still work. - r'^.*@(.*\.)?example\.com$', - r'^(^\w+\.\w+)@school.tld$', - ] - ) - @ddt.data( - ('bob@we-are.bad', False), - ('bob@edx.org.we-are.bad', False), - ('staff@edx.org', True), - ('student@example.com', True), - ('student@sub.example.com', True), - ('mr.teacher@school.tld', True), - ('student1234@school.tld', False), - ) - @ddt.unpack - def test_email_pattern_requirements(self, email, expect_success): - """ - Test the REGISTRATION_EMAIL_PATTERNS_ALLOWED setting, a feature which - can be used to only allow people register if their email matches a - against a whitelist of regexs. - """ - params = dict(self.minimal_params) - params["email"] = email - if expect_success: - self.assert_success(params) - else: - self.assert_error(params, "email", "Unauthorized email address.") - - def test_password(self): - params = dict(self.minimal_params) - - def assert_password_error(expected_error): - """ - Assert that requesting account creation results in the expected - error - """ - self.assert_error(params, "password", expected_error) - - # Missing - del params["password"] - assert_password_error("This field is required.") - - # Empty - params["password"] = "" - assert_password_error("This field is required.") - - # Too short - params["password"] = "a" - assert_password_error("This password is too short. It must contain at least 2 characters.") - - # Password policy is tested elsewhere - - # Matching username - params["username"] = params["password"] = "test_username_and_password" - assert_password_error("The password is too similar to the username.") - - def test_name(self): - params = dict(self.minimal_params) - - def assert_name_error(expected_error): - """ - Assert that requesting account creation results in the expected - error - """ - self.assert_error(params, "name", expected_error) - - # Missing - del params["name"] - assert_name_error("Your legal name must be a minimum of one character long") - - # Empty, too short - params["name"] = "" - assert_name_error("Your legal name must be a minimum of one character long") - - def test_honor_code(self): - params = dict(self.minimal_params) - - def assert_honor_code_error(expected_error): - """ - Assert that requesting account creation results in the expected - error - """ - self.assert_error(params, "honor_code", expected_error) - - with override_settings(REGISTRATION_EXTRA_FIELDS={"honor_code": "required"}): - # Missing - del params["honor_code"] - assert_honor_code_error("To enroll, you must follow the honor code.") - - # Empty, invalid - for honor_code in ["", "false", "not_boolean"]: - params["honor_code"] = honor_code - assert_honor_code_error("To enroll, you must follow the honor code.") - - # True - params["honor_code"] = "tRUe" - self.assert_success(params) - - with override_settings(REGISTRATION_EXTRA_FIELDS={"honor_code": "optional"}): - # Missing - del params["honor_code"] - # Need to change username/email because user was created above - params["username"] = "another_test_username" - params["email"] = "another_test_email@example.com" - self.assert_success(params) - - def test_terms_of_service(self): - params = dict(self.minimal_params) - - def assert_terms_of_service_error(expected_error): - """ - Assert that requesting account creation results in the expected - error - """ - self.assert_error(params, "terms_of_service", expected_error) - - # Missing - del params["terms_of_service"] - assert_terms_of_service_error("You must accept the terms of service.") - - # Empty, invalid - for terms_of_service in ["", "false", "not_boolean"]: - params["terms_of_service"] = terms_of_service - assert_terms_of_service_error("You must accept the terms of service.") - - # True - params["terms_of_service"] = "tRUe" - self.assert_success(params) - - @ddt.data( - ("level_of_education", 1, "A level of education is required"), - ("gender", 1, "Your gender is required"), - ("year_of_birth", 2, "Your year of birth is required"), - ("mailing_address", 2, "Your mailing address is required"), - ("goals", 2, "A description of your goals is required"), - ("city", 2, "A city is required"), - ("country", 2, "A country is required"), - ("custom_field", 2, "You are missing one or more required fields") - ) - @ddt.unpack - def test_extra_fields(self, field, min_length, expected_error): - params = dict(self.minimal_params) - - def assert_extra_field_error(): - """ - Assert that requesting account creation results in the expected - error - """ - self.assert_error(params, field, expected_error) - - with override_settings(REGISTRATION_EXTRA_FIELDS={field: "required"}): - # Missing - assert_extra_field_error() - - # Empty - params[field] = "" - assert_extra_field_error() - - # Too short - if min_length > 1: - params[field] = "a" - assert_extra_field_error() - - -@mock.patch.dict("student.models.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) -@mock.patch("openedx.core.djangoapps.django_comment_common.comment_client.User.base_url", TEST_CS_URL) -@mock.patch( - "openedx.core.djangoapps.django_comment_common.comment_client.utils.requests.request", - return_value=mock.Mock(status_code=200, text='{}') -) -class TestCreateCommentsServiceUser(TransactionTestCase): - """ Tests for creating comments service user. """ - - def setUp(self): - super(TestCreateCommentsServiceUser, self).setUp() - self.username = "test_user" - self.url = reverse("create_account") - self.params = { - "username": self.username, - "email": "test@example.org", - "password": "testpass", - "name": "Test User", - "honor_code": "true", - "terms_of_service": "true", - } - - config = ForumsConfig.current() - config.enabled = True - config.save() - - def test_cs_user_created(self, request): - "If user account creation succeeds, we should create a comments service user" - response = self.client.post(self.url, self.params) - self.assertEqual(response.status_code, 200) - self.assertTrue(request.called) - args, kwargs = request.call_args - self.assertEqual(args[0], 'put') - self.assertTrue(args[1].startswith(TEST_CS_URL)) - self.assertEqual(kwargs['data']['username'], self.params['username']) - - @mock.patch("student.models.Registration.register", side_effect=Exception) - def test_cs_user_not_created(self, register, request): - "If user account creation fails, we should not create a comments service user" - try: - self.client.post(self.url, self.params) - except: # pylint: disable=bare-except - pass - with self.assertRaises(User.DoesNotExist): - User.objects.get(username=self.username) - self.assertTrue(register.called) - self.assertFalse(request.called) - - -class TestUnicodeUsername(TestCase): - """ - Test for Unicode usernames which is an optional feature. - """ - - def setUp(self): - super(TestUnicodeUsername, self).setUp() - self.url = reverse('create_account') - - # The word below reads "Omar II", in Arabic. It also contains a space and - # an Eastern Arabic Number another option is to use the Esperanto fake - # language but this was used instead to test non-western letters. - self.username = u'عمر ٢' - - self.url_params = { - 'username': self.username, - 'email': 'unicode_user@example.com', - "password": "testpass", - 'name': 'unicode_user', - 'terms_of_service': 'true', - 'honor_code': 'true', - } - - @mock.patch.dict(settings.FEATURES, {'ENABLE_UNICODE_USERNAME': False}) - def test_with_feature_disabled(self): - """ - Ensures backward-compatible defaults. - """ - response = self.client.post(self.url, self.url_params) - - self.assertEquals(response.status_code, 400) - obj = json.loads(response.content.decode('utf-8')) - self.assertEquals(USERNAME_INVALID_CHARS_ASCII, obj['value']) - - with self.assertRaises(User.DoesNotExist): - User.objects.get(email=self.url_params['email']) - - @mock.patch.dict(settings.FEATURES, {'ENABLE_UNICODE_USERNAME': True}) - def test_with_feature_enabled(self): - response = self.client.post(self.url, self.url_params) - self.assertEquals(response.status_code, 200) - - self.assertTrue(User.objects.get(email=self.url_params['email'])) - - @mock.patch.dict(settings.FEATURES, {'ENABLE_UNICODE_USERNAME': True}) - def test_special_chars_with_feature_enabled(self): - """ - Ensures that special chars are still prevented. - """ - - invalid_params = self.url_params.copy() - invalid_params['username'] = '**john**' - - response = self.client.post(self.url, invalid_params) - self.assertEquals(response.status_code, 400) - - obj = json.loads(response.content.decode('utf-8')) - self.assertEquals(USERNAME_INVALID_CHARS_UNICODE, obj['value']) - - with self.assertRaises(User.DoesNotExist): - User.objects.get(email=self.url_params['email'])