Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #20064 from edx/revert-20003-mobile-access-denied

Browse Source 
Revert "REVMI-62 Return gated blocks in the course blocks API response with an authorization denial reason"
This commit is contained in:
Matthew Piatetsky
2019-03-27 18:18:19 -04:00
committed by GitHub
parent 6a15e57092 8eae694566
commit 470b085fcf
15 changed files with 44 additions and 203 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
common/lib/xmodule/xmodule/partitions/partitions.py
View File

@@ -250,13 +250,13 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche
)
)
def access_denied_message(self, block_key, user, user_group, allowed_groups):
def access_denied_message(self, block, user, user_group, allowed_groups):
"""
Return a message that should be displayed to the user when they are not allowed to access
content managed by this partition, or None if there is no applicable message.
Arguments:
block_key (:class:`.BlockUsageLocator`): The content being managed
block (:class:`.XBlock`): The content being managed
user (:class:`.User`): The user who was denied access
user_group (:class:`.Group`): The current Group the user is in
allowed_groups (list of :class:`.Group`): The groups who are allowed to see the content

6
common/lib/xmodule/xmodule/partitions/partitions_service.py
View File

@@ -104,7 +104,7 @@ def _create_enrollment_track_partition(course):
log.warning(
"Can't add 'enrollment_track' partition, as ID {id} is assigned to {partition} in course {course}.".format(
id=ENROLLMENT_TRACK_PARTITION_ID,
partition=get_partition_from_id(course.user_partitions, ENROLLMENT_TRACK_PARTITION_ID).name,
partition=_get_partition_from_id(course.user_partitions, ENROLLMENT_TRACK_PARTITION_ID).name,
course=unicode(course.id)
)
)
@@ -193,7 +193,7 @@ class PartitionService(object):
Returns:
A UserPartition, or None if not found.
"""
return get_partition_from_id(self.course_partitions, user_partition_id)
return _get_partition_from_id(self.course_partitions, user_partition_id)
def get_group(self, user, user_partition, assign=True):
"""
@@ -206,7 +206,7 @@ class PartitionService(object):
)
def get_partition_from_id(partitions, user_partition_id):
def _get_partition_from_id(partitions, user_partition_id):
"""
Look for a user partition with a matching id in the provided list of partitions.

8
lms/djangoapps/course_api/blocks/api.py
View File

@@ -5,7 +5,6 @@ API function for retrieving course blocks data
import lms.djangoapps.course_blocks.api as course_blocks_api
from lms.djangoapps.course_blocks.transformers.hidden_content import HiddenContentTransformer
from lms.djangoapps.course_blocks.transformers.hide_empty import HideEmptyTransformer
from lms.djangoapps.course_blocks.transformers.access_denied_filter import AccessDeniedMessageFilterTransformer
from openedx.core.djangoapps.content.block_structure.transformers import BlockStructureTransformers
from openedx.core.lib.mobile_utils import is_request_from_mobile_app
@@ -26,7 +25,6 @@ def get_blocks(
student_view_data=None,
return_type='dict',
block_types_filter=None,
hide_access_denials=False,
):
"""
Return a serialized representation of the course blocks.
@@ -53,9 +51,6 @@ def get_blocks(
the format for returning the blocks.
block_types_filter (list): Optional list of block type names used to filter
the final result of returned blocks.
hide_access_denials (bool): When True, filter out any blocks that were
denied access to the user, even if they have access denial messages
attached.
"""
# create ordered list of transformers, adding BlocksAPITransformer at end.
transformers = BlockStructureTransformers()
@@ -75,9 +70,6 @@ def get_blocks(
HiddenContentTransformer()
]
if hide_access_denials:
transformers += [AccessDeniedMessageFilterTransformer()]
# TODO: Remove this after REVE-52 lands and old-mobile-app traffic falls to < 5% of mobile traffic
if is_request_from_mobile_app(request):
transformers += [HideEmptyTransformer()]

16
lms/djangoapps/course_api/blocks/serializers.py
View File

