Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revamped + Enhanced Shibboleth support #842

Merged
merged 5 commits into from
Sep 10, 2013
Merged

Revamped + Enhanced Shibboleth support #842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
948c07c
Revamped + Enhanced Shibboleth support
jbau Aug 30, 2013
4d65606
Removing handling for ExternalAuthMap.MultipleObjectsReturned
jbau Sep 4, 2013
6a850e2
Address @brianhw review comments
jbau Sep 6, 2013
9dac2ef
actually flatten username suggestion to ascii
jbau Sep 9, 2013
7b9c6fb
remove spaces from usename suggestion
jbau Sep 10, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions common/djangoapps/external_auth/tests/test_helper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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'])
106 changes: 75 additions & 31 deletions common/djangoapps/external_auth/tests/test_shib.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# -*- coding: utf-8 -*-
"""
Tests for Shibboleth Authentication
@jbau
Expand All @@ -7,6 +8,7 @@

from django.conf import settings
from django.http import HttpResponseRedirect
from django.test import TestCase
from django.test.client import RequestFactory, Client as DjangoTestClient
from django.test.utils import override_settings
from django.core.urlresolvers import reverse
Expand All @@ -19,7 +21,7 @@
from xmodule.modulestore.django import editable_modulestore

from external_auth.models import ExternalAuthMap
from external_auth.views import shib_login, course_specific_login, course_specific_register
from external_auth.views import shib_login, course_specific_login, course_specific_register, _flatten_to_ascii

from student.views import create_account, change_enrollment
from student.models import UserProfile, Registration, CourseEnrollment
Expand All @@ -32,11 +34,12 @@
# b/c of how mod_shib works but should test the behavior with the rest of the attributes present/missing

# For the sake of python convention we'll make all of these variable names ALL_CAPS
IDP = 'https://idp.stanford.edu/'
REMOTE_USER = 'test_user@stanford.edu'
MAILS = [None, '', 'test_user@stanford.edu']
GIVENNAMES = [None, '', 'Jason', 'jas\xc3\xb6n; John; bob'] # At Stanford, the givenNames can be a list delimited by ';'
SNS = [None, '', 'Bau', '\xe5\x8c\x85; smith'] # At Stanford, the sns can be a list delimited by ';'
IDP = u'https://idp.stanford.edu/'
REMOTE_USER = u'test_user@stanford.edu'
MAILS = [None, u'', u'test_user@stanford.edu']
DISPLAYNAMES = [None, u'', u'Jason \u5305']
GIVENNAMES = [None, u'', u'jas\xf6n; John; bob'] # At Stanford, the givenNames can be a list delimited by ';'
SNS = [None, u'', u'\u5305; smith'] # At Stanford, the sns can be a list delimited by ';'


def gen_all_identities():
Expand All @@ -46,10 +49,12 @@ def gen_all_identities():
could potentially pass to django via request.META, i.e.
setting (or not) request.META['givenName'], etc.
"""
def _build_identity_dict(mail, given_name, surname):
def _build_identity_dict(mail, display_name, given_name, surname):
""" Helper function to return a dict of test identity """
meta_dict = {'Shib-Identity-Provider': IDP,
'REMOTE_USER': REMOTE_USER}
if display_name is not None:
meta_dict['displayName'] = display_name
if mail is not None:
meta_dict['mail'] = mail
if given_name is not None:
Expand All @@ -61,7 +66,8 @@ def _build_identity_dict(mail, given_name, surname):
for mail in MAILS:
for given_name in GIVENNAMES:
for surname in SNS:
yield _build_identity_dict(mail, given_name, surname)
for display_name in DISPLAYNAMES:
yield _build_identity_dict(mail, display_name, given_name, surname)


@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE, SESSION_ENGINE='django.contrib.sessions.backends.cache')
Expand All @@ -75,7 +81,7 @@ class ShibSPTest(ModuleStoreTestCase):
def setUp(self):
self.store = editable_modulestore()

@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True)
@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh. The 2nd argument of skipUnless is supposed to be a reason string. Lucky happenstance that this worked before.

def test_exception_shib_login(self):
"""
Tests that we get the error page when there is no REMOTE_USER
Expand All @@ -101,7 +107,7 @@ def _assert_shib_login_is_logged(self, audit_log_call, remote_user):
self.assertIn(u'logged in via Shibboleth', args[0])
self.assertEquals(remote_user, args[1])

@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True)
@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
def test_shib_login(self):
"""
Tests that:
Expand Down Expand Up @@ -195,11 +201,13 @@ def test_shib_login(self):
self.assertEquals(len(audit_log_calls), 0)
else:
self.assertEqual(response.status_code, 200)
self.assertContains(response, "<title>Register for")
self.assertContains(response,
("<title>Preferences for {platform_name}</title>"
.format(platform_name=settings.PLATFORM_NAME)))
# no audit logging calls
self.assertEquals(len(audit_log_calls), 0)

