Commit 47b81c92 by Will Daly

Allow users to submit initial verification at in-course checkpoints.

* Combine verification and reverification photo submission end-points.
* Combine verification and reverification Backbone models.
* Allow deletion of verification status in Django admin.
* Initial verification from a checkpoint is associated with the checkpoint.
* Fix critical bug in which an invalid link was sent to Software Secure
  for the photo ID image of reverification attempts.
parent cb217563
from ratelimitbackend import admin from ratelimitbackend import admin
from config_models.admin import ConfigurationModelAdmin
from verify_student.models import ( from verify_student.models import (
SoftwareSecurePhotoVerification, SoftwareSecurePhotoVerification,
InCourseReverificationConfiguration,
VerificationStatus, VerificationStatus,
SkippedReverification, SkippedReverification,
) )
...@@ -40,10 +38,6 @@ class VerificationStatusAdmin(admin.ModelAdmin): ...@@ -40,10 +38,6 @@ class VerificationStatusAdmin(admin.ModelAdmin):
return self.readonly_fields + ('status', 'checkpoint', 'user', 'response', 'error') return self.readonly_fields + ('status', 'checkpoint', 'user', 'response', 'error')
return self.readonly_fields 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): class SkippedReverificationAdmin(admin.ModelAdmin):
"""Admin for the SkippedReverification table. """ """Admin for the SkippedReverification table. """
...@@ -58,6 +52,5 @@ class SkippedReverificationAdmin(admin.ModelAdmin): ...@@ -58,6 +52,5 @@ class SkippedReverificationAdmin(admin.ModelAdmin):
admin.site.register(SoftwareSecurePhotoVerification, SoftwareSecurePhotoVerificationAdmin) admin.site.register(SoftwareSecurePhotoVerification, SoftwareSecurePhotoVerificationAdmin)
admin.site.register(InCourseReverificationConfiguration, ConfigurationModelAdmin)
admin.site.register(SkippedReverification, SkippedReverificationAdmin) admin.site.register(SkippedReverification, SkippedReverificationAdmin)
admin.site.register(VerificationStatus, VerificationStatusAdmin) admin.site.register(VerificationStatus, VerificationStatusAdmin)
"""
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
...@@ -60,10 +60,9 @@ class ReverificationService(object): ...@@ -60,10 +60,9 @@ class ReverificationService(object):
Re-verification link Re-verification link
""" """
course_key = CourseKey.from_string(course_id) course_key = CourseKey.from_string(course_id)
VerificationCheckpoint.objects.get_or_create(
course_id=course_key, # Get-or-create the verification checkpoint
checkpoint_location=related_assessment_location VerificationCheckpoint.get_or_create_verification_checkpoint(course_key, related_assessment_location)
)
re_verification_link = reverse( re_verification_link = reverse(
'verify_student_incourse_reverify', 'verify_student_incourse_reverify',
......
...@@ -40,7 +40,7 @@ class SoftwareSecureFakeView(View): ...@@ -40,7 +40,7 @@ class SoftwareSecureFakeView(View):
} }
try: 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 context["receipt_id"] = most_recent.receipt_id
except: # pylint: disable=bare-except except: # pylint: disable=bare-except
pass pass
......
...@@ -220,18 +220,6 @@ class TestPhotoVerification(ModuleStoreTestCase): ...@@ -220,18 +220,6 @@ class TestPhotoVerification(ModuleStoreTestCase):
return attempt 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): def test_submissions(self):
"""Test that we set our status correctly after a submission.""" """Test that we set our status correctly after a submission."""
# Basic case, things go well. # Basic case, things go well.
...@@ -501,7 +489,7 @@ class VerificationCheckpointTest(ModuleStoreTestCase): ...@@ -501,7 +489,7 @@ class VerificationCheckpointTest(ModuleStoreTestCase):
) )
@ddt.data('midterm', 'final') @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. Test that a reverification checkpoint is created properly.
""" """
...@@ -514,26 +502,40 @@ class VerificationCheckpointTest(ModuleStoreTestCase): ...@@ -514,26 +502,40 @@ class VerificationCheckpointTest(ModuleStoreTestCase):
checkpoint_location=checkpoint_location checkpoint_location=checkpoint_location
) )
self.assertEqual( self.assertEqual(
VerificationCheckpoint.get_verification_checkpoint(self.course.id, checkpoint_location), VerificationCheckpoint.get_or_create_verification_checkpoint(self.course.id, checkpoint_location),
verification_checkpoint verification_checkpoint
) )
def test_get_verification_checkpoint_for_not_existing_values(self): def test_get_or_create_verification_checkpoint_for_not_existing_values(self):
"""Test that 'get_verification_checkpoint' method returns None if user # Retrieving a checkpoint that doesn't yet exist will create it
tries to access a checkpoint with an invalid location. location = u'i4x://edX/DemoX/edx-reverification-block/invalid_location'
""" checkpoint = VerificationCheckpoint.get_or_create_verification_checkpoint(self.course.id, location)
# create the VerificationCheckpoint checkpoint
VerificationCheckpoint.objects.create(course_id=self.course.id, checkpoint_location=self.checkpoint_midterm)
# get verification for a non existing checkpoint self.assertIsNot(checkpoint, None)
self.assertEqual( self.assertEqual(checkpoint.course_id, self.course.id)
VerificationCheckpoint.get_verification_checkpoint( 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.course.id,
u'i4x://edX/DemoX/edx-reverification-block/invalid_location' self.checkpoint_midterm
),
None
) )
# 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): def test_unique_together_constraint(self):
""" """
Test the unique together constraint. Test the unique together constraint.
......
...@@ -91,7 +91,7 @@ urlpatterns = patterns( ...@@ -91,7 +91,7 @@ urlpatterns = patterns(
url( url(
r'^submit-photos/$', r'^submit-photos/$',
views.submit_photos_for_verification, views.SubmitPhotosView.as_view(),
name="verify_student_submit_photos" name="verify_student_submit_photos"
), ),
......
...@@ -1331,7 +1331,7 @@ incourse_reverify_js = [ ...@@ -1331,7 +1331,7 @@ incourse_reverify_js = [
'js/verify_student/views/error_view.js', 'js/verify_student/views/error_view.js',
'js/verify_student/views/image_input_view.js', 'js/verify_student/views/image_input_view.js',
'js/verify_student/views/webcam_photo_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/views/incourse_reverify_view.js',
'js/verify_student/incourse_reverify.js', 'js/verify_student/incourse_reverify.js',
] ]
......
/**
* 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 );
...@@ -8,7 +8,7 @@ ...@@ -8,7 +8,7 @@
*/ */
var edx = edx || {}; var edx = edx || {};
(function( $, _, Backbone ) { (function( $, Backbone ) {
'use strict'; 'use strict';
edx.verify_student = edx.verify_student || {}; edx.verify_student = edx.verify_student || {};
...@@ -16,22 +16,40 @@ ...@@ -16,22 +16,40 @@
edx.verify_student.VerificationModel = Backbone.Model.extend({ edx.verify_student.VerificationModel = Backbone.Model.extend({
defaults: { defaults: {
// If provided, change the user's full name when submitting photos.
fullName: null, fullName: null,
// Image data for the user's face photo.
faceImage: "", 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 ) { sync: function( method, model ) {
var headers = { 'X-CSRFToken': $.cookie( 'csrftoken' ) }, var headers = { 'X-CSRFToken': $.cookie( 'csrftoken' ) },
data = { data = {};
face_image: model.get( 'faceImage' ),
photo_id_image: model.get( 'identificationImage' ) 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, // Full name is an optional parameter; if not provided,
// it won't be changed. // it won't be changed.
if ( !_.isEmpty( model.get( 'fullName' ) ) ) { if (model.has("fullName")) {
data.full_name = model.get( 'fullName' ); data.full_name = model.get("fullName");
// Track the user's decision to change the name on their account // Track the user's decision to change the name on their account
window.analytics.track( 'edx.bi.user.full_name.changed', { window.analytics.track( 'edx.bi.user.full_name.changed', {
...@@ -39,6 +57,14 @@ ...@@ -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, // Submit the request to the server,
// triggering events on success and error. // triggering events on success and error.
$.ajax({ $.ajax({
...@@ -46,8 +72,8 @@ ...@@ -46,8 +72,8 @@
type: 'POST', type: 'POST',
data: data, data: data,
headers: headers, headers: headers,
success: function() { success: function( response ) {
model.trigger( 'sync' ); model.trigger( 'sync', response.url );
}, },
error: function( error ) { error: function( error ) {
model.trigger( 'error', error ); model.trigger( 'error', error );
...@@ -56,4 +82,4 @@ ...@@ -56,4 +82,4 @@
} }
}); });
})( jQuery, _, Backbone ); })( jQuery, Backbone );
...@@ -34,6 +34,8 @@ var edx = edx || {}; ...@@ -34,6 +34,8 @@ var edx = edx || {};
errorModel: errorView.model, errorModel: errorView.model,
displaySteps: el.data('display-steps'), displaySteps: el.data('display-steps'),
currentStep: el.data('current-step'), currentStep: el.data('current-step'),
courseKey: el.data('course-key'),
checkpointLocation: el.data('checkpoint-location'),
stepInfo: { stepInfo: {
'intro-step': { 'intro-step': {
courseName: el.data('course-name'), courseName: el.data('course-name'),
......
...@@ -30,9 +30,9 @@ ...@@ -30,9 +30,9 @@
this.usageId = obj.usageId || null; this.usageId = obj.usageId || null;
this.model = new edx.verify_student.ReverificationModel({ this.model = new edx.verify_student.VerificationModel({
courseKey: this.courseKey, courseKey: this.courseKey,
usageId: this.usageId checkpoint: this.usageId
}); });
this.listenTo( this.model, 'sync', _.bind( this.handleSubmitPhotoSuccess, this )); this.listenTo( this.model, 'sync', _.bind( this.handleSubmitPhotoSuccess, this ));
...@@ -74,8 +74,7 @@ ...@@ -74,8 +74,7 @@
}, },
handleSubmitPhotoSuccess: function(redirect_url) { handleSubmitPhotoSuccess: function(redirect_url) {
// Eventually this will be a redirect back into the courseware, // Redirect back to the courseware at the checkpoint location
// but for now we can return to the student dashboard.
window.location.href = redirect_url; window.location.href = redirect_url;
}, },
......
...@@ -27,6 +27,9 @@ var edx = edx || {}; ...@@ -27,6 +27,9 @@ var edx = edx || {};
initialize: function( obj ) { initialize: function( obj ) {
this.errorModel = obj.errorModel || null; this.errorModel = obj.errorModel || null;
this.displaySteps = obj.displaySteps || []; this.displaySteps = obj.displaySteps || [];
this.courseKey = obj.courseKey || null;
this.checkpointLocation = obj.checkpointLocation || null;
this.initializeStepViews( obj.stepInfo || {} ); this.initializeStepViews( obj.stepInfo || {} );
this.currentStepIndex = _.indexOf( this.currentStepIndex = _.indexOf(
_.pluck( this.displaySteps, 'name' ), _.pluck( this.displaySteps, 'name' ),
...@@ -61,7 +64,14 @@ var edx = edx || {}; ...@@ -61,7 +64,14 @@ var edx = edx || {};
// among the different steps. This allows // among the different steps. This allows
// one step to save photos and another step // one step to save photos and another step
// to submit them. // 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++ ) { for ( i = 0; i < this.displaySteps.length; i++ ) {
stepName = this.displaySteps[i].name; stepName = this.displaySteps[i].name;
......
...@@ -75,6 +75,13 @@ from verify_student.views import PayAndVerifyView ...@@ -75,6 +75,13 @@ from verify_student.views import PayAndVerifyView
data-already-verified='${already_verified}' data-already-verified='${already_verified}'
data-verification-good-until='${verification_good_until}' data-verification-good-until='${verification_good_until}'
data-capture-sound='${capture_sound}' 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
></div> ></div>
% if is_active: % if is_active:
......
...@@ -136,6 +136,7 @@ freezegun==0.1.11 ...@@ -136,6 +136,7 @@ freezegun==0.1.11
lettuce==0.2.20 lettuce==0.2.20
mock-django==0.6.6 mock-django==0.6.6
mock==1.0.1 mock==1.0.1
moto==0.3.1
nose-exclude nose-exclude
nose-ignore-docstring nose-ignore-docstring
nosexcover==1.0.7 nosexcover==1.0.7
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment