[34078525] OpenID provider cleanup and minor fixes

common/djangoapps/external_auth/views.py

@@ -9,17 +9,12 @@ from external_auth.models import ExternalAuthMap
 
 from django.conf import settings
 from django.contrib.auth import REDIRECT_FIELD_NAME, authenticate, login
-from django.contrib.auth.models import Group
 from django.contrib.auth.models import User
 from student.models import UserProfile
 
-from django.core.context_processors import csrf
-from django.core.urlresolvers import reverse
 from django.http import HttpResponse, HttpResponseRedirect
 from django.utils.http import urlquote
-from django.shortcuts import render_to_response
 from django.shortcuts import redirect
-from django.template import RequestContext
 from mitxmako.shortcuts import render_to_response, render_to_string
 try:
     from django.views.decorators.csrf import csrf_exempt
@@ -27,84 +22,103 @@ except ImportError:
     from django.contrib.csrf.middleware import csrf_exempt
 from django_future.csrf import ensure_csrf_cookie
 from util.cache import cache_if_anonymous
-    
-from django_openid_auth import auth as openid_auth
-from openid.consumer.consumer import (Consumer, SUCCESS, CANCEL, FAILURE)
+
 import django_openid_auth.views as openid_views
+from django_openid_auth import auth as openid_auth
+from openid.consumer.consumer import SUCCESS
 
-from openid.server.server import Server, ProtocolError, CheckIDRequest, EncodingError
+from openid.server.server import Server
 from openid.server.trustroot import TrustRoot
 from openid.store.filestore import FileOpenIDStore
-from openid.yadis.discover import DiscoveryFailure
-from openid.consumer.discover import OPENID_IDP_2_0_TYPE
 from openid.extensions import ax, sreg
-from openid.fetchers import HTTPFetchingError
 
 import student.views as student_views
 
 log = logging.getLogger("mitx.external_auth")
 
+
 @csrf_exempt
-def default_render_failure(request, message, status=403, template_name='extauth_failure.html', exception=None):
+def default_render_failure(request,
+                           message,
+                           status=403,
+                           template_name='extauth_failure.html',
+                           exception=None):
     """Render an Openid error page to the user."""
     message = "In openid_failure " + message
     log.debug(message)
-    data = render_to_string( template_name, dict(message=message, exception=exception))
+    data = render_to_string(template_name,
+                            dict(message=message, exception=exception))
     return HttpResponse(data, status=status)
 
 #-----------------------------------------------------------------------------
 # Openid
 
+
 def edXauth_generate_password(length=12, chars=string.letters + string.digits):
     """Generate internal password for externally authenticated user"""
     return ''.join([random.choice(chars) for i in range(length)])
 
+
 @csrf_exempt
-def edXauth_openid_login_complete(request,  redirect_field_name=REDIRECT_FIELD_NAME, render_failure=None):
+def edXauth_openid_login_complete(request,
+                                  redirect_field_name=REDIRECT_FIELD_NAME,
+                                  render_failure=None):
     """Complete the openid login process"""
 
-    redirect_to = request.REQUEST.get(redirect_field_name, '')
-    render_failure = render_failure or \
-                     getattr(settings, 'OPENID_RENDER_FAILURE', None) or \
-                     default_render_failure
-                                                   
+    redirect_to = request.REQUEST.get(redirect_field_name, '')  # TODO: [rocha] redirect_to never used?
+
+    render_failure = (render_failure or
+                      getattr(settings, 'OPENID_RENDER_FAILURE', None) or
+                      default_render_failure)
+
     openid_response = openid_views.parse_openid_response(request)
     if not openid_response:
-        return render_failure(request, 'This is an OpenID relying party endpoint.')
+        return render_failure(request,
+                              'This is an OpenID relying party endpoint.')
 
     if openid_response.status == SUCCESS:
         external_id = openid_response.identity_url
-        oid_backend =  openid_auth.OpenIDBackend()
+        oid_backend = openid_auth.OpenIDBackend()
         details = oid_backend._extract_user_details(openid_response)
 
         log.debug('openid success, details=%s' % details)
 
+        external_domain = "openid:%s" % settings.OPENID_SSO_SERVER_URL
+        fullname = '%s %s' % (details.get('first_name', ''),
+                              details.get('last_name', ''))
+
         return edXauth_external_login_or_signup(request,
-                                                external_id, 
-                                                "openid:%s" % settings.OPENID_SSO_SERVER_URL,
+                                                external_id,
+                                                external_domain,
                                                 details,
-                                                details.get('email',''),
-                                                '%s %s' % (details.get('first_name',''),details.get('last_name',''))
+                                                details.get('email', ''),
+                                                fullname,
                                                 )
