diff --git a/common/djangoapps/third_party_auth/migrations/0024_fix_edit_disallowed.py b/common/djangoapps/third_party_auth/migrations/0024_fix_edit_disallowed.py new file mode 100644 index 000000000000..1b0668957317 --- /dev/null +++ b/common/djangoapps/third_party_auth/migrations/0024_fix_edit_disallowed.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.11.20 on 2019-05-20 20:13 +from __future__ import unicode_literals + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('third_party_auth', '0023_auto_20190418_2033'), + ] + + operations = [ + migrations.AlterField( + model_name='ltiproviderconfig', + name='organization', + field=models.ForeignKey(blank=True, help_text='optional. If this provider is an Organization, this attribute can be used reference users in that Organization', null=True, on_delete=django.db.models.deletion.CASCADE, to='organizations.Organization'), + ), + migrations.AlterField( + model_name='oauth2providerconfig', + name='organization', + field=models.ForeignKey(blank=True, help_text='optional. If this provider is an Organization, this attribute can be used reference users in that Organization', null=True, on_delete=django.db.models.deletion.CASCADE, to='organizations.Organization'), + ), + migrations.AlterField( + model_name='samlproviderconfig', + name='organization', + field=models.ForeignKey(blank=True, help_text='optional. If this provider is an Organization, this attribute can be used reference users in that Organization', null=True, on_delete=django.db.models.deletion.CASCADE, to='organizations.Organization'), + ), + ] diff --git a/common/djangoapps/third_party_auth/models.py b/common/djangoapps/third_party_auth/models.py index 809f5e41ae36..a860f8df9f20 100644 --- a/common/djangoapps/third_party_auth/models.py +++ b/common/djangoapps/third_party_auth/models.py @@ -126,7 +126,7 @@ class ProviderConfig(ConfigurationModel): 'in a separate list of "Institution" login providers.' ), ) - organization = models.OneToOneField( + organization = models.ForeignKey( Organization, blank=True, null=True, diff --git a/lms/djangoapps/program_enrollments/api/v1/tests/test_views.py b/lms/djangoapps/program_enrollments/api/v1/tests/test_views.py index 16bbf5b817b2..e7d7a3b00453 100644 --- a/lms/djangoapps/program_enrollments/api/v1/tests/test_views.py +++ b/lms/djangoapps/program_enrollments/api/v1/tests/test_views.py @@ -20,6 +20,7 @@ from six.moves import range, zip from bulk_email.models import BulkEmailFlag, Optout +from course_modes.models import CourseMode from lms.djangoapps.certificates.tests.factories import GeneratedCertificateFactory from lms.djangoapps.certificates.models import CertificateStatuses from lms.djangoapps.courseware.tests.factories import GlobalStaffFactory @@ -29,6 +30,7 @@ MAX_ENROLLMENT_RECORDS, REQUEST_STUDENT_KEY, ) +from lms.djangoapps.program_enrollments.tests.factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory from lms.djangoapps.program_enrollments.models import ProgramEnrollment, ProgramCourseEnrollment from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL from openedx.core.djangoapps.catalog.tests.factories import CourseFactory @@ -40,7 +42,6 @@ from student.tests.factories import CourseEnrollmentFactory, UserFactory from xmodule.modulestore.tests.factories import CourseFactory as ModulestoreCourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from .factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory class ListViewTestMixin(object): @@ -357,6 +358,7 @@ def create_program_course_enrollment(self, program_enrollment, course_status='ac course_enrollment = CourseEnrollmentFactory.create( course_id=self.course_key, user=program_enrollment.user, + mode=CourseMode.MASTERS ) course_enrollment.is_active = course_status == "active" course_enrollment.save() @@ -371,7 +373,7 @@ def create_program_and_course_enrollments(self, external_user_key, user=False, c program_enrollment = self.create_program_enrollment(external_user_key, user) return self.create_program_course_enrollment(program_enrollment, course_status=course_status) - def assert_program_course_enrollment(self, external_user_key, expected_status, has_user): + def assert_program_course_enrollment(self, external_user_key, expected_status, has_user, mode=CourseMode.MASTERS): """ Convenience method to assert that a ProgramCourseEnrollment exists, and potentially that a CourseEnrollment also exists @@ -387,6 +389,7 @@ def assert_program_course_enrollment(self, external_user_key, expected_status, h self.assertIsNotNone(course_enrollment) self.assertEqual(expected_status == "active", course_enrollment.is_active) self.assertEqual(self.course_key, course_enrollment.course_id) + self.assertEqual(mode, course_enrollment.mode) else: self.assertIsNone(course_enrollment) @@ -499,13 +502,44 @@ def test_create_enrollments(self): self.assert_program_course_enrollment("learner-3", "active", False) self.assert_program_course_enrollment("learner-4", "inactive", False) - def test_user_already_enrolled_in_course(self): + def test_program_course_enrollment_exists(self): + """ + The program enrollments application already has a program_course_enrollment + record for this user and course + """ self.create_program_and_course_enrollments('learner-1') post_data = [self.learner_enrollment("learner-1")] response = self.request(self.default_url, post_data) self.assertEqual(422, response.status_code) self.assertDictEqual({'learner-1': CourseStatuses.CONFLICT}, response.data) + def test_user_currently_enrolled_in_course(self): + """ + If a user is already enrolled in a course through a different method + that enrollment should be linked but not overwritten as masters. + """ + CourseEnrollmentFactory.create( + course_id=self.course_key, + user=self.student, + mode=CourseMode.VERIFIED + ) + + self.create_program_enrollment('learner-1', user=self.student) + + post_data = [ + self.learner_enrollment("learner-1", "active") + ] + response = self.request(self.default_url, post_data) + + self.assertEqual(200, response.status_code) + self.assertDictEqual( + { + "learner-1": "active" + }, + response.data + ) + self.assert_program_course_enrollment("learner-1", "active", True, mode=CourseMode.VERIFIED) + def test_207_multistatus(self): self.create_program_enrollment('learner-1') post_data = [self.learner_enrollment("learner-1"), self.learner_enrollment("learner-2")] diff --git a/lms/djangoapps/program_enrollments/api/v1/views.py b/lms/djangoapps/program_enrollments/api/v1/views.py index 8c74b1813a3b..8eb40da8be30 100644 --- a/lms/djangoapps/program_enrollments/api/v1/views.py +++ b/lms/djangoapps/program_enrollments/api/v1/views.py @@ -758,7 +758,8 @@ def enroll_learner_in_course(self, enrollment_request, program_enrollment, progr """ if program_course_enrollment: return CourseEnrollmentResponseStatuses.CONFLICT - return ProgramCourseEnrollment.enroll( + + return ProgramCourseEnrollment.create_program_course_enrollment( program_enrollment, self.course_key, enrollment_request['status'] diff --git a/lms/djangoapps/program_enrollments/models.py b/lms/djangoapps/program_enrollments/models.py index 896a13f5bc1e..6c376c6fead9 100644 --- a/lms/djangoapps/program_enrollments/models.py +++ b/lms/djangoapps/program_enrollments/models.py @@ -10,11 +10,13 @@ from django.db import models from django.utils.translation import ugettext_lazy as _ from course_modes.models import CourseMode -from lms.djangoapps.program_enrollments.api.v1.constants import CourseEnrollmentResponseStatuses +from lms.djangoapps.program_enrollments.api.v1.constants import ( + CourseEnrollmentResponseStatuses as ProgramCourseEnrollmentResponseStatuses +) from model_utils.models import TimeStampedModel from opaque_keys.edx.django.models import CourseKeyField from simple_history.models import HistoricalRecords -from student.models import CourseEnrollment as StudentCourseEnrollment +from student.models import AlreadyEnrolledError, CourseEnrollment logger = logging.getLogger(__name__) # pylint: disable=invalid-name @@ -131,7 +133,7 @@ class Meta(object): related_name="program_course_enrollments" ) course_enrollment = models.OneToOneField( - StudentCourseEnrollment, + CourseEnrollment, null=True, blank=True, ) @@ -143,27 +145,19 @@ def __str__(self): return '[ProgramCourseEnrollment id={}]'.format(self.id) @classmethod - def enroll(cls, program_enrollment, course_key, status): + def create_program_course_enrollment(cls, program_enrollment, course_key, status): """ Create ProgramCourseEnrollment for the given course and program enrollment """ - course_enrollment = None - if program_enrollment.user: - course_enrollment = StudentCourseEnrollment.enroll( - program_enrollment.user, - course_key, - mode=CourseMode.MASTERS, - check_access=True, - ) - if status == CourseEnrollmentResponseStatuses.INACTIVE: - course_enrollment.deactivate() - program_course_enrollment = ProgramCourseEnrollment.objects.create( program_enrollment=program_enrollment, - course_enrollment=course_enrollment, course_key=course_key, status=status, ) + + if program_enrollment.user: + program_course_enrollment.enroll(program_enrollment.user) + return program_course_enrollment.status def change_status(self, status): @@ -175,9 +169,9 @@ def change_status(self, status): self.status = status if self.course_enrollment: - if status == CourseEnrollmentResponseStatuses.ACTIVE: + if status == ProgramCourseEnrollmentResponseStatuses.ACTIVE: self.course_enrollment.activate() - elif status == CourseEnrollmentResponseStatuses.INACTIVE: + elif status == ProgramCourseEnrollmentResponseStatuses.INACTIVE: self.course_enrollment.deactivate() else: message = ("Changed {enrollment} status to {status}, not changing course_enrollment" @@ -185,8 +179,8 @@ def change_status(self, status): logger.warn(message.format( enrollment=self, status=status, - active=CourseEnrollmentResponseStatuses.ACTIVE, - inactive=CourseEnrollmentResponseStatuses.INACTIVE + active=ProgramCourseEnrollmentResponseStatuses.ACTIVE, + inactive=ProgramCourseEnrollmentResponseStatuses.INACTIVE )) elif self.program_enrollment.user: logger.warn("User {user} {program_enrollment} {course_key} has no course_enrollment".format( @@ -196,3 +190,30 @@ def change_status(self, status): )) self.save() return self.status + + def enroll(self, user): + """ + Create a CourseEnrollment to enroll user in course + """ + try: + self.course_enrollment = CourseEnrollment.enroll( + user, + self.course_key, + mode=CourseMode.MASTERS, + check_access=True, + ) + except AlreadyEnrolledError: + course_enrollment = CourseEnrollment.objects.get( + user=user, + course_id=self.course_key, + ) + if course_enrollment.mode == CourseMode.AUDIT or course_enrollment.mode == CourseMode.HONOR: + course_enrollment.mode = CourseMode.MASTERS + course_enrollment.save() + self.course_enrollment = course_enrollment + message = ("Attempted to create course enrollment for user={user} and course={course}" + " but an enrollment already exists. Existing enrollment will be used instead") + logger.info(message.format(user=user.id, course=self.course_key)) + if self.status == ProgramCourseEnrollmentResponseStatuses.INACTIVE: + self.course_enrollment.deactivate() + self.save() diff --git a/lms/djangoapps/program_enrollments/signals.py b/lms/djangoapps/program_enrollments/signals.py index 5d01de3dd9b4..907fae47c6cb 100644 --- a/lms/djangoapps/program_enrollments/signals.py +++ b/lms/djangoapps/program_enrollments/signals.py @@ -3,10 +3,17 @@ """ from __future__ import absolute_import -from django.dispatch.dispatcher import receiver - -from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_MISC +import logging +from django.db.models.signals import post_save +from django.dispatch import receiver +from social_django.models import UserSocialAuth from lms.djangoapps.program_enrollments.models import ProgramEnrollment +from openedx.core.djangoapps.catalog.utils import get_programs +from openedx.core.djangoapps.user_api.accounts.signals import USER_RETIRE_LMS_MISC +from student.models import CourseEnrollmentException +from third_party_auth.models import SAMLProviderConfig + +logger = logging.getLogger(__name__) @receiver(USER_RETIRE_LMS_MISC) @@ -16,3 +23,69 @@ def _listen_for_lms_retire(sender, **kwargs): # pylint: disable=unused-argument """ user = kwargs.get('user') ProgramEnrollment.retire_user(user.id) + + +@receiver(post_save, sender=UserSocialAuth) +def matriculate_learner(sender, instance, created, **kwargs): # pylint: disable=unused-argument + """ + Post-save signal to update any waiting program enrollments with a user, + and enroll the user in any waiting course enrollments. + + In most cases this will just short-circuit. Enrollments will only be updated + for a SAML provider with a linked organization. + """ + if not created: + return + + try: + user = instance.user + provider_slug, external_user_key = instance.uid.split(':') + authorizing_org = SAMLProviderConfig.objects.current_set().get(slug=provider_slug).organization + + if not authorizing_org: + return + except (AttributeError, ValueError): + logger.info(u'Ignoring non-saml social auth entry for user=%s', user.id) + return + except SAMLProviderConfig.DoesNotExist: + logger.warning(u'Got incoming social auth for provider=%s but no such provider exists', provider_slug) + return + except SAMLProviderConfig.MultipleObjectsReturned: + logger.warning( + u'Unable to activate waiting enrollments for user=%s.' + u' Multiple active SAML configurations found for slug=%s. Expected one.', + user.id, + provider_slug) + return + + incomplete_enrollments = ProgramEnrollment.objects.filter( + external_user_key=external_user_key, + user=None, + ).prefetch_related('program_course_enrollments') + + for enrollment in incomplete_enrollments: + try: + enrollment_org = get_programs(uuid=enrollment.program_uuid)['authoring_organizations'][0] + if enrollment_org['key'] != authorizing_org.short_name: + continue + except (KeyError, TypeError): + logger.warning( + u'Failed to complete waiting enrollments for organization=%s.' + u' No catalog programs with matching authoring_organization exist.', + authorizing_org.short_name + ) + continue + + enrollment.user = user + enrollment.save() + for program_course_enrollment in enrollment.program_course_enrollments.all(): + try: + program_course_enrollment.enroll(user) + except CourseEnrollmentException as e: + logger.warning( + u'Failed to enroll user=%s with waiting program_course_enrollment=%s: %s', + user.id, + program_course_enrollment.id, + e, + ) + raise e diff --git a/lms/djangoapps/program_enrollments/api/v1/tests/factories.py b/lms/djangoapps/program_enrollments/tests/factories.py similarity index 100% rename from lms/djangoapps/program_enrollments/api/v1/tests/factories.py rename to lms/djangoapps/program_enrollments/tests/factories.py diff --git a/lms/djangoapps/program_enrollments/tests/test_models.py b/lms/djangoapps/program_enrollments/tests/test_models.py index ae701933fc05..f068f421e653 100644 --- a/lms/djangoapps/program_enrollments/tests/test_models.py +++ b/lms/djangoapps/program_enrollments/tests/test_models.py @@ -5,14 +5,19 @@ from uuid import uuid4 +import ddt from django.test import TestCase from opaque_keys.edx.keys import CourseKey from six.moves import range from testfixtures import LogCapture -from lms.djangoapps.program_enrollments.models import ProgramCourseEnrollment, ProgramEnrollment -from openedx.core.djangoapps.catalog.tests.factories import generate_course_run_key +from course_modes.models import CourseMode +from edx_django_utils.cache import RequestCache +from lms.djangoapps.program_enrollments.models import ProgramEnrollment, ProgramCourseEnrollment +from student.models import CourseEnrollment from student.tests.factories import CourseEnrollmentFactory, UserFactory +from openedx.core.djangoapps.catalog.tests.factories import generate_course_run_key +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory class ProgramEnrollmentModelTests(TestCase): @@ -103,6 +108,7 @@ def test_user_retirement(self): self.assertEquals(record.external_user_key, None) +@ddt.ddt class ProgramCourseEnrollmentModelTests(TestCase): """ Tests for the ProgramCourseEnrollment model. @@ -112,6 +118,7 @@ def setUp(self): Set up test data """ super(ProgramCourseEnrollmentModelTests, self).setUp() + RequestCache.clear_all_namespaces() self.user = UserFactory.create() self.program_uuid = uuid4() self.program_enrollment = ProgramEnrollment.objects.create( @@ -122,21 +129,37 @@ def setUp(self): status='enrolled' ) self.course_key = CourseKey.from_string(generate_course_run_key()) - self.course_enrollment = CourseEnrollmentFactory.create( + CourseOverviewFactory(id=self.course_key) + + def _create_completed_program_course_enrollment(self): + """ helper function create program course enrollment """ + course_enrollment = CourseEnrollmentFactory.create( course_id=self.course_key, user=self.user, + mode=CourseMode.MASTERS + ) + program_course_enrollment = ProgramCourseEnrollment.objects.create( + program_enrollment=self.program_enrollment, + course_key=self.course_key, + course_enrollment=course_enrollment, + status="active" ) - self.program_course_enrollment = ProgramCourseEnrollment.objects.create( + return program_course_enrollment + + def _create_waiting_program_course_enrollment(self): + """ helper function create program course enrollment with no lms user """ + return ProgramCourseEnrollment.objects.create( program_enrollment=self.program_enrollment, course_key=self.course_key, - course_enrollment=self.course_enrollment, + course_enrollment=None, status="active" ) def test_change_status_no_enrollment(self): + program_course_enrollment = self._create_completed_program_course_enrollment() with LogCapture() as capture: - self.program_course_enrollment.course_enrollment = None - self.program_course_enrollment.change_status("inactive") + program_course_enrollment.course_enrollment = None + program_course_enrollment.change_status("inactive") expected_message = "User {} {} {} has no course_enrollment".format( self.user, self.program_enrollment, @@ -147,12 +170,43 @@ def test_change_status_no_enrollment(self): ) def test_change_status_not_active_or_inactive(self): + program_course_enrollment = self._create_completed_program_course_enrollment() with LogCapture() as capture: status = "potential-future-status-0123" - self.program_course_enrollment.change_status(status) + program_course_enrollment.change_status(status) message = ("Changed {} status to {}, not changing course_enrollment" " status because status is not 'active' or 'inactive'") - expected_message = message.format(self.program_course_enrollment, status) + expected_message = message.format(program_course_enrollment, status) capture.check( ('lms.djangoapps.program_enrollments.models', 'WARNING', expected_message) ) + + def test_enroll_new_course_enrollment(self): + program_course_enrollment = self._create_waiting_program_course_enrollment() + program_course_enrollment.enroll(self.user) + + course_enrollment = CourseEnrollment.objects.get(user=self.user, course_id=self.course_key) + self.assertEqual(course_enrollment.user, self.user) + self.assertEqual(course_enrollment.course.id, self.course_key) + self.assertEqual(course_enrollment.mode, CourseMode.MASTERS) + + @ddt.data( + (CourseMode.VERIFIED, CourseMode.VERIFIED), + (CourseMode.AUDIT, CourseMode.MASTERS), + (CourseMode.HONOR, CourseMode.MASTERS) + ) + @ddt.unpack + def test_enroll_existing_course_enrollment(self, original_mode, result_mode): + course_enrollment = CourseEnrollmentFactory.create( + course_id=self.course_key, + user=self.user, + mode=original_mode + ) + program_course_enrollment = self._create_waiting_program_course_enrollment() + + program_course_enrollment.enroll(self.user) + + course_enrollment = CourseEnrollment.objects.get(user=self.user, course_id=self.course_key) + self.assertEqual(course_enrollment.user, self.user) + self.assertEqual(course_enrollment.course.id, self.course_key) + self.assertEqual(course_enrollment.mode, result_mode) diff --git a/lms/djangoapps/program_enrollments/tests/test_signals.py b/lms/djangoapps/program_enrollments/tests/test_signals.py index a01e288a309a..9791e81a3a11 100644 --- a/lms/djangoapps/program_enrollments/tests/test_signals.py +++ b/lms/djangoapps/program_enrollments/tests/test_signals.py @@ -4,10 +4,30 @@ from __future__ import absolute_import -from lms.djangoapps.program_enrollments.api.v1.tests.factories import ProgramEnrollmentFactory -from lms.djangoapps.program_enrollments.signals import _listen_for_lms_retire +from django.core.cache import cache +import mock +from opaque_keys.edx.keys import CourseKey +import pytest +from social_django.models import UserSocialAuth +from testfixtures import LogCapture + +from course_modes.models import CourseMode +from edx_django_utils.cache import RequestCache +from lms.djangoapps.program_enrollments.signals import _listen_for_lms_retire, logger +from lms.djangoapps.program_enrollments.tests.factories import ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory +from organizations.tests.factories import OrganizationFactory +from openedx.core.djangoapps.catalog.cache import PROGRAM_CACHE_KEY_TPL +from openedx.core.djangoapps.catalog.tests.factories import ( + OrganizationFactory as CatalogOrganizationFactory, ProgramFactory +) +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview +from openedx.core.djangoapps.content.course_overviews.tests.factories import CourseOverviewFactory from openedx.core.djangoapps.user_api.accounts.tests.retirement_helpers import fake_completed_retirement -from student.tests.factories import UserFactory +from openedx.core.djangolib.testing.utils import CacheIsolationTestCase +from student.models import CourseEnrollmentException +from student.tests.factories import CourseEnrollmentFactory, UserFactory +from third_party_auth.tests.factories import SAMLProviderConfigFactory +from third_party_auth.models import SAMLProviderConfig from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -77,3 +97,264 @@ def test_idempotent(self): _listen_for_lms_retire(sender=self.__class__, user=enrollment.user) self.assert_enrollment_and_history_retired(enrollment) + + +class SocialAuthEnrollmentCompletionSignalTest(CacheIsolationTestCase): + """ + Test post-save handler on UserSocialAuth + """ + ENABLED_CACHES = ['default'] + + @classmethod + def setUpClass(cls): + super(SocialAuthEnrollmentCompletionSignalTest, cls).setUpClass() + + cls.external_id = '0000' + cls.provider_slug = 'uox' + cls.course_keys = [ + CourseKey.from_string('course-v1:edX+DemoX+Test_Course'), + CourseKey.from_string('course-v1:edX+DemoX+Another_Test_Course'), + ] + cls.organization = OrganizationFactory.create( + short_name='UoX' + ) + cls.user = UserFactory.create() + + for course_key in cls.course_keys: + CourseOverviewFactory(id=course_key) + cls.provider_config = SAMLProviderConfigFactory.create(organization=cls.organization, slug=cls.provider_slug) + + def setUp(self): + super(SocialAuthEnrollmentCompletionSignalTest, self).setUp() + RequestCache.clear_all_namespaces() + catalog_org = CatalogOrganizationFactory.create(key=self.organization.short_name) + self.program_uuid = self._create_catalog_program(catalog_org)['uuid'] + + def _create_catalog_program(self, catalog_org): + """ helper method to create a cached catalog program """ + program = ProgramFactory.create( + authoring_organizations=[catalog_org] + ) + cache.set(PROGRAM_CACHE_KEY_TPL.format(uuid=program['uuid']), program, None) + return program + + def _create_waiting_program_enrollment(self): + """ helper method to create a waiting program enrollment """ + return ProgramEnrollmentFactory.create( + user=None, + external_user_key=self.external_id, + program_uuid=self.program_uuid, + ) + + def _create_waiting_course_enrollments(self, program_enrollment): + """ helper method to create waiting course enrollments """ + return [ + ProgramCourseEnrollmentFactory( + program_enrollment=program_enrollment, + course_enrollment=None, + course_key=course_key, + ) + for course_key in self.course_keys + ] + + def _assert_program_enrollment_user(self, program_enrollment, user): + """ validate program enrollment has a user """ + program_enrollment.refresh_from_db() + self.assertEqual(program_enrollment.user, user) + + def _assert_program_course_enrollment(self, program_course_enrollment, mode=CourseMode.MASTERS): + """ validate program course enrollment has a valid course enrollment """ + program_course_enrollment.refresh_from_db() + student_course_enrollment = program_course_enrollment.course_enrollment + self.assertEqual(student_course_enrollment.user, self.user) + self.assertEqual(student_course_enrollment.course.id, program_course_enrollment.course_key) + self.assertEqual(student_course_enrollment.mode, mode) + + def test_waiting_course_enrollments_completed(self): + program_enrollment = self._create_waiting_program_enrollment() + program_course_enrollments = self._create_waiting_course_enrollments(program_enrollment) + + UserSocialAuth.objects.create( + user=self.user, + uid='{0}:{1}'.format(self.provider_slug, self.external_id) + ) + + self._assert_program_enrollment_user(program_enrollment, self.user) + for program_course_enrollment in program_course_enrollments: + self._assert_program_course_enrollment(program_course_enrollment) + + def test_same_user_key_in_multiple_organizations(self): + uox_program_enrollment = self._create_waiting_program_enrollment() + + second_organization = OrganizationFactory.create() + SAMLProviderConfigFactory.create(organization=second_organization, slug='aiu') + catalog_org = CatalogOrganizationFactory.create(key=second_organization.short_name) + program_uuid = self._create_catalog_program(catalog_org)['uuid'] + + # aiu enrollment with the same student key as our uox user + aiu_program_enrollment = ProgramEnrollmentFactory.create( + user=None, + external_user_key=self.external_id, + program_uuid=program_uuid + ) + + UserSocialAuth.objects.create( + user=UserFactory.create(), + uid='{0}:{1}'.format('not_used', self.external_id), + ) + + UserSocialAuth.objects.create( + user=self.user, + uid='{0}:{1}'.format(self.provider_slug, self.external_id), + ) + self._assert_program_enrollment_user(uox_program_enrollment, self.user) + + aiu_user = UserFactory.create() + UserSocialAuth.objects.create( + user=aiu_user, + uid='{0}:{1}'.format('aiu', self.external_id), + ) + self._assert_program_enrollment_user(aiu_program_enrollment, aiu_user) + + def test_only_active_saml_config_used(self): + """ makes sure only the active row in SAMLProvider config is used """ + program_enrollment = self._create_waiting_program_enrollment() + + # update will create a second record + self.provider_config.organization = None + self.provider_config.save() + self.assertEqual(len(SAMLProviderConfig.objects.all()), 2) + + UserSocialAuth.objects.create( + user=self.user, + uid='{0}:{1}'.format(self.provider_slug, self.external_id) + ) + program_enrollment.refresh_from_db() + self.assertIsNone(program_enrollment.user) + + def test_learner_already_enrolled_in_course(self): + course_key = self.course_keys[0] + course = CourseOverview.objects.get(id=course_key) + CourseEnrollmentFactory(user=self.user, course=course, mode=CourseMode.VERIFIED) + + program_enrollment = self._create_waiting_program_enrollment() + program_course_enrollments = self._create_waiting_course_enrollments(program_enrollment) + + UserSocialAuth.objects.create( + user=self.user, + uid='{0}:{1}'.format(self.provider_slug, self.external_id) + ) + + self._assert_program_enrollment_user(program_enrollment, self.user) + + duplicate_program_course_enrollment = program_course_enrollments[0] + self._assert_program_course_enrollment(duplicate_program_course_enrollment, CourseMode.VERIFIED) + + program_course_enrollment = program_course_enrollments[1] + self._assert_program_course_enrollment(program_course_enrollment) + + def test_enrolled_with_no_course_enrollments(self): + program_enrollment = self._create_waiting_program_enrollment() + + UserSocialAuth.objects.create( + user=self.user, + uid='{0}:{1}'.format(self.provider_slug, self.external_id) + ) + + self._assert_program_enrollment_user(program_enrollment, self.user) + + def test_create_social_auth_with_no_waiting_enrollments(self): + """ + No exceptions should be raised if there are no enrollments to update + """ + UserSocialAuth.objects.create( + user=self.user, + uid='{0}:{1}'.format(self.provider_slug, self.external_id) + ) + + def test_create_social_auth_provider_has_no_organization(self): + """ + No exceptions should be raised if provider is not linked to an organization + """ + provider = SAMLProviderConfigFactory.create() + UserSocialAuth.objects.create( + user=self.user, + uid='{0}:{1}'.format(provider.slug, self.external_id) + ) + + def test_create_social_auth_non_saml_provider(self): + """ + No exceptions should be raised for a non-SAML uid + """ + UserSocialAuth.objects.create( + user=self.user, + uid='google-oauth-user@gmail.com' + ) + UserSocialAuth.objects.create( + user=self.user, + uid='123:123:123' + ) + + def test_saml_provider_not_found(self): + """ + An error should be logged for incoming social auth entries with a saml id but + no matching saml configuration exists + """ + with LogCapture(logger.name) as log: + UserSocialAuth.objects.create( + user=self.user, + uid='abc:123456' + ) + log.check_present( + ( + logger.name, + 'WARNING', + u'Got incoming social auth for provider={} but no such provider exists'.format('abc') + ) + ) + + def test_cannot_find_catalog_program(self): + """ + An error should be logged if a program enrollment exists but a matching catalog + program cannot be found + """ + self.program_uuid = self._create_catalog_program(None)['uuid'] + self._create_waiting_program_enrollment() + + with LogCapture(logger.name) as log: + UserSocialAuth.objects.create( + user=self.user, + uid='{0}:{1}'.format(self.provider_slug, self.external_id) + ) + error_tmpl = ( + u'Failed to complete waiting enrollments for organization={}.' + u' No catalog programs with matching authoring_organization exist.' + ) + log.check_present( + ( + logger.name, + 'WARNING', + error_tmpl.format('UoX') + ) + ) + + def test_log_on_enrollment_failure(self): + program_enrollment = self._create_waiting_program_enrollment() + program_course_enrollments = self._create_waiting_course_enrollments(program_enrollment) + + with mock.patch('student.models.CourseEnrollment.enroll') as enrollMock: + enrollMock.side_effect = CourseEnrollmentException('something has gone wrong') + with LogCapture(logger.name) as log: + with pytest.raises(CourseEnrollmentException): + UserSocialAuth.objects.create( + user=self.user, + uid='{0}:{1}'.format(self.provider_slug, self.external_id) + ) + error_tmpl = u'Failed to enroll user={} with waiting program_course_enrollment={}: {}' + log.check_present( + ( + logger.name, + 'WARNING', + error_tmpl.format(self.user.id, program_course_enrollments[0].id, 'something has gone wrong') + ) + ) diff --git a/lms/djangoapps/program_enrollments/tests/test_tasks.py b/lms/djangoapps/program_enrollments/tests/test_tasks.py index c0402c46e1ae..a5374d2f97fb 100644 --- a/lms/djangoapps/program_enrollments/tests/test_tasks.py +++ b/lms/djangoapps/program_enrollments/tests/test_tasks.py @@ -9,7 +9,7 @@ from testfixtures import LogCapture from lms.djangoapps.program_enrollments.models import ProgramEnrollment, ProgramCourseEnrollment from lms.djangoapps.program_enrollments.tasks import expire_waiting_enrollments, log -from lms.djangoapps.program_enrollments.api.v1.tests.factories import ( +from lms.djangoapps.program_enrollments.tests.factories import ( ProgramCourseEnrollmentFactory, ProgramEnrollmentFactory ) from student.tests.factories import UserFactory diff --git a/lms/djangoapps/program_enrollments/tests/test_utils.py b/lms/djangoapps/program_enrollments/tests/test_utils.py index 7fa9cba4d550..7e98d990fef5 100644 --- a/lms/djangoapps/program_enrollments/tests/test_utils.py +++ b/lms/djangoapps/program_enrollments/tests/test_utils.py @@ -18,6 +18,7 @@ OrganizationDoesNotExistException, ProgramDoesNotExistException, ProviderDoesNotExistException, + UserLookupException, get_user_by_program_id ) from student.tests.factories import UserFactory @@ -119,9 +120,25 @@ def test_lms_organization_not_found(self): def test_saml_provider_not_found(self): """ - Test an sdf is thrown if no SAML provider exists for this program's organization + Test an exception is thrown if no SAML provider exists for this program's organization """ OrganizationFactory.create(short_name=self.organization_key) with pytest.raises(ProviderDoesNotExistException): get_user_by_program_id(self.external_user_id, self.program_uuid) + + def test_multiple_active_saml_providers(self): + """ + If multiple samlprovider records exist with the same organization + an exception is raised + """ + organization = OrganizationFactory.create(short_name=self.organization_key) + provider = SAMLProviderConfigFactory.create(organization=organization) + + self.create_social_auth_entry(self.user, provider, self.external_user_id) + + # create a second active config for the same organization + SAMLProviderConfigFactory.create(organization=organization, slug='foox') + + with pytest.raises(UserLookupException): + get_user_by_program_id(self.external_user_id, self.program_uuid) diff --git a/lms/djangoapps/program_enrollments/utils.py b/lms/djangoapps/program_enrollments/utils.py index cfd731cdd8ac..d793e1431f3a 100644 --- a/lms/djangoapps/program_enrollments/utils.py +++ b/lms/djangoapps/program_enrollments/utils.py @@ -5,6 +5,7 @@ from openedx.core.djangoapps.catalog.utils import get_programs from organizations.models import Organization from social_django.models import UserSocialAuth +from third_party_auth.models import SAMLProviderConfig log = logging.getLogger(__name__) @@ -77,10 +78,16 @@ def get_user_by_organization(external_user_id, organization): ProviderDoesNotExistException if there is no SAML provider configured for the related organization. """ try: - provider_slug = organization.samlproviderconfig.provider_id.strip('saml-') - except Organization.samlproviderconfig.RelatedObjectDoesNotExist: + provider_slug = organization.samlproviderconfig_set.current_set().get().provider_id.strip('saml-') + except SAMLProviderConfig.DoesNotExist: log.error(u'No SAML provider found for organization id [%s]', organization.id) raise ProviderDoesNotExistException + except SAMLProviderConfig.MultipleObjectsReturned: + log.error( + u'Multiple active SAML configurations found for organization=%s. Expected one.', + organization.short_name, + ) + raise UserLookupException try: social_auth_uid = '{0}:{1}'.format(provider_slug, external_user_id)