Fix for error in TNL-363

Audit logs were causing open_id provider logins to fail when
username or e-mail address contained unicode data.

common/djangoapps/external_auth/tests/test_openid_provider.py

@@ -260,27 +260,100 @@ class OpenIdProviderTest(TestCase):
         # clear the ratelimit cache so that we don't fail other logins
         cache.clear()
 
+    def _attempt_login_and_perform_final_response(self, user, profile_name):
+        """
+        Performs full procedure of a successful OpenID provider login for user,
+        all required data is taken form ``user`` attribute which is an instance
+        of ``User`` model. As a convenience this method will also set
+        ``profile.name`` for the user.
+        """
+        url = reverse('openid-provider-login')
+
+        # login to the client so that we can persist session information
+        user.profile.name = profile_name
+        user.profile.save()
+        # It is asssumed that user's password is test (default for UserFactory)
+        self.client.login(username=user.username, password='test')
+        # login once to get the right session information
+        self.attempt_login(200)
+        post_args = {
+            'email': user.email,
+            'password': 'test'
+        }
+
+        # call url again, this time with username and password
+        return self.client.post(url, post_args)
+
+    @skipUnless(
+        settings.FEATURES.get('AUTH_USE_OPENID_PROVIDER'), 'OpenID not enabled')
+    def test_provider_login_can_handle_unicode_email(self):
+        user = UserFactory(email=u"user.ąęł@gmail.com")
+        resp = self._attempt_login_and_perform_final_response(user, u"Jan ĄĘŁ")
+        location = resp['Location']
+        parsed_url = urlparse(location)
+        parsed_qs = parse_qs(parsed_url.query)
+        self.assertEquals(parsed_qs['openid.ax.type.ext1'][0], 'http://axschema.org/contact/email')
+        self.assertEquals(parsed_qs['openid.ax.type.ext0'][0], 'http://axschema.org/namePerson')
+        self.assertEquals(parsed_qs['openid.ax.value.ext0.1'][0],
+                          user.profile.name.encode('utf-8'))  # pylint: disable=no-member
+        self.assertEquals(parsed_qs['openid.ax.value.ext1.1'][0],
+                          user.email.encode('utf-8'))  # pylint: disable=no-member
+
+    @skipUnless(
+        settings.FEATURES.get('AUTH_USE_OPENID_PROVIDER'), 'OpenID not enabled')
+    def test_provider_login_can_handle_unicode_email_invalid_password(self):
+        user = UserFactory(email=u"user.ąęł@gmail.com")
+        url = reverse('openid-provider-login')
+
+        # login to the client so that we can persist session information
+        user.profile.name = u"Jan ĄĘ"
+        user.profile.save()
+        # It is asssumed that user's password is test (default for UserFactory)
+        self.client.login(username=user.username, password='test')
+        # login once to get the right session information
+        self.attempt_login(200)
+        # We trigger situation where user password is invalid at last phase
+        # of openid login
+        post_args = {
+            'email': user.email,
+            'password': 'invalid-password'
+        }
+
+        # call url again, this time with username and password
+        return self.client.post(url, post_args)
+
+    @skipUnless(
+        settings.FEATURES.get('AUTH_USE_OPENID_PROVIDER'), 'OpenID not enabled')
+    def test_provider_login_can_handle_unicode_email_inactive_account(self):
+        user = UserFactory(email=u"user.ąęł@gmail.com", username=u"ąęół")
+        url = reverse('openid-provider-login')
+
+        # login to the client so that we can persist session information
+        user.profile.name = u'Jan ĄĘ'
+        user.profile.save()  # pylint: disable=no-member
+        self.client.login(username=user.username, password='test')
+        # login once to get the right session information
+        self.attempt_login(200)
+        # We trigger situation where user is not active at final phase of
+        # OpenId login.
+        user.is_active = False
+        user.save()  # pylint: disable=no-member
+        post_args = {
+            'email': user.email,
+            'password': 'test'
+        }
+        # call url again, this time with username and password
+        self.client.post(url, post_args)
+
     @skipUnless(settings.FEATURES.get('AUTH_USE_OPENID_PROVIDER'),
                 'OpenID not enabled')
     def test_openid_final_response(self):
 
