external_auth: handle request.META values as str, not unicode

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'])