Merge pull request #5594 from edx/diana/open-id-bug

Handle OpenID errors for POST requests.

common/djangoapps/external_auth/tests/test_openid_provider.py

@@ -5,7 +5,6 @@ Created on Jan 18, 2013
 @author: brian
 '''
 import openid
-import json
 from openid.fetchers import HTTPFetcher, HTTPResponse
 from urlparse import parse_qs, urlparse
 
@@ -73,7 +72,6 @@ class OpenIdProviderTest(TestCase):
     """
     Tests of the OpenId login
     """
-
     @skipUnless(settings.FEATURES.get('AUTH_USE_OPENID') and
                 settings.FEATURES.get('AUTH_USE_OPENID_PROVIDER'),
                 'OpenID not enabled')
@@ -155,10 +153,10 @@ class OpenIdProviderTest(TestCase):
             # <input name="openid.return_to" type="hidden" value="http://testserver/openid/complete/?janrain_nonce=2013-01-23T06%3A20%3A17ZaN7j6H" />
             # <input name="openid.assoc_handle" type="hidden" value="{HMAC-SHA1}{50ff8120}{rh87+Q==}" />
 
-    def attempt_login(self, expected_code, **kwargs):
+    def attempt_login(self, expected_code, login_method='POST', **kwargs):
         """ Attempt to log in through the open id provider login """
         url = reverse('openid-provider-login')
-        post_args = {
+        args = {
             "openid.mode": "checkid_setup",
             "openid.return_to": "http://testserver/openid/complete/?janrain_nonce=2013-01-23T06%3A20%3A17ZaN7j6H",
             "openid.assoc_handle": "{HMAC-SHA1}{50ff8120}{rh87+Q==}",
@@ -180,9 +178,15 @@ class OpenIdProviderTest(TestCase):
         }
         # override the default args with any given arguments
         for key in kwargs:
-            post_args["openid." + key] = kwargs[key]
+            args["openid." + key] = kwargs[key]
+
+        if login_method == 'POST':
+            resp = self.client.post(url, args)
+        elif login_method == 'GET':
+            resp = self.client.get(url, args)
+        else:
+            self.fail('Invalid login method')
 
-        resp = self.client.post(url, post_args)
         code = expected_code
         self.assertEqual(resp.status_code, code,
                          "got code {0} for url '{1}'. Expected code {2}"
@@ -224,7 +228,8 @@ class OpenIdProviderTest(TestCase):
         request = factory.post(reverse('openid-provider-login'), post_params)
         openid_setup = {
             'request': factory.request(),
-            'url': fake_url
+            'url': fake_url,
+            'post_params': {}
         }
         request.session = {
             'openid_setup': openid_setup
@@ -286,6 +291,35 @@ class OpenIdProviderTest(TestCase):
             self.assertEquals(parsed_qs['openid.ax.value.ext1.1'][0], user.email)
             self.assertEquals(parsed_qs['openid.ax.value.ext0.1'][0], user.profile.name)
 
+    @skipUnless(settings.FEATURES.get('AUTH_USE_OPENID_PROVIDER'),
+                'OpenID not enabled')
+    def test_openid_invalid_password(self):
+
+        url = reverse('openid-provider-login')
+        user = UserFactory()
+
+        # login to the client so that we can persist session information
+        for method in ['POST', 'GET']:
+            self.client.login(username=user.username, password='test')
+            self.attempt_login(200, method)
+            openid_setup = self.client.session['openid_setup']
+            self.assertIn('post_params', openid_setup)
+            post_args = {
+                'email': user.email,
+                'password': 'bad_password',
+            }
+
+            # call url again, this time with username and password
+            resp = self.client.post(url, post_args)
+            self.assertEquals(resp.status_code, 302)
+            redirect_url = resp['Location']
+            parsed_url = urlparse(redirect_url)
+            query_params = parse_qs(parsed_url[4])
+            self.assertIn('openid.return_to', query_params)
+            self.assertTrue(
+                query_params['openid.return_to'][0].startswith('http://testserver/openid/complete/')
+            )
+
 
 class OpenIdProviderLiveServerTest(LiveServerTestCase):
     """

common/djangoapps/external_auth/views.py

@@ -6,6 +6,7 @@ import re
 import string       # pylint: disable=W0402
 import fnmatch
 import unicodedata
+import urllib
 
 from textwrap import dedent
 from external_auth.models import ExternalAuthMap
@@ -810,7 +811,8 @@ def provider_login(request):
                 # remember request and original path
                 request.session['openid_setup'] = {
                     'request': openid_request,
-                    'url': request.get_full_path()
+                    'url': request.get_full_path(),
+                    'post_params': request.POST,
                 }
 
                 # user failed login on previous attempt
@@ -831,6 +833,20 @@ def provider_login(request):
         openid_setup = request.session['openid_setup']
         openid_request = openid_setup['request']
         openid_request_url = openid_setup['url']
+        post_params = openid_setup['post_params']
+        # We need to preserve the parameters, and the easiest way to do this is
+        # through the URL
+        url_post_params = {
+            param: post_params[param] for param in post_params if param.startswith('openid')
+        }
+
+        encoded_params = urllib.urlencode(url_post_params)
+
+        if '?' not in openid_request_url:
+            openid_request_url = openid_request_url + '?' + encoded_params
+        else:
+            openid_request_url = openid_request_url + '&' + encoded_params
+
         del request.session['openid_setup']
 
         # don't allow invalid trust roots