Skip to content

E
edx-platform
Commit 07b81446 by Jason Bau
Options

Merge pull request #5062 from Stanford-Online/jbau/shib-unicode 

external_auth: handle request.META values as str, not unicode
parents e1c7bb83 4d5a2380
Showing with 42 additions and 39 deletions
common/djangoapps/external_auth/tests/test_shib.py
......@@ -37,12 +37,13 @@ TEST_DATA_MIXED_MODULESTORE = mixed_store_config(settings.COMMON_TEST_DATA_ROOT,
# 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 = 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 ';'
# These values would all returned from request.META, so they need to be str, not unicode
IDP = 'https://idp.stanford.edu/'
REMOTE_USER = 'test_user@stanford.edu'
MAILS = [None, '', 'test_user@stanford.edu'] # unicode shouldn't be in emails, would fail django's email validator
DISPLAYNAMES = [None, '', 'Jason 包']
GIVENNAMES = [None, '', 'jasön; John; bob'] # At Stanford, the givenNames can be a list delimited by ';'
SNS = [None, '', '包; smith'] # At Stanford, the sns can be a list delimited by ';'
def gen_all_identities():
......@@ -108,6 +109,7 @@ class ShibSPTest(ModuleStoreTestCase):
def _assert_shib_login_is_logged(self, audit_log_call, remote_user):
"""Asserts that shibboleth login attempt is being logged"""
remote_user = _flatten_to_ascii(remote_user) # django usernames have to be ascii
method_name, args, _kwargs = audit_log_call
self.assertEquals(method_name, 'info')
self.assertEquals(len(args), 1)
......@@ -312,12 +314,13 @@ class ShibSPTest(ModuleStoreTestCase):
client = DjangoTestClient()
response1 = client.get(path='/shib-login/', data={}, follow=False, **identity)
# Then we have the user answer the registration form
postvars = {'email': 'post_email@stanford.edu',
'username': 'post_username',
'password': 'post_password',
'name': 'post_name',
'terms_of_service': 'true',
'honor_code': 'true'}
# These are unicode because request.POST returns unicode
postvars = {'email': u'post_email@stanford.edu',
'username': u'post_username', # django usernames can't be unicode
'password': u'post_pássword',
'name': u'post_náme',
'terms_of_service': u'true',
'honor_code': u'true'}
# use RequestFactory instead of TestClient here because we want access to request.user
request2 = self.request_factory.post('/create_account', data=postvars)
request2.session = client.session
......@@ -374,7 +377,7 @@ class ShibSPTest(ModuleStoreTestCase):
self.assertNotIn(u';', profile.name)
else:
self.assertEqual(profile.name, request2.session['ExternalAuthMap'].external_name)
self.assertEqual(profile.name, identity.get('displayName'))
self.assertEqual(profile.name, identity.get('displayName').decode('utf-8'))
# clean up for next loop
request2.session['ExternalAuthMap'].delete()
......@@ -596,9 +599,9 @@ class ShibUtilFnTest(TestCase):
DIACRITIC = u"àèìòùÀÈÌÒÙáéíóúýÁÉÍÓÚÝâêîôûÂÊÎÔÛãñõÃÑÕäëïöüÿÄËÏÖÜŸåÅçÇ" # pylint: disable=C0103
STR_DIACRI = "àèìòùÀÈÌÒÙáéíóúýÁÉÍÓÚÝâêîôûÂÊÎÔÛãñõÃÑÕäëïöüÿÄËÏÖÜŸåÅçÇ" # 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('jasön'), 'jason') # umlaut
self.assertEqual(_flatten_to_ascii('Jason包'), 'Jason') # mandarin, so it just gets dropped
self.assertEqual(_flatten_to_ascii('abc'), 'abc') # pass through
unicode_test = _flatten_to_ascii(DIACRITIC)
self.assertEqual(unicode_test, FLATTENED)
......
common/djangoapps/external_auth/views.py
......@@ -137,7 +137,7 @@ def _external_login_or_signup(request,
try:
eamap = ExternalAuthMap.objects.get(external_id=external_id,
external_domain=external_domain)
log.debug('Found eamap=%s', eamap)
log.debug(u'Found eamap=%s', eamap)
except ExternalAuthMap.DoesNotExist:
# go render form for creating edX user
eamap = ExternalAuthMap(external_id=external_id,
......@@ -146,7 +146,7 @@ def _external_login_or_signup(request,
eamap.external_email = email
eamap.external_name = fullname
eamap.internal_password = generate_password()
log.debug('Created eamap=%s', eamap)
log.debug(u'Created eamap=%s', eamap)
eamap.save()
log.info(u"External_Auth login_or_signup for %s : %s : %s : %s", external_domain, external_id, email, fullname)
......@@ -166,7 +166,7 @@ def _external_login_or_signup(request,
eamap.user = link_user
eamap.save()
internal_user = link_user
log.info('SHIB: Linking existing account for %s', eamap.external_id)
log.info(u'SHIB: Linking existing account for %s', eamap.external_id)
# now pass through to log in
else:
# otherwise, there must have been an error, b/c we've already linked a user with these external
......@@ -177,10 +177,10 @@ def _external_login_or_signup(request,
% getattr(settings, 'TECH_SUPPORT_EMAIL', 'techsupport@class.stanford.edu')))
return default_render_failure(request, failure_msg)
except User.DoesNotExist:
log.info('SHIB: No user for %s yet, doing signup', eamap.external_email)
log.info(u'SHIB: No user for %s yet, doing signup', eamap.external_email)
return _signup(request, eamap, retfun)
else:
log.info('No user for %s yet. doing signup', eamap.external_email)
log.info(u'No user for %s yet. doing signup', eamap.external_email)
return _signup(request, eamap, retfun)
# We trust shib's authentication, so no need to authenticate using the password again
......@@ -194,25 +194,25 @@ def _external_login_or_signup(request,
auth_backend = 'django.contrib.auth.backends.ModelBackend'
user.backend = auth_backend
if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
AUDIT_LOG.info('Linked user.id: {0} logged in via Shibboleth'.format(user.id))
AUDIT_LOG.info(u'Linked user.id: {0} logged in via Shibboleth'.format(user.id))
else:
AUDIT_LOG.info('Linked user "{0}" logged in via Shibboleth'.format(user.email))
AUDIT_LOG.info(u'Linked user "{0}" logged in via Shibboleth'.format(user.email))
elif uses_certs:
# Certificates are trusted, so just link the user and log the action
user = internal_user
user.backend = 'django.contrib.auth.backends.ModelBackend'
if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
AUDIT_LOG.info('Linked user_id {0} logged in via SSL certificate'.format(user.id))
AUDIT_LOG.info(u'Linked user_id {0} logged in via SSL certificate'.format(user.id))
else:
AUDIT_LOG.info('Linked user "{0}" logged in via SSL certificate'.format(user.email))
AUDIT_LOG.info(u'Linked user "{0}" logged in via SSL certificate'.format(user.email))
else:
user = authenticate(username=uname, password=eamap.internal_password, request=request)
if user is None:
# we want to log the failure, but don't want to log the password attempted:
if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
AUDIT_LOG.warning('External Auth Login failed')
AUDIT_LOG.warning(u'External Auth Login failed')
else:
AUDIT_LOG.warning('External Auth Login failed for "{0}"'.format(uname))
AUDIT_LOG.warning(u'External Auth Login failed for "{0}"'.format(uname))
return _signup(request, eamap, retfun)
if not user.is_active:
......@@ -222,14 +222,14 @@ def _external_login_or_signup(request,
user.is_active = True
user.save()
if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
AUDIT_LOG.info('Activating user {0} due to external auth'.format(user.id))
AUDIT_LOG.info(u'Activating user {0} due to external auth'.format(user.id))
else:
AUDIT_LOG.info('Activating user "{0}" due to external auth'.format(uname))
AUDIT_LOG.info(u'Activating user "{0}" due to external auth'.format(uname))
else:
if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
AUDIT_LOG.warning('User {0} is not active after external login'.format(user.id))
AUDIT_LOG.warning(u'User {0} is not active after external login'.format(user.id))
else:
AUDIT_LOG.warning('User "{0}" is not active after external login'.format(uname))
AUDIT_LOG.warning(u'User "{0}" is not active after external login'.format(uname))
# TODO: improve error page
msg = 'Account not yet activated: please look for link in your email'
return default_render_failure(request, msg)
......@@ -246,9 +246,9 @@ def _external_login_or_signup(request,
else:
student.views.try_change_enrollment(request)
if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
AUDIT_LOG.info("Login success - user.id: {0}".format(user.id))
AUDIT_LOG.info(u"Login success - user.id: {0}".format(user.id))
else:
AUDIT_LOG.info("Login success - {0} ({1})".format(user.username, user.email))
AUDIT_LOG.info(u"Login success - {0} ({1})".format(user.username, user.email))
if retfun is None:
return redirect('/')
return retfun()
......@@ -292,7 +292,7 @@ def _signup(request, eamap, retfun=None):
post_vars = dict(username=username,
honor_code=u'true',
terms_of_service=u'true')
log.info('doing immediate signup for %s, params=%s', username, post_vars)
log.info(u'doing immediate signup for %s, params=%s', username, post_vars)
student.views.create_account(request, post_vars)
# should check return content for successful completion before
if retfun is not None:
......@@ -331,7 +331,7 @@ def _signup(request, eamap, retfun=None):
except ValidationError:
context['ask_for_email'] = True
log.info('EXTAUTH: Doing signup for %s', eamap.external_id)
log.info(u'EXTAUTH: Doing signup for %s', eamap.external_id)
return student.views.register_user(request, extra_context=context)
......@@ -508,14 +508,14 @@ def shib_login(request):
"""))
if not request.META.get('REMOTE_USER'):
log.error("SHIB: no REMOTE_USER found in request.META")
log.error(u"SHIB: no REMOTE_USER found in request.META")
return default_render_failure(request, shib_error_msg)
elif not request.META.get('Shib-Identity-Provider'):
log.error("SHIB: no Shib-Identity-Provider in request.META")
log.error(u"SHIB: no Shib-Identity-Provider in request.META")
return default_render_failure(request, shib_error_msg)
else:
# If we get here, the user has authenticated properly
shib = {attr: request.META.get(attr, '')
shib = {attr: request.META.get(attr, '').decode('utf-8')
for attr in ['REMOTE_USER', 'givenName', 'sn', 'mail', 'Shib-Identity-Provider', 'displayName']}
# Clean up first name, last name, and email address
......@@ -525,7 +525,7 @@ def shib_login(request):
shib['givenName'] = shib['givenName'].split(";")[0].strip().capitalize()
# TODO: should we be logging creds here, at info level?
log.info("SHIB creds returned: %r", shib)
log.info(u"SHIB creds returned: %r", shib)
fullname = shib['displayName'] if shib['displayName'] else u'%s %s' % (shib['givenName'], shib['sn'])
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment