diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 6d653db6cc..490f49a41c 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -215,6 +215,7 @@ def preview_module_system(request, preview_id, descriptor): render_template=render_from_lms, debug=True, replace_urls=replace_urls, + user=request.user, ) diff --git a/cms/djangoapps/github_sync/tests/__init__.py b/cms/djangoapps/github_sync/tests/__init__.py index 581ac3cb25..e2b9a909a7 100644 --- a/cms/djangoapps/github_sync/tests/__init__.py +++ b/cms/djangoapps/github_sync/tests/__init__.py @@ -61,7 +61,7 @@ class GithubSyncTestCase(TestCase): self.assertIn( Location('i4x://edX/toy/chapter/Overview'), [child.location for child in self.import_course.get_children()]) - self.assertEquals(1, len(self.import_course.get_children())) + self.assertEquals(2, len(self.import_course.get_children())) @patch('github_sync.sync_with_github') def test_sync_all_with_github(self, sync_with_github): diff --git a/common/djangoapps/external_auth/views.py b/common/djangoapps/external_auth/views.py index 9e41d31c77..5cf21ca68d 100644 --- a/common/djangoapps/external_auth/views.py +++ b/common/djangoapps/external_auth/views.py @@ -4,19 +4,18 @@ import logging import random import re import string +import fnmatch 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.urlresolvers import reverse from django.http import HttpResponse, HttpResponseRedirect -from django.shortcuts import render_to_response +from django.utils.http import urlquote 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 @@ -24,100 +23,132 @@ 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 +from openid.server.trustroot import TrustRoot +from openid.store.filestore import FileOpenIDStore +from openid.extensions import ax, sreg import student.views as student_views log = logging.getLogger("mitx.external_auth") + +# ----------------------------------------------------------------------------- +# OpenID Common +# ----------------------------------------------------------------------------- + + @csrf_exempt -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)) +def default_render_failure(request, + message, + status=403, + template_name='extauth_failure.html', + exception=None): + """Render an Openid error page to the user""" + + log.debug("In openid_failure " + message) + + 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): +# ----------------------------------------------------------------------------- +# OpenID Authentication +# ----------------------------------------------------------------------------- + + +def 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)]) + choice = random.SystemRandom().choice + return ''.join([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 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 - + render_failure = (render_failure 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) - return edXauth_external_login_or_signup(request, - external_id, - "openid:%s" % settings.OPENID_SSO_SERVER_URL, - details, - details.get('email',''), - '%s %s' % (details.get('first_name',''),details.get('last_name','')) - ) - + url = getattr(settings, 'OPENID_SSO_SERVER_URL', None) + external_domain = "openid:%s" % url + fullname = '%s %s' % (details.get('first_name', ''), + details.get('last_name', '')) + + return external_login_or_signup(request, + external_id, + external_domain, + details, + 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, - retfun=None): +def external_login_or_signup(request, + external_id, + external_domain, + credentials, + email, + fullname, + retfun=None): + """Generic external auth login or signup""" + # 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 - eamap.internal_password = edXauth_generate_password() - log.debug('created eamap=%s' % eamap) + eamap.internal_password = generate_password() + log.debug('Created eamap=%s' % eamap) eamap.save() internal_user = eamap.user if internal_user is None: - log.debug('ExtAuth: no user for %s yet, doing signup' % eamap.external_email) - return edXauth_signup(request, eamap) - + log.debug('No user for %s yet, doing signup' % eamap.external_email) + return 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)) - return edXauth_signup(request, eamap) + log.warning("External Auth Login failed for %s / %s" % + (uname, eamap.internal_password)) + return signup(request, eamap) if not user.is_active: - log.warning("External Auth: user %s is not active" % (uname)) + log.warning("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 default_render_failure(request, msg) + login(request, user) request.session.set_expiry(0) student_views.try_change_enrollment(request) @@ -125,14 +156,11 @@ def edXauth_external_login_or_signup(request, external_id, external_domain, cred if retfun is None: return redirect('/') return retfun() - - -#----------------------------------------------------------------------------- -# generic external auth signup + @ensure_csrf_cookie @cache_if_anonymous -def edXauth_signup(request, eamap=None): +def signup(request, eamap=None): """ Present form to complete for signup via external authentication. Even though the user has external credentials, he/she still needs @@ -142,32 +170,39 @@ 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) + + log.debug('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: user = ss.group(1) @@ -181,40 +216,333 @@ def ssl_dn_extract_info(dn): return None return (user, email, fullname) + @csrf_exempt -def edXauth_ssl_login(request): +def 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: - pass + # try the direct apache2 SSL key + cert = request._req.subprocess_env.get(certkey, '') + except Exception: + cert = None + 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, - email=email, - fullname=fullname, - retfun = functools.partial(student_views.index, request)) + + retfun = functools.partial(student_views.index, request) + return external_login_or_signup(request, + external_id=email, + external_domain="ssl:MIT", + credentials=cert, + email=email, + fullname=fullname, + retfun=retfun) + + +# ----------------------------------------------------------------------------- +# OpenID Provider +# ----------------------------------------------------------------------------- + + +def get_xrds_url(resource, request): + """ + Return the XRDS url for a resource + """ + host = request.META['HTTP_HOST'] + + if not host.endswith('edx.org'): + return None + + location = host + '/openid/provider/' + resource + '/' + + if request.is_secure(): + return 'https://' + location + else: + return 'http://' + location + + +def add_openid_simple_registration(request, response, data): + sreg_data = {} + sreg_request = sreg.SRegRequest.fromOpenIDRequest(request) + sreg_fields = sreg_request.allRequestedFields() + + # if consumer requested simple registration fields, add them + if sreg_fields: + for field in sreg_fields: + if field == 'email' and 'email' in data: + sreg_data['email'] = data['email'] + elif field == 'fullname' and 'fullname' in data: + sreg_data['fullname'] = data['fullname'] + + # construct sreg response + sreg_response = sreg.SRegResponse.extractResponse(sreg_request, + sreg_data) + sreg_response.toMessage(response.fields) + + +def add_openid_attribute_exchange(request, response, data): + try: + ax_request = ax.FetchRequest.fromOpenIDRequest(request) + except ax.AXError: + # not using OpenID attribute exchange extension + pass + else: + ax_response = ax.FetchResponse() + + # if consumer requested attribute exchange fields, add them + if ax_request and ax_request.requested_attributes: + for type_uri in ax_request.requested_attributes.iterkeys(): + email_schema = 'http://axschema.org/contact/email' + name_schema = 'http://axschema.org/namePerson' + if type_uri == email_schema and 'email' in data: + ax_response.addValue(email_schema, data['email']) + elif type_uri == name_schema and 'fullname' in data: + ax_response.addValue(name_schema, data['fullname']) + + # construct ax response + ax_response.toMessage(response.fields) + + +def provider_respond(server, request, response, data): + """ + Respond to an OpenID request + """ + # get and add extensions + add_openid_simple_registration(request, response, data) + add_openid_attribute_exchange(request, response, data) + + # create http response from OpenID response + webresponse = server.encodeResponse(response) + http_response = HttpResponse(webresponse.body) + http_response.status_code = webresponse.code + + # add OpenID headers to response + for k, v in webresponse.headers.iteritems(): + http_response[k] = v + + return http_response + + +def validate_trust_root(openid_request): + """ + Only allow OpenID requests from valid trust roots + """ + + trusted_roots = getattr(settings, 'OPENID_PROVIDER_TRUSTED_ROOT', None) + + if not trusted_roots: + # not using trusted roots + return True + + # don't allow empty trust roots + if (not hasattr(openid_request, 'trust_root') or + not openid_request.trust_root): + log.error('no trust_root') + return False + + # ensure trust root parses cleanly (one wildcard, of form *.foo.com, etc.) + trust_root = TrustRoot.parse(openid_request.trust_root) + if not trust_root: + log.error('invalid trust_root') + return False + + # don't allow empty return tos + if (not hasattr(openid_request, 'return_to') or + not openid_request.return_to): + log.error('empty return_to') + return False + + # ensure return to is within trust root + if not trust_root.validateURL(openid_request.return_to): + log.error('invalid return_to') + return False + + # check that the root matches the ones we trust + if not any(r for r in trusted_roots if fnmatch.fnmatch(trust_root, r)): + log.error('non-trusted root') + return False + + return True + + +@csrf_exempt +def provider_login(request): + """ + OpenID login endpoint + """ + + # make and validate endpoint + endpoint = get_xrds_url('login', request) + if not endpoint: + return default_render_failure(request, "Invalid OpenID request") + + # initialize store and server + store = FileOpenIDStore('/tmp/openid_provider') + server = Server(store, endpoint) + + # handle OpenID request + querydict = dict(request.REQUEST.items()) + error = False + if 'openid.mode' in request.GET or 'openid.mode' in request.POST: + # decode request + openid_request = server.decodeRequest(querydict) + + if not openid_request: + return default_render_failure(request, "Invalid OpenID request") + + # don't allow invalid and non-trusted trust roots + if not validate_trust_root(openid_request): + 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), {}) + + # checkid_setup, so display login page + elif openid_request.mode == 'checkid_setup': + if openid_request.idSelect(): + # remember request and original path + request.session['openid_setup'] = { + 'request': openid_request, + 'url': request.get_full_path() + } + + # user failed login on previous attempt + if 'openid_error' in request.session: + error = True + del request.session['openid_error'] + + # OpenID response + else: + return provider_respond(server, openid_request, + server.handleRequest(openid_request), {}) + + # handle login + if request.method == 'POST' and 'openid_setup' in request.session: + # get OpenID request from session + openid_setup = request.session['openid_setup'] + openid_request = openid_setup['request'] + openid_request_url = openid_setup['url'] + del request.session['openid_setup'] + + # don't allow invalid trust roots + if not validate_trust_root(openid_request): + return default_render_failure(request, "Invalid OpenID trust root") + + # check if user with given email exists + email = request.POST.get('email', None) + try: + user = User.objects.get(email=email) + except User.DoesNotExist: + request.session['openid_error'] = True + msg = "OpenID login failed - Unknown user email: {0}".format(email) + log.warning(msg) + return HttpResponseRedirect(openid_request_url) + + # attempt to authenticate user + username = user.username + password = request.POST.get('password', None) + user = authenticate(username=username, password=password) + if user is None: + request.session['openid_error'] = True + msg = "OpenID login failed - password for {0} is invalid" + msg = msg.format(email) + log.warning(msg) + return HttpResponseRedirect(openid_request_url) + + # authentication succeeded, so log user in + if user is not None and user.is_active: + # remove error from session since login succeeded + if 'openid_error' in request.session: + del request.session['openid_error'] + + # fullname field comes from user profile + profile = UserProfile.objects.get(user=user) + log.info("OpenID login success - {0} ({1})".format(user.username, + user.email)) + + # redirect user to return_to location + url = endpoint + urlquote(user.username) + response = openid_request.answer(True, None, url) + + return provider_respond(server, + openid_request, + response, + { + 'fullname': profile.name, + 'email': user.email + }) + + request.session['openid_error'] = True + msg = "Login failed - Account not active for user {0}".format(username) + log.warning(msg) + return HttpResponseRedirect(openid_request_url) + + # determine consumer domain if applicable + return_to = '' + if 'openid.return_to' in request.REQUEST: + return_to = request.REQUEST['openid.return_to'] + matches = re.match(r'\w+:\/\/([\w\.-]+)', return_to) + return_to = matches.group(1) + + # display login page + response = render_to_response('provider_login.html', { + 'error': error, + 'return_to': return_to + }) + + # custom XRDS header necessary for discovery process + response['X-XRDS-Location'] = get_xrds_url('xrds', request) + return response + + +def provider_identity(request): + """ + XRDS for identity discovery + """ + + response = render_to_response('identity.xml', + {'url': get_xrds_url('login', request)}, + mimetype='text/xml') + + # custom XRDS header necessary for discovery process + response['X-XRDS-Location'] = get_xrds_url('identity', request) + return response + + +def provider_xrds(request): + """ + XRDS for endpoint discovery + """ + + response = render_to_response('xrds.xml', + {'url': get_xrds_url('login', request)}, + mimetype='text/xml') + + # custom XRDS header necessary for discovery process + response['X-XRDS-Location'] = get_xrds_url('xrds', request) + return response diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 9cde878d21..c37b9fce16 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -4,7 +4,7 @@ Models for Student Information Replication Notes In our live deployment, we intend to run in a scenario where there is a pool of -Portal servers that hold the canoncial user information and that user +Portal servers that hold the canoncial user information and that user information is replicated to slave Course server pools. Each Course has a set of servers that serves only its content and has users that are relevant only to it. @@ -61,6 +61,7 @@ from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) + class UserProfile(models.Model): """This is where we store all the user demographic fields. We have a separate table for this rather than extending the built-in Django auth_user. @@ -175,6 +176,7 @@ class PendingEmailChange(models.Model): new_email = models.CharField(blank=True, max_length=255, db_index=True) activation_key = models.CharField(('activation key'), max_length=32, unique=True, db_index=True) + class CourseEnrollment(models.Model): user = models.ForeignKey(User) course_id = models.CharField(max_length=255, db_index=True) @@ -184,6 +186,10 @@ class CourseEnrollment(models.Model): class Meta: unique_together = (('user', 'course_id'), ) + def __unicode__(self): + return "[CourseEnrollment] %s: %s (%s)" % (self.user, self.course_id, self.created) + + @receiver(post_save, sender=CourseEnrollment) def assign_default_role(sender, instance, **kwargs): if instance.user.is_staff: @@ -273,6 +279,7 @@ def add_user_to_default_group(user, group): utg.users.add(User.objects.get(username=user)) utg.save() + @receiver(post_save, sender=User) def update_user_information(sender, instance, created, **kwargs): try: @@ -283,6 +290,7 @@ def update_user_information(sender, instance, created, **kwargs): log.error(unicode(e)) log.error("update user info to discussion failed for user with id: " + str(instance.id)) + ########################## REPLICATION SIGNALS ################################# # @receiver(post_save, sender=User) def replicate_user_save(sender, **kwargs): @@ -292,6 +300,7 @@ def replicate_user_save(sender, **kwargs): for course_db_name in db_names_to_replicate_to(user_obj.id): replicate_user(user_obj, course_db_name) + # @receiver(post_save, sender=CourseEnrollment) def replicate_enrollment_save(sender, **kwargs): """This is called when a Student enrolls in a course. It has to do the @@ -317,12 +326,14 @@ def replicate_enrollment_save(sender, **kwargs): log.debug("Replicating user profile because of new enrollment") user_profile = UserProfile.objects.get(user_id=enrollment_obj.user_id) replicate_model(UserProfile.save, user_profile, enrollment_obj.user_id) - + + # @receiver(post_delete, sender=CourseEnrollment) def replicate_enrollment_delete(sender, **kwargs): enrollment_obj = kwargs['instance'] return replicate_model(CourseEnrollment.delete, enrollment_obj, enrollment_obj.user_id) - + + # @receiver(post_save, sender=UserProfile) def replicate_userprofile_save(sender, **kwargs): """We just updated the UserProfile (say an update to the name), so push that @@ -330,12 +341,13 @@ def replicate_userprofile_save(sender, **kwargs): user_profile_obj = kwargs['instance'] return replicate_model(UserProfile.save, user_profile_obj, user_profile_obj.user_id) - + ######### Replication functions ######### USER_FIELDS_TO_COPY = ["id", "username", "first_name", "last_name", "email", "password", "is_staff", "is_active", "is_superuser", "last_login", "date_joined"] + def replicate_user(portal_user, course_db_name): """Replicate a User to the correct Course DB. This is more complicated than it should be because Askbot extends the auth_user table and adds its own @@ -359,9 +371,10 @@ def replicate_user(portal_user, course_db_name): course_user.save(using=course_db_name) unmark(course_user) + def replicate_model(model_method, instance, user_id): """ - model_method is the model action that we want replicated. For instance, + model_method is the model action that we want replicated. For instance, UserProfile.save """ if not should_replicate(instance): @@ -376,8 +389,10 @@ def replicate_model(model_method, instance, user_id): model_method(instance, using=db_name) unmark(instance) + ######### Replication Helpers ######### + def is_valid_course_id(course_id): """Right now, the only database that's not a course database is 'default'. I had nicer checking in here originally -- it would scan the courses that @@ -387,26 +402,30 @@ def is_valid_course_id(course_id): """ return course_id != 'default' + def is_portal(): """Are we in the portal pool? Only Portal servers are allowed to replicate their changes. For now, only Portal servers see multiple DBs, so we use that to decide.""" return len(settings.DATABASES) > 1 + def db_names_to_replicate_to(user_id): """Return a list of DB names that this user_id is enrolled in.""" return [c.course_id for c in CourseEnrollment.objects.filter(user_id=user_id) if is_valid_course_id(c.course_id)] + def marked_handled(instance): """Have we marked this instance as being handled to avoid infinite loops caused by saving models in post_save hooks for the same models?""" return hasattr(instance, '_do_not_copy_to_course_db') and instance._do_not_copy_to_course_db + def mark_handled(instance): """You have to mark your instance with this function or else we'll go into - an infinite loop since we're putting listeners on Model saves/deletes and + an infinite loop since we're putting listeners on Model saves/deletes and the act of replication requires us to call the same model method. We create a _replicated attribute to differentiate the first save of this @@ -415,16 +434,18 @@ def mark_handled(instance): """ instance._do_not_copy_to_course_db = True + def unmark(instance): - """If we don't unmark a model after we do replication, then consecutive + """If we don't unmark a model after we do replication, then consecutive save() calls won't be properly replicated.""" instance._do_not_copy_to_course_db = False + def should_replicate(instance): """Should this instance be replicated? We need to be a Portal server and the instance has to not have been marked_handled.""" if marked_handled(instance): - # Basically, avoid an infinite loop. You should + # Basically, avoid an infinite loop. You should log.debug("{0} should not be replicated because it's been marked" .format(instance)) return False diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index ce15a2a003..cbb12e44cc 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -75,8 +75,11 @@ def index(request, extra_context={}, user=None): entry.summary = soup.getText() # The course selection work is done in courseware.courses. + domain = settings.MITX_FEATURES.get('FORCE_UNIVERSITY_DOMAIN') # normally False + if domain==False: # do explicit check, because domain=None is valid + domain = request.META.get('HTTP_HOST') universities = get_courses_by_university(None, - domain=request.META.get('HTTP_HOST')) + domain=domain) context = {'universities': universities, 'entries': entries} context.update(extra_context) return render_to_response('index.html', context) @@ -131,10 +134,14 @@ def dashboard(request): staff_access = True errored_courses = modulestore().get_errored_courses() + show_courseware_links_for = frozenset(course.id for course in courses + if has_access(request.user, course, 'load')) + context = {'courses': courses, 'message': message, 'staff_access': staff_access, - 'errored_courses': errored_courses,} + 'errored_courses': errored_courses, + 'show_courseware_links_for' : show_courseware_links_for} return render_to_response('dashboard.html', context) diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 17380bff18..066d83ed3e 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -1,6 +1,7 @@ import re import json import logging +import time from django.conf import settings from functools import wraps @@ -75,7 +76,7 @@ def grade_histogram(module_id): grades = list(cursor.fetchall()) grades.sort(key=lambda x: x[0]) # Add ORDER BY to sql query? - if len(grades) == 1 and grades[0][0] is None: + if len(grades) >= 1 and grades[0][0] is None: return [] return grades @@ -117,6 +118,14 @@ def add_histogram(get_html, module, user): data_dir = "" source_file = module.metadata.get('source_file','') # source used to generate the problem XML, eg latex or word + # useful to indicate to staff if problem has been released or not + # TODO (ichuang): use _has_access_descriptor.can_load in lms.courseware.access, instead of now>mstart comparison here + now = time.gmtime() + is_released = "unknown" + mstart = getattr(module.descriptor,'start') + if mstart is not None: + is_released = "Yes!" if (now > mstart) else "Not yet" + staff_context = {'definition': module.definition.get('data'), 'metadata': json.dumps(module.metadata, indent=4), 'location': module.location, @@ -130,7 +139,9 @@ def add_histogram(get_html, module, user): 'xqa_server' : settings.MITX_FEATURES.get('USE_XQA_SERVER','http://xqa:server@content-qa.mitx.mit.edu/xqa'), 'histogram': json.dumps(histogram), 'render_histogram': render_histogram, - 'module_content': get_html()} + 'module_content': get_html(), + 'is_released': is_released, + } return render_to_string("staff_problem_info.html", staff_context) return _get_html diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 82eb330174..f386c9fe24 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -14,6 +14,8 @@ This is used by capa_module. from __future__ import division +from datetime import datetime +import json import logging import math import numpy @@ -32,6 +34,7 @@ from correctmap import CorrectMap import eia import inputtypes from util import contextualize_text, convert_files_to_filenames +import xqueue_interface # to be replaced with auto-registering import responsetypes @@ -202,11 +205,24 @@ class LoncapaProblem(object): ''' Returns True if any part of the problem has been submitted to an external queue ''' - queued = False - for answer_id in self.correct_map: - if self.correct_map.is_queued(answer_id): - queued = True - return queued + return any(self.correct_map.is_queued(answer_id) for answer_id in self.correct_map) + + + def get_recentmost_queuetime(self): + ''' + Returns a DateTime object that represents the timestamp of the most recent queueing request, or None if not queued + ''' + if not self.is_queued(): + return None + + # Get a list of timestamps of all queueing requests, then convert it to a DateTime object + queuetime_strs = [self.correct_map.get_queuetime_str(answer_id) + for answer_id in self.correct_map + if self.correct_map.is_queued(answer_id)] + queuetimes = [datetime.strptime(qt_str, xqueue_interface.dateformat) for qt_str in queuetime_strs] + + return max(queuetimes) + def grade_answers(self, answers): ''' diff --git a/common/lib/capa/capa/correctmap.py b/common/lib/capa/capa/correctmap.py index eb6ef2d00c..52411a8e8c 100644 --- a/common/lib/capa/capa/correctmap.py +++ b/common/lib/capa/capa/correctmap.py @@ -15,7 +15,8 @@ class CorrectMap(object): - msg : string (may have HTML) giving extra message response (displayed below textline or textbox) - hint : string (may have HTML) giving optional hint (displayed below textline or textbox, above msg) - hintmode : one of (None,'on_request','always') criteria for displaying hint - - queuekey : a random integer for xqueue_callback verification + - queuestate : Dict {key:'', time:''} where key is a secret string, and time is a string dump + of a DateTime object in the format '%Y%m%d%H%M%S'. Is None when not queued Behaves as a dict. ''' @@ -31,14 +32,15 @@ class CorrectMap(object): def __iter__(self): return self.cmap.__iter__() - def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None, queuekey=None): + # See the documentation for 'set_dict' for the use of kwargs + def set(self, answer_id=None, correctness=None, npoints=None, msg='', hint='', hintmode=None, queuestate=None, **kwargs): if answer_id is not None: self.cmap[answer_id] = {'correctness': correctness, 'npoints': npoints, 'msg': msg, 'hint': hint, 'hintmode': hintmode, - 'queuekey': queuekey, + 'queuestate': queuestate, } def __repr__(self): @@ -52,25 +54,39 @@ class CorrectMap(object): def set_dict(self, correct_map): ''' - set internal dict to provided correct_map dict - for graceful migration, if correct_map is a one-level dict, then convert it to the new - dict of dicts format. + Set internal dict of CorrectMap to provided correct_map dict + + correct_map is saved by LMS as a plaintext JSON dump of the correctmap dict. This means that + when the definition of CorrectMap (e.g. its properties) are altered, existing correct_map dict + not coincide with the newest CorrectMap format as defined by self.set. + + For graceful migration, feed the contents of each correct map to self.set, rather than + making a direct copy of the given correct_map dict. This way, the common keys between + the incoming correct_map dict and the new CorrectMap instance will be written, while + mismatched keys will be gracefully ignored. + + Special migration case: + If correct_map is a one-level dict, then convert it to the new dict of dicts format. ''' if correct_map and not (type(correct_map[correct_map.keys()[0]]) == dict): - self.__init__() # empty current dict - for k in correct_map: self.set(k, correct_map[k]) # create new dict entries + self.__init__() # empty current dict + for k in correct_map: self.set(k, correct_map[k]) # create new dict entries else: - self.cmap = correct_map + self.__init__() + for k in correct_map: self.set(k, **correct_map[k]) def is_correct(self, answer_id): if answer_id in self.cmap: return self.cmap[answer_id]['correctness'] == 'correct' return None def is_queued(self, answer_id): - return answer_id in self.cmap and self.cmap[answer_id]['queuekey'] is not None + return answer_id in self.cmap and self.cmap[answer_id]['queuestate'] is not None def is_right_queuekey(self, answer_id, test_key): - return answer_id in self.cmap and self.cmap[answer_id]['queuekey'] == test_key + return self.is_queued(answer_id) and self.cmap[answer_id]['queuestate']['key'] == test_key + + def get_queuetime_str(self, answer_id): + return self.cmap[answer_id]['queuestate']['time'] def get_npoints(self, answer_id): npoints = self.get_property(answer_id, 'npoints') diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index ad54736359..a323a5dbfe 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -351,7 +351,7 @@ def filesubmission(element, value, status, render_template, msg=''): if status == 'incomplete': # Flag indicating that the problem has been queued, 'msg' is length of queue status = 'queued' queue_len = msg - msg = 'Submitted to grader. (Queue length: %s)' % queue_len + msg = 'Submitted to grader.' context = { 'id': eid, 'state': status, 'msg': msg, 'value': value, 'queue_len': queue_len, 'allowed_files': allowed_files, @@ -384,7 +384,7 @@ def textbox(element, value, status, render_template, msg=''): if status == 'incomplete': # Flag indicating that the problem has been queued, 'msg' is length of queue status = 'queued' queue_len = msg - msg = 'Submitted to grader. (Queue length: %s)' % queue_len + msg = 'Submitted to grader.' # For CodeMirror mode = element.get('mode','python') diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index b2d56b48ca..0cf0473348 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -8,6 +8,7 @@ Used by capa_problem.py ''' # standard library imports +import cgi import inspect import json import logging @@ -26,6 +27,7 @@ import xml.sax.saxutils as saxutils # specific library imports from calc import evaluator, UndefinedVariable from correctmap import CorrectMap +from datetime import datetime from util import * from lxml import etree from lxml.html.soupparser import fromstring as fromstring_bs # uses Beautiful Soup!!! FIXME? @@ -317,30 +319,37 @@ class JavascriptResponse(LoncapaResponse): def compile_display_javascript(self): - latestTimestamp = 0 - basepath = self.system.filestore.root_path + '/js/' - for filename in (self.display_dependencies + [self.display]): - filepath = basepath + filename - timestamp = os.stat(filepath).st_mtime - if timestamp > latestTimestamp: - latestTimestamp = timestamp - - h = hashlib.md5() - h.update(self.answer_id + str(self.display_dependencies)) - compiled_filename = 'compiled/' + h.hexdigest() + '.js' - compiled_filepath = basepath + compiled_filename + # TODO FIXME + # arjun: removing this behavior for now (and likely forever). Keeping + # until we decide on exactly how to solve this issue. For now, files are + # manually being compiled to DATA_DIR/js/compiled. - if not os.path.exists(compiled_filepath) or os.stat(compiled_filepath).st_mtime < latestTimestamp: - outfile = open(compiled_filepath, 'w') - for filename in (self.display_dependencies + [self.display]): - filepath = basepath + filename - infile = open(filepath, 'r') - outfile.write(infile.read()) - outfile.write(';\n') - infile.close() - outfile.close() + #latestTimestamp = 0 + #basepath = self.system.filestore.root_path + '/js/' + #for filename in (self.display_dependencies + [self.display]): + # filepath = basepath + filename + # timestamp = os.stat(filepath).st_mtime + # if timestamp > latestTimestamp: + # latestTimestamp = timestamp + # + #h = hashlib.md5() + #h.update(self.answer_id + str(self.display_dependencies)) + #compiled_filename = 'compiled/' + h.hexdigest() + '.js' + #compiled_filepath = basepath + compiled_filename - self.display_filename = compiled_filename + #if not os.path.exists(compiled_filepath) or os.stat(compiled_filepath).st_mtime < latestTimestamp: + # outfile = open(compiled_filepath, 'w') + # for filename in (self.display_dependencies + [self.display]): + # filepath = basepath + filename + # infile = open(filepath, 'r') + # outfile.write(infile.read()) + # outfile.write(';\n') + # infile.close() + # outfile.close() + + # TODO this should also be fixed when the above is fixed. + filename = self.system.ajax_url.split('/')[-1] + '.js' + self.display_filename = 'compiled/' + filename def parse_xml(self): self.generator_xml = self.xml.xpath('//*[@id=$id]//generator', @@ -384,19 +393,23 @@ class JavascriptResponse(LoncapaResponse): node_path = self.system.node_path + ":" + os.path.normpath(js_dir) tmp_env["NODE_PATH"] = node_path return tmp_env + + def call_node(self, args): + + subprocess_args = ["node"] + subprocess_args.extend(args) + + return subprocess.check_output(subprocess_args, env=self.get_node_env()) def generate_problem_state(self): generator_file = os.path.dirname(os.path.normpath(__file__)) + '/javascript_problem_generator.js' - output = subprocess.check_output(["node", - generator_file, - self.generator, - json.dumps(self.generator_dependencies), - json.dumps(str(self.system.seed)), - json.dumps(self.params) - ], - env=self.get_node_env()).strip() + output = self.call_node([generator_file, + self.generator, + json.dumps(self.generator_dependencies), + json.dumps(str(self.system.seed)), + json.dumps(self.params)]).strip() return json.loads(output) @@ -407,7 +420,8 @@ class JavascriptResponse(LoncapaResponse): for param in self.xml.xpath('//*[@id=$id]//responseparam', id=self.xml.get('id')): - params[param.get("name")] = json.loads(param.get("value")) + raw_param = param.get("value") + params[param.get("name")] = json.loads(contextualize_text(raw_param, self.context)) return params @@ -435,22 +449,23 @@ class JavascriptResponse(LoncapaResponse): (all_correct, evaluation, solution) = self.run_grader(json_submission) self.solution = solution correctness = 'correct' if all_correct else 'incorrect' - return CorrectMap(self.answer_id, correctness, msg=evaluation) + if all_correct: + points = self.get_max_score() + else: + points = 0 + return CorrectMap(self.answer_id, correctness, npoints=points, msg=evaluation) def run_grader(self, submission): if submission is None or submission == '': submission = json.dumps(None) grader_file = os.path.dirname(os.path.normpath(__file__)) + '/javascript_problem_grader.js' - outputs = subprocess.check_output(["node", - grader_file, - self.grader, - json.dumps(self.grader_dependencies), - submission, - json.dumps(self.problem_state), - json.dumps(self.params) - ], - env=self.get_node_env()).split('\n') + outputs = self.call_node([grader_file, + self.grader, + json.dumps(self.grader_dependencies), + submission, + json.dumps(self.problem_state), + json.dumps(self.params)]).split('\n') all_correct = json.loads(outputs[0].strip()) evaluation = outputs[1].strip() @@ -711,7 +726,8 @@ class NumericalResponse(LoncapaResponse): # I think this is just pyparsing.ParseException, calc.UndefinedVariable: # But we'd need to confirm except: - raise StudentInputError('Invalid input -- please use a number only') + raise StudentInputError("Invalid input: could not interpret '%s' as a number" %\ + cgi.escape(student_answer)) if correct: return CorrectMap(self.answer_id, 'correct') @@ -900,10 +916,12 @@ def sympy_check2(): try: exec self.code in self.context['global_context'], self.context correct = self.context['correct'] + messages = self.context['messages'] except Exception as err: print "oops in customresponse (code) error %s" % err print "context = ", self.context print traceback.format_exc() + raise StudentInputError("Error: Problem could not be evaluated with your input") # Notify student else: # self.code is not a string; assume its a function # this is an interface to the Tutor2 check functions @@ -953,7 +971,8 @@ def sympy_check2(): # build map giving "correct"ness of the answer(s) correct_map = CorrectMap() for k in range(len(idset)): - correct_map.set(idset[k], correct[k], msg=messages[k]) + correct_map.set(idset[k], correct[k], msg=messages[k], + npoints=self.maxpoints[idset[k]]) return correct_map def get_answers(self): @@ -1005,7 +1024,7 @@ class CodeResponse(LoncapaResponse): ''' Grade student code using an external queueing server, called 'xqueue' - Expects 'xqueue' dict in ModuleSystem with the following keys: + Expects 'xqueue' dict in ModuleSystem with the following keys that are needed by CodeResponse: system.xqueue = { 'interface': XqueueInterface object, 'callback_url': Per-StudentModule callback URL where results are posted (string), 'default_queuename': Default queuename to submit request (string) @@ -1026,7 +1045,7 @@ class CodeResponse(LoncapaResponse): TODO: Determines whether in synchronous or asynchronous (queued) mode ''' xml = self.xml - self.url = xml.get('url', None) # XML can override external resource (grader/queue) URL + self.url = xml.get('url', None) # TODO: XML can override external resource (grader/queue) URL self.queue_name = xml.get('queuename', self.system.xqueue['default_queuename']) # VS[compat]: @@ -1121,22 +1140,34 @@ class CodeResponse(LoncapaResponse): # Prepare xqueue request #------------------------------------------------------------ + qinterface = self.system.xqueue['interface'] + qtime = datetime.strftime(datetime.now(), xqueue_interface.dateformat) + + anonymous_student_id = self.system.anonymous_student_id # Generate header - queuekey = xqueue_interface.make_hashkey(str(self.system.seed)+self.answer_id) + queuekey = xqueue_interface.make_hashkey(str(self.system.seed) + qtime + + anonymous_student_id + + self.answer_id) xheader = xqueue_interface.make_xheader(lms_callback_url=self.system.xqueue['callback_url'], lms_key=queuekey, queue_name=self.queue_name) - + # Generate body if is_list_of_files(submission): - self.context.update({'submission': queuekey}) # For tracking. TODO: May want to record something else here + self.context.update({'submission': ''}) # TODO: Get S3 pointer from the Queue else: self.context.update({'submission': submission}) contents = self.payload.copy() + # Metadata related to the student submission revealed to the external grader + student_info = {'anonymous_student_id': anonymous_student_id, + 'submission_time': qtime, + } + contents.update({'student_info': json.dumps(student_info)}) + # Submit request. When successful, 'msg' is the prior length of the queue if is_list_of_files(submission): contents.update({'student_response': ''}) # TODO: Is there any information we want to send here? @@ -1148,16 +1179,21 @@ class CodeResponse(LoncapaResponse): (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) + # State associated with the queueing request + queuestate = {'key': queuekey, + 'time': qtime, + } + cmap = CorrectMap() if error: - cmap.set(self.answer_id, queuekey=None, + cmap.set(self.answer_id, queuestate=None, msg='Unable to deliver your submission to grader. (Reason: %s.) Please try again later.' % msg) else: # Queueing mechanism flags: - # 1) Backend: Non-null CorrectMap['queuekey'] indicates that the problem has been queued + # 1) Backend: Non-null CorrectMap['queuestate'] indicates that the problem has been queued # 2) Frontend: correctness='incomplete' eventually trickles down through inputtypes.textbox # and .filesubmission to inform the browser to poll the LMS - cmap.set(self.answer_id, queuekey=queuekey, correctness='incomplete', msg=msg) + cmap.set(self.answer_id, queuestate=queuestate, correctness='incomplete', msg=msg) return cmap @@ -1165,7 +1201,7 @@ class CodeResponse(LoncapaResponse): (valid_score_msg, correct, points, msg) = self._parse_score_msg(score_msg) if not valid_score_msg: - oldcmap.set(self.answer_id, msg='Error: Invalid grader reply.') + oldcmap.set(self.answer_id, msg='Invalid grader reply. Please contact the course staff.') return oldcmap correctness = 'correct' if correct else 'incorrect' @@ -1180,7 +1216,7 @@ class CodeResponse(LoncapaResponse): points = 0 elif points > self.maxpoints[self.answer_id]: points = self.maxpoints[self.answer_id] - oldcmap.set(self.answer_id, npoints=points, correctness=correctness, msg=msg.replace(' ', ' '), queuekey=None) # Queuekey is consumed + oldcmap.set(self.answer_id, npoints=points, correctness=correctness, msg=msg.replace(' ', ' '), queuestate=None) # Queuestate is consumed else: log.debug('CodeResponse: queuekey %s does not match for answer_id=%s.' % (queuekey, self.answer_id)) @@ -1197,26 +1233,41 @@ class CodeResponse(LoncapaResponse): ''' Grader reply is a JSON-dump of the following dict { 'correct': True/False, - 'score': # TODO -- Partial grading + 'score': Numeric value (floating point is okay) to assign to answer 'msg': grader_msg } Returns (valid_score_msg, correct, score, msg): valid_score_msg: Flag indicating valid score_msg format (Boolean) correct: Correctness of submission (Boolean) - score: # TODO: Implement partial grading + score: Points to be assigned (numeric, can be float) msg: Message from grader to display to student (string) ''' - fail = (False, False, -1, '') + fail = (False, False, 0, '') try: score_result = json.loads(score_msg) except (TypeError, ValueError): + log.error("External grader message should be a JSON-serialized dict. Received score_msg = %s" % score_msg) return fail if not isinstance(score_result, dict): + log.error("External grader message should be a JSON-serialized dict. Received score_result = %s" % score_result) return fail for tag in ['correct', 'score', 'msg']: - if not score_result.has_key(tag): + if tag not in score_result: + log.error("External grader message is missing one or more required tags: 'correct', 'score', 'msg'") return fail - return (True, score_result['correct'], score_result['score'], score_result['msg']) + + # Next, we need to check that the contents of the external grader message + # is safe for the LMS. + # 1) Make sure that the message is valid XML (proper opening/closing tags) + # 2) TODO: Is the message actually HTML? + msg = score_result['msg'] + try: + etree.fromstring(msg) + except etree.XMLSyntaxError as err: + log.error("Unable to parse external grader message as valid XML: score_msg['msg']=%s" % msg) + return fail + + return (True, score_result['correct'], score_result['score'], msg) #----------------------------------------------------------------------------- @@ -1471,11 +1522,12 @@ class FormulaResponse(LoncapaResponse): cs=self.case_sensitive) except UndefinedVariable as uv: log.debug('formularesponse: undefined variable in given=%s' % given) - raise StudentInputError(uv.message + " not permitted in answer") + raise StudentInputError("Invalid input: " + uv.message + " not permitted in answer") except Exception as err: #traceback.print_exc() log.debug('formularesponse: error %s in formula' % err) - raise StudentInputError("Error in formula") + raise StudentInputError("Invalid input: Could not parse '%s' as a formula" %\ + cgi.escape(given)) if numpy.isnan(student_result) or numpy.isinf(student_result): return "incorrect" if not compare_with_tolerance(student_result, instructor_result, self.tolerance): diff --git a/common/lib/capa/capa/templates/choicegroup.html b/common/lib/capa/capa/templates/choicegroup.html index 2bed2d875e..cb11350a63 100644 --- a/common/lib/capa/capa/templates/choicegroup.html +++ b/common/lib/capa/capa/templates/choicegroup.html @@ -1,4 +1,4 @@ -