diff --git a/common/djangoapps/auth_exchange/forms.py b/common/djangoapps/auth_exchange/forms.py index 2391486f99..cad3db1e1b 100644 --- a/common/djangoapps/auth_exchange/forms.py +++ b/common/djangoapps/auth_exchange/forms.py @@ -10,6 +10,7 @@ from provider.oauth2.forms import ScopeChoiceField, ScopeMixin from provider.oauth2.models import Client from requests import HTTPError from social.backends import oauth as social_oauth +from social.exceptions import AuthException from third_party_auth import pipeline @@ -54,7 +55,7 @@ class AccessTokenExchangeForm(ScopeMixin, OAuthForm): if self._errors: return {} - backend = self.request.social_strategy.backend + backend = self.request.backend if not isinstance(backend, social_oauth.BaseOAuth2): raise OAuthValidationError( { @@ -88,8 +89,8 @@ class AccessTokenExchangeForm(ScopeMixin, OAuthForm): user = None try: - user = backend.do_auth(self.cleaned_data.get("access_token")) - except HTTPError: + user = backend.do_auth(self.cleaned_data.get("access_token"), allow_inactive_user=True) + except (HTTPError, AuthException): pass if user and isinstance(user, User): self.cleaned_data["user"] = user diff --git a/common/djangoapps/auth_exchange/tests/test_forms.py b/common/djangoapps/auth_exchange/tests/test_forms.py index c89fec9d5f..ffeffb4e22 100644 --- a/common/djangoapps/auth_exchange/tests/test_forms.py +++ b/common/djangoapps/auth_exchange/tests/test_forms.py @@ -24,8 +24,11 @@ class AccessTokenExchangeFormTest(AccessTokenExchangeTestMixin): def setUp(self): super(AccessTokenExchangeFormTest, self).setUp() self.request = RequestFactory().post("dummy_url") + redirect_uri = 'dummy_redirect_url' SessionMiddleware().process_request(self.request) - self.request.social_strategy = social_utils.load_strategy(self.request, self.BACKEND) + self.request.social_strategy = social_utils.load_strategy(self.request) + # pylint: disable=no-member + self.request.backend = social_utils.load_backend(self.request.social_strategy, self.BACKEND, redirect_uri) def _assert_error(self, data, expected_error, expected_error_description): form = AccessTokenExchangeForm(request=self.request, data=data) diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py index dde71bd237..ae375e8bf4 100644 --- a/common/djangoapps/external_auth/tests/test_shib.py +++ b/common/djangoapps/external_auth/tests/test_shib.py @@ -20,6 +20,7 @@ from external_auth.views import ( shib_login, course_specific_login, course_specific_register, _flatten_to_ascii ) from mock import patch +from urllib import urlencode from student.views import create_account, change_enrollment from student.models import UserProfile, CourseEnrollment @@ -169,7 +170,7 @@ class ShibSPTest(ModuleStoreTestCase): if idp == "https://idp.stanford.edu/" and remote_user == 'withmap@stanford.edu': self.assertIsInstance(response, HttpResponseRedirect) self.assertEqual(request.user, user_w_map) - self.assertEqual(response['Location'], '/') + self.assertEqual(response['Location'], '/dashboard') # verify logging: self.assertEquals(len(audit_log_calls), 2) self._assert_shib_login_is_logged(audit_log_calls[0], remote_user) @@ -193,7 +194,7 @@ class ShibSPTest(ModuleStoreTestCase): self.assertIsNotNone(ExternalAuthMap.objects.get(user=user_wo_map)) self.assertIsInstance(response, HttpResponseRedirect) self.assertEqual(request.user, user_wo_map) - self.assertEqual(response['Location'], '/') + self.assertEqual(response['Location'], '/dashboard') # verify logging: self.assertEquals(len(audit_log_calls), 2) self._assert_shib_login_is_logged(audit_log_calls[0], remote_user) @@ -242,7 +243,7 @@ class ShibSPTest(ModuleStoreTestCase): self.assertTrue(inactive_user.is_active) self.assertIsInstance(response, HttpResponseRedirect) self.assertEqual(request.user, inactive_user) - self.assertEqual(response['Location'], '/') + self.assertEqual(response['Location'], '/dashboard') # verify logging: self.assertEquals(len(audit_log_calls), 3) self._assert_shib_login_is_logged(audit_log_calls[0], log_user_string) @@ -549,29 +550,20 @@ class ShibSPTest(ModuleStoreTestCase): # no enrollment before trying self.assertFalse(CourseEnrollment.is_enrolled(student, course.id)) self.client.logout() + params = [ + ('course_id', course.id.to_deprecated_string()), + ('enrollment_action', 'enroll'), + ('next', '/testredirect') + ] request_kwargs = {'path': '/shib-login/', - 'data': {'enrollment_action': 'enroll', 'course_id': course.id.to_deprecated_string(), 'next': '/testredirect'}, + 'data': dict(params), 'follow': False, 'REMOTE_USER': 'testuser@stanford.edu', 'Shib-Identity-Provider': 'https://idp.stanford.edu/'} response = self.client.get(**request_kwargs) - # successful login is a redirect to "/" + # successful login is a redirect to the URL that handles auto-enrollment self.assertEqual(response.status_code, 302) - self.assertEqual(response['location'], 'http://testserver/testredirect') - # now there is enrollment - self.assertTrue(CourseEnrollment.is_enrolled(student, course.id)) - - # Clean up and try again with POST (doesn't happen with real production shib, doing this for test coverage) - self.client.logout() - CourseEnrollment.unenroll(student, course.id) - self.assertFalse(CourseEnrollment.is_enrolled(student, course.id)) - - response = self.client.post(**request_kwargs) - # successful login is a redirect to "/" - self.assertEqual(response.status_code, 302) - self.assertEqual(response['location'], 'http://testserver/testredirect') - # now there is enrollment - self.assertTrue(CourseEnrollment.is_enrolled(student, course.id)) + self.assertEqual(response['location'], 'http://testserver/account/finish_auth?{}'.format(urlencode(params))) class ShibUtilFnTest(TestCase): diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 95d9ced656..df7391bf95 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -22,6 +22,7 @@ from django.core.exceptions import ValidationError if settings.FEATURES.get('AUTH_USE_CAS'): from django_cas.views import login as django_cas_login +from student.helpers import get_next_url_for_login_page from student.models import UserProfile from django.http import HttpResponse, HttpResponseRedirect, HttpRequest, HttpResponseForbidden @@ -118,7 +119,8 @@ def openid_login_complete(request, external_domain, details, details.get('email', ''), - fullname + fullname, + retfun=functools.partial(redirect, get_next_url_for_login_page(request)), ) return render_failure(request, 'Openid failure') @@ -236,14 +238,6 @@ def _external_login_or_signup(request, login(request, user) request.session.set_expiry(0) - # Now to try enrollment - # Need to special case Shibboleth here because it logs in via a GET. - # testing request.method for extra paranoia - if uses_shibboleth and request.method == 'GET': - enroll_request = _make_shib_enrollment_request(request) - student.views.try_change_enrollment(enroll_request) - else: - student.views.try_change_enrollment(request) if settings.FEATURES['SQUELCH_PII_IN_LOGS']: AUDIT_LOG.info(u"Login success - user.id: {0}".format(user.id)) else: @@ -449,9 +443,7 @@ def ssl_login(request): (_user, email, fullname) = _ssl_dn_extract_info(cert) - redirect_to = request.GET.get('next') - if not redirect_to: - redirect_to = '/' + redirect_to = get_next_url_for_login_page(request) retfun = functools.partial(redirect, redirect_to) return _external_login_or_signup( request, @@ -528,10 +520,8 @@ def shib_login(request): fullname = shib['displayName'] if shib['displayName'] else u'%s %s' % (shib['givenName'], shib['sn']) - redirect_to = request.REQUEST.get('next') - retfun = None - if redirect_to: - retfun = functools.partial(_safe_postlogin_redirect, redirect_to, request.get_host()) + redirect_to = get_next_url_for_login_page(request) + retfun = functools.partial(_safe_postlogin_redirect, redirect_to, request.get_host()) return _external_login_or_signup( request, @@ -558,31 +548,6 @@ def _safe_postlogin_redirect(redirect_to, safehost, default_redirect='/'): return redirect(default_redirect) -def _make_shib_enrollment_request(request): - """ - Need this hack function because shibboleth logins don't happen over POST - but change_enrollment expects its request to be a POST, with - enrollment_action and course_id POST parameters. - """ - enroll_request = HttpRequest() - enroll_request.user = request.user - enroll_request.session = request.session - enroll_request.method = "POST" - - # copy() also makes GET and POST mutable - # See https://docs.djangoproject.com/en/dev/ref/request-response/#django.http.QueryDict.update - enroll_request.GET = request.GET.copy() - enroll_request.POST = request.POST.copy() - - # also have to copy these GET parameters over to POST - if "enrollment_action" not in enroll_request.POST and "enrollment_action" in enroll_request.GET: - enroll_request.POST.setdefault('enrollment_action', enroll_request.GET.get('enrollment_action')) - if "course_id" not in enroll_request.POST and "course_id" in enroll_request.GET: - enroll_request.POST.setdefault('course_id', enroll_request.GET.get('course_id')) - - return enroll_request - - def course_specific_login(request, course_id): """ Dispatcher function for selecting the specific login method diff --git a/common/djangoapps/student/helpers.py b/common/djangoapps/student/helpers.py index 40116dd27b..5efa8f7cec 100644 --- a/common/djangoapps/student/helpers.py +++ b/common/djangoapps/student/helpers.py @@ -4,9 +4,11 @@ from datetime import datetime from pytz import UTC from django.utils.http import cookie_date from django.conf import settings +from django.core.urlresolvers import reverse, NoReverseMatch +import third_party_auth +import urllib from verify_student.models import SoftwareSecurePhotoVerification # pylint: disable=F0401 from course_modes.models import CourseMode -from student_account.helpers import auth_pipeline_urls # pylint: disable=unused-import,import-error def set_logged_in_cookie(request, response): @@ -199,3 +201,70 @@ def check_verify_status_by_course(user, course_enrollment_pairs, all_course_mode status_by_course[key]['verification_good_until'] = recent_verification_datetime.strftime("%m/%d/%Y") 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.NAME: third_party_auth.pipeline.get_login_url(provider.NAME, auth_entry, redirect_url=redirect_url) + for provider in third_party_auth.provider.Registry.enabled() + } + + +# 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') + + +def get_next_url_for_login_page(request): + """ + Determine the URL to redirect to following login/registration/third_party_auth + + The user is currently on a login or reigration page. + If 'course_id' is set, or other POST_AUTH_PARAMS, we will need to send the user to the + /account/finish_auth/ view following login, which will take care of auto-enrollment in + the specified course. + + Otherwise, we go to the ?next= query param or to the dashboard if nothing else is + specified. + """ + redirect_to = request.GET.get('next', None) + if not redirect_to: + try: + redirect_to = reverse('dashboard') + except NoReverseMatch: + redirect_to = reverse('home') + if any(param in request.GET for param in POST_AUTH_PARAMS): + # Before we redirect to next/dashboard, we need to handle auto-enrollment: + params = [(param, request.GET[param]) for param in POST_AUTH_PARAMS if param in request.GET] + params.append(('next', redirect_to)) # After auto-enrollment, user will be sent to payment page or to this URL + redirect_to = '{}?{}'.format(reverse('finish_auth'), urllib.urlencode(params)) + # Note: if we are resuming a third party auth pipeline, then the next URL will already + # be saved in the session as part of the pipeline state. That URL will take priority + # over this one. + return redirect_to diff --git a/common/djangoapps/student/tests/test_login.py b/common/djangoapps/student/tests/test_login.py index 9a9c8986a6..2c648be6ed 100644 --- a/common/djangoapps/student/tests/test_login.py +++ b/common/djangoapps/student/tests/test_login.py @@ -278,24 +278,6 @@ class LoginTest(TestCase): self.assertIsNone(response_content["redirect_url"]) self._assert_response(response, success=True) - def test_change_enrollment_200_redirect(self): - """ - Tests that "redirect_url" is the content of the HttpResponse returned - by change_enrollment, if there is content - """ - # add this post param to trigger a call to change_enrollment - extra_post_params = {"enrollment_action": "enroll"} - with patch('student.views.change_enrollment') as mock_change_enrollment: - mock_change_enrollment.return_value = HttpResponse("in/nature/there/is/nothing/melancholy") - response, _ = self._login_response( - 'test@edx.org', - 'test_password', - extra_post_params=extra_post_params, - ) - response_content = json.loads(response.content) - self.assertEqual(response_content["redirect_url"], "in/nature/there/is/nothing/melancholy") - self._assert_response(response, success=True) - def _login_response(self, email, password, patched_audit_log='student.views.AUDIT_LOG', extra_post_params=None): ''' Post the login info ''' post_params = {'email': email, 'password': password} diff --git a/common/djangoapps/student/tests/test_login_registration_forms.py b/common/djangoapps/student/tests/test_login_registration_forms.py index afc9fed206..3ebafb2358 100644 --- a/common/djangoapps/student/tests/test_login_registration_forms.py +++ b/common/djangoapps/student/tests/test_login_registration_forms.py @@ -21,13 +21,11 @@ THIRD_PARTY_AUTH_BACKENDS = ["google-oauth2", "facebook"] THIRD_PARTY_AUTH_PROVIDERS = ["Google", "Facebook"] -def _third_party_login_url(backend_name, auth_entry, course_id=None, redirect_url=None): +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)) - if course_id: - params.append(("enroll_course_id", course_id)) return u"{url}?{params}".format( url=reverse("social:begin", kwargs={"backend": backend_name}), @@ -35,6 +33,11 @@ def _third_party_login_url(backend_name, auth_entry, course_id=None, redirect_ur ) +def _finish_auth_url(params): + """ Construct the URL that follows login/registration if we are doing auto-enrollment """ + return u"{}?{}".format(reverse('finish_auth'), urllib.urlencode(params)) + + @ddt.ddt @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') class LoginFormTest(UrlResetMixin, ModuleStoreTestCase): @@ -46,7 +49,6 @@ class LoginFormTest(UrlResetMixin, ModuleStoreTestCase): self.url = reverse("signin_user") self.course = CourseFactory.create() self.course_id = unicode(self.course.id) - self.course_modes_url = reverse("course_modes_choose", kwargs={"course_id": self.course_id}) self.courseware_url = reverse("courseware", args=[self.course_id]) @patch.dict(settings.FEATURES, {"ENABLE_THIRD_PARTY_AUTH": False}) @@ -65,7 +67,8 @@ class LoginFormTest(UrlResetMixin, ModuleStoreTestCase): 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 - response = self.client.get(self.url, {"course_id": self.course_id}) + 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 @@ -73,35 +76,12 @@ class LoginFormTest(UrlResetMixin, ModuleStoreTestCase): expected_url = _third_party_login_url( backend_name, "login", - course_id=self.course_id, - redirect_url=self.course_modes_url + redirect_url=_finish_auth_url(params), ) self.assertContains(response, expected_url) @ddt.data(*THIRD_PARTY_AUTH_BACKENDS) - def test_third_party_auth_with_white_label_course(self, backend_name): - # Set the course mode to honor with a min price, - # indicating that the course is behind a paywall. - CourseModeFactory.create( - course_id=self.course.id, - mode_slug="honor", - mode_display_name="Honor", - min_price=100 - ) - - # Expect that we're redirected to the shopping cart - # instead of to the track selection page. - response = self.client.get(self.url, {"course_id": self.course_id}) - expected_url = _third_party_login_url( - backend_name, - "login", - course_id=self.course_id, - redirect_url=reverse("shoppingcart.views.show_cart") - ) - self.assertContains(response, expected_url) - - @ddt.data(*THIRD_PARTY_AUTH_BACKENDS) - def test_third_party_auth_with_redirect_url(self, backend_name): + 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) @@ -123,28 +103,47 @@ class LoginFormTest(UrlResetMixin, ModuleStoreTestCase): ) self.assertContains(response, expected_url) - @ddt.data(None, "true", "false") - def test_email_opt_in(self, opt_in_value): - params = { - 'course_id': self.course_id, - 'enrollment_action': 'enroll' - } + @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) + expected_url = _third_party_login_url( + backend_name, + "login", + redirect_url=_finish_auth_url(params), + ) + self.assertContains(response, expected_url) - if opt_in_value is not None: - params['email_opt_in'] = opt_in_value + @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) - # Verify that the hidden parameter is set correctly - hidden_param = ' string. Data about the + provider_details: dict of string -> string. Data about the user passed back by the provider. Returns: String or None. The user's email address, if any. """ - return None + return provider_details.get('email') @classmethod - def get_name(cls, unused_provider_details): + def get_name(cls, provider_details): """Gets user's name. Provider responses can contain arbitrary data. This method can be @@ -61,13 +61,13 @@ class BaseProvider(object): extracted by the social_details pipeline step. Args: - unused_provider_details: dict of string -> string. Data about the + provider_details: dict of string -> string. Data about the user passed back by the provider. Returns: String or None. The user's full name, if any. """ - return None + return provider_details.get('fullname') @classmethod def get_register_form_data(cls, pipeline_kwargs): @@ -121,14 +121,6 @@ class GoogleOauth2(BaseProvider): 'SOCIAL_AUTH_GOOGLE_OAUTH2_SECRET': None, } - @classmethod - def get_email(cls, provider_details): - return provider_details.get('email') - - @classmethod - def get_name(cls, provider_details): - return provider_details.get('fullname') - class LinkedInOauth2(BaseProvider): """Provider for LinkedIn's Oauth2 auth system.""" @@ -141,14 +133,6 @@ class LinkedInOauth2(BaseProvider): 'SOCIAL_AUTH_LINKEDIN_OAUTH2_SECRET': None, } - @classmethod - def get_email(cls, provider_details): - return provider_details.get('email') - - @classmethod - def get_name(cls, provider_details): - return provider_details.get('fullname') - class FacebookOauth2(BaseProvider): """Provider for LinkedIn's Oauth2 auth system.""" @@ -161,14 +145,6 @@ class FacebookOauth2(BaseProvider): 'SOCIAL_AUTH_FACEBOOK_SECRET': None, } - @classmethod - def get_email(cls, provider_details): - return provider_details.get('email') - - @classmethod - def get_name(cls, provider_details): - return provider_details.get('fullname') - class Registry(object): """Singleton registry of third-party auth providers. diff --git a/common/djangoapps/third_party_auth/settings.py b/common/djangoapps/third_party_auth/settings.py index b30076579d..12b362759c 100644 --- a/common/djangoapps/third_party_auth/settings.py +++ b/common/djangoapps/third_party_auth/settings.py @@ -46,7 +46,7 @@ If true, it: from . import provider -_FIELDS_STORED_IN_SESSION = ['auth_entry', 'next', 'enroll_course_id', 'email_opt_in'] +_FIELDS_STORED_IN_SESSION = ['auth_entry', 'next'] _MIDDLEWARE_CLASSES = ( 'third_party_auth.middleware.ExceptionMiddleware', ) @@ -105,6 +105,7 @@ def _set_global_settings(django_settings): 'social.pipeline.social_auth.social_user', 'third_party_auth.pipeline.associate_by_email_if_login_api', 'social.pipeline.user.get_username', + 'third_party_auth.pipeline.set_pipeline_timeout', 'third_party_auth.pipeline.ensure_user_information', 'social.pipeline.user.create_user', 'social.pipeline.social_auth.associate_user', @@ -112,7 +113,6 @@ def _set_global_settings(django_settings): 'social.pipeline.user.user_details', 'third_party_auth.pipeline.set_logged_in_cookie', 'third_party_auth.pipeline.login_analytics', - 'third_party_auth.pipeline.change_enrollment', ) # We let the user specify their email address during signup. @@ -123,6 +123,13 @@ def _set_global_settings(django_settings): # enable this when you want to get stack traces rather than redirections. django_settings.SOCIAL_AUTH_RAISE_EXCEPTIONS = False + # Allow users to login using social auth even if their account is not verified yet + # The 'ensure_user_information' step controls this and only allows brand new users + # to login without verification. Repeat logins are not permitted until the account + # gets verified. + django_settings.INACTIVE_USER_LOGIN = True + django_settings.INACTIVE_USER_URL = '/auth/inactive' + # Context processors required under Django. django_settings.SOCIAL_AUTH_UUID_LENGTH = 4 django_settings.TEMPLATE_CONTEXT_PROCESSORS += ( @@ -148,6 +155,9 @@ def _set_provider_settings(django_settings, enabled_providers, auth_info): def apply_settings(auth_info, django_settings): """Applies settings from auth_info dict to django_settings module.""" + if django_settings.FEATURES.get('ENABLE_DUMMY_THIRD_PARTY_AUTH_PROVIDER'): + # The Dummy provider is handy for testing and development. + from .dummy import DummyProvider # pylint: disable=unused-variable provider_names = auth_info.keys() provider.Registry.configure_once(provider_names) enabled_providers = provider.Registry.enabled() diff --git a/common/djangoapps/third_party_auth/tests/specs/base.py b/common/djangoapps/third_party_auth/tests/specs/base.py index 902a1beb1d..0db38295e1 100644 --- a/common/djangoapps/third_party_auth/tests/specs/base.py +++ b/common/djangoapps/third_party_auth/tests/specs/base.py @@ -140,7 +140,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): exception_middleware = middleware.ExceptionMiddleware() request, _ = self.get_request_and_strategy(auth_entry=auth_entry) response = exception_middleware.process_exception( - request, exceptions.AuthCanceled(request.social_strategy.backend)) + request, exceptions.AuthCanceled(request.backend)) location = response.get('Location') self.assertEqual(302, response.status_code) @@ -161,7 +161,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): """ _, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) self.create_user_models_for_existing_account( strategy, email, password, self.get_username(), skip_social_auth=True) @@ -239,7 +239,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): def assert_redirect_to_dashboard_looks_correct(self, response): """Asserts a response would redirect to /dashboard.""" self.assertEqual(302, response.status_code) - # pylint: disable-msg=protected-access + # pylint: disable=protected-access self.assertEqual(auth_settings._SOCIAL_AUTH_LOGIN_REDIRECT_URL, response.get('Location')) def assert_redirect_to_login_looks_correct(self, response): @@ -287,7 +287,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): See student.views.register and student.views._do_create_account. """ response_data = self.get_response_data() - uid = strategy.backend.get_user_id(response_data, response_data) + uid = strategy.request.backend.get_user_id(response_data, response_data) user = social_utils.Storage.user.create_user(email=email, password=password, username=username) profile = student_models.UserProfile(user=user) profile.save() @@ -310,7 +310,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): args = () kwargs = { 'request': strategy.request, - 'backend': strategy.backend, + 'backend': strategy.request.backend, 'user': None, 'response': self.get_response_data(), } @@ -355,8 +355,9 @@ class IntegrationTest(testutil.TestCase, test.TestCase): if auth_entry: request.session[pipeline.AUTH_ENTRY_KEY] = auth_entry - strategy = social_utils.load_strategy(backend=self.backend_name, redirect_uri=redirect_uri, request=request) + strategy = social_utils.load_strategy(request=request) request.social_strategy = strategy + request.backend = social_utils.load_backend(strategy, self.backend_name, redirect_uri) return request, strategy @@ -404,7 +405,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # configure the backend, and mock out wire traffic. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) pipeline.analytics.track = mock.MagicMock() request.user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username(), skip_social_auth=True) @@ -413,12 +414,12 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # expected state. self.client.get( pipeline.get_login_url(self.PROVIDER_CLASS.NAME, pipeline.AUTH_ENTRY_LOGIN)) - actions.do_complete(strategy, social_views._do_login) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login) # pylint: disable=protected-access mako_middleware_process_request(strategy.request) student_views.signin_user(strategy.request) student_views.login_user(strategy.request) - actions.do_complete(strategy, social_views._do_login) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login) # pylint: disable=protected-access # First we expect that we're in the unlinked state, and that there # really is no association in the backend. @@ -428,7 +429,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # We should be redirected back to the complete page, setting # the "logged in" cookie for the marketing site. self.assert_logged_in_cookie_redirect(actions.do_complete( - request.social_strategy, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access + request.backend, social_views._do_login, request.user, None, # pylint: disable=protected-access redirect_field_name=auth.REDIRECT_FIELD_NAME )) @@ -437,7 +438,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # Fire off the auth pipeline to link. self.assert_redirect_to_dashboard_looks_correct(actions.do_complete( - request.social_strategy, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access + request.backend, social_views._do_login, request.user, None, # pylint: disable=protected-access redirect_field_name=auth.REDIRECT_FIELD_NAME)) # Now we expect to be in the linked state, with a backend entry. @@ -449,7 +450,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # configure the backend, and mock out wire traffic. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username()) self.assert_social_auth_exists_for_user(user, strategy) @@ -461,12 +462,12 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # expected state. self.client.get( pipeline.get_login_url(self.PROVIDER_CLASS.NAME, pipeline.AUTH_ENTRY_LOGIN)) - actions.do_complete(strategy, social_views._do_login) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login) # pylint: disable=protected-access mako_middleware_process_request(strategy.request) student_views.signin_user(strategy.request) student_views.login_user(strategy.request) - actions.do_complete(strategy, social_views._do_login, user=user) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login, user=user) # pylint: disable=protected-access # First we expect that we're in the linked state, with a backend entry. self.assert_account_settings_context_looks_correct(account_settings_context(request), user, linked=True) @@ -474,7 +475,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # Fire off the disconnect pipeline to unlink. self.assert_redirect_to_dashboard_looks_correct(actions.do_disconnect( - request.social_strategy, request.user, None, redirect_field_name=auth.REDIRECT_FIELD_NAME)) + request.backend, request.user, None, redirect_field_name=auth.REDIRECT_FIELD_NAME)) # Now we expect to be in the unlinked state, with no backend entry. self.assert_account_settings_context_looks_correct(account_settings_context(request), user, linked=False) @@ -490,7 +491,8 @@ class IntegrationTest(testutil.TestCase, test.TestCase): username = self.get_username() _, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + backend = strategy.request.backend + backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) linked_user = self.create_user_models_for_existing_account(strategy, email, password, username) unlinked_user = social_utils.Storage.user.create_user( email='other_' + email, password=password, username='other_' + username) @@ -499,7 +501,8 @@ class IntegrationTest(testutil.TestCase, test.TestCase): self.assert_social_auth_does_not_exist_for_user(unlinked_user, strategy) with self.assertRaises(exceptions.AuthAlreadyAssociated): - actions.do_complete(strategy, social_views._do_login, user=unlinked_user) # pylint: disable-msg=protected-access + # pylint: disable=protected-access + actions.do_complete(backend, social_views._do_login, user=unlinked_user) def test_already_associated_exception_populates_dashboard_with_error(self): # Instrument the pipeline with an exception. We test that the @@ -511,21 +514,21 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # that the duplicate error has no effect on the state of the controls. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username()) self.assert_social_auth_exists_for_user(user, strategy) self.client.get('/login') self.client.get(pipeline.get_login_url(self.PROVIDER_CLASS.NAME, pipeline.AUTH_ENTRY_LOGIN)) - actions.do_complete(strategy, social_views._do_login) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login) # pylint: disable=protected-access mako_middleware_process_request(strategy.request) student_views.signin_user(strategy.request) student_views.login_user(strategy.request) - actions.do_complete(strategy, social_views._do_login, user=user) # pylint: disable-msg=protected-access + actions.do_complete(request.backend, social_views._do_login, user=user) # pylint: disable=protected-access - # Monkey-patch storage for messaging; pylint: disable-msg=protected-access + # Monkey-patch storage for messaging; pylint: disable=protected-access request._messages = fallback.FallbackStorage(request) middleware.ExceptionMiddleware().process_exception( request, @@ -539,7 +542,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # configure the backend, and mock out wire traffic. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) pipeline.analytics.track = mock.MagicMock() user = self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username()) @@ -558,8 +561,8 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # Next, the provider makes a request against /auth/complete/ # to resume the pipeline. - # pylint: disable-msg=protected-access - self.assert_redirect_to_login_looks_correct(actions.do_complete(strategy, social_views._do_login)) + # pylint: disable=protected-access + self.assert_redirect_to_login_looks_correct(actions.do_complete(request.backend, social_views._do_login)) mako_middleware_process_request(strategy.request) # At this point we know the pipeline has resumed correctly. Next we @@ -574,7 +577,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # We should be redirected back to the complete page, setting # the "logged in" cookie for the marketing site. self.assert_logged_in_cookie_redirect(actions.do_complete( - request.social_strategy, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access + request.backend, social_views._do_login, request.user, None, # pylint: disable=protected-access redirect_field_name=auth.REDIRECT_FIELD_NAME )) @@ -582,13 +585,13 @@ class IntegrationTest(testutil.TestCase, test.TestCase): self.set_logged_in_cookie(request) self.assert_redirect_to_dashboard_looks_correct( - actions.do_complete(strategy, social_views._do_login, user=user)) + actions.do_complete(request.backend, social_views._do_login, user=user)) self.assert_account_settings_context_looks_correct(account_settings_context(request), user) def test_signin_fails_if_account_not_active(self): _, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) user = self.create_user_models_for_existing_account(strategy, 'user@example.com', 'password', self.get_username()) user.is_active = False @@ -600,7 +603,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): def test_signin_fails_if_no_account_associated(self): _, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_LOGIN, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) self.create_user_models_for_existing_account( strategy, 'user@example.com', 'password', self.get_username(), skip_social_auth=True) @@ -625,7 +628,7 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # Mock out wire traffic. request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_REGISTER, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) # Begin! Grab the registration page and check the login control on it. self.assert_register_response_before_pipeline_looks_correct(self.client.get('/register')) @@ -637,8 +640,8 @@ class IntegrationTest(testutil.TestCase, test.TestCase): pipeline.get_login_url(self.PROVIDER_CLASS.NAME, pipeline.AUTH_ENTRY_LOGIN))) # Next, the provider makes a request against /auth/complete/. - # pylint: disable-msg=protected-access - self.assert_redirect_to_register_looks_correct(actions.do_complete(strategy, social_views._do_login)) + # pylint: disable=protected-access + self.assert_redirect_to_register_looks_correct(actions.do_complete(request.backend, social_views._do_login)) mako_middleware_process_request(strategy.request) # At this point we know the pipeline has resumed correctly. Next we @@ -672,33 +675,18 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # social auth. self.assert_social_auth_does_not_exist_for_user(created_user, strategy) - # Since the user's account is not yet active, we should be redirected to /login - self.assert_redirect_to_login_looks_correct( - actions.do_complete( - request.social_strategy, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access - redirect_field_name=auth.REDIRECT_FIELD_NAME - ) - ) - - # Activate the user's account - strategy.request.user.is_active = True - strategy.request.user.save() - - # Try again. This time, we should be redirected back to the complete page, setting + # We should be redirected back to the complete page, setting # the "logged in" cookie for the marketing site. self.assert_logged_in_cookie_redirect(actions.do_complete( - request.social_strategy, social_views._do_login, request.user, None, # pylint: disable-msg=protected-access + request.backend, social_views._do_login, request.user, None, # pylint: disable=protected-access redirect_field_name=auth.REDIRECT_FIELD_NAME )) # Set the cookie and try again self.set_logged_in_cookie(request) - - # Pick the pipeline back up. This will create the account association - # and send the user to the dashboard, where the association will be - # displayed. self.assert_redirect_to_dashboard_looks_correct( - actions.do_complete(strategy, social_views._do_login, user=created_user)) + actions.do_complete(strategy.request.backend, social_views._do_login, user=created_user)) + # Now the user has been redirected to the dashboard. Their third party account should now be linked. self.assert_social_auth_exists_for_user(created_user, strategy) self.assert_account_settings_context_looks_correct(account_settings_context(request), created_user, linked=True) @@ -710,18 +698,20 @@ class IntegrationTest(testutil.TestCase, test.TestCase): # Create a colliding username in the backend, then proceed with # assignment via pipeline to make sure a distinct username is created. strategy.storage.user.create_user(username=self.get_username(), email='user@email.com', password='password') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) - # pylint: disable-msg=protected-access - self.assert_redirect_to_register_looks_correct(actions.do_complete(strategy, social_views._do_login)) + backend = strategy.request.backend + backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + # pylint: disable=protected-access + self.assert_redirect_to_register_looks_correct(actions.do_complete(backend, social_views._do_login)) distinct_username = pipeline.get(request)['kwargs']['username'] self.assertNotEqual(original_username, distinct_username) def test_new_account_registration_fails_if_email_exists(self): request, strategy = self.get_request_and_strategy( auth_entry=pipeline.AUTH_ENTRY_REGISTER, redirect_uri='social:complete') - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) - # pylint: disable-msg=protected-access - self.assert_redirect_to_register_looks_correct(actions.do_complete(strategy, social_views._do_login)) + backend = strategy.request.backend + backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + # pylint: disable=protected-access + self.assert_redirect_to_register_looks_correct(actions.do_complete(backend, social_views._do_login)) mako_middleware_process_request(strategy.request) self.assert_register_response_in_pipeline_looks_correct( @@ -733,21 +723,21 @@ class IntegrationTest(testutil.TestCase, test.TestCase): def test_pipeline_raises_auth_entry_error_if_auth_entry_invalid(self): auth_entry = 'invalid' - self.assertNotIn(auth_entry, pipeline._AUTH_ENTRY_CHOICES) # pylint: disable-msg=protected-access + self.assertNotIn(auth_entry, pipeline._AUTH_ENTRY_CHOICES) # pylint: disable=protected-access _, strategy = self.get_request_and_strategy(auth_entry=auth_entry, redirect_uri='social:complete') with self.assertRaises(pipeline.AuthEntryError): - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) def test_pipeline_raises_auth_entry_error_if_auth_entry_missing(self): _, strategy = self.get_request_and_strategy(auth_entry=None, redirect_uri='social:complete') with self.assertRaises(pipeline.AuthEntryError): - strategy.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) + strategy.request.backend.auth_complete = mock.MagicMock(return_value=self.fake_auth_complete(strategy)) -class Oauth2IntegrationTest(IntegrationTest): # pylint: disable-msg=abstract-method +class Oauth2IntegrationTest(IntegrationTest): # pylint: disable=abstract-method """Base test case for integration tests of Oauth2 providers.""" # Dict of string -> object. Information about the token granted to the diff --git a/common/djangoapps/third_party_auth/tests/test_change_enrollment.py b/common/djangoapps/third_party_auth/tests/test_change_enrollment.py deleted file mode 100644 index 3fa4655673..0000000000 --- a/common/djangoapps/third_party_auth/tests/test_change_enrollment.py +++ /dev/null @@ -1,187 +0,0 @@ -# -*- coding: utf-8 -*- -"""Tests for the change enrollment step of the pipeline. """ -from collections import namedtuple - -import datetime -import unittest -from mock import patch -import ddt -import pytz -from util.testing import UrlResetMixin -from third_party_auth import pipeline -from shoppingcart.models import Order, PaidCourseRegistration # pylint: disable=import-error -from social.apps.django_app import utils as social_utils -from django.conf import settings -from django.contrib.sessions.backends import cache -from django.test import RequestFactory -from xmodule.modulestore.tests.factories import CourseFactory -from student.tests.factories import UserFactory, CourseModeFactory -from student.models import CourseEnrollment -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from openedx.core.djangoapps.user_api.models import UserOrgTag -from embargo.test_utils import restrict_course - - -THIRD_PARTY_AUTH_CONFIGURED = ( - settings.FEATURES.get('ENABLE_THIRD_PARTY_AUTH') and - getattr(settings, 'THIRD_PARTY_AUTH', {}) -) - - -@unittest.skipUnless(THIRD_PARTY_AUTH_CONFIGURED, "Third party auth must be configured") -@patch.dict(settings.FEATURES, {'EMBARGO': True}) -@ddt.ddt -class PipelineEnrollmentTest(UrlResetMixin, ModuleStoreTestCase): - """Test that the pipeline auto-enrolls students upon successful authentication. """ - - BACKEND_NAME = "google-oauth2" - - @patch.dict(settings.FEATURES, {'EMBARGO': True}) - def setUp(self): - """Create a test course and user. """ - super(PipelineEnrollmentTest, self).setUp('embargo') - self.course = CourseFactory.create() - self.user = UserFactory.create() - - @ddt.data( - ([], "honor", u"False", u"False"), - (["honor", "verified", "audit"], "honor", u"True", u"True"), - (["professional"], None, u"Fålsœ", u"False") - ) - @ddt.unpack - def test_auto_enroll_step(self, course_modes, enrollment_mode, email_opt_in, email_opt_in_result): - # Create the course modes for the test case - for mode_slug in course_modes: - CourseModeFactory.create( - course_id=self.course.id, - mode_slug=mode_slug, - mode_display_name=mode_slug.capitalize() - ) - - # Simulate the pipeline step, passing in a course ID - # to indicate that the user was trying to enroll - # when they started the auth process. - strategy = self._fake_strategy() - strategy.session_set('enroll_course_id', unicode(self.course.id)) - strategy.session_set('email_opt_in', email_opt_in) - - result = pipeline.change_enrollment(strategy, 1, user=self.user) # pylint: disable=assignment-from-no-return,redundant-keyword-arg - self.assertEqual(result, {}) - - # Check that the user was or was not enrolled - # (this will vary based on the course mode) - if enrollment_mode is not None: - actual_mode, is_active = CourseEnrollment.enrollment_mode_for_user(self.user, self.course.id) - self.assertTrue(is_active) - self.assertEqual(actual_mode, enrollment_mode) - else: - self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) - - # Check that the Email Opt In option was set - tag = UserOrgTag.objects.get(user=self.user) - self.assertIsNotNone(tag) - self.assertEquals(tag.value, email_opt_in_result) - - def test_add_white_label_to_cart(self): - # Create a white label course (honor with a minimum price) - CourseModeFactory.create( - course_id=self.course.id, - mode_slug="honor", - mode_display_name="Honor", - min_price=100 - ) - - # Simulate the pipeline step for enrolling in this course - strategy = self._fake_strategy() - strategy.session_set('enroll_course_id', unicode(self.course.id)) - result = pipeline.change_enrollment(strategy, 1, user=self.user) # pylint: disable=assignment-from-no-return,redundant-keyword-arg - self.assertEqual(result, {}) - - # Expect that the uesr is NOT enrolled in the course - # because the user has not yet paid - self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) - - # Expect that the course was added to the shopping cart - cart = Order.get_cart_for_user(self.user) - self.assertTrue(cart.has_items(PaidCourseRegistration)) - order_item = PaidCourseRegistration.objects.get(order=cart) - self.assertEqual(order_item.course_id, self.course.id) - - def test_auto_enroll_not_accessible(self): - # Set the course open date in the future - tomorrow = datetime.datetime.now(pytz.utc) + datetime.timedelta(days=1) - self.course.enrollment_start = tomorrow - self.update_course(self.course, self.user.id) - - # Finish authentication and try to auto-enroll - # This should fail silently, with no exception - strategy = self._fake_strategy() - strategy.session_set('enroll_course_id', unicode(self.course.id)) - result = pipeline.change_enrollment(strategy, 1, user=self.user) # pylint: disable=assignment-from-no-return,redundant-keyword-arg - self.assertEqual(result, {}) - - # Verify that we were NOT enrolled - self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) - - def test_no_course_id_skips_enroll(self): - strategy = self._fake_strategy() - result = pipeline.change_enrollment(strategy, 1, user=self.user) # pylint: disable=assignment-from-no-return,redundant-keyword-arg - self.assertEqual(result, {}) - self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) - - @patch.dict(settings.FEATURES, {'EMBARGO': True}) - def test_blocked_by_embargo(self): - strategy = self._fake_strategy() - strategy.session_set('enroll_course_id', unicode(self.course.id)) - - with restrict_course(self.course.id): - result = pipeline.change_enrollment(strategy, 1, user=self.user) # pylint: disable=assignment-from-no-return,redundant-keyword-arg - - # Verify that we were NOT enrolled - self.assertEqual(result, {}) - self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) - - def test_skip_enroll_from_dashboard(self): - strategy = self._fake_strategy() - strategy.session_set('enroll_course_id', unicode(self.course.id)) - - # Simulate completing the pipeline from the student account settings - # "link account" button. - result = pipeline.change_enrollment(strategy, 1, user=self.user, auth_entry=pipeline.AUTH_ENTRY_ACCOUNT_SETTINGS) # pylint: disable=assignment-from-no-return,redundant-keyword-arg - - # Verify that we were NOT enrolled - self.assertEqual(result, {}) - self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) - - def test_url_creation(self): - strategy = self._fake_strategy() - strategy.session_set('enroll_course_id', unicode(self.course.id)) - strategy.session_set('email_opt_in', u"False") - backend = namedtuple('backend', 'name') - backend.name = self.BACKEND_NAME - response = pipeline.ensure_user_information( - strategy=strategy, - pipeline_index=1, - details=None, - response=None, - uid=None, - auth_entry=pipeline.AUTH_ENTRY_REGISTER, - backend=backend - ) - self.assertIsNotNone(response) - self.assertEquals(response.status_code, 302) - - # Get the location - _, url = response._headers['location'] # pylint: disable=W0212 - self.assertIn("email_opt_in=False", url) - self.assertIn("course_id=".format(id=unicode(self.course.id)), url) - - def _fake_strategy(self): - """Simulate the strategy passed to the pipeline step. """ - request = RequestFactory().get(pipeline.get_complete_url(self.BACKEND_NAME)) - request.user = self.user - request.session = cache.SessionStore() - - return social_utils.load_strategy( - backend=self.BACKEND_NAME, request=request - ) diff --git a/common/djangoapps/third_party_auth/tests/utils.py b/common/djangoapps/third_party_auth/tests/utils.py index 6dcc2662cd..208930cdf4 100644 --- a/common/djangoapps/third_party_auth/tests/utils.py +++ b/common/djangoapps/third_party_auth/tests/utils.py @@ -66,7 +66,7 @@ class ThirdPartyOAuthTestMixin(object): class ThirdPartyOAuthTestMixinFacebook(object): """Tests oauth with the Facebook backend""" BACKEND = "facebook" - USER_URL = "https://graph.facebook.com/me" + USER_URL = "https://graph.facebook.com/v2.3/me" # In facebook responses, the "id" field is used as the user's identifier UID_FIELD = "id" @@ -74,6 +74,6 @@ class ThirdPartyOAuthTestMixinFacebook(object): class ThirdPartyOAuthTestMixinGoogle(object): """Tests oauth with the Google backend""" BACKEND = "google-oauth2" - USER_URL = "https://www.googleapis.com/oauth2/v1/userinfo" + USER_URL = "https://www.googleapis.com/plus/v1/people/me" # In google-oauth2 responses, the "email" field is used as the user's identifier UID_FIELD = "email" diff --git a/common/djangoapps/third_party_auth/urls.py b/common/djangoapps/third_party_auth/urls.py index dc02425ef3..b020e775b5 100644 --- a/common/djangoapps/third_party_auth/urls.py +++ b/common/djangoapps/third_party_auth/urls.py @@ -2,8 +2,10 @@ from django.conf.urls import include, patterns, url +from .views import inactive_user_view urlpatterns = patterns( '', + url(r'^auth/inactive', inactive_user_view), url(r'^auth/', include('social.apps.django_app.urls', namespace='social')), ) diff --git a/common/djangoapps/third_party_auth/views.py b/common/djangoapps/third_party_auth/views.py new file mode 100644 index 0000000000..5ae69db526 --- /dev/null +++ b/common/djangoapps/third_party_auth/views.py @@ -0,0 +1,15 @@ +""" +Extra views required for SSO +""" +from django.shortcuts import redirect + + +def inactive_user_view(request): + """ + A newly registered user has completed the social auth pipeline. + Their account is not yet activated, but we let them login this once. + """ + # 'next' may be set to '/account/finish_auth/.../' if this user needs to be auto-enrolled + # in a course. Otherwise, just redirect them to the dashboard, which displays a message + # about activating their account. + return redirect(request.GET.get('next', 'dashboard')) diff --git a/common/test/acceptance/pages/lms/fields.py b/common/test/acceptance/pages/lms/fields.py index d7e88a417a..803f5a7886 100644 --- a/common/test/acceptance/pages/lms/fields.py +++ b/common/test/acceptance/pages/lms/fields.py @@ -80,7 +80,7 @@ class FieldsMixin(object): query = self.q(css='.u-field-{} .u-field-message'.format(field_id)) return query.text[0] if query.present else None - def wait_for_messsage(self, field_id, message): + def wait_for_message(self, field_id, message): """ Wait for a message to appear in a field. """ diff --git a/common/test/acceptance/pages/lms/login_and_register.py b/common/test/acceptance/pages/lms/login_and_register.py index 2a917c2eb6..b61e25c547 100644 --- a/common/test/acceptance/pages/lms/login_and_register.py +++ b/common/test/acceptance/pages/lms/login_and_register.py @@ -187,10 +187,14 @@ class CombinedLoginAndRegisterPage(PageObject): """ # Fill in the form self.wait_for_element_visibility('#register-email', 'Email field is shown') - self.q(css="#register-email").fill(email) - self.q(css="#register-name").fill(full_name) - self.q(css="#register-username").fill(username) - self.q(css="#register-password").fill(password) + if email: + self.q(css="#register-email").fill(email) + if full_name: + self.q(css="#register-name").fill(full_name) + if username: + self.q(css="#register-username").fill(username) + if password: + self.q(css="#register-password").fill(password) if country: self.q(css="#register-country option[value='{country}']".format(country=country)).click() if (terms_of_service): @@ -220,6 +224,16 @@ class CombinedLoginAndRegisterPage(PageObject): # Submit it self.q(css=".login-button").click() + def click_third_party_dummy_provider(self): + """Clicks on the Dummy third party provider login button. + + Requires that the "login" form is visible. + This does NOT wait for the ensuing page[s] to load. + Only the "Dummy" provider is used for bok choy because it is the only + one that doesn't send traffic to external servers. + """ + self.q(css="button.{}-Dummy".format(self.current_form)).click() + def password_reset(self, email): """Navigates to, fills in, and submits the password reset form. @@ -268,6 +282,21 @@ class CombinedLoginAndRegisterPage(PageObject): elif self.q(css=".js-reset").visible: return "password-reset" + @property + def email_value(self): + """ Current value of the email form field """ + return self.q(css="#register-email").attrs('value')[0] + + @property + def full_name_value(self): + """ Current value of the full_name form field """ + return self.q(css="#register-name").attrs('value')[0] + + @property + def username_value(self): + """ Current value of the username form field """ + return self.q(css="#register-username").attrs('value')[0] + @property def errors(self): """Return a list of errors displayed to the user. """ @@ -294,3 +323,15 @@ class CombinedLoginAndRegisterPage(PageObject): success = self.success return (bool(success), success) return Promise(_check_func, "Success message is visible").fulfill() + + @unguarded # Because we go from this page -> temporary page -> this page again when testing the Dummy provider + def wait_for_auth_status_message(self): + """Wait for a status message to be visible following third_party registration, then return it.""" + def _check_func(): + """Return third party auth status notice message.""" + for selector in ['.already-authenticated-msg p', '.status p']: + msg_element = self.q(css=selector) + if msg_element.visible: + return (True, msg_element.text[0]) + return (False, None) + return Promise(_check_func, "Result of third party auth is visible").fulfill() diff --git a/common/test/acceptance/tests/lms/test_account_settings.py b/common/test/acceptance/tests/lms/test_account_settings.py index 1881e29006..c9ab3eb051 100644 --- a/common/test/acceptance/tests/lms/test_account_settings.py +++ b/common/test/acceptance/tests/lms/test_account_settings.py @@ -177,6 +177,7 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest): { 'title': 'Connected Accounts', 'fields': [ + 'Dummy', 'Facebook', 'Google', ] @@ -211,7 +212,7 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest): for new_value in new_valid_values: self.assertEqual(self.account_settings_page.value_for_text_field(field_id, new_value), new_value) - self.account_settings_page.wait_for_messsage(field_id, success_message) + self.account_settings_page.wait_for_message(field_id, success_message) if assert_after_reload: self.browser.refresh() self.assertEqual(self.account_settings_page.value_for_text_field(field_id), new_value) @@ -227,7 +228,7 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest): for new_value in new_values: self.assertEqual(self.account_settings_page.value_for_dropdown_field(field_id, new_value), new_value) - self.account_settings_page.wait_for_messsage(field_id, success_message) + self.account_settings_page.wait_for_message(field_id, success_message) if reloads_on_save: self.account_settings_page.wait_for_loading_indicator() else: @@ -242,7 +243,7 @@ class AccountSettingsPageTest(AccountSettingsTestMixin, WebAppTest): self.assertEqual(self.account_settings_page.title_for_field(field_id), title) self.assertEqual(self.account_settings_page.link_title_for_link_field(field_id), link_title) self.account_settings_page.click_on_link_in_link_field(field_id) - self.account_settings_page.wait_for_messsage(field_id, success_message) + self.account_settings_page.wait_for_message(field_id, success_message) def test_username_field(self): """ diff --git a/common/test/acceptance/tests/lms/test_lms.py b/common/test/acceptance/tests/lms/test_lms.py index f2faded951..ad885e34dc 100644 --- a/common/test/acceptance/tests/lms/test_lms.py +++ b/common/test/acceptance/tests/lms/test_lms.py @@ -18,6 +18,7 @@ from ..helpers import ( select_option_by_value, element_has_text ) +from ...pages.lms.account_settings import AccountSettingsPage from ...pages.lms.auto_auth import AutoAuthPage from ...pages.lms.create_mode import ModeCreationPage from ...pages.common.logout import LogoutPage @@ -131,6 +132,46 @@ class LoginFromCombinedPageTest(UniqueCourseTest): self.login_page.wait_for_errors() ) + def test_third_party_login(self): + """ + Test that we can login using third party credentials, and that the + third party account gets linked to the edX account. + """ + # Create a user account + email, password = self._create_unique_user() + + # Navigate to the login page and try to log in using "Dummy" provider + self.login_page.visit() + self.login_page.click_third_party_dummy_provider() + + # The user will be redirected somewhere and then back to the login page: + msg_text = self.login_page.wait_for_auth_status_message() + self.assertIn("You have successfully signed into Dummy", msg_text) + self.assertIn("To link your accounts, sign in now using your edX password", msg_text) + + # Now login with username and password: + self.login_page.login(email=email, password=password) + + # Expect that we reach the dashboard and we're auto-enrolled in the course + course_names = self.dashboard_page.wait_for_page().available_courses + self.assertIn(self.course_info["display_name"], course_names) + + # Now logout and check that we can log back in instantly (because the account is linked): + LogoutPage(self.browser).visit() + + self.login_page.visit() + self.login_page.click_third_party_dummy_provider() + + self.dashboard_page.wait_for_page() + + # Now unlink the account (To test the account settings view and also to prevent cross-test side effects) + account_settings = AccountSettingsPage(self.browser).visit() + field_id = "auth-dummy" + account_settings.wait_for_field(field_id) + self.assertEqual("Unlink", account_settings.link_title_for_link_field(field_id)) + account_settings.click_on_link_in_link_field(field_id) + account_settings.wait_for_message(field_id, "Successfully unlinked") + def _create_unique_user(self): """ Create a new user with a unique name and email. @@ -226,6 +267,50 @@ class RegisterFromCombinedPageTest(UniqueCourseTest): self.register_page.visit().toggle_form() self.assertEqual(self.register_page.current_form, "login") + def test_third_party_register(self): + """ + Test that we can register using third party credentials, and that the + third party account gets linked to the edX account. + """ + # Navigate to the register page and try to authenticate using the "Dummy" provider + self.register_page.visit() + self.register_page.click_third_party_dummy_provider() + + # The user will be redirected somewhere and then back to the register page: + msg_text = self.register_page.wait_for_auth_status_message() + self.assertEqual(self.register_page.current_form, "register") + self.assertIn("You've successfully signed into Dummy", msg_text) + self.assertIn("We just need a little more information", msg_text) + + # Now the form should be pre-filled with the data from the Dummy provider: + self.assertEqual(self.register_page.email_value, "adama@fleet.colonies.gov") + self.assertEqual(self.register_page.full_name_value, "William Adama") + self.assertIn("Galactica1", self.register_page.username_value) + + # Set country, accept the terms, and submit the form: + self.register_page.register(country="US", terms_of_service=True) + + # Expect that we reach the dashboard and we're auto-enrolled in the course + course_names = self.dashboard_page.wait_for_page().available_courses + self.assertIn(self.course_info["display_name"], course_names) + + # Now logout and check that we can log back in instantly (because the account is linked): + LogoutPage(self.browser).visit() + + login_page = CombinedLoginAndRegisterPage(self.browser, start_page="login") + login_page.visit() + login_page.click_third_party_dummy_provider() + + self.dashboard_page.wait_for_page() + + # Now unlink the account (To test the account settings view and also to prevent cross-test side effects) + account_settings = AccountSettingsPage(self.browser).visit() + field_id = "auth-dummy" + account_settings.wait_for_field(field_id) + self.assertEqual("Unlink", account_settings.link_title_for_link_field(field_id)) + account_settings.click_on_link_in_link_field(field_id) + account_settings.wait_for_message(field_id, "Successfully unlinked") + @attr('shard_4') class PayAndVerifyTest(EventsTestMixin, UniqueCourseTest): diff --git a/common/test/db_cache/bok_choy_schema.sql b/common/test/db_cache/bok_choy_schema.sql index 258ca4a755..bbba6a4aed 100644 --- a/common/test/db_cache/bok_choy_schema.sql +++ b/common/test/db_cache/bok_choy_schema.sql @@ -2468,59 +2468,6 @@ CREATE TABLE `shoppingcart_registrationcoderedemption` ( CONSTRAINT `registration_code_id_refs_id_4d01e47b` FOREIGN KEY (`registration_code_id`) REFERENCES `shoppingcart_courseregistrationcode` (`id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8; /*!40101 SET character_set_client = @saved_cs_client */; -DROP TABLE IF EXISTS `social_auth_association`; -/*!40101 SET @saved_cs_client = @@character_set_client */; -/*!40101 SET character_set_client = utf8 */; -CREATE TABLE `social_auth_association` ( - `id` int(11) NOT NULL AUTO_INCREMENT, - `server_url` varchar(255) NOT NULL, - `handle` varchar(255) NOT NULL, - `secret` varchar(255) NOT NULL, - `issued` int(11) NOT NULL, - `lifetime` int(11) NOT NULL, - `assoc_type` varchar(64) NOT NULL, - PRIMARY KEY (`id`) -) ENGINE=InnoDB DEFAULT CHARSET=utf8; -/*!40101 SET character_set_client = @saved_cs_client */; -DROP TABLE IF EXISTS `social_auth_code`; -/*!40101 SET @saved_cs_client = @@character_set_client */; -/*!40101 SET character_set_client = utf8 */; -CREATE TABLE `social_auth_code` ( - `id` int(11) NOT NULL AUTO_INCREMENT, - `email` varchar(75) NOT NULL, - `code` varchar(32) NOT NULL, - `verified` tinyint(1) NOT NULL, - PRIMARY KEY (`id`), - UNIQUE KEY `email` (`email`,`code`), - KEY `social_auth_code_65da3d2c` (`code`) -) ENGINE=InnoDB DEFAULT CHARSET=utf8; -/*!40101 SET character_set_client = @saved_cs_client */; -DROP TABLE IF EXISTS `social_auth_nonce`; -/*!40101 SET @saved_cs_client = @@character_set_client */; -/*!40101 SET character_set_client = utf8 */; -CREATE TABLE `social_auth_nonce` ( - `id` int(11) NOT NULL AUTO_INCREMENT, - `server_url` varchar(255) NOT NULL, - `timestamp` int(11) NOT NULL, - `salt` varchar(65) NOT NULL, - PRIMARY KEY (`id`) -) ENGINE=InnoDB DEFAULT CHARSET=utf8; -/*!40101 SET character_set_client = @saved_cs_client */; -DROP TABLE IF EXISTS `social_auth_usersocialauth`; -/*!40101 SET @saved_cs_client = @@character_set_client */; -/*!40101 SET character_set_client = utf8 */; -CREATE TABLE `social_auth_usersocialauth` ( - `id` int(11) NOT NULL AUTO_INCREMENT, - `user_id` int(11) NOT NULL, - `provider` varchar(32) NOT NULL, - `uid` varchar(255) NOT NULL, - `extra_data` longtext NOT NULL, - PRIMARY KEY (`id`), - UNIQUE KEY `provider` (`provider`,`uid`), - KEY `social_auth_usersocialauth_fbfc09f1` (`user_id`), - CONSTRAINT `user_id_refs_id_60fa311b` FOREIGN KEY (`user_id`) REFERENCES `auth_user` (`id`) -) ENGINE=InnoDB DEFAULT CHARSET=utf8; -/*!40101 SET character_set_client = @saved_cs_client */; DROP TABLE IF EXISTS `south_migrationhistory`; /*!40101 SET @saved_cs_client = @@character_set_client */; /*!40101 SET character_set_client = utf8 */; diff --git a/common/test/db_cache/lettuce.db b/common/test/db_cache/lettuce.db index 5060b4b2ee..3bd9676090 100644 Binary files a/common/test/db_cache/lettuce.db and b/common/test/db_cache/lettuce.db differ diff --git a/lms/djangoapps/student_account/helpers.py b/lms/djangoapps/student_account/helpers.py deleted file mode 100644 index 5c39d8141f..0000000000 --- a/lms/djangoapps/student_account/helpers.py +++ /dev/null @@ -1,77 +0,0 @@ -"""Helper functions for the student account app. """ -from django.core.urlresolvers import reverse -from opaque_keys.edx.keys import CourseKey -from course_modes.models import CourseMode -from third_party_auth import ( # pylint: disable=W0611 - pipeline, provider, - is_enabled as third_party_auth_enabled -) - - -def auth_pipeline_urls(auth_entry, redirect_url=None, course_id=None, email_opt_in=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. - - course_id (unicode): The ID of the course the user is enrolling in. - We use this to send users to the track selection page - if the course has a payment option. - Note that `redirect_url` takes precedence over the redirect - to the track selection page. - - email_opt_in (unicode): The user choice to opt in for organization wide emails. If set to 'true' - (case insensitive), user will be opted into organization-wide email. All other values will - be treated as False, and the user will be opted out of organization-wide email. - - Returns: - dict mapping provider names to URLs - - """ - if not third_party_auth_enabled(): - return {} - - if redirect_url is not None: - pipeline_redirect = redirect_url - elif course_id is not None: - # If the course is white-label (paid), then we send users - # to the shopping cart. (There is a third party auth pipeline - # step that will add the course to the cart.) - if CourseMode.is_white_label(CourseKey.from_string(course_id)): - pipeline_redirect = reverse("shoppingcart.views.show_cart") - - # Otherwise, send the user to the track selection page. - # The track selection page may redirect the user to the dashboard - # (if the only available mode is honor), or directly to verification - # (for professional ed). - else: - pipeline_redirect = reverse( - "course_modes_choose", - kwargs={'course_id': unicode(course_id)} - ) - else: - pipeline_redirect = None - - return { - provider.NAME: pipeline.get_login_url( - provider.NAME, auth_entry, - enroll_course_id=course_id, - email_opt_in=email_opt_in, - redirect_url=pipeline_redirect - ) - for provider in provider.Registry.enabled() - } diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index 880a622b74..ed8a10aaae 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -245,36 +245,39 @@ class StudentAccountLoginAndRegistrationTest(UrlResetMixin, ModuleStoreTestCase) ) @ddt.unpack def test_login_and_registration_form_signin_preserves_params(self, is_edx_domain, url_name): - params = { - 'enrollment_action': 'enroll', - 'course_id': 'edX/DemoX/Demo_Course' - } + params = [ + ('course_id', 'edX/DemoX/Demo_Course'), + ('enrollment_action', 'enroll'), + ] # The response should have a "Sign In" button with the URL # that preserves the querystring params with mock.patch.dict(settings.FEATURES, {'IS_EDX_DOMAIN': is_edx_domain}): response = self.client.get(reverse(url_name), params) - self.assertContains(response, "login?course_id=edX%2FDemoX%2FDemo_Course&enrollment_action=enroll") + expected_url = '/login?{}'.format(self._finish_auth_url_param(params + [('next', '/dashboard')])) + self.assertContains(response, expected_url) - # Add an additional "course mode" parameter - params['course_mode'] = 'honor' + # Add additional parameters: + params = [ + ('course_id', 'edX/DemoX/Demo_Course'), + ('enrollment_action', 'enroll'), + ('course_mode', 'honor'), + ('email_opt_in', 'true'), + ('next', '/custom/final/destination') + ] # Verify that this parameter is also preserved with mock.patch.dict(settings.FEATURES, {'IS_EDX_DOMAIN': is_edx_domain}): response = self.client.get(reverse(url_name), params) - expected_url = ( - "login?course_id=edX%2FDemoX%2FDemo_Course" - "&enrollment_action=enroll" - "&course_mode=honor" - ) + expected_url = '/login?{}'.format(self._finish_auth_url_param(params)) self.assertContains(response, expected_url) @mock.patch.dict(settings.FEATURES, {"ENABLE_THIRD_PARTY_AUTH": False}) @ddt.data("account_login", "account_register") def test_third_party_auth_disabled(self, url_name): response = self.client.get(reverse(url_name)) - self._assert_third_party_auth_data(response, None, []) + self._assert_third_party_auth_data(response, None, None, []) @ddt.data( ("account_login", None, None), @@ -286,175 +289,40 @@ class StudentAccountLoginAndRegistrationTest(UrlResetMixin, ModuleStoreTestCase) ) @ddt.unpack def test_third_party_auth(self, url_name, current_backend, current_provider): + params = [ + ('course_id', 'edX/DemoX/Demo_Course'), + ('enrollment_action', 'enroll'), + ('course_mode', 'honor'), + ('email_opt_in', 'true'), + ('next', '/custom/final/destination'), + ] + # Simulate a running pipeline if current_backend is not None: pipeline_target = "student_account.views.third_party_auth.pipeline" with simulate_running_pipeline(pipeline_target, current_backend): - response = self.client.get(reverse(url_name)) + response = self.client.get(reverse(url_name), params) # Do NOT simulate a running pipeline else: - response = self.client.get(reverse(url_name)) + response = self.client.get(reverse(url_name), params) # This relies on the THIRD_PARTY_AUTH configuration in the test settings expected_providers = [ { "name": "Facebook", "iconClass": "fa-facebook", - "loginUrl": self._third_party_login_url("facebook", "login"), - "registerUrl": self._third_party_login_url("facebook", "register") + "loginUrl": self._third_party_login_url("facebook", "login", params), + "registerUrl": self._third_party_login_url("facebook", "register", params) }, { "name": "Google", "iconClass": "fa-google-plus", - "loginUrl": self._third_party_login_url("google-oauth2", "login"), - "registerUrl": self._third_party_login_url("google-oauth2", "register") + "loginUrl": self._third_party_login_url("google-oauth2", "login", params), + "registerUrl": self._third_party_login_url("google-oauth2", "register", params) } ] - self._assert_third_party_auth_data(response, current_provider, expected_providers) - - @ddt.data([], ["honor"], ["honor", "verified", "audit"], ["professional"], ["no-id-professional"]) - def test_third_party_auth_course_id_verified(self, modes): - # Create a course with the specified course modes - course = CourseFactory.create() - for slug in modes: - CourseModeFactory.create( - course_id=course.id, - mode_slug=slug, - mode_display_name=slug - ) - - # Verify that the entry URL for third party auth - # contains the course ID and redirects to the track selection page. - course_modes_choose_url = reverse( - "course_modes_choose", - kwargs={"course_id": unicode(course.id)} - ) - expected_providers = [ - { - "name": "Facebook", - "iconClass": "fa-facebook", - "loginUrl": self._third_party_login_url( - "facebook", "login", - course_id=unicode(course.id), - redirect_url=course_modes_choose_url - ), - "registerUrl": self._third_party_login_url( - "facebook", "register", - course_id=unicode(course.id), - redirect_url=course_modes_choose_url - ) - }, - { - "name": "Google", - "iconClass": "fa-google-plus", - "loginUrl": self._third_party_login_url( - "google-oauth2", "login", - course_id=unicode(course.id), - redirect_url=course_modes_choose_url - ), - "registerUrl": self._third_party_login_url( - "google-oauth2", "register", - course_id=unicode(course.id), - redirect_url=course_modes_choose_url - ) - } - ] - - # Verify that the login page contains the correct provider URLs - response = self.client.get(reverse("account_login"), {"course_id": unicode(course.id)}) - self._assert_third_party_auth_data(response, None, expected_providers) - - def test_third_party_auth_course_id_shopping_cart(self): - # Create a course with a white-label course mode - course = CourseFactory.create() - CourseModeFactory.create( - course_id=course.id, - mode_slug="honor", - mode_display_name="Honor", - min_price=100 - ) - - # Verify that the entry URL for third party auth - # contains the course ID and redirects to the shopping cart - shoppingcart_url = reverse("shoppingcart.views.show_cart") - expected_providers = [ - { - "name": "Facebook", - "iconClass": "fa-facebook", - "loginUrl": self._third_party_login_url( - "facebook", "login", - course_id=unicode(course.id), - redirect_url=shoppingcart_url - ), - "registerUrl": self._third_party_login_url( - "facebook", "register", - course_id=unicode(course.id), - redirect_url=shoppingcart_url - ) - }, - { - "name": "Google", - "iconClass": "fa-google-plus", - "loginUrl": self._third_party_login_url( - "google-oauth2", "login", - course_id=unicode(course.id), - redirect_url=shoppingcart_url - ), - "registerUrl": self._third_party_login_url( - "google-oauth2", "register", - course_id=unicode(course.id), - redirect_url=shoppingcart_url - ) - } - ] - - # Verify that the login page contains the correct provider URLs - response = self.client.get(reverse("account_login"), {"course_id": unicode(course.id)}) - self._assert_third_party_auth_data(response, None, expected_providers) - - @mock.patch.dict(settings.FEATURES, {'EMBARGO': True}) - def test_third_party_auth_enrollment_embargo(self): - course = CourseFactory.create() - - # Start the pipeline attempting to enroll in a restricted course - with restrict_course(course.id) as redirect_url: - response = self.client.get(reverse("account_login"), {"course_id": unicode(course.id)}) - - # Expect that the course ID has been removed from the - # login URLs (so the user won't be enrolled) and - # the ?next param sends users to the blocked message. - expected_providers = [ - { - "name": "Facebook", - "iconClass": "fa-facebook", - "loginUrl": self._third_party_login_url( - "facebook", "login", - course_id=unicode(course.id), - redirect_url=redirect_url - ), - "registerUrl": self._third_party_login_url( - "facebook", "register", - course_id=unicode(course.id), - redirect_url=redirect_url - ) - }, - { - "name": "Google", - "iconClass": "fa-google-plus", - "loginUrl": self._third_party_login_url( - "google-oauth2", "login", - course_id=unicode(course.id), - redirect_url=redirect_url - ), - "registerUrl": self._third_party_login_url( - "google-oauth2", "register", - course_id=unicode(course.id), - redirect_url=redirect_url - ) - } - ] - self._assert_third_party_auth_data(response, None, expected_providers) + self._assert_third_party_auth_data(response, current_backend, current_provider, expected_providers) @override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME) def test_microsite_uses_old_login_page(self): @@ -477,33 +345,42 @@ class StudentAccountLoginAndRegistrationTest(UrlResetMixin, ModuleStoreTestCase) self.assertContains(resp, "Register for Test Microsite") self.assertContains(resp, "register-form") - def _assert_third_party_auth_data(self, response, current_provider, providers): + def _assert_third_party_auth_data(self, response, current_backend, current_provider, providers): """Verify that third party auth info is rendered correctly in a DOM data attribute. """ auth_info = markupsafe.escape( json.dumps({ "currentProvider": current_provider, - "providers": providers + "providers": providers, + "finishAuthUrl": "/auth/complete/{}?".format(current_backend) if current_backend else None, + "errorMessage": None, }) ) expected_data = u"data-third-party-auth='{auth_info}'".format( auth_info=auth_info ) + self.assertContains(response, expected_data) - def _third_party_login_url(self, backend_name, auth_entry, course_id=None, redirect_url=None): + def _third_party_login_url(self, backend_name, auth_entry, login_params): """Construct the login URL to start third party authentication. """ - params = [("auth_entry", auth_entry)] - if redirect_url: - params.append(("next", redirect_url)) - if course_id: - params.append(("enroll_course_id", course_id)) - - return u"{url}?{params}".format( + return u"{url}?auth_entry={auth_entry}&{param_str}".format( url=reverse("social:begin", kwargs={"backend": backend_name}), - params=urlencode(params) + auth_entry=auth_entry, + param_str=self._finish_auth_url_param(login_params), ) + def _finish_auth_url_param(self, params): + """ + Make the next=... URL parameter that indicates where the user should go next. + + >>> _finish_auth_url_param([('next', '/dashboard')]) + '/account/finish_auth?next=%2Fdashboard' + """ + return urlencode({ + 'next': '/account/finish_auth?{}'.format(urlencode(params)) + }) + class AccountSettingsViewTest(TestCase): """ Tests for the account settings view. """ diff --git a/lms/djangoapps/student_account/urls.py b/lms/djangoapps/student_account/urls.py index a172f25ba4..2792c46b82 100644 --- a/lms/djangoapps/student_account/urls.py +++ b/lms/djangoapps/student_account/urls.py @@ -1,7 +1,6 @@ from django.conf.urls import patterns, url from django.conf import settings - urlpatterns = [] if settings.FEATURES.get('ENABLE_COMBINED_LOGIN_REGISTRATION'): @@ -14,5 +13,6 @@ if settings.FEATURES.get('ENABLE_COMBINED_LOGIN_REGISTRATION'): urlpatterns += patterns( 'student_account.views', + url(r'^finish_auth$', 'finish_auth', name='finish_auth'), url(r'^settings$', 'account_settings', name='account_settings'), ) diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index 19d5b61ce5..dc178285ff 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -2,7 +2,6 @@ import logging import json -from ipware.ip import get_ip from django.conf import settings from django.contrib import messages @@ -19,12 +18,9 @@ from django.views.decorators.csrf import ensure_csrf_cookie from django.views.decorators.http import require_http_methods from lang_pref.api import released_languages -from opaque_keys import InvalidKeyError -from opaque_keys.edx.keys import CourseKey from edxmako.shortcuts import render_to_response from microsite_configuration import microsite -from embargo import api as embargo_api from external_auth.login_and_register import ( login as external_auth_login, register as external_auth_register @@ -34,16 +30,13 @@ from student.views import ( signin_user as old_login_view, register_user as old_register_view ) -from student_account.helpers import auth_pipeline_urls +from student.helpers import get_next_url_for_login_page import third_party_auth from third_party_auth import pipeline from util.bad_request_rate_limiter import BadRequestRateLimiter from openedx.core.djangoapps.user_api.accounts.api import request_password_change from openedx.core.djangoapps.user_api.errors import UserNotFound -from util.bad_request_rate_limiter import BadRequestRateLimiter - -from student_account.helpers import auth_pipeline_urls AUDIT_LOG = logging.getLogger("audit") @@ -61,9 +54,12 @@ def login_and_registration_form(request, initial_mode="login"): initial_mode (string): Either "login" or "register". """ + # Determine the URL to redirect to following login/registration/third_party_auth + redirect_to = get_next_url_for_login_page(request) + # If we're already logged in, redirect to the dashboard if request.user.is_authenticated(): - return redirect(reverse('dashboard')) + return redirect(redirect_to) # Retrieve the form descriptions from the user API form_descriptions = _get_form_descriptions(request) @@ -83,9 +79,10 @@ def login_and_registration_form(request, initial_mode="login"): # Otherwise, render the combined login/registration page context = { + 'login_redirect_url': redirect_to, # This gets added to the query string of the "Sign In" button in the header 'disable_courseware_js': True, 'initial_mode': initial_mode, - 'third_party_auth': json.dumps(_third_party_auth_context(request)), + 'third_party_auth': json.dumps(_third_party_auth_context(request, redirect_to)), 'platform_name': settings.PLATFORM_NAME, 'responsive': True, @@ -96,12 +93,6 @@ def login_and_registration_form(request, initial_mode="login"): 'login_form_desc': form_descriptions['login'], 'registration_form_desc': form_descriptions['registration'], 'password_reset_form_desc': form_descriptions['password_reset'], - - # We need to pass these parameters so that the header's - # "Sign In" button preserves the querystring params. - 'enrollment_action': request.GET.get('enrollment_action'), - 'course_id': request.GET.get('course_id'), - 'course_mode': request.GET.get('course_mode'), } return render_to_response('student_account/login_and_register.html', context) @@ -157,12 +148,14 @@ def password_change_request_handler(request): return HttpResponseBadRequest(_("No email address provided.")) -def _third_party_auth_context(request): +def _third_party_auth_context(request, redirect_to): """Context for third party auth providers and the currently running pipeline. Arguments: request (HttpRequest): The request, used to determine if a pipeline is currently running. + redirect_to: The URL to send the user to following successful + authentication. Returns: dict @@ -170,72 +163,43 @@ def _third_party_auth_context(request): """ context = { "currentProvider": None, - "providers": [] + "providers": [], + "finishAuthUrl": None, + "errorMessage": None, } - course_id = request.GET.get("course_id") - email_opt_in = request.GET.get('email_opt_in') - redirect_to = request.GET.get("next") - - # Check if the user is trying to enroll in a course - # that they don't have access to based on country - # access rules. - # - # If so, set the redirect URL to the blocked page. - # We need to set it here, rather than redirecting - # from within the pipeline, because a redirect - # from the pipeline can prevent users - # from completing the authentication process. - # - # Note that we can't check the user's country - # profile at this point, since the user hasn't - # authenticated. If the user ends up being blocked - # by their country preference, we let them enroll; - # they'll still be blocked when they try to access - # the courseware. - if course_id: - try: - course_key = CourseKey.from_string(course_id) - redirect_url = embargo_api.redirect_if_blocked( - course_key, - ip_address=get_ip(request), - url=request.path - ) - if redirect_url: - redirect_to = embargo_api.message_url_path(course_key, "enrollment") - except InvalidKeyError: - pass - - login_urls = auth_pipeline_urls( - third_party_auth.pipeline.AUTH_ENTRY_LOGIN, - course_id=course_id, - email_opt_in=email_opt_in, - redirect_url=redirect_to - ) - register_urls = auth_pipeline_urls( - third_party_auth.pipeline.AUTH_ENTRY_REGISTER, - course_id=course_id, - email_opt_in=email_opt_in, - redirect_url=redirect_to - ) - if third_party_auth.is_enabled(): context["providers"] = [ { "name": enabled.NAME, "iconClass": enabled.ICON_CLASS, - "loginUrl": login_urls[enabled.NAME], - "registerUrl": register_urls[enabled.NAME] + "loginUrl": pipeline.get_login_url( + enabled.NAME, + pipeline.AUTH_ENTRY_LOGIN, + redirect_url=redirect_to, + ), + "registerUrl": pipeline.get_login_url( + enabled.NAME, + pipeline.AUTH_ENTRY_REGISTER, + redirect_url=redirect_to, + ), } for enabled in third_party_auth.provider.Registry.enabled() ] - running_pipeline = third_party_auth.pipeline.get(request) + running_pipeline = pipeline.get(request) if running_pipeline is not None: current_provider = third_party_auth.provider.Registry.get_by_backend_name( running_pipeline.get('backend') ) context["currentProvider"] = current_provider.NAME + context["finishAuthUrl"] = pipeline.get_complete_url(current_provider.BACKEND_CLASS.name) + + # Check for any error messages we may want to display: + for msg in messages.get_messages(request): + if msg.extra_tags.split()[0] == "social-auth": + context['errorMessage'] = unicode(msg) + break return context @@ -326,6 +290,39 @@ def account_settings(request): return render_to_response('student_account/account_settings.html', account_settings_context(request)) +@login_required +@require_http_methods(['GET']) +def finish_auth(request): # pylint: disable=unused-argument + """ Following logistration (1st or 3rd party), handle any special query string params. + + See FinishAuthView.js for details on the query string params. + + e.g. auto-enroll the user in a course, set email opt-in preference. + + This view just displays a "Please wait" message while AJAX calls are made to enroll the + user in the course etc. This view is only used if a parameter like "course_id" is present + during login/registration/third_party_auth. Otherwise, there is no need for it. + + Ideally this view will finish and redirect to the next step before the user even sees it. + + Args: + request (HttpRequest) + + Returns: + HttpResponse: 200 if the page was sent successfully + HttpResponse: 302 if not logged in (redirect to login page) + HttpResponse: 405 if using an unsupported HTTP method + + Example usage: + + GET /account/finish_auth/?course_id=course-v1:blah&enrollment_action=enroll + + """ + return render_to_response('student_account/finish_auth.html', { + 'disable_courseware_js': True, + }) + + def account_settings_context(request): """ Context for the account settings page. diff --git a/lms/envs/aws.py b/lms/envs/aws.py index d2f480ae26..1f113305a6 100644 --- a/lms/envs/aws.py +++ b/lms/envs/aws.py @@ -532,6 +532,9 @@ X_FRAME_OPTIONS = ENV_TOKENS.get('X_FRAME_OPTIONS', X_FRAME_OPTIONS) ##### Third-party auth options ################################################ THIRD_PARTY_AUTH = AUTH_TOKENS.get('THIRD_PARTY_AUTH', THIRD_PARTY_AUTH) +# The reduced session expiry time during the third party login pipeline. (Value in seconds) +SOCIAL_AUTH_PIPELINE_TIMEOUT = ENV_TOKENS.get('SOCIAL_AUTH_PIPELINE_TIMEOUT', 600) + ##### OAUTH2 Provider ############## if FEATURES.get('ENABLE_OAUTH2_PROVIDER'): OAUTH_OIDC_ISSUER = ENV_TOKENS['OAUTH_OIDC_ISSUER'] diff --git a/lms/envs/bok_choy.auth.json b/lms/envs/bok_choy.auth.json index cd47547ed2..85a19928cc 100644 --- a/lms/envs/bok_choy.auth.json +++ b/lms/envs/bok_choy.auth.json @@ -118,6 +118,7 @@ }, "SECRET_KEY": "", "THIRD_PARTY_AUTH": { + "Dummy": {}, "Google": { "SOCIAL_AUTH_GOOGLE_OAUTH2_KEY": "test", "SOCIAL_AUTH_GOOGLE_OAUTH2_SECRET": "test" diff --git a/lms/envs/bok_choy.env.json b/lms/envs/bok_choy.env.json index 1eb0cffdba..8fde6044b5 100644 --- a/lms/envs/bok_choy.env.json +++ b/lms/envs/bok_choy.env.json @@ -79,6 +79,7 @@ "ENABLE_INSTRUCTOR_ANALYTICS": true, "ENABLE_S3_GRADE_DOWNLOADS": true, "ENABLE_THIRD_PARTY_AUTH": true, + "ENABLE_DUMMY_THIRD_PARTY_AUTH_PROVIDER": true, "ENABLE_COMBINED_LOGIN_REGISTRATION": true, "PREVIEW_LMS_BASE": "localhost:8003", "SUBDOMAIN_BRANDING": false, diff --git a/lms/envs/common.py b/lms/envs/common.py index 4581e0263e..d2fedc6fed 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1556,6 +1556,10 @@ PIPELINE_JS = { 'certificates_wv': { 'source_filenames': certificates_web_view_js, 'output_filename': 'js/certificates/web_view.js' + }, + 'utility': { + 'source_filenames': ['js/src/utility.js'], + 'output_filename': 'js/utility.js' } } diff --git a/lms/envs/test.py b/lms/envs/test.py index 38a30b944b..5a81e3cca2 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -242,11 +242,13 @@ THIRD_PARTY_AUTH = { "SOCIAL_AUTH_GOOGLE_OAUTH2_SECRET": "test", }, "Facebook": { - "SOCIAL_AUTH_GOOGLE_OAUTH2_KEY": "test", - "SOCIAL_AUTH_GOOGLE_OAUTH2_SECRET": "test", + "SOCIAL_AUTH_FACEBOOK_KEY": "test", + "SOCIAL_AUTH_FACEBOOK_SECRET": "test", }, } +FEATURES['ENABLE_DUMMY_THIRD_PARTY_AUTH_PROVIDER'] = True + ################################## OPENID ##################################### FEATURES['AUTH_USE_OPENID'] = True FEATURES['AUTH_USE_OPENID_PROVIDER'] = True diff --git a/lms/static/js/spec/main.js b/lms/static/js/spec/main.js index 68d9be3edf..0a3e224202 100644 --- a/lms/static/js/spec/main.js +++ b/lms/static/js/spec/main.js @@ -449,7 +449,6 @@ 'jquery', 'underscore', 'backbone', - 'gettext', 'history', 'utility', 'js/student_account/views/LoginView', @@ -458,10 +457,7 @@ 'js/student_account/models/LoginModel', 'js/student_account/models/PasswordResetModel', 'js/student_account/models/RegisterModel', - 'js/student_account/views/FormView', - 'js/student_account/emailoptin', - 'js/student_account/enrollment', - 'js/student_account/shoppingcart', + 'js/student_account/views/FormView' ] }, 'js/verify_student/models/verification_model': { @@ -598,6 +594,7 @@ 'lms/include/js/spec/instructor_dashboard/student_admin_spec.js', 'lms/include/js/spec/student_account/account_spec.js', 'lms/include/js/spec/student_account/access_spec.js', + 'lms/include/js/spec/student_account/finish_auth_spec.js', 'lms/include/js/spec/student_account/login_spec.js', 'lms/include/js/spec/student_account/register_spec.js', 'lms/include/js/spec/student_account/password_reset_spec.js', diff --git a/lms/static/js/spec/student_account/access_spec.js b/lms/static/js/spec/student_account/access_spec.js index db7ea08f2f..a82da514e2 100644 --- a/lms/static/js/spec/student_account/access_spec.js +++ b/lms/static/js/spec/student_account/access_spec.js @@ -8,9 +8,8 @@ define([ 'js/student_account/shoppingcart', 'js/student_account/emailoptin' ], function($, TemplateHelpers, AjaxHelpers, AccessView, FormView, EnrollmentInterface, ShoppingCartInterface) { + "use strict"; describe('edx.student.account.AccessView', function() { - 'use strict'; - var requests = null, view = null, FORM_DESCRIPTION = { @@ -41,10 +40,15 @@ define([ } ] }, - FORWARD_URL = '/courseware/next', - COURSE_KEY = 'edx/DemoX/Fall'; + FORWARD_URL = ( + '/account/finish_auth' + + '?course_id=edx%2FDemoX%2FFall' + + '&enrollment_action=enroll' + + '&next=%2Fdashboard' + ), + THIRD_PARTY_COMPLETE_URL = '/auth/complete/provider/'; - var ajaxSpyAndInitialize = function(that, mode) { + var ajaxSpyAndInitialize = function(that, mode, nextUrl, finishAuthUrl) { // Spy on AJAX requests requests = AjaxHelpers.requests(that); @@ -53,8 +57,10 @@ define([ mode: mode, thirdPartyAuth: { currentProvider: null, - providers: [] + providers: [], + finishAuthUrl: finishAuthUrl }, + nextUrl: nextUrl, // undefined for default platformName: 'edX', loginFormDesc: FORM_DESCRIPTION, registrationFormDesc: FORM_DESCRIPTION, @@ -84,20 +90,6 @@ define([ view.toggleForm(changeEvent); }; - /** - * Simulate query string params. - * - * @param {object} params Parameters to set, each of which - * should be prefixed with '?' - */ - var setFakeQueryParams = function( params ) { - spyOn( $, 'url' ).andCallFake(function( requestedParam ) { - if ( params.hasOwnProperty(requestedParam) ) { - return params[requestedParam]; - } - }); - }; - beforeEach(function() { setFixtures('
'); TemplateHelpers.installTemplate('templates/student_account/access'); @@ -156,83 +148,6 @@ define([ expect($("#password-reset-form")).not.toHaveClass('hidden'); }); - it('enrolls the user on auth complete', function() { - ajaxSpyAndInitialize(this, 'login'); - - // Simulate providing enrollment query string params - setFakeQueryParams({ - '?enrollment_action': 'enroll', - '?course_id': COURSE_KEY - }); - - // Trigger auth complete on the login view - view.subview.login.trigger('auth-complete'); - - // Expect that the view tried to enroll the student - expect( EnrollmentInterface.enroll ).toHaveBeenCalledWith( - COURSE_KEY, - '/course_modes/choose/' + COURSE_KEY + '/' - ); - }); - - it('sends the user to the payment flow when the course mode is not honor', function() { - ajaxSpyAndInitialize(this, 'login'); - - // Simulate providing enrollment query string params - // AND specifying a course mode. - setFakeQueryParams({ - '?enrollment_action': 'enroll', - '?course_id': COURSE_KEY, - '?course_mode': 'verified' - }); - - // Trigger auth complete on the login view - view.subview.login.trigger('auth-complete'); - - // Expect that the view tried to auto-enroll the student - // with a redirect into the payment flow. - expect( EnrollmentInterface.enroll ).toHaveBeenCalledWith( - COURSE_KEY, - '/verify_student/start-flow/' + COURSE_KEY + '/' - ); - }); - - it('sends the user to the student dashboard when the course mode is honor', function() { - ajaxSpyAndInitialize(this, 'login'); - - // Simulate providing enrollment query string params - // AND specifying a course mode. - setFakeQueryParams({ - '?enrollment_action': 'enroll', - '?course_id': COURSE_KEY, - '?course_mode': 'honor' - }); - - // Trigger auth complete on the login view - view.subview.login.trigger('auth-complete'); - - // Expect that the view tried auto-enrolled the student - // and sent the student to the dashboard - // (skipping the payment flow). - expect( EnrollmentInterface.enroll ).toHaveBeenCalledWith(COURSE_KEY, '/dashboard'); - }); - - it('adds a white-label course to the shopping cart on auth complete', function() { - ajaxSpyAndInitialize(this, 'register'); - - // Simulate providing "add to cart" query string params - setFakeQueryParams({ - '?enrollment_action': 'add_to_cart', - '?course_id': COURSE_KEY - }); - - // Trigger auth complete on the register view - view.subview.register.trigger('auth-complete'); - - // Expect that the view tried to add the course to the user's shopping cart - expect( ShoppingCartInterface.addCourseToCart ).toHaveBeenCalledWith( COURSE_KEY ); - }); - it('redirects the user to the dashboard on auth complete', function() { ajaxSpyAndInitialize(this, 'register'); @@ -243,11 +158,19 @@ define([ expect( view.redirect ).toHaveBeenCalledWith( '/dashboard' ); }); - it('redirects the user to the next page on auth complete', function() { - ajaxSpyAndInitialize(this, 'register'); + it('proceeds with the third party auth pipeline if active', function() { + ajaxSpyAndInitialize(this, 'register', '/', THIRD_PARTY_COMPLETE_URL); - // Simulate providing a ?next query string parameter - setFakeQueryParams({ '?next': FORWARD_URL }); + // Trigger auth complete + view.subview.register.trigger('auth-complete'); + + // Verify that we were redirected + expect( view.redirect ).toHaveBeenCalledWith( THIRD_PARTY_COMPLETE_URL ); + }); + + it('redirects the user to the next page on auth complete', function() { + // The 'next' argument is often used to redirect to the auto-enrollment view + ajaxSpyAndInitialize(this, 'register', FORWARD_URL); // Trigger auth complete view.subview.register.trigger('auth-complete'); @@ -257,11 +180,7 @@ define([ }); it('ignores redirect to external URLs', function() { - ajaxSpyAndInitialize(this, 'register'); - - // Simulate providing a ?next query string parameter - // that goes to an external URL - setFakeQueryParams({ '?next': "http://www.example.com" }); + ajaxSpyAndInitialize(this, 'register', "http://www.example.com"); // Trigger auth complete view.subview.register.trigger('auth-complete'); diff --git a/lms/static/js/spec/student_account/finish_auth_spec.js b/lms/static/js/spec/student_account/finish_auth_spec.js new file mode 100644 index 0000000000..1dd9e4dece --- /dev/null +++ b/lms/static/js/spec/student_account/finish_auth_spec.js @@ -0,0 +1,171 @@ +define([ + 'jquery', + 'utility', + 'common/js/spec_helpers/ajax_helpers', + 'js/student_account/views/FinishAuthView', + 'js/student_account/enrollment', + 'js/student_account/shoppingcart', + 'js/student_account/emailoptin' +], function($, utility, AjaxHelpers, FinishAuthView, EnrollmentInterface, ShoppingCartInterface, EmailOptInInterface) { + 'use strict'; + describe('FinishAuthView', function() { + var requests = null, + view = null, + FORWARD_URL = '/courseware/next', + COURSE_KEY = 'course-v1:edX+test+15'; + + var ajaxSpyAndInitialize = function(that) { + // Spy on AJAX requests + requests = AjaxHelpers.requests(that); + + // Initialize the access view + view = new FinishAuthView({}); + + // Mock the redirect call + spyOn( view, 'redirect' ).andCallFake( function() {} ); + + // Mock the enrollment and shopping cart interfaces + spyOn( EnrollmentInterface, 'enroll' ).andCallFake( function() {} ); + spyOn( ShoppingCartInterface, 'addCourseToCart' ).andCallFake( function() {} ); + spyOn( EmailOptInInterface, 'setPreference' ) + .andCallFake( function() { return {'always': function(r) { r(); }}; } ); + + view.render(); + }; + + /** + * Simulate query string params. + * + * @param {object} params Parameters to set, each of which + * should be prefixed with '?' + */ + var setFakeQueryParams = function( params ) { + spyOn( $, 'url' ).andCallFake(function( requestedParam ) { + if ( params.hasOwnProperty(requestedParam) ) { + return params[requestedParam]; + } + }); + }; + + beforeEach(function() { + // Stub analytics tracking + window.analytics = jasmine.createSpyObj('analytics', ['track', 'page', 'pageview', 'trackLink']); + }); + + it('saves the email opt-in preference before enrollment', function() { + // Simulate providing enrollment query string params + setFakeQueryParams({ + '?enrollment_action': 'enroll', + '?course_id': COURSE_KEY, + '?email_opt_in': 'true' + }); + + ajaxSpyAndInitialize(this); + + // Expect that the view tried to save the email opt in preference + expect( EmailOptInInterface.setPreference ).toHaveBeenCalledWith( + COURSE_KEY, + 'true' + ); + // Expect that the view tried to enroll the student + expect( EnrollmentInterface.enroll ).toHaveBeenCalledWith( + COURSE_KEY, + '/course_modes/choose/' + COURSE_KEY + '/' + ); + }); + + it('enrolls the user on auth complete', function() { + // Simulate providing enrollment query string params + setFakeQueryParams({ + '?enrollment_action': 'enroll', + '?course_id': COURSE_KEY + }); + + ajaxSpyAndInitialize(this); + + // Expect that the view tried to enroll the student + expect( EnrollmentInterface.enroll ).toHaveBeenCalledWith( + COURSE_KEY, + '/course_modes/choose/' + COURSE_KEY + '/' + ); + }); + + it('sends the user to the payment flow when the course mode is not honor', function() { + // Simulate providing enrollment query string params + // AND specifying a course mode. + setFakeQueryParams({ + '?enrollment_action': 'enroll', + '?course_id': COURSE_KEY, + '?course_mode': 'verified' + }); + + ajaxSpyAndInitialize(this); + + // Expect that the view tried to auto-enroll the student + // with a redirect into the payment flow. + expect( EnrollmentInterface.enroll ).toHaveBeenCalledWith( + COURSE_KEY, + '/verify_student/start-flow/' + COURSE_KEY + '/' + ); + }); + + it('sends the user to the student dashboard when the course mode is honor', function() { + // Simulate providing enrollment query string params + // AND specifying a course mode. + setFakeQueryParams({ + '?enrollment_action': 'enroll', + '?course_id': COURSE_KEY, + '?course_mode': 'honor' + }); + + ajaxSpyAndInitialize(this); + + // Expect that the view tried auto-enrolled the student + // and sent the student to the dashboard + // (skipping the payment flow). + expect( EnrollmentInterface.enroll ).toHaveBeenCalledWith(COURSE_KEY, '/dashboard'); + }); + + it('adds a white-label course to the shopping cart on auth complete', function() { + // Simulate providing "add to cart" query string params + setFakeQueryParams({ + '?enrollment_action': 'add_to_cart', + '?course_id': COURSE_KEY + }); + + ajaxSpyAndInitialize(this); + + // Expect that the view tried to add the course to the user's shopping cart + expect( ShoppingCartInterface.addCourseToCart ).toHaveBeenCalledWith( COURSE_KEY ); + }); + + it('redirects the user to the dashboard if no course is provided', function() { + ajaxSpyAndInitialize(this); + + // Since we did not provide a ?next query param, expect a redirect to the dashboard. + expect( view.redirect ).toHaveBeenCalledWith( '/dashboard' ); + }); + + it('redirects the user to the next page when done', function() { + // Simulate providing a ?next query string parameter + setFakeQueryParams({ '?next': FORWARD_URL }); + + ajaxSpyAndInitialize(this); + + // Verify that we were redirected + expect( view.redirect ).toHaveBeenCalledWith( FORWARD_URL ); + }); + + it('ignores redirect to external URLs', function() { + // Simulate providing a ?next query string parameter + // that goes to an external URL + setFakeQueryParams({ '?next': "http://www.example.com" }); + + ajaxSpyAndInitialize(this); + + // Expect that we ignore the external URL and redirect to the dashboard + expect( view.redirect ).toHaveBeenCalledWith( "/dashboard" ); + }); + }); + } +); diff --git a/lms/static/js/student_account/accessApp.js b/lms/static/js/student_account/accessApp.js index 4c67707fcf..7c6579b536 100644 --- a/lms/static/js/student_account/accessApp.js +++ b/lms/static/js/student_account/accessApp.js @@ -11,6 +11,7 @@ var edx = edx || {}; return new edx.student.account.AccessView({ mode: container.data('initial-mode'), thirdPartyAuth: container.data('third-party-auth'), + nextUrl: container.data('next-url'), platformName: container.data('platform-name'), loginFormDesc: container.data('login-form-desc'), registrationFormDesc: container.data('registration-form-desc'), diff --git a/lms/static/js/student_account/emailoptin.js b/lms/static/js/student_account/emailoptin.js index cd41d43a3d..b06eef6f62 100644 --- a/lms/static/js/student_account/emailoptin.js +++ b/lms/static/js/student_account/emailoptin.js @@ -22,13 +22,12 @@ var edx = edx || {}; * @param {string} courseKey Slash-separated course key. * @param {string} emailOptIn The preference to opt in or out of organization emails. */ - setPreference: function( courseKey, emailOptIn, context ) { + setPreference: function( courseKey, emailOptIn ) { return $.ajax({ url: this.urls.emailOptInUrl, type: 'POST', data: {course_id: courseKey, email_opt_in: emailOptIn}, - headers: this.headers, - context: context + headers: this.headers }); } }; diff --git a/lms/static/js/student_account/views/AccessView.js b/lms/static/js/student_account/views/AccessView.js index 7d2911850e..9cf1dcae15 100644 --- a/lms/static/js/student_account/views/AccessView.js +++ b/lms/static/js/student_account/views/AccessView.js @@ -1,19 +1,11 @@ var edx = edx || {}; -(function($, _, _s, Backbone, gettext) { +(function($, _, _s, Backbone, History) { 'use strict'; edx.student = edx.student || {}; edx.student.account = edx.student.account || {}; - // Bind to StateChange Event - History.Adapter.bind( window, 'statechange', function() { - /* Note: We are using History.getState() for legacy browser (IE) support - * using History.js plugin instead of the native event.state - */ - var State = History.getState(); - }); - edx.student.account.AccessView = Backbone.View.extend({ el: '#login-and-registration-container', @@ -29,11 +21,7 @@ var edx = edx || {}; passwordHelp: {} }, - urls: { - dashboard: '/dashboard', - payment: '/verify_student/start-flow/', - trackSelection: '/course_modes/choose/' - }, + nextUrl: '/dashboard', // The form currently loaded activeForm: '', @@ -54,6 +42,13 @@ var edx = edx || {}; providers: [] }; + if (obj.nextUrl) { + // Ensure that the next URL is internal for security reasons + if ( ! window.isExternal( obj.nextUrl ) ) { + this.nextUrl = obj.nextUrl; + } + } + this.formDescriptions = { login: obj.loginFormDesc, register: obj.registrationFormDesc, @@ -69,6 +64,10 @@ var edx = edx || {}; }); this.render(); + + // Once the third party error message has been shown once, + // there is no need to show it again, if the user changes mode: + this.thirdPartyAuth.errorMessage = null; }, render: function() { @@ -199,113 +198,18 @@ var edx = edx || {}; }, /** - * Once authentication has completed successfully, a user may need to: + * Once authentication has completed successfully: * - * - Enroll in a course. - * - Update email opt-in preferences - * - * These actions are delegated from the authComplete function to additional - * functions requiring authentication. + * If we're in a third party auth pipeline, we must complete the pipeline. + * Otherwise, redirect to the specified next step. * */ authComplete: function() { - var emailOptIn = edx.student.account.EmailOptInInterface, - queryParams = this.queryParams(); - - // Set the email opt in preference. - if (!_.isUndefined(queryParams.emailOptIn) && queryParams.enrollmentAction) { - emailOptIn.setPreference( - decodeURIComponent(queryParams.courseId), - queryParams.emailOptIn, - this - ).always(this.enrollment); + if (this.thirdPartyAuth && this.thirdPartyAuth.finishAuthUrl) { + this.redirect(this.thirdPartyAuth.finishAuthUrl); + // Note: the third party auth URL likely contains another redirect URL embedded inside } else { - this.enrollment(); - } - }, - - /** - * Designed to be invoked after authentication has completed. This function enrolls - * the student as requested. - * - * - Enroll in a course. - * - Add a course to the shopping cart. - * - Be redirected to the dashboard / track selection page / shopping cart. - * - * This handler is triggered upon successful authentication, - * either from the login or registration form. It checks - * query string params, performs enrollment/shopping cart actions, - * then redirects the user to the next page. - * - * The optional query string params are: - * - * ?next: If provided, redirect to this page upon successful auth. - * Django uses this when an unauthenticated user accesses a view - * decorated with @login_required. - * - * ?enrollment_action: Can be either "enroll" or "add_to_cart". - * If you provide this param, you must also provide a `course_id` param; - * otherwise, no action will be taken. - * - * ?course_id: The slash-separated course ID to enroll in or add to the cart. - * - */ - enrollment: function() { - var enrollment = edx.student.account.EnrollmentInterface, - shoppingcart = edx.student.account.ShoppingCartInterface, - redirectUrl = this.urls.dashboard, - queryParams = this.queryParams(); - - if ( queryParams.enrollmentAction === 'enroll' && queryParams.courseId ) { - var courseId = decodeURIComponent( queryParams.courseId ); - - // Determine where to redirect the user after auto-enrollment. - if ( !queryParams.courseMode ) { - /* Backwards compatibility with the original course details page. - The old implementation did not specify the course mode for enrollment, - so we'd always send the user to the "track selection" page. - The track selection page would allow the user to select the course mode - ("verified", "honor", etc.) -- or, if the only course mode was "honor", - it would redirect the user to the dashboard. */ - redirectUrl = this.urls.trackSelection + courseId + '/'; - } else if ( queryParams.courseMode === 'honor' || queryParams.courseMode === 'audit' ) { - /* The newer version of the course details page allows the user - to specify which course mode to enroll as. If the student has - chosen "honor", we send them immediately to the dashboard - rather than the payment flow. The user may decide to upgrade - from the dashboard later. */ - redirectUrl = this.urls.dashboard; - } else { - /* If the user selected any other kind of course mode, send them - to the payment/verification flow. */ - redirectUrl = this.urls.payment + courseId + '/'; - } - - /* Attempt to auto-enroll the user in a free mode of the course, - then redirect to the next location. */ - enrollment.enroll( courseId, redirectUrl ); - } else if ( queryParams.enrollmentAction === 'add_to_cart' && queryParams.courseId) { - /* - If this is a paid course, add it to the shopping cart and redirect - the user to the "view cart" page. - */ - shoppingcart.addCourseToCart( decodeURIComponent( queryParams.courseId ) ); - } else { - /* - Otherwise, redirect the user to the next page - Check for forwarding url and ensure that it isn't external. - If not, use the default forwarding URL. - */ - if ( !_.isNull( queryParams.next ) ) { - var next = decodeURIComponent( queryParams.next ); - - // Ensure that the URL is internal for security reasons - if ( !window.isExternal( next ) ) { - redirectUrl = next; - } - } - - this.redirect( redirectUrl ); + this.redirect(this.nextUrl); } }, @@ -314,25 +218,7 @@ var edx = edx || {}; * @param {string} url The URL to redirect to. */ redirect: function( url ) { - window.location.href = url; - }, - - /** - * Retrieve query params that we use post-authentication - * to decide whether to enroll a student in a course, add - * an item to the cart, or redirect. - * - * @return {object} The query params. If any param is not - * provided, it will default to null. - */ - queryParams: function() { - return { - next: $.url( '?next' ), - enrollmentAction: $.url( '?enrollment_action' ), - courseId: $.url( '?course_id' ), - courseMode: $.url( '?course_mode' ), - emailOptIn: $.url( '?email_opt_in') - }; + window.location.replace(url); }, form: { @@ -361,4 +247,4 @@ var edx = edx || {}; } } }); -})(jQuery, _, _.str, Backbone, gettext); +})(jQuery, _, _.str, Backbone, History); diff --git a/lms/static/js/student_account/views/FinishAuthView.js b/lms/static/js/student_account/views/FinishAuthView.js new file mode 100644 index 0000000000..49cb7055a4 --- /dev/null +++ b/lms/static/js/student_account/views/FinishAuthView.js @@ -0,0 +1,170 @@ +/** + * Once authentication has completed successfully, we may need to: + * + * - Enroll in a course. + * - Add a course to the shopping cart. + * - Update email opt-in preferences + * + * These actions are implemented by this view. + * + * This view may be initialized with the following optional parameters: + * - courseId: string ID of the course in which to auto-enroll the user + * - enrollmentAction: Can be either "enroll" or "add_to_cart". If you provide + * this param, you must also provide a `course_id` param; otherwise, no + * action will be taken. + * - courseMode: optional. The mode to enroll in, e.g. "honor" + * - emailOptIn: "true" or "false". Whether or not the user has opted in to + * emails from the course's organization. + * - nextUrl: Redirect to this URL upon completion of all tasks, if possible + * and safe to do so. + * + * One the actions have been completed, the user will be redirected to either: + * - The track selection or payment page (if they've been enrolled in a course that needs this) + * - The specified 'nextUrl' if safe, or + * - The dashboard + */ +;(function (define, undefined) { + 'use strict'; + define([ + 'underscore', + 'backbone', + 'gettext', + 'js/student_account/emailoptin', + 'js/student_account/enrollment', + 'js/student_account/shoppingcart' + ], function (_, Backbone, gettext, emailOptInInterface, enrollmentInterface, shoppingCartInterface) { + // These are not yet converted to requireJS: + var edx = window.edx || {}; + emailOptInInterface = emailOptInInterface || edx.student.account.EmailOptInInterface; + enrollmentInterface = enrollmentInterface || edx.student.account.EnrollmentInterface; + shoppingCartInterface = shoppingCartInterface || edx.student.account.ShoppingCartInterface; + + var FinishAuthView = Backbone.View.extend({ + el: '#finish-auth-status', + + urls: { + finishAuth: '/account/finish_auth', + defaultNextUrl: '/dashboard', + payment: '/verify_student/start-flow/', + trackSelection: '/course_modes/choose/' + }, + + initialize: function( obj ) { + var queryParams = { + next: $.url( '?next' ), + enrollmentAction: $.url( '?enrollment_action' ), + courseId: $.url( '?course_id' ), + courseMode: $.url( '?course_mode' ), + emailOptIn: $.url( '?email_opt_in') + }; + for (var key in queryParams) { + if (queryParams[key]) { + queryParams[key] = decodeURIComponent(queryParams[key]); + } + } + this.courseId = queryParams.courseId; + this.enrollmentAction = queryParams.enrollmentAction; + this.courseMode = queryParams.courseMode; + this.emailOptIn = queryParams.emailOptIn; + this.nextUrl = this.urls.defaultNextUrl; + if (queryParams.next) { + // Ensure that the next URL is internal for security reasons + if ( ! window.isExternal( queryParams.next ) ) { + this.nextUrl = queryParams.next; + } + } + }, + + render: function() { + try { + var next = _.bind(this.enrollment, this); + this.checkEmailOptIn(next); + } catch(err) { + this.updateTaskDescription(gettext("Error") + ": " + err.message); + this.redirect(this.nextUrl); + } + }, + + updateTaskDescription: function(desc) { + // We don't display any detailed status updates to the user + // but we do log them to the console to help with debugging. + console.log(desc); + }, + + /** + * Step 1: + * Update the user's email preferences and then proceed to the next step + */ + checkEmailOptIn: function(next) { + // Set the email opt in preference. this.emailOptIn is null or "true" or "false" + if ((this.emailOptIn === "true" || this.emailOptIn === "false") && this.enrollmentAction) { + this.updateTaskDescription(gettext("Saving your email preference")); + emailOptInInterface + .setPreference(this.courseId, this.emailOptIn) + .always(next); + } else { + next(); + } + }, + + /** + * Step 2. Handle enrollment: + * - Enroll in a course or add a course to the shopping cart. + * - Be redirected to the dashboard / track selection page / shopping cart. + */ + enrollment: function() { + var redirectUrl = this.nextUrl; + + if ( this.enrollmentAction === 'enroll' && this.courseId ) { + this.updateTaskDescription(gettext("Enrolling you in the selected course")); + var courseId = decodeURIComponent( this.courseId ); + + // Determine where to redirect the user after auto-enrollment. + if ( !this.courseMode ) { + /* Backwards compatibility with the original course details page. + The old implementation did not specify the course mode for enrollment, + so we'd always send the user to the "track selection" page. + The track selection page would allow the user to select the course mode + ("verified", "honor", etc.) -- or, if the only course mode was "honor", + it would redirect the user to the dashboard. */ + redirectUrl = this.urls.trackSelection + courseId + '/'; + } else if ( this.courseMode === 'honor' || this.courseMode === 'audit' ) { + /* The newer version of the course details page allows the user + to specify which course mode to enroll as. If the student has + chosen "honor", we send them immediately to the next URL + rather than the payment flow. The user may decide to upgrade + from the dashboard later. */ + } else { + /* If the user selected any other kind of course mode, send them + to the payment/verification flow. */ + redirectUrl = this.urls.payment + courseId + '/'; + } + + /* Attempt to auto-enroll the user in a free mode of the course, + then redirect to the next location. */ + enrollmentInterface.enroll( courseId, redirectUrl ); + } else if ( this.enrollmentAction === 'add_to_cart' && this.courseId) { + /* + If this is a paid course, add it to the shopping cart and redirect + the user to the "view cart" page. + */ + this.updateTaskDescription(gettext("Adding the selected course to your cart")); + shoppingCartInterface.addCourseToCart( this.courseId ); + } else { + // Otherwise, redirect the user to the next page. + this.redirect( redirectUrl ); + } + }, + + /** + * Redirect to a URL. Mainly useful for mocking out in tests. + * @param {string} url The URL to redirect to. + */ + redirect: function( url ) { + this.updateTaskDescription(gettext("Loading your courses")); + window.location.replace(url); + } + }); + return FinishAuthView; + }); +}).call(this, define || RequireJS.define); diff --git a/lms/static/js/student_account/views/LoginView.js b/lms/static/js/student_account/views/LoginView.js index 7b567b9fbd..79eb44d1ce 100644 --- a/lms/static/js/student_account/views/LoginView.js +++ b/lms/static/js/student_account/views/LoginView.js @@ -26,6 +26,7 @@ var edx = edx || {}; preRender: function( data ) { this.providers = data.thirdPartyAuth.providers || []; this.currentProvider = data.thirdPartyAuth.currentProvider || ''; + this.errorMessage = data.thirdPartyAuth.errorMessage || ''; this.platformName = data.platformName; this.resetModel = data.resetModel; @@ -42,6 +43,7 @@ var edx = edx || {}; context: { fields: fields, currentProvider: this.currentProvider, + errorMessage: this.errorMessage, providers: this.providers, platformName: this.platformName } diff --git a/lms/static/js/student_account/views/RegisterView.js b/lms/static/js/student_account/views/RegisterView.js index f93814603d..177bfe51da 100644 --- a/lms/static/js/student_account/views/RegisterView.js +++ b/lms/static/js/student_account/views/RegisterView.js @@ -23,6 +23,7 @@ var edx = edx || {}; preRender: function( data ) { this.providers = data.thirdPartyAuth.providers || []; this.currentProvider = data.thirdPartyAuth.currentProvider || ''; + this.errorMessage = data.thirdPartyAuth.errorMessage || ''; this.platformName = data.platformName; this.listenTo( this.model, 'sync', this.saveSuccess ); @@ -38,6 +39,7 @@ var edx = edx || {}; context: { fields: fields, currentProvider: this.currentProvider, + errorMessage: this.errorMessage, providers: this.providers, platformName: this.platformName } diff --git a/lms/static/sass/views/_login-register.scss b/lms/static/sass/views/_login-register.scss index d59523635e..81d43ee80d 100644 --- a/lms/static/sass/views/_login-register.scss +++ b/lms/static/sass/views/_login-register.scss @@ -515,3 +515,26 @@ $sm-btn-linkedin: #0077b5; } } } + +.finish-auth { + @include box-sizing(border-box); + @include outer-container; + $grid-columns: 12; + background: $white; + min-height: 100%; + width: 100%; + + .finish-auth-inner { + @include box-sizing(border-box); + max-width: 650px; + margin: 1em auto; + } + + #finish-auth-status { + padding-top: 30px; // Make room for the absolutely positioned loading animation + } + + #finish-auth-status li:last-child { + font-weight: bold; + } +} diff --git a/lms/templates/login.html b/lms/templates/login.html index 9fa02f1eae..ed666adf51 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -65,20 +65,17 @@ from microsite_configuration import microsite $('#login-form').on('ajax:success', function(event, json, xhr) { if(json.success) { - var u=decodeURI(window.location.search); - var next = u.split("next=")[1]; - if (next != undefined) { - // if next is undefined, decodeURI returns "undefined" causing a bad redirect. - next = decodeURIComponent(next); + var nextUrl = "${login_redirect_url}"; + if (json.redirect_url) { + nextUrl = json.redirect_url; // Most likely third party auth completion. This trumps 'nextUrl' above. } - if (next && !isExternal(next)) { - location.href=next; - } else if(json.redirect_url){ - location.href=json.redirect_url; + if (!isExternal(nextUrl)) { + location.href=nextUrl; } else { location.href="${reverse('dashboard')}"; } } else if(json.hasOwnProperty('redirect')) { + // Shibboleth authentication redirect requested by the server: var u=decodeURI(window.location.search); if (!isExternal(json.redirect)) { // a paranoid check. Our server is the one providing json.redirect location.href=json.redirect+u; @@ -162,6 +159,15 @@ from microsite_configuration import microsite

+ % if third_party_auth_error: + + % endif +

${_('Please provide the following information to log into your {platform_name} account. Required fields are noted by bold text and an asterisk (*).').format(platform_name=platform_name)}

@@ -196,15 +202,6 @@ from microsite_configuration import microsite -% if course_id and enrollment_action: - - -% endif - -% if email_opt_in: - -% endif -
diff --git a/lms/templates/main.html b/lms/templates/main.html index d61516acd3..7d2c3cc53c 100644 --- a/lms/templates/main.html +++ b/lms/templates/main.html @@ -171,18 +171,7 @@ from branding import api as branding_api <%def name="login_query()">${ - u"?course_id={0}&enrollment_action={1}{course_mode}{email_opt_in}".format( - urlquote_plus(course_id), - urlquote_plus(enrollment_action), - course_mode=( - u"&course_mode=" + urlquote_plus(course_mode) - if course_mode else "" - ), - email_opt_in=( - u"&email_opt_in=" + urlquote_plus(email_opt_in) - if email_opt_in else "" - ) - ) if course_id and enrollment_action else "" + u"?next={0}".format(urlquote_plus(login_redirect_url)) if login_redirect_url else "" } diff --git a/lms/templates/navigation.html b/lms/templates/navigation.html index a981e6a79b..f925bfee85 100644 --- a/lms/templates/navigation.html +++ b/lms/templates/navigation.html @@ -130,7 +130,7 @@ site_status_msg = get_site_status_msg(course_id) % else: % endif % endif diff --git a/lms/templates/register.html b/lms/templates/register.html index be979a3887..f885769658 100644 --- a/lms/templates/register.html +++ b/lms/templates/register.html @@ -54,8 +54,15 @@ import calendar }); $('#register-form').on('ajax:success', function(event, json, xhr) { - var url = json.redirect_url || "${reverse('dashboard')}"; - location.href = url; + var nextUrl = "${login_redirect_url}"; + if (json.redirect_url) { + nextUrl = json.redirect_url; // Most likely third party auth completion. This trumps 'nextUrl' above. + } + if (!isExternal(nextUrl)) { + location.href=nextUrl; + } else { + location.href="${reverse('dashboard')}"; + } }); $('#register-form').on('ajax:error', function(event, jqXHR, textStatus) { @@ -359,15 +366,6 @@ import calendar -% if course_id and enrollment_action: - - -% endif - -% if email_opt_in: - -% endif -
diff --git a/lms/templates/student_account/finish_auth.html b/lms/templates/student_account/finish_auth.html new file mode 100644 index 0000000000..99bddc2dff --- /dev/null +++ b/lms/templates/student_account/finish_auth.html @@ -0,0 +1,51 @@ +<%! from django.utils.translation import ugettext as _ %> +<%namespace name='static' file='/static_content.html'/> +<%inherit file="/main.html" /> + +<%block name="pagetitle">${_("Please Wait")} + +<%block name="js_extra"> + + <%static:js group='utility'/> + + +<%block name="headextra"> + + + + + +
+
+

${_('Please wait')}

+ +
+
+
+ +## This overwrites the "footer" block declared in main.html +## with an empty block, effectively hiding the footer. +<%block name="footer"/> diff --git a/lms/templates/student_account/login.underscore b/lms/templates/student_account/login.underscore index 2a7294adfa..5420e769c4 100644 --- a/lms/templates/student_account/login.underscore +++ b/lms/templates/student_account/login.underscore @@ -1,7 +1,7 @@ @@ -20,6 +20,13 @@ +<% if (context.errorMessage) { %> +
+

<%- _.sprintf( gettext("An error occurred when signing you in to %(platformName)s."), context ) %>

+
    <%- context.errorMessage %>
+
+<% } %> +