@@ -35,20 +35,6 @@ class BlockSerializer(serializers.Serializer): # pylint: disable=abstract-metho
Return a serializable representation of the requested block
"""
# create response data dict for basic fields
block_structure = self.context['block_structure']
authorization_denial_reason = block_structure.get_xblock_field(block_key, 'authorization_denial_reason')
authorization_denial_message = block_structure.get_xblock_field(block_key, 'authorization_denial_message')
if authorization_denial_reason and authorization_denial_message:
data = {
'id': unicode(block_key),
'block_id': unicode(block_key.block_id),
'authorization_denial_reason': authorization_denial_reason,
'authorization_denial_message': authorization_denial_message
}
return data
data = {
'id': unicode(block_key),
'block_id': unicode(block_key.block_id),
@@ -85,7 +71,7 @@ class BlockSerializer(serializers.Serializer): # pylint: disable=abstract-metho
data[supported_field.serializer_field_name] = field_value
if 'children' in self.context['requested_fields']:
children = block_structure.get_children(block_key)
children = self.context['block_structure'].get_children(block_key)
if children:
data['children'] = [unicode(child) for child in children]

15
lms/djangoapps/course_api/blocks/urls.py
View File

@@ -11,7 +11,6 @@ urlpatterns = [
url(
r'^v1/blocks/{}'.format(settings.USAGE_KEY_PATTERN),
BlocksView.as_view(),
kwargs={'hide_access_denials': True},
name="blocks_in_block_tree"
),
@@ -19,20 +18,6 @@ urlpatterns = [
url(
r'^v1/blocks/',
BlocksInCourseView.as_view(),
kwargs={'hide_access_denials': True},
name="blocks_in_course"
),
# This endpoint requires the usage_key for the starting block.
url(
r'^v2/blocks/{}'.format(settings.USAGE_KEY_PATTERN),
BlocksView.as_view(),
name="blocks_in_block_tree"
),
# This endpoint is an alternative to the above, but requires course_id as a parameter.
url(
r'^v2/blocks/',
BlocksInCourseView.as_view(),
name="blocks_in_course"
),
]

7
lms/djangoapps/course_api/blocks/views.py
View File

@@ -183,7 +183,7 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView):
Returned only if "show_correctness" is included in the "requested_fields" parameter.
"""
def list(self, request, usage_key_string, hide_access_denials=False): # pylint: disable=arguments-differ
def list(self, request, usage_key_string): # pylint: disable=arguments-differ
"""
REST API endpoint for listing all the blocks information in the course,
while regarding user access and roles.
@@ -213,7 +213,6 @@ class BlocksView(DeveloperErrorViewMixin, ListAPIView):
params.cleaned_data.get('student_view_data', []),
params.cleaned_data['return_type'],
params.cleaned_data.get('block_types_filter', None),
hide_access_denials=hide_access_denials,
)
)
except ItemNotFoundError as exception:
@@ -261,7 +260,7 @@ class BlocksInCourseView(BlocksView):
with a message indicating that the course_id is not valid.
"""
def list(self, request, hide_access_denials=False): # pylint: disable=arguments-differ
def list(self, request): # pylint: disable=arguments-differ
"""
Retrieves the usage_key for the requested course, and then returns the
same information that would be returned by BlocksView.list, called with
@@ -281,4 +280,4 @@ class BlocksInCourseView(BlocksView):
course_usage_key = modulestore().make_course_usage_key(course_key)
except InvalidKeyError:
raise ValidationError(u"'{}' is not a valid course key.".format(unicode(course_key_string)))
return super(BlocksInCourseView, self).list(request, course_usage_key, hide_access_denials=hide_access_denials)
return super(BlocksInCourseView, self).list(request, course_usage_key)

2
lms/djangoapps/course_api/tests/test_serializers.py
View File

@@ -65,7 +65,7 @@ class TestCourseSerializer(CourseApiFactoryMixin, ModuleStoreTestCase):
'end': u'2015-09-19T18:00:00Z',
'enrollment_start': u'2015-06-15T00:00:00Z',
'enrollment_end': u'2015-07-15T00:00:00Z',
'blocks_url': u'http://testserver/api/courses/v2/blocks/?course_id=edX%2Ftoy%2F2012_Fall',
'blocks_url': u'http://testserver/api/courses/v1/blocks/?course_id=edX%2Ftoy%2F2012_Fall',
'effort': u'6 hours',
'pacing': 'instructor',
'mobile_available': True,

43
lms/djangoapps/course_blocks/transformers/access_denied_filter.py
View File

@@ -1,43 +0,0 @@
"""
Access Denied Message Filter Transformer implementation.
"""
# TODO: Remove this file after REVE-52 lands and old-mobile-app traffic falls to < 5% of mobile traffic
from openedx.core.djangoapps.content.block_structure.transformer import (
BlockStructureTransformer
)
class AccessDeniedMessageFilterTransformer(BlockStructureTransformer):
"""
A transformer that removes any block from the course that has an
authorization_denial_reason or an authorization_denial_message.
"""
WRITE_VERSION = 1
READ_VERSION = 1
@classmethod
def name(cls):
"""
Unique identifier for the transformer's class;
same identifier used in setup.py.
"""
return "access_denied_message_filter"
@classmethod
def collect(cls, block_structure):
"""
Collects any information that's necessary to execute this
transformer's transform method.
"""
block_structure.request_xblock_fields('authorization_denial_reason', 'authorization_denial_message')
def transform(self, usage_info, block_structure):
def _filter(block_key):
reason = block_structure.get_xblock_field(block_key, 'authorization_denial_reason')
message = block_structure.get_xblock_field(block_key, 'authorization_denial_message')
return reason and message
for _ in block_structure.post_order_traversal(
filter_func=block_structure.create_removal_filter(_filter)
):
pass

2
lms/djangoapps/course_blocks/transformers/tests/test_split_test.py
View File

@@ -177,7 +177,7 @@ class SplitTestTransformerTestCase(CourseStructureTestCase):
# parents. However, we don't think this is a use case we need to
# support for split_test components (since they are now deprecated
# in favor of content groups and user partitions).
(0, ('course', 'A', 'D', 'E', 'H', 'L', 'O', 'P', )),
(0, ('course', 'A', 'D', 'E', 'H', 'L', 'O', 'P',)),
(1, ('course', 'A', 'D', 'F', 'J', 'M', 'I',)),
(2, ('course', 'A', 'D', 'G', 'O',)),
)

33
lms/djangoapps/course_blocks/transformers/tests/test_user_partitions.py
View File

@@ -4,18 +4,13 @@ Tests for UserPartitionTransformer.
"""
import string
from collections import namedtuple
from datetime import datetime
import ddt
from mock import patch
from course_modes.tests.factories import CourseModeFactory
from openedx.core.djangoapps.course_groups.cohorts import add_user_to_cohort
from openedx.core.djangoapps.course_groups.partition_scheme import CohortPartitionScheme
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory, config_course_cohorts
from openedx.core.djangoapps.course_groups.views import link_cohort_to_partition_group
from openedx.features.content_type_gating.models import ContentTypeGatingConfig
from openedx.features.content_type_gating.partitions import create_content_gating_partition
from student.tests.factories import CourseEnrollmentFactory
from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.partitions.partitions import Group, UserPartition
@@ -178,7 +173,7 @@ class UserPartitionTransformerTestCase(UserPartitionTestMixin, CourseStructureTe
'#type': 'vertical',
'#ref': 'K',
'#parents': ['E'],
'metadata': {'group_access': {self.user_partition.id: [4, 51]}},
'metadata': {'group_access': {self.user_partition.id: [4]}},
'#children': [{'#type': 'vertical', '#ref': 'N'}],
},
{
@@ -224,32 +219,6 @@ class UserPartitionTransformerTestCase(UserPartitionTestMixin, CourseStructureTe
self.get_block_key_set(self.blocks, *expected_blocks)
)
def test_transform_with_content_gating_partition(self):
self.setup_partitions_and_course()
CourseModeFactory.create(course_id=self.course.id, mode_slug='audit')
CourseModeFactory.create(course_id=self.course.id, mode_slug='verified')
ContentTypeGatingConfig.objects.create(enabled=True, enabled_as_of=datetime(2018, 1, 1))
partition = create_content_gating_partition(self.course)
self.user_partitions.append(partition)
cohort = self.partition_cohorts[0][1]
add_user_to_cohort(cohort, self.user.username)
with patch(
'lms.djangoapps.course_blocks.transformers.user_partitions.get_partition_from_id',
return_value=partition
), patch(
'lms.djangoapps.course_blocks.transformers.user_partitions._MergedGroupAccess.get_allowed_groups',
return_value={51: set([])}
):
trans_block_structure = get_course_blocks(
self.user,
self.course.location,
self.transformers,
)
xblocks_denial_reason = [trans_block_structure.get_xblock_field(b, 'authorization_denial_reason')
for b in trans_block_structure.get_block_keys()]
self.assertSetEqual(set(xblocks_denial_reason), set([u'Feature-based Enrollments']))
def test_transform_on_inactive_partition(self):
"""
Tests UserPartitionTransformer for inactive UserPartition.

92
lms/djangoapps/course_blocks/transformers/user_partitions.py
View File

@@ -6,11 +6,7 @@ from openedx.core.djangoapps.content.block_structure.transformer import (
BlockStructureTransformer,
FilteringTransformerMixin
)
from xmodule.partitions.partitions_service import (
get_all_partitions_for_course,
get_partition_from_id,
get_user_partition_groups
)
from xmodule.partitions.partitions_service import get_user_partition_groups, get_all_partitions_for_course
from .split_test import SplitTestTransformer
from .utils import get_field_on_block
@@ -83,38 +79,15 @@ class UserPartitionTransformer(FilteringTransformerMixin, BlockStructureTransfor
return [block_structure.create_universal_filter()]
user_groups = get_user_partition_groups(usage_info.course_key, user_partitions, user, 'id')
for block_key in block_structure.topological_traversal():
transformer_block_field = block_structure.get_transformer_block_field(
block_key, self, 'merged_group_access'
)
access_denying_partition_id = transformer_block_field.get_access_denying_partition(
user_groups
)
access_denying_partition = get_partition_from_id(user_partitions, access_denying_partition_id)
if not has_access(user, 'staff', block_key) and access_denying_partition:
user_group = user_groups.get(access_denying_partition.id)
allowed_groups = transformer_block_field.get_allowed_groups()[access_denying_partition.id]
access_denied_message = access_denying_partition.access_denied_message(
block_key, user, user_group, allowed_groups
)
block_structure.override_xblock_field(
block_key, 'authorization_denial_reason', access_denying_partition.name
)
block_structure.override_xblock_field(
block_key, 'authorization_denial_message', access_denied_message
)
group_access_filter = block_structure.create_removal_filter(
lambda block_key: (
not has_access(user, 'staff', block_key) and
block_structure.get_transformer_block_field(
block_key, self, 'merged_group_access'
).get_access_denying_partition(user_groups) is not None and
block_structure.get_xblock_field(block_key, 'authorization_denial_message') is None
lambda block_key: not (
has_access(user, 'staff', block_key) or
block_structure.get_transformer_block_field(block_key, self, 'merged_group_access').check_group_access(
user_groups
)
)
)
result_list.append(group_access_filter)
return result_list
@@ -180,6 +153,7 @@ class _MergedGroupAccess(object):
# Set the default to universal access, for the case when
# there are no parents.
merged_parent_group_ids = None
if merged_parent_access_list:
# Set the default to most restrictive as we iterate
# through all the parent chains.
@@ -214,9 +188,6 @@ class _MergedGroupAccess(object):
if merged_group_ids is not None:
self._access[partition.id] = merged_group_ids
def get_allowed_groups(self):
return self._access
@staticmethod
def _intersection(*sets):
"""
@@ -239,34 +210,6 @@ class _MergedGroupAccess(object):
else:
return None
def get_access_denying_partition(self, user_groups):
"""
Arguments:
dict[int: Group]: Given a user, a mapping from user
partition IDs to the group to which the user belongs in
each partition.
Returns:
bool: Which partition is denying access
"""
for partition_id, allowed_group_ids in self._access.iteritems():
# If the user is not assigned to a group for this partition,
# return partition that would deny access.
if partition_id not in user_groups:
return partition_id
# If the user belongs to one of the allowed groups for this
# partition, then move and check the next partition.
elif user_groups[partition_id].id in allowed_group_ids:
continue
# Else, return partition that would deny access.
else:
return partition_id
# The user has access for every partition, return none
return None
def check_group_access(self, user_groups):
"""
Arguments:
@@ -277,4 +220,21 @@ class _MergedGroupAccess(object):
Returns:
bool: Whether said user has group access.
"""
return self.get_access_denying_partition(user_groups) is None
for partition_id, allowed_group_ids in self._access.iteritems():
# If the user is not assigned to a group for this partition,
# deny access.
if partition_id not in user_groups:
return False
# If the user belongs to one of the allowed groups for this
# partition, then move and check the next partition.
elif user_groups[partition_id].id in allowed_group_ids:
continue
# Else, deny access.
else:
return False
# The user has access for every partition, grant access.
return True

3
lms/djangoapps/courseware/access.py
View File

@@ -503,12 +503,11 @@ def _has_group_access(descriptor, user, course_key):
if missing_groups:
partition, user_group, allowed_groups = missing_groups[0]
block_key = descriptor.scope_ids.usage_id
return IncorrectPartitionGroupError(
partition=partition,
user_group=user_group,
allowed_groups=allowed_groups,
user_message=partition.access_denied_message(block_key, user, user_group, allowed_groups),
user_message=partition.access_denied_message(descriptor, user, user_group, allowed_groups),
user_fragment=partition.access_denied_fragment(descriptor, user, user_group, allowed_groups),
)

13
openedx/features/content_type_gating/partitions.py
View File

@@ -85,8 +85,8 @@ class ContentTypeGatingPartition(UserPartition):
if (verified_mode is None or not self._is_audit_enrollment(user, course_key) or
user_group == FULL_ACCESS):
return None
ecommerce_checkout_link = self._get_checkout_link(user, verified_mode.sku)
request = crum.get_current_request()
frag = Fragment(render_to_string('content_type_gating/access_denied_message.html', {
'mobile_app': request and is_request_from_mobile_app(request),
@@ -95,19 +95,14 @@ class ContentTypeGatingPartition(UserPartition):
}))
return frag
def access_denied_message(self, block_key, user, user_group, allowed_groups):
course_key = block_key.course_key
def access_denied_message(self, block, user, user_group, allowed_groups):
course_key = self._get_course_key_from_course_block(block)
modes = CourseMode.modes_for_course_dict(course_key)
verified_mode = modes.get(CourseMode.VERIFIED)
if (verified_mode is None or not self._is_audit_enrollment(user, course_key) or
user_group == FULL_ACCESS):
return None
request = crum.get_current_request()
if request and is_request_from_mobile_app(request):
return _(u"Graded assessments are available to Verified Track learners.")
else:
return _(u"Graded assessments are available to Verified Track learners. Upgrade to Unlock.")
return _(u"Graded assessments are available to Verified Track learners. Upgrade to Unlock.")
def _is_audit_enrollment(self, user, course_key):
"""

2
openedx/features/content_type_gating/tests/test_partitions.py
View File

@@ -119,7 +119,7 @@ class TestContentTypeGatingPartition(CacheIsolationTestCase):
):
fragment = partition.access_denied_fragment(mock_block, global_staff, FULL_ACCESS, 'test_allowed_group')
self.assertIsNone(fragment)
message = partition.access_denied_message(mock_block.scope_ids.usage_id, global_staff, FULL_ACCESS, 'test_allowed_group')
message = partition.access_denied_message(mock_block, global_staff, FULL_ACCESS, 'test_allowed_group')
self.assertIsNone(message)
def test_acess_denied_fragment_for_null_request(self):

1
setup.py
View File

@@ -63,7 +63,6 @@ setup(
"completion = lms.djangoapps.course_api.blocks.transformers.block_completion:BlockCompletionTransformer",
"load_override_data = lms.djangoapps.course_blocks.transformers.load_override_data:OverrideDataTransformer",
"content_type_gate = openedx.features.content_type_gating.block_transformers:ContentTypeGateTransformer",
"access_denied_message_filter = lms.djangoapps.course_blocks.transformers.access_denied_filter:AccessDeniedMessageFilterTransformer",
],
"openedx.ace.policy": [
"bulk_email_optout = lms.djangoapps.bulk_email.policies:CourseEmailOptout"