diff --git a/lms/djangoapps/verify_student/admin.py b/lms/djangoapps/verify_student/admin.py index 6a7c69b9e2..47bd040862 100644 --- a/lms/djangoapps/verify_student/admin.py +++ b/lms/djangoapps/verify_student/admin.py @@ -1,8 +1,6 @@ from ratelimitbackend import admin -from config_models.admin import ConfigurationModelAdmin from verify_student.models import ( SoftwareSecurePhotoVerification, - InCourseReverificationConfiguration, VerificationStatus, SkippedReverification, ) @@ -40,10 +38,6 @@ class VerificationStatusAdmin(admin.ModelAdmin): return self.readonly_fields + ('status', 'checkpoint', 'user', 'response', 'error') return self.readonly_fields - def has_delete_permission(self, request, obj=None): - """The verification status table is append-only. """ - return False - class SkippedReverificationAdmin(admin.ModelAdmin): """Admin for the SkippedReverification table. """ @@ -58,6 +52,5 @@ class SkippedReverificationAdmin(admin.ModelAdmin): admin.site.register(SoftwareSecurePhotoVerification, SoftwareSecurePhotoVerificationAdmin) -admin.site.register(InCourseReverificationConfiguration, ConfigurationModelAdmin) admin.site.register(SkippedReverification, SkippedReverificationAdmin) admin.site.register(VerificationStatus, VerificationStatusAdmin) diff --git a/lms/djangoapps/verify_student/image.py b/lms/djangoapps/verify_student/image.py new file mode 100644 index 0000000000..a440e5b673 --- /dev/null +++ b/lms/djangoapps/verify_student/image.py @@ -0,0 +1,35 @@ +""" +Image encoding helpers for the verification app. +""" +import logging + + +log = logging.getLogger(__name__) + + +class InvalidImageData(Exception): + """ + The provided image data could not be decoded. + """ + pass + + +def decode_image_data(data): + """ + Decode base64-encoded image data. + + Arguments: + data (str): The raw image data, base64-encoded. + + Returns: + str + + Raises: + InvalidImageData: The image data could not be decoded. + + """ + try: + return (data.split(",")[1]).decode("base64") + except (IndexError, UnicodeEncodeError): + log.exception("Could not decode image data") + raise InvalidImageData diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index e24f7bc659..0b25c7e96a 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -26,7 +26,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.core.urlresolvers import reverse from django.core.cache import cache from django.dispatch import receiver -from django.db import models +from django.db import models, transaction, IntegrityError from django.utils.translation import ugettext as _, ugettext_lazy from boto.s3.connection import S3Connection @@ -589,14 +589,6 @@ class SoftwareSecurePhotoVerification(PhotoVerification): IMAGE_LINK_DURATION = 5 * 60 * 60 * 24 # 5 days in seconds - @classmethod - def original_verification(cls, user): - """ - Returns the most current SoftwareSecurePhotoVerification object associated with the user. - """ - query = cls.objects.filter(user=user).order_by('-updated_at') - return query[0] - @classmethod def get_initial_verification(cls, user): """Get initial verification for a user @@ -631,19 +623,6 @@ class SoftwareSecurePhotoVerification(PhotoVerification): s3_key = self._generate_s3_key("face") s3_key.set_contents_from_string(encrypt_and_encode(img_data, aes_key)) - @status_before_must_be("created") - def fetch_photo_id_image(self): - """ - Find the user's photo ID image, which was submitted with their original verification. - The image has already been encrypted and stored in s3, so we just need to find that - location - """ - if settings.FEATURES.get('AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING'): - return - - self.photo_id_key = self.original_verification(self.user).photo_id_key - self.save() - @status_before_must_be("created") def upload_photo_id_image(self, img_data): """ @@ -676,14 +655,20 @@ class SoftwareSecurePhotoVerification(PhotoVerification): self.save() @status_before_must_be("must_retry", "ready", "submitted") - def submit(self): + def submit(self, copy_id_photo_from=None): """ Submit our verification attempt to Software Secure for validation. This will set our status to "submitted" if the post is successful, and "must_retry" if the post fails. + + Keyword Arguments: + copy_id_photo_from (SoftwareSecurePhotoVerification): If provided, re-send the ID photo + data from this attempt. This is used for reverification, in which new face photos + are sent with previously-submitted ID photos. + """ try: - response = self.send_request() + response = self.send_request(copy_id_photo_from=copy_id_photo_from) if response.ok: self.submitted_at = datetime.now(pytz.UTC) self.status = "submitted" @@ -733,15 +718,28 @@ class SoftwareSecurePhotoVerification(PhotoVerification): log.error('PhotoVerification: Error parsing this error message: %s', self.error_msg) return _("There was an error verifying your ID photos.") - def image_url(self, name): + def image_url(self, name, override_receipt_id=None): """ We dynamically generate this, since we want it the expiration clock to start when the message is created, not when the record is created. + + Arguments: + name (str): Name of the image (e.g. "photo_id" or "face") + + Keyword Arguments: + override_receipt_id (str): If provided, use this receipt ID instead + of the ID for this attempt. This is useful for reverification + where we need to construct a URL to a previously-submitted + photo ID image. + + Returns: + string: The expiring URL for the image. + """ - s3_key = self._generate_s3_key(name) + s3_key = self._generate_s3_key(name, override_receipt_id=override_receipt_id) return s3_key.generate_url(self.IMAGE_LINK_DURATION) - def _generate_s3_key(self, prefix): + def _generate_s3_key(self, prefix, override_receipt_id=None): """ Generates a key for an s3 bucket location @@ -753,8 +751,14 @@ class SoftwareSecurePhotoVerification(PhotoVerification): ) bucket = conn.get_bucket(settings.VERIFY_STUDENT["SOFTWARE_SECURE"]["S3_BUCKET"]) + # Override the receipt ID if one is provided. + # This allow us to construct S3 keys to images submitted in previous attempts + # (used for reverification, where we send a new face photo with the same photo ID + # from a previous attempt). + receipt_id = self.receipt_id if override_receipt_id is None else override_receipt_id + key = Key(bucket) - key.key = "{}/{}".format(prefix, self.receipt_id) + key.key = "{}/{}".format(prefix, receipt_id) return key @@ -772,8 +776,19 @@ class SoftwareSecurePhotoVerification(PhotoVerification): return rsa_encrypted_face_aes_key.encode("base64") - def create_request(self): - """return headers, body_dict""" + def create_request(self, copy_id_photo_from=None): + """ + Construct the HTTP request to the photo verification service. + + Keyword Arguments: + copy_id_photo_from (SoftwareSecurePhotoVerification): If provided, re-send the ID photo + data from this attempt. This is used for reverification, in which new face photos + are sent with previously-submitted ID photos. + + Returns: + tuple of (header, body), where both `header` and `body` are dictionaries. + + """ access_key = settings.VERIFY_STUDENT["SOFTWARE_SECURE"]["API_ACCESS_KEY"] secret_key = settings.VERIFY_STUDENT["SOFTWARE_SECURE"]["API_SECRET_KEY"] @@ -782,11 +797,25 @@ class SoftwareSecurePhotoVerification(PhotoVerification): scheme, settings.SITE_NAME, reverse('verify_student_results_callback') ) + # If we're copying the photo ID image from a previous verification attempt, + # then we need to send the old image data with the correct image key. + photo_id_url = ( + self.image_url("photo_id") + if copy_id_photo_from is None + else self.image_url("photo_id", override_receipt_id=copy_id_photo_from.receipt_id) + ) + + photo_id_key = ( + self.photo_id_key + if copy_id_photo_from is None else + copy_id_photo_from.photo_id_key + ) + body = { "EdX-ID": str(self.receipt_id), "ExpectedName": self.name, - "PhotoID": self.image_url("photo_id"), - "PhotoIDKey": self.photo_id_key, + "PhotoID": photo_id_url, + "PhotoIDKey": photo_id_key, "SendResponseTo": callback_url, "UserPhoto": self.image_url("face"), "UserPhotoKey": self._encrypted_user_photo_key_str(), @@ -819,11 +848,18 @@ class SoftwareSecurePhotoVerification(PhotoVerification): return header_txt + "\n\n" + body_txt - def send_request(self): + def send_request(self, copy_id_photo_from=None): """ Assembles a submission to Software Secure and sends it via HTTPS. - Returns a request.Response() object with the reply we get from SS. + Keyword Arguments: + copy_id_photo_from (SoftwareSecurePhotoVerification): If provided, re-send the ID photo + data from this attempt. This is used for reverification, in which new face photos + are sent with previously-submitted ID photos. + + Returns: + request.Response + """ # If AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING is True, we want to # skip posting anything to Software Secure. We actually don't even @@ -835,14 +871,24 @@ class SoftwareSecurePhotoVerification(PhotoVerification): fake_response.status_code = 200 return fake_response - headers, body = self.create_request() + headers, body = self.create_request(copy_id_photo_from=copy_id_photo_from) response = requests.post( settings.VERIFY_STUDENT["SOFTWARE_SECURE"]["API_URL"], headers=headers, data=json.dumps(body, indent=2, sort_keys=True, ensure_ascii=False).encode('utf-8'), verify=False ) - log.debug("Sent request to Software Secure for {}".format(self.receipt_id)) + + log.info("Sent request to Software Secure for receipt ID %s.", self.receipt_id) + if copy_id_photo_from is not None: + log.info( + ( + "Software Secure attempt with receipt ID %s used the same photo ID " + "data as the receipt with ID %s" + ), + self.receipt_id, copy_id_photo_from.receipt_id + ) + log.debug("Headers:\n{}\n\n".format(headers)) log.debug("Body:\n{}\n\n".format(body)) log.debug("Return code: {}".format(response.status_code)) @@ -850,27 +896,6 @@ class SoftwareSecurePhotoVerification(PhotoVerification): return response - @classmethod - def submit_faceimage(cls, user, face_image, photo_id_key): - """Submit the face image to SoftwareSecurePhotoVerification. - - Arguments: - user(User): user object - face_image (bytestream): raw bytestream image data - photo_id_key (str) : SoftwareSecurePhotoVerification attribute - - Returns: - SoftwareSecurePhotoVerification Object - """ - b64_face_image = face_image.split(",")[1] - attempt = SoftwareSecurePhotoVerification(user=user) - attempt.upload_face_image(b64_face_image.decode('base64')) - attempt.photo_id_key = photo_id_key - attempt.mark_ready() - attempt.save() - attempt.submit() - return attempt - @classmethod def verification_status_for_user(cls, user, course_id, user_enrollment_mode): """ @@ -1068,21 +1093,30 @@ class VerificationCheckpoint(models.Model): return None @classmethod - def get_verification_checkpoint(cls, course_id, checkpoint_location): - """Get the verification checkpoint for given 'course_id' and + def get_or_create_verification_checkpoint(cls, course_id, checkpoint_location): + """ + Get or create the verification checkpoint for given 'course_id' and checkpoint name. Arguments: - course_id(CourseKey): CourseKey - checkpoint_location(str): Verification checkpoint location + course_id (CourseKey): CourseKey + checkpoint_location (str): Verification checkpoint location Returns: VerificationCheckpoint object if exists otherwise None """ try: + checkpoint, __ = cls.objects.get_or_create(course_id=course_id, checkpoint_location=checkpoint_location) + return checkpoint + except IntegrityError: + log.info( + u"An integrity error occurred while getting-or-creating the verification checkpoint " + "for course %s at location %s. This can occur if two processes try to get-or-create " + "the checkpoint at the same time and the database is set to REPEATABLE READ. " + "We will try committing the transaction and retrying." + ) + transaction.commit() return cls.objects.get(course_id=course_id, checkpoint_location=checkpoint_location) - except cls.DoesNotExist: - return None class VerificationStatus(models.Model): @@ -1141,6 +1175,29 @@ class VerificationStatus(models.Model): for checkpoint in checkpoints: cls.objects.create(checkpoint=checkpoint, user=user, status=status) + @classmethod + def get_user_status_at_checkpoint(cls, user, course_key, location): + """ + Get the user's latest status at the checkpoint. + + Arguments: + user (User): The user whose status we are retrieving. + course_key (CourseKey): The identifier for the course. + location (UsageKey): The location of the checkpoint in the course. + + Returns: + unicode or None + + """ + try: + return cls.objects.filter( + user=user, + checkpoint__course_id=course_key, + checkpoint__checkpoint_location=unicode(location), + ).latest().status + except cls.DoesNotExist: + return None + @classmethod def get_user_attempts(cls, user_id, course_key, related_assessment_location): """ @@ -1180,6 +1237,9 @@ class VerificationStatus(models.Model): return "" +# DEPRECATED: this feature has been permanently enabled. +# Once the application code has been updated in production, +# this table can be safely deleted. class InCourseReverificationConfiguration(ConfigurationModel): """Configure in-course re-verification. diff --git a/lms/djangoapps/verify_student/services.py b/lms/djangoapps/verify_student/services.py index bb607f17eb..64f615bc63 100644 --- a/lms/djangoapps/verify_student/services.py +++ b/lms/djangoapps/verify_student/services.py @@ -60,10 +60,9 @@ class ReverificationService(object): Re-verification link """ course_key = CourseKey.from_string(course_id) - VerificationCheckpoint.objects.get_or_create( - course_id=course_key, - checkpoint_location=related_assessment_location - ) + + # Get-or-create the verification checkpoint + VerificationCheckpoint.get_or_create_verification_checkpoint(course_key, related_assessment_location) re_verification_link = reverse( 'verify_student_incourse_reverify', diff --git a/lms/djangoapps/verify_student/tests/fake_software_secure.py b/lms/djangoapps/verify_student/tests/fake_software_secure.py index 5931287977..a3de08b7b1 100644 --- a/lms/djangoapps/verify_student/tests/fake_software_secure.py +++ b/lms/djangoapps/verify_student/tests/fake_software_secure.py @@ -40,7 +40,7 @@ class SoftwareSecureFakeView(View): } try: - most_recent = SoftwareSecurePhotoVerification.original_verification(user) + most_recent = SoftwareSecurePhotoVerification.objects.filter(user=user).order_by("-updated_at")[0] context["receipt_id"] = most_recent.receipt_id except: # pylint: disable=bare-except pass diff --git a/lms/djangoapps/verify_student/tests/test_models.py b/lms/djangoapps/verify_student/tests/test_models.py index f43fd4d187..8d7b4cb914 100644 --- a/lms/djangoapps/verify_student/tests/test_models.py +++ b/lms/djangoapps/verify_student/tests/test_models.py @@ -220,18 +220,6 @@ class TestPhotoVerification(ModuleStoreTestCase): return attempt - def test_fetch_photo_id_image(self): - user = UserFactory.create() - orig_attempt = SoftwareSecurePhotoVerification(user=user) - orig_attempt.save() - - old_key = orig_attempt.photo_id_key - - new_attempt = SoftwareSecurePhotoVerification(user=user) - new_attempt.save() - new_attempt.fetch_photo_id_image() - assert_equals(new_attempt.photo_id_key, old_key) - def test_submissions(self): """Test that we set our status correctly after a submission.""" # Basic case, things go well. @@ -501,7 +489,7 @@ class VerificationCheckpointTest(ModuleStoreTestCase): ) @ddt.data('midterm', 'final') - def test_get_verification_checkpoint(self, checkpoint): + def test_get_or_create_verification_checkpoint(self, checkpoint): """ Test that a reverification checkpoint is created properly. """ @@ -514,26 +502,40 @@ class VerificationCheckpointTest(ModuleStoreTestCase): checkpoint_location=checkpoint_location ) self.assertEqual( - VerificationCheckpoint.get_verification_checkpoint(self.course.id, checkpoint_location), + VerificationCheckpoint.get_or_create_verification_checkpoint(self.course.id, checkpoint_location), verification_checkpoint ) - def test_get_verification_checkpoint_for_not_existing_values(self): - """Test that 'get_verification_checkpoint' method returns None if user - tries to access a checkpoint with an invalid location. - """ - # create the VerificationCheckpoint checkpoint - VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_location=self.checkpoint_midterm) + def test_get_or_create_verification_checkpoint_for_not_existing_values(self): + # Retrieving a checkpoint that doesn't yet exist will create it + location = u'i4x://edX/DemoX/edx-reverification-block/invalid_location' + checkpoint = VerificationCheckpoint.get_or_create_verification_checkpoint(self.course.id, location) - # get verification for a non existing checkpoint - self.assertEqual( - VerificationCheckpoint.get_verification_checkpoint( - self.course.id, - u'i4x://edX/DemoX/edx-reverification-block/invalid_location' - ), - None + self.assertIsNot(checkpoint, None) + self.assertEqual(checkpoint.course_id, self.course.id) + self.assertEqual(checkpoint.checkpoint_location, location) + + def test_get_or_create_integrity_error(self): + # Create the checkpoint + VerificationCheckpoint.objects.create( + course_id=self.course.id, + checkpoint_location=self.checkpoint_midterm, ) + # Simulate that the get-or-create operation raises an IntegrityError + # This can happen when two processes both try to get-or-create at the same time + # when the database is set to REPEATABLE READ. + with patch.object(VerificationCheckpoint.objects, "get_or_create") as mock_get_or_create: + mock_get_or_create.side_effect = IntegrityError + checkpoint = VerificationCheckpoint.get_or_create_verification_checkpoint( + self.course.id, + self.checkpoint_midterm + ) + + # The checkpoint should be retrieved without error + self.assertEqual(checkpoint.course_id, self.course.id) + self.assertEqual(checkpoint.checkpoint_location, self.checkpoint_midterm) + def test_unique_together_constraint(self): """ Test the unique together constraint. diff --git a/lms/djangoapps/verify_student/tests/test_views.py b/lms/djangoapps/verify_student/tests/test_views.py index 73c1c958aa..cf8635b8f4 100644 --- a/lms/djangoapps/verify_student/tests/test_views.py +++ b/lms/djangoapps/verify_student/tests/test_views.py @@ -11,9 +11,12 @@ from uuid import uuid4 import ddt import httpretty import mock +import boto +import moto import pytz from bs4 import BeautifulSoup from mock import patch, Mock, ANY +import requests from django.conf import settings from django.core.urlresolvers import reverse @@ -44,8 +47,7 @@ from verify_student.views import ( ) from verify_student.models import ( VerificationDeadline, SoftwareSecurePhotoVerification, - VerificationCheckpoint, InCourseReverificationConfiguration, - VerificationStatus + VerificationCheckpoint, VerificationStatus ) from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -1307,6 +1309,82 @@ class TestSubmitPhotosForVerification(TestCase): # Check that the user's name was changed in the database self._assert_user_name(self.FULL_NAME) + def test_submit_photos_sends_confirmation_email(self): + self._submit_photos( + face_image=self.IMAGE_DATA, + photo_id_image=self.IMAGE_DATA + ) + self._assert_confirmation_email(True) + + def test_submit_photos_error_does_not_send_email(self): + # Error because invalid parameters, so no confirmation email + # should be sent. + self._submit_photos(expected_status_code=400) + self._assert_confirmation_email(False) + + # Disable auto-auth since we will be intercepting POST requests + # to the verification service ourselves in this test. + @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': False}) + @override_settings(VERIFY_STUDENT={ + "SOFTWARE_SECURE": { + "API_URL": "https://verify.example.com/submit/", + "API_ACCESS_KEY": "dcf291b5572942f99adaab4c2090c006", + "API_SECRET_KEY": "c392efdcc0354c5f922dc39844ec0dc7", + "FACE_IMAGE_AES_KEY": "f82400259e3b4f88821cd89838758292", + "RSA_PUBLIC_KEY": ( + "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDkgtz3fQdiXshy/RfOHkoHlhx/" + "SSPZ+nNyE9JZXtwhlzsXjnu+e9GOuJzgh4kUqo73ePIG5FxVU+mnacvufq2cu1SOx" + "lRYGyBK7qDf9Ym67I5gmmcNhbzdKcluAuDCPmQ4ecKpICQQldrDQ9HWDxwjbbcqpVB" + "PYWkE1KrtypGThmcehLmabf6SPq1CTAGlXsHgUtbWCwV6mqR8yScV0nRLln0djLDm9d" + "L8tIVFFVpAfBaYYh2Cm5EExQZjxyfjWd8P5H+8/l0pmK2jP7Hc0wuXJemIZbsdm+DSD" + "FhCGY3AILGkMwr068dGRxfBtBy/U9U5W+nStvkDdMrSgQezS5+V test@example.com" + ), + "AWS_ACCESS_KEY": "c987c7efe35c403caa821f7328febfa1", + "AWS_SECRET_KEY": "fc595fc657c04437bb23495d8fe64881", + "S3_BUCKET": "test.example.com", + } + }) + @httpretty.activate + @moto.mock_s3 + def test_submit_photos_for_reverification(self): + # Create the S3 bucket for photo upload + conn = boto.connect_s3() + conn.create_bucket("test.example.com") + + # Mock the POST to Software Secure + httpretty.register_uri(httpretty.POST, "https://verify.example.com/submit/") + + # Submit an initial verification attempt + self._submit_photos( + face_image=self.IMAGE_DATA + "4567", + photo_id_image=self.IMAGE_DATA + "8910", + ) + initial_data = self._get_post_data() + + # Submit a face photo for re-verification + self._submit_photos(face_image=self.IMAGE_DATA + "1112") + reverification_data = self._get_post_data() + + # Verify that the initial attempt sent the same ID photo as the reverification attempt + self.assertEqual(initial_data["PhotoIDKey"], reverification_data["PhotoIDKey"]) + + initial_photo_response = requests.get(initial_data["PhotoID"]) + self.assertEqual(initial_photo_response.status_code, 200) + + reverification_photo_response = requests.get(reverification_data["PhotoID"]) + self.assertEqual(reverification_photo_response.status_code, 200) + + self.assertEqual(initial_photo_response.content, reverification_photo_response.content) + + # Verify that the second attempt sent the updated face photo + initial_photo_response = requests.get(initial_data["UserPhoto"]) + self.assertEqual(initial_photo_response.status_code, 200) + + reverification_photo_response = requests.get(reverification_data["UserPhoto"]) + self.assertEqual(reverification_photo_response.status_code, 200) + + self.assertNotEqual(initial_photo_response.content, reverification_photo_response.content) + @ddt.data('face_image', 'photo_id_image') def test_invalid_image_data(self, invalid_param): params = { @@ -1326,19 +1404,32 @@ class TestSubmitPhotosForVerification(TestCase): ) self.assertEqual(response.content, "Name must be at least 2 characters long.") - @ddt.data('face_image', 'photo_id_image') - def test_missing_required_params(self, missing_param): + def test_missing_required_param(self): + # Missing face image parameter params = { - 'face_image': self.IMAGE_DATA, 'photo_id_image': self.IMAGE_DATA } - del params[missing_param] response = self._submit_photos(expected_status_code=400, **params) + self.assertEqual(response.content, "Missing required parameter face_image") + + def test_no_photo_id_and_no_initial_verification(self): + # Submit face image data, but not photo ID data. + # Since the user doesn't have an initial verification attempt, this should fail + response = self._submit_photos(expected_status_code=400, face_image=self.IMAGE_DATA) self.assertEqual( response.content, - "Missing required parameters: {missing}".format(missing=missing_param) + "Photo ID image is required if the user does not have an initial verification attempt." ) + # Create the initial verification attempt + self._submit_photos( + face_image=self.IMAGE_DATA, + photo_id_image=self.IMAGE_DATA, + ) + + # Now the request should succeed + self._submit_photos(face_image=self.IMAGE_DATA) + def _submit_photos(self, face_image=None, photo_id_image=None, full_name=None, expected_status_code=200): """Submit photos for verification. @@ -1367,7 +1458,13 @@ class TestSubmitPhotosForVerification(TestCase): response = self.client.post(url, params) self.assertEqual(response.status_code, expected_status_code) - if expected_status_code == 200: + return response + + def _assert_confirmation_email(self, expect_email): + """ + Check that a confirmation email was or was not sent. + """ + if expect_email: # Verify that photo submission confirmation email was sent self.assertEqual(len(mail.outbox), 1) self.assertEqual("Verification photos received", mail.outbox[0].subject) @@ -1375,8 +1472,6 @@ class TestSubmitPhotosForVerification(TestCase): # Verify that photo submission confirmation email was not sent self.assertEqual(len(mail.outbox), 0) - return response - def _assert_user_name(self, full_name): """Check the user's name. @@ -1390,6 +1485,11 @@ class TestSubmitPhotosForVerification(TestCase): account_settings = get_account_settings(self.user) self.assertEqual(account_settings['name'], full_name) + def _get_post_data(self): + """Retrieve POST data from the last request. """ + last_request = httpretty.last_request() + return json.loads(last_request.body) + class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): """ @@ -1613,10 +1713,6 @@ class TestPhotoVerificationResultsCallback(ModuleStoreTestCase): """ self.create_reverification_xblock() - incourse_reverify_enabled = InCourseReverificationConfiguration.current() - incourse_reverify_enabled.enabled = True - incourse_reverify_enabled.save() - data = { "EdX-ID": self.receipt_id, "Result": "PASS", @@ -1821,31 +1917,17 @@ class TestInCourseReverifyView(ModuleStoreTestCase): # Enroll the user in the default mode (honor) to emulate CourseEnrollment.enroll(self.user, self.course_key, mode="verified") - self.config = InCourseReverificationConfiguration(enabled=True) - self.config.save() # mocking and patching for bi events analytics_patcher = patch('verify_student.views.analytics') self.mock_tracker = analytics_patcher.start() self.addCleanup(analytics_patcher.stop) - @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) - def test_incourse_reverify_feature_flag_get(self): - self.config.enabled = False - self.config.save() - response = self.client.get(self._get_url(self.course_key, self.reverification_location)) - self.assertEquals(response.status_code, 404) - - @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) - def test_incourse_reverify_invalid_course_get(self): - response = self.client.get(self._get_url("invalid/course/key", self.reverification_location)) - - self.assertEquals(response.status_code, 404) - @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) def test_incourse_reverify_invalid_checkpoint_get(self): + # Retrieve a checkpoint that doesn't yet exist response = self.client.get(self._get_url(self.course_key, "invalid_checkpoint")) - self.assertEquals(response.status_code, 404) + self.assertEqual(response.status_code, 404) @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) def test_incourse_reverify_initial_redirect_get(self): @@ -1853,6 +1935,7 @@ class TestInCourseReverifyView(ModuleStoreTestCase): response = self.client.get(self._get_url(self.course_key, self.reverification_location)) url = reverse('verify_student_verify_now', kwargs={"course_id": unicode(self.course_key)}) + url += u"?{params}".format(params=urllib.urlencode({"checkpoint": self.reverification_location})) self.assertRedirects(response, url) @override_settings(SEGMENT_IO_LMS_KEY="foobar") @@ -1890,23 +1973,24 @@ class TestInCourseReverifyView(ModuleStoreTestCase): """Verify that POST requests including an invalid checkpoint location results in a 400 response. """ - response = self.client.post(self._get_url(self.course_key, self.reverification_location)) + response = self._submit_photos(self.course_key, self.reverification_location, self.IMAGE_DATA) self.assertEquals(response.status_code, 400) @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) - def test_incourse_reverify_initial_redirect_post(self): + def test_incourse_reverify_id_required_if_no_initial_verification(self): self._create_checkpoint() - response = self.client.post(self._get_url(self.course_key, self.reverification_location)) - url = reverse('verify_student_verify_now', kwargs={"course_id": unicode(self.course_key)}) - self.assertRedirects(response, url) + # Since the user has no initial verification and we're not sending the ID photo, + # we should expect a 400 bad request + response = self._submit_photos(self.course_key, self.reverification_location, self.IMAGE_DATA) + self.assertEqual(response.status_code, 400) @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) def test_incourse_reverify_index_error_post(self): self._create_checkpoint() self._create_initial_verification() - response = self.client.post(self._get_url(self.course_key, self.reverification_location), {"face_image": ""}) + response = self._submit_photos(self.course_key, self.reverification_location, "") self.assertEqual(response.status_code, 400) @override_settings(SEGMENT_IO_LMS_KEY="foobar") @@ -1915,13 +1999,16 @@ class TestInCourseReverifyView(ModuleStoreTestCase): self._create_checkpoint() self._create_initial_verification() - response = self.client.post( - self._get_url(self.course_key, self.reverification_location), - {"face_image": self.IMAGE_DATA} - ) + response = self._submit_photos(self.course_key, self.reverification_location, self.IMAGE_DATA) self.assertEqual(response.status_code, 200) - # test that Google Analytics event firs after successfully submitting + # Check that the checkpoint status has been updated + status = VerificationStatus.get_user_status_at_checkpoint( + self.user, self.course_key, self.reverification_location + ) + self.assertEqual(status, "submitted") + + # Test that Google Analytics event fires after successfully submitting # photo verification self.mock_tracker.track.assert_called_once_with( # pylint: disable=no-member self.user.id, @@ -1938,14 +2025,6 @@ class TestInCourseReverifyView(ModuleStoreTestCase): ) self.mock_tracker.reset_mock() - @patch.dict(settings.FEATURES, {'AUTOMATIC_VERIFY_STUDENT_IDENTITY_FOR_TESTING': True}) - def test_incourse_reverify_feature_flag_post(self): - self.config.enabled = False - self.config.save() - - response = self.client.post(self._get_url(self.course_key, self.reverification_location)) - self.assertEquals(response.status_code, 404) - def _create_checkpoint(self): """ Helper method for creating a reverification checkpoint. @@ -1981,6 +2060,16 @@ class TestInCourseReverifyView(ModuleStoreTestCase): } ) + def _submit_photos(self, course_key, checkpoint_location, face_image_data): + """ Submit photos for verification. """ + url = reverse("verify_student_submit_photos") + data = { + "course_key": unicode(course_key), + "checkpoint": checkpoint_location, + "face_image": face_image_data, + } + return self.client.post(url, data) + class TestEmailMessageWithCustomICRVBlock(ModuleStoreTestCase): """ diff --git a/lms/djangoapps/verify_student/urls.py b/lms/djangoapps/verify_student/urls.py index 05e4926ea1..764ac34311 100644 --- a/lms/djangoapps/verify_student/urls.py +++ b/lms/djangoapps/verify_student/urls.py @@ -91,7 +91,7 @@ urlpatterns = patterns( url( r'^submit-photos/$', - views.submit_photos_for_verification, + views.SubmitPhotosView.as_view(), name="verify_student_submit_photos" ), diff --git a/lms/djangoapps/verify_student/views.py b/lms/djangoapps/verify_student/views.py index 3103f6b45a..96067b1d14 100644 --- a/lms/djangoapps/verify_student/views.py +++ b/lms/djangoapps/verify_student/views.py @@ -6,6 +6,7 @@ import datetime import decimal import json import logging +import urllib from pytz import UTC from ipware.ip import get_ip @@ -25,8 +26,8 @@ from django.views.generic.base import View, RedirectView import analytics from eventtracking import tracker -from opaque_keys.edx.keys import CourseKey, UsageKey from opaque_keys import InvalidKeyError +from opaque_keys.edx.keys import CourseKey, UsageKey from commerce import ecommerce_api_client from commerce.utils import audit_log @@ -37,7 +38,7 @@ from edxmako.shortcuts import render_to_response, render_to_string from embargo import api as embargo_api from microsite_configuration import microsite from openedx.core.djangoapps.user_api.accounts import NAME_MIN_LENGTH -from openedx.core.djangoapps.user_api.accounts.api import get_account_settings, update_account_settings +from openedx.core.djangoapps.user_api.accounts.api import update_account_settings from openedx.core.djangoapps.user_api.errors import UserNotFound, AccountValidationError from openedx.core.djangoapps.credit.api import set_credit_requirement_status from student.models import CourseEnrollment @@ -51,12 +52,11 @@ from verify_student.models import ( SoftwareSecurePhotoVerification, VerificationCheckpoint, VerificationStatus, - InCourseReverificationConfiguration ) +from verify_student.image import decode_image_data, InvalidImageData from util.json_request import JsonResponse from util.date_utils import get_default_time_display from xmodule.modulestore.django import modulestore -from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem from staticfiles.storage import staticfiles_storage @@ -64,7 +64,8 @@ log = logging.getLogger(__name__) class PayAndVerifyView(View): - """View for the "verify and pay" flow. + """ + View for the "verify and pay" flow. This view is somewhat complicated, because the user can enter it from a number of different places: @@ -234,9 +235,9 @@ class PayAndVerifyView(View): course_key = CourseKey.from_string(course_id) course = modulestore().get_course(course_key) - # Verify that the course exists and has a verified mode + # Verify that the course exists if course is None: - log.warn(u"No course specified for verification flow request.") + log.warn(u"Could not find course with ID %s.", course_id) raise Http404 # Check whether the user has access to this course @@ -399,6 +400,7 @@ class PayAndVerifyView(View): 'contribution_amount': contribution_amount, 'course': course, 'course_key': unicode(course_key), + 'checkpoint_location': request.GET.get('checkpoint'), 'course_mode': relevant_course_mode, 'courseware_url': courseware_url, 'current_step': current_step, @@ -800,35 +802,169 @@ def create_order(request): return HttpResponse(json.dumps(payment_data), content_type="application/json") -@require_POST -@login_required -def submit_photos_for_verification(request): - """Submit a photo verification attempt. - - Arguments: - request (HttpRequest): The request to submit photos. - - Returns: - HttpResponse: 200 on success, 400 if there are errors. - +class SubmitPhotosView(View): + """ + End-point for submitting photos for verification. """ - # Check the required parameters - missing_params = set(['face_image', 'photo_id_image']) - set(request.POST.keys()) - if len(missing_params) > 0: - msg = _("Missing required parameters: {missing}").format(missing=", ".join(missing_params)) - return HttpResponseBadRequest(msg) - # If the user already has valid or pending request, the UI will hide - # the verification steps. For this reason, we reject any requests - # for users that already have a valid or pending verification. - if SoftwareSecurePhotoVerification.user_has_valid_or_pending(request.user): - return HttpResponseBadRequest(_("You already have a valid or pending verification.")) + @method_decorator(login_required) + def post(self, request): + """ + Submit photos for verification. - # If the user wants to change his/her full name, - # then try to do that before creating the attempt. - if request.POST.get('full_name'): + This end-point is used for the following cases: + + * Initial verification through the pay-and-verify flow. + * Initial verification initiated from a checkpoint within a course. + * Re-verification initiated from a checkpoint within a course. + + POST Parameters: + + face_image (str): base64-encoded image data of the user's face. + photo_id_image (str): base64-encoded image data of the user's photo ID. + full_name (str): The user's full name, if the user is requesting a name change as well. + course_key (str): Identifier for the course, if initiated from a checkpoint. + checkpoint (str): Location of the checkpoint in the course. + + """ + # If the user already has an initial verification attempt, we can re-use the photo ID + # the user submitted with the initial attempt. This is useful for the in-course reverification + # case in which users submit only the face photo and have it matched against their ID photos + # submitted with the initial verification. + initial_verification = SoftwareSecurePhotoVerification.get_initial_verification(request.user) + + # Validate the POST parameters + params, response = self._validate_parameters(request, bool(initial_verification)) + if response is not None: + return response + + # If necessary, update the user's full name + if "full_name" in params: + response = self._update_full_name(request.user, params["full_name"]) + if response is not None: + return response + + # Retrieve the image data + # Validation ensures that we'll have a face image, but we may not have + # a photo ID image if this is a reverification. + face_image, photo_id_image, response = self._decode_image_data( + params["face_image"], params.get("photo_id_image") + ) + if response is not None: + return response + + # Submit the attempt + attempt = self._submit_attempt(request.user, face_image, photo_id_image, initial_verification) + + # If this attempt was submitted at a checkpoint, then associate + # the attempt with the checkpoint. + submitted_at_checkpoint = "checkpoint" in params and "course_key" in params + if submitted_at_checkpoint: + checkpoint = self._associate_attempt_with_checkpoint( + request.user, attempt, + params["course_key"], + params["checkpoint"] + ) + + # If the submission came from an in-course checkpoint + if initial_verification is not None and submitted_at_checkpoint: + self._fire_event(request.user, "edx.bi.reverify.submitted", { + "category": "verification", + "label": unicode(params["course_key"]), + "checkpoint": checkpoint.checkpoint_name, + }) + + # Send a URL that the client can redirect to in order + # to return to the checkpoint in the courseware. + redirect_url = get_redirect_url(params["course_key"], params["checkpoint"]) + return JsonResponse({"url": redirect_url}) + + # Otherwise, the submission came from an initial verification flow. + else: + self._fire_event(request.user, "edx.bi.verify.submitted", {"category": "verification"}) + self._send_confirmation_email(request.user) + redirect_url = None + return JsonResponse({}) + + def _validate_parameters(self, request, has_initial_verification): + """ + Check that the POST parameters are valid. + + Arguments: + request (HttpRequest): The request object. + has_initial_verification (bool): Whether the user has an initial verification attempt. + + Returns: + HttpResponse or None + + """ + # Pull out the parameters we care about. + params = { + param_name: request.POST[param_name] + for param_name in [ + "face_image", + "photo_id_image", + "course_key", + "checkpoint", + "full_name" + ] + if param_name in request.POST + } + + # If the user already has an initial verification attempt, then we don't + # require the user to submit a photo ID image, since we can re-use the photo ID + # image from the initial attempt. + # If we don't have an initial verification OR a photo ID image, something has gone + # terribly wrong in the JavaScript. Log this as an error so we can track it down. + if "photo_id_image" not in params and not has_initial_verification: + log.error( + ( + "User %s does not have an initial verification attempt " + "and no photo ID image data was provided. " + "This most likely means that the JavaScript client is not " + "correctly constructing the request to submit photos." + ), request.user.id + ) + return None, HttpResponseBadRequest( + _("Photo ID image is required if the user does not have an initial verification attempt.") + ) + + # The face image is always required. + if "face_image" not in params: + msg = _("Missing required parameter face_image") + return None, HttpResponseBadRequest(msg) + + # If provided, parse the course key and checkpoint location + if "course_key" in params: + try: + params["course_key"] = CourseKey.from_string(params["course_key"]) + except InvalidKeyError: + return None, HttpResponseBadRequest(_("Invalid course key")) + + if "checkpoint" in params: + try: + params["checkpoint"] = UsageKey.from_string(params["checkpoint"]).replace( + course_key=params["course_key"] + ) + except InvalidKeyError: + return None, HttpResponseBadRequest(_("Invalid checkpoint location")) + + return params, None + + def _update_full_name(self, user, full_name): + """ + Update the user's full name. + + Arguments: + user (User): The user to update. + full_name (unicode): The user's updated full name. + + Returns: + HttpResponse or None + + """ try: - update_account_settings(request.user, {"name": request.POST.get('full_name')}) + update_account_settings(user, {"name": full_name}) except UserNotFound: return HttpResponseBadRequest(_("No profile found for user")) except AccountValidationError: @@ -837,38 +973,131 @@ def submit_photos_for_verification(request): ).format(min_length=NAME_MIN_LENGTH) return HttpResponseBadRequest(msg) - # Create the attempt - attempt = SoftwareSecurePhotoVerification(user=request.user) - try: - b64_face_image = request.POST['face_image'].split(",")[1] - b64_photo_id_image = request.POST['photo_id_image'].split(",")[1] - except IndexError: - msg = _("Image data is not valid.") - return HttpResponseBadRequest(msg) + def _decode_image_data(self, face_data, photo_id_data=None): + """ + Decode image data sent with the request. - attempt.upload_face_image(b64_face_image.decode('base64')) - attempt.upload_photo_id_image(b64_photo_id_image.decode('base64')) - attempt.mark_ready() - attempt.submit() + Arguments: + face_data (str): base64-encoded face image data. - log.info(u"Submitted initial verification attempt for user %s", request.user.id) + Keyword Arguments: + photo_id_data (str): base64-encoded photo ID image data. - account_settings = get_account_settings(request.user) + Returns: + tuple of (str, str, HttpResponse) - # Send a confirmation email to the user - context = { - 'full_name': account_settings['name'], - 'platform_name': settings.PLATFORM_NAME - } + """ + try: + # Decode face image data (used for both an initial and re-verification) + face_image = decode_image_data(face_data) - subject = _("Verification photos received") - message = render_to_string('emails/photo_submission_confirmation.txt', context) - from_address = microsite.get_value('default_from_email', settings.DEFAULT_FROM_EMAIL) - to_address = account_settings['email'] + # Decode the photo ID image data if it's provided + photo_id_image = ( + decode_image_data(photo_id_data) + if photo_id_data is not None else None + ) - send_mail(subject, message, from_address, [to_address], fail_silently=False) + return face_image, photo_id_image, None - return HttpResponse(200) + except InvalidImageData: + msg = _("Image data is not valid.") + return None, None, HttpResponseBadRequest(msg) + + def _submit_attempt(self, user, face_image, photo_id_image=None, initial_verification=None): + """ + Submit a verification attempt. + + Arguments: + user (User): The user making the attempt. + face_image (str): Decoded face image data. + + Keyword Arguments: + photo_id_image (str or None): Decoded photo ID image data. + initial_verification (SoftwareSecurePhotoVerification): The initial verification attempt. + """ + attempt = SoftwareSecurePhotoVerification(user=user) + + # We will always have face image data, so upload the face image + attempt.upload_face_image(face_image) + + # If an ID photo wasn't submitted, re-use the ID photo from the initial attempt. + # Earlier validation rules ensure that at least one of these is available. + if photo_id_image is not None: + attempt.upload_photo_id_image(photo_id_image) + elif initial_verification is None: + # Earlier validation should ensure that we never get here. + log.error( + "Neither a photo ID image or initial verification attempt provided. " + "Parameter validation in the view should prevent this from happening!" + ) + + # Submit the attempt + attempt.mark_ready() + attempt.submit(copy_id_photo_from=initial_verification) + + return attempt + + def _associate_attempt_with_checkpoint(self, user, attempt, course_key, usage_id): + """ + Associate the verification attempt with a checkpoint within a course. + + Arguments: + user (User): The user making the attempt. + attempt (SoftwareSecurePhotoVerification): The verification attempt. + course_key (CourseKey): The identifier for the course. + usage_key (UsageKey): The location of the checkpoint within the course. + + Returns: + VerificationCheckpoint + """ + checkpoint = VerificationCheckpoint.get_or_create_verification_checkpoint(course_key, usage_id) + checkpoint.add_verification_attempt(attempt) + VerificationStatus.add_verification_status(checkpoint, user, "submitted") + return checkpoint + + def _send_confirmation_email(self, user): + """ + Send an email confirming that the user submitted photos + for initial verification. + """ + context = { + 'full_name': user.profile.name, + 'platform_name': microsite.get_value("PLATFORM_NAME", settings.PLATFORM_NAME) + } + + subject = _("Verification photos received") + message = render_to_string('emails/photo_submission_confirmation.txt', context) + from_address = microsite.get_value('default_from_email', settings.DEFAULT_FROM_EMAIL) + to_address = user.email + + try: + send_mail(subject, message, from_address, [to_address], fail_silently=False) + except: # pylint: disable=bare-except + # We catch all exceptions and log them. + # It would be much, much worse to roll back the transaction due to an uncaught + # exception than to skip sending the notification email. + log.exception("Could not send notification email for initial verification for user %s", user.id) + + def _fire_event(self, user, event_name, parameters): + """ + Fire an analytics event. + + Arguments: + user (User): The user who submitted photos. + event_name (str): Name of the analytics event. + parameters (dict): Event parameters. + + Returns: None + + """ + if settings.FEATURES.get('SEGMENT_IO_LMS') and hasattr(settings, 'SEGMENT_IO_LMS_KEY'): + tracking_context = tracker.get_tracker().resolve_context() + context = { + 'Google Analytics': { + 'clientId': tracking_context.get('client_id') + } + } + analytics.track(user.id, event_name, parameters, context=context) def _compose_message_reverification_email( @@ -1066,21 +1295,22 @@ def results_callback(request): return HttpResponseBadRequest( "Result {} not understood. Known results: PASS, FAIL, SYSTEM FAIL".format(result) ) - incourse_reverify_enabled = InCourseReverificationConfiguration.current().enabled - if incourse_reverify_enabled: - checkpoints = VerificationCheckpoint.objects.filter(photo_verification=attempt).all() - VerificationStatus.add_status_from_checkpoints(checkpoints=checkpoints, user=attempt.user, status=status) - # If this is re-verification then send the update email - if checkpoints: - user_id = attempt.user.id - course_key = checkpoints[0].course_id - related_assessment_location = checkpoints[0].checkpoint_location - subject, message = _compose_message_reverification_email( - course_key, user_id, related_assessment_location, status, request - ) + checkpoints = VerificationCheckpoint.objects.filter(photo_verification=attempt).all() + VerificationStatus.add_status_from_checkpoints(checkpoints=checkpoints, user=attempt.user, status=status) + + # If this is re-verification then send the update email + if checkpoints: + user_id = attempt.user.id + course_key = checkpoints[0].course_id + related_assessment_location = checkpoints[0].checkpoint_location + + subject, message = _compose_message_reverification_email( + course_key, user_id, related_assessment_location, status, request + ) + + _send_email(user_id, subject, message) - _send_email(user_id, subject, message) return HttpResponse("OK!") @@ -1145,17 +1375,6 @@ class InCourseReverifyView(View): Returns: HttpResponse """ - # Check that in-course re-verification is enabled or not - incourse_reverify_enabled = InCourseReverificationConfiguration.current().enabled - - if not incourse_reverify_enabled: - log.error( - u"In-course reverification is not enabled. " - u"You can enable it in Django admin by setting " - u"InCourseReverificationConfiguration to enabled." - ) - raise Http404 - user = request.user course_key = CourseKey.from_string(course_id) course = modulestore().get_course(course_key) @@ -1163,8 +1382,9 @@ class InCourseReverifyView(View): log.error(u"Could not find course '%s' for in-course reverification.", course_key) raise Http404 - checkpoint = VerificationCheckpoint.get_verification_checkpoint(course_key, usage_id) - if checkpoint is None: + try: + checkpoint = VerificationCheckpoint.objects.get(course_id=course_key, checkpoint_location=usage_id) + except VerificationCheckpoint.DoesNotExist: log.error( u"No verification checkpoint exists for the " u"course '%s' and checkpoint location '%s'.", @@ -1174,7 +1394,7 @@ class InCourseReverifyView(View): initial_verification = SoftwareSecurePhotoVerification.get_initial_verification(user) if not initial_verification: - return self._redirect_no_initial_verification(user, course_key) + return self._redirect_to_initial_verification(user, course_key, usage_id) # emit the reverification event self._track_reverification_events('edx.bi.reverify.started', user.id, course_id, checkpoint.checkpoint_name) @@ -1189,86 +1409,6 @@ class InCourseReverifyView(View): } return render_to_response("verify_student/incourse_reverify.html", context) - @method_decorator(login_required) - def post(self, request, course_id, usage_id): - """Submits the re-verification attempt to SoftwareSecure. - - Args: - request(HttpRequest): HttpRequest object - course_id(str): Course Id - usage_id(str): Location of Reverification XBlock in courseware - - Returns: - HttpResponse with status_code 400 if photo is missing or any error - or redirect to the verification flow if initial verification - doesn't exist otherwise HttpResponse with status code 200 - """ - # Check the in-course re-verification is enabled or not - incourse_reverify_enabled = InCourseReverificationConfiguration.current().enabled - if not incourse_reverify_enabled: - raise Http404 - - user = request.user - try: - course_key = CourseKey.from_string(course_id) - usage_key = UsageKey.from_string(usage_id).replace(course_key=course_key) - except InvalidKeyError: - raise Http404(u"Invalid course_key or usage_key") - - course = modulestore().get_course(course_key) - if course is None: - log.error(u"Invalid course id '%s'", course_id) - return HttpResponseBadRequest(_("Invalid course location.")) - - checkpoint = VerificationCheckpoint.get_verification_checkpoint(course_key, usage_id) - if checkpoint is None: - log.error( - u"Checkpoint is not defined. Could not submit verification attempt" - u" for user '%s', course '%s' and checkpoint location '%s'.", - request.user.id, course_key, usage_id - ) - return HttpResponseBadRequest(_("Invalid checkpoint location.")) - - init_verification = SoftwareSecurePhotoVerification.get_initial_verification(user) - if not init_verification: - return self._redirect_no_initial_verification(user, course_key) - - try: - attempt = SoftwareSecurePhotoVerification.submit_faceimage( - request.user, request.POST['face_image'], init_verification.photo_id_key - ) - checkpoint.add_verification_attempt(attempt) - VerificationStatus.add_verification_status(checkpoint, user, "submitted") - - # emit the reverification event - self._track_reverification_events( - 'edx.bi.reverify.submitted', - user.id, course_id, checkpoint.checkpoint_name - ) - - redirect_url = get_redirect_url(course_key, usage_key) - response = JsonResponse({'url': redirect_url}) - - except (ItemNotFoundError, NoPathToItem): - log.warning(u"Could not find redirect URL for location %s in course %s", course_key, usage_key) - redirect_url = reverse("courseware", args=(unicode(course_key),)) - response = JsonResponse({'url': redirect_url}) - - except Http404 as expt: - log.exception("Invalid location during photo verification.") - response = HttpResponseBadRequest(expt.message) - - except IndexError: - log.exception("Invalid image data during photo verification.") - response = HttpResponseBadRequest(_("Invalid image data during photo verification.")) - - except Exception: # pylint: disable=broad-except - log.exception("Could not submit verification attempt for user %s.", request.user.id) - msg = _("Could not submit photos") - response = HttpResponseBadRequest(msg) - - return response - def _track_reverification_events(self, event_name, user_id, course_id, checkpoint): # pylint: disable=invalid-name """Track re-verification events for a user against a reverification checkpoint of a course. @@ -1304,31 +1444,35 @@ class InCourseReverifyView(View): } ) - def _redirect_no_initial_verification(self, user, course_key): - """Redirect because the user does not have an initial verification. + def _redirect_to_initial_verification(self, user, course_key, checkpoint): + """ + Redirect because the user does not have an initial verification. - NOTE: currently, we assume that courses are configured such that - the first re-verification always occurs AFTER the initial verification - deadline. Later, we may want to allow users to upgrade to a verified - track, then submit an initial verification that also counts - as a verification for the checkpoint in the course. + We will redirect the user to the initial verification flow, + passing the identifier for this checkpoint. When the user + submits a verification attempt, it will count for *both* + the initial and checkpoint verification. Arguments: user (User): The user who made the request. course_key (CourseKey): The identifier for the course for which the user is attempting to re-verify. + checkpoint (string): Location of the checkpoint in the courseware. Returns: HttpResponse """ - log.warning( + log.info( u"User %s does not have an initial verification, so " u"he/she will be redirected to the \"verify later\" flow " u"for the course %s.", user.id, course_key ) - return redirect(reverse('verify_student_verify_now', kwargs={'course_id': unicode(course_key)})) + base_url = reverse('verify_student_verify_now', kwargs={'course_id': unicode(course_key)}) + params = urllib.urlencode({"checkpoint": checkpoint}) + full_url = u"{base}?{params}".format(base=base_url, params=params) + return redirect(full_url) class VerifyLaterView(RedirectView): diff --git a/lms/envs/common.py b/lms/envs/common.py index 200533ac55..2d1185a048 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -1331,7 +1331,7 @@ incourse_reverify_js = [ 'js/verify_student/views/error_view.js', 'js/verify_student/views/image_input_view.js', 'js/verify_student/views/webcam_photo_view.js', - 'js/verify_student/models/reverification_model.js', + 'js/verify_student/models/verification_model.js', 'js/verify_student/views/incourse_reverify_view.js', 'js/verify_student/incourse_reverify.js', ] diff --git a/lms/static/js/verify_student/models/reverification_model.js b/lms/static/js/verify_student/models/reverification_model.js deleted file mode 100644 index def5f8c110..0000000000 --- a/lms/static/js/verify_student/models/reverification_model.js +++ /dev/null @@ -1,51 +0,0 @@ -/** - * Model for a reverification attempt. - * - * The re-verification model is responsible for - * storing face photo image data and submitting - * it back to the server. - */ -var edx = edx || {}; - -(function( $, _, Backbone ) { - 'use strict'; - - edx.verify_student = edx.verify_student || {}; - - edx.verify_student.ReverificationModel = Backbone.Model.extend({ - - defaults: { - courseKey: '', - faceImage: '', - usageId: '' - }, - - sync: function( method ) { - var model = this; - var headers = { 'X-CSRFToken': $.cookie( 'csrftoken' ) }, - data = { - face_image: model.get( 'faceImage' ) - }, - url = _.str.sprintf( - '/verify_student/reverify/%(courseKey)s/%(usageId)s/', { - courseKey: model.get('courseKey'), - usageId: model.get('usageId') - } - ); - - $.ajax({ - url: url, - type: 'POST', - data: data, - headers: headers, - success: function(response) { - model.trigger( 'sync', response.url); - }, - error: function( error ) { - model.trigger( 'error', error ); - } - }); - } - }); - -})( jQuery, _, Backbone ); diff --git a/lms/static/js/verify_student/models/verification_model.js b/lms/static/js/verify_student/models/verification_model.js index e9dc0a16a1..afe9763564 100644 --- a/lms/static/js/verify_student/models/verification_model.js +++ b/lms/static/js/verify_student/models/verification_model.js @@ -8,7 +8,7 @@ */ var edx = edx || {}; - (function( $, _, Backbone ) { + (function( $, Backbone ) { 'use strict'; edx.verify_student = edx.verify_student || {}; @@ -16,22 +16,40 @@ edx.verify_student.VerificationModel = Backbone.Model.extend({ defaults: { + // If provided, change the user's full name when submitting photos. fullName: null, + + // Image data for the user's face photo. faceImage: "", - identificationImage: "" + + // Image data for the user's ID photo. + // In the case of an in-course reverification, we won't + // need to send this because we'll send the ID photo that + // the user submitted with the initial verification attempt. + identificationImage: null, + + // If the verification attempt is associated with a checkpoint + // in a course, we send the the course and checkpoint location. + courseKey: null, + checkpoint: null, }, sync: function( method, model ) { var headers = { 'X-CSRFToken': $.cookie( 'csrftoken' ) }, - data = { - face_image: model.get( 'faceImage' ), - photo_id_image: model.get( 'identificationImage' ) - }; + data = {}; + + data.face_image = model.get("faceImage"); + + // The ID photo is optional, since in-course reverification + // re-uses the image from the initial verification attempt. + if (model.has("identificationImage")) { + data.photo_id_image = model.get("identificationImage"); + } // Full name is an optional parameter; if not provided, // it won't be changed. - if ( !_.isEmpty( model.get( 'fullName' ) ) ) { - data.full_name = model.get( 'fullName' ); + if (model.has("fullName")) { + data.full_name = model.get("fullName"); // Track the user's decision to change the name on their account window.analytics.track( 'edx.bi.user.full_name.changed', { @@ -39,6 +57,14 @@ }); } + // If the user entered the verification flow from a checkpoint + // in a course, we need to send the course and checkpoint + // location to associate the attempt with the checkpoint. + if (model.has("courseKey") && model.has("checkpoint")) { + data.course_key = model.get("courseKey"); + data.checkpoint = model.get("checkpoint"); + } + // Submit the request to the server, // triggering events on success and error. $.ajax({ @@ -46,8 +72,8 @@ type: 'POST', data: data, headers: headers, - success: function() { - model.trigger( 'sync' ); + success: function( response ) { + model.trigger( 'sync', response.url ); }, error: function( error ) { model.trigger( 'error', error ); @@ -56,4 +82,4 @@ } }); - })( jQuery, _, Backbone ); + })( jQuery, Backbone ); diff --git a/lms/static/js/verify_student/pay_and_verify.js b/lms/static/js/verify_student/pay_and_verify.js index 65d4781b2e..3503afc7db 100644 --- a/lms/static/js/verify_student/pay_and_verify.js +++ b/lms/static/js/verify_student/pay_and_verify.js @@ -34,6 +34,8 @@ var edx = edx || {}; errorModel: errorView.model, displaySteps: el.data('display-steps'), currentStep: el.data('current-step'), + courseKey: el.data('course-key'), + checkpointLocation: el.data('checkpoint-location'), stepInfo: { 'intro-step': { courseName: el.data('course-name'), diff --git a/lms/static/js/verify_student/views/incourse_reverify_view.js b/lms/static/js/verify_student/views/incourse_reverify_view.js index 1196816625..110f682bd8 100644 --- a/lms/static/js/verify_student/views/incourse_reverify_view.js +++ b/lms/static/js/verify_student/views/incourse_reverify_view.js @@ -30,9 +30,9 @@ this.usageId = obj.usageId || null; - this.model = new edx.verify_student.ReverificationModel({ + this.model = new edx.verify_student.VerificationModel({ courseKey: this.courseKey, - usageId: this.usageId + checkpoint: this.usageId }); this.listenTo( this.model, 'sync', _.bind( this.handleSubmitPhotoSuccess, this )); @@ -74,8 +74,7 @@ }, handleSubmitPhotoSuccess: function(redirect_url) { - // Eventually this will be a redirect back into the courseware, - // but for now we can return to the student dashboard. + // Redirect back to the courseware at the checkpoint location window.location.href = redirect_url; }, diff --git a/lms/static/js/verify_student/views/pay_and_verify_view.js b/lms/static/js/verify_student/views/pay_and_verify_view.js index bad7d9e487..5c6239479d 100644 --- a/lms/static/js/verify_student/views/pay_and_verify_view.js +++ b/lms/static/js/verify_student/views/pay_and_verify_view.js @@ -27,6 +27,9 @@ var edx = edx || {}; initialize: function( obj ) { this.errorModel = obj.errorModel || null; this.displaySteps = obj.displaySteps || []; + this.courseKey = obj.courseKey || null; + this.checkpointLocation = obj.checkpointLocation || null; + this.initializeStepViews( obj.stepInfo || {} ); this.currentStepIndex = _.indexOf( _.pluck( this.displaySteps, 'name' ), @@ -61,7 +64,14 @@ var edx = edx || {}; // among the different steps. This allows // one step to save photos and another step // to submit them. - verificationModel = new edx.verify_student.VerificationModel(); + // + // We also pass in the course key and checkpoint location. + // If we've been provided with a checkpoint in the courseware, + // this will associate the verification attempt with the checkpoint. + verificationModel = new edx.verify_student.VerificationModel({ + courseKey: this.courseKey, + checkpoint: this.checkpointLocation + }); for ( i = 0; i < this.displaySteps.length; i++ ) { stepName = this.displaySteps[i].name; diff --git a/lms/templates/verify_student/pay_and_verify.html b/lms/templates/verify_student/pay_and_verify.html index 9c02e5f40a..79361d1b72 100644 --- a/lms/templates/verify_student/pay_and_verify.html +++ b/lms/templates/verify_student/pay_and_verify.html @@ -75,6 +75,13 @@ from verify_student.views import PayAndVerifyView data-already-verified='${already_verified}' data-verification-good-until='${verification_good_until}' data-capture-sound='${capture_sound}' + + # If we reached the verification flow from an in-course checkpoint, + # then pass the checkpoint location in so that we can associate + # the attempt with the checkpoint on submission. + % if checkpoint_location is not None: + data-checkpoint-location='${checkpoint_location}' + % endif > % if is_active: diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f3f4239b68..229219ca91 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -136,6 +136,7 @@ freezegun==0.1.11 lettuce==0.2.20 mock-django==0.6.6 mock==1.0.1 +moto==0.3.1 nose-exclude nose-ignore-docstring nosexcover==1.0.7