Merge branch 'release' into release-2015-08-26-conflict

common/djangoapps/third_party_auth/admin.py

@@ -9,7 +9,17 @@ from config_models.admin import ConfigurationModelAdmin, KeyedConfigurationModel
 from .models import OAuth2ProviderConfig, SAMLProviderConfig, SAMLConfiguration, SAMLProviderData
 from .tasks import fetch_saml_metadata
 
-admin.site.register(OAuth2ProviderConfig, KeyedConfigurationModelAdmin)
+
+class OAuth2ProviderConfigAdmin(KeyedConfigurationModelAdmin):
+    """ Django Admin class for OAuth2ProviderConfig """
+    def get_list_display(self, request):
+        """ Don't show every single field in the admin change list """
+        return (
+            'name', 'enabled', 'backend_name', 'secondary', 'skip_registration_form',
+            'skip_email_verification', 'change_date', 'changed_by', 'edit_link',
+        )
+
+admin.site.register(OAuth2ProviderConfig, OAuth2ProviderConfigAdmin)
 
 
 class SAMLProviderConfigAdmin(KeyedConfigurationModelAdmin):
@@ -55,10 +65,12 @@ class SAMLConfigurationAdmin(ConfigurationModelAdmin):
 
     def key_summary(self, inst):
         """ Short summary of the key pairs configured """
-        if not inst.public_key or not inst.private_key:
+        public_key = inst.get_setting('SP_PUBLIC_CERT')
+        private_key = inst.get_setting('SP_PRIVATE_KEY')
+        if not public_key or not private_key:
             return u'<em>Key pair incomplete/missing</em>'
-        pub1, pub2 = inst.public_key[0:10], inst.public_key[-10:]
-        priv1, priv2 = inst.private_key[0:10], inst.private_key[-10:]
+        pub1, pub2 = public_key[0:10], public_key[-10:]
+        priv1, priv2 = private_key[0:10], private_key[-10:]
         return u'Public: {}…{}<br>Private: {}…{}'.format(pub1, pub2, priv1, priv2)
     key_summary.allow_tags = True
 

common/djangoapps/third_party_auth/models.py

@@ -178,7 +178,16 @@ class OAuth2ProviderConfig(ProviderConfig):
         )
     )
     key = models.TextField(blank=True, verbose_name="Client ID")
-    secret = models.TextField(blank=True, verbose_name="Client Secret")
+    secret = models.TextField(
+        blank=True,
+        verbose_name="Client Secret",
+        help_text=(
+            'For increased security, you can avoid storing this in your database by leaving '
+            ' this field blank and setting '
+            'SOCIAL_AUTH_OAUTH_SECRETS = {"(backend name)": "secret", ...} '
+            'in your instance\'s Django settings (or lms.auth.json)'
+        )
+    )
     other_settings = models.TextField(blank=True, help_text="Optional JSON object with advanced settings, if any.")
 
     class Meta(object):  # pylint: disable=missing-docstring
@@ -192,8 +201,13 @@ class OAuth2ProviderConfig(ProviderConfig):
 
     def get_setting(self, name):
         """ Get the value of a setting, or raise KeyError """
-        if name in ("KEY", "SECRET"):
-            return getattr(self, name.lower())
+        if name == "KEY":
+            return self.key
+        if name == "SECRET":
+            if self.secret:
+                return self.secret
+            # To allow instances to avoid storing secrets in the DB, the secret can also be set via Django:
+            return getattr(settings, 'SOCIAL_AUTH_OAUTH_SECRETS', {}).get(self.backend_name, '')
         if self.other_settings:
             other_settings = json.loads(self.other_settings)
             assert isinstance(other_settings, dict), "other_settings should be a JSON object (dictionary)"
@@ -310,10 +324,22 @@ class SAMLConfiguration(ConfigurationModel):
         help_text=(
             'To generate a key pair as two files, run '
             '"openssl req -new -x509 -days 3652 -nodes -out saml.crt -keyout saml.key". '
-            'Paste the contents of saml.key here.'
-        )
+            'Paste the contents of saml.key here. '
+            'For increased security, you can avoid storing this in your database by leaving '
+            'this field blank and setting it via the SOCIAL_AUTH_SAML_SP_PRIVATE_KEY setting '
+            'in your instance\'s Django settings (or lms.auth.json).'
+        ),
+        blank=True,
+    )
+    public_key = models.TextField(
+        help_text=(
+            'Public key certificate. '
+            'For increased security, you can avoid storing this in your database by leaving '
+            'this field blank and setting it via the SOCIAL_AUTH_SAML_SP_PUBLIC_CERT setting '
+            'in your instance\'s Django settings (or lms.auth.json).'
+        ),
+        blank=True,
     )
-    public_key = models.TextField(help_text="Public key certificate.")
     entity_id = models.CharField(max_length=255, default="http://saml.example.com", verbose_name="Entity ID")
     org_info_str = models.TextField(
         verbose_name="Organization Info",
@@ -360,9 +386,15 @@ class SAMLConfiguration(ConfigurationModel):
         if name == "SP_ENTITY_ID":
             return self.entity_id
         if name == "SP_PUBLIC_CERT":
-            return self.public_key
+            if self.public_key:
+                return self.public_key
+            # To allow instances to avoid storing keys in the DB, the key pair can also be set via Django:
+            return getattr(settings, 'SOCIAL_AUTH_SAML_SP_PUBLIC_CERT', '')
         if name == "SP_PRIVATE_KEY":
-            return self.private_key
+            if self.private_key:
+                return self.private_key
+            # To allow instances to avoid storing keys in the DB, the private key can also be set via Django:
+            return getattr(settings, 'SOCIAL_AUTH_SAML_SP_PRIVATE_KEY', '')
         other_config = json.loads(self.other_config_str)
         if name in ("TECHNICAL_CONTACT", "SUPPORT_CONTACT"):
             contact = {

common/djangoapps/third_party_auth/tests/test_provider.py

@@ -23,7 +23,7 @@ class RegistryTest(testutil.TestCase):
         enabled_providers = provider.Registry.enabled()
         self.assertEqual(len(enabled_providers), 1)
         self.assertEqual(enabled_providers[0].name, "Google")
-        self.assertEqual(enabled_providers[0].secret, "opensesame")
+        self.assertEqual(enabled_providers[0].get_setting("SECRET"), "opensesame")
 
         self.configure_google_provider(enabled=False)
         enabled_providers = provider.Registry.enabled()
@@ -32,7 +32,17 @@ class RegistryTest(testutil.TestCase):
         self.configure_google_provider(enabled=True, secret="alohomora")
         enabled_providers = provider.Registry.enabled()
         self.assertEqual(len(enabled_providers), 1)
-        self.assertEqual(enabled_providers[0].secret, "alohomora")
+        self.assertEqual(enabled_providers[0].get_setting("SECRET"), "alohomora")
+
+    def test_secure_configuration(self):
+        """ Test that some sensitive values can be configured via Django settings """
+        self.configure_google_provider(enabled=True, secret="")
+        enabled_providers = provider.Registry.enabled()
+        self.assertEqual(len(enabled_providers), 1)
+        self.assertEqual(enabled_providers[0].name, "Google")
+        self.assertEqual(enabled_providers[0].get_setting("SECRET"), "")
+        with self.settings(SOCIAL_AUTH_OAUTH_SECRETS={'google-oauth2': 'secret42'}):
+            self.assertEqual(enabled_providers[0].get_setting("SECRET"), "secret42")
 
     def test_cannot_load_arbitrary_backends(self):
         """ Test that only backend_names listed in settings.AUTHENTICATION_BACKENDS can be used """

common/djangoapps/third_party_auth/tests/test_views.py

@@ -4,6 +4,7 @@ Test the views served by third_party_auth.
 # pylint: disable=no-member
 import ddt
 from lxml import etree
+from onelogin.saml2.errors import OneLogin_Saml2_Error
 import unittest
 from .testutil import AUTH_FEATURE_ENABLED, SAMLTestCase
 
@@ -26,8 +27,7 @@ class SAMLMetadataTest(SAMLTestCase):
         response = self.client.get(self.METADATA_URL)
         self.assertEqual(response.status_code, 404)
 
-    @ddt.data('saml_key', 'saml_key_alt')  # Test two slightly different key pair export formats
-    def test_metadata(self, key_name):
+    def test_metadata(self):
         self.enable_saml()
         doc = self._fetch_metadata()
         # Check the ACS URL:
@@ -62,13 +62,44 @@ class SAMLMetadataTest(SAMLTestCase):
             support_email="joe@example.com"
         )
 
-    def test_signed_metadata(self):
+    @ddt.data(
+        # Test two slightly different key pair export formats
+        ('saml_key', 'MIICsDCCAhmgAw'),
+        ('saml_key_alt', 'MIICWDCCAcGgAw'),
+    )
+    @ddt.unpack
+    def test_signed_metadata(self, key_name, pub_key_starts_with):
         self.enable_saml(
+            private_key=self._get_private_key(key_name),
+            public_key=self._get_public_key(key_name),
             other_config_str='{"SECURITY_CONFIG": {"signMetadata": true} }',
         )
+        self._validate_signed_metadata(pub_key_starts_with=pub_key_starts_with)
+
+    def test_secure_key_configuration(self):
+        """ Test that the SAML private key can be stored in Django settings and not the DB """
+        self.enable_saml(
+            public_key='',
+            private_key='',
+            other_config_str='{"SECURITY_CONFIG": {"signMetadata": true} }',
+        )
+        with self.assertRaises(OneLogin_Saml2_Error):
+            self._fetch_metadata()  # OneLogin_Saml2_Error: Cannot sign metadata: missing SP private key.
+        with self.settings(
+            SOCIAL_AUTH_SAML_SP_PRIVATE_KEY=self._get_private_key('saml_key'),
+            SOCIAL_AUTH_SAML_SP_PUBLIC_CERT=self._get_public_key('saml_key'),
+        ):
+            self._validate_signed_metadata()
+
+    def _validate_signed_metadata(self, pub_key_starts_with='MIICsDCCAhmgAw'):
+        """ Fetch the SAML metadata and do some validation """
         doc = self._fetch_metadata()
         sig_node = doc.find(".//{}".format(etree.QName(XMLDSIG_XML_NS, 'SignatureValue')))
         self.assertIsNotNone(sig_node)
+        # Check that the right public key was used:
+        pub_key_node = doc.find(".//{}".format(etree.QName(XMLDSIG_XML_NS, 'X509Certificate')))
+        self.assertIsNotNone(pub_key_node)
+        self.assertIn(pub_key_starts_with, pub_key_node.text)
 
     def _fetch_metadata(self):
         """ Fetch and parse the metadata XML at self.METADATA_URL """

lms/djangoapps/ccx/views.py

@@ -410,7 +410,7 @@ def ccx_invite(request, course, ccx=None):
         try:
             validate_email(email)
             course_key = CCXLocator.from_course_locator(course.id, ccx.id)
-            email_params = get_email_params(course, auto_enroll)
+            email_params = get_email_params(course, auto_enroll, course_key=course_key, display_name=ccx.display_name)
             if action == 'Enroll':
                 enroll_email(
                     course_key,

lms/djangoapps/instructor/enrollment.py

@@ -261,7 +261,7 @@ def _reset_module_attempts(studentmodule):
     studentmodule.save()
 
 
-def get_email_params(course, auto_enroll, secure=True):
+def get_email_params(course, auto_enroll, secure=True, course_key=None, display_name=None):
     """
     Generate parameters used when parsing email templates.
 
@@ -270,6 +270,8 @@ def get_email_params(course, auto_enroll, secure=True):
     """
 
     protocol = 'https' if secure else 'http'
+    course_key = course_key or course.id.to_deprecated_string()
+    display_name = display_name or course.display_name_with_default
 
     stripped_site_name = microsite.get_value(
         'SITE_NAME',
@@ -285,7 +287,7 @@ def get_email_params(course, auto_enroll, secure=True):
     course_url = u'{proto}://{site}{path}'.format(
         proto=protocol,
         site=stripped_site_name,
-        path=reverse('course_root', kwargs={'course_id': course.id.to_deprecated_string()})
+        path=reverse('course_root', kwargs={'course_id': course_key})
     )
 
     # We can't get the url to the course's About page if the marketing site is enabled.
@@ -294,7 +296,7 @@ def get_email_params(course, auto_enroll, secure=True):
         course_about_url = u'{proto}://{site}{path}'.format(
             proto=protocol,
             site=stripped_site_name,
-            path=reverse('about_course', kwargs={'course_id': course.id.to_deprecated_string()})
+            path=reverse('about_course', kwargs={'course_id': course_key})
         )
 
     is_shib_course = uses_shib(course)
@@ -304,6 +306,7 @@ def get_email_params(course, auto_enroll, secure=True):
         'site_name': stripped_site_name,
         'registration_url': registration_url,
         'course': course,
+        'display_name': display_name,
         'auto_enroll': auto_enroll,
         'course_url': course_url,
         'course_about_url': course_about_url,
@@ -321,6 +324,7 @@ def send_mail_to_student(student, param_dict, language=None):
     [
         `site_name`: name given to edX instance (a `str`)
         `registration_url`: url for registration (a `str`)
+        `display_name` : display name of a course (a `str`)
         `course_id`: id of course (a `str`)
         `auto_enroll`: user input option (a `str`)
         `course_url`: url of course (a `str`)
@@ -338,8 +342,8 @@ def send_mail_to_student(student, param_dict, language=None):
     """
 
     # add some helpers and microconfig subsitutions
-    if 'course' in param_dict:
-        param_dict['course_name'] = param_dict['course'].display_name_with_default
+    if 'display_name' in param_dict:
+        param_dict['course_name'] = param_dict['display_name']
 
     param_dict['site_name'] = microsite.get_value(
         'SITE_NAME',

lms/djangoapps/instructor/tests/test_enrollment.py

@@ -5,6 +5,7 @@ Unit tests for instructor.enrollment methods.
 
 import json
 import mock
+from mock import patch
 from abc import ABCMeta
 from courseware.models import StudentModule
 from django.conf import settings
@@ -12,11 +13,17 @@ from django.test import TestCase
 from django.utils.translation import get_language
 from django.utils.translation import override as override_language
 from nose.plugins.attrib import attr
+from ccx_keys.locator import CCXLocator
 from student.tests.factories import UserFactory
 from xmodule.modulestore.django import modulestore
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 
+from ccx.tests.factories import CcxFactory
 from student.models import CourseEnrollment, CourseEnrollmentAllowed
+from student.roles import CourseCcxCoachRole  # pylint: disable=import-error
+from student.tests.factories import (  # pylint: disable=import-error
+    AdminFactory
+)
 from instructor.enrollment import (
     EmailEnrollmentState,
     enroll_email,
@@ -30,8 +37,9 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey
 
 from submissions import api as sub_api
 from student.models import anonymous_id_for_user
-from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
-
+from xmodule.modulestore.tests.django_utils import (
+    ModuleStoreTestCase, SharedModuleStoreTestCase, TEST_DATA_SPLIT_MODULESTORE
+)
 
 @attr('shard_1')
 class TestSettableEnrollmentState(TestCase):
@@ -567,6 +575,53 @@ class TestSendBetaRoleEmail(TestCase):
 
 
 @attr('shard_1')
+class TestGetEmailParamsCCX(ModuleStoreTestCase):
+    """
+    Test what URLs the function get_email_params for CCX student enrollment.
+    """
+
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
+    @patch.dict('django.conf.settings.FEATURES', {'CUSTOM_COURSES_EDX': True})
+    def setUp(self):
+        super(TestGetEmailParamsCCX, self).setUp()
+
+        self.course = CourseFactory.create()
+        self.coach = AdminFactory.create()
+        role = CourseCcxCoachRole(self.course.id)
+        role.add_users(self.coach)
+        self.ccx = CcxFactory(course_id=self.course.id, coach=self.coach)
+        self.course_key = CCXLocator.from_course_locator(self.course.id, self.ccx.id)
+        # Explicitly construct what we expect the course URLs to be
+        site = settings.SITE_NAME
+        self.course_url = u'https://{}/courses/{}/'.format(
+            site,
+            self.course_key
+        )
+        self.course_about_url = self.course_url + 'about'
+        self.registration_url = u'https://{}/register'.format(
+            site,
+        )
+
+    @patch.dict('django.conf.settings.FEATURES', {'CUSTOM_COURSES_EDX': True})
+    def test_ccx_enrollment_email_params(self):
+        # For a CCX, what do we expect to get for the URLs?
+        # Also make sure `auto_enroll` is properly passed through.
+        result = get_email_params(
+            self.course,
+            True,
+            course_key=self.course_key,
+            display_name=self.ccx.display_name
+        )
+
+        self.assertEqual(result['display_name'], self.ccx.display_name)
+        self.assertEqual(result['auto_enroll'], True)
+        self.assertEqual(result['course_about_url'], self.course_about_url)
+        self.assertEqual(result['registration_url'], self.registration_url)
+        self.assertEqual(result['course_url'], self.course_url)
+
+
+@attr('shard_1')
 class TestGetEmailParams(SharedModuleStoreTestCase):
     """
     Test what URLs the function get_email_params returns under different
@@ -616,7 +671,10 @@ class TestGetEmailParams(SharedModuleStoreTestCase):
 class TestRenderMessageToString(SharedModuleStoreTestCase):
     """
     Test that email templates can be rendered in a language chosen manually.
+    Test CCX enrollmet email.
     """
+    MODULESTORE = TEST_DATA_SPLIT_MODULESTORE
+
     @classmethod
     def setUpClass(cls):
         super(TestRenderMessageToString, cls).setUpClass()
@@ -626,6 +684,8 @@ class TestRenderMessageToString(SharedModuleStoreTestCase):
 
     def setUp(self):
         super(TestRenderMessageToString, self).setUp()
+        self.course_key = None
+        self.ccx = None
 
     def get_email_params(self):
         """
@@ -637,6 +697,27 @@ class TestRenderMessageToString(SharedModuleStoreTestCase):
 
         return email_params
 
+    def get_email_params_ccx(self):
+        """
+        Returns a dictionary of parameters used to render an email for CCX.
+        """
+        coach = AdminFactory.create()
+        role = CourseCcxCoachRole(self.course.id)
+        role.add_users(coach)
+        self.ccx = CcxFactory(course_id=self.course.id, coach=coach)
+        self.course_key = CCXLocator.from_course_locator(self.course.id, self.ccx.id)
+
+        email_params = get_email_params(
+            self.course,
+            True,
+            course_key=self.course_key,
+            display_name=self.ccx.display_name
+        )
+        email_params["email_address"] = "user@example.com"
+        email_params["full_name"] = "Jean Reno"
+
+        return email_params
+
     def get_subject_and_message(self, language):
         """
         Returns the subject and message rendered in the specified language.
@@ -648,6 +729,18 @@ class TestRenderMessageToString(SharedModuleStoreTestCase):
             language=language
         )
 
+    def get_subject_and_message_ccx(self):
+        """
+        Returns the subject and message rendered in the specified language for CCX.
+        """
+        subject_template = 'emails/enroll_email_enrolledsubject.txt'
+        message_template = 'emails/enroll_email_enrolledmessage.txt'
+        return render_message_to_string(
+            subject_template,
+            message_template,
+            self.get_email_params_ccx()
+        )
+
     def test_subject_and_message_translation(self):
         subject, message = self.get_subject_and_message('fr')
         language_after_rendering = get_language()
@@ -662,3 +755,18 @@ class TestRenderMessageToString(SharedModuleStoreTestCase):
             subject, message = self.get_subject_and_message(None)
             self.assertIn("You have been", subject)
             self.assertIn("You have been", message)
+
+    @patch.dict('django.conf.settings.FEATURES', {'CUSTOM_COURSES_EDX': True})
+    def test_render_message_ccx(self):
+        """
+        Test email template renders for CCX.
+        """
+        subject, message = self.get_subject_and_message_ccx()
+        self.assertIn(self.ccx.display_name, subject)
+        self.assertIn(self.ccx.display_name, message)
+        site = settings.SITE_NAME
+        course_url = u'https://{}/courses/{}/'.format(
+            site,
+            self.course_key
+        )
+        self.assertIn(course_url, message)

lms/envs/aws.py

@@ -558,6 +558,15 @@ if FEATURES.get('ENABLE_THIRD_PARTY_AUTH'):
     # The reduced session expiry time during the third party login pipeline. (Value in seconds)
     SOCIAL_AUTH_PIPELINE_TIMEOUT = ENV_TOKENS.get('SOCIAL_AUTH_PIPELINE_TIMEOUT', 600)
 
+    # Most provider configuration is done via ConfigurationModels but for a few sensitive values
+    # we allow configuration via AUTH_TOKENS instead (optionally).
+    # The SAML private/public key values do not need the delimiter lines (such as
+    # "-----BEGIN PRIVATE KEY-----", "-----END PRIVATE KEY-----" etc.) but they may be included
+    # if you want (though it's easier to format the key values as JSON without the delimiters).
+    SOCIAL_AUTH_SAML_SP_PRIVATE_KEY = AUTH_TOKENS.get('SOCIAL_AUTH_SAML_SP_PRIVATE_KEY', '')
+    SOCIAL_AUTH_SAML_SP_PUBLIC_CERT = AUTH_TOKENS.get('SOCIAL_AUTH_SAML_SP_PUBLIC_CERT', '')
+    SOCIAL_AUTH_OAUTH_SECRETS = AUTH_TOKENS.get('SOCIAL_AUTH_OAUTH_SECRETS', {})
+
     # third_party_auth config moved to ConfigurationModels. This is for data migration only:
     THIRD_PARTY_AUTH_OLD_CONFIG = AUTH_TOKENS.get('THIRD_PARTY_AUTH', None)
 

lms/static/sass/shared/_modal.scss

@@ -12,15 +12,15 @@
 
 .modal {
   @extend %ui-depth1;
-  background: $gray-d2;
-  border-radius: 3px;
-  box-shadow: 0 0px 5px 0 $shadow-d1;
-  color: $white;
   display: none;
+  position: absolute;
   left: 50%;
   padding: 8px;
-  position: absolute;
   width: grid-width(5);
+  border-radius: 3px;
+  box-shadow: 0 0px 5px 0 $shadow-d1;
+  background: $gray-d2;
+  color: $base-font-color;
 
   &.video-modal {
     left: 50%;
@@ -62,6 +62,11 @@
     padding-bottom: ($baseline/2);
     position: relative;
 
+    p {
+      font-size: .9em;
+      line-height: 1.4;
+    }
+
     header {
       @extend %ui-depth1;
       margin-bottom: ($baseline*1.5);

lms/templates/emails/enroll_email_enrolledmessage.txt

@@ -5,7 +5,7 @@ ${_("Dear {full_name}").format(full_name=full_name)}
 ${_("You have been enrolled in {course_name} at {site_name} by a member "
 	"of the course staff. The course should now appear on your {site_name} "
 	"dashboard.").format(
-		course_name=course.display_name_with_default,
+		course_name=display_name or course.display_name_with_default,
 		site_name=site_name
 	)}
 

lms/templates/emails/enroll_email_enrolledsubject.txt

 <%! from django.utils.translation import ugettext as _ %>
 
 ${_("You have been enrolled in {course_name}").format(
-		course_name=course.display_name_with_default
+		course_name=display_name or course.display_name_with_default
 	)}
\ No newline at end of file

requirements/edx/github.txt

@@ -57,7 +57,7 @@ git+https://github.com/edx/ecommerce-api-client.git@1.1.0#egg=ecommerce-api-clie
 -e git+https://github.com/edx/edx-user-state-client.git@30c0ad4b9f57f8d48d6943eb585ec8a9205f4469#egg=edx-user-state-client
 -e git+https://github.com/edx/edx-organizations.git@release-2015-08-03#egg=edx-organizations
 
-git+https://github.com/edx/edx-proctoring.git@0.7.1#egg=edx-proctoring==0.7.1
+git+https://github.com/edx/edx-proctoring.git@0.7.2#egg=edx-proctoring==0.7.2
 
 # Third Party XBlocks
 -e git+https://github.com/mitodl/edx-sga@172a90fd2738f8142c10478356b2d9ed3e55334a#egg=edx-sga