diff --git a/lms/djangoapps/verify_student/models.py b/lms/djangoapps/verify_student/models.py index 6a838e0697..98ac86e438 100644 --- a/lms/djangoapps/verify_student/models.py +++ b/lms/djangoapps/verify_student/models.py @@ -140,7 +140,7 @@ class IDVerificationAttempt(StatusModel): """ return ( - self.created_at < deadline and + self.created_at <= deadline and self.expiration_datetime > now() ) @@ -353,11 +353,11 @@ class PhotoVerification(IDVerificationAttempt): return self.error_msg @status_before_must_be("created") - def upload_face_image(self, img): + def upload_face_image(self, img_data): raise NotImplementedError @status_before_must_be("created") - def upload_photo_id_image(self, img): + def upload_photo_id_image(self, img_data): raise NotImplementedError @status_before_must_be("created") @@ -390,7 +390,7 @@ class PhotoVerification(IDVerificationAttempt): # At any point prior to this, they can change their names via their # student dashboard. But at this point, we lock the value into the # attempt. - self.name = self.user.profile.name + self.name = self.user.profile.name # pylint: disable=no-member self.status = self.STATUS.ready self.save() diff --git a/lms/djangoapps/verify_student/services.py b/lms/djangoapps/verify_student/services.py index cc71f22023..e565644b09 100644 --- a/lms/djangoapps/verify_student/services.py +++ b/lms/djangoapps/verify_student/services.py @@ -7,6 +7,7 @@ from itertools import chain from django.conf import settings from django.urls import reverse +from django.utils import timezone from django.utils.translation import ugettext as _ from course_modes.models import CourseMode @@ -16,7 +17,7 @@ from student.models import User from .models import ManualVerification, SoftwareSecurePhotoVerification, SSOVerification from .toggles import redirect_to_idv_microfrontend -from .utils import earliest_allowed_verification_date, most_recent_verification +from .utils import earliest_allowed_verification_date, most_recent_verification, active_verifications log = logging.getLogger(__name__) @@ -80,9 +81,9 @@ class IDVerificationService(object): Return a list of all verifications associated with the given user. """ verifications = [] - for verification in chain(SoftwareSecurePhotoVerification.objects.filter(user=user), - SSOVerification.objects.filter(user=user), - ManualVerification.objects.filter(user=user)): + for verification in chain(SoftwareSecurePhotoVerification.objects.filter(user=user).order_by('-created_at'), + SSOVerification.objects.filter(user=user).order_by('-created_at'), + ManualVerification.objects.filter(user=user).order_by('-created_at')): verifications.append(verification) return verifications @@ -177,22 +178,22 @@ class IDVerificationService(object): 'verification_expiry': '', } - # We need to check the user's most recent attempt. - try: - photo_id_verifications = SoftwareSecurePhotoVerification.objects.filter(user=user).order_by('-updated_at') - sso_id_verifications = SSOVerification.objects.filter(user=user).order_by('-updated_at') - manual_id_verifications = ManualVerification.objects.filter(user=user).order_by('-updated_at') + attempt = None - attempt = most_recent_verification( - photo_id_verifications, - sso_id_verifications, - manual_id_verifications, - 'updated_at' - ) + verifications = active_verifications( + cls.verifications_for_user(user), + timezone.now(), + ) - except IndexError: - # The user has no verification attempts, return the default set of data. - return user_status + if verifications: + attempt = verifications[0] + for verification in verifications: + if verification.status == 'approved': + # Always select the LATEST non-expired approved verification if there is such + if attempt.status != 'approved' or ( + attempt.expiration_datetime < verification.expiration_datetime + ): + attempt = verification if not attempt: return user_status diff --git a/lms/djangoapps/verify_student/tests/test_services.py b/lms/djangoapps/verify_student/tests/test_services.py index 548a85c8b6..e4a1f485ed 100644 --- a/lms/djangoapps/verify_student/tests/test_services.py +++ b/lms/djangoapps/verify_student/tests/test_services.py @@ -7,8 +7,9 @@ from datetime import datetime import ddt from django.conf import settings -from mock import patch +from django.test import TestCase from freezegun import freeze_time +from mock import patch from pytz import utc from lms.djangoapps.verify_student.models import ManualVerification, SoftwareSecurePhotoVerification, SSOVerification @@ -68,64 +69,6 @@ class TestIDVerificationService(ModuleStoreTestCase): attempt.save() self.assertTrue(IDVerificationService.user_has_valid_or_pending(user), status) - def test_user_status(self): - # each part of the test is in it's own 'frozen time' block because the status is dependent on recency of - # verifications and in order to control the recency, we just put everything inside of a frozen time - - # test for correct status when no error returned - user = UserFactory.create() - with freeze_time('2014-12-12'): - status = IDVerificationService.user_status(user) - expected_status = {'status': 'none', 'error': '', 'should_display': True, 'verification_expiry': '', - 'status_date': ''} - self.assertDictEqual(status, expected_status) - - with freeze_time('2015-01-02'): - # test for when photo verification has been created - SoftwareSecurePhotoVerification.objects.create(user=user, status='approved') - status = IDVerificationService.user_status(user) - expected_status = {'status': 'approved', 'error': '', 'should_display': True, 'verification_expiry': '', - 'status_date': datetime.now(utc)} - self.assertDictEqual(status, expected_status) - - with freeze_time('2015-02-02'): - # create another photo verification for the same user, make sure the denial - # is handled properly - SoftwareSecurePhotoVerification.objects.create( - user=user, status='denied', error_msg='[{"photoIdReasons": ["Not provided"]}]' - ) - status = IDVerificationService.user_status(user) - expected_status = { - 'status': 'must_reverify', 'error': ['id_image_missing'], 'should_display': True, 'verification_expiry': '', - 'status_date': '', - } - self.assertDictEqual(status, expected_status) - - with freeze_time('2015-03-02'): - # test for when sso verification has been created - SSOVerification.objects.create(user=user, status='approved') - status = IDVerificationService.user_status(user) - expected_status = {'status': 'approved', 'error': '', 'should_display': False, 'verification_expiry': '', - 'status_date': datetime.now(utc)} - self.assertDictEqual(status, expected_status) - - with freeze_time('2015-04-02'): - # create another sso verification for the same user, make sure the denial - # is handled properly - SSOVerification.objects.create(user=user, status='denied') - status = IDVerificationService.user_status(user) - expected_status = {'status': 'must_reverify', 'error': '', 'should_display': False, 'verification_expiry': '', - 'status_date': ''} - self.assertDictEqual(status, expected_status) - - with freeze_time('2015-05-02'): - # test for when manual verification has been created - ManualVerification.objects.create(user=user, status='approved') - status = IDVerificationService.user_status(user) - expected_status = {'status': 'approved', 'error': '', 'should_display': False, 'verification_expiry': '', - 'status_date': datetime.now(utc)} - self.assertDictEqual(status, expected_status) - @ddt.unpack @ddt.data( {'enrollment_mode': 'honor', 'status': None, 'output': 'N/A'}, @@ -169,3 +112,150 @@ class TestIDVerificationService(ModuleStoreTestCase): expected_user_ids = {user_a.id, user_b.id, user_c.id} self.assertEqual(expected_user_ids, verified_user_ids) + + +@patch.dict(settings.VERIFY_STUDENT, FAKE_SETTINGS) +@ddt.ddt +class TestIDVerificationServiceUserStatus(TestCase): + """ + Tests for the IDVerificationService.user_status() function. + because the status is dependent on recency of + verifications and in order to control the recency, + we just put everything inside of a frozen time + """ + def setUp(self): + super(TestIDVerificationServiceUserStatus, self).setUp() + self.user = UserFactory.create() + + def test_no_verification(self): + with freeze_time('2014-12-12'): + status = IDVerificationService.user_status(self.user) + expected_status = {'status': 'none', 'error': '', 'should_display': True, 'verification_expiry': '', + 'status_date': ''} + self.assertDictEqual(status, expected_status) + + def test_approved_software_secure_verification(self): + with freeze_time('2015-01-02'): + # test for when photo verification has been created + SoftwareSecurePhotoVerification.objects.create(user=self.user, status='approved') + status = IDVerificationService.user_status(self.user) + expected_status = {'status': 'approved', 'error': '', 'should_display': True, 'verification_expiry': '', + 'status_date': datetime.now(utc)} + self.assertDictEqual(status, expected_status) + + def test_denied_software_secure_verification(self): + with freeze_time('2015-02-02'): + # create denied photo verification for the user, make sure the denial + # is handled properly + SoftwareSecurePhotoVerification.objects.create( + user=self.user, status='denied', error_msg='[{"photoIdReasons": ["Not provided"]}]' + ) + status = IDVerificationService.user_status(self.user) + expected_status = { + 'status': 'must_reverify', 'error': ['id_image_missing'], + 'should_display': True, 'verification_expiry': '', 'status_date': '', + } + self.assertDictEqual(status, expected_status) + + def test_approved_sso_verification(self): + with freeze_time('2015-03-02'): + # test for when sso verification has been created + SSOVerification.objects.create(user=self.user, status='approved') + status = IDVerificationService.user_status(self.user) + expected_status = {'status': 'approved', 'error': '', 'should_display': False, 'verification_expiry': '', + 'status_date': datetime.now(utc)} + self.assertDictEqual(status, expected_status) + + def test_denied_sso_verification(self): + with freeze_time('2015-04-02'): + # create denied sso verification for the user, make sure the denial + # is handled properly + SSOVerification.objects.create(user=self.user, status='denied') + status = IDVerificationService.user_status(self.user) + expected_status = { + 'status': 'must_reverify', 'error': '', 'should_display': False, + 'verification_expiry': '', 'status_date': '' + } + self.assertDictEqual(status, expected_status) + + def test_manual_verification(self): + with freeze_time('2015-05-02'): + # test for when manual verification has been created + ManualVerification.objects.create(user=self.user, status='approved') + status = IDVerificationService.user_status(self.user) + expected_status = {'status': 'approved', 'error': '', 'should_display': False, 'verification_expiry': '', + 'status_date': datetime.now(utc)} + self.assertDictEqual(status, expected_status) + + @ddt.data( + 'submitted', + 'denied', + 'approved', + 'created', + 'ready', + 'must_retry' + ) + def test_expiring_software_secure_verification(self, new_status): + with freeze_time('2015-07-11') as frozen_datetime: + # create approved photo verification for the user + SoftwareSecurePhotoVerification.objects.create(user=self.user, status='approved') + expiring_datetime = datetime.now(utc) + frozen_datetime.move_to('2015-07-14') + # create another according to status passed in. + SoftwareSecurePhotoVerification.objects.create(user=self.user, status=new_status) + status_date = expiring_datetime + if new_status == 'approved': + status_date = datetime.now(utc) + expected_status = {'status': 'approved', 'error': '', 'should_display': True, 'verification_expiry': '', + 'status_date': status_date} + status = IDVerificationService.user_status(self.user) + self.assertDictEqual(status, expected_status) + + @ddt.data( + 'submitted', + 'denied', + 'approved', + 'created', + 'ready', + 'must_retry' + ) + def test_expired_verification(self, new_status): + with freeze_time('2015-07-11') as frozen_datetime: + # create approved photo verification for the user + SoftwareSecurePhotoVerification.objects.create(user=self.user, status='approved') + frozen_datetime.move_to('2015-07-22') + # create another according to status passed in. + SoftwareSecurePhotoVerification.objects.create(user=self.user, status=new_status) + + check_status = new_status + status_date = '' + if new_status in ('submitted', 'must_retry'): + check_status = 'pending' + elif new_status in ('created', 'ready'): + check_status = 'none' + elif new_status == 'denied': + check_status = 'must_reverify' + else: + status_date = datetime.now(utc) + + expected_status = {'status': check_status, 'error': '', 'should_display': True, 'verification_expiry': '', + 'status_date': status_date} + status = IDVerificationService.user_status(self.user) + self.assertDictEqual(status, expected_status) + + @ddt.data( + SSOVerification, + ManualVerification + ) + def test_override_verification(self, verification_type): + with freeze_time('2015-07-11') as frozen_datetime: + # create approved photo verification for the user + SoftwareSecurePhotoVerification.objects.create(user=self.user, status='approved') + frozen_datetime.move_to('2015-07-14') + verification_type.objects.create(user=self.user, status='approved') + expected_status = { + 'status': 'approved', 'error': '', 'should_display': False, + 'verification_expiry': '', 'status_date': datetime.now(utc) + } + status = IDVerificationService.user_status(self.user) + self.assertDictEqual(status, expected_status) diff --git a/lms/djangoapps/verify_student/utils.py b/lms/djangoapps/verify_student/utils.py index d53bf5fb6f..5591b14952 100644 --- a/lms/djangoapps/verify_student/utils.py +++ b/lms/djangoapps/verify_student/utils.py @@ -27,7 +27,7 @@ def submit_request_to_ss(user_verification, copy_id_photo_from): send_request_to_ss_for_user.delay( user_verification_id=user_verification.id, copy_id_photo_from=copy_id_photo_from ) - except Exception as error: + except Exception as error: # pylint: disable=broad-except log.error( "Software Secure submit request %r failed, result: %s", user_verification.user.username, text_type(error) ) @@ -53,6 +53,25 @@ def earliest_allowed_verification_date(): return now() - datetime.timedelta(days=days_good_for) +def active_verifications(candidates, deadline): + """ + Based on the deadline, only return verification attempts + that are considered active (non-expired and wasn't created for future) + """ + relevant_attempts = [] + if not candidates: + return relevant_attempts + + # Look for a verification that was in effect at the deadline, + # preferring recent verifications. + # If no such verification is found, implicitly return empty array + for verification in candidates: + if verification.active_at_datetime(deadline): + relevant_attempts.append(verification) + + return relevant_attempts + + def verification_for_datetime(deadline, candidates): """Find a verification in a set that applied during a particular datetime. @@ -79,16 +98,14 @@ def verification_for_datetime(deadline, candidates): if not candidates: return None - # If there's no deadline, then return the most recently created verification - if deadline is None: + if not deadline: return candidates[0] - # Otherwise, look for a verification that was in effect at the deadline, - # preferring recent verifications. - # If no such verification is found, implicitly return `None` - for verification in candidates: - if verification.active_at_datetime(deadline): - return verification + attempts = active_verifications(candidates, deadline) + if attempts: + return attempts[0] + else: + return None def most_recent_verification(photo_id_verifications, sso_id_verifications, manual_id_verifications, most_recent_key):