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

fix: no library context when not found (#27493)

Browse Source 
FAL-1848 (OpenCraft)
This commit is contained in:
Raul Gallegos
2021-05-28 12:01:35 -05:00
committed by GitHub
parent 7a6ac8642d
commit 24f9568c4e
5 changed files with 79 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
openedx/core/djangoapps/content_libraries/api.py
View File

@@ -325,7 +325,6 @@ def require_permission_for_library_key(library_key, user, permission):
Raises django.core.exceptions.PermissionDenied if the user doesn't have
permission.
"""
assert isinstance(library_key, LibraryLocatorV2)
library_obj = ContentLibrary.objects.get_by_key(library_key)
if not user.has_perm(permission, obj=library_obj):
raise PermissionDenied
@@ -338,7 +337,6 @@ def get_library(library_key):
Raises ContentLibraryNotFound if the library doesn't exist.
"""
assert isinstance(library_key, LibraryLocatorV2)
ref = ContentLibrary.objects.get_by_key(library_key)
bundle_metadata = get_bundle(ref.bundle_uuid)
lib_bundle = LibraryBundle(library_key, ref.bundle_uuid, draft_name=DRAFT_NAME)

4
openedx/core/djangoapps/content_libraries/library_context.py
View File

@@ -44,7 +44,7 @@ class LibraryContextImpl(LearningContext):
"""
try:
api.require_permission_for_library_key(usage_key.lib_key, user, permissions.CAN_EDIT_THIS_CONTENT_LIBRARY)
except PermissionDenied:
except (PermissionDenied, api.ContentLibraryNotFound):
return False
def_key = self.definition_for_usage(usage_key)
if not def_key:
@@ -67,7 +67,7 @@ class LibraryContextImpl(LearningContext):
api.require_permission_for_library_key(
usage_key.lib_key, user, permissions.CAN_LEARN_FROM_THIS_CONTENT_LIBRARY,
)
except PermissionDenied:
except (PermissionDenied, api.ContentLibraryNotFound):
return False
def_key = self.definition_for_usage(usage_key)
if not def_key:

1
openedx/core/djangoapps/content_libraries/tests/base.py
View File

@@ -40,6 +40,7 @@ URL_LIB_BLOCK_ASSET_FILE = URL_LIB_BLOCK + 'assets/{file_name}' # Get, delete,
URL_BLOCK_RENDER_VIEW = '/api/xblock/v2/xblocks/{block_key}/view/{view_name}/'
URL_BLOCK_GET_HANDLER_URL = '/api/xblock/v2/xblocks/{block_key}/handler_url/{handler_name}/'
URL_BLOCK_METADATA_URL = '/api/xblock/v2/xblocks/{block_key}/'
URL_BLOCK_XBLOCK_HANDLER = '/api/xblock/v2/xblocks/{block_key}/handler/{user_id}-{secure_token}/{handler_name}/'
# Decorator for tests that require blockstore

50
openedx/core/djangoapps/content_libraries/tests/test_content_libraries.py
View File

@@ -7,11 +7,20 @@ from unittest.mock import patch
import ddt
from django.conf import settings
from django.contrib.auth.models import Group
from django.test.client import Client
from django.test.utils import override_settings
from organizations.models import Organization
from rest_framework.test import APITestCase
from openedx.core.djangoapps.content_libraries.libraries_index import LibraryBlockIndexer, ContentLibraryIndexer
from openedx.core.djangoapps.content_libraries.tests.base import ContentLibrariesRestApiTest, elasticsearch_test
from openedx.core.djangoapps.content_libraries.tests.base import (
ContentLibrariesRestApiTest,
elasticsearch_test,
URL_BLOCK_METADATA_URL,
URL_BLOCK_RENDER_VIEW,
URL_BLOCK_GET_HANDLER_URL,
URL_BLOCK_XBLOCK_HANDLER,
)
from openedx.core.djangoapps.content_libraries.constants import VIDEO, COMPLEX, PROBLEM, CC_4_BY, ALL_RIGHTS_RESERVED
from common.djangoapps.student.tests.factories import UserFactory
@@ -766,3 +775,42 @@ class ContentLibrariesTest(ContentLibrariesRestApiTest):
assert types[0]['block_type'] == library_type
else:
assert len(types) > 1
@ddt.ddt
class ContentLibraryXBlockValidationTest(APITestCase):
"""Tests only focused on service validation, no Blockstore needed."""
@ddt.data(
(URL_BLOCK_METADATA_URL, dict(block_key='totally_invalid_key')),
(URL_BLOCK_RENDER_VIEW, dict(block_key='totally_invalid_key', view_name='random')),
(URL_BLOCK_GET_HANDLER_URL, dict(block_key='totally_invalid_key', handler_name='random')),
)
@ddt.unpack
def test_invalid_key(self, endpoint, endpoint_parameters):
"""Test all xblock related endpoints, when the key is invalid, return 404."""
response = self.client.get(
endpoint.format(**endpoint_parameters),
)
self.assertEqual(response.status_code, 404)
self.assertEqual(response.json(), {'detail': "Invalid XBlock key"})
def test_xblock_handler_invalid_key(self):
"""This endpoint is tested separately from the previous ones as it's not a DRF endpoint."""
client = Client()
response = client.get(URL_BLOCK_XBLOCK_HANDLER.format(**dict(
block_key='totally_invalid_key',
handler_name='random',
user_id='random',
secure_token='random',
)))
self.assertEqual(response.status_code, 404)
def test_not_found_fails_correctly(self):
"""Test fails with 404 when xblock key is valid but not found."""
valid_not_found_key = 'lb:valid:key:video:1'
response = self.client.get(URL_BLOCK_METADATA_URL.format(block_key=valid_not_found_key))
self.assertEqual(response.status_code, 404)
self.assertEqual(response.json(), {
'detail': f"XBlock {valid_not_found_key} does not exist, or you don't have permission to view it.",
})

33
openedx/core/djangoapps/xblock/rest_api/views.py
View File

@@ -7,14 +7,16 @@ Studio APIs cover use cases like adding/deleting/editing blocks.
from corsheaders.signals import check_request_enabled
from django.contrib.auth import get_user_model
from django.http import Http404
from django.views.decorators.clickjacking import xframe_options_exempt
from django.views.decorators.csrf import csrf_exempt
from rest_framework import permissions
from rest_framework.decorators import api_view, permission_classes # lint-amnesty, pylint: disable=unused-import
from rest_framework.exceptions import PermissionDenied, AuthenticationFailed
from rest_framework.exceptions import PermissionDenied, AuthenticationFailed, NotFound
from rest_framework.response import Response
from xblock.django.request import DjangoWebobRequest, webob_to_django_response
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import UsageKey
from openedx.core.lib.api.view_utils import view_auth_classes
from ..api import (
@@ -22,13 +24,16 @@ from ..api import (
get_handler_url as _get_handler_url,
load_block,
render_block_view as _render_block_view,
)
from ..utils import validate_secure_token_for_xblock_handler
User = get_user_model()
class InvalidNotFound(NotFound):
default_detail = "Invalid XBlock key"
@api_view(['GET'])
@view_auth_classes(is_authenticated=False)
@permission_classes((permissions.AllowAny, )) # Permissions are handled at a lower level, by the learning context
@@ -39,7 +44,11 @@ def block_metadata(request, usage_key_str):
Accepts an "include" query parameter which must be a comma separated list of keys to include. Valid keys are
"index_dictionary" and "student_view_data".
"""
usage_key = UsageKey.from_string(usage_key_str)
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise InvalidNotFound from e
block = load_block(usage_key, request.user)
includes = request.GET.get("include", "").split(",")
metadata_dict = get_block_metadata(block, includes=includes)
@@ -57,7 +66,11 @@ def render_block_view(request, usage_key_str, view_name):
"""
Get the HTML, JS, and CSS needed to render the given XBlock.
"""
usage_key = UsageKey.from_string(usage_key_str)
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise InvalidNotFound from e
block = load_block(usage_key, request.user)
fragment = _render_block_view(block, view_name, request.user)
response_data = get_block_metadata(block)
@@ -74,7 +87,11 @@ def get_handler_url(request, usage_key_str, handler_name):
The URL will expire but is guaranteed to be valid for a minimum of 2 days.
"""
usage_key = UsageKey.from_string(usage_key_str)
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise InvalidNotFound from e
handler_url = _get_handler_url(usage_key, handler_name, request.user)
return Response({"handler_url": handler_url})
@@ -93,7 +110,10 @@ def xblock_handler(request, user_id, secure_token, usage_key_str, handler_name,
auth token included in the URL (see below). As a result it can be exempt
from CSRF, session auth, and JWT/OAuth.
"""
usage_key = UsageKey.from_string(usage_key_str)
try:
usage_key = UsageKey.from_string(usage_key_str)
except InvalidKeyError as e:
raise Http404 from e
# To support sandboxed XBlocks, custom frontends, and other use cases, we
# authenticate requests using a secure token in the URL. see
@@ -146,4 +166,5 @@ def cors_allow_xblock_handler(sender, request, **kwargs): # lint-amnesty, pylin
"""
return request.path.startswith('/api/xblock/v2/xblocks/') and '/handler/' in request.path
check_request_enabled.connect(cors_allow_xblock_handler)