From c9dc4d116f5aa86f55bf0801bd66f35a6f9758fc Mon Sep 17 00:00:00 2001 From: Matthew Piatetsky Date: Thu, 3 Jan 2019 14:32:10 -0500 Subject: [PATCH] clean up where access checks happen --- .../features/content_type_gating/helpers.py | 8 +++ .../features/content_type_gating/models.py | 64 ++++++++++++++----- .../content_type_gating/partitions.py | 58 ++++------------- .../content_type_gating/tests/test_access.py | 6 +- .../features/course_duration_limits/models.py | 4 +- .../tests/test_course_expiration.py | 3 +- 6 files changed, 77 insertions(+), 66 deletions(-) diff --git a/openedx/features/content_type_gating/helpers.py b/openedx/features/content_type_gating/helpers.py index f93f80153d..4c42c7f323 100644 --- a/openedx/features/content_type_gating/helpers.py +++ b/openedx/features/content_type_gating/helpers.py @@ -17,6 +17,14 @@ from student.roles import ( OrgInstructorRole, GlobalStaff ) +from xmodule.partitions.partitions import Group + +CONTENT_TYPE_GATE_GROUP_IDS = { + 'limited_access': 1, + 'full_access': 2, +} +LIMITED_ACCESS = Group(CONTENT_TYPE_GATE_GROUP_IDS['limited_access'], 'Limited-access Users') +FULL_ACCESS = Group(CONTENT_TYPE_GATE_GROUP_IDS['full_access'], 'Full-access Users') def has_staff_roles(user, course_key): diff --git a/openedx/features/content_type_gating/models.py b/openedx/features/content_type_gating/models.py index 17e97dace1..1d0a2eec6a 100644 --- a/openedx/features/content_type_gating/models.py +++ b/openedx/features/content_type_gating/models.py @@ -5,21 +5,28 @@ Content Type Gating Configuration Models # -*- coding: utf-8 -*- from __future__ import unicode_literals +from django.conf import settings from django.core.exceptions import ValidationError from django.db import models from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ from django.utils import timezone -from lms.djangoapps.courseware.masquerade import get_course_masquerade, is_masquerading_as_specific_student +from course_modes.models import CourseMode +from lms.djangoapps.courseware.masquerade import ( + get_course_masquerade, + get_masquerading_user_group, + is_masquerading_as_specific_student, +) from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel from openedx.core.djangoapps.config_model_utils.utils import is_in_holdback -from openedx.features.content_type_gating.helpers import has_staff_roles +from openedx.features.content_type_gating.helpers import FULL_ACCESS, has_staff_roles, LIMITED_ACCESS from openedx.features.course_duration_limits.config import ( CONTENT_TYPE_GATING_FLAG, FEATURE_BASED_ENROLLMENT_GLOBAL_KILL_FLAG, ) from student.models import CourseEnrollment +from xmodule.partitions.partitions import ENROLLMENT_TRACK_PARTITION_ID @python_2_unicode_compatible @@ -51,7 +58,36 @@ class ContentTypeGatingConfig(StackedConfigurationModel): ) @classmethod - def enabled_for_enrollment(cls, enrollment=None, user=None, course_key=None): + def has_full_access_role_in_masquerade(cls, user, course_key, course_masquerade, student_masquerade, + user_partition): + """ + The roles of the masquerade user are used to determine whether the content gate displays. + The gate will not appear if the masquerade user has any of the following roles: + Staff, Instructor, Beta Tester, Forum Community TA, Forum Group Moderator, Forum Moderator, Forum Administrator + """ + if student_masquerade: + # If a request is masquerading as a specific user, the user variable will represent the correct user. + if user and user.id and has_staff_roles(user, course_key): + return True + elif user_partition: + # If the current user is masquerading as a generic student in a specific group, + # then return the value based on that group. + masquerade_group = get_masquerading_user_group(course_key, user, user_partition) + if masquerade_group is None: + audit_mode_id = settings.COURSE_ENROLLMENT_MODES.get(CourseMode.AUDIT, {}).get('id') + # We are checking the user partition id here because currently content + # cannot have both the enrollment track partition and content gating partition + # configured simultaneously. We may change this in the future and allow + # configuring both partitions on content and selecting both partitions in masquerade. + if course_masquerade.user_partition_id == ENROLLMENT_TRACK_PARTITION_ID: + return course_masquerade.group_id != audit_mode_id + elif masquerade_group is FULL_ACCESS: + return True + elif masquerade_group is LIMITED_ACCESS: + return False + + @classmethod + def enabled_for_enrollment(cls, enrollment=None, user=None, course_key=None, user_partition=None): """ Return whether Content Type Gating is enabled for this enrollment. @@ -91,20 +127,18 @@ class ContentTypeGatingConfig(StackedConfigurationModel): if user is None and enrollment is not None: user = enrollment.user - no_masquerade = get_course_masquerade(user, course_key) is None + course_masquerade = get_course_masquerade(user, course_key) + no_masquerade = course_masquerade is None student_masquerade = is_masquerading_as_specific_student(user, course_key) - # We can only use the user variable for the code below when the request is not in a masquerade state - # or is masquerading as a specific user. - # When a request is not in a masquerade state the user variable represents the correct user. - # When a request is in a masquerade state and not masquerading as a specific user, - # then then user variable will be the incorrect (original) user, not the masquerade user. - # If a request is masquerading as a specific user, the user variable will represent the correct user. user_variable_represents_correct_user = (no_masquerade or student_masquerade) - if user and user.id: - # TODO: Move masquerade checks to enabled_for_enrollment from content_type_gating/partitions.py - # TODO: Consolidate masquerade checks into shared function like has_staff_roles below - if user_variable_represents_correct_user and has_staff_roles(user, course_key): + + if course_masquerade: + if cls.has_full_access_role_in_masquerade(user, course_key, course_masquerade, student_masquerade, + user_partition): return False + # When a request is not in a masquerade state the user variable represents the correct user. + elif user and user.id and has_staff_roles(user, course_key): + return False # check if user is in holdback if user_variable_represents_correct_user and is_in_holdback(user): @@ -113,7 +147,7 @@ class ContentTypeGatingConfig(StackedConfigurationModel): # enrollment might be None if the user isn't enrolled. In that case, # return enablement as if the user enrolled today # Also, ignore enrollment creation date if the user is masquerading. - if enrollment is None or not no_masquerade: + if enrollment is None or course_masquerade: return cls.enabled_for_course(course_key=course_key, target_datetime=timezone.now()) else: current_config = cls.current(course_key=enrollment.course_id) diff --git a/openedx/features/content_type_gating/partitions.py b/openedx/features/content_type_gating/partitions.py index eb6de93406..effdc10443 100644 --- a/openedx/features/content_type_gating/partitions.py +++ b/openedx/features/content_type_gating/partitions.py @@ -7,24 +7,18 @@ of audit learners. import logging -from course_modes.models import CourseMode - import crum from django.apps import apps -from django.conf import settings from django.template.loader import render_to_string from django.utils.translation import ugettext_lazy as _ from web_fragments.fragment import Fragment +from course_modes.models import CourseMode from lms.djangoapps.commerce.utils import EcommerceService -from lms.djangoapps.courseware.masquerade import ( - get_course_masquerade, - is_masquerading_as_specific_student, - get_masquerading_user_group, -) -from xmodule.partitions.partitions import Group, UserPartition, UserPartitionError, ENROLLMENT_TRACK_PARTITION_ID +from xmodule.partitions.partitions import UserPartition, UserPartitionError from openedx.core.lib.mobile_utils import is_request_from_mobile_app from openedx.features.content_type_gating.models import ContentTypeGatingConfig +from openedx.features.content_type_gating.helpers import FULL_ACCESS, LIMITED_ACCESS LOG = logging.getLogger(__name__) @@ -33,12 +27,6 @@ LOG = logging.getLogger(__name__) CONTENT_GATING_PARTITION_ID = 51 -CONTENT_TYPE_GATE_GROUP_IDS = { - 'limited_access': 1, - 'full_access': 2, -} - - def create_content_gating_partition(course): """ Create and return the Content Gating user partition. @@ -124,9 +112,6 @@ class ContentTypeGatingPartitionScheme(object): the gated content despite not being verified users. """ - LIMITED_ACCESS = Group(CONTENT_TYPE_GATE_GROUP_IDS['limited_access'], 'Limited-access Users') - FULL_ACCESS = Group(CONTENT_TYPE_GATE_GROUP_IDS['full_access'], 'Full-access Users') - read_only = True @classmethod @@ -135,29 +120,12 @@ class ContentTypeGatingPartitionScheme(object): Returns the Group for the specified user. """ - # First, check if we have to deal with masquerading. - # If the current user is masquerading as a specific student, use the - # same logic as normal to return that student's group. If the current - # user is masquerading as a generic student in a specific group, then - # return that group. - course_masquerade = get_course_masquerade(user, course_key) - if course_masquerade and not is_masquerading_as_specific_student(user, course_key): - masquerade_group = get_masquerading_user_group(course_key, user, user_partition) - if masquerade_group is not None: - return masquerade_group - else: - audit_mode_id = settings.COURSE_ENROLLMENT_MODES.get(CourseMode.AUDIT, {}).get('id') - if course_masquerade.user_partition_id == ENROLLMENT_TRACK_PARTITION_ID: - if course_masquerade.group_id != audit_mode_id: - return cls.FULL_ACCESS - else: - return cls.LIMITED_ACCESS - # For now, treat everyone as a Full-access user, until we have the rest of the # feature gating logic in place. - if not ContentTypeGatingConfig.enabled_for_enrollment(user=user, course_key=course_key): - return cls.FULL_ACCESS + if not ContentTypeGatingConfig.enabled_for_enrollment(user=user, course_key=course_key, + user_partition=user_partition): + return FULL_ACCESS # If CONTENT_TYPE_GATING is enabled use the following logic to determine whether a user should have FULL_ACCESS # or LIMITED_ACCESS @@ -168,7 +136,7 @@ class ContentTypeGatingPartitionScheme(object): # If there is no verified mode, all users are granted FULL_ACCESS if not course_mode.has_verified_mode(modes_dict): - return cls.FULL_ACCESS + return FULL_ACCESS course_enrollment = apps.get_model('student.CourseEnrollment') @@ -188,15 +156,15 @@ class ContentTypeGatingPartitionScheme(object): mode_slug, course_key, ) - return cls.FULL_ACCESS + return FULL_ACCESS if mode_slug == CourseMode.AUDIT: - return cls.LIMITED_ACCESS + return LIMITED_ACCESS else: - return cls.FULL_ACCESS + return FULL_ACCESS else: # Unenrolled users don't get gated content - return cls.LIMITED_ACCESS + return LIMITED_ACCESS @classmethod def create_user_partition(cls, id, name, description, groups=None, parameters=None, active=True): # pylint: disable=redefined-builtin, invalid-name, unused-argument @@ -219,8 +187,8 @@ class ContentTypeGatingPartitionScheme(object): unicode(name), unicode(description), [ - cls.LIMITED_ACCESS, - cls.FULL_ACCESS, + LIMITED_ACCESS, + FULL_ACCESS, ], cls, parameters, diff --git a/openedx/features/content_type_gating/tests/test_access.py b/openedx/features/content_type_gating/tests/test_access.py index 166f94b006..7052c361fd 100644 --- a/openedx/features/content_type_gating/tests/test_access.py +++ b/openedx/features/content_type_gating/tests/test_access.py @@ -35,9 +35,9 @@ from lms.djangoapps.courseware.tests.factories import ( from openedx.core.djangoapps.user_api.tests.factories import UserCourseTagFactory from openedx.core.djangoapps.util.testing import TestConditionalContent from openedx.core.lib.url_utils import quote_slashes +from openedx.features.content_type_gating.helpers import CONTENT_TYPE_GATE_GROUP_IDS from openedx.features.content_type_gating.partitions import ( CONTENT_GATING_PARTITION_ID, - CONTENT_TYPE_GATE_GROUP_IDS, ContentTypeGatingPartition ) from openedx.features.content_type_gating.models import ContentTypeGatingConfig @@ -224,8 +224,8 @@ class TestProblemTypeAccess(SharedModuleStoreTestCase): graded=True, group_access={ CONTENT_GATING_PARTITION_ID: [ - settings.CONTENT_TYPE_GATE_GROUP_IDS['limited_access'], - settings.CONTENT_TYPE_GATE_GROUP_IDS['full_access'] + CONTENT_TYPE_GATE_GROUP_IDS['limited_access'], + CONTENT_TYPE_GATE_GROUP_IDS['full_access'] ] }, ) diff --git a/openedx/features/course_duration_limits/models.py b/openedx/features/course_duration_limits/models.py index 783c632dd1..7367852a80 100644 --- a/openedx/features/course_duration_limits/models.py +++ b/openedx/features/course_duration_limits/models.py @@ -20,8 +20,8 @@ from lms.djangoapps.courseware.masquerade import ( ) from openedx.core.djangoapps.config_model_utils.models import StackedConfigurationModel from openedx.core.djangoapps.config_model_utils.utils import is_in_holdback -from openedx.features.content_type_gating.helpers import has_staff_roles -from openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID, CONTENT_TYPE_GATE_GROUP_IDS +from openedx.features.content_type_gating.helpers import CONTENT_TYPE_GATE_GROUP_IDS, has_staff_roles +from openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID from openedx.features.course_duration_limits.config import ( CONTENT_TYPE_GATING_FLAG, FEATURE_BASED_ENROLLMENT_GLOBAL_KILL_FLAG, diff --git a/openedx/features/course_duration_limits/tests/test_course_expiration.py b/openedx/features/course_duration_limits/tests/test_course_expiration.py index 77a3a79002..3a7e0e91d5 100644 --- a/openedx/features/course_duration_limits/tests/test_course_expiration.py +++ b/openedx/features/course_duration_limits/tests/test_course_expiration.py @@ -29,7 +29,8 @@ from lms.djangoapps.courseware.tests.factories import ( ) from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory -from openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID, CONTENT_TYPE_GATE_GROUP_IDS +from openedx.features.content_type_gating.helpers import CONTENT_TYPE_GATE_GROUP_IDS +from openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID from openedx.features.course_duration_limits.access import get_user_course_expiration_date, MIN_DURATION, MAX_DURATION from openedx.features.course_duration_limits.config import EXPERIMENT_ID, EXPERIMENT_DATA_HOLDBACK_KEY from openedx.features.course_experience.tests.views.helpers import add_course_mode