-                                 
+
     return render_failure(request, 'Openid failure')
 
+
 #-----------------------------------------------------------------------------
 # generic external auth login or signup
 
-def edXauth_external_login_or_signup(request, external_id, external_domain, credentials, email, fullname,
+def edXauth_external_login_or_signup(request,
+                                     external_id,
+                                     external_domain,
+                                     credentials,
+                                     email,
+                                     fullname,
                                      retfun=None):
     # see if we have a map from this external_id to an edX username
     try:
-        eamap = ExternalAuthMap.objects.get(external_id = external_id,
-                                            external_domain = external_domain,
+        eamap = ExternalAuthMap.objects.get(external_id=external_id,
+                                            external_domain=external_domain,
                                             )
         log.debug('Found eamap=%s' % eamap)
     except ExternalAuthMap.DoesNotExist:
         # go render form for creating edX user
-        eamap = ExternalAuthMap(external_id = external_id,
-                                external_domain = external_domain,
-                                external_credentials = json.dumps(credentials),
+        eamap = ExternalAuthMap(external_id=external_id,
+                                external_domain=external_domain,
+                                external_credentials=json.dumps(credentials),
                                 )
         eamap.external_email = email
         eamap.external_name = fullname
@@ -115,20 +129,23 @@ def edXauth_external_login_or_signup(request, external_id, external_domain, cred
 
     internal_user = eamap.user
     if internal_user is None:
-        log.debug('ExtAuth: no user for %s yet, doing signup' % eamap.external_email)
+        log.debug('ExtAuth: no user for %s yet, doing signup' %
+                  eamap.external_email)
         return edXauth_signup(request, eamap)
-    
+
     uname = internal_user.username
     user = authenticate(username=uname, password=eamap.internal_password)
     if user is None:
-        log.warning("External Auth Login failed for %s / %s" % (uname,eamap.internal_password))
+        log.warning("External Auth Login failed for %s / %s" %
+                    (uname, eamap.internal_password))
         return edXauth_signup(request, eamap)
 
     if not user.is_active:
         log.warning("External Auth: user %s is not active" % (uname))
         # TODO: improve error page
-        return render_failure(request, 'Account not yet activated: please look for link in your email')
-     
+        msg = 'Account not yet activated: please look for link in your email'
+        return render_failure(request, msg)  # TODO: [rocha] render_failure not defined?
+
     login(request, user)
     request.session.set_expiry(0)
     student_views.try_change_enrollment(request)
@@ -136,8 +153,8 @@ def edXauth_external_login_or_signup(request, external_id, external_domain, cred
     if retfun is None:
         return redirect('/')
     return retfun()
-        
-    
+
+
 #-----------------------------------------------------------------------------
 # generic external auth signup
 
@@ -153,31 +170,36 @@ def edXauth_signup(request, eamap=None):
     eamap is an ExteralAuthMap object, specifying the external user
     for which to complete the signup.
     """
-    
+
     if eamap is None:
         pass
 
-    request.session['ExternalAuthMap'] = eamap	# save this for use by student.views.create_account
-    
+    # save this for use by student.views.create_account
+    request.session['ExternalAuthMap'] = eamap
+
+    # default conjoin name, no spaces
+    username = eamap.external_name.replace(' ', '')
+
     context = {'has_extauth_info': True,
-               'show_signup_immediately' : True,
+               'show_signup_immediately': True,
                'extauth_email': eamap.external_email,
-               'extauth_username' : eamap.external_name.replace(' ',''), # default - conjoin name, no spaces
+               'extauth_username': username,
                'extauth_name': eamap.external_name,
                }
-    
+
     log.debug('ExtAuth: doing signup for %s' % eamap.external_email)
 
     return student_views.index(request, extra_context=context)
 
+
 #-----------------------------------------------------------------------------
 # MIT SSL
 
 def ssl_dn_extract_info(dn):
     '''
-    Extract username, email address (may be anyuser@anydomain.com) and full name
-    from the SSL DN string.  Return (user,email,fullname) if successful, and None
-    otherwise.
+    Extract username, email address (may be anyuser@anydomain.com) and
+    full name from the SSL DN string.  Return (user,email,fullname) if
+    successful, and None otherwise.
     '''
     ss = re.search('/emailAddress=(.*)@([^/]+)', dn)
     if ss:
@@ -192,43 +214,50 @@ def ssl_dn_extract_info(dn):
         return None
     return (user, email, fullname)
 
+
 @csrf_exempt
 def edXauth_ssl_login(request):
     """
-    This is called by student.views.index when MITX_FEATURES['AUTH_USE_MIT_CERTIFICATES'] = True
+    This is called by student.views.index when
+    MITX_FEATURES['AUTH_USE_MIT_CERTIFICATES'] = True
 
-    Used for MIT user authentication.  This presumes the web server (nginx) has been configured 
-    to require specific client certificates.
+    Used for MIT user authentication.  This presumes the web server
+    (nginx) has been configured to require specific client
+    certificates.
 
-    If the incoming protocol is HTTPS (SSL) then authenticate via client certificate.  
-    The certificate provides user email and fullname; this populates the ExternalAuthMap.
-    The user is nevertheless still asked to complete the edX signup.  
+    If the incoming protocol is HTTPS (SSL) then authenticate via
+    client certificate.  The certificate provides user email and
+    fullname; this populates the ExternalAuthMap.  The user is
+    nevertheless still asked to complete the edX signup.
 
     Else continues on with student.views.index, and no authentication.
     """
-    certkey = "SSL_CLIENT_S_DN"	 # specify the request.META field to use
-    
-    cert = request.META.get(certkey,'')
+    certkey = "SSL_CLIENT_S_DN"  # specify the request.META field to use
+
+    cert = request.META.get(certkey, '')
     if not cert:
-        cert = request.META.get('HTTP_'+certkey,'')
+        cert = request.META.get('HTTP_' + certkey, '')
     if not cert:
         try:
-            cert = request._req.subprocess_env.get(certkey,'')	 # try the direct apache2 SSL key
-        except Exception as err:
+            # try the direct apache2 SSL key
+            cert = request._req.subprocess_env.get(certkey, '')
+        except Exception:
             pass
     if not cert:
         # no certificate information - go onward to main index
         return student_views.index(request)
 
     (user, email, fullname) = ssl_dn_extract_info(cert)
-    
-    return edXauth_external_login_or_signup(request, 
-                                            external_id=email, 
-                                            external_domain="ssl:MIT", 
-                                            credentials=cert, 
+
+    retfun = functools.partial(student_views.index, request)
+    return edXauth_external_login_or_signup(request,
+                                            external_id=email,
+                                            external_domain="ssl:MIT",
+                                            credentials=cert,
                                             email=email,
                                             fullname=fullname,
-                                            retfun = functools.partial(student_views.index, request))
+                                            retfun=retfun)
+
 
 def get_dict_for_openid(data):
     """
@@ -237,6 +266,7 @@ def get_dict_for_openid(data):
 
     return dict((k, v) for k, v in data.iteritems())
 
+
 def get_xrds_url(resource, request):
     """
     Return the XRDS url for a resource
@@ -250,12 +280,13 @@ def get_xrds_url(resource, request):
 
     return url
 
+
 def provider_respond(server, request, response, data):
     """
     Respond to an OpenID request
     """
 
-    # get simple registration request 
+    # get simple registration request
     sreg_data = {}
     sreg_request = sreg.SRegRequest.fromOpenIDRequest(request)
     sreg_fields = sreg_request.allRequestedFields()
@@ -269,7 +300,8 @@ def provider_respond(server, request, response, data):
                 sreg_data['fullname'] = data['fullname']
 
         # construct sreg response
-        sreg_response = sreg.SRegResponse.extractResponse(sreg_request, sreg_data)
+        sreg_response = sreg.SRegResponse.extractResponse(sreg_request,
+                                                          sreg_data)
         sreg_response.toMessage(response.fields)
 
     # get attribute exchange request
@@ -287,9 +319,8 @@ def provider_respond(server, request, response, data):
             for type_uri in ax_request.requested_attributes.iterkeys():
                 if type_uri == 'http://axschema.org/contact/email' and 'email' in data:
                     ax_response.addValue('http://axschema.org/contact/email', data['email'])
-
                 elif type_uri == 'http://axschema.org/namePerson' and 'fullname' in data:
-                    ax_response.addValue('http://axschema.org/namePerson', data['fullname']);
+                    ax_response.addValue('http://axschema.org/namePerson', data['fullname'])
 
             # construct ax response
             ax_response.toMessage(response.fields)
@@ -313,24 +344,24 @@ def validate_trust_root(openid_request):
 
     # verify the trust root/return to
     trust_root = openid_request.trust_root
-    return_to = openid_request.return_to
+    return_to = openid_request.return_to  # TODO: [rocha] never used?
 
     # don't allow empty trust roots
     if openid_request.trust_root is None:
-        return false 
+        return False
 
     # ensure trust root parses cleanly (one wildcard, of form *.foo.com, etc.)
-    trust_root = TrustRoot.parse(openid_request.trust_root) 
+    trust_root = TrustRoot.parse(openid_request.trust_root)
     if trust_root is None:
-        return false
+        return False
 
     # don't allow empty return tos
     if openid_request.return_to is None:
-        return false
+        return False
 
     # ensure return to is within trust root
     if not trust_root.validateURL(openid_request.return_to):
-        return false
+        return False
 
     # only allow *.cs50.net for now
     return trust_root.host.endswith('cs50.net')
@@ -356,11 +387,12 @@ def provider_login(request):
 
         # don't allow invalid and non-*.cs50.net trust roots
         if not validate_trust_root(openid_request):
-            return default_render_failure(request, "Invalid OpenID trust root") 
+            return default_render_failure(request, "Invalid OpenID trust root")
 
         # checkid_immediate not supported, require user interaction
         if openid_request.mode == 'checkid_immediate':
-            return provider_respond(server, openid_request, openid_request.answer(false), {})
+            return provider_respond(server, openid_request,
+                                    openid_request.answer(False), {})
 
         # checkid_setup, so display login page
         elif openid_request.mode == 'checkid_setup':
@@ -378,7 +410,8 @@ def provider_login(request):
 
         # OpenID response
         else:
-            return provider_respond(server, openid_request, server.handleRequest(openid_request), {})
+            return provider_respond(server, openid_request,
+                                    server.handleRequest(openid_request), {})
 
     # handle login
     if request.method == 'POST' and 'openid_request' in request.session:
@@ -388,7 +421,7 @@ def provider_login(request):
 
         # don't allow invalid and non-*.cs50.net trust roots
         if not validate_trust_root(openid_request):
-            return default_render_failure(request, "Invalid OpenID trust root") 
+            return default_render_failure(request, "Invalid OpenID trust root")
 
         # check if user with given email exists
         email = request.POST['email']
@@ -397,7 +430,8 @@ def provider_login(request):
             user = User.objects.get(email=email)
         except User.DoesNotExist:
             request.session['openid_error'] = True
-            log.warning("OpenID login failed - Unknown user email: {0}".format(email))
+            msg = "OpenID login failed - Unknown user email: {0}".format(email)
+            log.warning(msg)
             return HttpResponseRedirect(openid_request['url'])
 
         # attempt to authenticate user
@@ -405,7 +439,8 @@ def provider_login(request):
         user = authenticate(username=username, password=password)
         if user is None:
             request.session['openid_error'] = True
-            log.warning("OpenID login failed - password for {0} is invalid".format(email))
+            msg = "OpenID login failed - password for {0} is invalid".format(email)
+            log.warning(msg)
             return HttpResponseRedirect(openid_request['url'])
 
         # authentication succeeded, so log user in
@@ -416,14 +451,19 @@ def provider_login(request):
 
             # fullname field comes from user profile
             profile = UserProfile.objects.get(user=user)
-            log.info("OpenID login success - {0} ({1})".format(user.username, user.email))
+            log.info("OpenID login success - {0} ({1})".format(user.username,
+                                                               user.email))
 
             # redirect user to return_to location
             response = openid_request['request'].answer(True, None, endpoint + urlquote(user.username))
-            return provider_respond(server, openid_request['request'], response, {
-                'fullname': profile.name,
-                'email': user.email
-            })
+
+            return provider_respond(server,
+                                    openid_request['request'],
+                                    response,
+                                    {
+                                        'fullname': profile.name,
+                                        'email': user.email
+                                    })
 
         request.session['openid_error'] = True
         log.warning("Login failed - Account not active for user {0}".format(username))
@@ -445,6 +485,7 @@ def provider_login(request):
     response['X-XRDS-Location'] = get_xrds_url('xrds', request)
     return response
 
+
 def provider_identity(request):
     """
     XRDS for identity discovery
@@ -458,6 +499,7 @@ def provider_identity(request):
     response['X-XRDS-Location'] = get_xrds_url('identity', request)
     return response
 
+
 def provider_xrds(request):
     """
     XRDS for endpoint discovery
@@ -470,4 +512,3 @@ def provider_xrds(request):
     # custom XRDS header necessary for discovery process
     response['X-XRDS-Location'] = get_xrds_url('xrds', request)
     return response
-