-        url = reverse('openid-provider-login')
         user = UserFactory()
 
         # login to the client so that we can persist session information
         for name in ['Robot 33', '☃']:
-            user.profile.name = name
-            user.profile.save()
-            self.client.login(username=user.username, password='test')
-            # login once to get the right session information
-            self.attempt_login(200)
-            post_args = {
-                'email': user.email,
-                'password': 'test',
-            }
-
-            # call url again, this time with username and password
-            resp = self.client.post(url, post_args)
+            resp = self._attempt_login_and_perform_final_response(user, name)
             # all information is embedded in the redirect url
             location = resp['Location']
             # parse the url

common/djangoapps/external_auth/views.py

@@ -836,9 +836,9 @@ def provider_login(request):
         except User.DoesNotExist:
             request.session['openid_error'] = True
             if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
-                AUDIT_LOG.warning("OpenID login failed - Unknown user email")
+                AUDIT_LOG.warning(u"OpenID login failed - Unknown user email")
             else:
-                msg = "OpenID login failed - Unknown user email: {0}".format(email)
+                msg = u"OpenID login failed - Unknown user email: {0}".format(email)
                 AUDIT_LOG.warning(msg)
             return HttpResponseRedirect(openid_request_url)
 
@@ -849,16 +849,16 @@ def provider_login(request):
         try:
             user = authenticate(username=username, password=password, request=request)
         except RateLimitException:
-            AUDIT_LOG.warning('OpenID - Too many failed login attempts.')
+            AUDIT_LOG.warning(u'OpenID - Too many failed login attempts.')
             return HttpResponseRedirect(openid_request_url)
 
         if user is None:
             request.session['openid_error'] = True
             if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
-                AUDIT_LOG.warning("OpenID login failed - invalid password")
+                AUDIT_LOG.warning(u"OpenID login failed - invalid password")
             else:
-                msg = "OpenID login failed - password for {0} is invalid".format(email)
-                AUDIT_LOG.warning(msg)
+                AUDIT_LOG.warning(
+                    u"OpenID login failed - password for %s is invalid", email)
             return HttpResponseRedirect(openid_request_url)
 
         # authentication succeeded, so fetch user information
@@ -869,11 +869,10 @@ def provider_login(request):
                 del request.session['openid_error']
 
             if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
-                AUDIT_LOG.info("OpenID login success - user.id: {0}".format(user.id))
+                AUDIT_LOG.info(u"OpenID login success - user.id: %s", user.id)
             else:
-                AUDIT_LOG.info("OpenID login success - {0} ({1})".format(
-                               user.username, user.email))
-
+                AUDIT_LOG.info(
+                    u"OpenID login success - %s (%s)", user.username, user.email)
             # redirect user to return_to location
             url = endpoint + urlquote(user.username)
             response = openid_request.answer(True, None, url)
@@ -892,10 +891,11 @@ def provider_login(request):
         # the account is not active, so redirect back to the login page:
         request.session['openid_error'] = True
         if settings.FEATURES['SQUELCH_PII_IN_LOGS']:
-            AUDIT_LOG.warning("Login failed - Account not active for user.id {0}".format(user.id))
+            AUDIT_LOG.warning(
+                u"Login failed - Account not active for user.id %s", user.id)
         else:
-            msg = "Login failed - Account not active for user {0}".format(username)
-            AUDIT_LOG.warning(msg)
+            AUDIT_LOG.warning(
+                u"Login failed - Account not active for user %s", username)
         return HttpResponseRedirect(openid_request_url)
 
     # determine consumer domain if applicable