diff --git a/common/djangoapps/third_party_auth/pipeline.py b/common/djangoapps/third_party_auth/pipeline.py index cc403e890f669c5c41265a5c681dbf4cfe440f5e..b6561c9259fd137f9b3aa92975d67af7464209f5 100644 --- a/common/djangoapps/third_party_auth/pipeline.py +++ b/common/djangoapps/third_party_auth/pipeline.py @@ -61,6 +61,7 @@ import random import string # pylint: disable-msg=deprecated-module from collections import OrderedDict import urllib +from ipware.ip import get_ip import analytics from eventtracking import tracker @@ -73,6 +74,7 @@ from social.exceptions import AuthException from social.pipeline import partial import student +from embargo import api as embargo_api from shoppingcart.models import Order, PaidCourseRegistration # pylint: disable=import-error from shoppingcart.exceptions import ( # pylint: disable=import-error CourseDoesNotExistException, @@ -634,7 +636,7 @@ def login_analytics(strategy, *args, **kwargs): @partial.partial -def change_enrollment(strategy, user=None, *args, **kwargs): +def change_enrollment(strategy, user=None, is_dashboard=False, *args, **kwargs): """Enroll a user in a course. If a user entered the authentication flow when trying to enroll @@ -653,17 +655,55 @@ def change_enrollment(strategy, user=None, *args, **kwargs): upon completion of the authentication pipeline (configured using the ?next parameter to the third party auth login url). + Keyword Arguments: + user (User): The user being authenticated. + is_dashboard (boolean): Whether the user entered the authentication + pipeline from the "link account" button on the student dashboard. + """ + # We skip enrollment if the user entered the flow from the "link account" + # button on the student dashboard. At this point, either: + # + # 1) The user already had a linked account when they started the enrollment flow, + # in which case they would have been enrolled during the normal authentication process. + # + # 2) The user did NOT have a linked account, in which case they would have + # needed to go through the login/register page. Since we preserve the querystring + # args when sending users to this page, successfully authenticating through this page + # would also enroll the student in the course. enroll_course_id = strategy.session_get('enroll_course_id') - if enroll_course_id: + if enroll_course_id and not is_dashboard: course_id = CourseKey.from_string(enroll_course_id) modes = CourseMode.modes_for_course_dict(course_id) + # If the email opt in parameter is found, set the preference. email_opt_in = strategy.session_get(AUTH_EMAIL_OPT_IN_KEY) if email_opt_in: opt_in = email_opt_in.lower() == 'true' profile.update_email_opt_in(user.username, course_id.org, opt_in) - if CourseMode.can_auto_enroll(course_id, modes_dict=modes): + + # Check whether we're blocked from enrolling by a + # country access rule. + # Note: We skip checking the user's profile setting + # for country here because the "redirect URL" pointing + # to the blocked message page is set when the user + # *enters* the pipeline, at which point they're + # not authenticated. If they end up being blocked + # from the courseware, it's better to let them + # enroll and then show the message when they + # enter the course than to skip enrollment + # altogether. + is_blocked = not embargo_api.check_course_access( + course_id, ip_address=get_ip(strategy.request), + url=strategy.request.path + ) + if is_blocked: + # If we're blocked, skip enrollment. + # A redirect URL should have been set so the user + # ends up on the embargo page when enrollment completes. + pass + + elif CourseMode.can_auto_enroll(course_id, modes_dict=modes): try: CourseEnrollment.enroll(user, course_id, check_access=True) except CourseEnrollmentException: diff --git a/common/djangoapps/third_party_auth/tests/test_change_enrollment.py b/common/djangoapps/third_party_auth/tests/test_change_enrollment.py index 159cac6c75b97b944d187296002ffd1bf78d2d3e..461211ed91db815c29cddc9bcda26274a36bf97c 100644 --- a/common/djangoapps/third_party_auth/tests/test_change_enrollment.py +++ b/common/djangoapps/third_party_auth/tests/test_change_enrollment.py @@ -4,20 +4,22 @@ 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 django.test.utils import override_settings 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 = ( @@ -27,15 +29,17 @@ THIRD_PARTY_AUTH_CONFIGURED = ( @unittest.skipUnless(THIRD_PARTY_AUTH_CONFIGURED, "Third party auth must be configured") +@patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) @ddt.ddt -class PipelineEnrollmentTest(ModuleStoreTestCase): +class PipelineEnrollmentTest(UrlResetMixin, ModuleStoreTestCase): """Test that the pipeline auto-enrolls students upon successful authentication. """ BACKEND_NAME = "google-oauth2" + @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) def setUp(self): """Create a test course and user. """ - super(PipelineEnrollmentTest, self).setUp() + super(PipelineEnrollmentTest, self).setUp('embargo') self.course = CourseFactory.create() self.user = UserFactory.create() @@ -125,6 +129,30 @@ class PipelineEnrollmentTest(ModuleStoreTestCase): self.assertEqual(result, {}) self.assertFalse(CourseEnrollment.is_enrolled(self.user, self.course.id)) + @patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': 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 dashboard's + # "link account" button. + result = pipeline.change_enrollment(strategy, 1, user=self.user, is_dashboard=True) # 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)) diff --git a/lms/djangoapps/student_account/test/test_views.py b/lms/djangoapps/student_account/test/test_views.py index af2e2080e385e8687a8a749dd5a2125056203a31..035a8a7856cccd589fa3ef2065966b3040bfb639 100644 --- a/lms/djangoapps/student_account/test/test_views.py +++ b/lms/djangoapps/student_account/test/test_views.py @@ -17,6 +17,7 @@ from django.test.utils import override_settings from util.testing import UrlResetMixin from third_party_auth.tests.testutil import simulate_running_pipeline +from embargo.test_utils import restrict_course from openedx.core.djangoapps.user_api.api import account as account_api from openedx.core.djangoapps.user_api.api import profile as profile_api from xmodule.modulestore.tests.django_utils import ( @@ -374,13 +375,17 @@ class StudentAccountUpdateTest(UrlResetMixin, TestCase): @ddt.ddt -class StudentAccountLoginAndRegistrationTest(ModuleStoreTestCase): +class StudentAccountLoginAndRegistrationTest(UrlResetMixin, ModuleStoreTestCase): """ Tests for the student account views that update the user's account information. """ USERNAME = "bob" EMAIL = "bob@example.com" PASSWORD = "password" + @mock.patch.dict(settings.FEATURES, {'ENABLE_COUNTRY_ACCESS': True}) + def setUp(self): + super(StudentAccountLoginAndRegistrationTest, self).setUp('embargo') + @ddt.data( ("account_login", "login"), ("account_register", "register"), @@ -546,6 +551,49 @@ class StudentAccountLoginAndRegistrationTest(ModuleStoreTestCase): 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, {'ENABLE_COUNTRY_ACCESS': 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) + @override_settings(SITE_NAME=settings.MICROSITE_TEST_HOSTNAME) def test_microsite_uses_old_login_page(self): # Retrieve the login page from a microsite domain diff --git a/lms/djangoapps/student_account/views.py b/lms/djangoapps/student_account/views.py index c2700044669f799ceef8ed6e6b7e9e0a49145c4f..29afd4ab2dee6858755b88bb4637a94c63984de0 100644 --- a/lms/djangoapps/student_account/views.py +++ b/lms/djangoapps/student_account/views.py @@ -2,6 +2,8 @@ import logging import json +from ipware.ip import get_ip + from django.conf import settings from django.http import ( HttpResponse, HttpResponseBadRequest, HttpResponseForbidden @@ -14,8 +16,12 @@ from django.utils.translation import ugettext as _ from django_future.csrf import ensure_csrf_cookie from django.contrib.auth.decorators import login_required from django.views.decorators.http import require_http_methods + +from opaque_keys.edx.keys import CourseKey +from opaque_keys import InvalidKeyError from edxmako.shortcuts import render_to_response, render_to_string from microsite_configuration import microsite +from embargo import api as embargo_api import third_party_auth from external_auth.login_and_register import ( login as external_auth_login, @@ -321,6 +327,36 @@ def _third_party_auth_context(request): 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, @@ -330,7 +366,8 @@ def _third_party_auth_context(request): register_urls = auth_pipeline_urls( third_party_auth.pipeline.AUTH_ENTRY_REGISTER, course_id=course_id, - email_opt_in=email_opt_in + email_opt_in=email_opt_in, + redirect_url=redirect_to ) if third_party_auth.is_enabled(): diff --git a/lms/templates/dashboard.html b/lms/templates/dashboard.html index 1a001725a3662107edbed65f9e1172cb3a38b2a2..b6ec3f3a692826533895a882f8e0d600f1081844 100644 --- a/lms/templates/dashboard.html +++ b/lms/templates/dashboard.html @@ -132,7 +132,7 @@ ${_("Unlink")} </a> % else: - <a href="${pipeline.get_login_url(state.provider.NAME, pipeline.AUTH_ENTRY_DASHBOARD)}"> + <a href="${pipeline.get_login_url(state.provider.NAME, pipeline.AUTH_ENTRY_DASHBOARD, redirect_url='/')}"> ## Translators: clicking on this creates a link between a user's edX account and their account with an external authentication provider (like Google or LinkedIn). ${_("Link")} </a>