diff --git a/common/djangoapps/external_auth/tests/test_helper.py b/common/djangoapps/external_auth/tests/test_helper.py new file mode 100644 index 0000000000..ff463e3355 --- /dev/null +++ b/common/djangoapps/external_auth/tests/test_helper.py @@ -0,0 +1,29 @@ +""" +Tests for utility functions in external_auth module +""" +from django.test import TestCase +from external_auth.views import _safe_postlogin_redirect + + +class ExternalAuthHelperFnTest(TestCase): + """ + Unit tests for the external_auth.views helper function + """ + def test__safe_postlogin_redirect(self): + """ + Tests the _safe_postlogin_redirect function with different values of next + """ + HOST = 'testserver' # pylint: disable=C0103 + ONSITE1 = '/dashboard' # pylint: disable=C0103 + ONSITE2 = '/courses/org/num/name/courseware' # pylint: disable=C0103 + ONSITE3 = 'http://{}/my/custom/url'.format(HOST) # pylint: disable=C0103 + OFFSITE1 = 'http://www.attacker.com' # pylint: disable=C0103 + + for redirect_to in [ONSITE1, ONSITE2, ONSITE3]: + redir = _safe_postlogin_redirect(redirect_to, HOST) + self.assertEqual(redir.status_code, 302) + self.assertEqual(redir['location'], redirect_to) + + redir2 = _safe_postlogin_redirect(OFFSITE1, HOST) + self.assertEqual(redir2.status_code, 302) + self.assertEqual("/", redir2['location']) diff --git a/common/djangoapps/external_auth/tests/test_shib.py b/common/djangoapps/external_auth/tests/test_shib.py index b1a03711ce..ab12de6aa9 100644 --- a/common/djangoapps/external_auth/tests/test_shib.py +++ b/common/djangoapps/external_auth/tests/test_shib.py @@ -1,4 +1,3 @@ - """ Tests for Shibboleth Authentication @jbau @@ -227,17 +226,17 @@ class ShibSPTest(ModuleStoreTestCase): sn_empty = not identity.get('sn') given_name_empty = not identity.get('givenName') displayname_empty = not identity.get('displayName') - fullname_input_HTML = '[^/]+/[^/]+/[^/]+)', input_str) if m_obj: return m_obj.group('course_id') return None -def get_course_enrollment_domain(course_id): +def _get_course_enrollment_domain(course_id): + """ + Helper function to get the enrollment domain set for a course with id course_id + @param course_id: + @return: + """ try: course = course_from_id(course_id) return course.enrollment_domain @@ -435,10 +443,10 @@ def accounts_login(request): return redirect(reverse('cas-login')) # see if the "next" parameter has been set, whether it has a course context, and if so, whether # there is a course-specific place to redirect - next = request.GET.get('next') - if next: - course_id = parse_course_id_from_string(next) - if course_id and get_course_enrollment_domain(course_id): + redirect_to = request.GET.get('next') + if redirect_to: + course_id = _parse_course_id_from_string(redirect_to) + if course_id and _get_course_enrollment_domain(course_id): return external_auth.views.course_specific_login(request, course_id) return render_to_response('login.html') @@ -465,11 +473,11 @@ def login_user(request, error=""): if settings.MITX_FEATURES.get('AUTH_USE_SHIB') and user: try: eamap = ExternalAuthMap.objects.get(user=user) - if eamap.external_domain.startswith(SHIB_DOMAIN_PREFIX): + if eamap.external_domain.startswith(external_auth.views.SHIBBOLETH_DOMAIN_PREFIX): return HttpResponse(json.dumps({'success': False, 'redirect': reverse('shib-login')})) except ExternalAuthMap.DoesNotExist: # This is actually the common case, logging in user without external linked login - log.info("User %s w/o external auth attempting login", user) + AUDIT_LOG.info("User %s w/o external auth attempting login", user) # if the user doesn't exist, we want to set the username to an invalid # username so that authentication is guaranteed to fail and we can take @@ -674,7 +682,8 @@ def create_account(request, post_override=None): # Can't have terms of service for certain SHIB users, like at Stanford tos_not_required = (settings.MITX_FEATURES.get("AUTH_USE_SHIB") and settings.MITX_FEATURES.get('SHIB_DISABLE_TOS') and - DoExternalAuth and eamap.external_domain.startswith(SHIB_DOMAIN_PREFIX)) + DoExternalAuth and + eamap.external_domain.startswith(external_auth.views.SHIBBOLETH_DOMAIN_PREFIX)) if not tos_not_required: if post_vars.get('terms_of_service', 'false') != u'true': diff --git a/lms/envs/common.py b/lms/envs/common.py index 5d6cc5598d..d8f2511153 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -94,6 +94,8 @@ MITX_FEATURES = { 'AUTH_USE_OPENID': False, 'AUTH_USE_MIT_CERTIFICATES': False, 'AUTH_USE_OPENID_PROVIDER': False, + # Even though external_auth is in common, shib assumes the LMS views / urls, so it should only be enabled + # in LMS 'AUTH_USE_SHIB': False, 'AUTH_USE_CAS': False, diff --git a/lms/templates/login.html b/lms/templates/login.html index 9a06fc4717..ac17e33319 100644 --- a/lms/templates/login.html +++ b/lms/templates/login.html @@ -53,9 +53,12 @@ } else { location.href="${reverse('dashboard')}"; } - } else if(json.hasOwnProperty('redirect')){ - var u=decodeURI(window.location.search); - location.href=json.redirect+u; + } else if(json.hasOwnProperty('redirect')) { + 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; + } // else we just remain on this page, which is fine since this particular path implies a login failure + // that has been generated via packet tampering (json.redirect has been messed with). } else { toggleSubmitButton(true); $('.message.submission-error').addClass('is-shown').focus();