@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True)
@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
def test_registration_form(self):
"""
Tests the registration form showing up with the proper parameters.
Expand All @@ -219,17 +227,18 @@ def test_registration_form(self):
self.assertNotContains(response, mail_input_HTML)
sn_empty = not identity.get('sn')
given_name_empty = not identity.get('givenName')
fullname_input_HTML = '<input id="name" type="text" name="name"'
if sn_empty and given_name_empty:
self.assertContains(response, fullname_input_HTML)
displayname_empty = not identity.get('displayName')
fullname_input_html = '<input id="name" type="text" name="name"'
if sn_empty and given_name_empty and displayname_empty:
self.assertContains(response, fullname_input_html)
else:
self.assertNotContains(response, fullname_input_HTML)
self.assertNotContains(response, fullname_input_html)

# clean up b/c we don't want existing ExternalAuthMap for the next run
client.session['ExternalAuthMap'].delete()

@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True)
def test_registration_formSubmit(self):
@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
def test_registration_form_submit(self):
"""
Tests user creation after the registration form that pops is submitted. If there is no shib
ExternalAuthMap in the session, then the created user should take the username and email from the
Expand Down Expand Up @@ -292,18 +301,26 @@ def test_registration_formSubmit(self):
profile = UserProfile.objects.get(user=user)
sn_empty = not identity.get('sn')
given_name_empty = not identity.get('givenName')
if sn_empty and given_name_empty:
self.assertEqual(profile.name, postvars['name'])
displayname_empty = not identity.get('displayName')

if displayname_empty:
if sn_empty and given_name_empty:
self.assertEqual(profile.name, postvars['name'])
else:
self.assertEqual(profile.name, request2.session['ExternalAuthMap'].external_name)
self.assertNotIn(u';', profile.name)
else:
self.assertEqual(profile.name, request2.session['ExternalAuthMap'].external_name)
self.assertEqual(profile.name, identity.get('displayName'))

# clean up for next loop
request2.session['ExternalAuthMap'].delete()
UserProfile.objects.filter(user=user).delete()
Registration.objects.filter(user=user).delete()
user.delete()

@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True)
def test_course_specificLoginAndReg(self):
@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
def test_course_specific_login_and_reg(self):
"""
Tests that the correct course specific login and registration urls work for shib
"""
Expand All @@ -322,8 +339,8 @@ def test_course_specificLoginAndReg(self):
'?course_id=MITx/999/Robot_Super_Course' +
'&enrollment_action=enroll')
_reg_request = self.request_factory.get('/course_specific_register/MITx/999/Robot_Super_Course' +
'?course_id=MITx/999/course/Robot_Super_Course' +
'&enrollment_action=enroll')
'?course_id=MITx/999/course/Robot_Super_Course' +
'&enrollment_action=enroll')

login_response = course_specific_login(login_request, 'MITx/999/Robot_Super_Course')
reg_response = course_specific_register(login_request, 'MITx/999/Robot_Super_Course')
Expand Down Expand Up @@ -357,8 +374,8 @@ def test_course_specificLoginAndReg(self):
'?course_id=DNE/DNE/DNE' +
'&enrollment_action=enroll')
_reg_request = self.request_factory.get('/course_specific_register/DNE/DNE/DNE' +
'?course_id=DNE/DNE/DNE/Robot_Super_Course' +
'&enrollment_action=enroll')
'?course_id=DNE/DNE/DNE/Robot_Super_Course' +
'&enrollment_action=enroll')

login_response = course_specific_login(login_request, 'DNE/DNE/DNE')
reg_response = course_specific_register(login_request, 'DNE/DNE/DNE')
Expand All @@ -374,7 +391,7 @@ def test_course_specificLoginAndReg(self):
'?course_id=DNE/DNE/DNE' +
'&enrollment_action=enroll')

@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True)
@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
def test_enrollment_limit_by_domain(self):
"""
Tests that the enrollmentDomain setting is properly limiting enrollment to those who have
Expand Down Expand Up @@ -438,10 +455,12 @@ def test_enrollment_limit_by_domain(self):
self.assertEqual(response.status_code, 400)
self.assertFalse(CourseEnrollment.is_enrolled(student, course.id))

@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), True)
@unittest.skipUnless(settings.MITX_FEATURES.get('AUTH_USE_SHIB'), "AUTH_USE_SHIB not set")
def test_shib_login_enrollment(self):
"""
A functionality test that a student with an existing shib login can auto-enroll in a class with GET params
A functionality test that a student with an existing shib login
can auto-enroll in a class with GET or POST params. Also tests the direction functionality of
the 'next' GET/POST param
"""
student = UserFactory.create()
extauth = ExternalAuthMap(external_id='testuser@stanford.edu',
Expand All @@ -465,13 +484,38 @@ def test_shib_login_enrollment(self):
self.assertFalse(CourseEnrollment.is_enrolled(student, course.id))
self.client.logout()
request_kwargs = {'path': '/shib-login/',
'data': {'enrollment_action': 'enroll', 'course_id': course.id},
'data': {'enrollment_action': 'enroll', 'course_id': course.id, 'next': '/testredirect'},
'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 "/"
self.assertEqual(response.status_code, 302)
self.assertEqual(response['location'], 'http://testserver/')
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))


class ShibUtilFnTest(TestCase):
"""
Tests util functions in shib module
"""
def test__flatten_to_ascii(self):
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added test for flattening to ascii.

DIACRITIC = u"àèìòùÀÈÌÒÙáéíóúýÁÉÍÓÚÝâêîôûÂÊÎÔÛãñõÃÑÕäëïöüÿÄËÏÖÜŸåÅçÇ" # pylint: disable=C0103
FLATTENED = u"aeiouAEIOUaeiouyAEIOUYaeiouAEIOUanoANOaeiouyAEIOUYaAcC" # pylint: disable=C0103
self.assertEqual(_flatten_to_ascii(u'jas\xf6n'), u'jason') # umlaut
self.assertEqual(_flatten_to_ascii(u'Jason\u5305'), u'Jason') # mandarin, so it just gets dropped
self.assertEqual(_flatten_to_ascii(u'abc'), u'abc') # pass through
self.assertEqual(_flatten_to_ascii(DIACRITIC), FLATTENED)
Loading