Merge pull request #18137 from edx/efischer/nvm

Revert #18125
This commit is contained in:
Eric Fischer
2018-05-04 17:50:38 -04:00
committed by GitHub
2 changed files with 49 additions and 98 deletions

View File

@@ -95,7 +95,7 @@ def has_ccx_coach_role(user, course_key):
return False
def has_access(user, action, obj, course_key=None, prefetched_group_data=None):
def has_access(user, action, obj, course_key=None):
"""
Check whether a user has the access to do action on obj. Handles any magic
switching based on various settings.
@@ -150,7 +150,7 @@ def has_access(user, action, obj, course_key=None, prefetched_group_data=None):
# NOTE: any descriptor access checkers need to go above this
if isinstance(obj, XBlock):
return _has_access_descriptor(user, action, obj, course_key, prefetched_group_data)
return _has_access_descriptor(user, action, obj, course_key)
if isinstance(obj, CourseKey):
return _has_access_course_key(user, action, obj)
@@ -399,21 +399,7 @@ def _has_access_error_desc(user, action, descriptor, course_key):
return _dispatch(checkers, action, user, descriptor)
def _partition_and_user_groups_prefetch(prefetched_group_data, merged_access):
"""Compute the needed partition_groups and user_groups structures using prefetched data."""
partition_groups = {
partition_id: set(group_ids).intersection(
set(prefetched_group_data.get(partition_id, {}).get('all_group_ids', []))
) for partition_id, group_ids in merged_access.iteritems()
}
user_groups = {
partition_id: prefetched_group_data.get(partition_id, {}).get('user_group_id')
for partition_id in merged_access.iterkeys()
}
return partition_groups, user_groups
def _has_group_access(descriptor, user, course_key, prefetched_group_data):
def _has_group_access(descriptor, user, course_key):
"""
This function returns a boolean indicating whether or not `user` has
sufficient group memberships to "load" a block (the `descriptor`)
@@ -431,63 +417,60 @@ def _has_group_access(descriptor, user, course_key, prefetched_group_data):
log.warning("Group access check excludes all students, access will be denied.", exc_info=True)
return ACCESS_DENIED
partition_groups = {}
user_groups = {}
if prefetched_group_data:
# Use prefetched data, if possible. See EDUCATOR-2618 for context.
partition_groups, user_groups = _partition_and_user_groups_prefetch(prefetched_group_data, merged_access)
else:
# resolve the partition IDs in group_access to actual
# partition objects, skipping those which contain empty group directives.
# If a referenced partition could not be found, it will be denied
# If the partition is found but is no longer active (meaning it's been disabled)
# then skip the access check for that partition.
partitions = []
for partition_id, group_ids in merged_access.iteritems():
try:
partition = descriptor._get_user_partition(partition_id) # pylint: disable=protected-access
if partition.active:
if group_ids is not None:
partitions.append(partition)
else:
log.debug(
"Skipping partition with ID %s in course %s because it is no longer active",
partition.id, course_key
)
except NoSuchUserPartitionError:
log.warning("Error looking up user partition, access will be denied.", exc_info=True)
return ACCESS_DENIED
# next resolve the group IDs specified within each partition
# resolve the partition IDs in group_access to actual
# partition objects, skipping those which contain empty group directives.
# If a referenced partition could not be found, it will be denied
# If the partition is found but is no longer active (meaning it's been disabled)
# then skip the access check for that partition.
partitions = []
for partition_id, group_ids in merged_access.items():
try:
for partition in partitions:
groups = [
partition.get_group(group_id)
for group_id in merged_access[partition.id]
]
if groups:
partition_groups[partition.id] = [group.id for group in groups]
# look up the user's group for each partition
user_group = partition.scheme.get_group_for_user(
course_key,
user,
partition,
)
user_groups[partition.id] = user_group.id if user_group else None
except NoSuchUserPartitionGroupError:
log.warning("Error looking up referenced user partition group, access will be denied.", exc_info=True)
partition = descriptor._get_user_partition(partition_id) # pylint: disable=protected-access
if partition.active:
if group_ids is not None:
partitions.append(partition)
else:
log.debug(
"Skipping partition with ID %s in course %s because it is no longer active",
partition.id, course_key
)
except NoSuchUserPartitionError:
log.warning("Error looking up user partition, access will be denied.", exc_info=True)
return ACCESS_DENIED
# next resolve the group IDs specified within each partition
partition_groups = []
try:
for partition in partitions:
groups = [
partition.get_group(group_id)
for group_id in merged_access[partition.id]
]
if groups:
partition_groups.append((partition, groups))
except NoSuchUserPartitionGroupError:
log.warning("Error looking up referenced user partition group, access will be denied.", exc_info=True)
return ACCESS_DENIED
# look up the user's group for each partition
user_groups = {}
for partition, groups in partition_groups:
user_groups[partition.id] = partition.scheme.get_group_for_user(
course_key,
user,
partition,
)
# finally: check that the user has a satisfactory group assignment
# for each partition.
if not all(user_groups.get(partition_id) in group_ids for partition_id, group_ids in partition_groups.iteritems()):
if not all(user_groups.get(partition.id) in groups for partition, groups in partition_groups):
return ACCESS_DENIED
# all checks passed.
return ACCESS_GRANTED
def _has_access_descriptor(user, action, descriptor, course_key=None, prefetched_group_data=None):
def _has_access_descriptor(user, action, descriptor, course_key=None):
"""
Check if user has access to this descriptor.
@@ -510,7 +493,7 @@ def _has_access_descriptor(user, action, descriptor, course_key=None, prefetched
# access to this content, then deny access. The problem with calling _has_staff_access_to_descriptor
# before this method is that _has_staff_access_to_descriptor short-circuits and returns True
# for staff users in preview mode.
if not _has_group_access(descriptor, user, course_key, prefetched_group_data):
if not _has_group_access(descriptor, user, course_key):
return ACCESS_DENIED
# If the user has staff access, they can load the module and checks below are not needed.

View File

@@ -4,7 +4,7 @@ from collections import defaultdict
from datetime import datetime
from django.conf import settings
from django.contrib.auth.models import AnonymousUser, User
from django.contrib.auth.models import User
from django.core.urlresolvers import reverse
from django.db import connection
from django.http import HttpResponse
@@ -131,45 +131,13 @@ def get_accessible_discussion_xblocks_by_course_id(course_id, user=None, include
Checks for the given user's access if include_all is False.
"""
all_xblocks = modulestore().get_items(course_id, qualifiers={'category': 'discussion'}, include_orphans=False)
if not all_xblocks:
return []
prefetched_group_data = prefetch_group_data(user, all_xblocks[0], course_id)
return [
xblock for xblock in all_xblocks
if (
has_required_keys(xblock) and (
include_all or
has_access(user, 'load', xblock, course_id, prefetched_group_data=prefetched_group_data)
)
)
if has_required_keys(xblock) and (include_all or has_access(user, 'load', xblock, course_id))
]
def prefetch_group_data(user, sample_block, course_id):
"""
Return a dict containing:
- id of every partition in the course, mapped to:
- all of their corresponding group_ids
- the group_id appropriate for user
"""
all_partitions = sample_block.runtime.service(sample_block, 'partitions').course_partitions
if not user:
user = AnonymousUser()
def _group_id_or_none(partition, course_id, user):
"""If an appropriate group for this user exists, return its id. Else return None."""
group = partition.scheme.get_group_for_user(course_id, user, partition)
return group.id if group else None
return {
partition.id: {
'all_group_ids': [group.id for group in partition.groups],
'user_group_id': _group_id_or_none(partition, course_id, user),
} for partition in all_partitions if partition.active
}
def get_discussion_id_map_entry(xblock):
"""
Returns a tuple of (discussion_id, metadata) suitable for inclusion in the results of get_discussion_id_map().